Skip to content
This repository has been archived by the owner on Nov 6, 2023. It is now read-only.

Discussion for guidelines on removing HSTS preload rulesets #7126

Closed
jeremyn opened this issue Sep 25, 2016 · 85 comments
Closed

Discussion for guidelines on removing HSTS preload rulesets #7126

jeremyn opened this issue Sep 25, 2016 · 85 comments
Labels

Comments

@jeremyn
Copy link
Contributor

jeremyn commented Sep 25, 2016

@gloomy-ghost has submitted several pull requests deleting rulesets that are in the HSTS preload lists: Chromium, Firefox, Tor. ( @gloomy-ghost has helpfully included links to these rulesets in their pull requests.) @J0WI and I have separately handled these and it turns out we disagree on when a ruleset should be deleted. We have mainly discussed this in pull request #7081 , but I'll repeat the discussion below.

@J0WI rejects rulesets for deletion if the Strict-Transport-Security header does not contain the preload header that is required in the HSTS preload submission requirements, even if the domain is in the preload lists. I assume that the preload header was in the lists when the domain was submitted to the lists and was later removed for whatever reason. This turns to be a common situation. My own opinion is that if the domain is in the preload lists and it has a Strict-Transport-Security header, then we can delete the ruleset even if preload is missing. @J0WI correctly points out that according to the submission requirements,

Chrome has not yet removed any domains from the preload list for failing to keep up the requirements after submission, but there are plans to do so in the future.

I think it is too restrictive to not delete a ruleset in the preload lists just because it might be removed from the lists later.

Another problem that I found in pull request #7053 : https://wiki.openssl.org has a complete Strict-Transport-Security:"max-age=31536000; includeSubDomains; preload" header, but its redirect target https://wiki.openssl.org/index.php/Main_Page doesn't even have a Strict-Transport-Security header. Should we delete a ruleset if some of the responses have the header but some don't? If so, how many URLs do we have to check? What if only preload is missing?

Pinging @fuglede and @Hainish for their input.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 25, 2016

@J0WI wrote here:

As @ScottHelme analyzed in his blog post some admins just copied the config from somewhere else and got added to the preload list without knowing the consequences.
So I think we should make sure, that they are in the current preload list (e.g. Tor), as well as in the preload list of future versions (e.g. mozilla central) and that they serve a valid HSTS preload header.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 25, 2016

@J0WI You can't get added to the HSTS lists without having preload in the header. For the sites we're seeing that are in the lists but don't have preload, I assume the site admin had preload when they were added but then decided to remove it for some reason. So these sites aren't just blindly copy-and-pasting, in fact the opposite: they had preload at one point and then specifically decided to remove it from the header, possibly because they were already in the lists and didn't feel like they needed preload anymore.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 25, 2016

Also, we should clearly document any requirements we have for the Strict-Transport-Security header. @gloomy-ghost has submitted about 120 pull requests on this and it is not fair to them to let them do all that work if we are going to reject a lot of it because of undocumented requirements.

@J0WI
Copy link
Contributor

J0WI commented Sep 25, 2016

I was not able to reproduce the issue in #7081 (comment), but since this already is a subdomain and the main host (openssl.org) is preloaded it's not required to send a full HSTS header there.

@J0WI
Copy link
Contributor

J0WI commented Sep 25, 2016

I think we have to check the Alexa top-* rules more carefully if they really pass all HSTS requirements. If they are configured solid, it's likely to have them still preloaded in future. If they are not well configured, it's even more likely that Google will kick them off the list.

@J0WI
Copy link
Contributor

J0WI commented Sep 25, 2016

