Closed Bug 1541927 Opened 5 months ago Closed 4 months ago

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

Categories

(Firefox :: Enterprise Policies, defect, P1)

66 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 68
Tracking Status
firefox-esr60 --- verified
firefox66 --- wontfix
firefox67 - verified
firefox68 --- verified

People

(Reporter: hlavacekj, Assigned: mkaply)

References

Details

Attachments

(2 files)

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

Not an exploitable issue that needs to stay hidden.

Group: firefox-core-security

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

Flags: needinfo?(gijskruitbosch+bugs)

(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.

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dkeeler)
Component: Untriaged → Security: PSM
Product: Firefox → Core

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.

Component: Security: PSM → Enterprise Policies
Flags: needinfo?(dkeeler)
Product: Core → Firefox
Summary: Master password dialog appears on every start of the browser → Master password dialog appears on every start of the browser when using enterprise policies to add a certificate
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1

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.

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.

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?

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

I really hate our master password stuff.

[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:

(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:

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).

Flags: needinfo?(madperson)

(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.

(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

Flags: needinfo?(madperson)

Does this still happen on 67?

Flags: needinfo?(hlavacekj)

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.

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

Flags: needinfo?(hlavacekj)
Duplicate of this bug: 1543774

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.

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++;
}

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.

Flags: needinfo?(dkeeler)

(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!

Flags: needinfo?(dkeeler)
Assignee: nobody → mozilla

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. :-)

Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/377dc8053f59
Don't readd CA via policy if it already exists. r=keeler
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

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

Flags: needinfo?(mozilla)

(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 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:
Attachment #9060173 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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

Uplift accepted for 67 beta 16, thanks.

Attachment #9060173 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]
Duplicate of this bug: 1549552

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
Attachment #9060173 - Flags: approval-mozilla-esr60?

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

Flags: needinfo?(mozilla)

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

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.

Attachment #9060173 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

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

That patch wouldn't affect this.

Flags: needinfo?(mozilla)
Flags: qe-verify+

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

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

Flags: qe-verify+

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.

Duplicate of this bug: 1552642

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"

Ni for comment #42

Flags: needinfo?(mozilla)

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

Flags: needinfo?(mozilla)

Yes, file exists.

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"

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

Google not is it. This forum delete one backslash.

Attached file wscert.der

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

Depends on: 1553233
You need to log in before you can comment on or make changes to this bug.