1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[:Cykesiopka]

NSS 3.21 removes some roots from the root store: Bug 1214729

Some of the roots are referred to by PreloadedHPKPins.json, which means the periodic HPKP updates (or manual updates) will fail once NSS 3.21 lands in m-c with an error similar to the following:
> JavaScript error: , line 0: uncaught exception: Can't find Verisign Class 4 Public Primary Certification Authority - G3 in certNameToSKD

Comment 1

[:Cykesiopka]

Attachment: MozReview Request: Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.

Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.

Comment 2

[:Cykesiopka]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2dc18de3fa85

Comment 3

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8687222 [details]
MozReview Request: Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.

https://reviewboard.mozilla.org/r/25149/#review22659

Good catch. Thanks for taking care of this.

Comment 4

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

You can land this with "DONTBUILD NPOTB" in the commit message since this isn't actually part of the build process.

Comment 5

[:Cykesiopka]

Thanks for the review.

(In reply to David Keeler [:keeler] (use needinfo?) from comment #4)
> You can land this with "DONTBUILD NPOTB" in the commit message since this
> isn't actually part of the build process.

Heh, well, no L3 access, so whoever checks this in for me can I guess.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f191524a1f14

Comment 7

[Kai Engert (:KaiE:)]

Comment on attachment 8687222 [details]
MozReview Request: Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.

Approval Request Comment
[Feature/regressing bug #]: Required to land NSS 3.21 final with new root CAs. (blocks bug 1211568)

Comment 8

[Kai Engert (:KaiE:)]

Sorry, I might have requested approval-aurora on the wrong bug.

But let me ask to be certain.

If NSS 3.21 lands on Aurora 44, should this land on Aurora 44, too?

If no, please remove my approval request. Thanks.

Comment 9

[:Cykesiopka]

(In reply to Kai Engert (:kaie) from comment #8)
> Sorry, I might have requested approval-aurora on the wrong bug.
> 
> But let me ask to be certain.
> 
> If NSS 3.21 lands on Aurora 44, should this land on Aurora 44, too?
> 
> If no, please remove my approval request. Thanks.

This is the correct bug, don't worry.
This needs to land on Aurora 44 if NSS 3.21 (and the associate root removals) lands there as well.

Comment 10

[Kathleen Wilson]

The list of root certs that were removed or had the websites trust bit turned off in NSS 3.21 is here:
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.21_release_notes#Notable_Changes_in_NSS_3.21

Here's what I think needs to be commented out of PreloadedHPKPins.json
100         "Verisign Class 4 Public Primary Certification Authority - G3",
190         "Verisign Class 4 Public Primary Certification Authority - G3",
174         "TC TrustCenter Universal CA III",
180         "UTN DATACorp SGC Root CA",
148         "Equifax Secure CA",

Note, these previously had the Websites trust bit removed, so I think they should also be commented out:
173         "TC TrustCenter Class 3 CA II",
149         "Equifax Secure eBusiness CA 1",
151         "Equifax Secure Global eBusiness CA",

Comment 11

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Thanks for pointing this out, Kathleen. I filed bug 1225288 to deal with the trust bit issues.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 13

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

Comment on attachment 8687222 [details]
MozReview Request: Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.

This is needed as part of upgrading FF44 to use NSS 3.21. Let's uplift to Aurora44.

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c9902056c184

Comment 15

[Martin Thomson [:mt:]]

Comment on attachment 8687222 [details]
MozReview Request: Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.

Approval Request Comment
[Feature/regressing bug #]: 1224481
[User impact if declined]: Security changes in bug 1211568
[Describe test coverage new/current, TreeHerder]: Plenty of time in Nightly and Aurora.
[Risks and why]: None
[String/UUID change made/needed]: None

Comment 16

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Tracking for 43 since we intend to uplift to beta.

Comment 17

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8687222 [details]
MozReview Request: Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.


Approved for uplift to beta and m-r. We'd like to get this into the 43 RC build today.

Comment 18

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8687222 [details]
MozReview Request: Bug 1224481 - Comment out CA certs removed in NSS 3.21 in PreloadedHPKPins.json to keep periodic Static HPKP updates working.

On further discussion let's not uplift this; we haven't merged it yet into 43. If we need to get it into 43 before the planned disclosure, I think it would be better to have more time and plan it as a 43 dot release.

Comment 19

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

The update in bug 1158489 didn't need these changes; wontfix for 43.