Just a few more thoughts:
The motivation to remove HSTS preloaded rules is to shrink the extension and the resources it uses.
Before the preload list was shared between all major browsers, we had a platform for them. We could fetch all (pending) hosts from the preload to auto generate a huge ruleset (see #4396), but this would also blow up the extension.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 25, 2016

For the https://wiki.openssl.org issue: on review I agree that HSTS works at the subdomain level, so if the header is good for https://openssl.org and includes subdomains, it is also good for https://wiki.openssl.org/index.php/Main_Page , which is what you said. So we agree. Also the submission requirements say:

If you are serving an additional redirect from your HTTPS site, that redirect must still have the HSTS header (rather than the page it redirects to).

which is the behavior I'm seeing for the redirect I described, which is fine.

The header requirements should be the same for any ruleset, not just the Alexa ones, but I agree that as usual the higher Alexa domains should get more attention.

After more thought I've changed my mind and agree with you @J0WI about checking that the submission requirements are met before we delete a ruleset, to make sure that the site admin knows what they're doing. In practice this means:

  • HTTPS works on base domain
  • HTTP redirects to HTTPS on the base domain (without HTTPS Everywhere)
  • The HTTPS page serves a valid Strict-Transport-Security header with max-age, includeSubDomains, and preload.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 25, 2016

Also, if we reject a deletion pull request because the requirements aren't met, we should just close the pull request with a comment because that's not something the contributor can fix.

I've revert the deletions you noticed earlier, thanks for checking them.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 25, 2016

@J0WI When we check the header, we should check the base domain and not worry about www. For example in #6995 (comment) , https://www.v2ex.com has a bad header, but https://v2ex.com is fine, so the deletion is valid. What do you think?

@J0WI
Copy link
Contributor

J0WI commented Sep 25, 2016

I merged the revert commits and can confirm that v2ex.com sends a valid header.

As soon as we have a guideline about this, we can also add checks for it to our ruleset tests.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 25, 2016

@J0WI Do you think we should stop reviewing and merging other HSTS deletion pull requests until we hear from the others? I think you and I agree on the rules.

@gloomy-ghost
Copy link
Collaborator

For HTTP redirects to HTTPS on the base domain (without HTTPS Everywhere)

What if the site does not support http anymore after the preload? Such as calomel.org

$ curl -I https://calomel.org | grep preload
strict-transport-security: max-age=31556926; includesubdomains; preload
$ curl -I http://calomel.org --connect-timeout 30
curl: (28) Connection timed out after 30000 milliseconds

@gloomy-ghost
Copy link
Collaborator

I have created about 128 PRs if my record is correct. There are some statistical data:

HTTP Timeout: 1
Not sending header (anymore): 7
Incomplete certificate chain (Still working after cache intermediate) : 1

HSTS Header: 121
HSTS Preload: 105
HSTS Subdomain: 105
HSTS MaxAge > 18 weeks: 120

@J0WI
Copy link
Contributor

J0WI commented Sep 26, 2016

What if the site does not support http anymore after the preload? Such as calomel.org

That a requirement I not really agree with Google, but I think we should strictly follow their guideline.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 26, 2016

For redirecting: once a site is HSTS preloaded, the HTTP version of the site becomes inaccessible, which I guess is why Google checks that HTTP redirects to HTTPS.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 26, 2016

Something else to watch out for: for pull request #7077 , http://raymii.org returns a HTTP 200 but it uses a "meta refresh" to redirect to https://raymii.org/s/ . So unless this configuration was changed after it was accepted into the preload list, it means that:

  • meta refresh redirects are valid, so a HTTP 200 does not mean the site is misconfigured, and
  • the redirect can go to a different path on the same host, in this case / to /s/.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 26, 2016

@J0WI You didn't answer the question in #7126 (comment) but I'm assuming deletion pull requests are open for review now.

@jeremyn
Copy link
Contributor Author

jeremyn commented Sep 26, 2016

Another thing to look for: in pull request #7060 , http://whatwg.org redirects to http://www.whatwg.org , which redirects to https://whatwg.org . So unless the configuration was changed after it was accepted into the preload list, this means that:

  • as long as the ultimate redirect destination of the HTTP URL is a HTTPS URL on the same host, it's okay if the redirect chain goes through other HTTP URLs or URLs with a different host, in this case ^ and www.

@jeremyn
Copy link
Contributor Author

jeremyn commented Oct 20, 2016

Thanks @mnordhoff . My interest is knowing which tag/HSTS file my current browser is using. It looks like that is the RELEASE tags like FIREFOX_49_0_1_RELEASE which you linked. I don't know if we want to check ESR, other than the version included with Tor, and if so which versions. I haven't been checking them.

@Hainish
Copy link
Member

Hainish commented Oct 20, 2016

@mnordhoff the most reliable way to determine the current release of ESR and stable that I've found is:

STABLE_VERSION=$(curl -D /dev/stdout "https://download.mozilla.org/?product=firefox-latest&os=linux64&lang=en_US" 2>&1 | sed -n '/Location: /{s|.*/firefox-\(.*\)\.tar.*|\1|p;q;}') && \
DEV_VERSION=$(curl -D /dev/stdout "https://download.mozilla.org/?product=firefox-aurora-latest-ssl&os=linux64&lang=en-US" 2>&1 | sed -n '/Location: /{s|.*/firefox-\(.*\)\.tar.*|\1|p;q;}') && \
ESR_VERSION=$(curl -D /dev/stdout "https://download.mozilla.org/?product=firefox-esr-latest&os=linux64&lang=en_US" 2>&1 | sed -n '/Location: /{s|.*/firefox-\(.*\)\.tar.*|\1|p;q;}')

This is the method that we use in the https://github.com/EFForg/https-everywhere-docker-base/ repo.

Once we get that, I think it makes sense to checkout the tag corresponding to whichever release you're interested in, and then looking at the preload list included there.

@mnordhoff
Copy link
Contributor

Sounds legit. Or sort of legit. With a little more sed and curl it's enough to download the current lists from hgweb, or check them out locally.

@Hainish
Copy link
Member

Hainish commented Jan 5, 2017

A little guidance here:

We should only remove target domains which are HSTS preloaded if the preload is active for all currently supported versions of Firefox (ESR, Stable, and Dev) as well as the current version of Chromium.

I'll add this to the CONTRIBUTING.md, and I've created a new task to script this process: #8122

@Hainish
Copy link
Member

Hainish commented Jan 7, 2017

#8127 is a node utility which fetches the HSTS rulesets from Firefox {Dev, Stable, ESR} and Chromium (head), and deletes targets contained in all of them. It also deletes rulesets completely if all targets would have otherwise been deleted.

Once this is merged we can incorporate this into the release process.

@Hainish
Copy link
Member

Hainish commented Jan 7, 2017

#8128 is the result of running the abovementioned utility

@Hainish
Copy link
Member

Hainish commented Jan 10, 2017

freedomofpress/securedrop#1517 (comment) provides additional context for the process of HSTS removal, which has not been formalized just yet. I suspect once this process is formalized, we will have to revisit the HSTS pruning utility in #8127

@lgarron
Copy link

lgarron commented Jan 10, 2017

freedomofpress/securedrop#1517 (comment) provides additional context for the process of HSTS removal, which has not been formalized just yet.

To add more context, the first step will likely be to start removing "new" (since Feb. 29, 2016) entries that respond over HTTPS, but do not send exactly one header, or that do not have one of:

  • max-age >= 10886400
  • includeSubDomains
  • preload

Any other removal conditions will be tricky, but we will need to look at them carefully.

@Hainish
Copy link
Member

Hainish commented Jan 10, 2017

Thanks, @lgarron - I'll add these additional conditions to our internal hsts pruning utility.

@cooperq
Copy link
Contributor

cooperq commented Jan 10, 2017

@Hainish lmk when this is ready for a final review

@Hainish
Copy link
Member

Hainish commented Jan 10, 2017

@lgarron for our hsts pruning utility, we attempt to exempt HTTPS Everywhere target domains from removal if they are likely to be removed from the preload list, to ensure that once you remove these domains they will not be left unprotected by HTTPS Everywhere as well. This means our exemption logic should match up with your removal logic. To this end, I have a few questions:

  1. Will any of the above directives be case sensitive?
  2. Are you going to be checking for automatic redirection from HTTP to HTTPS?

@lgarron
Copy link

lgarron commented Jan 10, 2017

Will any of the above directives be case sensitive?

No, since the spec is not case-sensitive.

Are you going to be checking for automatic redirection from HTTP to HTTPS?

The scans will be checking for it, but I don't know if it's safe to include in the policy yet.
The initial policy is meant to avoid allowing an attacker to drop the site from the preload list unless they can respond with a valid cert from the site, so we probably won't start with it – but in principle I'd like to keep enforcing all requirements.

I recommend the more conservative approach, but talk to me if the numbers look burdensome?

@Hainish
Copy link
Member

Hainish commented Jan 11, 2017

@lgarron in our case, the most conservative approach is to use the most stringent conditions in order to limit the amount of rules removed. We'd like to avoid as much as possible removing target domains that may be removed from the preload list in the future. For this reason we'll implement checking for HTTP redirects as well, in case you implement that in the future.

@lgarron
Copy link

lgarron commented Jan 11, 2017

This means our exemption logic should match up with your removal logic.

Also, if you want to test against the actual requirements checker, https://github.com/chromium/hstspreload should be easy to run if you have go.

@Hainish
Copy link
Member

Hainish commented Jan 12, 2017

To wrap up this discussion, the guidelines on removing HSTS preload targets are formalized in the hsts-prune utility, but to be explicit:

Let included domain denote either a target, or a parent of a target. Let supported browsers include the ESR, Dev, and Stable releases of Firefox, and the Stable release of Chromium. If included domain is a parent of the target, the included domain must be present in the HSTS preload list for all supported browsers with the relevant flag which denotes inclusion of subdomains set to true. If included domain is the target itself, it must be included the HSTS preload list for all supported browsers. Additionally, if the http endpoint of the target exists, it must issue a 3XX redirect to the https endpoint for that target. Additionally, the https endpoint for the target must deliver a Strict-Transport-Security header with the following directives present:

  • max-age >= 10886400
  • includeSubDomains
  • preload

If all the above conditions are met, a contributor may remove the target from the HTTPS Everywhere rulesets. If all targets are removed for a ruleset, the contributor is advised to remove the ruleset file itself. The ruleset rule and test tags may need to be modified in order to pass the ruleset coverage test.

@Hainish
Copy link
Member

Hainish commented Jan 12, 2017

I'll include this in the documentation for CONTRIBUTING.md that I'm working through

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests