Strip auth headers on cross-origin redirects#36
Conversation
oalders
commented
May 14, 2026
oalders
commented
- refuse https to http redirects by default
- strip auth headers on cross-origin redirects
- Fix protocol-relative Location handling so it can't be used to bypass credential strip
- demonstrate that https upgrade now strips credentials
Allow opt in via allow_downgrade
so it can't be used to bypass credential strip
as it is a change of origin
|
This changes defaults, so I think it merits a very close look and possibly some arguments. |
| allow_downgrade cookie_jar default_headers http_proxy https_proxy | ||
| keep_alive local_address max_redirect max_size proxy no_proxy | ||
| SSL_options verify_SSL | ||
| allow_credentialed_redirects allow_downgrade cookie_jar default_headers |
There was a problem hiding this comment.
allow_credential_redirects is a footgun.
Some people will enable it because it seems to make their lives easier (or maybe they were frobnicating and left it enabled), and it will come back to hurt them badly.
Instead, what you want is to look at the CORS headers from the server, notably https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Access-Control-Allow-Credentials will say whether cookies and authentication headers can be shared.
I'd leave out the allow_credential_redirects and take the time to implement CORS handling in a later change.
There was a problem hiding this comment.
The CORS header doesn't really apply to this.
With an Access-Control-Allow-Credendials: true, the credentials (basic auth, cookies) for the new host should be included in the cross origin request. This requires having a cookie jar, and tracking per-host authorization. LWP does this, but HTTP::Tiny does not.
The behavior with this option just carries the manually set authorization headers through to the redirected requests. No attempt at per-host handling is done. This matches how curl's CURLOPT_UNRESTRICTED_AUTH option works.
haarg
left a comment
haarg
left a comment
There was a problem hiding this comment.
The tests should also cover redirects from requests providing basic auth via the URL (https://user:pass@host/) rather than a manually set Authorization header, with and without the allow_credentialed_redirects option.
| allow_downgrade cookie_jar default_headers http_proxy https_proxy | ||
| keep_alive local_address max_redirect max_size proxy no_proxy | ||
| SSL_options verify_SSL | ||
| allow_credentialed_redirects allow_downgrade cookie_jar default_headers |
There was a problem hiding this comment.
The CORS header doesn't really apply to this.
With an Access-Control-Allow-Credendials: true, the credentials (basic auth, cookies) for the new host should be included in the cross origin request. This requires having a cookie jar, and tracking per-host authorization. LWP does this, but HTTP::Tiny does not.
The behavior with this option just carries the manually set authorization headers through to the redirected requests. No attempt at per-host handling is done. This matches how curl's CURLOPT_UNRESTRICTED_AUTH option works.
…he URL rather than a manually set Authorization header, with and without the allow_credentialed_redirects option.
|
I've added new tests. |
|
Anything left to do here? |
|
Thinking about this a bit more, should we be including any of the original headers when performing a redirect? |
|
https://www.rfc-editor.org/rfc/rfc9110.html#section-15.4-6.3.1
See also https://curl.se/docs/CVE-2018-1000007.html
We could also consider allowing the caller to specify a list of additional headers to strip. |
|
I don't think we need to make the problem more complex. If someone needs non-standard behavior for redirects, they can disable the built in redirect handling and do it themselves. HTTP::Tiny does not need to handle every case built in. |
3 participants
