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

Change cnonce values more frequently #50

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@gabrielsjoberg
Contributor

gabrielsjoberg commented Nov 9, 2012

When using only 1 second precision, curl doesn't change cnonce values frequently enough for all uses.

For example, issuing the following command multiple times within a few seconds to a recent Tomcat causes authentication failures (after one success):

curl --digest -utest:test http://tomcat.test.com:8080/manager/list

This is because curl uses the same cnonce for several seconds, but doesn't increment the nonce counter.  Tomcat correctly interprets this as a replay attack and rejects the request.

When microsecond-precision is available, this commit causes curl to change cnonce values much more frequently.

Added microseconds into cnonce calculation.
When using only 1 second precision, curl doesn't create new cnonce values quickly enough for all uses.

For example, issuing the following command multiple times to a recent Tomcat causes authentication failures:

curl --digest -utest:test http://tomcat.test.com:8080/manager/list

This is because curl uses the same cnonce for several seconds, but doesn't increment the nonce counter.  Tomcat correctly interprets this as a replay attack and rejects the request.

When microsecond-precision is available, this commit causes curl to change cnonce values much more frequently.
@bagder

This comment has been minimized.

Member

bagder commented Nov 10, 2012

Oh what an excellent fix. But with adding microseconds there, don't you also think we should extend the nonce string to be longer than just 6 letters?

@gabrielsjoberg

This comment has been minimized.

Contributor

gabrielsjoberg commented Nov 10, 2012

Yeah, I was thinking that it was a bit short, but then I wouldn't be able to claim a one-line fix. :-D

If the cnonce changes enough (has enough entropy, if we want to get formal), then the length shouldn't matter too much. A couple of quick Google searches show that there isn't really any consensus on cnonce length and RFC 2617 is silent on the matter. Most implementations seem to use around 8 bytes, but Java uses 40. I like powers of 2 myself, so how does 32 bytes sound?

@bagder

This comment has been minimized.

Member

bagder commented Nov 10, 2012

32 sounds perfect to me! :-)

@bagder

This comment has been minimized.

Member

bagder commented Nov 12, 2012

I edited your fix to also extend the length to 32 characters and pushed it just now as commit e237402. Thanks a lot!

@bagder bagder closed this Nov 12, 2012

@gabrielsjoberg

This comment has been minimized.

Contributor

gabrielsjoberg commented Nov 12, 2012

Ah, dang. I was working on a slightly different approach and forgot to withdraw my pull request. What you committed should work, but since long integers will never fill the 32-byte buffer (on architectures using integers smaller than 128 bits, anyways), we'll have a lot of base64-encoded zeroes in the cnonce.

If you'd like, I can provide a different method that scatters the bits over the entire 32-byte buffer before encoding it to base64.

@bagder

This comment has been minimized.

Member

bagder commented Nov 12, 2012

oh right, 32 bytes is a bit overkill as it works right now... yeah, I would be interested in your method!

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