1197607 - Automated hsts & hpkp updates are failing on mozilla-central, mozilla-aurora, mozilla-esr38

Status: VERIFIED FIXED

(Core :: Security: PSM, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

Looking across the various trees, the only automated update I see to have successfully run in the last while now is automated blocklist updates on mozilla-beta. Trunk/Aurora/esr38 haven't seen anything.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

The timing is such that I wonder if this is more FTP shutoff fallout.

Comment 2

[Phil Ringnalda (:philor)]

They're running, but I'd say that http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64/mozilla-central-linux64-periodicupdate-bm72-build1-build1.txt.gz is two bugs, one that something broke HSTS updates between the 1st and 8th of August, and a second that periodic updates fail out and don't do anything else when that step fails, while only leaving the negative error report of having not pushed to alert anyone.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Coop, can you maybe look into this?

Comment 4

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

There's actually two separate bustages

Last working     mozilla-central-linux64-periodicupdate-bm72-build1-build0.txt.gz, 2015-08-01 03:02 PT
HKPK busted      mozilla-central-linux64-periodicupdate-bm91-build1-build0.txt.gz, 2015-08-08 03:03 PT
HSTS busted too  mozilla-central-linux64-periodicupdate-bm91-build1-build0.txt.gz, 2015-08-15 03:02 PT

Aurora breaks between the 8th and 15th, when we merged from m-c. ESR38 is broken for a different reason (no firefox/latest-mozilla-esr38 now). Beta and Release only do blocklist, which is still functional.

Our script hasn't changed in that window. getHSTSPreloadList.js, genHPKPStaticPins.js, and other ancillary files haven't changed either. The latest xpcshell launches fine by itself (ie not another dep library problem). But tellingly, if I try to reproduce the 2015-08-01 run I get an error too.

Which eventually leads to this diff
  https://chromium.googlesource.com/chromium/src/net/+/a51974cea8966bd4b0bc39c2c8286d3271e8367d%5E!/#F2
where Google is adding report_uri entries like
+      "report_uri": "http://clients3.google.com/cert_upload_json"

Our regex at
  http://hg.mozilla.org/mozilla-central/file/default/security/manager/tools/getHSTSPreloadList.js#l89
handily replaces that with
        "report_uri": "http:    },
which is invalid JSON at line 92.

HKPK does a similar thing at 
  http://hg.mozilla.org/mozilla-central/file/default/security/manager/tools/genHPKPStaticPins.js#l148
on the same file.

Comment 5

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

I'm testing a fix for this.

Comment 6

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

http://hg.mozilla.org/build/tools/rev/6601ee76ecd5 for a minor script update, so that we echo the command used correctly.

Comment 7

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

Attachment: Update regex for new use of // in transport_security_state_static.json

Honza, would you mind looking at this while dkeeler is out ?

This patch removes lines with comments at the start or in the middle of lines, while leaving lines containing urls alone. It also removes empty lines, which is tidy but not necessary. This would need to be uplifted to aurora but needn't go further for MoCo, as we don't process hsts or hpkp updates on beta or release.

Comment 8

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

Attachment: Effect of regex change

This is the diff between the retrieved file, unmodified vs modified with the new regex (as dumped by the getHSTSPreloadList.js).

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8655796 [details] [diff] [review]
Update regex for new use of // in transport_security_state_static.json

I can't do review on this, but perhaps you
meant to ask Jan Bambas (aka mayhemer)?

Honza

Comment 10

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

I did, thanks for the tip.

Comment 11

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

Comment on attachment 8655796 [details] [diff] [review]
Update regex for new use of // in transport_security_state_static.json

Cykesiopka, I'm looking for a reviewer for this patch, can you help ? Background information in comments #4 and #7.

Comment 12

[:Cykesiopka]

Comment on attachment 8655796 [details] [diff] [review]
Update regex for new use of // in transport_security_state_static.json

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

Hmm, so this new regex regresses removing trailing comments...

"{'x': true}, // test\n".replace(/\/\/[^\n]*\n/g, "");
> "{'x': true}, "

"{'x': true}, // test\n".replace(/^(\s*)?\/\/[^\n]*\n/mg, "");
> "{'x': true}, // test
> "

However, it doesn't look like the data we receive has trailing comments (yet).
I guess we can deal with this later.

In addition, Bug 1190794 updated the root store for Firefox 42 and above, and removed the "TC TrustCenter Universal CA I" and "TC TrustCenter Class 2 CA II" roots.
Essentially, genHPKPStaticPins.js will still fail even after the regex fix.

So, r+ with these two lines commented out as well:
https://hg.mozilla.org/mozilla-central/annotate/0d376102df5f/security/manager/tools/PreloadedHPKPins.json#l172
https://hg.mozilla.org/mozilla-central/annotate/0d376102df5f/security/manager/tools/PreloadedHPKPins.json#l174

(Note: I started testing the changes to getHSTSPreloadList.js, but gave up since it takes a really long time to run).

Thanks for looking into this!

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c6a43112cb1

Comment 14

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

2c6a43112cb1 includes the two requested modifications to PreloadedHPKPins.json. We won't need to uplift those to aurora or esr38.

Comment 15

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

https://hg.mozilla.org/mozilla-central/rev/2c6a43112cb1

Comment 16

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

I forced the job that lands updates, and got:

hsts       https://hg.mozilla.org/mozilla-central/rev/e5b9d76874f8
hpkp       https://hg.mozilla.org/mozilla-central/rev/bb0711d3f60a
blocklist  https://hg.mozilla.org/mozilla-central/rev/7f987c38bd3e

Comment 17

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

Comment on attachment 8655796 [details] [diff] [review]
Update regex for new use of // in transport_security_state_static.json

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Stale hsts and hpkp security data in releases
Fix Landed on Version: 43.0
Risk to taking this patch (and alternatives if risky): This isn't part of the Firefox build, it's used in a once-a-week-job to update the security data
String or UUID changes made by this patch: None

Comment 18

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/5a18731300d8
https://hg.mozilla.org/releases/mozilla-esr38/rev/16f91ff2ddf8

ni? myself to verify after the weekend.

Comment 19

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

Worked on mozilla-esr38
   https://hg.mozilla.org/releases/mozilla-esr38/rev/b4acf88f64a5
   https://hg.mozilla.org/releases/mozilla-esr38/rev/791b899f15ac
   https://hg.mozilla.org/releases/mozilla-esr38/rev/710ef1cc4985
and mozilla-central
   https://hg.mozilla.org/mozilla-central/rev/91107089dffa
   https://hg.mozilla.org/mozilla-central/rev/139446daedab

On aurora hpkp failed because I missed that NSS_3_19_3_RTM is there too, so I uplifted the removal of the two CAs:
   https://hg.mozilla.org/releases/mozilla-aurora/rev/4d808743d860
and on running the update job got
   https://hg.mozilla.org/releases/mozilla-aurora/rev/a92ca81c6579
   https://hg.mozilla.org/releases/mozilla-aurora/rev/4baf7affe751
   https://hg.mozilla.org/releases/mozilla-aurora/rev/f9f0b25ca40d

All done here.