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

auth: compute user:realm:pass digest w/o userhash

Closed
wants to merge 1 commit into from

Conversation

gstrauss
Copy link
Contributor

@gstrauss gstrauss commented Nov 28, 2021

auth: compute user:realm:pass digest w/o userhash

https://datatracker.ietf.org/doc/html/rfc7616#section-3.4.4
... the client MUST calculate a hash of the username after
any other hash calculation ...

Signed-off-by: Glenn Strauss gstrauss@gluelogic.com


I am implementing HTTP Digest Auth userhash support in lighttpd and I read RFC 7616 Section 3.4.4 differently from how it is currently implemented in cURL from 2b5b37c. The patch in this PR works with my development version of lighttpd, and I am looking for confirmation on whether or not this is an issue in curl, or if I have misunderstood RFC 7616 Section 3.4.4. Thanks.

@gstrauss
Copy link
Contributor Author

gstrauss commented Nov 28, 2021

I am looking for feedback before I update the curl digest calculations in the curl HTTP Digest auth tests with userhash=true

TESTFAIL: These test cases failed: 2059 2063 2066 2069

https://datatracker.ietf.org/doc/html/rfc7616#section-3.4.4
  ... the client MUST calculate a hash of the username after
      any other hash calculation ...

Signed-off-by: Glenn Strauss <gstrauss@gluelogic.com>
@gstrauss
Copy link
Contributor Author

gstrauss commented Nov 28, 2021

I updated tests to make this PR complete. The hashes in the previous code matched sha256sum output, not shasum -a 512256, so I updated the algorithm from SHA-512-256 to instead be SHA-256.

@gstrauss
Copy link
Contributor Author

gstrauss commented Nov 28, 2021

To be clear, the existing code calculates HA1 as H( userhash ":" realm ":" password ) and I believe that HA1 should be calculated as H( username ":" realm ":" password ), the same as with userhash=false (the default if the userhash param is omitted).

@gstrauss
Copy link
Contributor Author

gstrauss commented Nov 28, 2021

Manually calculating the response in the example in https://datatracker.ietf.org/doc/html/rfc7616#section-3.9.2 suggests that my intepretation of RFC 7616 userhash is correct H( username ":" realm ":" password ) -- not userhash which curl incorrectly uses and which this PR fixes.

bagder
bagder approved these changes Nov 29, 2021
Copy link
Member

@bagder bagder left a comment

Looks correct to me!

lighttpd-git pushed a commit to lighttpd/lighttpd1.4 that referenced this issue Nov 29, 2021
RFC7616 HTTP Digest username* and userhash support (if configured)

userhash support must be configured to enable:
  auth.require = ( "/" => ( "userhash" => "enable", ... ) )
and one of
  auth.backend = "htdigest"  # mod_authn_file
    or
  auth.backend = "dbi"       # mod_authn_dbi
and appropriate modification to add userhash into htdigest or db table
along with adding "sql-userhash" => "..." SQL query for mod_authn_dbi

Note: open issue with curl preventing userhash from working with curl:
  curl/curl#8066
@bagder
Copy link
Member

bagder commented Nov 30, 2021

Thanks!

@bagder bagder closed this in aae235b Nov 30, 2021
sftcd pushed a commit to sftcd/lighttpd1.4 that referenced this issue Aug 18, 2022
RFC7616 HTTP Digest username* and userhash support (if configured)

userhash support must be configured to enable:
  auth.require = ( "/" => ( "userhash" => "enable", ... ) )
and one of
  auth.backend = "htdigest"  # mod_authn_file
    or
  auth.backend = "dbi"       # mod_authn_dbi
and appropriate modification to add userhash into htdigest or db table
along with adding "sql-userhash" => "..." SQL query for mod_authn_dbi

Note: open issue with curl preventing userhash from working with curl:
  curl/curl#8066
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants