Swap every header that carries a placeholder

deploy/proxy/swap.py

@@ -172,31 +172,46 @@ async def requestheaders(self, flow: http.HTTPFlow) -> None:
             if not is_reachable(await self.resolve(flow.request.host)):
                 flow.response = http.Response.make(403, b"destination refused\n")
         else:
-            for name, value in flow.request.headers.items():
-                if placeholder := placeholder_in(value):
-                    await self.swap(flow, name, placeholder)
-                    break
+            found = [
+                (name, placeholder)
+                for name, value in flow.request.headers.items(multi=True)
+                if (placeholder := placeholder_in(value))
+            ]
+            if found:
+                await self.swap(flow, found)
         flow.request.stream = not flow.response
 
-    async def swap(self, flow: http.HTTPFlow, name: str, placeholder: str) -> None:
+    async def swap(self, flow: http.HTTPFlow, found: list[tuple[str, str]]) -> None:
+        """Every placeholder is authorized first. Then all are replaced at once,
+        so a refusal leaves the request untouched and unsent. Header names
+        compare without case, as mitmproxy keeps them."""
         # The authority the client sends must match the CONNECT host.
         approved = flow.server_conn.sni or flow.request.host
         authority = urllib.parse.urlsplit(f"//{flow.request.host_header or ''}").hostname
         if authority != approved.lower():
             flow.response = http.Response.make(403, b"host does not match the connection\n")
             return
-        try:
-            header, credential = await self.exchange.authorize(placeholder, approved)
-        except Refused:
-            logger.info("refused a placeholder for %s", approved)
-            flow.response = http.Response.make(403, b"placeholder refused\n")
-            return
-        except ExchangeUnavailable as exc:
-            logger.warning("the exchange gave no answer for %s: %s", approved, exc)
-            flow.response = http.Response.make(503, b"exchange unavailable\n")
-            return
-        del flow.request.headers[name]
-        flow.request.headers[header] = credential
+        credentials: dict[str, tuple[str, str]] = {}
+        for _, placeholder in found:
+            try:
+                header, credential = await self.exchange.authorize(placeholder, approved)
+            except Refused:
+                logger.info("refused a placeholder for %s", approved)
+                flow.response = http.Response.make(403, b"placeholder refused\n")
+                return
+            except ExchangeUnavailable as exc:
+                logger.warning("the exchange gave no answer for %s: %s", approved, exc)
+                flow.response = http.Response.make(503, b"exchange unavailable\n")
+                return
+            if header.lower() in credentials:
+                logger.info("two placeholders for one header on %s", approved)
+                flow.response = http.Response.make(403, b"placeholders conflict\n")
+                return
+            credentials[header.lower()] = (header, credential)
+        for name in {name.lower() for name, _ in found}:
+            del flow.request.headers[name]
+        for header, credential in credentials.values():
+            flow.request.headers[header] = credential
 
     def responseheaders(self, flow: http.HTTPFlow) -> None:
         flow.response.stream = True

docs/architecture.md

@@ -144,7 +144,8 @@ It terminates TLS only for the hosts the exchange lists at `/upstreams`, the
 hosts with a registered secret, and tunnels every other host blind. For a
 request with a placeholder it asks the exchange at `/authorize`, with the
 placeholder and the destination host, for the header the upstream reads and
-the real credential. It swaps that one header and streams the request on.
+the real credential. It swaps every header that carries a placeholder and
+streams the request on. One refusal refuses the whole request.
 A destination that resolves to a loopback, private, link-local, or metadata
 address is refused, so a sandbox cannot reach the exchange or the API through
 the proxy. Every connection goes to the address the proxy checked, and the

docs/security.md

@@ -97,11 +97,12 @@ database and never returned by the API.
 
 The placeholder, `drk.<host id>.<service>.<random>`, works only at the secrets
 proxy, and only for the host and the service it names. The entry stores a
-fingerprint of it, so a database read cannot replay it. The proxy swaps that
-one header, on HTTPS to a registered host, and touches nothing else. Plain
-HTTP is forwarded unchanged. The proxy refuses a loopback, private, link-local,
-or metadata destination, so a box cannot reach the exchange or the API through
-it. It logs no credential.
+fingerprint of it, so a database read cannot replay it. The proxy swaps every
+header that carries a placeholder, on HTTPS to a registered host, and touches
+nothing else. One placeholder it cannot resolve refuses the whole request.
+Plain HTTP is forwarded unchanged. The proxy refuses a loopback, private,
+link-local, or metadata destination, so a box cannot reach the exchange or the
+API through it. It logs no credential.
 
 The real value is encrypted in the database. It passes through the exchange
 and the proxy for one request, and the exchange keeps an issuer's value in

src/secrets_exchange/tests/test_swap.py

@@ -21,6 +21,38 @@
 PLACEHOLDER = "drk.0123abcd.anthropic.s3cret"
 
 
