147280 - Ensure Mozilla uses the shipped nssckbi or a newer builtin roots module

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[Michael Coburn]

I had folders for Mozilla (talkback/zip) from 0.9.6 up to RC3 stored in
c:\Program Files\Mozilla\. On trying to delete all folders prior to rc3 while
rc3 was running I found that 0.9.6 could not be deleted as nssckbi.dll was in use.

I quit Mozilla and tried to delete again with no success.

I then quit QuickLaunch and was able to delete the 0.9.6 folder, including
nssckbi.dll.

Not sure if instability I was experiencing was due to Mozilla grabbing this
older dll file.

Comment 1

[John Morrison]

Confirming this. I just had a build fail because nssckbi.dll in d:\build_dir
was in use by a netscape exe location in e:\install_dir (i.e., two completely
independent installations, or should be).

I see the the patch 'd:\build_dir\nssckbi.dll' is hard-persisted into the 
file 'secmod.db' in the profile that I was using. I note that bug 128290
is touching on this issue (may even be this issue to some degree). 

This seems like something that Netscape should consider resolving for rtm.
It seems like a good recipe for instability to mix dll's from different 
releases. Note that the reporter found RC3 mozilla to be using 0.9.6 version
of that DLL. 

(The counter argument would be that users are not "expected" to have multiple 
installations, but I don't think I buy that argument).

Comment 2

[Wan-Teh Chang]

Bob, could you look at this bug?

Comment 3

[John Morrison]

cc: junruh for client qa

Comment 4

[John Unruh]

I recommend this to be an RTM stopper.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

I think this is another argument against storing nssckbi.dll with the 
secmod DB, and instead storing it with the the other NSS DLLs.  
When a new version of a product is installed, the version of nssckbi
should also be updated automatically.  

Comment 6

[Robert Relyea]

This bug has nothing to do with the problem nelson describes as mozilla doesn't
store nssckbi with the profile, but stores it with the DLLs.

NSS's automatic lookup of nssckbi is actually interfering with the semantics
mozilla wants.

bob

Comment 7

[Wan-Teh Chang]

Moved to NSS 3.6.

Comment 8

[Kai Engert [:KaiE:]]

Let me try to summarize:

- nssckbi is stored in the same place where all shared libraries are stored.
That's fine.

- secmod.db is reported to store a filename to the nssckbi library, which
includes the full path. Do I understand that correctly?

Without knowing more details, I'd suspect:

- When secmod.db is created, or when it needs the nssckbi the first time, the
nssckbi is located, and the filename of the found file is remembered in the db.

- Later, when a user executes a different version of Mozilla, installed in a
different directory on the same computer, the same profile with the same
secmod.db can be used. If that is done, NSS seems to load the library that is
referenced in the secmod.db. But that's not good. It should not use the nssbkci
library as referenced in the secmod.db, because that could point to nssbkci of a
different installation from what is currently being executed.

In simpler words:

c:\mozilla0.9.9\ contains a 0.9.9 installation

c:\mozilla1.0.0\ contains a 1.0.0 installation

The user has only one profile.

secmod.db in the user's profile points to c:\mozilla0.9.9\nccbkci.dll

Regardless of whether the user executes 0.9.9 or 1.0.0, NSS always loads the
0.9.9 nssbkci.dll

Did I understand that correctly?


John suggested this bug to become a RTM stopper. I suggest he did so, because he
fears, a mixture of incompatible DLLs in memory could lead to instability.


I think this bug suggests, that the behaviour of NSS should change.

Actual behaviour: NSS looks up nssbkci once, and continues to use that library
forever, regardless of whether NSS is being run from a different directory.

Expected behaviour: NSS should again lookup the correct nssbkci library each
time an application session is started.

Comment 9

[John Morrison]

Yes, that's a good summary of the problem. Specifically, the full path to 
nssckbi.dll is stored in secmod.db of a user's profile, and when running
mozilla with that profile, that version of nssckbi.dll will be loaded
regardless of what binary version and location of mozilla is running.

So, a user can wind up running an obsolete version of nssckbi.dll with their 
brand-new installation of Netscape 7.

Now, if NSS can say, without a doubt, that this can present absolutely no
problems then, well, I guess that's "ok" then.

But I don't think that anybody can really claim that running downlevel bits 
with the rest of the current shipping code is "ok", and I do think that this 
needs to addressed before we ship Netscape 7.

Comment 10

[Stephane Saux]

It's up to the application to do the logic of what to do.  I believe that the
nssckbi.dll has a version number. The application should compare the version
number and make sure that it's compatible with the binary currently executing.

Note that PSM will detect that the full path loaded in secmod.db is not there
and load the last one.

Note also that incompatibility between mozilla version do not affect only the
security databases. If you open profiles with widely different versions of
Mozilla, and you do so back and forth (i.e., upgrade, then downgrade, and so
forth), you're liable to create problems.
Software often provides an upgrade path (you run newer software on old data and
it works), but very rarely provides an downgrade path.
The problem is that many people install a new version of Mozilla in a different
place (just to try it), then open an old profile, but may decide to go back down
to a previous more stable version.

For the security database, if we change future versions of the product to
automatically upgrade the version of nssckbi.dll to the one for the current
executable, then that profile may become incompatible with previous versions of
mozilla, which will not do the same thing.  In this scenario, trying a new
version of an executable just once could mean that the a profile cannot be run
by older versions of Moz.

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

nssckbi.dll uses the well-defined PKCS#11 binary interface standard.
The DLL exists solely to provide a list of well-known CA certificates.
The only problem with having a new version of an application running
with an old version of nssckbi is that it will not have access to the 
newest (most recently added) root CA certificates.  Nonetheless, I 
think this problem should be fixed, in NSS if possible.

Comment 12

[Julien Pierre]

Nelson,

While it's true that the DLL provides the root certificates through the PKCS#11
interface, the new version provides new roots, and fixes problems in the PKCS#11
modules such as memory leaks. In addition, the module supports trust, unlike
standard PKCS#11 modules. It would be preferable to ensure that the correct
version of nssckbi is loaded with the version of NSS for which it was produced.
This can be managed at the application level if nssckbi.dll is not in the path.
That would prevent NSS from automatically searching for nssckbi. So PSM could
look for the module, load it, and check the version.

Comment 13

[Robert Relyea]

Ok, there's some misinformation here about what NSS does and does not do and
what NSS should and should not do that needs to be cleared up.

First, nssckbi.dll is a PKCS #11 standard library, which is the 'loadable
built-ins'. It is meant to be loaded into old versions of NSS and still work.
NSS in turn is supposed to work with old versions of nssckbi.dll. So there isn't
a functional compatibility problem, as Nelson points out.

Second, NSS has a feature, which was meant to help applications which were built
pre-loadable root certs, where NSS will automatically load the built-ins if no
built-in module is loaded, and the built-ins module is found in the profile
directory (the only directory NSS has access to). 

Third, PSM/mozilla/Netscape 6,7 have never used the above feature NSS has no
idea where the built-ins for these programs are supposed to be because NSS has
no idea where the shared libraries for these applications are supposed to be
stored. Since PSM does not place the shared library in the profile directory
(quite sensibly), NSS does not automatically find the built-ins module. Instead,
NSS provides an API so PSM can tell if the built-ins have been loaded, and if
not, PSM can add to built-ins just like any other PKCS #11 module. From then on
NSS will load the build-ins module the same way it will load your smart card
driver. PSM is told the build-ins have been loaded and continues on it's merry way.

So what should happen:
I had already work this out with David several months ago, unfortunately it had
not been recorded in this bug or passed on to Kai, so now we have a last minute
stop ship (sigh). 

PSM should check the built-ins module and NSS loaded on startup. If the version
number of the built-ins is older than the version number PSM expects, PSM should
delete the module and install it's own. This will roll nssckbi.dll modules
forward without having them back out. We want applications to use the latest
version of nssckbi.dll no matter how old the application is. That is because the
latest version will have the newest roots in them.

Kai, if you have any questions on how to make this happen, let me know. If there
is something missing in NSS (which there could be since David didn't actually
sit down and implement the proposal) let me know and I'll attach patches to this
bug.

bob

Comment 14

[Stephane Saux]

I discussed this with Bob Relyea.
Let's fix it according to Bob's suggestions in the Buffy time frame. This is not
a stop ship.

Comment 15

[Stephane Saux]

John,
  Can you test that a profile created with N6.0 can still be opened with a machV
release.  Can you report which set of CAs the profile will use (using the old
set is not a stop-ship as long as the product loads and shows a set of CAs)?
  Can you test the same thing with a profile created with 6.1/6.2?

  You should test that the cert manager ca tab shows CA certs.
  you should test that the device manager shows a root ca module.
  You should test that basic SSL works.

  Whom should we ask the following questions:

  When the installer runs, is the default install location the same as N6? N6.1?
N6.2?.  Will the software honor such locations or will it create parallel folders.


  If the software runs (which is my expectation) the only visible effect to the
user will be that the list of CA cert will reflect the version of the software
that was used to create the profile as long as that version of the software is
installed. Uninstalling that version will resolve the problem.
  Having a different list of CAs should have minimal impact. Most current CA's
were also present in earlier versions of the software, and that's certainly true
of the most commonly used issuers.

Comment 16

[Stephane Saux]

I created a N6.0 profile (do not try this, it hid all my profiles, which I had
to work hard to salvage.) When I then started N7, the only profile there was
that profile, and it migrated fine to N7 and the newer N7 nssckbi.dll was there.
It may be because N6.0 didn't have PSM installed by default (I was unable to
visit ssl sites.)
I'm not sure how conclusive this test is.

