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

fix DigestAuth default value for qop field #2600

Closed
wants to merge 2 commits into from
Closed

Conversation

skewty
Copy link

@skewty skewty commented Feb 22, 2023

Make the default qop value the same as Chrome, curl and others.

Without this some servers will obviously give status 401.

Make the default qop value the same as Chrome, curl and others.
Without this some servers will obviously give status 401.
@skewty
Copy link
Author

skewty commented Feb 22, 2023

if you will accept this change, I will adjust the tests as well.

@tomchristie
Copy link
Member

Make the default qop value the same as Chrome, curl and others.

Are you able to show us how to confirm this?

Using this command:

$ curl -v  'https://jigsaw.w3.org/HTTP/Digest/' --digest -u guest:guest

I see the following header...

> Authorization: Digest username="guest", realm="test", nonce="e4f988c95c2f85eb3e8501b313f76576", uri="/HTTP/Digest/", response="f8d36a13eaf246e1eb5268c7806a294a" 

Without this some servers will obviously give status 401.

Are you able to provide an example against a public URL?

@skewty
Copy link
Author

skewty commented Feb 23, 2023

I am not aware of any public URLs as Digest auth isn't so popular anymore.
I encountered this issue working with some off the shelf SIP telephone devices.

Here is an example:

$ curl --digest -u Admin:456 http://192.168.1.141/polling/deviceHandler
<PolycomIPPhone>
<DeviceInformation>
<MACAddress>00907a147020</MACAddress>
<PhoneDN>Line1:2009</PhoneDN>
<AppLoadID>5.0.0.2079 06-Sep-16 08:24</AppLoadID>
<UpdaterID>5.0.0.2079</UpdaterID>
<ModelNumber>Spectralink 8440</ModelNumber>
<TimeStamp>2023-02-23T09:15:51-08:00</TimeStamp>
</DeviceInformation>
$ python
>>> import httpx
>>> httpx.get("http://192.168.1.141/polling/deviceHandler", auth=httpx.DigestAuth("Admin", "456"))
<Response [401 Unauthorized]>

I also used my JetBrains PyCharm Pro IDE built in HTTP client and it doesn't give 401 either.

# Get Firmware Version
GET http://192.168.1.141/polling/deviceHandler
Authorization: Digest Admin 456

###

I also tried Brave (Chrome) and it brought up a dialog to enter username + password and didn't give 401 either.

I can do a Zoom / Teams / Code with Me call with you to show this isn't cheating with the copy and paste.

@skewty
Copy link
Author

skewty commented Mar 2, 2023

@tomchristie any decision on this yet?

@tomchristie
Copy link
Member

From my reading of https://httpwg.org/specs/rfc7616.html#authorization.request.header.field I think you've got this correct.

The RFC states that...

qop: This parameter MUST be used by all implementations.

Are you able to show use --verbose what headers you're getting in the requests and responses?
Presumably these are cases where the server is incorrectly omitting the qop?

In any case, yes it looks to me like a more robust behaviour would be treating an omitted qop as having a value of "auth".

It looks like the typing of this...

class _DigestAuthChallenge(typing.NamedTuple):
    realm: bytes
    nonce: bytes
    algorithm: str
    opaque: typing.Optional[bytes]
    qop: typing.Optional[bytes]  # No longer optional

...would need to be updated, and that we've got a couple of code branches checking if qop is None that can be removed.

@skewty
Copy link
Author

skewty commented Apr 9, 2023

Here is the command I used:

curl http://192.168.1.141/push -vv -d "<PolycomIPPhone><Data priority=\"Normal\">Testing</Data></PolycomIPPhone>" --digest -u Admin:456

Here is the output from the command:

*   Trying 192.168.1.141:80...
* Connected to 192.168.1.141 (192.168.1.141) port 80 (#0)
* Server auth using Digest with user 'Admin'
> POST /push HTTP/1.1
> Host: 192.168.1.141
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 0
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 401 Unauthorized
< Server: Wireless IP Telephone HTTPd
< Date: Sun, 09 Apr 2023 15:59:45 GMT
< Connection: close
< WWW-Authenticate: Digest realm="PUSH Authentication", nonce="168105334", algorithm="MD5"
< Content-type: text/html
<
* Closing connection 0
* Issue another request to this URL: 'http://192.168.1.141/push'
* Hostname 192.168.1.141 was found in DNS cache
*   Trying 192.168.1.141:80...
* Connected to 192.168.1.141 (192.168.1.141) port 80 (#1)
* Server auth using Digest with user 'Admin'
> POST /push HTTP/1.1
> Host: 192.168.1.141
> Authorization: Digest username="Admin", realm="PUSH Authentication", nonce="168105334", uri="/push", response="b5d6559d6391c2f193d82fb2f261349d", algorithm=MD5
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 71
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Server: Wireless IP Telephone HTTPd
< Date: Sun, 09 Apr 2023 15:59:47 GMT
< Connection: keep-alive
< Content-length: 43
< Content-type: text/html
<
* Connection #1 to host 192.168.1.141 left intact
Push Message will be displayed successfully

Comments

The above command / operation worked as expected / designed; "Testing" was displayed on the screen of the phone / device.

The qop field was indeed not included in the header from the handset but curl assumed it had value auth and authentication worked.

@tomchristie
Copy link
Member

if you will accept this change, I will adjust the tests as well.

I think this is worth progressing.

It looks to me like there's a couple of other changes this will impact...

class _DigestAuthChallenge(typing.NamedTuple):
    realm: bytes
    nonce: bytes
    algorithm: str
    opaque: typing.Optional[bytes]
    qop: typing.Optional[bytes]  # <--- no longer optional

And...

        if qop is None:
            digest_data = [HA1, challenge.nonce, HA2]  # <--- Assume this branch can no longer be followed?
        else:
            digest_data = [challenge.nonce, nc_value, cnonce, qop, HA2]

@the-ress
Copy link
Contributor

the-ress commented Jan 7, 2024

TLDR: This is not the right fix; I believe it's the same issue #3045 solves.

Unspecified qop needs different behavior than qop=auth. The best explanation I could find is on the digest auth wiki page.

In short, there are two variants of digest auth - old one from RFC 2069, and a newer one from RFC 2617 (later updated in RFC 7616). The hash in constructed differently depending on which one is being used. Clients can differentiate by the presence of the qop parameter.

        if qop is None:
            digest_data = [HA1, challenge.nonce, HA2]  # This branch is RFC 2069
        else:
            digest_data = [challenge.nonce, nc_value, cnonce, qop, HA2] # This branch is RFC 2617/7616

Curl's doing the same thing: https://github.com/curl/curl/blob/8edcfedc1a144f438bd1cdf814a0016cbe678aaf/lib/vauth/digest.c#L793-L799

It looks like your SIP phone's using the older variant (2069). RFC 2069 implementation in httpx is currently broken, and #3045 fixes it.

@tomchristie
Copy link
Member

✨ thanks for getting a handle on this @the-ress. ✨

@tomchristie tomchristie closed this Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants