[ClientSession behavior] Authorization header dropped during HTTP redirect

[tonywu7]

🐞 Describe the behavior

When using aiohttp.ClientSession to make a GET request that carries an Authorization header, the header is silently dropped whenever the remote server responds with HTTP 3xx redirections to the initial request.

📋 Versions

python 3.8.1 aiohttp 3.7.4.post0 multidict 4.7.6 yarl 1.5.1

💡 To Reproduce

Script to reproduce the behavior using the Discord HTTP API and a non-sensitive OAuth2 access token (note that the access tokens expires on Jun 14 2021, although the behavior should still be observable through Response.request_info.headers):

import asyncio, logging, sys
import aiohttp, multidict, yarl

# Non-sensitive, expires on Jun 14 2021
OAUTH2_ACCESS_TOKEN = 'UHwmFVVCnXiywoXp4WMAauaPLkIDzx'
log = logging.getLogger('debug')

async def make_request(url: str):
    async with aiohttp.ClientSession() as session:
        async with session.get(url, headers={
            'Authorization': f'Bearer {OAUTH2_ACCESS_TOKEN}',  # Set Authorization header
        }) as res:
            log.info(f'Requested URL: {url}')
            log.info(f'Real URL: {res.real_url}')
            log.info(f'Request headers: { {**res.request_info.headers} }')
            log.info(f'Status: {res.status}')

async def main(no_redirect: str, redirected: str):
    logging.basicConfig(
        level=logging.DEBUG, format='%(levelname)-8s [%(name)s] %(message)s',
    )
    log.info(f'aiohttp {aiohttp.__version__}')
    log.info(f'multidict {multidict.__version__}')
    log.info(f'yarl {yarl.__version__}')
    await make_request(no_redirect)
    await asyncio.sleep(1)
    await make_request(redirected)

if __name__ == '__main__':
    asyncio.run(main(sys.argv[1], sys.argv[2]))

Save as redirect.py and run

python3 redirect.py https://discord.com/api/users/@me http://discord.com/api/users/@me

Output:

INFO     [debug] aiohttp 3.7.4.post0
INFO     [debug] multidict 4.7.6
INFO     [debug] yarl 1.5.1
INFO     [debug] Requested URL: https://discord.com/api/users/@me
INFO     [debug] Real URL: https://discord.com/api/users/@me
INFO     [debug] Request headers: {'Host': 'discord.com', 'Authorization': 'Bearer UHwmFVVCnXiywoXp4WMAauaPLkIDzx', 'Accept': '*/*', 'Accept-Encoding': 'gzip, deflate', 'User-Agent': 'Python/3.8 aiohttp/3.7.4.post0'}
INFO     [debug] Status: 200
INFO     [debug] Requested URL: http://discord.com/api/users/@me
INFO     [debug] Real URL: https://discord.com/api/users/@me
INFO     [debug] Request headers: {'Host': 'discord.com', 'Accept': '*/*', 'Accept-Encoding': 'gzip, deflate', 'User-Agent': 'Python/3.8 aiohttp/3.7.4.post0'}
INFO     [debug] Status: 401

💡 Expected behavior

Provided that the authorization info is valid, I expect that the remote server not return an HTTP 401 Unauthorized response due to aiohttp dropping the auth header.

📋 Additional context

This was raised a year ago in #4568.

These are the specific lines causing this behavior for 3.7.4.post0:

aiohttp/aiohttp/client.py

Lines 610 to 612 in 184274d

	if url.origin() != parsed_url.origin():
	auth = None
	headers.pop(hdrs.AUTHORIZATION, None)

For the test script, the first URL is HTTPS, for which Discord returns HTTP 200 directly, but for the second, HTTP version of the API, Discord issues an HTTP 301 first to redirect the client to the HTTPS version. aiohttp then proceeds to drop the Authorization header because it is being redirected to a different origin (from http://discord.com to https://discord.com).

This seems to be done for security considerations, to prevent sending the authorization info to a different location. Notably this is also the behavior for the requests library (see this SO and this PR on requests).

However, for aiohttp, this is undocumented and is therefore surprising for developers, especially in this scenario when the remote server would like to simply upgrade from HTTP to HTTPS but stay on the same host. It is also surprising because it is different than the behaviors seen on major browsers (even though the goal of aiohttp is not to emulate browser behaviors). For browsers, the Fetch API spec decides that the same request info including non-forbidden headers should be reused for all redirections (see issue 553 at whatwg/fetch).

---

For aiohttp, my opinion is that it seems less surprising to at least keep the header when the redirect is to the same host/authority as in the previous request, and is done to upgrade from HTTP to HTTPS (http://discord.com to https://discord.com), instead of dropping it whenever the origin changes (emulating curl, see below):

if (
    url.authority() != parsed_url.authority()
    or url.scheme == 'https' and parsed_url.scheme == 'http'  # downgrade
):
    auth = None
    headers.pop(hdrs.AUTHORIZATION, None)

and to perhaps provide some warning when it happens, or to document it somewhere.

---

I set up a test server at https://aiohttp-issue4568.tonywu.org/ that unconditionally redirects any request to https://discord.com/api/users/@me. It is setup to respond correctly to CORS preflights.

On Chrome and Firefox, open the console while at https://discord.com, then run the following:

await fetch('https://aiohttp-issue4568.tonywu.org/', {mode: 'cors', headers: {'Authorization': 'Bearer UHwmFVVCnXiywoXp4WMAauaPLkIDzx'}})

The fetch will succeed without receiving an HTTP 401, meaning the Authorization header survived through redirections.

Safari on the other hand has the same behavior and drops the Authorization header.

curl seems to be more specific: it seems to throw out the header when it is redirected to a different host, but keep it when it is upgrading on the same host:

$ curl -V
curl 7.77.0 (x86_64-apple-darwin20.4.0) libcurl/7.77.0 ...

$ curl -Lv 'http://discord.com/api/users/@me' -H 'Authorization: Bearer UHwmFVVCnXiywoXp4WMAauaPLkIDzx'
...
< HTTP/1.1 301 Moved Permanently
...
* Issue another request to this URL: 'https://discord.com/api/users/@me'
...
> GET /api/users/@me HTTP/2
> Host: discord.com
> user-agent: curl/7.77.0
> accept: */*
> authorization: Bearer UHwmFVVCnXiywoXp4WMAauaPLkIDzx
>
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Connection state changed (MAX_CONCURRENT_STREAMS == 256)!
< HTTP/2 200

$ curl -Lv 'https://aiohttp-issue4568.tonywu.org/' -H 'Authorization: Bearer UHwmFVVCnXiywoXp4WMAauaPLkIDzx'
...
< HTTP/1.1 301 Moved Permanently
...
* Issue another request to this URL: 'https://discord.com/api/users/@me'
...
> GET /api/users/@me HTTP/2
> Host: discord.com
> user-agent: curl/7.77.0
> accept: */*
>
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Connection state changed (MAX_CONCURRENT_STREAMS == 256)!
< HTTP/2 401

[webknjaz]

Thanks for the detailed write-up! This indeed sounds like it was designed with security in mind. We'll need to see what the RFCs say about the scenario you described before making any decisions on changing anything.
I guess the obvious action item is to update the docs and the workaround would be not following the redirect automatically but doing it on the app side rather than in the framework.
If we decide to change the logic of forwarding the auth, this would be a breaking change, meaning that it could only go into aiohttp v4 and won't be backported to the 3.x stream.

[greshilov]

It was added in PR #2328 as a fix for the #1699. #1699 is about two different hosts. Moreover, Andrew added a test for this case:

aiohttp/tests/test_client_functional.py

Lines 2336 to 2345 in 09ac1cb

	async def test_drop_auth_on_redirect_to_other_host(aiohttp_server: Any) -> None:
	async def srv1(request):
	assert request.host == "host1.com"
	assert request.headers["Authorization"] == "Basic dXNlcjpwYXNz"
	raise web.HTTPFound("http://host2.com/path2")
	async def srv2(request):
	assert request.host == "host2.com"
	assert "Authorization" not in request.headers
	return web.Response()

Seems like it was designed with the different hosts in mind, and http->https redirect is just a corner case that wasn't considered at the time, but I might be wrong.

[webknjaz]

Fair enough. PRs with a better regression test (per https://pganssle-talks.github.io/xfail-lightning/) followed by the fix is welcome.

[betolink]

There are some valid cases where a user gets redirected to a different host for OAuth. I think it would be very useful if aiohttp could at least make this behavior optional.

[webknjaz]

In that case, disable automatic redirection, copy whatever headers you need, and make a new request. It's just a few lines of code. I think it's wrong for us to bake in such potential for a security vulnerability in the core of the framework.

[HMaker]

I had this issue where the server redirected HTTP to HTTPS, aiohttp dropped the Authorization header.

[Dreamsorcerer]

I suspect that allowing HTTP -> HTTPS redirect would be OK, if a simple change. But, it obviously makes sense to start your request with HTTPS anyway.

[webknjaz]

Send the same header to a different host? That doesn't sound safe or reasonable at all. aiohttp wouldn't know of the header semantics of the second server. Plus, the security point still stands. What does RFC say? Why can't the users redirect manually, after carefully examining the previous response and their app context?

[webknjaz]

So requests ended up doing the same we do (allow same-host http->https), but that not what the browsers do. And it's not what the RFC says clients should do: psf/requests#4716 (comment).

urllib3 follows the RFC: urllib3/urllib3#1346

So the PR that resolved this issue was likely incorrect to begin with.

The clients that implement this correctly, use a realm check that takes into account all of schema, hostname and port as the RFC says that the header should be dropped if the canonical root URI is different. And I understand why — a different port means that a different web server program may end up receiving the request.

P.S. Note that cURL is not always RFC-compliant because it's low-level, and the original example may mean that it adds all the user-requested headers to all the requests w/o analyzing them.

@Dreamsorcerer it appears this behavior should be hidden behind an insecure_headers_in_redirect flag or something like that. WDYT?

[Dreamsorcerer]

Send the same header to a different host?

Oh, I thought they were talking about same-host (forgot that this is what we already do).

@Dreamsorcerer it appears this behavior should be hidden behind an insecure_headers_in_redirect flag or something like that. WDYT?

Could do, though I'm not too worried when it's http -> https with the same host. The urllib3 one you linked has a configurable list of headers to drop, so unsafe behaviour can be done by setting it to an empty list.