aio-libs / aio-libs/aiohttp

Security improvement: raise_for_status + ClientError prevent tokens from leaking

Abierto
#8,402 5 comentarios 3 reacciones 0 asignados Ver en GitHub
enhancement
Lenguaje dominante
Python
Estrellas
16.5k
Forks
2.4k
Merge medio
17 h 22 min
PR fusionados (30 d)
212

Descripción

### 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

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.