Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider raising maxRedirects to 20 to match Chrome's behavior #109

Open
nharper opened this issue Dec 3, 2018 · 9 comments
Open

Consider raising maxRedirects to 20 to match Chrome's behavior #109

nharper opened this issue Dec 3, 2018 · 9 comments

Comments

@nharper
Copy link
Collaborator

nharper commented Dec 3, 2018

https://cs.chromium.org/chromium/src/net/url_request/url_request.h?sq=package:chromium&dr=C&g=0&l=137 sets the redirect limit in chrome, based on the Fetch spec.

@nharper
Copy link
Collaborator Author

nharper commented Dec 3, 2018

@lgarron do you remember why 3 was used as maxRedirects in redirects.go?

@lgarron
Copy link
Collaborator

lgarron commented Dec 3, 2018

@lgarron do you remember why 3 was used as maxRedirects in redirects.go?

It was a fairly arbitrary choice. It seemed fine to pick a low initial value and keep it if it didn't cause problems in practice.

Some sites have had to reduce redirects to meet the requirement, but I think most of them don't have a big problem doing so. More redirects are longer to test and more brittle (potentially causing the preloadability of a site to depend on multiple other sites), so I'd prefer to keep it fairly low. But I don't feel very strongly.

@nharper
Copy link
Collaborator Author

nharper commented Jun 5, 2020

