Security improvement: raise_for_status + ClientError prevent tokens from leaking
- Dominant language
- Python
- Stars
- 16.5k
- Forks
- 2.4k
- Avg merge
- 17h 22m
- Merged PRs (30d)
- 212
Description
### 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
Contributor guide
Assessment
This issue has not been assessed yet.