Closed
Bug 1092606
Opened 10 years ago
Closed 10 years ago
Aurora is busted after today's HPKP update
Categories
(Core :: Security: PSM, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: RyanVM, Assigned: mmc)
Details
Attachments
(4 files, 1 obsolete file)
8.77 KB,
patch
|
Details | Diff | Splinter Review | |
7.34 KB,
patch
|
mmc
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
2.12 KB,
patch
|
keeler
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
10.51 KB,
patch
|
Details | Diff | Splinter Review |
Aurora is closed. https://treeherder.mozilla.org/ui/logviewer.html#?job_id=363459&repo=mozilla-aurora In file included from /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/PublicKeyPinningService.cpp:23:0: /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/StaticHPKPins.h:762:49: error: redefinition of 'const char* kPinset_facebook_sha256_Data []' /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/StaticHPKPins.h:378:20: error: 'const char* kPinset_facebook_sha256_Data [3]' previously defined here /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/StaticHPKPins.h:767:33: error: redefinition of 'const StaticFingerprints kPinset_facebook_sha256' /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/StaticHPKPins.h:383:33: error: 'const StaticFingerprints kPinset_facebook_sha256' previously defined here /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/StaticHPKPins.h:772:27: error: redefinition of 'const StaticPinset kPinset_facebook' /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/StaticHPKPins.h:388:27: error: 'const StaticPinset kPinset_facebook' previously defined here make[6]: *** [PublicKeyPinningService.i_o] Error 1 make[6]: Leaving directory `/builds/slave/m-aurora-lx-000000000000000000/build/obj-firefox/security/manager/boot/src' make[5]: *** [security/manager/boot/src/target] Error 2
Comment 1•10 years ago
|
||
Backed out in https://hg.mozilla.org/releases/mozilla-aurora/rev/f4385d27bff1 These things increasingly seem too prone to bustage for Saturday morning automation, which is a bad thing since they have never ever in even the slightest seemed to not be too prone to bustage for it.
Assignee | ||
Comment 2•10 years ago
|
||
This is because Chromium started pinning facebook, for which we were already injecting a pinset. Fix coming.
Updated•10 years ago
|
status-firefox35:
--- → affected
status-firefox36:
--- → affected
tracking-firefox35:
--- → +
tracking-firefox36:
--- → +
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8516818 [details] [diff] [review] Don't import Chromium pinsets for domains that are already in our list ( Review of attachment 8516818 [details] [diff] [review]: ----------------------------------------------------------------- This fixes all the JS warnings as well.
Attachment #8516818 -
Flags: review?(jjones)
Attachment #8516818 -
Flags: review?(dkeeler)
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8516819 [details] [diff] [review] Not for checkin This is sample output.
Comment on attachment 8516818 [details] [diff] [review] Don't import Chromium pinsets for domains that are already in our list ( Review of attachment 8516818 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. If you feel like it, I think there are about 7 instances of 'var' we could change to 'let' while we're making these changes. Thanks for taking care of this. ::: security/manager/tools/genHPKPStaticPins.js @@ +195,5 @@ > let pemCert = ""; > let hash = ""; > let chromeNameToHash = {}; > let chromeNameToMozName = {} > + let mozName; I think we can declare mozName closer to where it's used.
Attachment #8516818 -
Flags: review?(dkeeler) → review+
Comment 8•10 years ago
|
||
Comment on attachment 8516818 [details] [diff] [review] Don't import Chromium pinsets for domains that are already in our list ( Review of attachment 8516818 [details] [diff] [review]: ----------------------------------------------------------------- Keeler got here ahead of me. I don't see any issues with this patch, either.
Attachment #8516818 -
Flags: review?(jjones) → review+
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d0c12148cf6
Assignee | ||
Comment 10•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8516818 -
Attachment is obsolete: true
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8516844 [details] [diff] [review] Don't import Chromium pinsets for domains that are already in our list ( Review of attachment 8516844 [details] [diff] [review]: ----------------------------------------------------------------- Fixed comments and checked in.
Attachment #8516844 -
Flags: review+
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8516844 [details] [diff] [review] Don't import Chromium pinsets for domains that are already in our list ( Approval Request Comment [Feature/regressing bug #]: Chromium started pinning facebook (comment 2) [User impact if declined]: No pin updates on Aurora, no pinning enforcement on release if buildbot updates don't happen in Aurora (pin lifetimes are 14 weeks since last update, which is typically 8 weeks after release) [Describe test coverage new/current, TBPL]: ran manually on mozilla-aurora [Risks and why]: pretty low [String/UUID change made/needed]: none
Attachment #8516844 -
Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/1d0c12148cf6
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•10 years ago
|
Attachment #8516844 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reporter | ||
Comment 14•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/f46d7b20b032
Comment 15•10 years ago
|
||
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=399715&repo=mozilla-aurora /builds/slave/m-aurora-lx-000000000000000000/build/security/manager/boot/src/StaticHPKPins.h:762:49: error: redefinition of 'const char* kPinset_facebook_sha256_Data []' Backed out this week's update in https://hg.mozilla.org/releases/mozilla-aurora/rev/2f9a72986443
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(mmc)
Assignee | ||
Comment 16•10 years ago
|
||
Sigh -- this fix suppressed the duplicate pinsets but not their constituent parts. Sorry about that.
Flags: needinfo?(mmc)
Assignee | ||
Comment 17•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8516844 -
Attachment is obsolete: true
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8523991 [details] [diff] [review] Filter out duplicate pinsets as well as domains ( Review of attachment 8523991 [details] [diff] [review]: ----------------------------------------------------------------- Tested by running and building on Aurora.
Attachment #8523991 -
Flags: review?(dkeeler)
Assignee | ||
Comment 19•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8516844 -
Attachment is obsolete: false
Comment on attachment 8523991 [details] [diff] [review] Filter out duplicate pinsets as well as domains ( Review of attachment 8523991 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. However, looking at our definition of Facebook's pinset on aurora and chrome's definition of Facebook's pinset, we have a discrepancy: "Verisign Class 3 Public Primary Certification Authority - G3", "DigiCert High Assurance EV Root CA", "DigiCert ECC Secure Server CA" vs "SymantecClass3EVG3", "DigiCertECCSecureServerCA", "DigiCertEVRoot", "FacebookBackup" We should probably uplift the patch in bug 1004781 that fixed this in mozilla-central.
Attachment #8523991 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 21•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fcbd0ed5a28
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to David Keeler (:keeler) [use needinfo?] from comment #20) > Comment on attachment 8523991 [details] [diff] [review] > Filter out duplicate pinsets as well as domains ( > > Review of attachment 8523991 [details] [diff] [review]: > ----------------------------------------------------------------- > > LGTM. > However, looking at our definition of Facebook's pinset on aurora and > chrome's definition of Facebook's pinset, we have a discrepancy: > > "Verisign Class 3 Public Primary Certification Authority - G3", > "DigiCert High Assurance EV Root CA", > "DigiCert ECC Secure Server CA" > > vs > > "SymantecClass3EVG3", > "DigiCertECCSecureServerCA", > "DigiCertEVRoot", > "FacebookBackup" > > We should probably uplift the patch in bug 1004781 that fixed this in > mozilla-central. Well, I'd like to make sure that this patch resolves the duplication issue in case we ever have conflicting pinsets again. I think it's OK that facebook's test pinset will be different from Nightly's the last 2 weeks of Aurora.
Assignee | ||
Comment 23•10 years ago
|
||
Comment on attachment 8523991 [details] [diff] [review] Filter out duplicate pinsets as well as domains ( Approval Request Comment [Feature/regressing bug #]: See comment 2 [User impact if declined]: No updates to Aurora in 3 weeks, decreased pinset lifetime on release. [Describe test coverage new/current, TBPL]: This time I ran the pin generation manually and did *not* forget to build the result. [Risks and why]: Should be pretty low, this just fixes existing bustage. [String/UUID change made/needed]: None.
Attachment #8523991 -
Flags: approval-mozilla-aurora?
Comment 24•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6fcbd0ed5a28
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Attachment #8523991 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 25•10 years ago
|
||
Pushed: https://hg.mozilla.org/releases/mozilla-aurora/rev/776650aa7757 Coop, would you mind running the build script manually on aurora? If it didn't work, I'd like to know before Saturday.
Flags: needinfo?(coop)
Comment 27•10 years ago
|
||
(In reply to Chris Cooper [:coop] from comment #26) > It's running now. Will report back with status. Success: https://hg.mozilla.org/releases/mozilla-aurora/rev/cffcb649cd0d
Assignee | ||
Comment 28•10 years ago
|
||
Awesome, thanks Coop!
You need to log in
before you can comment on or make changes to this bug.
Description
•