fix(auth): sanitize HTTP headers and preserve empty responses

Author: leandrodamascena

aws_lambda_powertools/utilities/auth_alpha/_internal/http.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+from io import BytesIO
 from typing import TYPE_CHECKING, Any, cast
 
 import urllib3
@@ -109,7 +110,7 @@ def request(
             )
             content = self._read_body(response, deadline)
             return urllib3.HTTPResponse(
-                body=content,
+                body=BytesIO(content),
                 status=response.status,
                 headers=response.headers,
                 reason=response.reason,

aws_lambda_powertools/utilities/auth_alpha/_internal/transport.py

@@ -8,6 +8,10 @@
 
 from urllib3.connection import HTTPSConnection
 from urllib3.connectionpool import HTTPSConnectionPool
+from urllib3.exceptions import HeaderParsingError
+from urllib3.util.response import assert_header_parsing
+
+from aws_lambda_powertools.utilities.auth_alpha._internal.deadline import RequestError
 
 if TYPE_CHECKING:
     from collections.abc import Iterator
@@ -77,6 +81,15 @@ def __init__(
         # The stream owns the socket reference even for Connection: close.
         self.fp = BufferedReader(_DeadlineReader(self.fp, sock, deadline), buffer_size=8192)
 
+    def begin(self) -> None:
+        super().begin()
+        try:
+            # urllib3 otherwise logs malformed headers, including provider data,
+            # before the public Auth operation can sanitize the failure.
+            assert_header_parsing(self.msg)
+        except (HeaderParsingError, TypeError):
+            raise RequestError() from None
+
 
 class _DeadlineHTTPSConnection(HTTPSConnection):
     response_class = _DeadlineResponse

aws_lambda_powertools/utilities/auth_alpha/oauth2/client.py

@@ -28,6 +28,8 @@
 
 _BEARER_TOKEN = re.compile(r"[-A-Za-z0-9._~+/]+=*")
 _HEADER_NAME = re.compile(r"[-!#$%&'*+.^_`|~0-9A-Za-z]+")
+# HTTP field values allow horizontal tabs, visible ASCII, and extended Latin-1 bytes.
+_HEADER_VALUE = re.compile(r"[\t\x20-\x7e\x80-\xff]*")
 _RESOURCE_CHARACTERS = frozenset(ascii_letters + digits + "-._~:/?[]@!$&'()*+,;=")
 _HEX_DIGITS = frozenset(hexdigits)
 
@@ -265,7 +267,7 @@ def _request_headers(headers: Mapping[str, str] | None) -> dict[str, str]:
                 or not isinstance(value, str)
                 or not _HEADER_NAME.fullmatch(name)
                 or name.lower() == "authorization"
-                or any(character in value for character in ("\r", "\n"))
+                or not _HEADER_VALUE.fullmatch(value)
             ):
                 raise ValueError("Request headers must be valid and must not include Authorization")
         return dict(headers)

docs/utilities/oauth2.md

@@ -106,6 +106,8 @@ The helper requires HTTPS, rejects an existing Authorization header, and never f
 
 Header names must use HTTP token syntax: letters, digits, and the permitted token punctuation. Empty names, whitespace (including trailing spaces or tabs), and delimiters such as colons are rejected before token acquisition. Authorization is rejected regardless of casing.
 
+Header values must fit Latin-1 and cannot contain ASCII control characters other than horizontal tabs. Invalid names and values are rejected before loading the client secret.
+
 !!! warning "Use trusted destination URLs"
     `request()` does not derive or restrict destinations from the configured audience or resource. Supply trusted URLs from application configuration; never pass a caller-controlled destination. A token intended for one API must not be sent to another.
 

tests/functional/auth_alpha/oauth2/test_client.py

@@ -704,11 +704,31 @@ def load_secret():
     assert http.requests == []
 
 
-def test_http_token_punctuation_is_allowed_in_header_names(http):
+@pytest.mark.parametrize("value", ["\x00", "\x01", "\x1f", "\x7f", "東京", "\ud800"])
+def test_invalid_header_values_are_rejected_before_token_acquisition(http, value):
+    secrets = []
+
+    def load_secret():
+        secrets.append("test-secret")
+        return secrets[-1]
+
+    http.serve(TOKEN_URL, {"access_token": "token", "token_type": "Bearer", "expires_in": 100}, method="POST")
+    http.serve("https://api.example.com/orders", {"orders": []})
+    subject = client(client_secret=load_secret)
+
+    with pytest.raises(ValueError, match="Request headers"):
+        subject.request("GET", "https://api.example.com/orders", headers={"X-Trace": f"trace{value}value"})
+
+    assert secrets == []
+    assert http.requests == []
+
+
+@pytest.mark.parametrize("value", ["", "trace-id", "trace\tvalue", "\x80", "\xff", "caf\xe9"])
+def test_valid_header_names_and_values_are_preserved(http, value):
     http.serve(TOKEN_URL, {"access_token": "token", "token_type": "Bearer", "expires_in": 100}, method="POST")
     http.serve("https://api.example.com/orders", {"orders": []})
     subject = client()
-    headers = {"X-Trace!#$%&'*+.^_`|~09": "trace-id"}
+    headers = {"X-Trace!#$%&'*+.^_`|~09": value}
 
     response = subject.request("GET", "https://api.example.com/orders", headers=headers)
 

tests/integration/auth_alpha/conftest.py

@@ -156,6 +156,9 @@ def do_GET(self):  # noqa: N802
         def do_POST(self):  # noqa: N802
             self.respond()
 
+        def do_HEAD(self):  # noqa: N802
+            self.respond()
+
         def respond(self):
             body = self.rfile.read(int(self.headers.get("Content-Length", 0)))
             endpoint.requests.append((self.command, self.path, dict(self.headers), body))
@@ -180,7 +183,8 @@ def respond(self):
                     ):
                         return
                 self.end_headers()
-                _write_body(self.wfile, reply, endpoint.stop)
+                if self.command != "HEAD":
+                    _write_body(self.wfile, reply, endpoint.stop)
             except (OSError, ssl.SSLError):
                 # Timeout and oversized-body tests deliberately close early.
                 pass

tests/integration/auth_alpha/jwt/test_https.py

@@ -1,4 +1,5 @@
 import time
+import traceback
 
 import jwt
 import pytest
@@ -70,3 +71,19 @@ def test_key_endpoint_failures_are_bounded_and_do_not_follow_redirects(https_ser
     assert time.monotonic() - started < 1
     assert error.value.__context__ is None
     assert [request[1] for request in https_server.requests] == ["/keys"]
+
+
+def test_malformed_jwks_headers_fail_without_logging_provider_data(https_server, caplog):
+    private_data = "local-test-private-provider-data"
+    https_server.serve("/keys", {"keys": []}, headers={"Broken header": private_data})
+    subject = verifier(https_server, jwks_uri=https_server.url + "/keys")
+
+    with pytest.raises(JWKSFetchError) as error:
+        subject.prefetch()
+
+    assert error.value.__context__ is None
+    assert error.value.__cause__ is None
+    assert private_data not in "".join(traceback.format_exception(error.value))
+    assert private_data not in caplog.text
+    assert not [record for record in caplog.records if record.name == "urllib3.connection"]
+    assert [request[1] for request in https_server.requests] == ["/keys"]

tests/integration/auth_alpha/oauth2/test_https.py

@@ -1,6 +1,7 @@
 import base64
 import threading
 import time
+import traceback
 from concurrent.futures import ThreadPoolExecutor
 from urllib.parse import parse_qs
 
@@ -223,3 +224,58 @@ def test_downstream_gzip_response_remains_readable(https_server, chunked):
 
     response = subject.request("GET", https_server.url + "/inventory")
     assert response.json() == {"items": [123]}
+
+
+@pytest.mark.parametrize(("method", "status"), [("GET", 200), ("GET", 204), ("GET", 304), ("HEAD", 200)])
+def test_empty_downstream_responses_preserve_bytes_and_status(https_server, method, status):
+    https_server.serve("/token", TOKEN_RESPONSE)
+    https_server.serve("/inventory", b"not sent for HEAD" if method == "HEAD" else b"", status=status)
+    subject = client(https_server)
+
+    response = subject.request(method, https_server.url + "/inventory")
+
+    assert response.status == status
+    assert response.data == b""
+    assert response.data.decode() == ""
+    assert [request[1] for request in https_server.requests] == ["/token", "/inventory"]
+
+
+@pytest.mark.parametrize("endpoint", ["token", "inventory"])
+def test_malformed_response_headers_fail_without_logging_credentials(https_server, caplog, endpoint):
+    private_data = "Bearer local-test-private-token"
+    https_server.serve("/token", TOKEN_RESPONSE)
+    https_server.serve("/inventory", {"items": [123]})
+    payload = TOKEN_RESPONSE if endpoint == "token" else {"items": [123]}
+    https_server.serve(f"/{endpoint}", payload, headers={"Broken header": private_data})
+    subject = client(https_server)
+    expected = TokenExchangeError if endpoint == "token" else DownstreamRequestError
+
+    with pytest.raises(expected) as error:
+        subject.request("GET", https_server.url + "/inventory")
+
+    assert not error.value.retryable
+    assert error.value.__context__ is None
+    assert error.value.__cause__ is None
+    assert private_data not in "".join(traceback.format_exception(error.value))
+    assert private_data not in caplog.text
+    assert not [record for record in caplog.records if record.name == "urllib3.connection"]
+    expected_paths = ["/token"] if endpoint == "token" else ["/token", "/inventory"]
+    assert [request[1] for request in https_server.requests] == expected_paths
+
+    https_server.serve(f"/{endpoint}", payload)
+    assert subject.request("GET", https_server.url + "/inventory").json() == {"items": [123]}
+
+
+def test_valid_extended_header_values_are_sent_over_tls(https_server):
+    https_server.serve("/token", TOKEN_RESPONSE)
+    https_server.serve("/inventory", {"items": [123]})
+    value = "caf\xe9\t\x80\xff"
+
+    response = client(https_server).request(
+        "GET",
+        https_server.url + "/inventory",
+        headers={"X-Trace": value},
+    )
+
+    assert response.status == 200
+    assert https_server.requests[-1][2]["X-Trace"] == value