Comment 17

[John Unruh]

Here are the test results with every point release - 6.0, 6.01, 6.1, 6.2, 6.2.1, 
6.2.2, 6.2.3, 7.0 PR1.
1.) Upgrading to 7.0 RTM into a new directory, using a profile created with the 
old product:
6.0 and 6.01 now have the new CAs
6.1 and higher have the old CAs
2.) Upgrading to 7.0 RTM into the same directory as the old product, using a 
profile created with the old product:
6.0 and 6.01 - installation fails.
6.1 and higher have the new CAs.
3.) Conclusion: A fix will need to be made to use the new CAs when upgrading 
from 6.1 and higher when installing into a new directory.

Comment 18

[Stephane Saux]

The installation failure has nothing to do with the root ca. Is this correct?


To summarize.
  We haven't found a situation where the software doesn't run because of this bug.
  Most users will not run into this problem (because they don't have any N6X
installed).
  Some users will end up using the old set of roots.
     Most of these users will not be affected, as most roots are the same as
before (certainly true of the ones most commonly used).
     Some users may experience warning messages like "Untrusted issuers" when
connnecting to sites with SSL certs issued by a cert that is new in N7. They can
still visit these sites.

At any time a user using an old set of root may repair his deployment by
deciding to remove an old version of the software, creating a new profile, etc...

