Closed
Bug 1035923
Opened 10 years ago
Closed 10 years ago
remove 1024 bit roots from google_root_pems
Categories
(Core :: Security: PSM, defect)
Tracking
()
RESOLVED
FIXED
mozilla33
Tracking | Status | |
---|---|---|
firefox30 | --- | unaffected |
firefox31 | --- | unaffected |
firefox32 | + | fixed |
firefox33 | --- | fixed |
People
(Reporter: mmc, Assigned: mmc)
References
Details
Attachments
(1 file, 2 obsolete files)
3.88 KB,
patch
|
mmc
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
bug 1029561 removed some certs that are in google_root_pems, thus breaking the pinset generator (on purpose, since we should not be relying on builtins that don't exist).
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8452418 [details] [diff] [review] Remove deprecated certs from google_root_pems ( Review of attachment 8452418 [details] [diff] [review]: ----------------------------------------------------------------- Ugh -- I guess the way the code is written now we import PEMs from Chrome's transport_security_static.certs, which includes builtins that we no longer support. Is this the desired behavior? Either way, we should notify AGL. ::: security/manager/boot/src/StaticHPKPins.h @@ +1003,5 @@ > { "wf-staging-hr.appspot.com", true, false, false, -1, &kPinset_google_root_pems }, > { "wf-training-hrd.appspot.com", true, false, false, -1, &kPinset_google_root_pems }, > { "wf-training-master.appspot.com", true, false, false, -1, &kPinset_google_root_pems }, > { "wf-trial-hrd.appspot.com", true, false, false, -1, &kPinset_google_root_pems }, > + { "www.dropbox.com", true, true, false, -1, &kPinset_dropbox }, Looks like they rolled back this change to www only.
Attachment #8452418 -
Flags: review?(dkeeler)
Assignee | ||
Comment 3•10 years ago
|
||
>
> Ugh -- I guess the way the code is written now we import PEMs from Chrome's
> transport_security_static.certs, which includes builtins that we no longer
> support. Is this the desired behavior?
What I mean to say is that twitterCDN includes some of these 1024-bit roots, which we import from the raw PEM file. If that's not the desired behavior, that should be a followup bug.
Comment on attachment 8452418 [details] [diff] [review] Remove deprecated certs from google_root_pems ( Review of attachment 8452418 [details] [diff] [review]: ----------------------------------------------------------------- I think it's correct that we import certs from google's list - if they were intermediates, they wouldn't be in our root program, but we still want to be able to pin to their fingerprints, right? Anyway, this looks correct to me. ::: security/manager/boot/src/StaticHPKPins.h @@ +1014,5 @@ > { "youtube.com", true, false, false, -1, &kPinset_google_root_pems }, > { "ytimg.com", true, false, false, -1, &kPinset_google_root_pems }, > }; > > +static const int kPublicKeyPinningPreloadListLength = 325; We discussed what happened here. Let's file a bug on using the static length of these data structures.
Attachment #8452418 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee1da9d45ce2
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8452418 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8452702 -
Flags: review+
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8452702 [details] [diff] [review] Aurora-specific patch for removing google's 1024-bit root certs ( Approval Request Comment [Feature/regressing bug #]: 1029561 removed these root certs from our builtins, which breaks pin generation on aurora now that buildbot generates these. [User impact if declined]: buildbot generation will break on aurora, we will ship incorrect expiration dates and pinned certs for FF32 [Describe test coverage new/current, TBPL]: pinning is unittested in security/manager/ssl/tests/unit/test_pinning.js, buildbot errors appear as changes to security/manager/boot/src/StaticHPKPins.errors [Risks and why]: low risk [String/UUID change made/needed]: none
Attachment #8452702 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 8•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8452707 -
Attachment is obsolete: true
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ee1da9d45ce2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Updated•10 years ago
|
status-firefox30:
--- → unaffected
status-firefox31:
--- → unaffected
status-firefox32:
--- → affected
status-firefox33:
--- → fixed
tracking-firefox32:
--- → +
Comment 10•10 years ago
|
||
Comment on attachment 8452702 [details] [diff] [review] Aurora-specific patch for removing google's 1024-bit root certs ( Aurora+
Attachment #8452702 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/d9c3d923cb3e
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•