1578882 - Reading certificates from Windows store happens too late for homepage

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Mike Kaply [:mkaply]]

We had this report from a customer. Unfortunately I can't get a hold of them anymore, but still worth looking at:

Since Firefox ESR 68 we have a problem with internal certificates.

Firefox has the setting « security.enterprise_roots.enabled » enabled. In the Certificate Store of Windows 10 LTSC we have imported a Trusted internal CA. One of the internal sites is defined as a startpage and is loaded if firefox is opened .Firefox worked over years like a charm with this startpage and all other internal sites (Certificate signed with internal CA) until ESR 68.

Since ESR 68 if we start firefox, firefox starts the internal site, with is defined as « startpage ». Firefox displays a warning, that this site is not trusted !!. If i hit F5 (reload) the site is loaded und the certificate warning is gone.

This happens with all internal sites, if they are loaded as startpage or with the following syntax : firefox.exe https://internal.url.local. If firefox already open and i browse to an internal site, all sites are trusted.

Is there a timing problem with certificates (from the windows certificates store) on the start of firefox?

Comment 1

[J.C. Jones [:jcj] (he/him) [taking a break]]

Ugh, we did that for speed, to not pause first pageload on cert loading.

Comment 2

[Mike Kaply [:mkaply]]

Ugh, we did that for speed, to not pause first pageload on cert loading.

Can we detect cert error and reload or something?

Comment 3

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

If something calls GetDefaultCertVerifier() before the background task that loads loadable roots (including enterprise roots) finishes, the cert verifier returned by that call won't know about any enterprise roots. We can probably fix this by making that call block on the background loading task (note that we already wait for those loadable roots anyway).

Comment 4

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Attachment: bug 1578882 - wait on the loadable roots background task before handing out CertVerifier handles r?tjr

If code acquires a handle on the certificate verifier before the loadable roots
background task completes, that instance of the verifier may not know about any
enterprise certificates loaded, and so early certificate verifications relying
on those certificates may fail. To prevent this, this patch ensures that the
background task has completed before returning the handle. Note that there
should be no effect on performance since CertVerifier already ensures that the
background task has completed internally before looking for potential issuer
certificates.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0586b06a7aab
wait on the loadable roots background task before handing out CertVerifier handles r=tjr

Comment 6

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/0586b06a7aab

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Sounds like this is something we may want to uplift to Beta and ESR68?

Comment 8

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Yes, we probably should.

Comment 9

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 9093434 [details]
bug 1578882 - wait on the loadable roots background task before handing out CertVerifier handles r?tjr

Beta/Release Uplift Approval Request

• User impact if declined: Users visiting sites that use 3rd party root certificates may not work early in sessions without a refresh.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small change. We already wait on the background task in a number of other places - this adds another instance of that waiting. Also, it shouldn't have an impact on performance.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Users visiting sites that use 3rd party root certificates may not work early in sessions without a refresh (we assume that many ESR users rely on 3rd party roots).
• User impact if declined: ^
• Fix Landed on Version: 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small change. We already wait on the background task in a number of other places - this adds another instance of that waiting. Also, it shouldn't have an impact on performance.
• String or UUID changes made by this patch:

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9093434 [details]
bug 1578882 - wait on the loadable roots background task before handing out CertVerifier handles r?tjr

Fixes some possible issues with 3rd party root certificates. Approved for 70.0b8 and 68.2esr.

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6afdad3c5f0f

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/579e9b6c1560