-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wasi-http: Filter forbidden headers #7538
wasi-http: Filter forbidden headers #7538
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be backported to Wasmtime 15 as well?
I'm wondering if this is the really the right place for this logic, particularly on incoming requests (as opposed to outgoing requests). I'm using the incoming request type in the context of detections (rather than in request handlers) where it's preferred that these headers aren't stripped because they may carry information indicating malicious behavior by a client. If they're stripped in a constructor before they can be inspected, detections will be potentially working with missing crucial information. I want to be able to compose detection logic with request handlers, which is why I'm using the same type. |
@elliottt Sorry for chiming in super late here, but @bacongobbler and I tripped over this code today and have some questions about it. For context: we're currently working on porting an ASP.NET Core app to WASI. This app happens to use Dapr, which in turn uses gRPC, which sets a Our questions:
I have other questions about how to use gRPC via wasi-http, but they're probably off topic here, so I'll ask those elsewhere. |
That was intentional. The list seemed pretty small, and the overlap between requests and responses seemed acceptable.
It's called in wasmtime/crates/wasi-http/src/types_impl.rs Line 131 in 8abaa75
That function ultimately bottoms out in the wasmtime/crates/wasi-http/src/types.rs Line 253 in 8abaa75
There's a test that ensures it's working by adding a custom forbidden header. wasmtime/crates/wasi-http/tests/all/main.rs Lines 204 to 208 in 8abaa75
It grew slightly over time, and was informed by some folks that @lukewagner coordinated with in Fastly, he might remember more context about where the full list came from. (edited to add the link to the actual call to |
Thanks, @elliottt! As I learn more about gRPC, I'm realizing that gRPC-on-wasi-http isn't possible given the current state of wasi-http, so we'll pursue the wasi-sockets route instead. |
FWIW, the list is in this doc, this section. Ideally I think this list would go into the wasi-http spec itself, but what's in the doc was the result of asking various HTTP folks (inside and outside Fastly). |
It's unfortunate that wasi-http can't support gRPC as currently specified! Concretely, it seems to me that setting a header |
We'll also need some way for the guest to require HTTP/2. And yeah, I'd definitely be in favor of making wasi-http gRPC-compatible! |
Yes, agreed on the goal of making wasi-http gRPC-compatible; that has been a goal since the beginning (and the reason we have The goal of putting |
I think the trailers issue and the "require H2" issue are somewhat orthogonal. The latter is because gRPC is defined to use HTTP/2 specifically. |
This will be impossible or interesting in JCO because of browser |
That's true, but a wasi-aockets-based solution would also be difficult or impossible. The grpc-web protocol exists specifically to handle the browser case, so presumably an app intended to be maximally portable would be prepared to fall back to that. |
Make sure that forbidden headers don't show up in either
incoming-request
oroutgoing-response
values. One place that this PR doesn't currently address is incoming trailers. Thefuture-trailers.get
method returns a newfields
resource every time it's called once it has resolved, and it's not clear where the best place to filter forbidden headers is in that case. I think the right fix for that would be to makefuture-trailers.get
method return a result exactly once, like our otherfuture-*.get
methods, enabling us to apply the filter when we create the new ownedfields
resource.