Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #21253 -- PBKDF2 with cached HMAC key #1638

Closed
wants to merge 1 commit into from

Conversation

Sc00bz
Copy link

@Sc00bz Sc00bz commented Sep 17, 2013

This gives a 2x speed increase and removes the ability to DoS with large passwords.

Sorry this is my hello world for Python, and I don't even know how to run it. It probably works but you should test it.

https://code.djangoproject.com/ticket/21253

This gives a 2x speed increase and removes the ability to DoS with large passwords.

Sorry this is my hello world for Python, and I don't even know how to run it. It probably works but you should test it.
@apollo13
Copy link
Member

Hi @Sc00bz, thank you for you pull request, we are already aware of this. For the next time please report security sensitive stuff to security@djangoproject.com like outlined in https://djangoproject.com/security/

@apollo13 apollo13 closed this Sep 18, 2013
@Sc00bz
Copy link
Author

Sc00bz commented Sep 29, 2013

Just thought I would let you know that I benchmarked this code vs #1653 and this code is about 50% faster (84.6 ms vs 125 ms for 10,000 iterations). So you might want to reconsider this. Also I checked that it gives the same output.

@apollo13
Copy link
Member

Hi Steve, thx will take a look at it. Could you provide your
benchmarkscript so we can rule out errors there?
On Sep 29, 2013 4:33 AM, "Steve Thomas" notifications@github.com wrote:

Just thought I would let you know that I benchmarked this code vs #1653https://github.com/django/django/pull/1653and this code is about 50% faster (84.6 ms vs 125 ms for 10,000
iterations). So you might want to reconsider this. Also I checked that it
gives the same output.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1638#issuecomment-25313084
.

@apollo13
Copy link
Member

Ok, I think the copy saves a few cpu cycles every round. That said, getting rid of _cached_hmac completely and unrolling it into the main function might be a bit faster too, I doubt that later is worth it though. I am waiting for feedback from our security guys to see if they can see any issues with your implementation (I personally can't find one)

@Sc00bz
Copy link
Author

Sc00bz commented Sep 29, 2013

This takes around 20 seconds to run: http://pastebin.com/kaxN2pV3
I removed _cached_hmac and it got a little faster 80.5 ms.

The reason this is faster is with HMAC if you don't cache the results from the inner and outer pads those need to be recalculated each time. This does 20,002 SHA256 blocks vs 40,000 SHA256 blocks for 10,000 iterations of PBKDF2.

@timgraham
Copy link
Member

Rebased to apply cleanly: #1724

@timgraham timgraham closed this Oct 11, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants