[Yanel-dev] Very poor performance of latest Yarep/Security Code

Michael Wechner michael.wechner at wyona.com
Fri Aug 19 10:00:21 EDT 2016


Hi Balz

Thanks very much for your debugging and patch!

I am back now from vacation and will have a look at your patch more
closely very soon and will keep you posted when integrating it.

All the best

Michael

Am 12.08.16 um 15:16 schrieb basZero:
> Hi Michael,
>
> I've implemented the following which works as expected:
> - fast performance, equal to the yanel version before the introduction
> of BCrypt
> - also does the autoupdate of passwords in old formats
>
> Code Changes in org.wyona.security.impl.yarep.YarepUser.java:
>
> Constructor public YarepUser(UserManager userManager, GroupManager
> groupManager, Node node):
> I just removed this section:
> if(hashingAlgorithm != null && !hashingAlgorithm.startsWith("bcrypt")) {
>   upgradeDoubleHash(this.hashedPassword, this.hashingAlgorithm);
> }
>
> and inserted it into a new overriding method:
>
> @Override
> public void save() throws AccessManagementException {
>     try {
>         // Check if we need to upgrade the password hash
>         if(hashingAlgorithm != null &&
> !hashingAlgorithm.startsWith("bcrypt")) {
>             upgradeDoubleHash(this.hashedPassword, this.hashingAlgorithm);
>         }
>     } catch (Exception e) {
>     }
>     super.save();
> }
>
>
> The result is obvious, only if the user object gets saved and the
> password is not yet in the new format (bcrypt), the hashed passwords
> gets upgraded and then saved (calling save() of the YarepItem class).
>
> This seems to me a very solid solution.
>
> What do you think?
>
> Do you need a git pull request for this?
>
> This is NOT URGENT for me personally, as I have overwritten the class
> YarepUser in my realm, but I think it is an essential patch for yanel
> and users who are using yanel with a high user base.
>
> Cheers, 
> Balz
>
>
>
>
> On Fri, Aug 12, 2016 at 2:45 PM, basZero <baszero at gmail.com
> <mailto:baszero at gmail.com>> wrote:
>
>     Hi Michael,
>
>     I found the root cause for the very poor performance!
>     As you know of course, the user's passwords are encrypted and
>     generated with the newly introduced bcrypt algorithm.
>
>     When I did the upgrade to the latest Yanel version I was aware of
>     this upgrade, however I somehow assumed that the password-update
>     in the user XML would only be updated at LOGIN or when the user
>     XML gets saved.
>
>     My assumption was wrong.
>
>     In the YarepUser.java class file I see in the constructor, that
>     for each user object that gets created (even if it is created for
>     read-only purposes), it is checked whether the password is stored
>     in another format than bcrypt, and if so, the new password hash
>     gets generated. And THIS consumes TIME!
>
>     Code Line:
>     upgradeDoubleHash(this.hashedPassword, this.hashingAlgorithm);
>
>     I think it is a very bad approach to put that logic in the
>     constructor, because reading an object means that you want to get
>     an object as represented by a stored XML.
>
>     A better way would be to put the logic into the save-logic of a
>     user, because then it makes sense to store the new hashed password.
>
>     What do you think?
>
>     I will now code a workaround and let you know where I put it. 
>
>     Cheers, Balz
>
>     On Thu, Aug 11, 2016 at 7:49 PM, Michael Wechner
>     <michael.wechner at wyona.com <mailto:michael.wechner at wyona.com>> wrote:
>
>         Hi Balz
>
>         I am currently on vacation (until August 17th), but what you
>         describe sounds really not good at all.
>
>         I will have a look at it as soon as I will be back. Please
>         keep us posted in case you find something out in the meantime.
>
>         Thanks
>
>         Michael
>
>         Am 11.08.16 um 09:59 schrieb basZero:
>>         Hi,
>>
>>         please read this with high priority as it could be very
>>         critical to performance of Yanel with many users:
>>         https://github.com/wyona/security/issues/6
>>         <https://github.com/wyona/security/issues/6>
>>
>>         Cheers, Bas
>>
>>
>
>
>         --
>         Yanel-development mailing list Yanel-development at wyona.com
>         <mailto:Yanel-development at wyona.com>
>         http://mx2.wyona.com/cgi-bin/mailman/listinfo/yanel-development <http://mx2.wyona.com/cgi-bin/mailman/listinfo/yanel-development>
>
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mx2.wyona.com/pipermail/yanel-development/attachments/20160819/20d0ebc7/attachment.html>


More information about the Yanel-development mailing list