In an e10s window, Nightly does not bring up the master password dialog when required for TLS client authentication

RESOLVED FIXED in Firefox 44

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: mozbugzilla, Assigned: mconley)

Tracking

({regression})

43 Branch
mozilla44
regression
Points:
---

Firefox Tracking Flags

(e10sm8+, firefox43 affected, firefox44 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
Created attachment 8660401 [details]
Self-signed client test certificate, valid thru 2025-09-12, no passphrase set

In an e10 window, Nightly does not prompt for the master password when it is required for performing TLS client authentication. The "User Identification Request" dialog ("This site has requested that you identify yourself with a certificate:", "Choose a certificate to present as identification:") is shown, and while a cert can be selected, Nightly will continue the handshake without authenticating with that certificate. Manually logging into the Software Security Device (Options -> Advanced -> Security Devices) before connecting to the respective site will adress the problem, though this can't be considered an acceptable workaround.

For testing purposes, I'm attaching a PKCS#12 file with a self-signed certificate which can be used to reproduce the issue with the https://www.mikestoolbox.net test site e.g. (feel free to use another site which prompts for a client cert). www.mikestoolbox.net requests - but does not require - a client certificate, and will show its details in the "Client Certificate" section at the end of its diagnostic output.
Assignee: nobody → mconley
tracking-e10s: ? → m8+
(Assignee)

Comment 1

3 years ago
Created attachment 8664497 [details]
MozReview Request: Bug 1204301 - HttpChannelParent needs to be able to GetInterface to an nsIPrompt. r?michal

Bug 1204301 - HttpChannelParent needs to be able to GetInterface to an nsIPrompt. r?michal

NSS TLS authentication prompts require us to ask the user for their
master password in the event that the user has one set up. In that
case, the nsIInterfaceRequestor passed to PK11_DoPassword needs to
be able to GetInterface to an nsIPrompt in order to prompt the user
for the password.

This nsIInterfaceRequestor, when running with e10s enabled, happens
to be HttpChannelParent.
Attachment #8664497 - Flags: review?(michal.novotny)
(Assignee)

Comment 2

3 years ago
I should point out that I have only the barest knowledge of how NSS works, and of how Necko interacts with it. While the code I've written might be sound, the overall solution might be suboptimal and I'd never know about it. Please treat with skepticism. :)
Comment on attachment 8664497 [details]
MozReview Request: Bug 1204301 - HttpChannelParent needs to be able to GetInterface to an nsIPrompt. r?michal

I'm not the right person to review this. Let's see if Honza can take a look.
Flags: needinfo?(honzab.moz)
Attachment #8664497 - Flags: review?(michal.novotny)
So this code here is in necko but the logic is all about getting the right incantation in e10s to return an nsIPrompt from a TabParent.  I'm not sure how well Honza knows that (and he's also swamped with e10s new security model stuff)

Bill, do you know this stuff well enough to review, or know someone else who can take it?
Flags: needinfo?(wmccloskey)
(Assignee)

Comment 6

3 years ago
(In reply to Jason Duell [:jduell] (needinfo? me) from comment #5)
> So this code here is in necko but the logic is all about getting the right
> incantation in e10s to return an nsIPrompt from a TabParent.  I'm not sure
> how well Honza knows that (and he's also swamped with e10s new security
> model stuff)
> 

Right - and I'm reasonably confident I've done the right incantations to get at the TabParent's owning nsIDOMWindow to spawn the prompt. I guess I'm just not really familiar with HttpChannel(Parent|Child), and I'm _very_ much not familiar with NSS. If somebody can just tell me if GetInterface'ing a HttpChannelParent to an nsIPrompt makes sense, then I'll feel much more confident about this.
The code looks fine and it seems reasonable to do the GetInterface thing. All NSS stuff is supposed to happen in the parent. I'm a little leery of reviewing it without knowing how HttpChannelParent actually ends up in NSS. Mike, if you could explain that part a bit more, I'd be happy to r+.
Flags: needinfo?(wmccloskey) → needinfo?(mconley)
(Assignee)

Comment 8

3 years ago
It's a bit convoluted, but I think this is how it goes.

So we have these nsHttpChannels, and these inherit from nsBaseChannel. nsBaseChannel has a protected member called mCallbacks which is set via nsBaseChannel::SetNotificationCallbacks in... a number of places.

For HttpChannelParent, SetNotificationCallbacks is called here: https://dxr.mozilla.org/mozilla-central/rev/79a5b2968d01512470eb6c25d6638d8b9565575e/netwerk/protocol/http/HttpChannelParent.cpp#423

The callback it passes is an HttpChannelParentListener, which implements nsIInterfaceRequestor, and forwards GetInterface to the HttpChannelParent.

Fast-forward to when we want to show the user the certificate dialog.

The network layer eventually figures out that we're doing TLS client auth, and calls nsNSS_SSLGetClientAuthData: https://dxr.mozilla.org/mozilla-central/rev/79a5b2968d01512470eb6c25d6638d8b9565575e/security/manager/ssl/nsNSSIOLayer.cpp#1988

It then instantiates a ClientAuthDataRunnable (this is what's going to show the dialog): https://dxr.mozilla.org/mozilla-central/rev/79a5b2968d01512470eb6c25d6638d8b9565575e/security/manager/ssl/nsNSSIOLayer.cpp#2023

When constructing that runnable, it passes "info", which in the case for e10s, happens to be a TransportSecurityInfo::TransportSecurityInfo.

A reference to that TransportSecurityInfo is stored inside the ClientAuthDataRunnable as mSocketInfo, which when the runnable is fired, is cast to a void* pointer here: https://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/nsNSSIOLayer.cpp#2054 and passed around as "wincx".

There's quite a bit of indirection, but eventually that void* wincx pointer is cast back to an nsIInterfaceRequestor here when we decide we want to open up the master password dialog: https://dxr.mozilla.org/mozilla-central/rev/79a5b2968d01512470eb6c25d6638d8b9565575e/security/manager/ssl/nsNSSCallbacks.cpp#865

We then GetInterface for nsIPrompt off of the nsIInterfaceRequestor.

So, to sum, that nsIInterfaceRequestor eventually gets forwarded to the TransportSecurityInfo, which eventually forwards the GetInterface to the HttpChannelParent.

Those "eventually"'s can be pretty long and complex, and take me into the guts of netwerk. It's quite extraordinary. So this is my high-level understanding of how HttpChannelParent got into NSS. If you need me to go deeper, let me know.
Flags: needinfo?(mconley) → needinfo?(wmccloskey)
Comment on attachment 8664497 [details]
MozReview Request: Bug 1204301 - HttpChannelParent needs to be able to GetInterface to an nsIPrompt. r?michal

OK, sounds good to me. I mostly just wanted to see if this was the right place to do the GetInterface.

One way to make this a bit shorter would be to delegate the GetInterface call to the chrome docshell. But this way looks fine too.
Flags: needinfo?(wmccloskey)
Attachment #8664497 - Flags: review+
(Assignee)

Comment 10

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/077a6b1a1ba5abd927990e01740806c76f899f9b
Bug 1204301 - HttpChannelParent needs to be able to GetInterface to an nsIPrompt. r=billm.
https://hg.mozilla.org/mozilla-central/rev/077a6b1a1ba5
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Flags: needinfo?(honzab.moz)
You need to log in before you can comment on or make changes to this bug.