1541927 - Master password dialog appears on every start of the browser when using enterprise policies to add a certificate

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[Jakub Hlavacek]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.86 Safari/537.36

Steps to reproduce:

1. open regedit
2. navigate to HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Mozilla\Firefox\Certificates\Install
3. add new string value and specify the path where certificate is stored
4. open browser > burger menu > options > privacy & security
5. select Use a master password options
6. Enter and re-enter new password and click OK button
7. set Homepage and new windows to Blank page, set New tabs to Blank page
8. close browser
9. start browser

Actual results:

"Please enter your master password" dialog appears on every start of the browser.

Expected results:

Master password dialog should not appear on

Comment 1

[:Gijs (he/him)]

Not an exploitable issue that needs to stay hidden.

Comment 2

[David Sacal]

Hi Gijs, then you consider this to be an enhancement or it should be closed as "resolved-invalid"? Thanks for your collaboration.

Comment 3

[:Gijs (he/him)]

(In reply to David Sacal from comment #2)

Hi Gijs, then you consider this to be an enhancement or it should be closed as "resolved-invalid"? Thanks for your collaboration.

I don't know enough about our master password and client cert code. Perhaps Dana knows.

Comment 4

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

My guess is this line https://searchfox.org/mozilla-central/rev/dd7e27f4a805e4115d0dbee70e1220b23b23c567/browser/components/enterprisepolicies/Policies.jsm#203 is causing Firefox to modify the trust db, which is authenticated with the key protected by the master password, which is why you're being asked for it. Presumably we could first check if the certificates being imported are already present and trusted to avoid this.

Comment 5

[Mike Kaply [:mkaply]]

Presumably we could first check if the certificates being imported are already present and trusted to avoid this.

If you had any way to do this, that would be great. Previously we used nicknames which made this pretty easy.

Unfortunately none of the APIs actually return the certificates that are added.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Are there other methods to add certificates that don't require Master Password auth? Not knowing too many details about it, I was surprised to see that this operation would require master password.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

One alternative would be to wrap this policy in a runOncePerModification.. If we do this, we need to think about the following case: if adding the certificates fail due to the user canceling the Master Password dialog, can we detect it and clear the runOnce flag to try it again?

Comment 8

[Mike Kaply [:mkaply]]

There's also the issue that the user can uninstall the certificate.

I really hate our master password stuff.

Comment 9

[[:philipp]]

[Tracking Requested - why for this release]:
according to user reports this issue is now triggered in the wild by avast update 19.4.2374 (April 2019) that got released yesterday:

• https://forum.avast.com/index.php?topic=226485.0
• https://www.camp-firefox.de/forum/viewtopic.php?f=1&t=127615
• changelog: https://forum.avast.com/index.php?topic=226462.msg1501258#msg1501258

Comment 10

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

(In reply to [:philipp] from comment #9)

[Tracking Requested - why for this release]:
according to user reports this issue is now triggered in the wild by avast update 19.4.2374 (April 2019) that got released yesterday:

• https://forum.avast.com/index.php?topic=226485.0
• https://www.camp-firefox.de/forum/viewtopic.php?f=1&t=127615
• changelog: https://forum.avast.com/index.php?topic=226462.msg1501258#msg1501258

Does avast use enterprise policies? If not, those reports are likely to have a different root cause from this bug (also that looks like bug 1514118 anyway, which is fixed in 67).

Comment 11

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

(In reply to Mike Kaply [:mkaply] from comment #5)

Presumably we could first check if the certificates being imported are already present and trusted to avoid this.

If you had any way to do this, that would be great. Previously we used nicknames which made this pretty easy.

Unfortunately none of the APIs actually return the certificates that are added.

Use nsIX509CertDB.constructCert{,FromBase64} to create a certificate without adding it to the permanent database and then use nsIX509CertDB.asyncVerifyCertAtTime and check the length of the chain to see if a certificate is already a trusted root.

Comment 12

[[:philipp]]

(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #10)

Does avast use enterprise policies? If not, those reports are likely to have a different root cause from this bug (also that looks like bug 1514118 anyway, which is fixed in 67).

i don't know, but would assume so as this bug was reported by an avast employee and got referenced [1] in their thread about the new avast update.

[1] https://forum.avast.com/index.php?topic=226462.msg1501396#msg1501396

Comment 13

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

Does this still happen on 67?

Comment 14

[Mike Kaply [:mkaply]]

Side note - you (Avast) should not be modifying HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Mozilla\Firefox\Certificates\Install directly.

You should be installing into the Windows store and flipping the enterprise roots pref.

Comment 15

[[:philipp]]

i can still reproduce the issue on 67.0b and nightly.

Comment 17

[Mike Kaply [:mkaply]]

nsIX509CertDB.asyncVerifyCertAtTime and check the length of the chain to see if a certificate is already a trusted root.

Is there an easy way to check the length of nsIX509CertList?

https://searchfox.org/mozilla-central/source/security/manager/ssl/nsIX509CertList.idl#16

I don't see an interface.

Comment 18

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

Not directly, currently, but you can get an enumerator and count how many elements end up in it:

let count = 0;
for (let cert of certChain.getEnumerator()) {
  count++;
}

Comment 19

[Mike Kaply [:mkaply]]

Can I just check the aPRErrorCode for non zero? It should be zero if it is there already, correct?

Also, once i have the X509 cert, can I import that directly instead of using addCert or addCertFromBase64? I can't find an API that takes a trust string and an X509 cert.

Thanks.

Comment 20

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

(In reply to Mike Kaply [:mkaply] from comment #19)

Can I just check the aPRErrorCode for non zero? It should be zero if it is there already, correct?

Assuming those who are using this are only importing roots, yes. If you get a chain with more than one certificate, it might be a good idea to import the second certificate with the empty trust string (",,") so it inherits its trust from its root.

Also, once i have the X509 cert, can I import that directly instead of using addCert or addCertFromBase64? I can't find an API that takes a trust string and an X509 cert.

nsIX509CertDB.setCertTrust might auto-import temporary certificates into the permanent DB, but I'm not actually sure. It would be safest to use one of the addCerts.

Thanks.

Sure thing!

Comment 21

[Mike Kaply [:mkaply]]

Attachment: Bug 1541927 - Don't readd CA via policy if it already exists.

Comment 22

[Juergen]

Hi,
the issue seems to be very complex. Its me who reported the "[Bug 1543774] Just after start of program shows the request for master password".

I like to add the information that I found indeed the certificate "C:\ProgramData\AVG\Antivirus\wscert.der" with regedit, but the date of the cert is 22.05.2018. The master password dialog however appeared the first time directly after start of Firefox in April 2019. Right after changing the installation of AVG back to the free Version, that was installed prior to a 60 days test period of the "AVG Internet Security - Unlimited" software. Perhaps that can bring some light into that case? Ihope it helps. :-)

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/377dc8053f59
Don't readd CA via policy if it already exists. r=keeler

Comment 24

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/377dc8053f59

Comment 25

[Mike Kaply [:mkaply]]

pascal: I'm guessing you needinfoed in case I wanted it on the ESR?

Comment 26

[Pascal Chevrel:pascalc]

(In reply to Mike Kaply [:mkaply] from comment #25)

pascal: I'm guessing you needinfoed in case I wanted it on the ESR?

For some reason my comment with the needinfo request got lost… I am triaging potential uplifts to beta/ESR, this landed in 68 so it will be in ESR 68. Would it make sense to have it in ESR 60 as well? 67 Beta? Thanks

Comment 27

[Mike Kaply [:mkaply]]

Comment on attachment 9060173 [details]
Bug 1541927 - Don't readd CA via policy if it already exists.

Beta/Release Uplift Approval Request

• User impact if declined: Master password dialogs shows at startup when certs are added (affects some antivirus)
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Set a master password.
  Use the policy to add a certificate.
  Verify that you only see it once.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Policy only
• String changes made/needed:

Comment 28

[Pascal Chevrel:pascalc]

Comment on attachment 9060173 [details]
Bug 1541927 - Don't readd CA via policy if it already exists.

Uplift accepted for 67 beta 16, thanks.

Comment 29

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/360d480593d5

Comment 32

[Mike Kaply [:mkaply]]

Comment on attachment 9060173 [details]
Bug 1541927 - Don't readd CA via policy if it already exists.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Policy related, annoyance to users.
• User impact if declined: Users of certain antivirus and enterprises see a popup at every startup if they have a master password enabled
• Fix Landed on Version: 68/67
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Policy only, well tested
• String or UUID changes made by this patch: None

Comment 33

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this tested with the current state of ESR (with the patch from bug 1549249)?

Comment 34

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello all,

Reproduced this issue on the Nightly build from 2019-04-04 68.0a1(20190404014638) with AVG Free 19.4.

Confirming this issue as verified fixed on the latest Nightly 68.0a1 (2019-05-09) and the latest Beta 67.0b18 with the above specified AV's on Windows 10x64.

Comment 35

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9060173 [details]
Bug 1541927 - Don't readd CA via policy if it already exists.

Policy fix for add-on cert issue. OK for ESR 60.7 uplift.

Comment 36

[Mike Kaply [:mkaply]]

Is this tested with the current state of ESR (with the patch from bug 1549249)?

That patch wouldn't affect this.

Comment 37

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/b967274c8e5c

Comment 38

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello
Confirming this issue as verified on 60.7.0esr(buildID:20190514153425)on Windows 10x64 as well.

Comment 39

[S.Amposta]

Reproduced in 66.0.3 (64-bit) Linux Mint-1.0
Where are Enterprise policies in Linux?

Comment 40

[Mike Kaply [:mkaply]]

Reproduced in 66.0.3 (64-bit) Linux Mint-1.0

It's not fixed in 66.

Where are Enterprise policies in Linux?

policies.json file in the distribution directory of the EXE.

Comment 42

[Mila]

In Firefox 67, Beta 68 or Nightly 69 is the same problem. Master password dialog appears on every start of the browser.
In about:policies is: "Certificates Install "C:\ProgramData\AVG\Antivirus\wscert.der"".

Windows register (regedit):
[HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Mozilla\Firefox\Certificates\Install]
"1"="C:\ProgramData\AVG\Antivirus\wscert.der"

Comment 43

[:Gijs (he/him)]

Ni for comment #42

Comment 44

[Mike Kaply [:mkaply]]

Can you please verify that the file C:\ProgramData\AVG\Antivirus\wscert.der exists?

Comment 45

[Mila]

Yes, file exists.

Comment 46

[Mila]

In about:policies at certificate path are two backslash "\", this forum displays only one backslash "". Probably mistake of Google Translate, which I used.
"C:\ProgramData\AVG\Antivirus\wscert.der"

Comment 47

[Mike Kaply [:mkaply]]

Can you please attach the cert to this bug and I'll try to recreate? Maybe there is something unique about that cert.

Comment 48

[Mila]

Google not is it. This forum delete one backslash.

Comment 49

[Mila]

Attachment: wscert.der

Certificate is read-only at AVG path. Cannot rename or modify at original place (not even as Administrator).