902439 - New Tab Page launches User Identification Request dialog before page is selected

Status: RESOLVED DUPLICATE of bug 910207

(Core :: Security: PSM, defect)

Description

[drisacher]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20130806030203

Steps to reproduce:

1. Have one or more sites in the "New Tab page" which will request client authentication. 
2. Open a new tab.



Actual results:

A "User Identification Request" dialog is launched for each site that requests client authentication (presumably as that site is preloaded)


Expected results:

The User Identification Request dialog should not be displayed for sites that the user has not navigated to.

Comment 1

[drisacher]

Further experimentation indicates that it only launches the UIR dialog when the mouse passes over the site thumbnail, not merely when the new tab page is opened.

Comment 2

[drisacher]

Attachment: Screenshot

Comment 3

[Loic]

Maybe the authentication dialog box appears when Firefox is trying to take and cache the thumbnail of this website.

It's a kind of edge case, I believe.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

Bug 875157 managed to suppress most auth prompts, but obviously not this one.  Does anyone happen to know how I could reproduce this (ie, how I could see this dialog locally)?

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Attachment: Check nsIDOMWindowUtils::isParentWindowMainWidgetVisible before showing the dialog

Digging a little, this dialog is opened by http://mxr.mozilla.org/mozilla-central/source/security/manager/pki/src/nsNSSDialogs.cpp#260.  Unfortunately, I don't know how to arrange to see such a dialog, and I can't locate any tests that use it.

Assuming this dialog is being opened by the parent process, the following patch *should* do the right thing - but I'm going to need help verifying this.  Gavin, can you help me find a good person?

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Seems like maybe we should put that check in nsNSSDialogHelper::openDialog(), which seems to be common to a bunch of the different prompting methods in that file?

bsmith may be able to explain how to test some of these dialogs in practice.

Comment 7

[drisacher]

You might try going to https://www.whitepages.mil/ or https://ataaps.csd.disa.mil/ and see if Firefox pops up the dialog.  You won't have a valid user certificate, but I think it will still launch the prompt.  If you create a bookmark for those pages you can drag from the bookmarks menu to the new tab page.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

(In reply to drisacher from comment #7)
> You might try going to https://www.whitepages.mil/ or
> https://ataaps.csd.disa.mil/ and see if Firefox pops up the dialog.

Sadly they don't.  The first causes Fx to display the error:
"
An error occurred during a connection to www.whitepages.mil. Unable to find the certificate or key necessary for authentication. (Error code: ssl_error_no_certificate)
"
and the second:
"
An error occurred during a connection to ataaps.csd.disa.mil. SSL peer was unable to negotiate an acceptable set of security parameters. (Error code: ssl_error_handshake_failure_alert)
"

I've no certificates at all - I might see if I can find some certificate to install - having one installed might be the difference.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Signing up for a free account (or logging into it) at http://www.startssl.com/ will probably trigger some of these dialogs.

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #6)
> Seems like maybe we should put that check in
> nsNSSDialogHelper::openDialog(), which seems to be common to a bunch of the
> different prompting methods in that file?
> 
> bsmith may be able to explain how to test some of these dialogs in practice.

I am about to leave for PTO for a week. If the goal is simply to prevent the prompts from showing up, then what Gavin suggests is reasonable. However, it would be best if all the loads that happen during the preloading had the LOAD_ANONYMOUS flag set on them, somehow, because there's also a way for client certs to be chosen without the prompt, that is only prevented through LOAD_ANONYMOUS, IIRC.

AFAICT, none of the PSM UI code that I've inherited has ever had test and I've never written a test for UI code so I have no clue about the testing.

Note I am PTO for the next week.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

(In reply to Brian Smith (:briansmith), was [e-mail] (:bsmith) - Not reading bugmail until August 19th, 2013 from comment #10)
> However, it would be best if all the loads that happen during the preloading had
> the LOAD_ANONYMOUS flag set on them, somehow, because there's also a way for
> client certs to be chosen without the prompt, that is only prevented through
> LOAD_ANONYMOUS, IIRC.

FWIW this happens in a private browsing mode docShell, not sure if that's relevant. We're likely going to end up ensuring LOAD_ANONYMOUS is set in bug 875986.

Comment 12

[Mark Hammond [:markh] [:mhammond]]

(In reply to Mark Hammond (:markh) from comment #5)
> Created attachment 787346 [details] [diff] [review]
> Check nsIDOMWindowUtils::isParentWindowMainWidgetVisible before showing the
> dialog

> Assuming this dialog is being opened by the parent process, the following
> patch *should* do the right thing - but I'm going to need help verifying
> this.  Gavin, can you help me find a good person?

Now I can reproduce the problem, I discover the patch doesn't solve it.  ChooseCertificate tries to get the window from the first nsIInterfaceRequestor *ctx param, but this fails as ctx is actually a nsNSSSocketInfo rather than something that can return an nsIDOMWindow.

So LOAD_ANONYMOUS is the only way forward I can currently see.  I guess I'll start a thread on mozilla.dev.tech.network

Comment 13

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-902439-use-an-nsIWebProgressListener-to-force-ea.patch

This patch uses an nsIWebProgressListener to force all requests made by the docShell to have the LOAD_ANONYMOUS flag and solves the problem for me.  It will obviously cause some conflicts with bug 901294, but I tried to minimize them as much as possible.

Comment 14

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 792646 [details] [diff] [review]
0001-Bug-902439-use-an-nsIWebProgressListener-to-force-ea.patch

Based on feedback from dev.tech.network, I'm going to try a different approach based on a few docShell flag.

Comment 15

[Mark Hammond [:markh] [:mhammond]]

FWIW, my previous patch using nsIWebProgressListener didn't actually work in all cases.  I now suspect some bad interactions between speculative connections and SSL - for more details, see https://groups.google.com/forum/#!topic/mozilla.dev.tech.network/DKozJ3ggVE4

Comment 16

[Mark Hammond [:markh] [:mhammond]]

This will magically be fixed when both bug 910207 and 875986 become fixed.

Comment 17

[Mark Hammond [:markh] [:mhammond]]

Now that bug 875986 has landed, the majority of prompts caused by thumbnails will no longer be shown.  However, bug 910207 means the prompt is still likely to happen if you move the mouse over the thumbnail for a site that prompts for a certificate, so I'll leave this open until that is fixed.

Comment 18

[Marti Raudsepp]

Ping! I am still affected by this issue.

Comment 20

[Mark Hammond [:markh] [:mhammond]]

Bug 910207 isn't getting any traction and there's nothing I can do until it does, so unassigning myself from this.