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

DoS with long password #82

Open
Sc00bz opened this issue Oct 12, 2018 · 5 comments
Open

DoS with long password #82

Sc00bz opened this issue Oct 12, 2018 · 5 comments

Comments

@Sc00bz
Copy link

Sc00bz commented Oct 12, 2018

If you enter a long password it will take significantly longer. This runs in O(pwLen * rounds) time instead of in O(pwLen + rounds) time.

Ideally you'd want to do a cached HMAC for a 2x speed increase (on normal sized passwords):

var cachedCtx = createHmac(digest, password)

...

-var T = createHmac(digest, password).update(block1).digest()
+var T = "cachedCtx.clone()".update(block1).digest()

...

-U = createHmac(digest, password).update(U).digest()
+U = "cachedCtx.clone()".update(U).digest()

Their are some problems with the "create-hmac" package and once those are fixed cached HMAC will be the best way to go. See browserify/createHmac#27. Also I do not know the proper way to clone an object in Node.js. Thus the quotes around cachedCtx.clone().

@calvinmetcalf
Copy link
Contributor

ok so the fist big issue is that you can't actually clone node crypto objects like this, but there is a reason we aren't actually using createHmac here and it's becasue it has a bunch of overhead related to streaming object creation that we didn't want which means that we probably can fix this in the library.

that being said looking at how we use the hmac and how the hmac works are you sure that applies here ?

@Sc00bz
Copy link
Author

Sc00bz commented Oct 12, 2018

Sorry I removed the reference to the lines of code https://github.com/crypto-browserify/pbkdf2/blob/master/lib/sync.js#L36-L40

@calvinmetcalf
Copy link
Contributor

calvinmetcalf commented Oct 13, 2018 via email

@calvinmetcalf
Copy link
Contributor

This library is pretty complex as it handles several different scenarios:

  • in node.js but an old one that doesn't support some of the newer features
  • the browser in synchronous mode or in browser that doesn't support webcypto
  • the browser asynchronously via web crypto

so it's not necessarily obvious at first glance what code path will be taken it what environment but I believe I actually tested this back in the day and it was faster to create a node.js hmac object then to use a non streaming one because the node.js one would use a native hmac function while in the browser it wouldn't.

@webmaster128
Copy link
Contributor

webmaster128 commented Jun 3, 2020

Unfortunately, the Hmac class does not have the copy method as the Hash class:

Bildschirmfoto 2020-06-03 um 23 28 49

But what could be pulled out of the loop is this key normalization https://github.com/crypto-browserify/pbkdf2/blob/v3.1.0/lib/sync-browser.js#L26-L30, such that the native hmac always gets the correct key length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants