1406396 - fix PSM to better handle NSS command-line utils spuriously adding a "Root Certs" module in some situations

Status: RESOLVED FIXED

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

Description

[suivios]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170926190823

Steps to reproduce:

1.	Install Firefox 57 Beta 6
2.	Check Certificates in “about:preferences#privacy” -> certificates => all certificates are installed 
3.	Close Firefox
4.	Use certutil.exe tool to add or delete certificates
	-	certutil.exe -A -n “AC-SRVSVCNAM-256” -i “AC-SRVSVCNAM.pem" -t "TCu,TCu,TCu" -d .
	or
	-	certutil.exe -D -n “AC-SRVSVCNAM” -i “AC-SRVSVCNAM.pem” -d .
5.	Open Firefox
6.	Check Certificates in “about:preferences#privacy” -> certificates => Empty, no certificate, all are lost



Actual results:

When using certutil.exe tools to add or delete a specific certificate from the certificate database, all the certificates are lost when opening Firefox 57.


Expected results:

All the certificates installed by firefox should be kept after using certutil.

if certutil.exe is incompatible with Firefox 57, can you tell us the method to add or remove certificates ?

Comment 1

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

Can you repeat those steps with the environment variable "MOZ_LOG" set to "pipnss:4" while running Firefox from a console? That should give us some debugging output that might explain what's going on.

Comment 2

[suivios]

Attachment: requested log with with the environment variable "MOZ_LOG" set to "pipnss:4"

See in the attached file "MozLog.txt" the debugging result with the environment variable "MOZ_LOG" set to "pipnss:4".

This is the result when launching Firefox after the certutil command.


Best Regards.

Comment 3

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

Thanks! Do you have any non-ascii characters in the path where Firefox is installed? Also, what version of NSS is your certutil.exe compiled against? Another thing to try would be to look in the device manager (about:preferences -> Privacy & Security -> Security Devices). What information do you see there?

Comment 4

[suivios]

Attachment: certutil.zip

Comment 5

[suivios]

Hi,

1. No we don't have non-ascii characters in the path. We are using the default installation path: C:\Program Files\Mozilla Firefox.

2. We are using a certutil tool find on internet. Find in attachment the ZIP file containing our certutil tool. 
We tryed with an other one found at https://gallery.technet.microsoft.com/scriptcenter/modify-mozilla-firefox-to-51b707c1. 

3. After certutil command, the device manager of Firefox 57 (about:preferences -> Privacy & Security -> Security Devices) is empty too. 


Best Regards.

Comment 6

[suivios]

Hi,

For information, we build our certutil.exe with NSS 3.33 and we reproduced the same problem.

Comment 7

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

Thanks! Here's a couple more things to try:

Compare the results of running `modutil.exe -dbdir {your profile directory} -list` before and after running certutil (modutil is part of NSS - if you built certutil then modutil will probably be in the same place).

