-
Notifications
You must be signed in to change notification settings - Fork 241
Description
The HTTP client's redirect behavior currently is pretty bare-bones in that it merely swaps out the URI for the one given in the redirect response's Location
header and possibly changes the request method, depending on the redirect response status. Notably, this means that sensitive headers (specifically Authorization
) are also sent to the redirect destination which can lead to problems (quoting @ferdinand-beyer from Clojurians Slack):
I noticed that Aleph’s HTTP client follows redirects by just replacing the URL and possibly the method. It will however keep all other request options, in particular any credentials from the original request. That means that by default, if one sends credentials to an endpoint that responds with a redirect, Aleph will happily send these credentials along. I’m not saying this behaviour is wrong, but I noticed that while debugging this scenario: GitHub’s Asset download REST endpoint requires authentication using a bearer token, and will respond with a 302 to a CDN URL. There seems to be some load balancing between different CDN providers and my redirect sometimes succeeded (AWS) and sometimes failed with 403 Unauthorized (Azure). Turned out that the Azure CDN choked on the access token from the original GitHub API request.
The pertinent section 15.4 of RFC 9110 states:
Consider removing header fields that were not automatically generated by the implementation (i.e., those present in the request because they were added by the calling context) where there are security implications; this includes but is not limited to Authorization and Cookie.
So we should perhaps change the default behavior to not forward Authorization
and Cookie
headers on redirect. However, there may be legitimate reasons for wanting to forward them anyway, so we should probably introduce an option that makes it possible to opt into the old behavior again. But what about custom sensitive headers which users might not expect to be forwarded, say something like X-Secret-Token
? Perhaps we should provide an option like :strip-headers-on-redirect
which defaults to #{:authorization :cookie}
? Or provide a :redirect-middleware
with a default of (fn [req] (update req :headers dissoc :authorization :cookie)
? That would allow for supporting even more exotic cases.
We should also consider all other cases given in the aforementioned RFC section to see if they apply to Aleph and possibly change behavior.