Closed Bug 1351301 Opened 3 years ago Closed 3 years ago

Don't require '.' to accept subdomains in "*.auth.trusted-uris" preferences (Kerberos stops working in Firefox 54)

Categories

(Core :: Networking, defect)

54 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 + fixed
firefox55 + fixed

People

(Reporter: sergey, Assigned: mayhemer)

References

Details

(Keywords: regression, Whiteboard: [ntlm][necko-active])

Attachments

(3 files)

Attached file 53-54-moz-log.zip
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170328084734

Steps to reproduce:

We have a corporate wiki that requires Kerberos authentication. Wiki still opens fine in Firefox 53 but in 54 I see an error page saying that Kerberos authentication failed.

Here are some excerpts from the logs:

Firefox 53:
[Main Thread]: D/negotiateauth   using REQ_DELEGATE
[Main Thread]: D/negotiateauth   service = wiki.example.com
[Main Thread]: D/negotiateauth   using negotiate-gss
[Main Thread]: D/negotiateauth entering nsAuthGSSAPI::nsAuthGSSAPI()
[Main Thread]: D/negotiateauth Attempting to load gss functions
[Main Thread]: D/negotiateauth entering nsAuthGSSAPI::Init()
[Lazy Idle]: D/negotiateauth nsHttpNegotiateAuth::GenerateCredentials() [challenge=Negotiate]
[Lazy Idle]: D/negotiateauth entering nsAuthGSSAPI::GetNextToken()
[Lazy Idle]: D/negotiateauth   leaving nsAuthGSSAPI::GetNextToken [rv=0]
[Lazy Idle]: D/negotiateauth   Sending a token of length 4295

Firefox 54:
[Main Thread]: D/negotiateauth nsHttpNegotiateAuth::ChallengeReceived URI blocked

Both times Firefox is started with a clean profile and preconfigured 'prefs.js' containing:
user_pref("network.negotiate-auth.delegation-uris", "example.com");
user_pref("network.negotiate-auth.trusted-uris", "example.com");
Component: Untriaged → Networking
Product: Firefox → Core
Whiteboard: [ntlm]
You have to add "." at the start of the string in preferences.  This has changed in bug 1328791.  But I'm no longer sure the change in handling the leading '.' is correct.  

To my understanding, allowing subdomains w/o having a '.' at the start of the preference value was incorrect.
Blocks: 1328791
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Summary: Kerberos stops working in Firefox 54 → Don't require '.' to accept subdomains in "*.auth.trusted-uris" preferences (Kerberos stops working in Firefox 54)
Wow, adding '.' fixes the problem. Thank a lot! I guess the ticket can be closed now?
(In reply to Sergey Vlasov from comment #2)
> Wow, adding '.' fixes the problem. Thank a lot! I guess the ticket can be
> closed now?

No, I think we should fix Necko (Firefox) to accept subdomains w/o a need to have the leading '.' in preferences.  You are the third person complaining this has changed.
[Tracking Requested - why for this release]: This is a regression from bug 1328791.
Tracking 54/55+ for this regression.
Jason, is there someone who can take a look at this regression?
Flags: needinfo?(jduell.mcbugs)
Honza, you wrote the patch that regressed this.
Assignee: nobody → honzab.moz
Flags: needinfo?(jduell.mcbugs) → needinfo?(honzab.moz)
Whiteboard: [ntlm] → [ntlm][necko-active]
Attached patch v1Splinter Review
no try, since no test
Flags: needinfo?(honzab.moz)
Attachment #8855432 - Flags: review?(valentin.gosu)
Status: NEW → ASSIGNED
Attachment #8855432 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8855589 [details] [diff] [review]
Test that '.' is not required to accept subdomains in auth::URIMatchesPrefPattern

Review of attachment 8855589 [details] [diff] [review]:
-----------------------------------------------------------------

thanks.
Attachment #8855589 - Flags: review?(honzab.moz) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/53f1ff7723eb
Don't require '.' to accept subdomains in *.auth.trusted-uris preferences. r=valentin
https://hg.mozilla.org/integration/mozilla-inbound/rev/7550aabb54b8
Test that '.' is not required to accept subdomains in auth::URIMatchesPrefPattern. r=mayhemer
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/53f1ff7723eb
https://hg.mozilla.org/mozilla-central/rev/7550aabb54b8
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Hi :mayhemer,
Since this is a regression, do you think this is worth uplifting to Aurora54?
Flags: needinfo?(honzab.moz)
Yes.  Will ask.
Flags: needinfo?(honzab.moz)
Comment on attachment 8855432 [details] [diff] [review]
v1

Approval Request Comment
[Feature/Bug causing the regression]: 1328791 
[User impact if declined]: inability to connect to NTLM servers under some common setup that used to work before
[Is this code covered by automated tests?]: partially
[Has the fix been verified in Nightly?]: only locally and manually, none of the original reporters has answered so far
[Needs manual test from QE? If yes, steps to reproduce]: no, there is no easy way to test this w/o an NTLM server setup
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: simple change to a preference parser cover by tests.
[String changes made/needed]: none
Attachment #8855432 - Flags: approval-mozilla-aurora?
Comment on attachment 8855589 [details] [diff] [review]
Test that '.' is not required to accept subdomains in auth::URIMatchesPrefPattern

Approval Request Comment - see comment 15
Attachment #8855589 - Flags: approval-mozilla-aurora?
Comment on attachment 8855432 [details] [diff] [review]
v1

Fix a regression and include tests. Aurora54+.
Attachment #8855432 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8855589 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.