Closed
Bug 1101969
Opened 10 years ago
Closed 9 years ago
media.mozilla.com uses the wrong pinset
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: keeler, Assigned: mmc)
References
Details
Attachments
(2 files, 1 obsolete file)
1.43 KB,
patch
|
keeler
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
3.76 KB,
patch
|
mmc
:
review+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Pinning fails for https://media.mozilla.com/. Looks like it's issued by GlobalSign, which isn't in the pinset we use for media.mozilla.com.
Assignee | ||
Comment 1•9 years ago
|
||
Do we know if resources come from media.mozilla.com or a subdomain, and if those are affected?
Assignee | ||
Comment 2•9 years ago
|
||
This is the second time we've gotten the pinset wrong for media.mozilla.com. I don't think that the current setup (no per-host telemetry) is sufficient for us to proceed, especially in cases where the volume of visits is relatively small. We should instead push on webops to use HPKP so it's under their own control, and we may not be able to solve this problem for CDNs.
Attachment #8535071 -
Flags: review?(dkeeler)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mmc
Reporter | ||
Comment 3•9 years ago
|
||
Comment on attachment 8535071 [details] [diff] [review] media Review of attachment 8535071 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, let's go back and do this properly. The entry for media.mozilla.com wasn't properly vetted when it was added the first time. I'm not sure it's the case that we can never have a built-in pin for this, but we do need to be more careful about CDNs. Also, it would be nice to have more HPKP adoption.
Attachment #8535071 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 4•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c65234c3ead
https://hg.mozilla.org/mozilla-central/rev/4c65234c3ead
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8535071 [details] [diff] [review] media Approval Request Comment [Feature/regressing bug #]: 1016442 [User impact if declined]: Users will not be able to load https://media.mozilla.com [Describe test coverage new/current, TBPL]: m-c [Risks and why]: Very low. This just removes the pinset for media.mozilla.com. [String/UUID change made/needed]: None.
Attachment #8535071 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
Updated•9 years ago
|
Attachment #8535071 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 8•9 years ago
|
||
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8537532 [details] [diff] [review] Disable pinning on media.mozilla.com ( Review of attachment 8537532 [details] [diff] [review]: ----------------------------------------------------------------- Beta-specific patch, since buildbot does not run on beta. Tested by generating the output file, rebuilding, and visiting https://media.mozilla.com (which now gets a 200). I removed the timestamp changes since we don't actually want to extend the expiration date.
Attachment #8537532 -
Flags: review?(dkeeler)
Reporter | ||
Comment 10•9 years ago
|
||
Comment on attachment 8537532 [details] [diff] [review] Disable pinning on media.mozilla.com ( Review of attachment 8537532 [details] [diff] [review]: ----------------------------------------------------------------- LGTM with comment addressed. ::: security/manager/boot/src/StaticHPKPins.h @@ -1081,5 @@ > { "m.facebook.com", true, true, false, -1, &kPinset_facebook }, > { "mail.google.com", true, false, false, -1, &kPinset_google_root_pems }, > { "market.android.com", true, false, false, -1, &kPinset_google_root_pems }, > { "mbasic.facebook.com", true, true, false, -1, &kPinset_facebook }, > - { "media.mozilla.com", true, false, true, -1, &kPinset_mozilla }, We should probably also subtract one from the comment that says how long the list is.
Attachment #8537532 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8537532 -
Attachment is obsolete: true
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8537545 [details] [diff] [review] Disable pinning on media.mozilla.com (beta-specific-patch) Approval Request Comment [Feature/regressing bug #]:See comment 6. I'd like to fix this for 35. [User impact if declined]: Users won't be able to visit https://media.mozilla.com in 35. [Describe test coverage new/current, TBPL]: manual and m-c [Risks and why]: very low [String/UUID change made/needed]: none. Good point, David. Fixed comment.
Attachment #8537545 -
Flags: review+
Attachment #8537545 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•9 years ago
|
Attachment #8537545 -
Attachment description: Disable pinning on media.mozilla.com ( → Disable pinning on media.mozilla.com (beta-specific-patch)
Updated•9 years ago
|
Attachment #8537545 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 13•9 years ago
|
||
Thanks, Lukas! https://hg.mozilla.org/releases/mozilla-beta/rev/73bbf3b1c3ef
status-firefox35:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•