[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