Comment 19

[Robert Relyea]

.... or by explicitly removing the built-ins and restarting.

bob

Comment 20

[Robert Relyea]

I'm assigning this to Kai since there is no actual NSS work here. If kai needs
something from NSS, then we can open up a separate bug. Kai, you probably want
to change the component and target as appropriate for PSM.

Comment 21

[Kai Engert [:KaiE:]]

Bob,

re-reading comment 13, the following seems to be required, and if you give me
some pointers it sounds simple to do.

- make PSM include a compile time version number of the DLL

  Does NSS provide an identifier with that version number already?


- right after PSM has called the NSS init routines,
  compare the compile time expected version number with the
  loaded version number

  How can I query the loaded version number?


- if the versions are different, unload the existing one
  (using the delete module function?
   which function to obtain a handle to the loaded module?)
  find the application installation directory 
  (using Mozilla logic)
  and load the newest nssckbi from there.
  (which function would I use to load?)


If you know the answers right away, that would be great, otherwise I have to
research the answers. Thanks!

Comment 22

[Kai Engert [:KaiE:]]

Bob, I thought you were cc'ed already, but you were not. :)

Could you please have a look at my questions in comment 21?
Thanks.

Comment 23

[Robert Relyea]

The version number is currently in nss/lib/ckfw/builtins/constants.c
I suspect we should put it in an exported versions file for you eh?

You can query it at run time by using the PK11_GetModInfo call and looking at
the library version number.

bob

Comment 24

[Kai Engert [:KaiE:]]

cc'ing ddrinan and rangansen FYI
This bug is to make sure the PSM will use the latest version of the builtin
roots module will be used, even if NSS has already remembered a DLL in a
different location.

Comment 25

[Kai Engert [:KaiE:]]

I filed bug 165297 to request the exported version number information.

Comment 26

[Kai Engert [:KaiE:]]

*** Bug 102631 has been marked as a duplicate of this bug. ***

Comment 27

[Kai Engert [:KaiE:]]

Attachment: Patch v1

This patch assumes library version will be defined by
  NSS_BUILTINS_LIBRARY_VERSION_MAJOR
  NSS_BUILTINS_LIBRARY_VERSION_MINOR

The patch needs to be updated when the final symbol names and header file names
are known.

The patch can not land until it's dependent bugs have been marked fixed.

Comment 28

[Kai Engert [:KaiE:]]

Attachment: Patch v1b

Comment 29

[Kai Engert [:KaiE:]]

Javi, can you please review?

Comment 30

[Javier Delgadillo]

Comment on attachment 101372 [details] [diff] [review]
Patch v1b

You don't seem to free the list returned by SECMOD_GetDefaultModuleListLock
anywhere.  Are we not supposed to free that memory?

If not, r=javi

Comment 31

[Kai Engert [:KaiE:]]

> You don't seem to free the list returned by SECMOD_GetDefaultModuleListLock
> anywhere.  Are we not supposed to free that memory?

Looking at code in pk11slot.c, that code isn't freeing anything either.
If I understand correctly, no memory is being allocated, no reference counts
increased. We obtain pointers to global objects.

Someone please correct me if I'm wrong.

Comment 32

[Wan-Teh Chang]

Kai is right.  We are not supposed to free the list returned
by SECMOD_GetDefaultModuleList and the lock returned by
SECMOD_GetDefaultModuleListLock.  Kai, you might want to add
a comment about that because we are now very concerned about
NSS object reference leaks.

Comment 33

[Daniel Veditz [:dveditz]]

Comment on attachment 101372 [details] [diff] [review]
Patch v1b

sr=dveditz

Comment 34

[Asa Dotzler [:asa]]

Comment on attachment 101372 [details] [diff] [review]
Patch v1b

a=asa for checkin to 1.2beta (on behalf of drivers)

Comment 35

[Kai Engert [:KaiE:]]

Fixed on trunk.

Comment 36

[Kai Engert [:KaiE:]]

marking fixed

Comment 37

[John Unruh]

Verified.

Comment 38

[Kai Engert [:KaiE:]]

*** Bug 176501 has been marked as a duplicate of this bug. ***