From dac1fab7f3b72f8353374d475b9680ffce99e56b Mon Sep 17 00:00:00 2001 From: Paulo Date: Sun, 8 Nov 2026 14:12:47 +0200 Subject: [PATCH] DRU-1110 -- Let the secrets proxy reach named private networks The proxy refused every destination that is not public. A box had no way to reach a server on a private network through it, such as an on-prem model server. The proxy now reads SECRETS_PROXY_PRIVATE_NETWORKS, a comma-separated list of CIDRs, into its private_networks option. An address inside a named network is reachable for CONNECT, for plain HTTP, and for the upstream connect. The list is empty by default, so an install that does not set it does not change. Loopback stays refused inside a named network, because the exchange listens there, and so does the unspecified address, which connects to loopback. An invalid CIDR stops the proxy at start. Upstream CA trust stays with mitmproxy's own ssl_verify_upstream_trusted_ca option. The deploy, security, and architecture docs name the setting. The proxy test file drops a one-caller helper and an unused argument, as the checklist asks of a touched file. --- deploy/proxy/swap.py | 43 ++++++++++++++----- docs/architecture.md | 4 +- docs/deploy.md | 6 ++- docs/security.md | 4 +- src/secrets_exchange/tests/test_swap.py | 56 +++++++++++++++++-------- 5 files changed, 82 insertions(+), 31 deletions(-) diff --git a/deploy/proxy/swap.py b/deploy/proxy/swap.py index 79cdbb9..bac3b75 100644 --- a/deploy/proxy/swap.py +++ b/deploy/proxy/swap.py @@ -3,7 +3,9 @@ mitmdump -s /addon/swap.py --set exchange_url=http://127.0.0.1:8781 TLS is terminated for hosts with a registered secret only. A loopback, -private, link-local, or metadata destination is refused, for HTTP and CONNECT. +private, link-local, or metadata destination is refused, for HTTP and CONNECT, +unless private_networks names its network. Loopback stays refused: the +exchange listens there. """ import asyncio @@ -32,6 +34,7 @@ _PLACEHOLDER_PREFIX = "drk." Address = ipaddress.IPv4Address | ipaddress.IPv6Address +Network = ipaddress.IPv4Network | ipaddress.IPv6Network class Refused(Exception): @@ -59,13 +62,6 @@ def placeholder_in(value: str) -> str: return "" -def is_reachable(addresses: set[Address]) -> bool: - """Every address is public and none is multicast.""" - return bool(addresses) and all( - address.is_global and not address.is_multicast for address in addresses - ) - - class Exchange: def __init__(self, url: str) -> None: self.url = url.rstrip("/") @@ -101,6 +97,7 @@ async def _get(self, path: str, headers: dict[str, str]) -> httpclient.HTTPRespo class Swap: def __init__(self) -> None: self.exchange: Exchange + self.private_networks: list[Network] = [] self._upstreams: set[str] = set() self._upstreams_at = float("-inf") self._upstreams_known = False @@ -112,10 +109,22 @@ def load(self, loader) -> None: os.environ.get("SECRETS_EXCHANGE_URL", ""), "Address of the secrets exchange process.", ) + loader.add_option( + "private_networks", + str, + os.environ.get("SECRETS_PROXY_PRIVATE_NETWORKS", ""), + "Private networks the proxy can reach, as comma-separated CIDRs.", + ) def configure(self, updated: set[str]) -> None: if "exchange_url" in updated: self.exchange = Exchange(ctx.options.exchange_url) + if "private_networks" in updated: + self.private_networks = [ + ipaddress.ip_network(cidr) + for cidr in map(str.strip, ctx.options.private_networks.split(",")) + if cidr + ] async def upstreams(self) -> set[str]: """While the exchange gives no answer, the last list stands.""" @@ -135,8 +144,19 @@ async def resolve(self, host: str) -> set[Address]: return set() return {ipaddress.ip_address(entry[4][0]) for entry in found} + def can_reach(self, addresses: set[Address]) -> bool: + """Every address is public or inside a named private network. Loopback and + multicast are never reachable, nor the unspecified address, which connects + to loopback.""" + return bool(addresses) and all( + (address.is_global or any(address in network for network in self.private_networks)) + and not (address.is_loopback or address.is_unspecified or address.is_multicast) + for address in addresses + ) + async def http_connect(self, flow: http.HTTPFlow) -> None: - if not is_reachable(await self.resolve(flow.request.host)): + addresses = await self.resolve(flow.request.host) + if not self.can_reach(addresses): flow.response = http.Response.make(403, b"destination refused\n") async def server_connect(self, data: server_hooks.ServerConnectionHookData) -> None: @@ -148,7 +168,7 @@ async def server_connect(self, data: server_hooks.ServerConnectionHookData) -> N return host, port = data.server.address addresses = await self.resolve(host) - if not is_reachable(addresses): + if not self.can_reach(addresses): data.server.error = "destination refused" return chosen = min(addresses, key=lambda address: (address.version, address.packed)) @@ -169,7 +189,8 @@ async def tls_clienthello(self, data: tls.ClientHelloData) -> None: async def requestheaders(self, flow: http.HTTPFlow) -> None: if flow.request.scheme != "https": - if not is_reachable(await self.resolve(flow.request.host)): + addresses = await self.resolve(flow.request.host) + if not self.can_reach(addresses): flow.response = http.Response.make(403, b"destination refused\n") else: found = [ diff --git a/docs/architecture.md b/docs/architecture.md index 60df93e..a4fd04e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -161,7 +161,9 @@ 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 +the proxy. The proxy can reach the private networks that +`SECRETS_PROXY_PRIVATE_NETWORKS` names. +Every connection goes to the address the proxy checked, and the upstream certificate is checked against the CONNECT host. A request whose `Host` differs from the CONNECT host is refused. On docker-sbx the sandbox gets only the placeholder. Drukbox puts the value in sbx's own secret store for that sandbox, and sbx's proxy swaps diff --git a/docs/deploy.md b/docs/deploy.md index 2e4d98d..8983a45 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -423,7 +423,11 @@ placeholder for the real credential on the way out. Two pieces run this: addon into the official image instead. It terminates TLS only for the hosts that have a registered secret and tunnels every other host blind. It refuses a destination that resolves to a loopback, private, link-local, or - metadata address. It makes its CA on first start and keeps it in a volume. + metadata address. It can reach the private networks that + `SECRETS_PROXY_PRIVATE_NETWORKS` names, as comma-separated CIDRs such as + `10.20.0.0/16`. The list is empty by default. Loopback stays refused, + because the exchange listens there. It makes its CA on first start and + keeps it in a volume. - **The exchange process** runs as `python -m secrets_exchange` from this image. The proxy asks it which hosts to terminate, and, for a request with a placeholder, for the header the upstream reads and the real credential. diff --git a/docs/security.md b/docs/security.md index 13731e9..62c85a2 100644 --- a/docs/security.md +++ b/docs/security.md @@ -106,7 +106,9 @@ 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. +API through it. It logs no credential. The proxy can reach the private networks +that `SECRETS_PROXY_PRIVATE_NETWORKS` names. Loopback stays refused inside a +named network, so a box cannot reach the exchange. 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 diff --git a/src/secrets_exchange/tests/test_swap.py b/src/secrets_exchange/tests/test_swap.py index 517b4b2..f48444f 100644 --- a/src/secrets_exchange/tests/test_swap.py +++ b/src/secrets_exchange/tests/test_swap.py @@ -61,23 +61,17 @@ def __init__(self, url: str, headers: dict[str, str], request_timeout: float) -> class FakeResponse: - def __init__(self, status_code: int, content: bytes, headers: dict[str, str]) -> None: + def __init__(self, status_code: int, content: bytes) -> None: self.status_code = status_code self.content = content - self.headers = headers - - @classmethod - def make( - cls, status_code: int = 200, content: bytes = b"", headers: dict[str, str] | None = None - ) -> "FakeResponse": - return cls(status_code, content, headers or {}) -def _stand_ins() -> dict[str, ModuleType]: +@pytest.fixture(scope="module") +def swap() -> Iterator[ModuleType]: mitmproxy = ModuleType("mitmproxy") mitmproxy.ctx = SimpleNamespace(options=SimpleNamespace(exchange_url="http://exchange.test")) # type: ignore[attr-defined] http = ModuleType("mitmproxy.http") - http.Response = FakeResponse # type: ignore[attr-defined] + http.Response = SimpleNamespace(make=FakeResponse) # type: ignore[attr-defined] http.HTTPFlow = object # type: ignore[attr-defined] tls = ModuleType("mitmproxy.tls") tls.ClientHelloData = object # type: ignore[attr-defined] @@ -90,7 +84,7 @@ def _stand_ins() -> dict[str, ModuleType]: httpclient.HTTPRequest = FakeRequest # type: ignore[attr-defined] httpclient.HTTPResponse = object # type: ignore[attr-defined] httpclient.HTTPClientError = type("HTTPClientError", (Exception,), {}) # type: ignore[attr-defined] - return { + stand_ins = { "mitmproxy": mitmproxy, "mitmproxy.http": http, "mitmproxy.tls": tls, @@ -99,11 +93,6 @@ def _stand_ins() -> dict[str, ModuleType]: "tornado": tornado, "tornado.httpclient": httpclient, } - - -@pytest.fixture(scope="module") -def swap() -> Iterator[ModuleType]: - stand_ins = _stand_ins() sys.modules.update(stand_ins) try: spec = importlib.util.spec_from_file_location("swap", ADDON) @@ -221,7 +210,40 @@ def test_a_placeholder_is_read_from_a_bearer_a_basic_password_or_a_bare_value( def test_only_a_destination_with_public_addresses_is_reachable( swap: ModuleType, addresses: set[str], reachable: bool ) -> None: - assert swap.is_reachable({ipaddress.ip_address(a) for a in addresses}) is reachable + assert swap.Swap().can_reach({ipaddress.ip_address(a) for a in addresses}) is reachable + + +@pytest.mark.parametrize( + ("network", "addresses", "reachable"), + [ + ("172.17.0.0/16", {"172.17.0.1"}, True), + ("172.17.0.0/16", {"172.17.0.1", "8.8.8.8"}, True), + ("172.17.0.0/16", {"10.1.2.3"}, False), + ("fd00::/8", {"fd00::1"}, True), + ("127.0.0.0/8", {"127.0.0.1"}, False), + ("::/0", {"::1"}, False), + ("0.0.0.0/0", {"0.0.0.0"}, False), + ], +) +def test_a_named_private_network_is_reachable_and_loopback_never_is( + swap: ModuleType, network: str, addresses: set[str], reachable: bool +) -> None: + addon = swap.Swap() + addon.private_networks = [ipaddress.ip_network(network)] + + assert addon.can_reach({ipaddress.ip_address(a) for a in addresses}) is reachable + + +def test_the_private_networks_option_is_read_as_comma_separated_cidrs(swap: ModuleType) -> None: + swap.ctx.options.private_networks = " 10.9.0.0/16, 172.17.0.1 ," + addon = swap.Swap() + + addon.configure({"private_networks"}) + + assert addon.private_networks == [ + ipaddress.ip_network("10.9.0.0/16"), + ipaddress.ip_network("172.17.0.1/32"), + ] async def test_a_connect_to_a_private_destination_is_refused(swap: ModuleType) -> None: