Closed
Bug 378489
Opened 17 years ago
Closed 17 years ago
Add multiple new roots to NSS
Categories
(NSS :: Libraries, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
3.11.8
People
(Reporter: KaiE, Assigned: KaiE)
References
Details
Attachments
(5 files)
43.30 KB,
patch
|
nelson
:
review+
alvolkov.bgs
:
review+
rrelyea
:
superreview+
|
Details | Diff | Splinter Review |
854 bytes,
patch
|
nelson
:
review+
alvolkov.bgs
:
review+
rrelyea
:
superreview+
|
Details | Diff | Splinter Review |
6.43 KB,
patch
|
nelson
:
review+
alvolkov.bgs
:
review+
rrelyea
:
superreview+
|
Details | Diff | Splinter Review |
510 bytes,
text/plain
|
Details | |
268.00 KB,
application/x-msdos-program
|
Details |
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
Assignee | ||
Comment 1•17 years ago
|
||
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)
Assignee | ||
Comment 2•17 years ago
|
||
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)
Assignee | ||
Comment 3•17 years ago
|
||
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.
Assignee | ||
Comment 4•17 years ago
|
||
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.
Assignee | ||
Comment 5•17 years ago
|
||
Reverting dependencies. (Thanks to Nelson for making me aware of the mistake)
Comment 6•17 years ago
|
||
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.
Assignee | ||
Comment 7•17 years ago
|
||
(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.
Assignee | ||
Comment 8•17 years ago
|
||
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.
Comment 9•17 years ago
|
||
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?
Comment 10•17 years ago
|
||
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
Comment 11•17 years ago
|
||
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.
Comment 12•17 years ago
|
||
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
Assignee | ||
Comment 13•17 years ago
|
||
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.
Comment 14•17 years ago
|
||
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 15•17 years ago
|
||
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+
Updated•17 years ago
|
Attachment #262530 -
Flags: review?(alexei.volkov.bugs) → review+
Comment 16•17 years ago
|
||
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+
Assignee | ||
Comment 17•17 years ago
|
||
(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.
Assignee | ||
Comment 18•17 years ago
|
||
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)
Assignee | ||
Updated•17 years ago
|
Attachment #263045 -
Flags: superreview? → superreview?(rrelyea)
Assignee | ||
Updated•17 years ago
|
Attachment #263045 -
Attachment is patch: true
Attachment #263045 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Updated•17 years ago
|
Blocks: 379032
Summary: Add 3 new roots to NSS → Add multiple new roots to NSS
Comment 19•17 years ago
|
||
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 20•17 years ago
|
||
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+
Comment 21•17 years ago
|
||
Comment 22•17 years ago
|
||
Comment on attachment 262529 [details] [diff] [review] Patch v1 looks ok. See attachment 263270 [details]
Attachment #262529 -
Flags: review?(alexei.volkov.bugs) → review+
Updated•17 years ago
|
Attachment #263270 -
Attachment mime type: application/octet-stream → text/plain
Comment 23•17 years ago
|
||
Comment on attachment 263045 [details] [diff] [review] Additional Patch v2 looks ok.
Attachment #263045 -
Flags: review?(alexei.volkov.bugs) → review+
Comment 24•17 years ago
|
||
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
Updated•17 years ago
|
Attachment #262530 -
Flags: superreview?(rrelyea) → superreview+
Updated•17 years ago
|
Attachment #263045 -
Flags: superreview?(rrelyea) → superreview+
Updated•17 years ago
|
Attachment #262529 -
Flags: superreview?(rrelyea) → superreview+
Comment 25•17 years ago
|
||
Given Bob's comment that "0-255 is the permissible range", I think we should go ahead and commit these new CA certs now.
Comment 26•17 years ago
|
||
Comment 27•17 years ago
|
||
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
Comment 28•17 years ago
|
||
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.
Comment 29•17 years ago
|
||
GlobalSign have confirmed in bug 378163, comment #5. Just waiting for DigiCert and QuoVadis. Gerv
Comment 30•17 years ago
|
||
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
Comment 31•17 years ago
|
||
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
Comment 32•17 years ago
|
||
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
Assignee | ||
Comment 33•17 years ago
|
||
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.
Description
•