diff --git a/Dockerfile b/Dockerfile index d513f00..27b21e0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,14 +2,23 @@ ARG HAPROXY_VERSION=lts FROM haproxy:${HAPROXY_VERSION}-alpine EXPOSE 2375 -ENV ALLOW_RESTARTS=0 \ +ENV ALLOW_ARCHIVE=0 \ + ALLOW_ATTACH=0 \ + ALLOW_EXEC=0 \ + ALLOW_EXPORT=0 \ + ALLOW_KILL=0 \ + ALLOW_LOGS=0 \ + ALLOW_PAUSE=0 \ + ALLOW_RESTARTS=0 \ ALLOW_STOP=0 \ ALLOW_START=0 \ + ALLOW_UNPAUSE=0 \ AUTH=0 \ BUILD=0 \ COMMIT=0 \ CONFIGS=0 \ CONTAINERS=0 \ + DELETE=0 \ DISABLE_IPV6=0 \ DISTRIBUTION=0 \ EVENTS=1 \ @@ -20,9 +29,11 @@ ENV ALLOW_RESTARTS=0 \ LOG_LEVEL=info \ NETWORKS=0 \ NODES=0 \ + PATCH=0 \ PING=1 \ PLUGINS=0 \ POST=0 \ + PUT=0 \ SECRETS=0 \ SERVICES=0 \ SESSION=0 \ diff --git a/README.md b/README.md index 8c1aceb..61e4216 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,7 @@ default. Maximum caution when enabling these. - `SECRETS` - `POST`: When disabled, only `GET` and `HEAD` operations are allowed, meaning any section of the API is read-only. +- `PUT`, `PATCH` and `DELETE`: additional HTTP-method toggles, disabled by default. #### Not always needed @@ -124,14 +125,29 @@ extremely critical but can expose some information that your service does not ne - `BUILD` - `COMMIT` - `CONFIGS` -- `CONTAINERS` +- `CONTAINERS`: Grants access to `/containers/*` metadata (listing, inspect, stats). + Dangerous sub-paths (`archive`, `attach`, `export`, `logs`) remain blocked unless + explicitly enabled via their respective `ALLOW_*` variables. +- `ALLOW_ARCHIVE` (containers/`id`/`archive`) +- `ALLOW_ATTACH` (containers/`id`/`attach`) +- `ALLOW_EXPORT` (containers/`id`/`export`) +- `ALLOW_LOGS` (containers/`id`/`logs`) +- `ALLOW_PAUSE` (containers/`id`/`pause`) +- `ALLOW_RESTARTS` (containers/`id`/`stop`|`restart`|`kill`) - `ALLOW_START` (containers/`id`/`start`) - `ALLOW_STOP` (containers/`id`/`stop`) -- `ALLOW_RESTARTS` (containers/`id`/`stop`|`restart`|`kill`) -- `ALLOW_PAUSE` (containers/`id`/`pause`) - `ALLOW_UNPAUSE` (containers/`id`/`unpause`) +- `ALLOW_EXEC` (containers/`id`/`exec`) — required IN ADDITION to `CONTAINERS=1` to + create new exec instances. Without it, even with `CONTAINERS=1 POST=1`, + `docker exec` is denied. +- `ALLOW_KILL` (containers/`id`/`kill`) — required IN ADDITION to `CONTAINERS=1` to + kill a container. Without it, `docker kill` is denied even with + `CONTAINERS=1 POST=1`. +- `DELETE`: global toggle for the HTTP `DELETE` method. - `DISTRIBUTION` -- `EXEC` +- `EXEC` — controls only `/exec/{id}/start|resize|inspect` (operations on _existing_ + exec sessions). Creation of new exec instances via `POST /containers/{id}/exec` is + controlled by `ALLOW_EXEC` (above). - `GRPC` - `IMAGES` - `INFO` diff --git a/system_files/usr/local/etc/haproxy/haproxy.cfg.template b/system_files/usr/local/etc/haproxy/haproxy.cfg.template index d61b1d4..85f1238 100644 --- a/system_files/usr/local/etc/haproxy/haproxy.cfg.template +++ b/system_files/usr/local/etc/haproxy/haproxy.cfg.template @@ -45,9 +45,33 @@ backend docker-events frontend dockerfrontend bind ${BIND_CONFIG} - http-request deny unless METH_GET || { env(POST) -m bool } + # --- Method ACLs --- + acl method_GET method GET + acl method_HEAD method HEAD + acl method_POST method POST + acl method_DELETE method DELETE + acl method_PUT method PUT + acl method_PATCH method PATCH - # Allowed endpoints + # --- Allow only enabled methods --- + http-request deny unless METH_GET || method_POST { env(POST) -m bool } || method_DELETE { env(DELETE) -m bool } || method_PUT { env(PUT) -m bool } || method_PATCH { env(PATCH) -m bool } + + # --- Granular deny rules, evaluated BEFORE the allow rules below. --- + # Container-mutating operations stay disabled unless their dedicated flag + http-request deny if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/exec } !{ env(ALLOW_EXEC) -m bool } + http-request deny if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/kill } !{ env(ALLOW_KILL) -m bool } + + # Attach is a get request, but allows to write to a container's stdin. + http-request deny if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/attach } !{ env(ALLOW_ATTACH) -m bool } + + # Filesystem / information-exposing container sub-endpoints, gated behind + # their own flags even when CONTAINERS=1 is set. + http-request deny if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/archive } !{ env(ALLOW_ARCHIVE) -m bool } + http-request deny if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/export } !{ env(ALLOW_EXPORT) -m bool } + http-request deny if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/logs } !{ env(ALLOW_LOGS) -m bool } + + # --- Allowed endpoints --- + # GET, HEAD, PUT & PATCH http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/((stop)|(restart)|(kill)) } { env(ALLOW_RESTARTS) -m bool } http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/start } { env(ALLOW_START) -m bool } http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/stop } { env(ALLOW_STOP) -m bool } diff --git a/tests/test_service.py b/tests/test_service.py index 4724d3f..86a1fe5 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -42,11 +42,64 @@ def test_default_permissions(proxy_factory): def test_container_permissions(proxy_factory): with proxy_factory(CONTAINERS=1) as test_container: + allowed_calls = [ + ("inspect", test_container), + ] + forbidden_calls = [ + ("logs", test_container), + ("export", test_container), + ("cp", f"{test_container}:/etc/passwd", "-"), + ("wait", test_container), + ("run", "--rm", "alpine"), + ("rm", "-f", test_container), + ("restart", test_container), + ] + _check_permissions(allowed_calls, forbidden_calls) + + +def test_container_logs_permissions(proxy_factory): + with proxy_factory(CONTAINERS=1, ALLOW_LOGS=1) as test_container: allowed_calls = [ ("logs", test_container), ("inspect", test_container), ] forbidden_calls = [ + ("export", test_container), + ("cp", f"{test_container}:/etc/passwd", "-"), + ("wait", test_container), + ("run", "--rm", "alpine"), + ("rm", "-f", test_container), + ("restart", test_container), + ] + _check_permissions(allowed_calls, forbidden_calls) + + +def test_container_export_permissions(proxy_factory): + with proxy_factory(CONTAINERS=1, ALLOW_EXPORT=1) as test_container: + allowed_calls = [ + ("export", test_container), + ("inspect", test_container), + ] + forbidden_calls = [ + ("logs", test_container), + ("cp", f"{test_container}:/etc/passwd", "-"), + ("wait", test_container), + ("run", "--rm", "alpine"), + ("rm", "-f", test_container), + ("restart", test_container), + ] + _check_permissions(allowed_calls, forbidden_calls) + + +def test_container_archive_permissions(proxy_factory): + with proxy_factory(CONTAINERS=1, ALLOW_ARCHIVE=1) as test_container: + allowed_calls = [ + ("cp", f"{test_container}:/etc/passwd", "-"), + ("inspect", test_container), + ] + forbidden_calls = [ + ("logs", test_container), + ("export", test_container), ("wait", test_container), ("run", "--rm", "alpine"), ("rm", "-f", test_container), @@ -72,16 +125,89 @@ def test_network_post_permissions(proxy_factory): allowed_calls = [ ("network", "ls"), ("network", "create", "foo"), + ] + forbidden_calls = [ ("network", "rm", "foo"), ] - forbidden_calls = [] + _check_permissions(allowed_calls, forbidden_calls) + docker(["network", "rm", "foo"]) + + +def test_network_delete_permissions(proxy_factory): + with proxy_factory(NETWORKS=1, POST=1, DELETE=1): + allowed_calls = [ + ("network", "create", "net-to-delete"), + ("network", "rm", "net-to-delete"), + ] + forbidden_calls = [ + ("volume", "create", "test-volume"), + ] _check_permissions(allowed_calls, forbidden_calls) def test_exec_permissions(proxy_factory): - with proxy_factory(CONTAINERS=1, EXEC=1, POST=1) as container_id: + # ALLOW_EXEC=1 is required IN ADDITION to CONTAINERS+EXEC+POST to actually + # create new exec sessions, see issue #114. EXEC controls only operations + # on already-created exec sessions (/exec//start|resize|inspect). + with proxy_factory(CONTAINERS=1, EXEC=1, POST=1, ALLOW_EXEC=1) as container_id: allowed_calls = [ ("exec", container_id, "ls"), ] forbidden_calls = [] _check_permissions(allowed_calls, forbidden_calls) + + +def test_exec_denied_without_allow_exec(proxy_factory): + """CONTAINERS=1 + POST=1 must NOT be enough to create new exec sessions. + + Regression test for https://github.com/Tecnativa/docker-socket-proxy/issues/114. + """ + with proxy_factory(CONTAINERS=1, EXEC=1, POST=1) as container_id: + forbidden_calls = [ + ("exec", container_id, "ls"), + ] + _check_permissions((), forbidden_calls) + + +def test_kill_denied_without_allow_kill(proxy_factory): + """CONTAINERS=1 + POST=1 must NOT allow `docker kill`.""" + with proxy_factory(CONTAINERS=1, POST=1) as container_id: + forbidden_calls = [ + ("kill", container_id), + ] + _check_permissions((), forbidden_calls) + + +def test_delete_denied_without_delete(proxy_factory): + """CONTAINERS=1 + POST=1 must NOT allow DELETE /containers/.""" + with proxy_factory(CONTAINERS=1, POST=1) as container_id: + forbidden_calls = [ + ("rm", "-f", container_id), + ] + _check_permissions((), forbidden_calls) + + +def test_container_delete_permissions(proxy_factory): + with proxy_factory( + CONTAINERS=1, + DELETE=1, + IMAGES=1, + POST=1, + ): + allowed_calls = [ + ("pull", "alpine"), + ("run", "--rm", "-dt", "--name", "test-delete", "alpine"), + ("rm", "-f", "test-delete"), + ] + forbidden_calls = [] + _check_permissions(allowed_calls, forbidden_calls) + + +def test_image_delete_permissions(proxy_factory): + with proxy_factory(IMAGES=1, DELETE=1, POST=1): + allowed_calls = [ + ("pull", "hello-world"), + ("image", "rmi", "hello-world"), + ] + forbidden_calls = [] + _check_permissions(allowed_calls, forbidden_calls)