From b6e7fc12a9cc30a7eb619cff2942ac906c704f54 Mon Sep 17 00:00:00 2001 From: Paulo Date: Mon, 7 Sep 2026 12:37:47 +0200 Subject: [PATCH] Accept plain HTTP issuer URLs An issuer can run inside the same deployment as the exchange, where the exchange already answers the proxy over plain HTTP. Requiring HTTPS there forced a certificate and a trust store into every process that fetches, and protected nothing. The schema now accepts http and https. It still refuses credentials and fragments in the URL, since the URL is stored readable. The security and deploy docs say which scheme to use where. A header value is printable ASCII now, and a header that h11 refuses raises an IssuerError with no chain, so a value never reaches the log through the provisioning failure. --- docs/deploy.md | 3 ++- docs/security.md | 8 +++++--- src/host_secrets/schemas.py | 13 ++++++++++--- src/host_secrets/tests/test_schemas.py | 17 ++++++++++++++++- src/hosts/tests/test_secrets.py | 20 ++++++++++++++++++++ src/secrets_exchange/secrets.py | 3 +++ src/secrets_exchange/tests/test_app.py | 19 +++++++++++++++++++ src/secrets_exchange/tests/test_secrets.py | 12 ++++++++++++ 8 files changed, 87 insertions(+), 8 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index 941f8e9..67c8d32 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -381,7 +381,8 @@ Give secrets to `POST /hosts`. Provisioning delivers the placeholders in the sandbox's boot environment, on every provider, the same way as `env`. A refreshable secret, one given with `issuer`, is fetched by the exchange process on first use and kept in memory until shortly before it expires. The -exchange process must reach the issuer URL. On docker-sbx the API process +exchange process must reach the issuer URL, over plain HTTP inside the +deployment or HTTPS outside it. On docker-sbx the API process fetches it once at provisioning, since sbx holds the value. The exchange process pushes a fresh one before it expires, and logs each push. A pool host takes no secrets: a request with secrets always provisions a new sandbox. diff --git a/docs/security.md b/docs/security.md index b87b81f..5525e28 100644 --- a/docs/security.md +++ b/docs/security.md @@ -118,9 +118,11 @@ certificate, from `SECRETS_PROXY_CA_FILE`. `POST /hosts` never returns a secret. A validation response omits the rejected input, so a bad value or a bad issuer header does not reach the caller. An -issuer URL must use HTTPS. It must not carry user credentials or a fragment. -Put credentials only in the issuer headers, which Drukbox encrypts. The value -an issuer returns is never stored. +issuer URL must not carry user credentials or a fragment. It can use plain +HTTP inside the deployment, where the exchange already answers the proxy in +the clear. An issuer outside the deployment uses HTTPS. Put credentials only +in the issuer headers, which Drukbox encrypts. The value an issuer returns is +never stored. ## What `env` is and is not diff --git a/src/host_secrets/schemas.py b/src/host_secrets/schemas.py index a7c3320..c4e8b9f 100644 --- a/src/host_secrets/schemas.py +++ b/src/host_secrets/schemas.py @@ -34,15 +34,22 @@ class SecretIssuer(BaseModel): @field_validator("url") @classmethod - def require_secure_issuer(cls, url: HttpUrl) -> HttpUrl: - if url.scheme != "https": - raise ValueError("issuer URL must use https") + def refuse_secrets_in_url(cls, url: HttpUrl) -> HttpUrl: if url.username or url.password: raise ValueError("issuer URL must not contain credentials") if url.fragment: raise ValueError("issuer URL must not contain a fragment") return url + @field_validator("headers") + @classmethod + def refuse_control_characters(cls, headers: dict[str, SecretStr]) -> dict[str, SecretStr]: + for value in headers.values(): + secret = value.get_secret_value() + if not (secret.isascii() and secret.isprintable()): + raise ValueError("issuer header values must be printable ASCII") + return headers + def to_storage(self) -> dict[str, Any]: return { "url": str(self.url), diff --git a/src/host_secrets/tests/test_schemas.py b/src/host_secrets/tests/test_schemas.py index f86cad8..5919e91 100644 --- a/src/host_secrets/tests/test_schemas.py +++ b/src/host_secrets/tests/test_schemas.py @@ -94,12 +94,27 @@ def test_registration_rejects_ambiguous_or_incomplete_shapes( SecretEntry.model_validate(payload) +@pytest.mark.parametrize( + "url", + [ + "https://mint.example.test/token", + "http://127.0.0.1:8001/api/mint/grant/github", + "http://web:8000/api/mint/grant/github", + ], +) +def test_issuer_accepts_http_inside_the_deployment_and_https_anywhere(url: str) -> None: + entry = SecretEntry.model_validate({"issuer": _issuer(url=url)}) + + assert entry.issuer and str(entry.issuer.url) == url + + @pytest.mark.parametrize( "issuer", [ - _issuer(url="http://mint.example.test/token"), + _issuer(url="ftp://mint.example.test/token"), _issuer(url="https://user:password@mint.example.test/token"), _issuer(url="https://mint.example.test/token#credential"), + _issuer(headers={"Authorization": "Bearer secret\n"}), _issuer(refresh="0m"), _issuer(refresh="50minutes"), _issuer(headers={}), diff --git a/src/hosts/tests/test_secrets.py b/src/hosts/tests/test_secrets.py index a3e168b..8e7fd4c 100644 --- a/src/hosts/tests/test_secrets.py +++ b/src/hosts/tests/test_secrets.py @@ -164,6 +164,26 @@ async def test_a_provider_that_holds_the_value_gets_it_at_boot( assert "HTTPS_PROXY" not in environment +@respx.mock +async def test_an_http_issuer_inside_the_deployment_is_fetched_at_boot( + settings: Settings, create_vm: AsyncMock, stub_provider: StubVMProvider +) -> None: + recording = RecordingInjection() + stub_provider.secrets = recording + issuer = {**ISSUER, "url": "http://web:8000/api/mint/grant/github"} + respx.get(issuer["url"]).respond(json={"value": "ghs_minted"}) + + async with async_session_factory() as session: + await HostService(session, settings=settings).create_host( + env={}, + secrets={"github": {**SECRETS["github"], "issuer": issuer}}, + image=None, + provider="stub", + ) + + assert recording.values == {"github": "ghs_minted"} + + class OversizedInjection(RecordingInjection): """Hands the box a value that pam_env cannot read.""" diff --git a/src/secrets_exchange/secrets.py b/src/secrets_exchange/secrets.py index e427c37..da4e9a7 100644 --- a/src/secrets_exchange/secrets.py +++ b/src/secrets_exchange/secrets.py @@ -45,6 +45,9 @@ async def fetch(cls, issuer: dict[str, Any], client: httpx.AsyncClient) -> Self: response = await client.get(issuer["url"], headers=issuer["headers"]) response.raise_for_status() secret = cls.model_validate(response.json()) + except httpx.LocalProtocolError: + # The message would carry the header value. + raise IssuerError("issuer headers are not valid HTTP") from None except httpx.HTTPStatusError as exc: raise IssuerError(f"status {exc.response.status_code}") from exc except httpx.HTTPError as exc: diff --git a/src/secrets_exchange/tests/test_app.py b/src/secrets_exchange/tests/test_app.py index 0dfb227..8d303b9 100644 --- a/src/secrets_exchange/tests/test_app.py +++ b/src/secrets_exchange/tests/test_app.py @@ -108,6 +108,25 @@ async def test_an_issuer_entry_is_fetched_and_exchanged(edge) -> None: assert route.calls[0].request.headers["X-Key"] == "k" +@respx.mock +async def test_an_http_issuer_inside_the_deployment_is_fetched(edge) -> None: + host_id = uuid.uuid4() + minted = Placeholder.mint(host_id, "github") + issuer = { + "url": "http://web:8000/api/mint/grant/github", + "headers": {"X-Key": "k"}, + "refresh": "1h", + } + await _create_host( + host_id, {"github": {"issuer": issuer, "placeholder_fingerprint": minted.fingerprint}} + ) + respx.get(issuer["url"]).respond(json={"value": "ghs_minted"}) + + response = await edge.get("/authorize", headers=_headers(str(minted), "api.github.com")) + + assert response.headers["X-Upstream-Credential"] == "Bearer ghs_minted" + + @respx.mock async def test_an_issuer_that_gives_nothing_usable_answers_503(edge) -> None: host_id = uuid.uuid4() diff --git a/src/secrets_exchange/tests/test_secrets.py b/src/secrets_exchange/tests/test_secrets.py index 74ad669..6a926a5 100644 --- a/src/secrets_exchange/tests/test_secrets.py +++ b/src/secrets_exchange/tests/test_secrets.py @@ -264,3 +264,15 @@ async def test_a_fetch_that_fails_pushes_nothing(secrets) -> None: await secrets.push(uuid.uuid4(), VM, "github", ENTRY, injection) injection.push_secret.assert_not_awaited() + + +async def test_a_header_h11_refuses_never_reaches_the_error_chain() -> None: + def refuse(request: httpx.Request) -> httpx.Response: + raise httpx.LocalProtocolError("Illegal header value b'Bearer sk-live\\n'") + + async with httpx.AsyncClient(transport=httpx.MockTransport(refuse)) as client: + with pytest.raises(IssuerError, match="not valid HTTP") as caught: + await Secret.fetch(ISSUER, client) + + assert not caught.value.__cause__ + assert "sk-live" not in repr(caught.value)