aio-libs / aio-libs/aiohttp

Security improvement: raise_for_status + ClientError prevent tokens from leaking

Aperta
#8,402 5 commenti 3 reazioni 0 assegnatari Vedi su GitHub
enhancement
Lingua principale
Python
Stelle
16.5k
Fork
2.4k
Merge medio
17h 22m
PR unite (30g)
212

Descrizione

### Describe the bug

When building a client like this:

```python
client = aiohttp.ClientSession(raise_for_status=True)
```

or more generally, when using `raise_for_status`, any call to `client.get`, `client.post`, etc, can throw a `ClientResponseError`. By default this exception will include the server headers: https://github.com/aio-libs/aiohttp/blob/2eccb8b47ff7c77596955071cfb4dbbd5dfe63d5/aiohttp/client_reqrep.py#L1007.

If the request was authenticated (i.e: if the headers contained an `Authorization` field) and if the server sends back the headers, the token will be printed out in the exception.

This scenario just happened to me:
- I have an authenticated client that made a request to an API
- This API returned a 500
- The API returned some server headers, including the `Authorization` field
- An exception was thrown, with the headers printed out fully in clear
- Since my application logs exceptions, I now have an auth token in clear in my logs

This is problematic. I could do this to solve this problem for myself:

```python
client = aiohttp.ClientSession(raise_for_status=my_own_function_that_ignores_headers)
```

But I think good security should be provided by default. I would argue that most requests these days are authenticated and that tokens should never be printed out by default. This could be a very sneaky problem, if some day an API decides to send back some headers with a token it could surprise people.

I see several approaches:
- Add a `use_headers` flag to the `raise_for_status` function, set to True at first (to not break compatibility), and set to False in a few versions
- Add an optional callable to filter which fields of the headers will be used/obfuscated

I am happy to submit a PR if people think it's a good idea.

### To Reproduce

The server needs to send back server errors and the get/post needs to fail.
```python
client = aiohttp.ClientSession(raise_for_status=True)
client.get(url)
```

### Expected behavior

When a `ClientError` is raised, the headers shouldn't be present in the exception OR we should be able to disable them, to not leak tokens.

### Logs/tracebacks

```python-traceback
N/A
```

### Python Version

```console
$ python --version

3.12.1
```

### aiohttp Version

```console
$ python -m pip show aiohttp

3.9.5
```

### multidict Version

```console
$ python -m pip show multidict
6.0.5
```

### yarl Version

```console
$ python -m pip show yarl

1.9.4
```

### OS

macOS

### Related component

Client

### Additional context

_No response_

### Code of Conduct

- [X] I agree to follow the aio-libs Code of Conduct

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.