Repository navigation
atunnel: strip X-Ate-Assignment-Stale from upstream actor responses - #2410
Pancho Trujillo (panchoruy) wants to merge 1 commit into
Conversation
ce204e9 to
f4d3815
Compare
f4d3815 to
f44f1c9
Compare
| // Prevent untrusted actor responses from spoofing routing rejections | ||
| // to trigger cache evictions and stream restarts in the ingress router. | ||
| ModifyResponse: func(resp *http.Response) error { | ||
| resp.Header.Del(StaleAssignmentHeader) |
There was a problem hiding this comment.
Follow up: is this the only header that we need to strip?
There was a problem hiding this comment.
Hm that's a good point. From my exploration it seems no other headers like this one are being set at this time, but I interpret this question as whether we should eagerly strip "any headers" of this nature in the future as well. It would seem to me like the right thing to do to prevent exposing internal implementation details to untrusted actors. Lmk if you'd like this PR to strip all headers prefixed with "X-ate" or "ate" etc, or do it in a follow-up!
atunnel uses the X-Ate-Assignment-Stale response header when returning HTTP 421 (Misdirected Request) to distinguish an infrastructure-level routing rejection (when an actor is not active on this worker) from a 421 returned by the actor application itself. However, newActorProxy previously lacked response sanitization, allowing an untrusted actor workload inside the sandbox to emit this header and spoof an atunnel routing rejection to downstream routers or clients. Add a ModifyResponse hook to newActorProxy that deletes the header from actor responses, ensuring only atunnel's reject handler can emit it.
Head branch was pushed to by a user without write access
f44f1c9 to
6b70992
Compare
|
Rebased to head and updated, since e2e tests failure seems unrelated and it passed in an otherwise PR against my own fork: panchoruy#1 |
2 participants
Motivation
atunneldefinesStaleAssignmentHeader = "X-Ate-Assignment-Stale"to distinguish anatunnelinfrastructure-level routing rejection (HTTP 421Misdirected Request, returned when an actor is no longer active on that worker) from an HTTP 421 returned by the guest actor application itself (internal/atunnel/ingress.go#L45-L47).While
newActorProxyalready strips internal Substrate headers (ate-target-port,ate-target-actor) on the request leg into the sandbox, it lacked response sanitization on the return leg. As a result, an untrusted actor workload running inside the container could returnX-Ate-Assignment-Stale: trueand spoof an infrastructure routing rejection to downstream routers or clients.Changes
internal/atunnel/ingress.go: Added aModifyResponsehook tonewActorProxy()that removesX-Ate-Assignment-Stalefrom any response received from the actor upstream.internal/atunnel/ingress_test.go: Added unit testTestServeHTTPStripsStaleAssignmentHeaderFromActorasserting that even if the actor application returnsX-Ate-Assignment-Stale: true, the header is stripped before the response reaches the client.Testing
go test -race -count=1 ./internal/atunnel/..../hack/verify/boilerplate.sh./hack/verify/gofmt.sh./hack/verify/golangci-lint.shAI was used to assist in the development of these changes.