Closed
Bug 1315332
Opened 8 years ago
Closed 8 years ago
Getting "The connection was reset" when I don't make it to enter correct NTLM credentials on time
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: mayhemer, Assigned: mayhemer)
References
Details
(Whiteboard: [ntlm][necko-active])
Attachments
(1 file, 1 obsolete file)
1.51 KB,
patch
|
mcmanus
:
review+
mayhemer
:
feedback+
jcristau
:
approval-mozilla-esr52+
|
Details | Diff | Splinter Review |
I am afraid that the patch for bug 486508 is too aggressive. When the server closes the connection simply before I fill in the credentials, only result (even the creds are actually correct) is to get an error page with "The connection was reset" message.
We used to restart the transaction on a new connection, but that was faulty in many ways.
To avoid this stupid error page (interesting that IIS server doesn't keep the connection longer anyway...) we need to reinstate restart, but in a smarter way. Probably by a redirect to the same URL. Anyway, not that easy to fix.
Assignee | ||
Comment 1•8 years ago
|
||
Good news is that just pressing the big "Try Again" button fixes the problem - creds are cached. But we should do this "try" internally for the user.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Whiteboard: [ntlm][necko-next] → [ntlm][necko-active]
Assignee | ||
Comment 2•8 years ago
|
||
This patch is incomplete, since before the seconds try we don't reach GenCredsAndSetEntry that would make the second try use cached credentials w/o an auth dialog. Hence, the auth dialog pops up twice anyway and if users retype their username/password (believing the creds were passed wrong) they just may get into the same loop again.
I'm not sure what is the right fix here and maybe this is just a WONTFIX. It's easy to press [F5] and [enter] to re-send the user name and password stored in the password manager and see that the server is the one that reset the connection.
Putting the patch here just for reference. I don't want to make the code around NTLM be more overcomplicated.
Assignee | ||
Updated•8 years ago
|
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
Whiteboard: [ntlm][necko-active] → [ntlm][necko-would-take]
Assignee | ||
Comment 3•8 years ago
|
||
I found a simple solution!
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Whiteboard: [ntlm][necko-would-take] → [ntlm][necko-active]
Assignee | ||
Comment 4•8 years ago
|
||
- this is using api 1309438 in bug 1309438
- it seems to be OK (at least with IIS) to send the first round response (message type 1) on a new connection ; hence when we ask user for creds and the schema is conn-based, we simply always open a new connection to send the first round message
This is mostly what we were doing before bug 486508, but this time controlled and correct.
Gary, is this OK to do with squid and any other NTLM server you can thing of?
Attachment #8809837 -
Attachment is obsolete: true
Attachment #8810959 -
Flags: review?(mcmanus)
Attachment #8810959 -
Flags: feedback?(gary)
Assignee | ||
Updated•8 years ago
|
status-firefox53:
--- → affected
Comment 5•8 years ago
|
||
I think this in the correct fix. I would just change the comment to say we always want a new connection, to be sure we haven't sent NTLM packets on it already.
Andrew Bartlett
Updated•8 years ago
|
Attachment #8810959 -
Flags: review?(mcmanus) → review+
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8810959 [details] [diff] [review]
v1 (when auth dialog is open, don't reuse the sticky conn)
Taking comment 5 as f+ for this patch.
Thanks Patrick for the review.
Attachment #8810959 -
Flags: feedback?(gary) → feedback+
Assignee | ||
Comment 7•8 years ago
|
||
Can't land, deps on 1309438 that needs r.
Assignee | ||
Comment 8•8 years ago
|
||
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/132bb5e7c5b8
Don't reuse the connection for conn-based auth schemes when asking user for credentials, r=mcmanus
Keywords: checkin-needed
Comment 10•8 years ago
|
||
Backout because the dependency bug 1309438 didn't land (-> bustage):
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb3dc03015529ea5f36b51c360bb407357b7e5e2
Flags: needinfo?(honzab.moz)
Assignee | ||
Comment 11•8 years ago
|
||
Yes, of course. The dependency bug is also marked checkin-needed. I was hoping you notice the dep and land that one first (used to work in the past).
Flags: needinfo?(honzab.moz)
Assignee | ||
Comment 12•8 years ago
|
||
Dep landed, please land now this one as well. Thanks.
Keywords: checkin-needed
Comment 13•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1219cdda7496
Don't reuse the connection for conn-based auth schemes when asking user for credentials. r=mcmanus
Keywords: checkin-needed
Comment 14•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Updated•8 years ago
|
Comment 16•8 years ago
|
||
If this affected to ESR52, It seems to be worth to up lift this to ESR52.
status-firefox-esr52:
--- → ?
Assignee | ||
Comment 17•8 years ago
|
||
Comment on attachment 8810959 [details] [diff] [review]
v1 (when auth dialog is open, don't reuse the sticky conn)
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: major functionality breakage
User impact if declined: inability to log into an NTLM/Negotiate protected site
Fix Landed on Version: 53
Risk to taking this patch (and alternatives if risky): low, this has been riding trains a long time
String or UUID changes made by this patch: none
See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8810959 -
Flags: approval-mozilla-esr52?
Comment 18•8 years ago
|
||
This depends on bug 1309438 which is not in 52.
Flags: needinfo?(honzab.moz)
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #18)
> This depends on bug 1309438 which is not in 52.
Aggrrr... yes, that too should have been uplifted. Never mind now.
Flags: needinfo?(honzab.moz)
Updated•8 years ago
|
Comment 20•8 years ago
|
||
Comment on attachment 8810959 [details] [diff] [review]
v1 (when auth dialog is open, don't reuse the sticky conn)
Clearing esr52 approval flag so this gets off my radar, feel free to set it again if bug 1309438 is upliftable.
Attachment #8810959 -
Flags: approval-mozilla-esr52?
Assignee | ||
Comment 21•8 years ago
|
||
when bug 1309438 gets to esr52, ask for a?esr52
Flags: needinfo?(honzab.moz)
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8810959 [details] [diff] [review]
v1 (when auth dialog is open, don't reuse the sticky conn)
DEPENDS ON BUG 1309438 !
applies cleanly on esr52
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: major issues with NTLM authentication
User impact if declined: can't authenticate to an NTLM site
Fix Landed on Version: 53
Risk to taking this patch (and alternatives if risky): low if not none
String or UUID changes made by this patch: none
See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(honzab.moz)
Attachment #8810959 -
Flags: approval-mozilla-esr52?
Comment 23•8 years ago
|
||
Comment on attachment 8810959 [details] [diff] [review]
v1 (when auth dialog is open, don't reuse the sticky conn)
ntlm auth fix for esr52
Attachment #8810959 -
Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Comment 24•8 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•