Closed Bug 378489 Opened 17 years ago Closed 17 years ago

Add multiple new roots to NSS

Categories

(NSS :: Libraries, defect)

3.11.7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
3.11.8

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

Attachments

(5 files)

This bug is track the technical work to get 3 new roots added to NSS trunk and the next version of NSS, version 3.11.7
Attached patch Patch v1Splinter Review
This patch adds 6 new root CA certificates to NSS, as explained in the dependent bugs.

I propose to land this patch on both NSS trunk and NSS 3.11 branch, targetting inclusion in NSS 3.11.7

(In addition to this patch, at check in time, I will run "gmake generate" on each branch, to automatically generate the certdata.c files.)
Assignee: nobody → kengert
Status: NEW → ASSIGNED
Attachment #262529 - Flags: superreview?(rrelyea)
Attachment #262529 - Flags: review?(nelson)
This second patch shall be applied to NSS 3.11 branch, only.
It increments the version number of the roots module.
Attachment #262530 - Flags: superreview?(rrelyea)
Attachment #262530 - Flags: review?(nelson)
FYI, I used the following commands to add the certs:

addbuiltin -n "GlobalSign Root CA - R2" -t C,C,C < ~/moz/nss/trunk/ca/globalsign-r2-378161.der >> certdata.txt
addbuiltin -n "DigiCert High Assurance EV Root CA" -t C,C, < ~/moz/nss/trunk/ca/digicert-high-378161.der >> certdata.txt
addbuiltin -n "DigiCert Global Root CA" -t C,C, < ~/moz/nss/trunk/ca/digicert-global-378161.der >> certdata.txt
addbuiltin -n "DigiCert Assured ID Root CA" -t C,C, < ~/moz/nss/trunk/ca/digicert-assid-378161.der >> certdata.txt
addbuiltin -n "QuoVadis Root CA 3" -t C,C,C < ~/moz/nss/trunk/ca/quovadis-ca3-378161.der >> certdata.txt
addbuiltin -n "QuoVadis Root CA 2" -t C,C,C < ~/moz/nss/trunk/ca/quovadis-ca2-378161.der >> certdata.txt


I tested this new code by compiling a libnssckbi.so and using it with a SeaMonkey build. All new certs correctly showed up with the right name and fingerprint and trust, as indicated in the dependent bugs.
I would like to add:

One of the new roots describes itself as a "High Assurance EV" root cert.

As of today, we don't evaluate additional HA/EV flags, so as of today this root cert will be used as a traditional SSL cert only.
Reverting dependencies. (Thanks to Nelson for making me aware of the mistake)
Blocks: 378161, 378162, 378163
No longer depends on: 378161, 378162, 378163
This discussion concerns the second patch attached to this bug.

There is an issue here for which we need to think and plan carefully.
I request that Gerv and Wan-Teh participate in resolving this issue.

We have a version numbering system for nssckbi.  nssckbi has its own 
version number.  For NSS 3.11, nssckbi's version was 1.70.  
For NSS 3.12, it is presently planned to be 1.80.  This has been our
strategy for those version numbers, we skip to the next multiple of 10
for each new "minor" release, leaving room in the number space for up 
to 9 revisions of nssckbi between minor releases.  

In the NSS 3.11.x "micro" releases, there have already been 3 revisions
to nssckbi since 3.11.0, and the second patch above bumps it again to 1.74.
That leaves us with only 5 more subsequent revisions before we run out of
available version numbers.  

I perceive that Gervase is making great progress in reducing the backlog
of root CA inclusion requests.  I expect that over the next few months,
Gerv will file a stream of new NSS bugs, each requesting the addition of 
one or more root CA certs to nssckbi.  That's GOOD!  (Thanks in advance,
Gerv!)  

At the present time, we seem to be making a new NSS "micro" release 
approximately every month.  Let us imagine that this pattern continues,
and that NSS 3.12 is not released for another 6 months (worst case?),
and that we add a few more root certs into each NSS 3.11.x release during
that time, as Gerv approves more and more roots.  Then we will run out of 
space in the nssckbi version numbering system before we release 3.12.  

Also, we may need to keep 3.11.x alive for some time after we release 3.12,
(especially if 3.12 has backwards compatibility problems due to new cert DBs
and new cert validation implementation), so we may need to add root certs to 
the 3.11 branch (and bump the nssckbi version) even after 3.12 is released.  

I think there are (at least) 6 things we could do about this:

A) keep adding new certs to every 3.11.x release until we run out of 
   version number space, then stop adding new certs to 3.11.x.

B) Intentionally delay the introduction of new root certs, to reduce the 
   frequency with which we bump the version number.  E.g.  Maybe we only 
   introduce new CA certs no more often than once every 90 days.

C) Bump the version number for NSS 3.12 from 1.70 to 1.80 NOW, giving us 
   room in the number space for 10 additional revisions to nssckbi in 3.11.x.

D) Keep an eye on those version numbers, and if the 3.11.x version gets 
   too close to 1.70 before we release 3.12, THEN bump the 3.12 version 
   number of nssckbi to 1.80 at some later time, BEFORE we relase NSS 3.12. 

   For proposal D, we must decide how close is "too close".  
   Are we too close when there are only 2 unused numbers left?  or 3? or 5?
   Can we predict how many more 3.11.x releases will be needed after 3.12 ships?  
   If we don't do this now while we're thinking about it, will we forget to 
   do this later?  

E) Let 3.12 ship with version 1.70.  Then, if and when NSS 3.11.x version
   numbers need to get bumped above 1.69, bump them up to 1.71 or 1.72, 
   skipping over any version numbers already used by 3.12.x, and let the 
   two releases play "leap frog" with version numbers after that.  
   (I wonder if that translates.  Do children play "leap frog" in the UK?
    or in Germany?)

F) Force all the branches (and trunk) to stay ABSOLUTELY in sync with the 
   set of certs they include and their version numbers.  Allow the same
   version number to be used on both 3.11.x and 3.12.x, because the contents
   of (say) nssckbi revision 1.70 will be absolutely the same on branch and
   trunk.  Is that insane?

Gerv, it might help if you could attempt to predict for us how much longer
it will take to clear the root CA request backlog.   Also, would you care
to speculate on what percentage of the existing requests will receive 
approval?  

If you see any obvious problems with any of the above proposals, please 
speak up.  I like C and F the best.  I think C has the least risk.  
(In reply to comment #6)
> We have a version numbering system for nssckbi.  nssckbi has its own 
> version number.  For NSS 3.11, nssckbi's version was 1.70.  
> For NSS 3.12, it is presently planned to be 1.80.  

IIUC you made a minor mistake:
3.11: 1.60
3.12: currently planned to be 1.70

In the remainder of your comment you were using correct numbers.


I don't like A, because we potentially might need to remove certs.

Variations of B are an option, by asking Gerv to add CAs in larger chunks. I think we could look at that later, after we get a feeling about the backlog that will be addressed short term.

I like C best. In the unlikely event that NSS 3.12 RTM gets delayed very much, we could even bump it again to 1.90 ;-)

D is a variation of C. Because 3.11.x seems to become a long live branched, it seems reasonable to me to do it now.

I don't like E, it seems confusing and I expect we'd be making mistakes.

F seems like a good idea that we could keep in mind as a backup plan.
Nelson, we should also investigate the answer to the following question, which appears in file nssckbi.h :

> NSS_BUILTINS_LIBRARY_VERSION_MINOR is a CK_BYTE.  It's not clear
> whether we may use its full range (0-255) or only 0-99 because
> of the comment in the CK_VERSION type definition.
In reply to comment 7, you're right.  It was an "off by 10" error.  :-/

In reply to comment 8, I propose that we take a two-prong attack on this
question.  
#1) empirical: Kai, please create an nssckbi shared library that has version
    1.200, and let's try it out and see if it works, and if the version
    number appears correctly in all places where it appears (where DOES it 
    appear?).  It would be VERY GOOD if it "just works".  

#2) rational: Let's ask our chief PKCS#11 expert, Bob Relyea, to tell us 
    if there are any flaws with that plan that are unknown and inobvious 
    to us more casual PKCS#11 developers.  Bob?

If we find that we cannot use minor numbers greater than 99, then I guess 
we switch to library versions starting with 2.00 when we run out of 1.xx
numbers.  I'm not aware of any serious problems with doing that to the 
library version number.  Any one else?  Bob?  
I don't like A, B, D or E. But yes, children play "leap frog" in the UK. :-)

It's all just internal numbers, right? So if we are doing C), then why not (as Nelson suggests) just bump the 3.12 version number to 2.00 and be done with it? And then increase the major number with each future NSS release (3.13, 3.14 etc)?

I also like the idea of F). If the NSS team are always happy to take new roots on any branch, even stable ones, then there's no reason F wouldn't work. And it would make it much, much easier to track which certs were in which releases or branches - because the set of included certs would be defined precisely by a single number, the nssckbi version. Does anyone know if that number is exposed in the UI of any NSS-using products?

To answer Nelson's questions about time and percentage: I anticipate that it will take a few more months to clear the backlog. I consider it highly unlikely that less than 80% of the current applicants will be successful. There are currently 23 open request bugs, including one each for the three changes this bug relates to.

Gerv
We will just reach the midpoint of the range reserved for
NSS 3.11.x.  It's easier to reduce the frequency with which
we add new root CA certs.
wtc: I don't understand your first sentence - can you give it more context?

As for your second, I feel I would be laughed at if some CA asked me "Why is my cert not in this new NSS release, given that it's been approved?", and I said to them "Because we're trying to conserve internal version numbers for the library, and have decided not to shift up the ones planned for the next release to make room".

Are there technical ramifications to C) that I've missed that might make it not as easy as it looks?

Gerv
Gerv, I think with his first sentence, wtc suggests to us, we shouldn't be *that* worried about version numbers at this time, because we've only reached the middle of the pre-assigned version number range.
OK... but Nelson is right, in that if things continue as planned, we will run out of numbers. And so, if we don't want to reduce the frequency of inclusion (and I suggest that we don't) then we do need a plan.

(Note: one more CA has made it through the process - Keynectis, whose inclusion bug is 379032. Once they confirm the details, they could be rolled into this patch to save version numbers.)

Gerv
Comment on attachment 262530 [details] [diff] [review]
Additional Patch for 3.11 branch only

r=nelson.  This patch is the correct patch for bumping the version number to the next value.  

Checkin approval for this patch depends on the existence of at least one other reviewed CA change patch for this release.
We should try to get as many new CA certs as we can into any new NSS release in which there are any new certs.  
We should not bump the version number in any releases in which there are no new certs, obviously.
Attachment #262530 - Flags: review?(nelson)
Attachment #262530 - Flags: review?(alexei.volkov.bugs)
Attachment #262530 - Flags: review+
Attachment #262530 - Flags: review?(alexei.volkov.bugs) → review+
Comment on attachment 262529 [details] [diff] [review]
Patch v1

Some comments about this review, the aspects of the patch that were
and were not reviewed:

1. I did not review the contents of the certificates themselves for 
correctness.  That is the job of the CAs themselves, to ensure that
the CA cert that went in were the ones that they and Mozilla 
Foundation previously agreed were the right ones.

2. I did not review the contents of the certificates themselves for 
conformance with Mozilla Foundation's root CA cert policy.  That job 
is supposedly done before NSS is asked to add the cert to the module.

3. I did review the patch to ensure that it correctly added the trust
flags that were approved by Mozilla Foundation.

4. I reviewed the certs to ensure that they did not contradict the 
approved trust flags.  (We once were asked to give code signing 
trust to a cert whose EKU explicitly forbade that use.)

5. I reviewed the Nicknames for the certs in the patch, to ensure
that they match the expected values given in the relevant RFEs.

6. There are some parts of this patch that cannot be reviewed by 
inspection, and require actually building and testing the code to 
complete the review.  These are the octal strings containing info
that is only used for the PKCS#11 API, such as the Subject Name,
Issuer Name, and Serial Number fields that are stored separately
from the cert itself.  No existing tool or Mozilla UI servers to 
facilitate the checking of these values.  We could devise a test
tool for that purpose, one that used the PKCS#11 API and read out
and verified that the values of these separate PKCS#11 attributes
match the values in the cert itself.  But I do not propose to do 
that for the review of this patch.  Instead I will trust that
a) the addbuiltin program did its job correctly, and
b) no deliberate or accidental changes were made to the output 
of that program.  
I guess I could rerun the program to double-check that the 
output matches.  If anyone reading this comment thinks that is 
(or should be) a necessary part of the review, please comment 
to that effect.

