Closed
Bug 1423278
Opened 7 years ago
Closed 6 years ago
Sending NTLM Type3 message to a proxy previously asking for Negotiate auth on a new connection
Categories
(Core :: Networking: HTTP, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla63
People
(Reporter: mayhemer, Assigned: mayhemer)
References
Details
(Whiteboard: [necko-triaged][ntlm])
Attachments
(1 file, 1 obsolete file)
1.36 KB,
patch
|
jduell.mcbugs
:
review+
RyanVM
:
approval-mozilla-beta-
RyanVM
:
approval-mozilla-esr60+
|
Details | Diff | Splinter Review |
Reported externally on Firefox 54 via mozilla-symantec-discuss forum.
Observed in a PCAP log received via the same forum.
I don't have a log for a session were this issue has happened hence it's hard to say what happens exactly.
From what I know so far it seems like the proxy closes the connection after sending back Type2 message which is illegal and quite illogical, but our reaction to it seems unexpected. We restart the authentication on a new connection and try to send the Type3 message - that should never happen, hence the uncertainty and marking UNCONFIRMED.
It's impossible to send Type3 message when communicating with the proxy for the first time (new connection) as a cached credential (we don't cache the request header, we only cache credentials when available and generate headers using those creds.) In case of Windows client SSO via SSPI we don't cache anything, as there is actually nothing to cache, AFAIK - the SSPI module does the negotiation internally w/o asking for credentials.
Note that the negotiation dance happens under the hood of a single HTTP request and the module is held as a continuation state inside nsHttpChannelAuthProvider (1-1 rel with a channel) hence it can't leak between different requests.
Next step: obtain more detailed logs.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → honzab.moz
Updated•7 years ago
|
Priority: -- → P3
Whiteboard: [ntlm] → [necko-triaged][ntlm]
Assignee | ||
Comment 1•7 years ago
|
||
Closing for lack of response in the "[mozilla-symantec-discuss] Strange Mozilla Behaviour with Symantec ProxySG and Authentication" thread.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
Assignee | ||
Updated•6 years ago
|
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INCOMPLETE → ---
Assignee | ||
Comment 2•6 years ago
|
||
I've obtained logs from Symantec that clearly demonstrate the issue and I was capable of identifying the cause.
The situation is reproducible for the following sequence of responses:
GET https://foo -> 407 Proxy-Authenticate: NEGOTIATE, NTLM, BASIC
GET https://foo, Proxy-Authorization: NEGOTIATE type1 -> 407 Proxy-Authenticate: NEGOTIATE type2
GET https://foo, Proxy-Authorization: NEGOTIATE type3 -> 401 WWW-Authenticate: Basic (from the server)
GET https://foo, Proxy-Authorization: NEGOTIATE type3 (the bug), Authorization: Basic
The reason we resend the type3 message is because in nsHttpChannelAuthProvider::PrepareForAuthentication we incorrectly create the authenticator instance using the raw (upper case) NEGOTIATE proxy challenge as a type.
The correct way is to lower case it or use an existing method that does it correctly.
Patch comes.
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 3•6 years ago
|
||
Attachment #9005024 -
Flags: review?(jduell.mcbugs)
Assignee | ||
Comment 4•6 years ago
|
||
(missed something)
Attachment #9005024 -
Attachment is obsolete: true
Attachment #9005024 -
Flags: review?(jduell.mcbugs)
Attachment #9005025 -
Flags: review?(jduell.mcbugs)
Updated•6 years ago
|
Attachment #9005025 -
Flags: review?(jduell.mcbugs) → review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 5•6 years ago
|
||
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/10c2d7ed3aaf
yBug 1423278 - Correctly instantiate proxy authenticator with a lowercase schema, r=jduell
Keywords: checkin-needed
Comment 7•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/10c2d7ed3aaf (landed 21 hours ago)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago → 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee | ||
Comment 8•6 years ago
|
||
Comment on attachment 9005025 [details] [diff] [review]
v1
Approval Request Comment
[Feature/Bug causing the regression]: bug 278885 (fixed 2005)
[User impact if declined]: connection dropping under some circumstances, inability to load web pages
[Is this code covered by automated tests?]: I'm afraid not
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: the setup is not simple to build, the issue is so trivial that an automated test is IMO not even needed
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: this only changes a contract id for getting an authenticator to be correctly built using the lower case string of the proxy auth schema
[String changes made/needed]: none
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
This bug makes NTLM/Negotiate proxies mainly used in corporate (ESR audience) environments under some circumstances drop connections.
User impact if declined:
It's manifesting as inability of the browser to load web pages.
Fix Landed on Version:
63
Risk to taking this patch (and alternatives if risky):
Not risky.
String or UUID changes made by this patch:
None
Attachment #9005025 -
Flags: approval-mozilla-esr60?
Attachment #9005025 -
Flags: approval-mozilla-beta?
Updated•6 years ago
|
status-firefox61:
--- → wontfix
status-firefox62:
--- → fix-optional
status-firefox-esr52:
--- → wontfix
status-firefox-esr60:
--- → affected
Comment 9•6 years ago
|
||
Comment on attachment 9005025 [details] [diff] [review]
v1
63 is already on Beta and 62 is on release. Is this something you'd want us to dot release for?
Attachment #9005025 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Updated•6 years ago
|
Flags: needinfo?(honzab.moz)
Updated•6 years ago
|
Assignee | ||
Comment 10•6 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> Comment on attachment 9005025 [details] [diff] [review]
> v1
>
> 63 is already on Beta and 62 is on release. Is this something you'd want us
> to dot release for?
No. What I think is way more important is to get this to ESR, all branches we support, because the audience of ESR is likely way more affected than regular users in non-corporate envs.
Thanks.
Flags: needinfo?(honzab.moz)
Comment 11•6 years ago
|
||
Comment on attachment 9005025 [details] [diff] [review]
v1
Fixes connectivity issues for users on NTLM domains. Approved for ESR 60.3.
Attachment #9005025 -
Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Comment 12•6 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•