nsIHttpChannel not trying to authenticate if presented BASIC and an unknown auth method

[john.bieling--- via dev-platform]

Hi,

today I wanted to authenticate a PROPFIND against
https://contacts.icloud.com

which returns the following WWW-Authentication header:
WWW-Authenticate: X-MobileMe-AuthToken realm="Newcastle", Basic realm="Newcastle"

Usually, on a fresh/new connection, nsIHttpChannel will first do an unauthenticated request, get the auth methods to pick one, call promptAuth callback to ask for password and then initiate a second request to try to actually authenticate.

However, it looks like, nsIHttpChannel is not calling promptAuth and is not doing that second request. As if the there is no valid auth method returned by the server.

I replicated that WWW-Authentication header on one of my own servers, reproduced the findings and than removed the X-MobileMe-AuthToken from the allowed methods (so just sending Basic) and tried again to connect and this time it went through as expected.

Is this a bug?

[Boris Zbarsky]

Post by john.bieling--- via dev-platform
WWW-Authenticate: X-MobileMe-AuthToken realm="Newcastle", Basic realm="Newcastle"

I expect this would work if you sent it as:

WWW-Authenticate: X-MobileMe-AuthToken realm="Newcastle"
WWW-Authenticate: Basic realm="Newcastle"

Yes, per spec they are theoretically supposed to be the same. But
https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/netwerk/protocol/http/nsHttpHeaderArray.h#268,271-273
is probably relevant here: ',' appears in some authentication
information in the wild, so you can't just split WWW-Authenticate on
commas to get a list of authentication methods.

Whether this is a bug is something I'll let the networking team comment
on; I don't know the full set of compat constraints here.

-Boris

[john.bieling--- via dev-platform]

FYI: I observed this with Thunderbird 60.3.1 (the current stable release)

Is this related to
https://bugzilla.mozilla.org/show_bug.cgi?id=1491010

I reported this bug because fetch()'s response.headers.get("WWW-Authenticate") returned "null" if TWO such headers are received (as you suggested).

In the course of that bug, they discussed how to return such headers and decided to flatten them and return them as a list separated by ", ". This is according to the specs.

Furthermore they realized, that XHR currently is NOT flattening such multiple WWW-Authenticate headers, but returns them as a list separated by "\n". They decided to change that. This forces all XHR users to manually split WWW-Authenticate headers (which is complicated, as you pointed out). But it is according to the specs, so we have to live with that.

Now it looks like that nsIHttpChannel itself is not able to split WWW-Authenticate headers?

Should I add a link to this thread to the existing bug?

[Anne van Kesteren]

On Tue, Nov 20, 2018 at 9:20 AM john.bieling--- via dev-platform

Post by john.bieling--- via dev-platform
Now it looks like that nsIHttpChannel itself is not able to split WWW-Authenticate headers?

Right, I reported that in
https://bugzilla.mozilla.org/show_bug.cgi?id=1491010#c22.

Post by john.bieling--- via dev-platform
Should I add a link to this thread to the existing bug?

I think it's worth filing a new bug on parsing WWW-Authenticate, as
this bug at this point is really about how fetch() and XMLHttpRequest
end up exposing things. And while they arguably should have the same
fix, the priority and complexity between the two might vary quite a
bit.

[john.bieling--- via dev-platform]

@Anne van Kesteren

Thanks for your feedback. As you have the much deeper knowledge about these thinks, I think it would be better if you file that bug? I think you can get the report much more to the point than I could describe it?

Related Question: In my Add-On I made the transition from fetch() to nsIHttpChannel to get around the reported bug. But now I am stuck again :-) Is there a flag in the channel somewhere I can check after getting the 401, if nsIHttpChannel did not find any valid auth method, so I can try to manually parse the header in that case (until this is fixed)?

[john.bieling--- via dev-platform]

@Anne van Kesteren

Solved that by checking getRequestHeader("Authorization") in case of 401 and if that is missing, I know nsIHttpChannel did not try to authenticate.

[Honza Bambas]

Post by john.bieling--- via dev-platform
@Anne van Kesteren
Solved that by checking getRequestHeader("Authorization") in case of 401 and if that is missing, I know nsIHttpChannel did not try to authenticate.

First, I can confirm that we expect multiple authentication challenges
sent via separate response headers (WWW-Authenticate and
Proxy-Authenticate.)  Internally we then separate them using \n instead
of ',' [1] because comma can be contained in a single header value
(against what the spec says).  We can't correctly separate the headers
by commas, potentially even opening security holes if we do that blindly.

(We also expose the auth challenge headers this way (\n separated) to be
consumed by XHR and fetch() and exposed to DOM, where our fetch()
implementation has a bug and returns an empty string instead when \n is
contained in the header value.  Not fixed at least that issue till today
because of number of spec compliance arguments instead.)

When your server sends `WWW-Authenticate: X-MobileMe-AuthToken
realm="Newcastle", Basic realm="Newcastle"` we see it as:
schema=`X-MobileMe-AuthToken`
challenge data=`realm="Newcastle", Basic realm="Newcastle"`



Second, if you want to check whether the channel HAS NOT performed the
requested authentication, just check the HTTP response code to be 401,
use [2].  Looking for the Authorization header is just a big hack.


Thanks
-hb-


[1]
https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/netwerk/protocol/http/nsHttpHeaderArray.h#267-274
[2]
https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/netwerk/protocol/http/nsIHttpChannel.idl#258

Post by john.bieling--- via dev-platform
_______________________________________________
dev-platform mailing list
https://lists.mozilla.org/listinfo/dev-platform

[Boris Zbarsky]

Post by Honza Bambas
because comma can be contained in a single header value
(against what the spec says).  We can't correctly separate the headers
by commas, potentially even opening security holes if we do that blindly.

Do we know what other UAs do here?

-Boris

[Anne van Kesteren]

Post by Boris Zbarsky

Post by Honza Bambas
because comma can be contained in a single header value
(against what the spec says). We can't correctly separate the headers
by commas, potentially even opening security holes if we do that blindly.

Do we know what other UAs do here?

Similar, e.g., https://bugs.chromium.org/p/chromium/issues/detail?id=872772.
Doesn't seem like a high priority for anyone to fix.

[Boris Zbarsky]

Post by Anne van Kesteren
Similar, e.g., https://bugs.chromium.org/p/chromium/issues/detail?id=872772.
Doesn't seem like a high priority for anyone to fix.

Well... If:

1) All the browsers agree here (do they?)
2) There are concerns that there may be sites depending on the behavior.
3) If those concerns are true there would be security issues caused by
changing the behavior.

then why would we expect anyone to ever change the behavior?

-Boris

[john.bieling--- via dev-platform]

I have a working impl. now and just looking at 401 is not sufficient.

The user could indeed have provided a wrong password. The only way to know, if the 401 was caused because nsIHttpChannel did not even try to authenticate, is by checking wether it has send a Authorization header back to the server.

If not, I can manually check the WWW-Authentication header and manually check for BASIC and reinitiate the request and manually add the BASIC auth header.

For me it would be interesting to understand, if you consider the header send by contacts.icloud.com to by invalid, even though it fully complies with the spec. If i remember, the same spec is the reason why you are changing XHR to return the list no longer as "\n" separated, but as "\," separated?

[john.bieling--- via dev-platform]

I mean ", " separated of course.

[Honza Bambas]

Post by john.bieling--- via dev-platform
For me it would be interesting to understand, if you consider the header send by contacts.icloud.com to by invalid, even though it fully complies with the spec

Our implementation reflects the reality we can see in the wild.  I
believe the spec has always been wrong here, and apparently has never
been widely respected by servers because commas may be contained in the
challenge header values.  The spec should consider authentication as an
exception, similarly to Set-Cookies.  This is, tho, only my opinion.
-hb-

[john.bieling--- via dev-platform]

This should be decided once and for all. The complicated parsing of ", " separated auth headers was exactly my argumentation against changing XHR's behaviour. But it was discarded.

[Anne van Kesteren]

Our implementation reflects the reality we can see in the wild. I
believe the spec has always been wrong here, and apparently has never
been widely respected by servers because commas may be contained in the
challenge header values. The spec should consider authentication as an
exception, similarly to Set-Cookies. This is, tho, only my opinion.

Given that intermediaries are free to combine headers (other than
Set-Cookie) that seems problematic. It also seems doable to define a
parser that acts on the combined value, but I agree that doing so
requires buy-in from others and due diligence with respect to tests
and compatibility. (Also, per
https://github.com/httpwg/http-core/issues/136 it looks like the HTTP
WG isn't close to consensus on accepting the browser status quo, if
any exists.)

[Anne van Kesteren]

On Tue, Nov 20, 2018 at 9:50 AM john.bieling--- via dev-platform

Post by john.bieling--- via dev-platform
Thanks for your feedback. As you have the much deeper knowledge about these thinks, I think it would be better if you file that bug?

I forgot that it was already filed and marked as a dependency:
https://bugzilla.mozilla.org/show_bug.cgi?id=669675.

Post by john.bieling--- via dev-platform
Solved that by checking getRequestHeader("Authorization") in case of 401 and if that is missing, I know nsIHttpChannel did not try to authenticate.

Once we fix the bug you filed that will no longer be true though,
right? But I guess you can determine from the value that Firefox
wasn't able to parse it somehow...