Update set of HPKP pins for Google domains

RESOLVED FIXED in Firefox 46

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: ryan.sleevi, Assigned: keeler)

Tracking

unspecified
mozilla46
Points:
---

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
Please update the set of HPKP pins for Google properties to the set at https://pki.google.com/roots.pem

You can confirm that this set is the most recent by ensuring that the string "58:11:9f:0e:12:82:87:ea:50:fd:d9:87:45:6f:4f:78:dc:fa:d6:d4" does not appear.
Reporter

Comment 1

4 years ago
Richard: Can you figure out who can do this?
Flags: needinfo?(rlb)
Keeler: Can you advise?
Flags: needinfo?(rlb) → needinfo?(dkeeler)
Sorry - I had meant to address this sooner.
Assignee: nobody → dkeeler
Flags: needinfo?(dkeeler)
Ryan, I'm assuming the bytes in comment 0 represent a sha1 hash. Since we recently moved to sha-256 for the pinning information, that hash wouldn't be there either way. Is there a particular root we should make sure isn't present any more? Or, if you have time, perhaps it would be faster if you had a quick look to make sure the root list is as expected. Thanks!
Flags: needinfo?(ryan.sleevi)
Reporter

Comment 6

3 years ago
Thanks. The removal was the Symantec "Class 3 Public Primary CA" (which was our most recent change), and I can confirm your change picks that up. It's got all the right additions and removals, thanks.
Flags: needinfo?(ryan.sleevi)
Comment on attachment 8702368 [details]
MozReview Request: bug 1232766 - update the preloaded pinset for Google domains r?rbarnes

https://reviewboard.mozilla.org/r/29115/#review26167

Overall, this looks fine.  I would just like to understand what's up with the expiration date.

::: security/manager/ssl/StaticHPKPins.h
(Diff revision 1)
> -  "BRz5+pXkDpuD7a7aaWH2Fox4ecRmAXJHnN1RqwPOpis=";

I'm a little mixed whether we should remove the ones that are no longer used (vs. marking them unused).  If they're not expired, presumably they could come back.  But I can go along with reducing the dead code.

::: security/manager/ssl/StaticHPKPins.h:1183
(Diff revision 1)
> -static const PRTime kPreloadPKPinsExpirationTime = INT64_C(1459598124230000);
> +static const PRTime kPreloadPKPinsExpirationTime = INT64_C(1459801686821000);

Why are you changing this date?  (By 2 days?)  It would be helpful to have a comment here explaining what this date is and what it should be set to.

::: security/manager/tools/PreloadedHPKPins.json:177
(Diff revision 1)
> +        "thawte Primary Root CA",

It looks like your alphabetization might be case-sensitive.  I would prefer that it not be, i.e., that lower-case and upper-case "T" go together.
Attachment #8702368 - Flags: review?(rlb)
https://reviewboard.mozilla.org/r/29115/#review26167

> I'm a little mixed whether we should remove the ones that are no longer used (vs. marking them unused).  If they're not expired, presumably they could come back.  But I can go along with reducing the dead code.

None of the changes to this file were by hand - it's automated by genHPKPStaticPins.js. If a pinset uses these in the future, they'll come back without us having to do any extra work.

> Why are you changing this date?  (By 2 days?)  It would be helpful to have a comment here explaining what this date is and what it should be set to.

Again, this is just automation. The script sets the expiration date based on when the information was known to be correct (i.e. when it was last run).

> It looks like your alphabetization might be case-sensitive.  I would prefer that it not be, i.e., that lower-case and upper-case "T" go together.

Ok - sounds good.
Comment on attachment 8702368 [details]
MozReview Request: bug 1232766 - update the preloaded pinset for Google domains r?rbarnes

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29115/diff/1-2/
Attachment #8702368 - Attachment description: MozReview Request: bug 1232766 - update the preloaded pinset for Google domains → MozReview Request: bug 1232766 - update the preloaded pinset for Google domains r?rbarnes
Attachment #8702368 - Flags: review?(rlb)
Comment on attachment 8702368 [details]
MozReview Request: bug 1232766 - update the preloaded pinset for Google domains r?rbarnes

https://reviewboard.mozilla.org/r/29115/#review27105

Thanks, this LGTM.
Attachment #8702368 - Flags: review?(rlb) → review+

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ad017d0208ef
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.