HSTS preload list automatic update needs to not remove URLs that are used in tests

RESOLVED FIXED in Firefox 23

Status

()

Core
Security: PSM
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: keeler)

Tracking

Trunk
mozilla26
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox23 fixed, firefox24 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
+++ This bug was initially created as a clone of Bug #883582 +++

See bug 883582. Some ******* ***** decided that it would be a good idea to keep going, getting us another broken update. Are we going to fix it this time?

Comment 1

5 years ago
I'll re-iterate what I said in https://bugzilla.mozilla.org/show_bug.cgi?id=883582#c5 - it's pretty simple for releng to turn off the updating on a particular branch for a while if we need to do so. 

If the automated updates are causing hardship, let me know if I should disabled them and on which branches.
Fun question: what happens with the update if the machine running it cannot reach the network (like, say, a releng slave behind bug 812342)?
Oh, which is of course behind the infra wall, but is about disabling network access from releng slaves via the firewall.
And since everyone seems to be on vacation and won't find the update in question on tbpl, the update was https://hg.mozilla.org/mozilla-central/rev/fd59f2cfbbff, changing everything to disabled because "TypeError: gSSService.processStsHeader is not a function", thus breaking both the feature, and the tests a la https://tbpl.mozilla.org/php/getParsedLog.php?id=26396026&tree=Mozilla-Central. Backed out in https://hg.mozilla.org/mozilla-central/rev/6cd6960ea7f6.

Does the update generating script have tests which are run on-push, checking that it does the right thing with a good host and a bad host, so that when someone breaks the script with a Gecko change we find out then, rather than finding out a week later at 3am on a Saturday?
(In reply to :Ms2ger from comment #0)
> See bug 883582. Some ******* ***** decided that it would be a good idea to
> keep going, getting us another broken update. Are we going to fix it this
> time?

https://bugzilla.mozilla.org/page.cgi?id=etiquette.html. I can guarantee you that nobody on my team would ever file a bug report written that way in your modules. Please treat us with the same level of respect with which we treat you.

It seems like this actually found a legitimate bug that would have went unnoticed otherwise. So, while the breakage is annoying, and we should try to to better, I still think it is a net win to know about these problems now.

Definitely, we can't be mass-deleting things from the preload list on error.

We should stop deleting entries automatically from the list. Somebody will need to review the proposed deletions manually and push the update to delete entries manually, unless/until we can find a better way of automatically deleting things (if such automation is even necessary).
Created attachment 790974 [details] [diff] [review]
fix getHSTSPreloadList.js

Looks like I missed a function rename in bug 887052. This patch will fix that and be more strict about removing HSTS preload list entries. In the future, it would be best to have xpcshell tests for the preload list script, but it looks like that will require bug 466524, and I don't want to block this fix on that at the moment.
Assignee: nobody → dkeeler
Status: NEW → ASSIGNED
Attachment #790974 - Flags: review?(cviecco)
Attachment #790974 - Flags: review?(cviecco) → review+
Thanks, Camilo.

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

I'll file a bug for adding testing for the script.
https://hg.mozilla.org/mozilla-central/rev/af90d4723727
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
(In reply to David Keeler (:keeler) from comment #7)
> Thanks, Camilo.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/af90d4723727
> 
> I'll file a bug for adding testing for the script.

Filed bug 905852.
You need to log in before you can comment on or make changes to this bug.