Closed Bug 1224968 Opened 4 years ago Closed 4 years ago

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

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(firefox45 fixed, firefox-esr38 fixed)

RESOLVED FIXED
Tracking Status
firefox45 --- fixed
firefox-esr38 --- fixed

People

(Reporter: philor, Assigned: Cykesiopka)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Flags: needinfo?(dkeeler)
(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.
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.
Flags: needinfo?(dkeeler)
Nice sleuthing. I've forced another job, now that bug 1224481 has merged to central.
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.
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.
Attachment #8693454 - Flags: review?(dkeeler)
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.
The changes are NPOTB, but since I was doing a try push anyways: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a86294658287
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).
Attachment #8693454 - Flags: review?(dkeeler) → review+
https://reviewboard.mozilla.org/r/26459/#review23951

Thanks for the review!

I filed Bug 1229284 for the two comments below.
+ Mention Bug 1229284 in TODOs for removing SHA1 support
Assignee: nobody → cykesiopka.bmo
Attachment #8693454 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8694062 - Flags: review+
Try push is in comment 7, but again, the changes here are NPOTB.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1634149ae629
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
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!
Flags: needinfo?(nthomas)
Running it now, will post the log when it's done.
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.
(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 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.
Attachment #8694062 - Flags: approval-mozilla-esr38?
Attachment #8694062 - Flags: approval-mozilla-aurora?
(Changing the summary of the bug, since I did kind of morph it a bit.)
Summary: HSTS/HPKP periodic updates broken on mozilla-central and mozilla-aurora → HPKP periodic updates broken on mozilla-central, mozilla-aurora and mozilla-esr38
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.
Attachment #8694062 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1231035
Comment on attachment 8694062 [details] [diff] [review]
bug1224968_support-public-keys-in-hpkp-script_v2.patch

It's NPOTB, taking it for esr38.6
Attachment #8694062 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Liz, Jordan, just fyi, I took this change for esr38.6. If you have any concerns, please let me know.
Flags: needinfo?(lhenry)
Flags: needinfo?(jlund)
sounds good.
Flags: needinfo?(jlund)
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.