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)