compare scheme in redirect same-authority check#848
Conversation
| if (h1 == null || h2 == null) { | ||
| return false; | ||
| } | ||
| if (!h1.getSchemeName().equalsIgnoreCase(h2.getSchemeName())) { |
There was a problem hiding this comment.
A scheme change makes the origins different, but does not by itself require rejecting the redirect.
There was a problem hiding this comment.
agreed, and that's the intent here. the scheme mismatch only marks the origins as different, it doesn't reject the redirect on its own. an https to http hop still goes through unless there's an authorization or cookie header to protect, exactly as it already behaves for a differing host or port. the last assertion in the added test covers the plain https to http redirect staying allowed.
There was a problem hiding this comment.
Fair enough. Still, this changes the boundary from authority to origin and extends the existing rejection policy to scheme changes. That needs a clear rationale
There was a problem hiding this comment.
@dxbjavid Why should scheme even matter, given the strategy takes the port into account? Can there realistically be multiple protocol schemes on the same port?
There was a problem hiding this comment.
in practice you're right that a port is normally bound to a single listener, so the same host and port serving both schemes at once is unusual. the concern isn't that the server offers both though, it's that the redirect names the next hop as cleartext: on https://host:8443 to http://host:8443 the client opens a plaintext connection and writes the authorization and cookie headers before any tls, so credentials tied to the secure origin go out in the clear. host and port alone can't tell us that, the scheme can. that's also the rationale for the origin framing (scheme, host, port per rfc 6454) rather than authority, since forwarding those headers is really an origin decision. it doesn't reject the redirect, only strips the sensitive headers, the same as a differing host or port already does.
|
@dxbjavid I am now even more confused. The strategy does not strip any headers. The comment to that effect was wrong. It merely decides if it OK to proceed with the redirect. It does so, only if the request with sensitive headers has already been sent to the origin server and got redirected to the same very authority that has already seen it with all the headers and what not. I do not understand how the proposed change makes anything safer or better. I do not mind merging your change-set. I just do not understand the rationale. |
2191373 to
f1a961a
Compare
|
You're right, and sorry for muddying it: nothing gets stripped, the strategy just declines to follow the redirect when the target differs and the request carries sensitive headers. My diff had also reintroduced the old comment wording, so I've rebased on master and kept your corrected phrasing. The rationale is about the wire rather than the server. The origin server has indeed already seen those headers, but it saw them under TLS. If the Location downgrades to http on the same host and port, the redirected request is a copy of the original, so Authorization and Cookie get written to a plaintext connection where any on-path observer can read them. Host and port identify the endpoint but say nothing about whether the transport is encrypted, which is what the scheme check adds. It's the same class of issue as curl's CVE-2022-27774, where credentials followed a redirect to the same host on a different protocol. |
|
Cherry-picked to |
3 participants
DefaultRedirectStrategy decides whether to keep the Authorization and Cookie headers on a redirect by comparing the current and new targets, but isSameAuthority only looks at the host name and the resolved port and leaves the scheme out. A response that redirects from https to http on the same host and the same explicit port, say https://host:8443/ to http://host:8443/, is treated as the same origin, so those credentials are retained and end up being sent in the clear. I've added the scheme to that comparison so a cross-scheme downgrade counts as a different origin and the sensitive headers are stripped as they already are for a different host or port.