Closed Bug 527759 Opened 12 years ago Closed 12 years ago

Add multiple roots to NSS (single patch)

Categories

(NSS :: Libraries, defect)

3.12.5
defect
Not set
normal

Tracking

(status1.9.2 beta5-fixed, blocking1.9.1 .8+, status1.9.1 .8-fixed)

RESOLVED FIXED
3.12.6
Tracking Status
status1.9.2 --- beta5-fixed
blocking1.9.1 --- .8+
status1.9.1 --- .8-fixed

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

Attachments

(2 files, 4 obsolete files)

Add multiple roots to NSS (single patch)

See list of bugs this one blocks.
Attached patch Patch v1 (obsolete) — Splinter Review
I used the following 4 commands to add the 4 new root certs, using friendly names and trust flags as indicated in the related 4 bugs.

addbuiltin -n "certSIGN ROOT CA" -t C,C,C <  ~/moz/nss/head/cert-526532.der >> certdata.txt

addbuiltin -n "CNNIC ROOT" -t C,,  <  ~/moz/nss/head/cert-525008 >> certdata.txt

addbuiltin -n "ApplicationCA - Japanese Government" -t C,,C  <  ~/moz/nss/head/cert-523434.der >> certdata.txt

addbuiltin -n "GeoTrust Primary Certification Authority - G3" -t C,C,C  <  ~/moz/nss/head/cert-517234.der >> certdata.txt
Attachment #411492 - Flags: review?(nelson)
Comment on attachment 411492 [details] [diff] [review]
Patch v1

Maybe you'll want to postpone the review until the test build has been confirmed. I'll create a try server build, ask CAs for confirmation, and afterwards ask you again for review.
Attachment #411492 - Flags: review?(nelson)
adding 2 more bugs, 4 more certs
Blocks: 521869, 515462
for the additional thawte and verisign roots I used the following commands:

addbuiltin -n "thawte Primary Root CA - G2" -t C,,C  <  ~/moz/nss/head/cert-521869-g2 >> certdata.txt

addbuiltin -n "thawte Primary Root CA - G3" -t C,,C  <  ~/moz/nss/head/cert-521869-g3 >> certdata.txt

addbuiltin -n "Verisign Class 1 Public Primary Certification Authority" -t ,C,  <  ~/moz/nss/head/cert-515462-pca1.der >> certdata.txt

addbuiltin -n "Verisign Class 3 Public Primary Certification Authority" -t C,C,C  <  ~/moz/nss/head/cert-515462-pca3.der >> certdata.txt
I'll produce a test build based on stable Firefox 3.5.x, mozilla-1.9.1 branch

That branch still uses NSS 3.12.4, I'll copy certdata.c and certdata.txt from my patched trunk version of NSS to the NSS snapshot currently used in mozilla-1.9.1

