Closed Bug 1197607 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: Security: PSM, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla43
Tracking Status
firefox40 --- unaffected
firefox41 --- unaffected
firefox42 --- verified
firefox43 --- verified
firefox-esr38 --- verified

People

(Reporter: RyanVM, Assigned: nthomas)

Details

Attachments

(2 files)

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.
The timing is such that I wonder if this is more FTP shutoff fallout.
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.
Coop, can you maybe look into this?
Flags: needinfo?(coop)
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.
I'm testing a fix for this.
Assignee: nobody → nthomas
Component: General Automation → Security: PSM
Flags: needinfo?(coop)
Product: Release Engineering → Core
QA Contact: catlee
Summary: Automated updates aren't running on most trees → Automated hsts hkpk updates aren't running on most trees
Summary: Automated hsts hkpk updates aren't running on most trees → Automated hsts & hpkp updates are failing on mozilla-central, mozilla-aurora, mozilla-esr38
http://hg.mozilla.org/build/tools/rev/6601ee76ecd5 for a minor script update, so that we echo the command used correctly.
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.
Attachment #8655796 - Flags: review?(odvarko)
Attached file 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).
Attachment #8655797 - Attachment mime type: text/x-patch → text/plain
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
Attachment #8655796 - Flags: review?(odvarko)
I did, thanks for the tip.
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.
Attachment #8655796 - Flags: review?(cykesiopka.bmo)
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!
Attachment #8655796 - Flags: review?(cykesiopka.bmo) → review+
2c6a43112cb1 includes the two requested modifications to PreloadedHPKPins.json. We won't need to uplift those to aurora or esr38.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
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
Attachment #8655796 - Flags: approval-mozilla-esr38?
Attachment #8655796 - Flags: approval-mozilla-aurora?
Attachment #8655796 - Flags: approval-mozilla-esr38?
Attachment #8655796 - Flags: approval-mozilla-esr38+
Attachment #8655796 - Flags: approval-mozilla-aurora?
Attachment #8655796 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.