Once case where the redirect limit of 3 is causing an issue is with a domain that redirects to an identity provider, and that IdP takes more than 2 redirects to get to a login page. It seems like their site is behaving reasonably (there aren't any extra redirects happening on their end) and the limit should be raised to allow that.

I could raise the limit to something between 3 and 20, though if changing the limit I'm most inclined to go with 20 for consistency. The check could also have a limit per domain or eTLD+1, but at that point it seems like I'm making up arbitrary rules.

@grobie
Copy link

grobie commented Jul 15, 2020

I don't fully understand why the hstspreload bot follows all redirects at all? Like given the following redirect chain: http://mydomain.com -> https://mydomain.com (includes HSTS header) -> https://accounts.google.com/.... -> (another 3 redirects imposed by Google), shouldn't the preload checker stop on the 2nd response, given the HSTS header is present and delivered over TLS? If all redirects were ignored after the HSTS header has been delivered, the arbitrarily imposed redirect limit of 3 wouldn't even matter that much.

@lgarron @nharper any chance to resolve this so that more sites can be submitted to the HSTS preload list?

@lgarron
Copy link
Collaborator

lgarron commented Jul 15, 2020

@lgarron @nharper any chance to resolve this so that more sites can be submitted to the HSTS preload list?

Given the growth of the list, I would say that sites that want proper security aren't facing significant issues meeting this requirement.

If the redirect chain includes an insecure page then preloading doesn't really give a benefit against a meddler in the middle, so it seems like a valuable requirement for the benefit of the end user.

(Although I still think having a redirect limit over 3 would be okay, even if the chain would be more brittle and slower for the user.)

@grobie
Copy link

grobie commented Jul 15, 2020

If the redirect chain includes an insecure page then preloading doesn't really give a benefit against a meddler in the middle, so it seems like a valuable requirement for the benefit of the end user.

While I don't have intentions to argue in favor sites not using TLS, I don't see how the this project should have any authority in adding requirements not defined in the HSTS spec. The RFC already lists examples where proper redirects can't be configured in real-world examples. If someone has a legitimate need to redirect from https://hstspreloadcandidate.com to http://www.somethingelse.com why should this project deny hstspreloadcandidate.com to be added to the hsts preload list?

(Although I still think having a redirect limit over 3 would be okay, even if the chain would be more brittle and slower for the user.)

The redirect chain experienced by the bot without any cookies set can be completely different to a real user opening the same page every day. Long redirect chains for unauthenticated users are not necessarily representing the general user experience. And again I don't see how this project should have any say on that. AFAIK there are no requirements defined by the HSTS spec in regards to the number of redirects.

I'd love to see this project verifying the requirements defined by RFC 6797, and maybe even making helpful suggestions how to increase user experience and security even further, without preventing harder to imagine edge use cases from getting submitted to the list.

# real-world example using Google's OAuth implementation.
$ curl -Ls -D - -o /dev/null http://<redacted>.org | grep -i location | cut -d\? -f1
Location: https://<redacted>.org/
location: https://accounts.google.com/o/oauth2/v2/auth
location: https://accounts.google.com/signin/oauth
location: https://accounts.google.com/AccountChooser
location: https://accounts.google.com/ServiceLogin

Although I still think having a redirect limit over 3 would be okay

Thank you. What do you think would be the next steps? Are there any requirements who can make this change? Would you merge a pull request if someone were to make changes? Do you have any requirements for the changes?

@lgarron
Copy link
Collaborator

lgarron commented Jul 15, 2020

While I don't have intentions to argue in favor sites not using TLS, I don't see how the this project should have any authority in adding requirements not defined in the HSTS spec.

This is a site/service maintained by Chromium project. It may be appropriate to standardize some parts or open up governance in the future (both of which I tried to move towards when I was active maintainer of the HSTS preload list), but fundamentally this is just a form for sites to ask to be included in the Chromium list, which the Chromium project has full prerogative over.

The redirect chain experienced by the bot without any cookies set can be completely different to a real user opening the same page every day. Long redirect chains for unauthenticated users are not necessarily representing the general user experience.

I would love it if there were a general solution here, but the scanner can only do so much.

And again I don't see how this project should have any say on that.

I suppose we'll have to disagree on that part. Chromium is a user agent, and it is the prerogative of the user agent to prioritize secure end user browsing over simplicity for developers. (This also matches the priority of constituencies, which might apply if preloading were standardized more.)

Thank you. What do you think would be the next steps? Are there any requirements who can make this change? Would you merge a pull request if someone were to make changes? Do you have any requirements for the changes?

At a technical level, this is as simple as changing a few lines of code. @nharper should be in charge of a final decision about this.

@nharper
Copy link
Collaborator Author

nharper commented Jul 15, 2020

If someone has a legitimate need to redirect from https://hstspreloadcandidate.com to http://www.somethingelse.com why should this project deny hstspreloadcandidate.com to be added to the hsts preload list?

The benefit of HSTS is to only use a secure transport, to prevent a monster in the middle from intercepting insecure requests/responses. If a domain that uses HSTS always redirects to plaintext http, then the HSTS policy on the first domain provides the user no benefit: the second request, which goes over http, can be MitM'd instead.

My plan is to raise the limit to 20 (to match the Fetch spec and browser behavior). The actual change should be fairly simple. However, tests in this repo are currently broken due to issue #112, and I'd like to fix that issue before making this change.

@grobie
Copy link

grobie commented Jul 23, 2020

First of all, let me thank you both @lgarron and @nharper for your work on this project and to make the web more secure. I generally applaud the Chromium project's push for TLS and HSTS and even its higher requirements for websites than defined in the base HSTS specification. This list and submission requirements have become the baseline for all browsers and by that for all websites, but I admit this doesn't change anything about the ownership of this submission criteria. Thank you for that reminder @lgarron.

I would love it if there were a general solution here, but the scanner can only do so much.

I guess my point is that given the scanner can't fully reflect real browser behavior, it shouldn't attempt to enforce rules which don't necessarily impact actual users.

If a domain that uses HSTS always redirects to plaintext http, then the HSTS policy on the first domain provides the user no benefit: the second request, which goes over http, can be MitM'd instead.

There is the benefit for the user that all requests to any subdomain of hstspreloadcandidate.com are now encrypted, even if the other domain is still vulnerable. Like setups in large organizations with many different products in varying stages of supporting TLS. Anyway, I admit it is a fairly contrived example. The only actually issue I have with this scanner is that it prohibits real-world use-cases (ironically imposed by Google's own products) using fully encrypted redirect chains for no reason.


@nharper Do you have any rough ETA for that work? I'm just afraid this restriction will exist for another 2 years. Alternatively, maybe you could detail the necessary work and your expected changes in #112 so others could help out?

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

No branches or pull requests

3 participants