-
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
Add allow list for HTTP methods/schemes/authorities #6401
Conversation
cc @pchickey |
I'm gonna move review over to @pchickey who's more in-depth with this stuff than I am, but it also looks like there's a CI failure you may be interested in. |
I'll look into the CI failure. "It worked on my machine"(tm) |
Ok, looks like this was incomptable with the changes in #6385 and I forgot to re-run the tests after a rebase. CI should now pass, ready for a real review. |
@@ -43,6 +43,18 @@ fn port_for_scheme(scheme: &Option<Scheme>) -> &str { | |||
} | |||
} | |||
|
|||
fn is_allowed(allow_list: &Vec<String>, value: String) -> bool { | |||
for allowed in allow_list.iter() { | |||
if allowed == "*" { |
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.
Do you want to follow the convention in wasi-experimental-http that uses "insecure:allow-all" to match everything?
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.
I think I will leave wild-card in for now. I'm not sure we want to imply wild-card == insecure, that seems to be up to the user of wasmtime
to determine what is secure or not in their environment. (or we should define this in the wasi-http spec)
Given discussion here: Closing this. |
This adds allow lists to HTTP which enable the creator of the wasm runtime to specify:
It also supports a wildcard value
*
which matches everything.Unit tests to validate the functionality are also included.