+class Headers:
+    """mitmproxy's Headers, as far as the addon uses them: names compare
+    without case, a repeated name keeps every field, and ``items()`` folds
+    the fields of one name into one value."""
+
+    def __init__(self, *fields: tuple[str, str]) -> None:
+        self.fields = list(fields)
+
+    def items(self, multi: bool = False) -> list[tuple[str, str]]:
+        if multi:
+            return list(self.fields)
+        names: dict[str, str] = {}
+        values: dict[str, list[str]] = {}
+        for name, value in self.fields:
+            names.setdefault(name.lower(), name)
+            values.setdefault(name.lower(), []).append(value)
+        return [(name, ", ".join(values[key])) for key, name in names.items()]
+
+    def __delitem__(self, name: str) -> None:
+        kept = [field for field in self.fields if field[0].lower() != name.lower()]
+        if len(kept) == len(self.fields):
+            raise KeyError(name)
+        self.fields = kept
+
+    def __setitem__(self, name: str, value: str) -> None:
+        self.fields = [field for field in self.fields if field[0].lower() != name.lower()]
+        self.fields.append((name, value))
+
+    def __eq__(self, other: object) -> bool:
+        return dict(self.items()) == other
+
+
 class FakeRequest:
     def __init__(self, url: str, headers: dict[str, str], request_timeout: float) -> None:
         self.url = url
@@ -126,7 +158,7 @@ def _flow(scheme: str, host: str, headers: dict[str, str], connect_host: str = "
             host="104.18.0.1" if scheme == "https" else host,
             host_header=f"{host}:443" if scheme == "https" else host,
             port=443,
-            headers=headers,
+            headers=Headers(*headers.items()),
             stream=False,
         ),
         server_conn=SimpleNamespace(sni=connect_host if scheme == "https" else None),
@@ -352,6 +384,188 @@ async def test_a_basic_placeholder_is_swapped_the_same_way(swap: ModuleType) ->
     assert flow.request.headers == {"Authorization": basic}
 
 
