diff --git a/AGENTS.md b/AGENTS.md index fe9f89c..301859c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,17 +39,17 @@ deploy/ — Docker Compose + integration tests - Zero external deps where possible (Go: yaml.v3, Rust: tokio/hyper/serde/clap, TS: yaml) ## Test Coverage -- Go: 110 unit tests (main/listener: 24, policy: 10, middleware: 25, proxy: 47, audit: 4) -- Rust: 142 unit tests (main/listener: 23, policy: 15, middleware: 41, proxy: 47, handler: 10, audit: 4, transport: 2) -- TypeScript: 160 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 32, proxy: 34, policy: 10, handler: 13, shutdown: 5, transport: 5, audit: 4) -- Integration, per implementation: 53 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose) -- Quint: `make test-spec` runs the `spec/listener.qnt` `run` tests (instances `listener_locked`, `listener_unlocked`) and the `spec/router.qnt` `run` tests (instances `router`, `router_pre48`, `router_pre53`, `router_pre55`, `router_readonly`, whose `readonly*` tests run with `--match`, and `router_pre76`) +- Go: 112 unit tests (main/listener: 24, policy: 10, middleware: 25, proxy: 49, audit: 4) +- Rust: 144 unit tests (main/listener: 23, policy: 15, middleware: 41, proxy: 48, handler: 11, audit: 4, transport: 2) +- TypeScript: 163 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 32, proxy: 35, policy: 10, handler: 15, shutdown: 5, transport: 5, audit: 4) +- Integration, per implementation: 60 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose) +- Quint: `make test-spec` runs the `spec/listener.qnt` `run` tests (instances `listener_locked`, `listener_unlocked`) and the `spec/router.qnt` `run` tests (instances `router`, `router_pre48`, `router_pre53`, `router_pre55`, `router_readonly`, whose `readonly*` tests run with `--match`, `router_pre76` and `router_pre78`) ## Test Conventions - Go: stdlib `testing` package, `go test ./...` - Rust: `#[cfg(test)]` inline modules, `cargo test` - TypeScript: `node:test` framework, `npm run build && node --test dist/*.test.js` -- Integration: `make test-integration` (53 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose +- Integration: `make test-integration` (60 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose ## Contribution Workflow diff --git a/Makefile b/Makefile index d7293c3..ee94473 100644 --- a/Makefile +++ b/Makefile @@ -104,6 +104,7 @@ test-spec: $(QUINT) test $(ROUTER_SPEC) --main=router_pre55 $(QUINT) test $(ROUTER_SPEC) --main=router_readonly --match='^readonly.*Test$$' $(QUINT) test $(ROUTER_SPEC) --main=router_pre76 + $(QUINT) test $(ROUTER_SPEC) --main=router_pre78 verify-ts: $(QUINT) run $(SPEC) --max-steps=50 --invariants allInvariants --backend typescript diff --git a/README.md b/README.md index f3deff5..34f9fbf 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Key features: - **Policy-driven**: Per-service YAML policies, selected by image name, control images, volumes, flags, and env vars. The listening socket is the trust boundary: every caller of a socket can use every policy behind it (see [Trust model](#trust-model)) - **Middleware pipeline**: 4 validation gates + 1 config mutator chain - **Default-deny router**: Only explicitly allowed endpoints pass through -- **Formally verified**: [Quint](https://quint-lang.org/) specification: 9 request-handling invariants, 6 listener invariants, and router properties for `--readonly`, percent-encoded paths, empty segments, exec, attach and the container lifecycle +- **Formally verified**: [Quint](https://quint-lang.org/) specification: 9 request-handling invariants, 6 listener invariants, and router properties for `--readonly`, percent-encoded paths, empty segments, exec, attach, the read-only passthrough and the container lifecycle - **Audit logging**: JSON-structured logs for all requests and decisions - **Three implementations**: [Go](go/), [Rust](rs/), [TypeScript](ts/) — equal peer languages - **Minimal dependencies**: Zero external deps for Go, crate-based for Rust, npm for TypeScript @@ -100,9 +100,9 @@ All three implementations expose the same API surface, share the same [Quint spe | Language | Directory | Tests | Stack | |----------|-----------|-------|-------| -| Go | [go/](go/) | 110 unit + 53 integration + 15 socket | stdlib net/http + yaml.v3 | -| Rust | [rs/](rs/) | 142 unit + 53 integration + 15 socket | tokio, hyper, serde, clap | -| TypeScript | [ts/](ts/) | 160 unit (1 skipped) + 53 integration + 15 socket | Node 22 ESM, built-in http | +| Go | [go/](go/) | 112 unit + 60 integration + 15 socket | stdlib net/http + yaml.v3 | +| Rust | [rs/](rs/) | 144 unit + 60 integration + 15 socket | tokio, hyper, serde, clap | +| TypeScript | [ts/](ts/) | 163 unit (1 skipped) + 60 integration + 15 socket | Node 22 ESM, built-in http | ### Build All @@ -204,6 +204,7 @@ The chain runs only on container create. `--readonly` is not a gate: the router | HTTP Method | Path | Action | |-------------|------|--------| +| CONNECT | Any target | **DENIED** (checked first, before routing) | | Any | Request with an `Upgrade` header, or `Connection: upgrade` | **DENIED** (checked before routing) | | Not GET/HEAD | Any (with `--readonly`) | **DENIED** (checked first in the router) | | Any | Path containing `%` | **DENIED** (see below) | @@ -219,7 +220,7 @@ The chain runs only on container create. `--readonly` is not a gate: the router | POST | `/auth` | **DENIED** | | POST | `/build` | **DENIED** | | POST | `/commit` | **DENIED** | -| GET/HEAD | Any other path without `%` or `//` | Allowed (read-only) | +| GET/HEAD | Any other path without `%` or `//`, including `/_ping`, `/version`, `/info`, `/events` | Allowed (read-only) | | Other | Other | **DENIED** | Any request whose path contains a percent-encoded byte (`%`) is denied with 403 for every method, GET and HEAD included, because the daemon decodes the path before routing. The query string is not inspected, so filters such as `docker ps --filter …` still work. The Go implementation also denies paths that contain raw characters it must re-encode, such as non-ASCII bytes or `{`; the Docker CLI never sends these. A consequence is that networks whose names need percent-encoding (for example a space or `%`) cannot be inspected by name through the proxy; inspecting them by ID still works, and other network operations are denied regardless. @@ -228,6 +229,8 @@ Exec is matched on whole path segments, so containers named like `exec-runner` w Attach is matched the same way: an `attach` segment under `/containers` is denied for every method, so `attach-runner` is an ordinary name, while a container literally named `attach` cannot be reached by name. Protocol upgrades are refused outright: any request with an `Upgrade` header, or a `Connection` header that lists `upgrade`, gets 403 `protocol upgrade not allowed` before it is routed, whatever its path ([#76](https://github.com/ChainSafe/docker-socket-policy/issues/76)). Forwarding one would hand the caller a raw stream to the daemon. Every Docker endpoint that upgrades (attach, exec start, the build session) is denied anyway; the refusal keeps any path the router allows from becoming one. +`/_ping`, `/version`, `/info` and `/events` are ordinary reads: `GET` and `HEAD` pass, and every other method is denied with `endpoint is not allowed`, `OPTIONS` (a CORS preflight) included ([#78](https://github.com/ChainSafe/docker-socket-policy/issues/78)). `CONNECT` is refused with 403 `CONNECT is not allowed` before anything else, whatever its target or headers; the TypeScript proxy answers it on the raw socket, since Node never hands `CONNECT` to the request handler. + ## Configuration ### CLI Flags @@ -445,7 +448,7 @@ not at the socket. ## Formal Verification -This project includes a [Quint](https://quint-lang.org/) formal specification that models the security invariants as a state machine. Random-simulation verification runs 10,000 sampled traces of up to 100 steps each, checking all 9 invariants on every state transition. A second module, `spec/listener.qnt`, models listening-socket startup (group selection, existing-path checks, the single-instance lock) with 6 more invariants. A third module, `spec/router.qnt`, models the router's denies for writes under `--readonly` ([#75](https://github.com/ChainSafe/docker-socket-policy/issues/75)), percent-encoded paths ([#53](https://github.com/ChainSafe/docker-socket-policy/issues/53)), empty interior segments ([#55](https://github.com/ChainSafe/docker-socket-policy/issues/55)), exec ([#49](https://github.com/ChainSafe/docker-socket-policy/issues/49)) and attach ([#76](https://github.com/ChainSafe/docker-socket-policy/issues/76)), the `POST /containers/create` route, container-name extraction and the container-lifecycle branch, not the full routing table ([#24](https://github.com/ChainSafe/docker-socket-policy/issues/24), [#48](https://github.com/ChainSafe/docker-socket-policy/issues/48)). Its 9 properties are checked exhaustively over the methods `GET`, `HEAD`, `POST`, `DELETE` and every path of 1 to 3 segments over the model's word set; exec is denied by the router, not by a middleware gate. +This project includes a [Quint](https://quint-lang.org/) formal specification that models the security invariants as a state machine. Random-simulation verification runs 10,000 sampled traces of up to 100 steps each, checking all 9 invariants on every state transition. A second module, `spec/listener.qnt`, models listening-socket startup (group selection, existing-path checks, the single-instance lock) with 6 more invariants. A third module, `spec/router.qnt`, models the router's denies for writes under `--readonly` ([#75](https://github.com/ChainSafe/docker-socket-policy/issues/75)), percent-encoded paths ([#53](https://github.com/ChainSafe/docker-socket-policy/issues/53)), empty interior segments ([#55](https://github.com/ChainSafe/docker-socket-policy/issues/55)), exec ([#49](https://github.com/ChainSafe/docker-socket-policy/issues/49)) and attach ([#76](https://github.com/ChainSafe/docker-socket-policy/issues/76)), the GET/HEAD-only read passthrough ([#78](https://github.com/ChainSafe/docker-socket-policy/issues/78)), the `POST /containers/create` route, container-name extraction and the container-lifecycle branch, not the full routing table ([#24](https://github.com/ChainSafe/docker-socket-policy/issues/24), [#48](https://github.com/ChainSafe/docker-socket-policy/issues/48)). Its 10 properties are checked exhaustively over the methods `GET`, `HEAD`, `POST`, `DELETE` and every path of 1 to 3 segments over the model's word set; exec is denied by the router, not by a middleware gate. The CI pipeline runs verification on every push and PR. A violation blocks the build. diff --git a/deploy/test.sh b/deploy/test.sh index 92a1523..d05733e 100755 --- a/deploy/test.sh +++ b/deploy/test.sh @@ -4,8 +4,9 @@ # and the deny message on every expected proxy 403. # The proxy's RegistryGate, MountSourceGate, EnvFileGate, CmdGate, and # ContainerConfigMutator are all exercised; exec and attach are denied by the -# router, protocol upgrades by the handler (#76), and every write on a second -# proxy run with --readonly (#75). +# router, protocol upgrades by the handler (#76), non-read methods on the read +# endpoints by the router and CONNECT by the handler (#78), and every write on a +# second proxy run with --readonly (#75). set -e @@ -166,7 +167,7 @@ case "$rc" in ;; esac -# ─── Read-only endpoints (always allowed) ─────────────────────── +# ─── Read-only endpoints (GET and HEAD allowed, #78) ──────────── echo "--- Read-only endpoints ---" @@ -473,6 +474,42 @@ check_denied "GET /containers/*/attach/ws -> 403 (attach denied)" "$S" "$BODY" ' S=$(upgrade_get "$PROXY/_ping") check_denied "GET /_ping with Upgrade: websocket -> 403 (upgrade refused)" "$S" "$BODY" 'protocol upgrade not allowed' +# #78: the read endpoints admit only GET and HEAD. Before #78 an early router +# clause allowed /_ping, /version, /info and /events… for every method, so the +# daemon answered POST /_ping. CONNECT is refused by the handler before +# routing; before #78 the TypeScript proxy dropped it with no response. +# method_status METHOD URL [CURL_ARGS...] +method_status() { + : >"$BODY" + m="$1" + u="$2" + shift 2 + out=$(curl -s -o "$BODY" -w '%{http_code}' $TIMEOUT --unix-socket "$PROXY_SOCK" \ + -X "$m" "$@" "$u" 2>/dev/null || true) + echo "${out:-000}" +} + +S=$(method_status POST "$PROXY/_ping") +check_denied "POST /_ping -> 403 (read endpoint, GET/HEAD only)" "$S" "$BODY" 'endpoint POST /_ping is not allowed' + +S=$(method_status DELETE "$PROXY/version") +check_denied "DELETE /version -> 403 (read endpoint, GET/HEAD only)" "$S" "$BODY" 'endpoint DELETE /version is not allowed' + +S=$(method_status OPTIONS "$PROXY/_ping") +check_denied "OPTIONS /_ping -> 403 (CORS preflight denied)" "$S" "$BODY" 'endpoint OPTIONS /_ping is not allowed' + +S=$(method_status CONNECT "$PROXY/_ping") +check_denied "CONNECT /_ping -> 403 (CONNECT refused)" "$S" "$BODY" 'CONNECT is not allowed' + +S=$(method_status CONNECT "$PROXY/" --request-target localhost:80) +check_denied "CONNECT localhost:80 -> 403 (CONNECT refused, authority form)" "$S" "$BODY" 'CONNECT is not allowed' + +S=$(method_status HEAD "$PROXY/_ping" -I) +check "HEAD /_ping -> 200" "200" "$S" + +S=$(get_status "$PROXY/events?since=1&until=2") +check "GET /events (bounded) -> 200" "200" "$S" + # ─── Read-only mode (#75) ──────────────────────────────── # The same config behind --readonly. Every request whose method is not GET or diff --git a/go/internal/proxy/handler.go b/go/internal/proxy/handler.go index d6c5a94..15d0ea2 100644 --- a/go/internal/proxy/handler.go +++ b/go/internal/proxy/handler.go @@ -33,6 +33,16 @@ func NewHandler(router *Router, chain *middleware.Chain, auditLog *audit.Logger, } func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // Refuse CONNECT before anything else, whatever its target or headers: it + // asks for a tunnel, which the proxy never provides (#78). + if r.Method == http.MethodConnect { + const msg = "CONNECT is not allowed" + slog.Warn("denied", "method", r.Method, "path", r.RequestURI, "reason", msg) + h.auditLog.Deny(r.Method, r.RequestURI, msg, map[string]interface{}{"method": r.Method, "path": r.RequestURI}) + http.Error(w, msg, http.StatusForbidden) + return + } + // Refuse protocol upgrades before anything else: the reverse proxy would // relay a 101 and hand the caller a raw stream to the daemon (#76). if isUpgradeRequest(r.Header) { diff --git a/go/internal/proxy/handler_test.go b/go/internal/proxy/handler_test.go index 0f459b9..db12cd9 100644 --- a/go/internal/proxy/handler_test.go +++ b/go/internal/proxy/handler_test.go @@ -453,3 +453,78 @@ func TestHandlerRefusesProtocolUpgrade(t *testing.T) { t.Fatalf("protocol upgrade mismatches:\n%s", strings.Join(mismatches, "\n")) } } + +// #78: CONNECT is refused before the upgrade check and before routing, so the +// transport never sees it, in origin form and in authority form. Before #78 +// CONNECT /_ping matched the early read-endpoint allow and was forwarded. +func TestHandlerRefusesConnect(t *testing.T) { + dir := t.TempDir() + os.WriteFile(filepath.Join(dir, "default.yaml"), []byte("service_name: default\nallowed_image_prefixes:\n - scratch\n"), 0644) + policyManager, err := policy.NewManager(dir) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + logPath := filepath.Join(t.TempDir(), "audit.log") + auditLog, err := audit.NewLogger(logPath) + if err != nil { + t.Fatalf("NewLogger failed: %v", err) + } + defer auditLog.Close() + const refused = "CONNECT is not allowed" + + tests := []struct { + name string + target string + headers map[string]string + }{ + {"origin form", "/_ping", nil}, + {"authority form", "localhost:80", nil}, + {"with Upgrade", "/_ping", map[string]string{"Upgrade": "websocket", "Connection": "Upgrade"}}, + } + var mismatches []string + for _, tt := range tests { + rec := &recorderTransport{} + h := NewHandler(NewRouter(policyManager), middleware.NewChain(), auditLog, rec) + req := httptest.NewRequest("CONNECT", tt.target, nil) + for k, v := range tt.headers { + req.Header.Set(k, v) + } + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if got := strings.TrimSpace(w.Body.String()); w.Code != http.StatusForbidden || got != refused { + mismatches = append(mismatches, tt.name+": "+http.StatusText(w.Code)+" "+got) + } + if rec.lastRequest != nil { + mismatches = append(mismatches, tt.name+": forwarded") + } + } + + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("reading audit log: %v", err) + } + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + wantURIs := []string{"/_ping", "localhost:80", "/_ping"} + for i, wantURI := range wantURIs { + if i >= len(lines) { + mismatches = append(mismatches, "audit entry missing for "+wantURI) + continue + } + var entry struct { + Method string `json:"method"` + URI string `json:"uri"` + Decision string `json:"decision"` + Reason string `json:"reason"` + } + if err := json.Unmarshal([]byte(lines[i]), &entry); err != nil { + t.Fatalf("audit line %d: %v", i, err) + } + if entry.Method != "CONNECT" || entry.URI != wantURI || entry.Decision != "DENY" || entry.Reason != refused { + mismatches = append(mismatches, "audit entry: "+lines[i]) + } + } + + if len(mismatches) > 0 { + t.Fatalf("CONNECT mismatches:\n%s", strings.Join(mismatches, "\n")) + } +} diff --git a/go/internal/proxy/router.go b/go/internal/proxy/router.go index 3438dc6..8e3a43b 100644 --- a/go/internal/proxy/router.go +++ b/go/internal/proxy/router.go @@ -59,10 +59,6 @@ func (r *Router) Route(method, path string, body map[string]interface{}) *RouteR path = stripAPIVersion(path) - if path == "/_ping" || path == "/version" || path == "/info" || strings.HasPrefix(path, "/events") { - return &RouteResult{Action: ActionAllow} - } - if strings.HasPrefix(path, "/auth") { return &RouteResult{Action: ActionDeny, DenyMsg: "auth endpoint is not allowed"} } diff --git a/go/internal/proxy/router_test.go b/go/internal/proxy/router_test.go index e225b9e..1bab54a 100644 --- a/go/internal/proxy/router_test.go +++ b/go/internal/proxy/router_test.go @@ -795,3 +795,68 @@ allowed_image_prefixes: }) } } + +// TestRouteReadEndpointMethods is the cross-language parity guard for #78. +// +// /_ping, /version, /info and /events admit only GET and HEAD, through the +// read passthrough; every other method gets the default deny. Before #78 an +// early clause allowed them for every method, and /events as a prefix also +// matched /eventsxyz. Rows mirror the ping* runs in spec/router.qnt. +func TestRouteReadEndpointMethods(t *testing.T) { + m := newTestManager(t, map[string]string{ + "beacon.yaml": ` +service_name: beacon +allowed_image_prefixes: + - chainsafe/lodestar +`, + }) + r := NewRouter(m) + + tests := []struct { + method string + path string + want Action + wantMsg string + }{ + // pingPostDeniedTest (#78) + {"POST", "/_ping", ActionDeny, "endpoint POST /_ping is not allowed"}, + // pingDeleteDeniedTest (#78) + {"DELETE", "/_ping", ActionDeny, "endpoint DELETE /_ping is not allowed"}, + // #78: every read endpoint, every non-read method. + {"DELETE", "/version", ActionDeny, "endpoint DELETE /version is not allowed"}, + {"PUT", "/info", ActionDeny, "endpoint PUT /info is not allowed"}, + {"POST", "/events", ActionDeny, "endpoint POST /events is not allowed"}, + {"CONNECT", "/_ping", ActionDeny, "endpoint CONNECT /_ping is not allowed"}, + // #78: the daemon's CORS preflight, denied on purpose. + {"OPTIONS", "/_ping", ActionDeny, "endpoint OPTIONS /_ping is not allowed"}, + {"PATCH", "/_ping", ActionDeny, "endpoint PATCH /_ping is not allowed"}, + // A lowercase method is an extension method, not CONNECT: the router denies it. + {"connect", "/_ping", ActionDeny, "endpoint connect /_ping is not allowed"}, + // #78: the old /events prefix match. + {"POST", "/eventsxyz", ActionDeny, "endpoint POST /eventsxyz is not allowed"}, + // #78: the check runs after the API-version strip. + {"POST", "/v1.45/_ping", ActionDeny, "endpoint POST /_ping is not allowed"}, + // pingGetAllowedTest (#78) + {"GET", "/_ping", ActionAllow, ""}, + // pingHeadAllowedTest (#78) + {"HEAD", "/_ping", ActionAllow, ""}, + {"GET", "/version", ActionAllow, ""}, + {"HEAD", "/version", ActionAllow, ""}, + {"GET", "/info", ActionAllow, ""}, + {"HEAD", "/info", ActionAllow, ""}, + {"GET", "/events", ActionAllow, ""}, + {"HEAD", "/events", ActionAllow, ""}, + {"GET", "/v1.45/_ping", ActionAllow, ""}, + // #78 pin: a read on an unknown path stays on the passthrough. + {"GET", "/eventsxyz", ActionAllow, ""}, + } + for _, tt := range tests { + t.Run(tt.method+" "+tt.path, func(t *testing.T) { + got := r.Route(tt.method, tt.path, nil) + if got.Action != tt.want || got.DenyMsg != tt.wantMsg { + t.Errorf("Route(%s, %s) = %v/%q, want %v/%q", + tt.method, tt.path, got.Action, got.DenyMsg, tt.want, tt.wantMsg) + } + }) + } +} diff --git a/rs/src/handler.rs b/rs/src/handler.rs index d7c67f9..7e9eba2 100644 --- a/rs/src/handler.rs +++ b/rs/src/handler.rs @@ -6,7 +6,7 @@ use crate::transport::TransportError; use bytes::Bytes; use http_body_util::BodyExt; use http_body_util::Full; -use hyper::{header, Request, Response, StatusCode}; +use hyper::{header, Method, Request, Response, StatusCode}; use std::collections::HashMap; use std::sync::Arc; use tracing::{info, warn}; @@ -45,7 +45,19 @@ impl Handler { let headers = parts.headers.clone(); let version = parts.version; - // Refuse protocol upgrades before anything else: forwarding one would + // Refuse CONNECT before anything else, whatever its target or headers: + // it asks for a tunnel, which the proxy never provides (#78). + if parts.method == Method::CONNECT { + let msg = "CONNECT is not allowed"; + warn!("denied {} {}: {}", method, full_uri, msg); + self.audit.deny(&method, &full_uri, msg); + return Response::builder() + .status(StatusCode::FORBIDDEN) + .body(Full::new(Bytes::from(msg))) + .unwrap(); + } + + // Refuse protocol upgrades next: forwarding one would // hand the caller a raw stream to the daemon (#76). if is_upgrade_request(&headers) { let msg = "protocol upgrade not allowed"; @@ -627,4 +639,75 @@ mod tests { } assert!(mismatches.is_empty(), "mismatches:\n{}", mismatches.join("\n")); } + + /// #78: CONNECT is refused before the upgrade check and before routing, + /// so the transport never sees it, in origin form and in authority form. + /// Before #78 CONNECT /_ping matched the early read-endpoint allow and + /// was forwarded. + #[tokio::test] + async fn test_handler_refuses_connect() { + let log_path = std::env::temp_dir().join(format!( + "handler-connect-{}-{}.log", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let _ = std::fs::remove_file(&log_path); + let manager = Manager::from_map(std::collections::HashMap::new()); + let router = Arc::new(Router::new(manager)); + let audit = AuditLogger::new(log_path.to_str().unwrap()).unwrap(); + let captured_uri = Arc::new(std::sync::Mutex::new(None)); + let transport = UriRecordingTransport { + captured_uri: captured_uri.clone(), + }; + let handler = Handler::new(router, Chain::new(), audit, Box::new(transport)); + let refused = "CONNECT is not allowed"; + + let cases: [(&str, &str, &[(&str, &str)]); 3] = [ + ("origin form", "/_ping", &[]), + ("authority form", "localhost:80", &[]), + ( + "with Upgrade", + "/_ping", + &[("Upgrade", "websocket"), ("Connection", "Upgrade")], + ), + ]; + let mut mismatches = Vec::new(); + for (name, target, headers) in cases { + let mut req = Request::builder().method("CONNECT").uri(target); + for (k, v) in headers { + req = req.header(*k, *v); + } + let resp = handler.handle(req.body(Full::new(Bytes::new())).unwrap()).await; + let status = resp.status(); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + if status != StatusCode::FORBIDDEN || body != refused { + mismatches.push(format!("{}: got {} {:?}", name, status, String::from_utf8_lossy(&body))); + } + if captured_uri.lock().unwrap().take().is_some() { + mismatches.push(format!("{}: forwarded", name)); + } + } + + let contents = std::fs::read_to_string(&log_path).unwrap(); + let _ = std::fs::remove_file(&log_path); + let lines: Vec<&str> = contents.lines().collect(); + for (i, want_uri) in ["/_ping", "localhost:80", "/_ping"].iter().enumerate() { + let Some(line) = lines.get(i) else { + mismatches.push(format!("audit entry missing for {}", want_uri)); + continue; + }; + let entry: serde_json::Value = serde_json::from_str(line).unwrap(); + if entry["method"] != "CONNECT" + || entry["uri"] != *want_uri + || entry["decision"] != "DENY" + || entry["reason"] != refused + { + mismatches.push(format!("audit entry: {}", entry)); + } + } + assert!(mismatches.is_empty(), "mismatches:\n{}", mismatches.join("\n")); + } } diff --git a/rs/src/proxy.rs b/rs/src/proxy.rs index 6b9140a..88ce76b 100644 --- a/rs/src/proxy.rs +++ b/rs/src/proxy.rs @@ -56,19 +56,6 @@ impl Router { let path = strip_api_version(path); - // Read-only endpoints - if path == "/_ping" || path == "/version" || path == "/info" || path.starts_with("/events") { - return RouteResult { - action: Action::Allow, - service: None, - policy: None, - body: None, - container: None, - image: None, - deny_msg: None, - }; - } - // Denied endpoints if path.starts_with("/auth") { return deny("auth endpoint is not allowed"); @@ -949,6 +936,115 @@ mod tests { assert!(mismatches.is_empty(), "mismatches:\n{}", mismatches.join("\n")); } + /// Cross-language parity guard for #78. /_ping, /version, /info and + /// /events admit only GET and HEAD, through the read passthrough; every + /// other method gets the default deny. Before #78 an early clause allowed + /// them for every method, and /events as a prefix also matched + /// /eventsxyz. Rows mirror the ping* runs in spec/router.qnt. + #[test] + fn test_route_read_endpoint_methods() { + let router = Router::new(make_manager(vec!["alpine"])); + let cases = [ + // pingPostDeniedTest (#78) + ( + "POST", + "/_ping", + Action::Deny, + Some("endpoint POST /_ping is not allowed"), + ), + // pingDeleteDeniedTest (#78) + ( + "DELETE", + "/_ping", + Action::Deny, + Some("endpoint DELETE /_ping is not allowed"), + ), + // #78: every read endpoint, every non-read method. + ( + "DELETE", + "/version", + Action::Deny, + Some("endpoint DELETE /version is not allowed"), + ), + ("PUT", "/info", Action::Deny, Some("endpoint PUT /info is not allowed")), + ( + "POST", + "/events", + Action::Deny, + Some("endpoint POST /events is not allowed"), + ), + ( + "CONNECT", + "/_ping", + Action::Deny, + Some("endpoint CONNECT /_ping is not allowed"), + ), + // #78: the daemon's CORS preflight, denied on purpose. + ( + "OPTIONS", + "/_ping", + Action::Deny, + Some("endpoint OPTIONS /_ping is not allowed"), + ), + ( + "PATCH", + "/_ping", + Action::Deny, + Some("endpoint PATCH /_ping is not allowed"), + ), + // A lowercase method is an extension method, not CONNECT: the router denies it. + ( + "connect", + "/_ping", + Action::Deny, + Some("endpoint connect /_ping is not allowed"), + ), + // #78: the old /events prefix match. + ( + "POST", + "/eventsxyz", + Action::Deny, + Some("endpoint POST /eventsxyz is not allowed"), + ), + // #78: the check runs after the API-version strip. + ( + "POST", + "/v1.45/_ping", + Action::Deny, + Some("endpoint POST /_ping is not allowed"), + ), + // pingGetAllowedTest (#78) + ("GET", "/_ping", Action::Allow, None), + // pingHeadAllowedTest (#78) + ("HEAD", "/_ping", Action::Allow, None), + ("GET", "/version", Action::Allow, None), + ("HEAD", "/version", Action::Allow, None), + ("GET", "/info", Action::Allow, None), + ("HEAD", "/info", Action::Allow, None), + ("GET", "/events", Action::Allow, None), + ("HEAD", "/events", Action::Allow, None), + ("GET", "/v1.45/_ping", Action::Allow, None), + // #78 pin: a read on an unknown path stays on the passthrough. + ("GET", "/eventsxyz", Action::Allow, None), + ]; + let mut mismatches = Vec::new(); + for (method, path, want, want_msg) in cases { + let got = router.route(method, path, None); + if got.action != want || got.deny_msg.as_deref() != want_msg { + mismatches.push(format!( + "{} {}: got {:?}/{}, want {:?}/{}", + method, + path, + got.action, + got.deny_msg.as_deref().unwrap_or("-"), + want, + want_msg.unwrap_or("-") + )); + } + } + assert!(mismatches.is_empty(), "mismatches:\n{}", mismatches.join("\n")); + } + #[test] fn test_extract_container_name_skips_empty_segment() { for path in ["/containers/", "/containers//start"] { diff --git a/spec/README.md b/spec/README.md index 6361841..21fe9cc 100644 --- a/spec/README.md +++ b/spec/README.md @@ -9,7 +9,7 @@ This directory contains a [Quint](https://quint-lang.org/) formal specification | `docker_socket_policy.qnt` | Request-handling spec: policy types, state machine, endpoint routing table, 9 invariants (6 P0 / 3 P1), 6 attack scenario simulations | | `listener.qnt` | Listening-socket startup: flag/group selection, existing-path checks, single-instance lock, 6 invariants, one `run` test per design-table row. Instances `listener_locked` (Go, Rust) and `listener_unlocked` (TypeScript) | | `listener-design.md` | Design of the listening socket (dockerd parity) that `listener.qnt` models | -| `router.qnt` | The percent-encoded path deny, the empty interior segment deny, the segment-exact exec deny and `POST /containers/create` route, the segment-exact attach deny, container-name extraction and the container-lifecycle branch of the router only, not the full router. The daemon's percent-decoding as a fixed table. One `run` test per table row. Instances `router` (Go, TypeScript, and Rust after #48, #53, #49, #55 and #76), `router_pre48` (Rust's extraction rule before #48), `router_pre53` (Rust and TypeScript before #53, routing on the raw path), `router_pre55` (all three before #55, letting empty interior segments through), `router_readonly` (as `router`, with `--readonly`, #75) and `router_pre76` (all three before #76, letting attach through). Every instance includes the #49 exec and create rule, so the pre instances isolate one earlier rule each rather than being full snapshots | +| `router.qnt` | The percent-encoded path deny, the empty interior segment deny, the segment-exact exec deny and `POST /containers/create` route, the segment-exact attach deny, the GET/HEAD-only read passthrough with `/_ping` standing for the read endpoints, container-name extraction and the container-lifecycle branch of the router only, not the full router. The daemon's percent-decoding as a fixed table. One `run` test per table row. Instances `router` (Go, TypeScript, and Rust after #48, #53, #49, #55, #76 and #78), `router_pre48` (Rust's extraction rule before #48), `router_pre53` (Rust and TypeScript before #53, routing on the raw path), `router_pre55` (all three before #55, letting empty interior segments through), `router_readonly` (as `router`, with `--readonly`, #75) `router_pre76` (all three before #76, letting attach through) and `router_pre78` (all three before #78, allowing the read endpoints for every method). Every instance includes the #49 exec and create rule, so the pre instances isolate one earlier rule each rather than being full snapshots | ## How to Run @@ -34,7 +34,7 @@ quint run spec/listener.qnt --main=listener_locked --max-steps=30 --invariant al quint test spec/listener.qnt --main=listener_locked quint test spec/listener.qnt --main=listener_unlocked -# Router model: typecheck, run the table tests on all six instances +# Router model: typecheck, run the table tests on all seven instances quint typecheck spec/router.qnt quint test spec/router.qnt --main=router quint test spec/router.qnt --main=router_pre48 @@ -42,6 +42,7 @@ quint test spec/router.qnt --main=router_pre53 quint test spec/router.qnt --main=router_pre55 quint test spec/router.qnt --main=router_readonly --match='^readonly.*Test$' quint test spec/router.qnt --main=router_pre76 +quint test spec/router.qnt --main=router_pre78 # Formal model-checking via Apalache (exhaustive, requires Java) quint verify --max-steps=10 --invariants allInvariants spec/docker_socket_policy.qnt @@ -87,7 +88,7 @@ quint run spec/listener.qnt --main=listener_unlocked --max-steps=30 --invariant ### Router path parsing (`router.qnt`) -A path is a list of raw, still percent-encoded segments with the query string removed, so `/containers/` is `["containers", ""]`, `/containers//start` is `["containers", "", "start"]` and `/containers/%2F` is `["containers", "%2F"]`. With `READONLY` set, the read-only check runs before all of these and denies every method other than `GET` and `HEAD`, for every path (#75). The first check in `route` after it denies any path with an encoded segment, for every method (#53). The second denies any path with an empty segment before the last one, i.e. `//` in the path, for every method (#55); a single trailing slash, as in `/containers/`, is not interior. The third denies exec, for every method, as `denyExec` (#49): when the first segment is `exec` (the `/exec//…` namespace), or when the first segment is `containers` and any later segment is exactly `exec`. A segment that only starts with or contains `exec`, such as `exec-runner`, is not matched. The fourth denies attach, for every method (#76): when the first segment is `containers` and any later segment is exactly `attach`; `attach-runner` is a name like any other. The fifth routes `POST` with exactly the segments `["containers", "create"]` to `routeCreate`; a longer path falls through. After these, the second segment is a container name unless it is empty or one of `create`, `json`, `exec`. `HEAD` is routed like `GET` (#55). Every property checks every method in `GET`, `HEAD`, `POST`, `DELETE` against every path of 1 to 3 segments: +A path is a list of raw, still percent-encoded segments with the query string removed, so `/containers/` is `["containers", ""]`, `/containers//start` is `["containers", "", "start"]` and `/containers/%2F` is `["containers", "%2F"]`. With `READONLY` set, the read-only check runs before all of these and denies every method other than `GET` and `HEAD`, for every path (#75). The first check in `route` after it denies any path with an encoded segment, for every method (#53). The second denies any path with an empty segment before the last one, i.e. `//` in the path, for every method (#55); a single trailing slash, as in `/containers/`, is not interior. With `ALLOW_ANY_METHOD_PING` set, `router_pre78` allows `/_ping` for every method between the second and third checks, as the routers did before #78. The third denies exec, for every method, as `denyExec` (#49): when the first segment is `exec` (the `/exec//…` namespace), or when the first segment is `containers` and any later segment is exactly `exec`. A segment that only starts with or contains `exec`, such as `exec-runner`, is not matched. The fourth denies attach, for every method (#76): when the first segment is `containers` and any later segment is exactly `attach`; `attach-runner` is a name like any other. The fifth routes `POST` with exactly the segments `["containers", "create"]` to `routeCreate`; a longer path falls through. After these, the second segment is a container name unless it is empty or one of `create`, `json`, `exec`. `HEAD` is routed like `GET` (#55). Every property checks every method in `GET`, `HEAD`, `POST`, `DELETE` against every path of 1 to 3 segments: - `lifecycleOnlyTargetsRealNames` holds when each lifecycle allow (`allowKnown` or `allowUnknown`) targets a non-empty, non-reserved name. - `routerSeesWhatDaemonSees` holds when every request the router does not deny reads the same to the daemon: decoding leaves its path unchanged. The daemon's decoding is the table `DAEMON_DECODING`: `%2F` and `%2f` decode to `["", ""]` (a `/` splits the segment), `%6A%73%6F%6E` to `["json"]`, and `beacon%2Fstart` to `["beacon", "start"]`. @@ -97,9 +98,10 @@ A path is a list of raw, still percent-encoded segments with the query string re - `emptyInteriorDenied` holds when every path with an empty segment before the last one is denied. - `emptySegmentsHideNoExec` holds when every path that is an exec path once its empty segments are dropped is denied. - `attachNeverAllowed` holds when every path whose first segment is `containers`, with `attach` at some later segment, is denied. Like `execNeverAllowed`, it writes the positions out instead of calling `isAttachPath`. +- `readPassthroughOnlyForReads` holds when every request routed to the read passthrough (`allowRead`) is a `GET` or a `HEAD` (#78). It does not mention `ALLOW_ANY_METHOD_PING`, so `router_pre78` fails it. - `readonlyAllowsOnlyReads` holds when every request whose method is not `GET` or `HEAD` is denied, for every path. It is the read-only claim and holds only with `READONLY = true`; the router's read-only check, which runs before every other check, denies such requests (#75). -On `router`, `soundTest` asserts `lifecycleOnlyTargetsRealNames`, `percentSoundTest` asserts `routerSeesWhatDaemonSees`, `execSoundTest` asserts `execNeverAllowed`, `execPrefixSoundTest` asserts `execPrefixNamesRoute`, `createExactSoundTest` asserts `createOnlyExact`, `emptyInteriorSoundTest` asserts `emptyInteriorDenied`, `emptySegmentsHideNoExecTest` asserts `emptySegmentsHideNoExec` and `attachSoundTest` asserts `attachNeverAllowed`. +On `router`, `soundTest` asserts `lifecycleOnlyTargetsRealNames`, `percentSoundTest` asserts `routerSeesWhatDaemonSees`, `execSoundTest` asserts `execNeverAllowed`, `execPrefixSoundTest` asserts `execPrefixNamesRoute`, `createExactSoundTest` asserts `createOnlyExact`, `emptyInteriorSoundTest` asserts `emptyInteriorDenied`, `emptySegmentsHideNoExecTest` asserts `emptySegmentsHideNoExec`, `attachSoundTest` asserts `attachNeverAllowed` and `readPassthroughSoundTest` asserts `readPassthroughOnlyForReads`. | Row (`run Test`) | Request | Outcome | Instances | |-----|---------|---------|-----------| @@ -153,6 +155,15 @@ Rows for the attach deny ([#76](https://github.com/ChainSafe/docker-socket-polic | `attachHeadDenied` | `HEAD /containers/mycontainer/attach` | deny | | `attachNamedContainerDenied` | `GET /containers/attach/ws` | deny (a container literally named `attach`) | +Rows for the read endpoints ([#78](https://github.com/ChainSafe/docker-socket-policy/issues/78)). `/_ping` stands for `/_ping`, `/version`, `/info` and `/events`. The implementations deny the others with the default `endpoint is not allowed`: + +| Row (`run Test`) | Request | Outcome | Instances | +|-----|---------|---------|-----------| +| `pingPostDenied` | `POST /_ping` | deny | `router` | +| `pingDeleteDenied` | `DELETE /_ping` | deny | `router` | +| `pingGetAllowed` | `GET /_ping` | allow (read) | all | +| `pingHeadAllowed` | `HEAD /_ping` | allow (read) | all | + Rows for `--readonly` ([#75](https://github.com/ChainSafe/docker-socket-policy/issues/75)), all on `router_readonly`, which also runs `readonlySoundTest` (asserts `readonlyAllowsOnlyReads`). The implementations deny these with `read-only mode: requests are not allowed`: | Row (`run Test`) | Request | Outcome | @@ -165,7 +176,7 @@ Rows for `--readonly` ([#75](https://github.com/ChainSafe/docker-socket-policy/i The rows marked "all" above run on every `READONLY = false` instance; `router_readonly` runs only its own `readonly*` tests (`--match='^readonly.*Test$'`), since with the flag on the shared write rows are denied. -Each implementation's router tests use the same row names in comments (added with the #48, #53, #49, #55 and #76 fixes). `router_pre48` runs `pre48UnsoundTest`, which asserts that `lifecycleOnlyTargetsRealNames` fails and that both `emptyName*Denied` requests are allowed as an unknown container ([#48](https://github.com/ChainSafe/docker-socket-policy/issues/48)). `router_pre53` runs `pre53UnsoundTest`, which asserts that `routerSeesWhatDaemonSees` fails, that `DELETE /containers/%2F` and `POST /containers/%2F/start` are allowed as an unknown container, and that `GET /containers/%2F` is allowed through the passthrough. `router_pre55` runs `pre55UnsoundTest`, which asserts that `emptyInteriorDenied` fails and that `GET //exec/abc` is allowed through the passthrough: its first segment is empty, not `exec`, so the exec check misses it. `router_pre76` runs `pre76UnsoundTest`, which asserts that `attachNeverAllowed` fails and that `GET /containers/mycontainer/attach` is allowed through the passthrough ([#76](https://github.com/ChainSafe/docker-socket-policy/issues/76)). +Each implementation's router tests use the same row names in comments (added with the #48, #53, #49, #55, #76 and #78 fixes). `router_pre48` runs `pre48UnsoundTest`, which asserts that `lifecycleOnlyTargetsRealNames` fails and that both `emptyName*Denied` requests are allowed as an unknown container ([#48](https://github.com/ChainSafe/docker-socket-policy/issues/48)). `router_pre53` runs `pre53UnsoundTest`, which asserts that `routerSeesWhatDaemonSees` fails, that `DELETE /containers/%2F` and `POST /containers/%2F/start` are allowed as an unknown container, and that `GET /containers/%2F` is allowed through the passthrough. `router_pre55` runs `pre55UnsoundTest`, which asserts that `emptyInteriorDenied` fails and that `GET //exec/abc` is allowed through the passthrough: its first segment is empty, not `exec`, so the exec check misses it. `router_pre76` runs `pre76UnsoundTest`, which asserts that `attachNeverAllowed` fails and that `GET /containers/mycontainer/attach` is allowed through the passthrough ([#76](https://github.com/ChainSafe/docker-socket-policy/issues/76)). `router_pre78` runs `pre78UnsoundTest`, which asserts that `readPassthroughOnlyForReads` fails and that `POST /_ping` and `DELETE /_ping` are allowed as reads ([#78](https://github.com/ChainSafe/docker-socket-policy/issues/78)). ### Modeling Notes @@ -182,13 +193,14 @@ Two invariants are structurally tautological within the Quint model — they can - **The query string is outside the model.** Paths are segment lists with the query string already removed, so the model says nothing about `%` in the query. The implementations never inspect the query string for this rule; encoded queries such as `?filters=%7B…%7D` are routine and must still be forwarded. That is pinned by the language handler tests and the integration tests, not by the model. - **GET is covered.** The percent check runs before the GET passthrough, and `routerSeesWhatDaemonSees` ranges over `GET` as well as `POST` and `DELETE`. On `router_pre53` the passthrough lets `GET /containers/%2F` through, and the property fails for it too. - **No instance models Go before #53.** Go decoded before routing, which is `route(m, decodePath(segs))`. With this table that agrees with the daemon by construction, so `routerSeesWhatDaemonSees` cannot express Go's risk: decoding that differs from the daemon's, such as encoded version segments ([#57](https://github.com/ChainSafe/docker-socket-policy/issues/57)). Go's change is covered by the language handler tests. -- **`router.qnt` is not the full router.** `route` models the read-only check, which runs first in all three routers, the percent check, the empty interior segment check, the exec check, the attach check, the exact `POST /containers/create` route and the container-lifecycle branch, in that order. It leaves out the API-version strip and the other checks that the routers run before the lifecycle branch, such as the read-only endpoints, auth, and the build and commit denials, as well as `POST /images/create` and the endpoints after the lifecycle branch. For paths that those checks catch, the model's outcome can differ from what an implementation does: `GET /build` is `allowRead` in the model, for example, while the routers deny it as `build is not allowed`. The `images` rows of #49 (`GET /images/exec` allowed, `POST /images/create/extra` denied) are therefore language-only rows, as are `GET /containers/myexec/json` and `DELETE /containers/executor`: the model's words are atoms, so it cannot express a name that merely contains `exec`. Only the properties and the table rows are claims about the code. +- **`router.qnt` is not the full router.** `route` models the read-only check, which runs first in all three routers, the percent check, the empty interior segment check, the exec check, the attach check, the exact `POST /containers/create` route and the container-lifecycle branch, in that order. It leaves out the API-version strip and the other checks that the routers run before the lifecycle branch, such as auth and the build and commit denials, as well as `POST /images/create` and the endpoints after the lifecycle branch. For paths that those checks catch, the model's outcome can differ from what an implementation does: `GET /build` is `allowRead` in the model, for example, while the routers deny it as `build is not allowed`. The `images` rows of #49 (`GET /images/exec` allowed, `POST /images/create/extra` denied) are therefore language-only rows, as are `GET /containers/myexec/json` and `DELETE /containers/executor`: the model's words are atoms, so it cannot express a name that merely contains `exec`. Only the properties and the table rows are claims about the code. - **Exec and create match whole segments (#49).** Before #49 the routers disagreed: Go matched `exec` only in the name position (`/containers/exec`), so `DELETE /containers/mycontainer/exec` went down the lifecycle branch and was allowed. Rust matched the substring `/exec` anywhere after `/containers/`, and TypeScript anywhere in the path, so Rust and TypeScript denied containers named `exec-runner` or `executor`. Neither Go nor Rust checked the `/exec/…` namespace, so both let `GET /exec//json` through the GET passthrough. Go also accepted `POST /containers/create/extra` as a container create. In the model the exec check runs before the create route, the lifecycle branch and the GET passthrough, so it denies every method, GET included: exec inspect shows the full command line. `reservedExecDeleteDenied` (`DELETE /containers/exec`) is decided by the exec check, as in all three implementations, and now asserts `denyExec`. `exec` stays in the reserved set, but no row, in the model or in the language tests, would catch it being dropped from there. - **The #49 properties are not vacuous.** None of them is shown on a separate instance; replacing the rule in `route` makes them fail. With a substring-style rule (a segment that starts with `exec`, modeled as the set `exec`, `exec-runner`), `execPrefixSoundTest` and the three `execPrefixName*Allowed` rows fail. With Go's rule before #49 (`exec` only as the second segment under `containers`), `execSoundTest` and the four `exec*Denied` rows fail. `execSubpathPostDenied` and `execNamespacePostDenied` fail there only because the outcome is the default `deny`, not `denyExec`: the request is still refused, as before #49, but not by the exec check. With Go's prefix match for create, `createExactSoundTest` and `createSubpathDenied` fail. `execNeverAllowed` writes out the exec positions itself instead of calling `isExecPath`, so that a narrower `isExecPath` cannot narrow the property with it. - **HEAD is routed like GET (#55).** `METHODS` is `GET`, `HEAD`, `POST`, `DELETE`. Before #55 the lifecycle branches disagreed on `HEAD /containers/x`: Go and TypeScript fell through to the GET/HEAD passthrough and allowed it, while Rust denied it. All three now allow it like `GET`, and the model's lifecycle branch allows `GET` and `HEAD` alike. - **Empty interior segments are denied (#55).** Before #55, `GET //exec//json` (and `/v1.45//exec/…`) had an empty first segment, so the exec check did not match and the GET passthrough forwarded it. A live daemon answers such paths with a `301` to the clean path, so nothing leaked, but the router had allowed an exec path. `emptyInteriorDenied` states the rule, and on `router` it is close to a tautology, because `route` tests `hasEmptyInterior` directly. `emptySegmentsHideNoExec` states the security claim: a path that is an exec path once its empty segments are dropped, as the daemon's redirect does, is denied for every method. Both fail on `router_pre55`, and setting `router`'s `ALLOW_EMPTY_INTERIOR` to `true` makes `emptyInteriorSoundTest`, `emptySegmentsHideNoExecTest`, `emptyLeadingExecGetDenied` and `emptyInteriorGetDenied` fail. `router_pre48` keeps the rule off: with it on, `POST /containers//start` is denied before it reaches the name branch, and the instance could no longer show the #48 extraction bug. For the same reason, on `router` the empty-segment check now decides `emptyNameStartDenied`; the #48 extraction stays covered there by `emptyNameDeleteDenied` (a trailing slash) and on `router_pre48`. Dot segments (`/./exec/…`, `/x/../exec/…`) are not modelled and not denied yet (#70); the daemon redirects them to the clean path as it does for `//`. The implementations check for `//` in the path without its query, so `GET /containers/json?filters=a//b` is still forwarded; that is pinned by the TypeScript router test, since Go and Rust route on the path alone. - **`--readonly` is a router rule (#75).** Before #75 the flag reached only the middleware chain, and the handler runs the chain only on the create route, so lifecycle `POST`s, `DELETE /containers/{name}` and `POST /images/create` were forwarded in read-only mode. That was handler wiring, not routing, so there is no `pre75` instance: the `READONLY = false` instances are the behaviour without the flag, which before the fix was also the behaviour with it for every route but create. `readonlyAllowsOnlyReads` does not mention `READONLY`, so it is not vacuous: setting `router_readonly`'s `READONLY` to `false` makes `readonlySoundTest` and the three `readonly*Denied` rows fail. `METHODS` has no `PUT`; the implementations' tests cover it. - **Attach is a router rule; the upgrade refusal is not modelled (#76).** Before #76, `GET /containers/{name}/attach/ws` with `Upgrade: websocket` fell through to the GET passthrough, and Go relayed the daemon's `101`, giving the caller the container's stdin. The router now denies `attach` as a whole segment after `containers`, for every method, right after the exec check. As with `/containers/exec/…`, a container literally named `attach` is denied too (`attachNamedContainerDenied`). `attachNeverAllowed` fails on `router_pre76`, and setting `router`'s `ALLOW_ATTACH` to `true` makes `attachSoundTest` and three of the four `attach*` rows fail; `attachPostDenied` is a pin, since the lifecycle branch already denies `POST` when the last segment is not an action. The handler also refuses every request with an `Upgrade` header, or a `Connection` header listing `upgrade`, with `protocol upgrade not allowed`, before routing. That check is on headers, which the model does not have, so like the query string it is pinned by the language handler tests and the integration tests, not by the model. +- **The read endpoints admit only `GET` and `HEAD`; the `CONNECT` refusal is not modelled (#78).** Before #78 an early clause in all three routers allowed `/_ping`, `/version`, `/info` and any path starting with `/events` for every method, so `POST /_ping` and `DELETE /version` reached the daemon, and `/eventsxyz` matched too. The clause is gone: `GET` and `HEAD` on those paths reach the read passthrough as before, and every other method falls through to the default deny. `readPassthroughOnlyForReads` fails on `router_pre78`, and setting `router`'s `ALLOW_ANY_METHOD_PING` to `true` makes `readPassthroughSoundTest`, `pingPostDenied` and `pingDeleteDenied` fail. The handlers also refuse every `CONNECT` request with `CONNECT is not allowed` before routing, ahead of the upgrade check; `METHODS` has no `CONNECT`, so that is pinned by the language handler tests and the integration tests, not by the model. - **Listener fault bias.** `step` crashes an instance on 1 in 10 draws instead of half of all steps, so random runs actually interleave live instances. Every crash stays reachable from every phase, so the reachable state space is unchanged. ### Attack Scenarios Prevented by Invariants diff --git a/spec/router.qnt b/spec/router.qnt index d950125..536482c 100644 --- a/spec/router.qnt +++ b/spec/router.qnt @@ -5,11 +5,12 @@ // path deny that runs first in the router (#53), the empty interior segment // deny that follows it (#55), the segment-exact exec deny and // POST /containers/create route (#49), and the segment-exact attach deny -// (#76). Mirrors +// (#76), and that the read passthrough admits only GET and HEAD, with +// `/_ping` standing for the read endpoints (#78). Mirrors // `extract_container_name` and the lifecycle branch of `Router::route` in // rs/src/proxy.rs. This is not the full router: the API-version strip and // the other checks that run between the percent check and the lifecycle -// branch (build, commit, the read-only endpoints, auth) are left out, as is +// branch (build, commit, auth) are left out, as is // POST /images/create. Go (`extractContainerName`) and // TypeScript (`extractContainerName`) have the same lifecycle branch. // HEAD is routed like GET in all three (#55). @@ -49,6 +50,13 @@ // whose first segment is `containers` with `attach` as any later segment // is denied, for every method, right after the exec check. // +// ALLOW_ANY_METHOD_PING = true models all three before #78, where an early +// clause allowed `/_ping`, `/version`, `/info` and `/events…` for every +// method, right after the empty interior segment check. false models all +// three after the fix: those paths reach the GET/HEAD passthrough like any +// other read, and every other method is denied by default. `/_ping` stands +// for all four. +// // No instance models Go before #53. Go decoded before routing, which is // route(m, decodePath(segs)); with this table that agrees with the daemon // by construction, so the property cannot express Go's risk: decoding that @@ -61,6 +69,7 @@ module router_model { const ALLOW_EMPTY_INTERIOR: bool const READONLY: bool const ALLOW_ATTACH: bool + const ALLOW_ANY_METHOD_PING: bool // ─── Daemon decoding ──────────────────────────────────────────────── @@ -139,7 +148,8 @@ module router_model { outcome == "deny" or outcome == "denyExec" // The read-only check (#75) comes first, then the percent check (#53), - // then the empty interior segment check (#55). The exec check (#49) + // then the empty interior segment check (#55). Before #78 the read + // endpoints were allowed next, for every method. The exec check (#49) // and the attach check (#76) follow, before // the create route, the lifecycle branch and the GET passthrough, so it // denies every method. POST /containers/create matches only the exact @@ -148,6 +158,7 @@ module router_model { if (READONLY and not(method == "GET" or method == "HEAD")) "deny" else if (not(ALLOW_PERCENT) and hasEncodedSeg(segs)) "deny" else if (not(ALLOW_EMPTY_INTERIOR) and hasEmptyInterior(segs)) "deny" + else if (ALLOW_ANY_METHOD_PING and segs == ["_ping"]) "allowRead" else if (isExecPath(segs)) "denyExec" else if (not(ALLOW_ATTACH) and isAttachPath(segs)) "deny" else if (method == "POST" and segs == ["containers", "create"]) "routeCreate" @@ -167,7 +178,7 @@ module router_model { pure val WORDS = Set( "containers", "", "json", "create", "exec", "mycontainer", "beacon", "start", "stop", "%2F", "%2f", "%6A%73%6F%6E", "beacon%2Fstart", - "exec-runner", "abc", "extra", "attach", "ws", + "exec-runner", "abc", "extra", "attach", "ws", "_ping", ) pure val METHODS = Set("GET", "HEAD", "POST", "DELETE") @@ -256,15 +267,23 @@ module router_model { not(m == "GET" or m == "HEAD") implies isDenied(route(m, segs)) )) + // The read passthrough is taken only by GET and HEAD (#78): every other + // method on a path that no earlier rule allows is denied. It does not + // mention ALLOW_ANY_METHOD_PING, so that router_pre78 fails it. + pure val readPassthroughOnlyForReads: bool = + METHODS.forall(m => PATHS.forall(segs => + route(m, segs) == "allowRead" implies (m == "GET" or m == "HEAD") + )) + // ─── Tests: one run per table row ─────────────────────────────────── // // Rows whose outcome depends on none of ALLOW_EMPTY_NAME, ALLOW_PERCENT, - // ALLOW_EMPTY_INTERIOR and ALLOW_ATTACH run on every READONLY = false - // instance. The rows + // ALLOW_EMPTY_INTERIOR, ALLOW_ATTACH and ALLOW_ANY_METHOD_PING run on + // every READONLY = false instance. The rows // that do and the // property tests (soundTest, percentSoundTest, execSoundTest, // execPrefixSoundTest, createExactSoundTest, emptyInteriorSoundTest, - // attachSoundTest) live + // attachSoundTest, readPassthroughSoundTest) live // in `router`. `router_readonly` runs only its own `readonly*` tests // (`--match`): with READONLY = true the write rows here are denied. @@ -317,15 +336,24 @@ module router_model { run headNamedContainerAllowedTest = assert(route("HEAD", ["containers", "mycontainer"]) == "allowRead") + + // The read endpoints stay readable (#78). + + run pingGetAllowedTest = + assert(route("GET", ["_ping"]) == "allowRead") + + run pingHeadAllowedTest = + assert(route("HEAD", ["_ping"]) == "allowRead") } // ─── Instances ────────────────────────────────────────────────────────── -// Go, TypeScript, and Rust after #48, #53, #49, #55 and #76: an empty +// Go, TypeScript, and Rust after #48, #53, #49, #55, #76 and #78: an empty // segment is not a name, a percent-encoded path or one with an empty -// interior segment is denied, and attach is denied. +// interior segment is denied, attach is denied, and the read endpoints are +// read-only. module router { - import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = false, READONLY = false, ALLOW_ATTACH = false).* + import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = false, READONLY = false, ALLOW_ATTACH = false, ALLOW_ANY_METHOD_PING = false).* run emptyNameDeleteDeniedTest = assert(route("DELETE", ["containers", ""]) == "deny") @@ -407,6 +435,18 @@ module router { run attachSoundTest = assert(attachNeverAllowed) + + // The read endpoints admit only GET and HEAD (#78). Before #78 the early + // clause allowed them for every method. + + run pingPostDeniedTest = + assert(route("POST", ["_ping"]) == "deny") + + run pingDeleteDeniedTest = + assert(route("DELETE", ["_ping"]) == "deny") + + run readPassthroughSoundTest = + assert(readPassthroughOnlyForReads) } // Rust before #48: `/containers/` and `/containers//start` reach @@ -415,7 +455,7 @@ module router { // is off: with it on, `/containers//start` would be denied before it reached // the name branch, and the instance could not show #48. module router_pre48 { - import router_model(ALLOW_EMPTY_NAME = true, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = true, READONLY = false, ALLOW_ATTACH = false).* + import router_model(ALLOW_EMPTY_NAME = true, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = true, READONLY = false, ALLOW_ATTACH = false, ALLOW_ANY_METHOD_PING = false).* run pre48UnsoundTest = assert(and { @@ -429,7 +469,7 @@ module router_pre48 { // encoded name such as `%2F` counted as an unknown container and the // request was forwarded. The daemon decodes it and reads a different path. module router_pre53 { - import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = true, ALLOW_EMPTY_INTERIOR = false, READONLY = false, ALLOW_ATTACH = false).* + import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = true, ALLOW_EMPTY_INTERIOR = false, READONLY = false, ALLOW_ATTACH = false, ALLOW_ANY_METHOD_PING = false).* run pre53UnsoundTest = assert(and { @@ -445,7 +485,7 @@ module router_pre53 { // check missed it and the GET passthrough forwarded it. The daemon answers // with a 301 to the clean path, so nothing leaked, but the router allowed it. module router_pre55 { - import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = true, READONLY = false, ALLOW_ATTACH = false).* + import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = true, READONLY = false, ALLOW_ATTACH = false, ALLOW_ANY_METHOD_PING = false).* run pre55UnsoundTest = assert(and { @@ -459,7 +499,7 @@ module router_pre55 { // `--match='^readonly.*Test$'`; the shared write rows in router_model assume the // flag is off. module router_readonly { - import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = false, READONLY = true, ALLOW_ATTACH = false).* + import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = false, READONLY = true, ALLOW_ATTACH = false, ALLOW_ANY_METHOD_PING = false).* run readonlySoundTest = assert(readonlyAllowsOnlyReads) @@ -485,7 +525,7 @@ module router_readonly { // and Go relayed the 101, giving the caller the container's stdin. Other // flags as `router`. module router_pre76 { - import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = false, READONLY = false, ALLOW_ATTACH = true).* + import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = false, READONLY = false, ALLOW_ATTACH = true, ALLOW_ANY_METHOD_PING = false).* run pre76UnsoundTest = assert(and { @@ -493,3 +533,17 @@ module router_pre76 { route("GET", ["containers", "mycontainer", "attach"]) == "allowRead", }) } + +// All three before #78: an early clause allowed `/_ping`, `/version`, +// `/info` and `/events…` for every method, so `POST /_ping` and +// `DELETE /version` reached the daemon. Other flags as `router`. +module router_pre78 { + import router_model(ALLOW_EMPTY_NAME = false, ALLOW_PERCENT = false, ALLOW_EMPTY_INTERIOR = false, READONLY = false, ALLOW_ATTACH = false, ALLOW_ANY_METHOD_PING = true).* + + run pre78UnsoundTest = + assert(and { + not(readPassthroughOnlyForReads), + route("POST", ["_ping"]) == "allowRead", + route("DELETE", ["_ping"]) == "allowRead", + }) +} diff --git a/ts/src/handler.test.ts b/ts/src/handler.test.ts index 2c7af30..838353f 100644 --- a/ts/src/handler.test.ts +++ b/ts/src/handler.test.ts @@ -3,9 +3,9 @@ import assert from "node:assert/strict"; import { mkdtempSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { Readable } from "node:stream"; +import { Readable, type Duplex } from "node:stream"; import { createServer, request, type IncomingMessage, type ServerResponse } from "node:http"; -import type { AddressInfo } from "node:net"; +import { connect, type AddressInfo } from "node:net"; import { Handler } from "./handler.js"; import { Chain } from "./middleware.js"; import { Router } from "./proxy.js"; @@ -304,4 +304,86 @@ describe("Handler", () => { await new Promise((resolve) => server.close(resolve)); } }); + + // #78: CONNECT is refused before the upgrade check and before routing, so + // the transport never sees it, in origin form and in authority form. Before + // #78 CONNECT /_ping matched the early read-endpoint allow and was forwarded. + it("refuses CONNECT", async () => { + const dir = makeEnv(defaultConfig); + const { handler, transport } = newHandler(dir); + const refused = "CONNECT is not allowed"; + const cases: [string, string, Record][] = [ + ["origin form", "/_ping", {}], + ["authority form", "localhost:80", {}], + ["with Upgrade", "/_ping", { upgrade: "websocket", connection: "Upgrade" }], + ]; + const mismatches: string[] = []; + for (const [name, target, headers] of cases) { + const { res, recorder } = makeResponse(); + await handler.handle(makeRequest("CONNECT", target, undefined, headers), res); + if (recorder.statusCode !== 403 || recorder.body !== refused) { + mismatches.push(`${name}: got ${recorder.statusCode} ${JSON.stringify(recorder.body)}`); + } + if (transport.lastRequest) mismatches.push(`${name}: forwarded`); + transport.lastRequest = undefined; + } + + const lines = readFileSync(join(dir, "audit.log"), "utf-8").trim().split("\n"); + ["/_ping", "localhost:80", "/_ping"].forEach((uri, i) => { + const entry = JSON.parse(lines[i] ?? "{}") as Record; + if (entry.method !== "CONNECT" || entry.uri !== uri || entry.decision !== "DENY" || entry.reason !== refused) { + mismatches.push(`audit entry ${i}: ${lines[i]}`); + } + }); + assert.deepEqual(mismatches, []); + }); + + // #78, TS-only: Node never passes CONNECT to the request handler. With no + // "connect" listener it destroys the socket, so the caller got no response + // and nothing was audited. The proxy wires Handler.refuseConnect to it. + it("refuses CONNECT through a real http.Server", async () => { + const dir = makeEnv(defaultConfig); + const { handler, transport } = newHandler(dir); + const server = createServer((req, res) => void handler.handle(req, res)); + server.on("connect", (req: IncomingMessage, socket: Duplex) => handler.refuseConnect(req, socket)); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + const send = (target: string) => + new Promise((resolve, reject) => { + const sock = connect(port, "127.0.0.1"); + let got = ""; + sock.on("data", (c: Buffer) => (got += c.toString())); + sock.on("close", () => resolve(got)); + sock.on("error", reject); + sock.setTimeout(2000, () => sock.destroy(new Error("no response within 2s"))); + sock.write(`CONNECT ${target} HTTP/1.1\r\nHost: localhost:80\r\n\r\n`); + }); + try { + const mismatches: string[] = []; + for (const target of ["localhost:80", "/_ping"]) { + const got = await send(target); + if (!got.startsWith("HTTP/1.1 403 Forbidden\r\n") || !got.endsWith("\r\n\r\nCONNECT is not allowed")) { + mismatches.push(`${target}: got ${JSON.stringify(got)}`); + } + } + if (transport.lastRequest) mismatches.push("forwarded"); + const log = readFileSync(join(dir, "audit.log"), { encoding: "utf-8", flag: "a+" }); + const lines = log.trim().split("\n"); + ["localhost:80", "/_ping"].forEach((uri, i) => { + const entry = JSON.parse(lines[i] || "{}") as Record; + if ( + entry.method !== "CONNECT" || + entry.uri !== uri || + entry.decision !== "DENY" || + entry.reason !== "CONNECT is not allowed" + ) { + mismatches.push(`audit entry ${i}: ${lines[i]}`); + } + }); + assert.deepEqual(mismatches, []); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => server.close(resolve)); + } + }); }); diff --git a/ts/src/handler.ts b/ts/src/handler.ts index 58d0a6a..314750e 100644 --- a/ts/src/handler.ts +++ b/ts/src/handler.ts @@ -1,5 +1,6 @@ import { randomBytes } from "node:crypto"; import type { IncomingMessage, ServerResponse } from "node:http"; +import type { Duplex } from "node:stream"; import { AuditLogger } from "./audit.js"; import { Chain } from "./middleware.js"; import { Action, Router } from "./proxy.js"; @@ -18,7 +19,18 @@ export class Handler { const method = req.method ?? "GET"; const path = req.url ?? "/"; - // Refuse protocol upgrades before anything else: forwarding one would + // Refuse CONNECT before anything else, whatever its target or headers + // (#78). Node's http.Server never delivers CONNECT here; refuseConnect + // answers it on the server's "connect" event. + if (method === "CONNECT") { + console.warn(`[${requestId}] DENIED ${method} ${path}: ${CONNECT_REFUSED}`); + this.audit.deny(method, path, CONNECT_REFUSED); + res.writeHead(403); + res.end(CONNECT_REFUSED); + return; + } + + // Refuse protocol upgrades next: forwarding one would // hand the caller a raw stream to the daemon (#76). The server registers // no "upgrade" listener, so Node delivers these requests here. if (isUpgradeRequest(req)) { @@ -72,8 +84,29 @@ export class Handler { this.audit.allow(method, path); await this.transport.forward(req, res, bodyBytes.length > 0 ? bodyBytes : undefined); } + + // Answers a CONNECT request with 403 on the raw socket (#78). Node emits + // CONNECT as the server's "connect" event, never as a request, and without + // a listener it destroys the socket, so the caller got no response and + // nothing was audited. + refuseConnect(req: IncomingMessage, socket: Duplex): void { + socket.on("error", () => {}); + const method = req.method ?? "CONNECT"; + const target = req.url ?? ""; + console.warn(`[${randomBytes(8).toString("hex")}] DENIED ${method} ${target}: ${CONNECT_REFUSED}`); + this.audit.deny(method, target, CONNECT_REFUSED); + const resp = + "HTTP/1.1 403 Forbidden\r\n" + + "Content-Type: text/plain; charset=utf-8\r\n" + + `Content-Length: ${Buffer.byteLength(CONNECT_REFUSED)}\r\n` + + "Connection: close\r\n\r\n" + + CONNECT_REFUSED; + socket.end(resp, () => socket.destroy()); + } } +const CONNECT_REFUSED = "CONNECT is not allowed"; + // An Upgrade header, or a Connection header that lists the upgrade token. function isUpgradeRequest(req: IncomingMessage): boolean { if (req.headers.upgrade !== undefined) return true; diff --git a/ts/src/index.ts b/ts/src/index.ts index 88fd153..e25c99d 100644 --- a/ts/src/index.ts +++ b/ts/src/index.ts @@ -77,6 +77,8 @@ const server = createServer((req, res) => { }); }); +server.on("connect", (req, socket) => handler.refuseConnect(req, socket)); + // A bind failure leaves the process with no listener at all, so it is fatal. // This handler is scoped to the bind: net.Server also emits "error" for accept // failures (EMFILE and friends), and exiting on those would let any caller kill diff --git a/ts/src/proxy.test.ts b/ts/src/proxy.test.ts index 9807c2d..6901e0f 100644 --- a/ts/src/proxy.test.ts +++ b/ts/src/proxy.test.ts @@ -539,4 +539,57 @@ describe("Router", () => { } assert.deepEqual(mismatches, []); }); + + // Cross-language parity guard for #78. /_ping, /version, /info and /events + // admit only GET and HEAD, through the read passthrough; every other method + // gets the default deny. Before #78 an early clause allowed them for every + // method, and /events as a prefix also matched /eventsxyz. Rows mirror the + // ping* runs in spec/router.qnt. + it("allows only GET and HEAD on the read endpoints", () => { + const cases: [string, string, Action, string | undefined][] = [ + // pingPostDeniedTest (#78) + ["POST", "/_ping", Action.Deny, "endpoint POST /_ping is not allowed"], + // pingDeleteDeniedTest (#78) + ["DELETE", "/_ping", Action.Deny, "endpoint DELETE /_ping is not allowed"], + // #78: every read endpoint, every non-read method. + ["DELETE", "/version", Action.Deny, "endpoint DELETE /version is not allowed"], + ["PUT", "/info", Action.Deny, "endpoint PUT /info is not allowed"], + ["POST", "/events", Action.Deny, "endpoint POST /events is not allowed"], + ["CONNECT", "/_ping", Action.Deny, "endpoint CONNECT /_ping is not allowed"], + // #78: the daemon's CORS preflight, denied on purpose. + ["OPTIONS", "/_ping", Action.Deny, "endpoint OPTIONS /_ping is not allowed"], + ["PATCH", "/_ping", Action.Deny, "endpoint PATCH /_ping is not allowed"], + // A lowercase method is an extension method, not CONNECT: the router denies it. + ["connect", "/_ping", Action.Deny, "endpoint connect /_ping is not allowed"], + // #78: the old /events prefix match. + ["POST", "/eventsxyz", Action.Deny, "endpoint POST /eventsxyz is not allowed"], + // #78: the check runs after the API-version strip. + ["POST", "/v1.45/_ping", Action.Deny, "endpoint POST /_ping is not allowed"], + // pingGetAllowedTest (#78) + ["GET", "/_ping", Action.Allow, undefined], + // pingHeadAllowedTest (#78) + ["HEAD", "/_ping", Action.Allow, undefined], + ["GET", "/version", Action.Allow, undefined], + ["HEAD", "/version", Action.Allow, undefined], + ["GET", "/info", Action.Allow, undefined], + ["HEAD", "/info", Action.Allow, undefined], + ["GET", "/events", Action.Allow, undefined], + ["HEAD", "/events", Action.Allow, undefined], + ["GET", "/v1.45/_ping", Action.Allow, undefined], + // #78 pin: a read on an unknown path stays on the passthrough. + ["GET", "/eventsxyz", Action.Allow, undefined], + // #78, TS-only: the TS router receives the raw target, query included. + ["POST", "/events?since=1", Action.Deny, "endpoint POST /events is not allowed"], + ]; + const mismatches: string[] = []; + for (const [method, path, want, wantMsg] of cases) { + const r = router.route(method, path); + if (r.action !== want || r.denyMsg !== wantMsg) { + mismatches.push( + `${method} ${path}: got ${Action[r.action]}/${r.denyMsg ?? "-"}, want ${Action[want]}/${wantMsg ?? "-"}`, + ); + } + } + assert.deepEqual(mismatches, []); + }); }); diff --git a/ts/src/proxy.ts b/ts/src/proxy.ts index 959927e..63b6885 100644 --- a/ts/src/proxy.ts +++ b/ts/src/proxy.ts @@ -41,11 +41,6 @@ export class Router { if (cleanPath.includes("//")) return { action: Action.Deny, denyMsg: "empty path segment not allowed" }; path = stripAPIVersion(cleanPath); - // Read-only endpoints - if (path === "/_ping" || path === "/version" || path === "/info" || path.startsWith("/events")) { - return { action: Action.Allow }; - } - // Denied endpoints if (path.startsWith("/auth")) return { action: Action.Deny, denyMsg: "auth endpoint is not allowed" }; if (isExecPath(path)) return { action: Action.Deny, denyMsg: "exec is not allowed" };