1224968 - HPKP periodic updates broken on mozilla-central, mozilla-aurora and mozilla-esr38

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Phil Ringnalda (:philor)]

Fun fun. We had successful updates on mozilla-central and mozilla-aurora on 2015-10-24, then on 2015-10-31, 2015-11-07 and 2015-11-14 they hung somewhere (never trust a log not to have some output caching that hides things from you) after what should have been the end of the HSTS fetching and start of the HSTS new file writing, e.g. http://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-aurora-linux64/mozilla-aurora-linux64-periodicupdate-bm73-build1-build0.txt.gz

That's a plenty weird failure, since the buildstep takes 12 hours 36 minutes, ending with a 20 minute no-output timeout, which would argue that it took more than 12 hours to run through what should actually only take around 40 minutes, but then, never trust a log to show you everything.

So Nick kicked off another one so he could look at what it did while it was hung, and instead got http://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-linux64/mozilla-central-linux64-periodicupdate-bm72-build1-build0.txt.gz where the HSTS update ran just fine, and then HPKP silently failed after what are apparently, given http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/StaticHPKPins.errors, expected messages.

Through all three of those failure weekends, esr38 has just plugged along being perfectly successful, which argues that we landed something on m-c and m-a that broke it between 2015-10-24 and 2015-10-31, though that half-successful manually triggered run suggests... we don't have a clue what to even guess.

Comment 1

[:Cykesiopka]

(In reply to Phil Ringnalda (:philor) from comment #0)
> So Nick kicked off another one so he could look at what it did while it was
> hung, and instead got
> http://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-linux64/
> mozilla-central-linux64-periodicupdate-bm72-build1-build0.txt.gz where the
> HSTS update ran just fine, and then HPKP silently failed after what are
> apparently, given
> http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/
> StaticHPKPins.errors, expected messages.

I have no idea what cause the previous failures, but now that Bug 1211568 has landed in m-c, Bug 1224481 needs to as well to keep the HPKP updates working. I meant to ping someone about this, but forgot about it completely. Sorry about that.

Comment 2

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

Coincidentally, last week I had a look at why the automated updates were failing. This led me to filing bug 1224328, which I'm fairly sure is the cause of the timeouts in the HSTS script. That was fixed rather quickly, but not fast enough to have merged before the scripts ran on Saturday. My guess is it merged after the automated scripts but before Nick ran the job in comment 0. At the same time, we updated the NSS in mozilla-central to 3.21, which removed some root certificates. Unfortunately I forgot that this could (and indeed did) break the HPKP updater. Cykesiopka filed bug 1224481 to address that (it looks like it's inbound now). Once that lands, the automated scripts should work.

In any case, sorry about the confusion. I suppose I was too optimistic in thinking the fixes would get merged before the scripts ran on Saturday. I'll keep an eye out to make sure everything gets uplifted to aurora so both branches (hopefully) successfully run the updates this Saturday.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Nice sleuthing. I've forced another job, now that bug 1224481 has merged to central.

Comment 4

[:Cykesiopka]

So updates happened normally on 2015-11-21 at least:
https://hg.mozilla.org/mozilla-central/rev/e2a5d5ece4b4
https://hg.mozilla.org/mozilla-central/rev/f176beca9f85
https://hg.mozilla.org/releases/mozilla-aurora/rev/e0b4cee06507
https://hg.mozilla.org/releases/mozilla-aurora/rev/3999c84a3ba4

Unfortunately, it looks like something has broken the scripts again - I don't see anything for today (2015-11-28).
I'll look into fixing HPKP at least.

Comment 5

[:Cykesiopka]

Attachment: MozReview Request: Bug 1224968 - Support public key input to unbreak periodic HPKP updates.

Bug 1224968 - Support public key input to unbreak periodic HPKP updates.

https://chromium.googlesource.com/chromium/src/net/+/be448badb1982bfa13572d430774ff6f275aeb0c%5E!/#F0 switched SHA1 hashes to public keys for static pins. This broke genHPKPStaticPins.js and thus periodic HPKP updates, since the file doesn't handle public keys.

The changes here mostly mirror https://github.com/agl/transport-security-state-generate/commit/ba1f296240ceed7a84ee4a79a4eb23afea965621.

Comment 6

[:Cykesiopka]

https://reviewboard.mozilla.org/r/26459/#review23881

Note: https://chromium.googlesource.com/chromium/src/net/+/045698d824608d492f545455fc51ecc678a8c8ff should mean that genHPKPStaticPins.js technically doesn't need to handle SHA1 hashes from the Chromium transport_security_state_static.certs file anymore.

However, I decided to defer any simplifications to the code just in case the Chromium change gets backed out.

Comment 7

[:Cykesiopka]

The changes are NPOTB, but since I was doing a try push anyways: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a86294658287

Comment 8

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

Comment on attachment 8693454 [details]
MozReview Request: Bug 1224968 - Support public key input to unbreak periodic HPKP updates.

https://reviewboard.mozilla.org/r/26459/#review23951

Great work - thanks!

::: security/manager/tools/genHPKPStaticPins.js:253
(Diff revision 1)
>          if (line.startsWith(SHA1_PREFIX) ||

If I understand correctly, this bit is now probably going to be dead code moving forward. Let's file a bug on removing it.

::: security/manager/tools/genHPKPStaticPins.js:299
(Diff revision 1)
> +          // TODO: Switch to SHA-256 instead. SHA1 is used here mainly to

Let's file a bug for this work (actually, this can probably be part of the other bug mentioned above).

Comment 9

[:Cykesiopka]

https://reviewboard.mozilla.org/r/26459/#review23951

Thanks for the review!

I filed Bug 1229284 for the two comments below.

Comment 10

[:Cykesiopka]

Attachment: bug1224968_support-public-keys-in-hpkp-script_v2.patch

+ Mention Bug 1229284 in TODOs for removing SHA1 support

Comment 11

[:Cykesiopka]

Try push is in comment 7, but again, the changes here are NPOTB.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1634149ae629

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/1634149ae629

Comment 14

[:Cykesiopka]

Hi Nick,

Would it be possible to force a HPKP and HSTS job? I believe the changes here are going to need to be uplifted to m-a and esr38 as well, so confirmation that the HPKP script works again would be nice. I also want to make sure that HSTS hasn't been broken either.

If not, waiting till Saturday is probably OK as well.

Thanks!

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

Running it now, will post the log when it's done.

Comment 16

[Phil Ringnalda (:philor)]

It better hope it doesn't run into merges, since the only protection it has against being midaired is running in the wee hours of Saturday when none of us would be merging.

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

Looks like it worked:
remote: View your changes here:
remote:   https://hg.mozilla.org/mozilla-central/rev/880b58f2221a
remote:   https://hg.mozilla.org/mozilla-central/rev/291bf32f6341
remote:   https://hg.mozilla.org/mozilla-central/rev/eec9a0bfd929

https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-linux64/mozilla-central-linux64-periodicupdate-bm74-build1-build0.txt.gz

Comment 18

[:Cykesiopka]

(In reply to Phil Ringnalda (:philor) from comment #16)
> It better hope it doesn't run into merges, since the only protection it has
> against being midaired is running in the wee hours of Saturday when none of
> us would be merging.

Ah - apologies if this has already caused issues, or does in the future.

Comment 19

[:Cykesiopka]

Comment on attachment 8694062 [details] [diff] [review]
bug1224968_support-public-keys-in-hpkp-script_v2.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Stale static HPKP pins.
Fix Landed on Version: 45
Risk to taking this patch (and alternatives if risky): None. This is a NPOTB change that fixes periodic HPKP updates.
String or UUID changes made by this patch: None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 20

[:Cykesiopka]

(Changing the summary of the bug, since I did kind of morph it a bit.)

Comment 21

[:Cykesiopka]

Comment on attachment 8694062 [details] [diff] [review]
bug1224968_support-public-keys-in-hpkp-script_v2.patch

Cancelling aurora request since the request wasn't approved before the merge.

- HPKP periodic updates aren't run on beta, so changes to the script don't really matter there.
- The beta pins expire on 2015-02-27, and release is on 2015-01-26, so tests won't break.
- The preloaded pins will still be valid for a month. This is less than usual, which is annoying, but not a huge deal.

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8694062 [details] [diff] [review]
bug1224968_support-public-keys-in-hpkp-script_v2.patch

It's NPOTB, taking it for esr38.6

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Liz, Jordan, just fyi, I took this change for esr38.6. If you have any concerns, please let me know.

Comment 25

[Jordan Lund (:jlund)]

sounds good.

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/25cdd180ee2e