+OTHER = PLACEHOLDER.replace(".anthropic.", ".acme.")
+THIRD = PLACEHOLDER.replace(".anthropic.", ".third.")
+
+
+@pytest.mark.parametrize("reverse", [False, True])
+async def test_every_header_with_a_placeholder_is_swapped(swap: ModuleType, reverse: bool) -> None:
+    exchange = FakeExchange(
+        swap,
+        {"api.acme.test"},
+        {
+            (PLACEHOLDER, "api.acme.test"): ("Authorization", "Bearer real-one"),
+            (OTHER, "api.acme.test"): ("X-Acme-Key", "real-two"),
+        },
+    )
+    addon = _swap(swap, exchange, "104.18.0.1")
+    headers = {"Authorization": f"Bearer {PLACEHOLDER}", "X-Acme-Key": OTHER, "Accept": "*/*"}
+    if reverse:
+        headers = dict(reversed(headers.items()))
+    flow = _flow("https", "api.acme.test", headers)
+
+    await addon.requestheaders(flow)
+
+    assert flow.request.headers == {
+        "Authorization": "Bearer real-one",
+        "X-Acme-Key": "real-two",
+        "Accept": "*/*",
+    }
+    assert sorted(exchange.asked) == sorted(
+        [(PLACEHOLDER, "api.acme.test"), (OTHER, "api.acme.test")]
+    )
+    assert not flow.response
+
+
+async def test_a_credential_is_never_scanned_as_a_placeholder(swap: ModuleType) -> None:
+    exchange = FakeExchange(
+        swap,
+        {"api.acme.test"},
+        {(PLACEHOLDER, "api.acme.test"): ("Authorization", f"Bearer {OTHER}")},
+    )
+    addon = _swap(swap, exchange, "104.18.0.1")
+    flow = _flow("https", "api.acme.test", {"Authorization": f"Bearer {PLACEHOLDER}"})
+
+    await addon.requestheaders(flow)
+
+    assert flow.request.headers == {"Authorization": f"Bearer {OTHER}"}
+    assert exchange.asked == [(PLACEHOLDER, "api.acme.test")]
+
+
+async def test_two_placeholders_for_one_header_refuse_the_request(swap: ModuleType) -> None:
+    exchange = FakeExchange(
+        swap,
+        {"api.acme.test"},
+        {
+            (PLACEHOLDER, "api.acme.test"): ("Authorization", "Bearer real-one"),
+            (OTHER, "api.acme.test"): ("Authorization", "Bearer real-two"),
+        },
+    )
+    addon = _swap(swap, exchange, "104.18.0.1")
+    headers = {"Authorization": f"Bearer {PLACEHOLDER}", "X-Acme-Key": OTHER}
+    flow = _flow("https", "api.acme.test", headers)
+
+    await addon.requestheaders(flow)
+
+    assert flow.response.status_code == 403
+    assert flow.request.headers == headers
+    assert flow.request.stream is False
+
+
+async def test_a_refused_second_placeholder_leaves_the_request_untouched(
+    swap: ModuleType,
+) -> None:
+    exchange = FakeExchange(
+        swap,
+        {"api.acme.test"},
+        {(PLACEHOLDER, "api.acme.test"): ("Authorization", "Bearer real-one")},
+    )
+    addon = _swap(swap, exchange, "104.18.0.1")
+    headers = {"Authorization": f"Bearer {PLACEHOLDER}", "X-Acme-Key": OTHER}
+    flow = _flow("https", "api.acme.test", headers)
+
+    await addon.requestheaders(flow)
+
+    assert flow.response.status_code == 403
+    assert flow.request.headers == headers
+    assert flow.request.stream is False
+
+
+@pytest.mark.parametrize(
+    ("first", "second"),
+    [("Authorization", "authorization"), ("Authorization", "Authorization")],
+)
+async def test_one_destination_twice_is_refused_whatever_the_case_or_the_value(
+    swap: ModuleType, first: str, second: str
+) -> None:
+    exchange = FakeExchange(
+        swap,
+        {"api.acme.test"},
+        {
+            (PLACEHOLDER, "api.acme.test"): (first, "Bearer real-one"),
+            (OTHER, "api.acme.test"): (second, "Bearer real-one"),
+        },
+    )
+    addon = _swap(swap, exchange, "104.18.0.1")
+    headers = {"Authorization": f"Bearer {PLACEHOLDER}", "X-Acme-Key": OTHER}
+    flow = _flow("https", "api.acme.test", headers)
+
+    await addon.requestheaders(flow)
+
+    assert flow.response.status_code == 403
+    assert flow.request.headers == headers
+
+
+async def test_every_field_of_a_repeated_name_carries_its_own_placeholder(
+    swap: ModuleType,
+) -> None:
+    exchange = FakeExchange(
+        swap,
+        {"api.acme.test"},
+        {
+            (PLACEHOLDER, "api.acme.test"): ("Authorization", "Bearer real-one"),
+            (OTHER, "api.acme.test"): ("X-Acme-Key", "real-two"),
+            (THIRD, "api.acme.test"): ("X-Acme-Key", "real-three"),
+        },
+    )
+    addon = _swap(swap, exchange, "104.18.0.1")
+    flow = _flow("https", "api.acme.test", {})
+    fields = [
+        ("X-Acme-Key", OTHER),
+        ("X-Acme-Key", THIRD),
+        ("Authorization", f"Bearer {PLACEHOLDER}"),
+    ]
+    flow.request.headers = Headers(*fields)
+
+    await addon.requestheaders(flow)
+
+    assert flow.response.status_code == 403
+    assert flow.request.headers.items(multi=True) == fields
+    assert exchange.asked == [(OTHER, "api.acme.test"), (THIRD, "api.acme.test")]
+
+
+async def test_a_repeated_field_without_a_placeholder_passes_as_it_is(swap: ModuleType) -> None:
+    exchange = FakeExchange(swap, {"api.acme.test"}, {})
+    addon = _swap(swap, exchange, "104.18.0.1")
+    flow = _flow("https", "api.acme.test", {})
+    flow.request.headers = Headers(("Accept", "text/plain"), ("Accept", "application/json"))
+
+    await addon.requestheaders(flow)
+
+    assert flow.request.headers.items(multi=True) == [
+        ("Accept", "text/plain"),
+        ("Accept", "application/json"),
+    ]
+    assert exchange.asked == []
+
+
+class ExchangeDownAfterOneAnswer(FakeExchange):
+    async def authorize(self, placeholder: str, host: str) -> tuple[str, str]:
+        self.unavailable = bool(self.asked)
+        return await super().authorize(placeholder, host)
+
+
+async def test_an_exchange_that_falls_silent_midway_gives_503_and_swaps_nothing(
+    swap: ModuleType,
+) -> None:
+    exchange = ExchangeDownAfterOneAnswer(
+        swap,
+        {"api.acme.test"},
+        {
+            (PLACEHOLDER, "api.acme.test"): ("Authorization", "Bearer real-one"),
+            (OTHER, "api.acme.test"): ("X-Acme-Key", "real-two"),
+        },
+    )
+    addon = _swap(swap, exchange, "104.18.0.1")
+    headers = {"Authorization": f"Bearer {PLACEHOLDER}", "X-Acme-Key": OTHER}
+    flow = _flow("https", "api.acme.test", headers)
+
+    await addon.requestheaders(flow)
+
+    assert flow.response.status_code == 503
+    assert flow.request.headers == headers
+
+
 async def test_a_host_header_that_differs_from_the_connect_host_is_refused(
     swap: ModuleType,
 ) -> None: