Closed Bug 1030135 Opened 10 years ago Closed 9 years ago

enable pinning on services.mozilla.com

Categories

(Core :: Security: PSM, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox32 --- unaffected
firefox33 + wontfix
firefox34 + wontfix
firefox35 + fixed
firefox36 --- fixed

People

(Reporter: freddy, Assigned: mmc)

References

Details

Attachments

(2 files, 7 obsolete files)

2.87 KB, patch
Details | Diff | Splinter Review
1.54 KB, patch
keeler
: review+
Details | Diff | Splinter Review
+++ This bug was initially created as a clone of Bug #1020485 +++

I suggest we enable certificate pinning for the loop servers.

As per bug 1020485 comment 5 and https://wiki.mozilla.org/SecurityEngineering/Public_Key_Pinning/SiteOperators this would require the loop server to give notice 14 weeks before changing your issuer CA (including CDNs and optionally subdomains).

Adam, Ben: Are you the right people to talk to about this? If not, can you please assign and CC the relevant people?
Depends on: 1038424
Frederik -- What changes do we need for this?  Does this block v2.0?
Flags: needinfo?(fbraun)
This should probably block loop, but not loop-mobile-client. No v2.0 blocker.
Flags: needinfo?(fbraun)
Should we add this to the v2.1 backlog?
Flags: needinfo?(fbraun)
Nope. As Monica explained in bug 1020485 comment 27, it doesn't make a lot of sense in b2g. But it's still desirable for Loop on Firefox desktop.

Changing blocker/depending fields accordingly.
No longer depends on: 1038424
Flags: needinfo?(fbraun)
Thanks for clarification.  That makes much more sense.
Can we expand this to all of *.services.mozilla.com? Freddy, can you coordinate with cloud services team to figure this out?
Flags: needinfo?(fbraun)
Summary: enable pinning on loop servers → enable pinning on services.mozilla.com
*.services.mozilla.com would include at least tracking protection, tiles, and sync (from scanning about:config).
Hey ckolos,

I remember that you helped out with the FxA bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1020485). Could you provide us a similar list for services.mozilla.com, or redirect this bug to someone who can?

Thanks,
Monica
Flags: needinfo?(ckolos)
[Tracking Requested - why for this release]: I very much would like the tiles ping to be pin-protected in 33.
:mmc I would think that we would not recommend that *.services.m.c be pinned only because we regularly add and delete things from it that domain for various reasons, including the development of products that ultimately do nothing/go nowhere.

There are a number of certs currently under the umbrella of s.m.c, including:

*.services.mozilla.com
*.sync.services.mozilla.com
loads.services.mozilla.com
location.services.mozilla.com
loop.services.mozilla.com
presence.services.mozilla.com
push.services.mozilla.com
tracking.services.mozilla.com

As I said, this list can and does change.
Flags: needinfo?(ckolos) → needinfo?(mmc)
OK, in that case, could we start out with subdomains that are known to be reaching release soonish, or already in production?

sync.services.mozilla.com
location.services.mozilla.com
loop.services.mozilla.com
tiles.services.mozilla.com
Flags: needinfo?(mmc) → needinfo?(ckolos)
:mmc  I'd rather the wildcard not get pinned, but if we have to, then I suppose we have to, but it *feels* like bad practice to do so. As it turns out, we have a mish-mosh of "stuff" out there, some individually cert'd and others using the wildcard. I'll need to clunk heads with the team and see if we can hammer out a strategy for this.

That being said, the list I pasted in c10 is a good place to start for individual certs. Tiles is not yet cert'd and is one of the props using the wildcard. Shavar is tracking.s.m.c.
Flags: needinfo?(ckolos)
Hey ckolos, no need to pin *.s.m.c if you don't think it is a good idea. All we need are the certs that are in use (including the wildcarded cert). We can be very liberal with the pin list.
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #13)
> Hey ckolos, no need to pin *.s.m.c if you don't think it is a good idea. All
> we need are the certs that are in use (including the wildcarded cert). We
> can be very liberal with the pin list.

Meaning that the list of acceptable certs can be large and only apply to subdomains of s.m.c you think would be safe.
In case it's not clear, we have generally not pinned the specific machine key since those expire and change. Rather most of our sites have been pinned to a Certificate Authority root (or two). Doesn't help if the CA we use gets hacked but eliminates the risk from a hundred other CAs (and gives us incentive to pick our CA for quality rather than on cost alone).
Seems like this is already taken care of by ckolos. Clearing needinfo.
Flags: needinfo?(fbraun)
:dveditz :mmc - Ah, yes, I forgot that it has more to do with the CA than it does with the actual cert itself. We are using Digicert (exclusively?) for our certs so it shouldn't be a problem to pin against the same CA as we did for accounts.firefox.com
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Attachment #8485100 - Flags: review?(dkeeler)
Attachment #8485100 - Flags: review?(dkeeler) → review+
https://hg.mozilla.org/mozilla-central/rev/54d37aeae2ce
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Attachment #8485100 - Flags: approval-mozilla-beta?
Attachment #8485100 - Flags: approval-mozilla-aurora?
Huh, the approval comments form didn't come up. I am requesting uplift because bsmedberg requested to get this into 33. Specific branch patches will be needed because of buildbot integration.

Approval Request Comment
[Feature/regressing bug #]: None.

[User impact if declined]: services.mozilla.org won't be pinned until Firefox 35 (http://monica-at-mozilla.blogspot.com/2014/08/firefox-32-supports-public-key-pinning.html)

[Describe test coverage new/current, TBPL]: telemetry dashboard at people.mozilla.org/~mchew/pinning_dashboard

[Risks and why]: 
This is to enable pinning in test mode, so it only collects telemetry. It has no other effect. Any follow up changes to enable pinning for real has higher risk.

[String/UUID change made/needed]: None.
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #21)
> Huh, the approval comments form didn't come up. I am requesting uplift
> because bsmedberg requested to get this into 33. Specific branch patches
> will be needed because of buildbot integration.

Please create the branch specific patches and request approval on those when they're ready. I'm going to deny your current patches with the expectation that we need different patches for both aurora and beta.

> Approval Request Comment
> [Feature/regressing bug #]: None.
> 
> [User impact if declined]: services.mozilla.org won't be pinned until
> Firefox 35
> (http://monica-at-mozilla.blogspot.com/2014/08/firefox-32-supports-public-
> key-pinning.html)

After speaking with Monica and Benjamin, the beta request is to pin the tiles server. As this is a subset of services.mozilla.org, can we restrict pinning to that domain in a separate bug for 33?

> [Risks and why]: 
> This is to enable pinning in test mode, so it only collects telemetry. It
> has no other effect. Any follow up changes to enable pinning for real has
> higher risk.

This makes me fairly uncomfortable as pinning for real likely won't happen until beta7. I will be somewhat more comfortable if we restrict pinning to tiles but I think we need to weight the risk vs reward of doing this in 33 before proceeding. I have asked Benjamin to speak with Monica about the points in this response.
Comment on attachment 8485100 [details] [diff] [review]
Enable pinning on services.mozilla.com in test mode (

I'm clearing the beta and aurora approval noms as Monica tells me that branch specific patches are needed. Please request approval on the branch specific patches when ready.

Given that this change needs ~2 weeks of measurement time, I would highly prefer to take this change in beta3, which goes to build tomorrow (Thursday).
Attachment #8485100 - Flags: approval-mozilla-beta?
Attachment #8485100 - Flags: approval-mozilla-aurora?
Comment on attachment 8487369 [details] [diff] [review]
Aurora patch to enable pinning on services.mozilla.com in test mode (

Review of attachment 8487369 [details] [diff] [review]:
-----------------------------------------------------------------

See comment 21.
Attachment #8487369 - Flags: approval-mozilla-aurora?
I don't think we should make these changes in beta. Even after setting test-only pins, waiting for telemetry to catch up, and then promoting the test pins to production, there's still a considerable chance that we'll break something. We don't want to do that in a late beta.
I agree with keeler and am requesting approval for 34 only.
Attachment #8487369 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Re-opening until everything's in production mode.
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
Attachment #8485100 - Attachment is obsolete: true
Attachment #8499152 - Attachment is obsolete: true
Attachment #8499156 - Attachment is obsolete: true
Comment on attachment 8499161 [details] [diff] [review]
Set is_moz if the pinset name contains mozilla (

Review of attachment 8499161 [details] [diff] [review]:
-----------------------------------------------------------------

The problem with the previous change is that violations for pinning.mozilla.org were being thrown into the "test" bucket which also contains facebook at the moment. This throws them into the "mozilla test" bucket instead. I thought about setting the bucket id but that seems kind of like cheating, because some but not all subdomains of services.mozilla.com pass the "operationally critical" test.

I didn't bother updating the output file because it's Thursday and m-i is closed.
Attachment #8499161 - Flags: review?(dkeeler)
Attachment #8499204 - Flags: review?(dkeeler)
Attachment #8499161 - Attachment is obsolete: true
Attachment #8499161 - Flags: review?(dkeeler)
Comment on attachment 8499204 [details] [diff] [review]
Set is_moz if the pinset name contains mozilla, set bucket id for pinsets containing the string mozilla (

Review of attachment 8499204 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Does this change the telemetry in the tests at all? I guess we'll find out :)
Attachment #8499204 - Flags: review?(dkeeler) → review+
* * *
Bug 1046430: Make cookie behavior the same on beta/release as on aurora/nightly
Attachment #8499204 - Attachment is obsolete: true
Attachment #8499245 - Attachment is obsolete: true
Comment on attachment 8499246 [details] [diff] [review]
Set is_moz if the pinset name contains mozilla, set bucket id for pinsets containing the string mozilla (

Review of attachment 8499246 [details] [diff] [review]:
-----------------------------------------------------------------

It did affect the unittest, so fixed. Reverting the generated part of the patch and waiting until try is open.
Attachment #8499246 - Flags: review+
FF 35 is not fixed until we promote this pin to production mode. FF 34 is already in beta, and we have an agreement not to make pinning changes in beta. I cleared the b2g flags because pinning is not enabled on b2g.
Attachment #8499246 - Attachment is obsolete: true
Comment on attachment 8518973 [details] [diff] [review]
Promote pin for services.mozilla.com to production mode (

Review of attachment 8518973 [details] [diff] [review]:
-----------------------------------------------------------------

Violation rates seem about on par with api.accounts.firefox.com. Since this covers services that are also used on startup (e.g., tiles) we expect this rate to be a little higher than normal.
Attachment #8518973 - Flags: review?(dkeeler)
Attachment #8518973 - Flags: review?(dkeeler) → review+
Comment on attachment 8518973 [details] [diff] [review]
Promote pin for services.mozilla.com to production mode (

Approval Request Comment
[Feature/regressing bug #]: None, new feature.
[User impact if declined]: tiles.services.mozilla.com won't be pinned until 36. bsmedberg particularly wanted Directory Tiles to be pinned.
[Describe test coverage new/current, TBPL]: Already in m-c for a week with no complaints
[Risks and why]: Pretty low risk
[String/UUID change made/needed]: None
Attachment #8518973 - Flags: approval-mozilla-aurora?
Attachment #8518973 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updating TM to 36 since that's where it landed on nightly.
Target Milestone: mozilla35 → mozilla36
Status: REOPENED → RESOLVED
Closed: 10 years ago9 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.