r+=nelson
Attachment #262529 - Flags: review?(nelson) → review+
(In reply to comment #9)
> #1) empirical: Kai, please create an nssckbi shared library that has version
>     1.200, and let's try it out and see if it works, and if the version
>     number appears correctly in all places where it appears (where DOES it 
>     appear?).  It would be VERY GOOD if it "just works".  

What failure might I see?
If cert manager in Firefox with such a root module still displays root certs and their trust flags, and connecting to a secure site works, can we conclude it works?

> #2) rational: Let's ask our chief PKCS#11 expert, Bob Relyea, to tell us 
>     if there are any flaws with that plan that are unknown and inobvious 
>     to us more casual PKCS#11 developers.  Bob?

Sure, if he happens to know that, it helps.

> If we find that we cannot use minor numbers greater than 99, then I guess 
> we switch to library versions starting with 2.00 when we run out of 1.xx
> numbers.  I'm not aware of any serious problems with doing that to the 
> library version number.  Any one else?  Bob?  

I like that idea. 
addbuiltin -n "Certplus Class 2 Primary CA" -t C,C, < ~/moz/nss/trunk/ca/certplus-379032.der >> certdata.txt

This additional patch adds the cert from bug 379032.
Attachment #263045 - Flags: superreview?
Attachment #263045 - Flags: review?(nelson)
Attachment #263045 - Flags: superreview? → superreview?(rrelyea)
Attachment #263045 - Attachment is patch: true
Attachment #263045 - Attachment mime type: application/octet-stream → text/plain
Blocks: 379032
Summary: Add 3 new roots to NSS → Add multiple new roots to NSS
Comment on attachment 262529 [details] [diff] [review]
Patch v1

requesting additional reviewers to increase the probability of this patch making it into NSS 3.11.7
Attachment #262529 - Flags: review?(alexei.volkov.bugs)
Comment on attachment 263045 [details] [diff] [review]
Additional Patch v2

r=nelson.  
My comment 16 above applies to this patch also.
Requesting additional reviewers to facilitate NSS 3.11.7
Attachment #263045 - Flags: review?(nelson)
Attachment #263045 - Flags: review?(alexei.volkov.bugs)
Attachment #263045 - Flags: review+
Attached file cert list differences
Comment on attachment 262529 [details] [diff] [review]
Patch v1

looks ok. See attachment 263270 [details]
Attachment #262529 - Flags: review?(alexei.volkov.bugs) → review+
Attachment #263270 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 263045 [details] [diff] [review]
Additional Patch v2

looks ok.
Attachment #263045 - Flags: review?(alexei.volkov.bugs) → review+
re kaie's question about range:

Version numbers are pure unsigned byte integers, not strings, so 0-255 is the permissible range.

I'm in favor of leapfrogging 3.12. If we think we are getting close to 255 we can bump the major  version.

bob
Attachment #262530 - Flags: superreview?(rrelyea) → superreview+
Attachment #263045 - Flags: superreview?(rrelyea) → superreview+
Attachment #262529 - Flags: superreview?(rrelyea) → superreview+
Given Bob's comment that "0-255 is the permissible range", 
I think we should go ahead and commit these new CA certs now.
Blocks: 289979
I apologise to all the CAs concerned for the delay here.

I have attached a new version of nssckbi.dll which contains the new roots that this bug covers. CAs who own one of these roots need to:

- Download and install Firefox 2.0.0.3 on Windows
- Replace nssckbi.dll with the attached one
- Create a fresh profile, that does not yet have any certs imported manually
  (firefox -profilemanager)
- Open the Certificate Manager
- Verify they are happy with the display of their roots
- Verify they are happy with the trust flags that can be seen with the "edit 
  trust" button
- Test that the cert works as expected, e.g. they can connect to an SSL site 
  that uses a server cert issued by their root
- Comment publicly in this bug that they are completely happy

Gerv
We tested the DLL for Certplus/Keynectis as indicated.

Root display: OK
Trust flags: OK
Root working with a web site: OK

The code freeze happened on April 30th, if I correctly read the wiki, and our CA was added on April 27th (comment #18), will the upcoming 2.0.0.4 version of Firefox contain our CA?

Thanks, Gerv.
GlobalSign have confirmed in bug 378163, comment #5.

Just waiting for DigiCert and QuoVadis.

Gerv
We have tested the new nssckbi.dll as directed.  I confirm the accuracy (root display, trust flags) and that the roots work as expected.
Many thanks to all of you!  Kind regards, Stephen
Hi Gerv, and Firefox team, 

It feels great to see these roots in Firefox!  I personally checked the roots, as did one of my engineers.  Everything checks out fine.  The sha1 and md5 fingerprints match exactly with our roots.  The names appear as we expected them, and our test sites worked perfectly when we browsed to them.

Just one question:  Kai Engbert commented on 2007-04-23:

> One of the new roots describes itself as a "High Assurance EV" root cert.

> As of today, we don't evaluate additional HA/EV flags, so as of today 
> this root cert will be used as a traditional SSL cert only.

Will we need to do anything special when Firefox is updated with EV capabilities, or will you roll that in automatically?

Thanks again,
Ken Bretschneider
DigiCert
The process of getting roots approved for EV is not yet decided, and will be separate from the root update process (although they may run concurrently for a particular cert).

Kai: please go ahead and check in :-)

Gerv
I checked all certs into both NSS trunk and NSS 3.11 branch for 3.11.8
I checked in the patch to increase the version number on 3.11 branch.
I combined the check in with the patch from bug 289979.


Checking in certdata.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.c,v  <--  certdata.c
new revision: 1.42; previous revision: 1.41
done
Checking in certdata.txt;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.txt,v  <--  certdata.txt
new revision: 1.42; previous revision: 1.41
done


Checking in certdata.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.c,v  <--  certdata.c
new revision: 1.36.24.5; previous revision: 1.36.24.4
done
Checking in certdata.txt;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.txt,v  <--  certdata.txt
new revision: 1.37.24.5; previous revision: 1.37.24.4
done
Checking in nssckbi.h;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.h,v  <--  nssckbi.h
new revision: 1.14.2.4; previous revision: 1.14.2.3
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: 3.11.7 → 3.11.8
You need to log in before you can comment on or make changes to this bug.