aio-libs / aio-libs/aiohttp

Security improvement: raise_for_status + ClientError prevent tokens from leaking

Đang mở
#8,402 5 bình luận 3 reaction 0 người được giao Xem trên GitHub
enhancement
Ngôn ngữ chính
Python
Star
16.5k
Fork
2.4k
Merge trung bình
17 giờ 22 phút
Pull request đã merge (30 ngày)
212

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.