Skip to content

fix: deny attach and refuse protocol upgrades in all three (#76) - #79

Merged
abienkowski merged 3 commits into
mainfrom
fix/deny-attach-upgrade-76
Oct 12, 2026
Merged

abienkowski merged 3 commits into
mainfrom
fix/deny-attach-upgrade-76

Conversation

@abienkowski

Copy link
Copy Markdown
Collaborator

Description

Closes #76.

Security fix. Through the Go proxy, a client could write to any container's stdin with a websocket attach: GET /containers/{name}/attach/ws?stdin=1&stream=1 plus Upgrade: websocket. The router treated it as a read (GET passthrough), and httputil.ReverseProxy relayed the 101 Switching Protocols. With a shell as the container's process, that is equivalent to exec. It worked with and without --readonly. Rust and TS forwarded the request too, but in testing no stdin got through (Rust's connection broke after relaying 101; TS timed out).

What changes

  1. The router denies attach, in all three, like exec: attach as a whole segment after /containers/{name}, for every method, with attach is not allowed. That covers /attach, /attach/ws and version-prefixed forms. A container named attach-runner is unaffected; a container literally named attach can no longer be addressed by name (the same trade-off as exec, documented).
  2. Every proxy refuses protocol upgrades, before reading the body and before routing: a request with an Upgrade header, or a Connection header containing the upgrade token (case-insensitive, comma-separated, across repeated headers), gets 403 protocol upgrade not allowed and an audit entry. No endpoint can be turned into a raw bidirectional stream. The Docker CLI upgrades only for attach, exec and BuildKit sessions, all of which are denied, so legitimate use is unaffected; logs -f and events stream without upgrading.
    • TS: the server registers no 'upgrade' listener, so Node delivers upgrade requests to the normal request handler; a real-http.Server test pins that.

Review notes

Verification

  • Spec mutation: router with ALLOW_ATTACH = true fails attachSoundTest and the GET, HEAD and named-container rows. attachPostDenied still passes because POST attach was already default-denied; it is documented as a pin.
  • Before the fix (d110b8a), the new tests fail; and main's Go code with this branch's integration suite:
    FAIL: GET /containers/*/attach/ws with Upgrade: websocket -> 403 (upgrade refused) (expected 403 with "protocol upgrade not allowed", got 101: )
    FAIL: GET /containers/*/attach/ws -> 403 (attach denied) (expected 403 with "attach is not allowed", got 404: {"message":"No such container: no-such-container"})
    FAIL: GET /_ping with Upgrade: websocket -> 403 (upgrade refused) (expected 403 with "protocol upgrade not allowed", got 200: OK)
    50 PASSED, 3 FAILED
    
  • The original exploit, replayed against this branch's binaries (websocket attach plus a stdin frame to a real busybox container): Go, Rust and TS each answer HTTP/1.1 403 Forbidden (protocol upgrade not allowed), and nothing reaches the container. Before the fix, Go delivered the frame.
  • This branch: make test-all Go 110, Rust 142, TS 160 (1 skipped); make lint-all, make test-spec (8 runs), make verify pass; integration ALL 53 TESTS PASSED for Go, Rust and TS; socket suite ALL 15 TESTS PASSED.

Type of change

  • Bug fix (security)
  • New feature
  • Breaking change
  • Documentation update

Implementation(s) changed

  • Go
  • Rust
  • TypeScript
  • Quint specification
  • CI / infrastructure

Testing

  • Unit tests pass (make test-all)
  • Integration tests pass (53 per implementation; socket suite 15)
  • Quint verification passes (make verify, make test-spec)
  • New tests added for the change

Checklist

  • I have read CONTRIBUTING.md
  • My code follows the project's coding style
  • I have updated documentation as needed

@abienkowski abienkowski added Priority: P0 Added to issues and PRs relating to a critical severity bugs. Type: Bug Added to issues and PRs if they are addressing a bug labels Oct 12, 2026
@abienkowski
abienkowski merged commit c874aad into main Oct 12, 2026
7 checks passed
@abienkowski
abienkowski deleted the fix/deny-attach-upgrade-76 branch October 12, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: P0 Added to issues and PRs relating to a critical severity bugs. Type: Bug Added to issues and PRs if they are addressing a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Go proxy allows websocket attach (GET /containers/{name}/attach/ws), giving stdin access to containers

1 participant