linuxserver / docker-socket-proxy

Proxy over your Docker socket to restrict which requests it accepts
GNU General Public License v3.0
45 stars 3 forks source link

[BUG] ALLOW_* options only work if POST is granted #14

Closed Cheezzhead closed 2 weeks ago

Cheezzhead commented 3 weeks ago

Is there an existing issue for this?

Current Behavior

ALLOW_START, ALLOW_STOP, ALLOW_RESTARTS are all POST methods.

Currently, the behavior is:

So as it stands, these permissions have no practical effect.

I would assume the problem is easily fixed by adjusting the respective limit_except directives in the template files, e.g.:

# Old
location ~* ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/((stop)|(restart)|(kill)) {limit_except GET HEAD {deny all;}if ($path_restarts = 0){return 403;}proxy_pass http://unix:$dockersocket;}

# New
location ~* ^(/v[\d\.]+)?/containers/[a-zA-Z0-9_.-]+/((stop)|(restart)|(kill)) {limit_except GET HEAD POST {deny all;}if ($path_restarts = 0){return 403;}proxy_pass http://unix:$dockersocket;}

(Side note: I would also leave out HEAD because it is implied by GET)

Expected Behavior

The ALLOW_ permissions should supercede the POST permissions, or they should be deprecated.

Steps To Reproduce

Attempting to start/stop/restart a container in (for example) Portainer will fail, using the configuration provided below.

Environment

- OS:Ubuntu 20.04
- How docker service was installed: Compose

Docker creation

services:
  docker-socket:
    image: lscr.io/linuxserver/socket-proxy:latest
    container_name: docker-socket
    read_only: true
    tmpfs:
      - /run
    env_file: docker-socket.env
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock:ro

docker-socket.env:

DISABLE_IPV6=0

# Granted
#########
EVENTS=1
PING=1
VERSION=1
ALLOW_START=1
ALLOW_STOP=1
ALLOW_RESTARTS=1

# Portainer
CONTAINERS=1
IMAGES=1
INFO=1
NETWORKS=1
SERVICES=1
TASKS=1
VOLUMES=1

# Revoked
#########
# Security critical
AUTH=0
SECRETS=0
POST=0

# Not necessary
BUILD=0
COMMIT=0
CONFIGS=0
DISTRIBUTION=0
EXEC=0
NODES=0
PLUGINS=0
SESSION=0
SWARM=0
SYSTEM=0

Container logs

// ...
2024/08/20 12:58:40 [error] 47#47: *3472 access forbidden by rule, client: X.X.X.X, server: _, request: "POST /containers/<CONTAINER_UUID>/pause HTTP/1.1", host: "docker-socket:2375", referrer: "https://portainer.domain.tld/"
// ...
github-actions[bot] commented 3 weeks ago

Thanks for opening your first issue here! Be sure to follow the relevant issue templates, or risk having this issue marked as invalid.

thespad commented 3 weeks ago

If POST=1, the operations are allowed even if the above environment variables are set to 0.

This is not correct.

    environment:
      - ALLOW_START=0
      - ALLOW_STOP=0
      - ALLOW_RESTARTS=0
      - CONTAINERS=1
      - POST=1
172.22.0.51 - - [20/Aug/2024:17:34:47 +0000] "POST /containers/d8de6a4266a57cd65d149a0f19fdc3be5b6ce0a8b6f7dabb0ae4c3a7d4d8715d/stop HTTP/1.1" 403 146 "https://portainer.example.com/" "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:129.0) Gecko/20100101 Firefox/129.0"

Note the 403. nginx parses locations starting with the most specific path and so will hit the stop/start/restart/kill locations and return a 403 before hitting the generic container location.

Now, I am open to adding variables for allowing Start/Stop/Restart without POST=1 if it's desired functionality.

Cheezzhead commented 3 weeks ago

This is not correct.

Ah, apologies. I do remember quickly testing that scenario prior to posting but probably didn't really look at the response properly.

Now, I am open to adding variables for allowing Start/Stop/Restart without POST=1 if it's desired functionality.

I think it would be. I'm open to making a PR, though my proposed solution (changing the limit_except directives) would probably be too minimal to warrant one?

thespad commented 2 weeks ago

It's fine, I'll put together a PR to change the behaviour and update the documentation.