Keep the database off the network a Bot's shell is on - #218
Conversation
18b15af to
5a0897a
Compare
Compose declared no networks at all, so every service joined one and reached the others by service name. One of those services is agent-computer, which is where a Bot's shell runs, and another is postgres, whose username and password are three lines above it in the same file. So a Bot could open postgres:5432 and authenticate. That is the audit trail, the policy store and the agent tables, from the one container whose whole job is to run what a Bot was asked to do, driven by an operator or by content on a page the Bot was told to read. shell.ts already says what this breaks: isolation is the container's job, a shell can reach whatever the container can reach. The role Compose creates is the instance owner, so the append-only trail was not a defence against this either. The trigger refuses a delete and its owner can drop the trigger. postgres and migrate, the only service that reaches it by name, now have a data network to themselves. Nothing else moves, so the supervisor, SPIRE and the two Bots keep every relationship they had, and a server on the host is unaffected because it reaches the database through the published port rather than the shared network. The per-Bot path already did this correctly, which is what made the gap look like an omission: with COMPUTER_NETWORK unset the supervisor gives each computer no NetworkMode, so it lands on the default bridge away from Compose, with CapDrop ALL and no-new-privileges besides.
Says what an operator has to do if they run the server inside Compose, and that a deployment which has been running this way should look at its trail rather than assume the change is retroactive.
Taking postgres off the Bots' network removes the name and leaves the address. A container's default gateway is the host, and a port published on every interface answers there: from inside agent-computer, the gateway on 5432 accepted a connection and began authenticating as openbot on openbot, with the password three lines above it in this file. So the reachability this change is about survived it. Every other port here is already bound to 127.0.0.1 with a comment saying why. Driven both ways: the name and the gateway are now both unreachable from the computer container, the container still reaches the internet, migrate still exits 0 across the network split, and the server on the host still connects.
5a0897a to
991b4cd
Compare
davidmckayv
left a comment
There was a problem hiding this comment.
Merging. The finding is real and the fix is the right shape, and the proof in the description is the kind that should be asked for more often. One thing it did not close, which I found by driving the same probe a second way.
The change removed the name and left the address. A container's default gateway is the host, and postgres published its port on every interface while every other service in that file is bound to 127.0.0.1 with a comment saying why. So from inside agent-computer, on this branch as submitted:
postgres by name: no such host
postgres:5432 unreachable
172.26.0.1:5432 TCP-OPEN <- the host, via the default gateway
and it was not just an open socket. Speaking the startup message to that address as openbot on openbot got back a SASL authentication request, with the password three lines above it in the same file. The reachability this PR is about survived the network split.
Fixed in 991b4cd by binding the published port to loopback, matching what the rest of the file already does. Driven both ways:
postgres by name: no such host
postgres:5432 unreachable
172.26.0.1:5432 unreachable
internet: still reachable <- which a browser needs
docker ps shows 127.0.0.1:5432->5432/tcp, migrate still exits 0 with migrations applied successfully across the split, and a client on the host still connects. The changelog now names the one operator-visible consequence: a deployment reaching this port from another machine has to reach it another way, which is what publishing on every interface was doing for it.
Full gate green: 1445 pass, 0 fail, format and lint clean.
Separately, and not for this PR: the all-in-one image runs postgres and computer as s6 services in one container, and docker/s6/scripts/postgres-init.sh runs initdb -A trust -U openbot. Its comment reasons that "the only client is the process beside it, inside this container, and a password would be a secret with nobody to keep it from". The process beside it is also the one that runs /bin/bash -c for a Bot, so in that image a Bot has passwordless access as the instance owner, which is the same finding as this one without even a password in the way. Filing it separately, since fixing it means giving that cluster real auth and plumbing the secret between s6 services.
Closes #208.
What this changes
docker-compose.ymldeclared nonetworks:at all, so Compose put every service on one network withDNS by service name. One of those services is
agent-computer, which is where a Bot's shell runs(
shell.tsspawns/bin/bash -c), and another ispostgres, whose username and password are threelines above it in the same file.
So a Bot could open
postgres:5432and authenticate.computer_run_commandis a governed verb andthe shipped default policy is
{deny: [], allow: ["true"]}, so a fresh deployment permits it, driveneither by the Bot's operator or by content on a page the Bot was asked to read.
shell.ts:18-20already states what this breaks:
The role Compose creates is the instance owner, so the append-only trail was not a second line of
defence: the trigger refuses a delete, and its owner can drop the trigger.
postgresandmigrate, the only service that reaches it by name, now have adatanetwork tothemselves. Nothing else moves. The supervisor, both SPIRE services and the two Bots keep every
relationship they had, and a server on the host is unaffected because it reaches the database through
the published port rather than the shared network. A deployment that runs the API server inside
Compose joins that service to both networks, which the compose comment and the changelog both say.
The per-Bot path already did this correctly, which is what made the gap read as an omission: with
COMPUTER_NETWORKunset the supervisor gives each computer noNetworkMode, so it lands on thedefault bridge away from Compose, with
CapDrop: ["ALL"]andno-new-privilegesbesides.Where it runs
state, and it removes reachability rather than adding coordination.
Boundary and audit
the gateway never mediated, because a shell opening a socket is not an action it sees. That is
the point: the gateway cannot govern what the container can reach, only what the Bot asks it to
do, which is why
shell.tsputs isolation on the container.Changelog
Unreleased, including the one thing an operator has to act on (a server running insideCompose needs both networks) and the fact that the change is not retroactive.
Proof
Not a config review. I built
agent-computerand drove the real stack both ways, through the real/execendpoint, which is the surface a Bot's shell actually uses.Before, on this file unmodified:
Superuser on the deployment's database, from the Bot's shell, with the credentials in this file.
After:
And the stack still works, which is the half that matters more for a topology change. Full
compose up:postgres,agent-computerandsupervisorall reach healthy, andmigrateexits 0with
migrations applied successfully, so the one service that does need the database still has itacross the split. Chromium still browses:
and the container still reaches the internet, which is what a browser needs (
example.com -> 200).bun run lint,typecheck,test(1390 pass, 0 fail) andbuildall pass, anddocker compose configvalidates.One thing deliberately left
This puts the database out of reach. It does not isolate
agent-computerfrom everything else ondefault, so a shell can still reachsupervisor:4300and the SPIRE services, both of which requirea token. Giving the computer a network of its own would close that too and is a bigger change to how
a server inside Compose reaches it, so it seemed worth separating from the credentialed-database
problem this fixes.