(can't simply apply the patch because additional roots have been added recently, after 3.12.4)
Attached patch Patch v2, 8 new roots (obsolete) — Splinter Review
Attachment #411492 - Attachment is obsolete: true
Attachment #411639 - Flags: review?(nelson)
test build:
https://build.mozilla.org/tryserver-builds/kaie@kuix.de-bug527759/

I've updated the individual bugs, asking for testing and feedback.
Attachment #411639 - Flags: review?(nelson) → review+
Comment on attachment 411639 [details] [diff] [review]
Patch v2, 8 new roots

Behold the rubber stamp.
Blocks: 528277
Target Milestone: --- → 3.12.6
Blocks: 517242
Blocks: 515470
Blocks: 515472
Attached patch Patch v3, 11 new roots (obsolete) — Splinter Review
We missed 3 roots!
I added 3 more dependent bugs, added them to this patch.

Patch v3 is equivalent to v2, it just adds 3 additional roots.

Here are the commands I had used to add them:

addbuiltin -n "GeoTrust Primary Certificate Authority - G2" -t C,C,C < ~/moz/nss/head/cert-517242.der >> certdata.txt

addbuiltin -n "VeriSign Universal Root Certification Authority" -t C,C,C < ~/moz/nss/head/cert-515470 >> certdata.txt

addbuiltin -n "VeriSign Class 3 Public Primary Certificate Authority - G4" -t C,C,C < ~/moz/nss/head/cert-515472 >> certdata.txt
Attachment #412641 - Flags: review?(nelson)
Comment on attachment 412641 [details] [diff] [review]
Patch v3, 11 new roots

Per bug 515472 comment 7, two of these should have different nicknames.

"GeoTrust Primary Certificate Authority - G2" 
should be
"GeoTrust Primary Certification Authority - G2" 

"VeriSign Class 3 Public Primary Certificate Authority - G4"
should be
"VeriSign Class 3 Public Primary Certification Authority - G4"
Attachment #412641 - Flags: review?(nelson) → review-
Attached patch Patch v4 (obsolete) — Splinter Review
I reverted my tree to the clean original cvs contents.

I repeated my 11 commands, now using the 2 corrected nicknames.

addbuiltin -n "certSIGN ROOT CA" -t C,C,C <  ~/moz/nss/head/cert-526532.der >> certdata.txt
addbuiltin -n "CNNIC ROOT" -t C,,  <  ~/moz/nss/head/cert-525008 >> certdata.txt
addbuiltin -n "ApplicationCA - Japanese Government" -t C,,C  < ~/moz/nss/head/cert-523434.der >> certdata.txt
addbuiltin -n "GeoTrust Primary Certification Authority - G3" -t C,C,C  < ~/moz/nss/head/cert-517234.der >> certdata.txt
addbuiltin -n "thawte Primary Root CA - G2" -t C,,C  < ~/moz/nss/head/cert-521869-g2 >> certdata.txt
addbuiltin -n "thawte Primary Root CA - G3" -t C,,C  < ~/moz/nss/head/cert-521869-g3 >> certdata.txt
addbuiltin -n "Verisign Class 1 Public Primary Certification Authority" -t ,C, <  ~/moz/nss/head/cert-515462-pca1.der >> certdata.txt
addbuiltin -n "Verisign Class 3 Public Primary Certification Authority" -t C,C,C  <  ~/moz/nss/head/cert-515462-pca3.der >> certdata.txt
addbuiltin -n "GeoTrust Primary Certification Authority - G2" -t C,C,C < ~/moz/nss/head/cert-517242.der >> certdata.txt
addbuiltin -n "VeriSign Universal Root Certification Authority" -t C,C,C < ~/moz/nss/head/cert-515470 >> certdata.txt
addbuiltin -n "VeriSign Class 3 Public Primary Certification Authority - G4" -t C,C,C < ~/moz/nss/head/cert-515472 >> certdata.txt

and ran "make generate"

This gave me this updated patch.

I compared previous patch v3 and this patch v4.
The only difference is in nickname strings and string length constants.

(I think another test build is not necessary.)
Attachment #411639 - Attachment is obsolete: true
Attachment #412641 - Attachment is obsolete: true
Attachment #413313 - Flags: review?(nelson)
Please delay reviewing this patch until we have feedback from verisign.

They complained that trust bits are missing, but I can't confirm their impression.
Comment on attachment 413313 [details] [diff] [review]
Patch v4

Since you've asked me to delay the review, I'm canceling the review request.  Please request review again when you're ready.
Attachment #413313 - Flags: review?(nelson)
It appears bug 515462 needs some more analysis. I therefore propose to omit it from this round of cert additions, in order to get the other certs shipped soon.
No longer blocks: 515462
Attached patch Patch v5, rootsSplinter Review
Attachment #413313 - Attachment is obsolete: true
Attachment #415099 - Flags: review?(nelson)
Patch v5 adds 9 roots.
(When compared to the previously attached patch, I removed the 2 roots from bug 515462)

NSS 3.12.5 got released with ckbi v 1.76.
When adding the 9 roots, I propose to upgrade the ckbi version number to v 1.77.
I propose to release ckbi v 1.77 immediately after adding the roots and produce a NSS 3.12.4 + ckbi 1.77 tag.

The first part of the patch updates the version number.


As NSS 3.12 turns out to be a surprisingly long lived branch, I propose to increase the range of version numbers we reserve for 3.12.

The second part of the patch adds this proposal and states the proposed new version number ranges.
Attachment #415100 - Flags: review?(nelson)
Please review both patches, thanks in advance.
Comment on attachment 415100 [details] [diff] [review]
Patch v6, version numbers

I see no problem with this.
Attachment #415100 - Flags: review?(nelson) → review+
Comment on attachment 415099 [details] [diff] [review]
Patch v5, roots

r=nelson
I have verified that the roots in this patch have the nicknames requested in their respective NSS RFEs. 
I have not verified the trust flags.  I am relying on the CAs, who have each approved their respective cert additions, for this purpose.
Attachment #415099 - Flags: review?(nelson) → review+
Thanks for the reviews.

Marking resolved fixed.

I forgot to copy the output from cvs commit, but here are the version's I've committed:

/certdata.c/1.58/Thu Dec  3 21:22:36 2009//TNSSCKBI_1_77_RTM
/certdata.txt/1.57/Thu Dec  3 21:22:36 2009//TNSSCKBI_1_77_RTM
/nssckbi.h/1.22/Thu Dec  3 21:22:36 2009//TNSSCKBI_1_77_RTM
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 415099 [details] [diff] [review]
Patch v5, roots

I assume you'll want to add these to Firefox 3.0 and 3.5 as well
Attachment #415099 - Flags: approval1.9.2?
Attachment #415099 - Flags: approval1.9.1.7?
Attachment #415099 - Flags: approval1.9.0.17?
Attachment #415100 - Flags: approval1.9.2?
Attachment #415100 - Flags: approval1.9.1.7?
Attachment #415100 - Flags: approval1.9.0.17?
Comment on attachment 415099 [details] [diff] [review]
Patch v5, roots

Firefox 3.0.x is still on NSS 3.12.3.1 and basically EOL, let's not.
Attachment #415099 - Flags: approval1.9.0.17?
Attachment #415100 - Flags: approval1.9.0.17?
Johnathan's comment in Bug 528277 comment 3 should be seen as approval-1.9.2+ for both r+'ed patches in this bug (attachment 415099 [details] [diff] [review] and attachment 415100 [details] [diff] [review])
Comment on attachment 415099 [details] [diff] [review]
Patch v5, roots

Approved for 1.9.1.8, a=dveditz for release-drivers
Attachment #415099 - Flags: approval1.9.1.8? → approval1.9.1.8+
Comment on attachment 415100 [details] [diff] [review]
Patch v6, version numbers

Approved for 1.9.1.8, a=dveditz for release-drivers
Attachment #415100 - Flags: approval1.9.1.8? → approval1.9.1.8+
This has been fixed for 1.9.2 in bug 528277.
Whiteboard: [needs 1.9.1 landing]
blocking1.9.1: --- → .8+
Whiteboard: [needs 1.9.1 landing]
Attachment #415099 - Flags: approval1.9.2?
Attachment #415100 - Flags: approval1.9.2?
You need to log in before you can comment on or make changes to this bug.