Closed Bug 1204301 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Security: PSM, defect)

43 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
e10s m8+ ---
firefox43 --- affected
firefox44 --- fixed

People

(Reporter: mozbgz, Assigned: mconley)

Details

(Keywords: regression)

Attachments

(3 files)

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
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)
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)
(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)
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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/077a6b1a1ba5abd927990e01740806c76f899f9b Bug 1204301 - HttpChannelParent needs to be able to GetInterface to an nsIPrompt. r=billm.
Status: NEW → RESOLVED
Closed: 10 years ago
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.

Attachment

General

Creator:
Created:
Updated:
Size: