1004781 - enable pinning for facebook

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[[:mmc] Monica Chew (no longer reading bugmail)]

Facebook is not in Chrome's pinset. However, from looking through the CT database, they use 44 certs. That's pretty good. We should keep an eye on them over time to see if the cert set changes, as well as make contact to see if they're willing to coordinate.

Comment 1

[Reed Loden [:reed]]

Why would we ever want to pin something without explicit consent from a site? That seems like a massive problem just waiting to happen.

Comment 2

[[:mmc] Monica Chew (no longer reading bugmail)]

(In reply to Reed Loden [:reed] from comment #1)
> Why would we ever want to pin something without explicit consent from a
> site? That seems like a massive problem just waiting to happen.

We don't, due to coordination issues. They're an obvious candidate due to high volume, though, and responded positively to our request.

From Scott: *.facebook.com should be pinned to

DigiCert EV Root (Subject Key Identifier
B1:3E:C3:69:03:F8:BF:47:01:D4:98:26:1A:08:02:EF:63:64:2B:C3) and the

Verisign Class 3 G3 root (no SKI, but serial number
9b:7e:06:49:a3:3e:62:b9:d5:ee:90:48:71:29:ef:57).

Comment 3

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Enable pinning in test mode for facebook (

Comment 4

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8480144 [details] [diff] [review]
Enable pinning in test mode for facebook (

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

David's out today, let's try to get this merged into m-c before the script generation on Saturday to avoid conflicts.

Comment 5

[Camilo Viecco (:cviecco)]

Comment on attachment 8480144 [details] [diff] [review]
Enable pinning in test mode for facebook (

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

Looks good to me. I assume you have tried browsing basic fb pages?

Comment 6

[[:mmc] Monica Chew (no longer reading bugmail)]

(In reply to Camilo Viecco (:cviecco) from comment #5)
> Comment on attachment 8480144 [details] [diff] [review]
> Enable pinning in test mode for facebook (
> 
> Review of attachment 8480144 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me. I assume you have tried browsing basic fb pages?

That wouldn't do anything, since we're in test mode.

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/27305d251dd6

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/27305d251dd6

Comment 9

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: follow-up to add DigiCert ECC Secure Server CA

This doesn't seem like too much of a hack. We could even add Google's missing certs, if we wanted.

Comment 10

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8485223 [details] [diff] [review]
follow-up to add DigiCert ECC Secure Server CA

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

Thanks!

Comment 11

[[:mmc] Monica Chew (no longer reading bugmail)]

Btw, buildbot touches these 3am Saturday so you either need to make sure it merges to m-c before then or wait until after that to checkin.

Comment 12

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/593d3c34f077

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/593d3c34f077

Comment 14

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8480144 [details] [diff] [review]
Enable pinning in test mode for facebook (

This is a similar approval request to https://bugzilla.mozilla.org/show_bug.cgi?id=1030135#c21, except I'd like to target FF 34.

Approval Request Comment
[Feature/regressing bug #]: None.
[User impact if declined]: Facebook won't be pinned until Firefox 35.
[Describe test coverage new/current, TBPL]: Telemetry dashboard at people.mozilla.org/~mchew/pinning_dashboard.
[Risks and why]: This is a telemetry change at this point.
[String/UUID change made/needed]: None.

Comment 15

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8485223 [details] [diff] [review]
follow-up to add DigiCert ECC Secure Server CA

Approval Request Comment

See comment 14.

Comment 16

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Aurora-specific patch to enable pinning in test mode for facebook (

Comment 17

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8487414 [details] [diff] [review]
Aurora-specific patch to enable pinning in test mode for facebook (

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

See comment 14.

Comment 18

[[:mmc] Monica Chew (no longer reading bugmail)]

I misattributed this patch to a=lmandel instead of bhavana. Sorry!

https://hg.mozilla.org/releases/mozilla-aurora/rev/6f5c35805e16

Comment 19

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Remove our pinset for facebook since it's in chromium now (

Comment 20

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8513899 [details] [diff] [review]
Remove our pinset for facebook since it's in chromium now (

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

https://chromium.googlesource.com/chromium/src/+/master/net/http/transport_security_state_static.json + ctrl-f "facebook"

Comment 21

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8513899 [details] [diff] [review]
Remove our pinset for facebook since it's in chromium now (

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

LGTM. Have you confirmed that the pinning information generation script successfully finds all the certificates it needs to with this change?

Comment 22

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: fixup

Yep, this is the output of manually re-running the generator script.

Comment 23

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Actually remove the pinset

Comment 24

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Output of generator script

Comment 25

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Actually remove the pinset (

Comment 26

[[:mmc] Monica Chew (no longer reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/90f7e5edc367

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cef5aaa2c0da
https://hg.mozilla.org/mozilla-central/rev/90f7e5edc367

Comment 28

[Reed Loden [:reed]]

Comment on attachment 8514670 [details] [diff] [review]
Actually remove the pinset (

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

::: security/manager/tools/PreloadedHPKPins.json
@@ +230,4 @@
>    ],
>  
>    "extra_certificates": [
>       // DigiCert ECC Secure Server CA (for Facebook)

Should this be removed as well, or is it still needed, even though that cert could be pulled from the Chromium list now?

Comment 29

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Remove unnecessary cert for facebook (

Comment 30

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8516821 [details] [diff] [review]
Remove unnecessary cert for facebook (

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

Thanks, Reed. We don't need that entry since we're also pulling certs from chromium. The only difference is that since it's not a root cert, it will be called kGOOGLE_PIN_DigiCertECCSecureServerCAFingerprint instead of kDigiCert_ECC_Secure_Server_CAFingerprint in the output file.

Comment 31

[[:mmc] Monica Chew (no longer reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9c8ca05b975a

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9c8ca05b975a

Comment 33

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: facebook

Metrics look good.

Comment 34

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: output

Output, not for checkin.

Comment 35

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8535082 [details] [diff] [review]
facebook

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

Great!

Comment 36

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8535083 [details] [diff] [review]
output

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

This looks right. I'll just give feedback+ since this isn't for check-in.

Comment 37

[[:mmc] Monica Chew (no longer reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ced6676b1d6c

Comment 38

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ced6676b1d6c

Comment 39

[[:mmc] Monica Chew (no longer reading bugmail)]

I should have marked this bug fixed when we switched to production mode. We're enforcing Chrome's pins for facebook as of FF 37.