Try using Process Monitor ( https://docs.microsoft.com/en-us/sysinternals/downloads/procmon ) and filter on the file "nssckbi.dll" to maybe see if certutil is locking the file somehow (or maybe it'll tell you why Firefox fails to open it).

Comment 8

[suivios]

Hi,

I performed the required manip but don't see what is wrong.

For more information, I precise that this problem doesn't occure with Firefox 56.0.1 and before. This problem is only reproduced with Firefox 57 Beta.

If I compare the result of running `modutil.exe -dbdir {your profile directory} -list` with FF56.0.1 and FF 57, I don't see any difference:

[before using certutil:]
---------------------

C:\Program Files (x86)\santesocial\srvsvcnam\firefox>modutil.exe -dbdir C:\Users\ValidDIP\AppData\Roaming\Mozilla\Firefox\Profiles\bzpiciu7.default-1507879123614 -list

Listing of PKCS #11 Modules
-----------------------------------------------------------
  1. NSS Internal PKCS #11 Module
           uri: pkcs11:library-manufacturer=Mozilla%20Foundation;library-description=NSS%20Internal%20Crypto%20Services;library-version=3.33
         slots: 2 slots attached
        status: loaded

         slot: NSS Internal Cryptographic Services
        token: NSS Generic Crypto Services
          uri: pkcs11:token=NSS%20Generic%20Crypto%20Services;manufacturer=Mozilla%20Foundation;serial=0000000000000000;model=NSS%203

         slot: NSS User Private Key and Certificate Services
        token: NSS Certificate DB
          uri: pkcs11:token=NSS%20Certificate%20DB;manufacturer=Mozilla%20Foundation;serial=0000000000000000;model=NSS%203
-----------------------------------------------------------

[After using certutil with FF57:]
---------------

C:\Program Files (x86)\santesocial\srvsvcnam\firefox>modutil.exe -dbdir C:\Users\ValidDIP\AppData\Roaming\Mozilla\Firefox\Profiles\bzpiciu7.default-1507879123614 -list

Listing of PKCS #11 Modules
-----------------------------------------------------------
  1. NSS Internal PKCS #11 Module
           uri: pkcs11:library-manufacturer=Mozilla%20Foundation;library-description=NSS%20Internal%20Crypto%20Services;library-version=3.33
         slots: 2 slots attached
        status: loaded

         slot: NSS Internal Cryptographic Services
        token: NSS Generic Crypto Services
          uri: pkcs11:token=NSS%20Generic%20Crypto%20Services;manufacturer=Mozilla%20Foundation;serial=0000000000000000;model=NSS%203

         slot: NSS User Private Key and Certificate Services
        token: NSS Certificate DB
          uri: pkcs11:token=NSS%20Certificate%20DB;manufacturer=Mozilla%20Foundation;serial=0000000000000000;model=NSS%203

  2. Root Certs
        library name: ./nssckbi.dll
           uri: pkcs11:library-manufacturer=Mozilla%20Foundation;library-description=NSS%20Builtin%20Object%20Cryptoki%20Modu;library-version=2.16
         slots: 1 slot attached
        status: loaded

         slot: NSS Builtin Objects
        token: Builtin Object Token
          uri: pkcs11:token=Builtin%20Object%20Token;manufacturer=Mozilla%20Foundation;serial=1;model=1
-----------------------------------------------------------



[With FF 56.0.1:]
---------------

C:\Program Files (x86)\santesocial\srvsvcnam\firefox>modutil.exe -dbdir C:\Users\ValidDIP\AppData\Roaming\Mozilla\Firefox\Profiles\zqtkqc1e.default-1507881335827 -list

Listing of PKCS #11 Modules
-----------------------------------------------------------
  1. NSS Internal PKCS #11 Module
           uri: pkcs11:library-manufacturer=Mozilla%20Foundation;library-description=NSS%20Internal%20Crypto%20Services;library-version=3.33
         slots: 2 slots attached
        status: loaded

         slot: NSS Internal Cryptographic Services
        token: NSS Generic Crypto Services
          uri: pkcs11:token=NSS%20Generic%20Crypto%20Services;manufacturer=Mozilla%20Foundation;serial=0000000000000000;model=NSS%203

         slot: NSS User Private Key and Certificate Services
        token: NSS Certificate DB
          uri: pkcs11:token=NSS%20Certificate%20DB;manufacturer=Mozilla%20Foundation;serial=0000000000000000;model=NSS%203

  2. Root Certs
        library name: ./nssckbi.dll
           uri: pkcs11:library-manufacturer=Mozilla%20Foundation;library-description=NSS%20Builtin%20Object%20Cryptoki%20Modu;library-version=2.16
         slots: 1 slot attached
        status: loaded

         slot: NSS Builtin Objects
        token: Builtin Object Token
          uri: pkcs11:token=Builtin%20Object%20Token;manufacturer=Mozilla%20Foundation;serial=1;model=1
-----------------------------------------------------------


Same if I compare Logs of process monitor. See attached Logfilexxx.PML

Regards.

Comment 9

[suivios]

Attachment: Logfile FF56.0.1.PML

Comment 10

[suivios]

Attachment: Logfile FF57.PML

Comment 11

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

Thanks! I think I may have figured out the issue - do you have a file called "nssckbi.dll" in C:\Users\ValidDIP\AppData\Roaming\Mozilla\Firefox\Profiles\zqtkqc1e.default-1507881335827 ? Unless I'm mistaken, it shouldn't be necessary to have that file there (it should already be in the application directory for Firefox - C:\Program Files\Mozilla Firefox). Using the NSS command-line utilities appears to cause a problem by essentially adding that file to your profile's PKCS#11 module DB (at startup, Firefox tries to load that exact same library by another means, which fails). You could try moving, renaming, or deleting that file. In any case, we'll update Firefox to handle this situation better.

Comment 12

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

Attachment: bug 1406396 - work around NSS utils potentially loading spurious root cert modules

NSS command-line utilities may add a built-in root certificate module with the
name "Root Certs" if run on a profile that has a copy of the module file (which
is an unexpected configuration in general for Firefox). This can cause breakage.
To work around this, PSM now simply deletes any module named "Root Certs" at
startup. In an effort to prevent PSM from deleting unrelated modules
coincidentally named "Root Certs", we also prevent the user from using the
Firefox UI to name modules "Root Certs".

Review commit: https://reviewboard.mozilla.org/r/189298/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189298/

Comment 13

[Mark Goodwin [:mgoodwin]]

Comment on attachment 8918454 [details]
bug 1406396 - work around NSS utils potentially loading spurious root cert modules

https://reviewboard.mozilla.org/r/189298/#review194824

Looks good to me.

Comment 14

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

Comment on attachment 8918454 [details]
bug 1406396 - work around NSS utils potentially loading spurious root cert modules

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189298/diff/1-2/

Comment 15

[suivios]

Thanks for your help.

We confirm the "Root Certs" certificate module is the cause of our problem.
The work around looks good to us too.

Now we hope to find it in Firefox 57, if possible.

Best Regards.

Comment 16

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

Comment on attachment 8918454 [details]
bug 1406396 - work around NSS utils potentially loading spurious root cert modules

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189298/diff/2-3/

Comment 17

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

Comment on attachment 8918454 [details]
bug 1406396 - work around NSS utils potentially loading spurious root cert modules

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189298/diff/3-4/

Comment 18

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

Ok, couple of false-starts, but try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=573020d9e24c

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3a4bc559c9c
work around NSS utils potentially loading spurious root cert modules r=mgoodwin

Comment 20

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/e3a4bc559c9c

Comment 21

[Ryan VanderMeulen [:RyanVM]]

This sounds potentially crappy. Should this be nominated for Beta approval or can it ride the 58 train?

Comment 22

[suivios]

Hi, 
If there is no correction in Firefox 57 we will have 35000 health professionals impacted by this problem.

Today, several applications are deployed using certutil tool. So Many health professional will have this problem when Firefox will be automatically updated to Firefox 57.

If it's possible, thanks to integrate the correction to Firefox 57 Beta for approval.

Regards,

Comment 23

[Sylvestre Ledru [:Sylvestre]]

yeah, we probably want that.
I nied the reviewer too to get an uplift request asap.

Flod, there is a new string. wdyt?

Comment 24

[Francesco Lodolo [:flod]]

I'm honestly not thrilled by landing strings these late in the cycle, but they also seem hidden enough.

Comment 25

[Mark Goodwin [:mgoodwin]]

Comment on attachment 8918454 [details]
bug 1406396 - work around NSS utils potentially loading spurious root cert modules

Approval Request Comment
[Feature/Bug causing the regression]:
See the commit message on https://reviewboard.mozilla.org/r/189298/

[User impact if declined]:
Opening Fx 57 with this issue results in certificates being lost from a user's profile. We know of users that will be affected by this issue (see Comment 22)

[Is this code covered by automated tests?]:
Yes.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
Unknown
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:
Two new errors are added to the dialog for loading PKCS #11 modules. It's pretty well hidden.

Comment 26

[Sylvestre Ledru [:Sylvestre]]

Could you please explicit the following items?
----
[Is the change risky?]:
[Why is the change risky/not risky?]:
----

Comment 27

[Sylvestre Ledru [:Sylvestre]]

Seems that it doesn't apply to m-b:

grafting 430835:e3a4bc559c9c "bug 1406396 - work around NSS utils potentially loading spurious root cert modules r=mgoodwin"
merging security/manager/ssl/PKCS11ModuleDB.cpp
 warning: conflicts while merging security/manager/ssl/PKCS11ModuleDB.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

Comment 28

[Mark Goodwin [:mgoodwin]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #26)
> Could you please explicit the following items?
> ----
> [Is the change risky?]:
> [Why is the change risky/not risky?]:
> ----

I don't think it's risky but I'll defer to :keeler

Comment 29

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

It's not very risky. My only concern is unintentionally removing unrelated PKCS#11 modules that happened to be called "Root Certs" (the user could always add it again under a different name). I'm not sure how likely that is, though. I think it's rare.

Suivios, can you provide any insight as to why the "nssckbi.dll" file is in the profile directory? That's not a normal configuration for Firefox to be in (and I believe removing that file would fix this problem independently of code changes in Firefox).

In any case, I have a rebased patch - I'm just making sure it compiles/passes the PSM tests first.

Comment 30

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

Attachment: 1406396-Root-Certs-beta.diff

Comment 31

[suivios]

We check but the "nssckbi.dll" file is not in the profile directory but only in the certutil directory.

This DLL is a part of the certutil directory tool. 

We just test without it and it seems that we don't need "nssckbi.dll" file.

If this DLL is not necessary we will remove it to avoid to install the Certs root module.

Comment 32

[Daniel Veditz [:dveditz]]

Which is likely to be more common: folks using certutil with an extra nssckbi.dll hanging around, or folks legitimately adding a "Root Certs" module? This can go bad both with and without the fix. If we don't fix this in 57 how hard is it for people to fix this? Just delete the offending nssckbi.dll? or re-run certutil to correct the error?

Comment 33

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

Well, given that we did get a report of this sort of error (i.e. the extra nssckbi.dll issue), and given that if users had tried to add an unrelated "Root Certs" module and had also run certutil with the extra nssckbi.dll, adding the module would have already failed for them (in earlier versions), I think users naming a module "Root Certs" is the less likely scenario.

That said, the workaround is reasonably easy, given that they've already run certutil:

1. delete the extra nssckbi.dll
2. run `modutil.exe -dbdir {their profile directory} -delete "Root Certs"`

or:

1. delete the extra nssckbi.dll
2. delete secmod.db in their profile (a bit more extreme - will also remove other loaded PKCS#11 modules)

Comment 34

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8918454 [details]
bug 1406396 - work around NSS utils potentially loading spurious root cert modules

Dan and I chatted about this one, while the fix is not ideal, I'll take it especially given the workaround Keeler mentioned, Beta57+

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/86534d5daeef

Comment 36

[Andrei Vaida [:avaida]]

(In reply to Mark Goodwin [:mgoodwin] from comment #25)
> [Is this code covered by automated tests?]:
> Yes.
> 
> [Has the fix been verified in Nightly?]:
> Yes.
> 
> [Needs manual test from QE? If yes, steps to reproduce]: 

Setting qe-verify- based on Mark's assessment on manual testing needs and the fact that this fix has automated coverage.