Skip to content

Add options to WasiCtx for toggling TCP and UDP on and off#7647

Merged
alexcrichton merged 3 commits into
bytecodealliance:mainfrom
rylev:toggle-udp-tcp
Dec 7, 2023
Merged

Add options to WasiCtx for toggling TCP and UDP on and off#7647
alexcrichton merged 3 commits into
bytecodealliance:mainfrom
rylev:toggle-udp-tcp

Conversation

@rylev
Copy link
Copy Markdown
Contributor

@rylev rylev commented Dec 6, 2023

This adds options to WasiCtx that allow disabling TCP and UDP wholesale. For TCP this prevents usage of binding, connecting, listening, and accepting while for UDP it prevents binding.

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
@rylev rylev requested a review from a team as a code owner December 6, 2023 16:19
@rylev rylev requested review from alexcrichton and removed request for a team December 6, 2023 16:19
Copy link
Copy Markdown
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Mind adding a test for this as well?

If you're up for it too it'd be nice to get this hooked up to -S/--wasi options on the CLI, for example -S tcp would enable TCP connections while -S udp would enable UDP. (similarly -S tcp=n would disable TCP.

Comment thread crates/wasi/src/preview2/ctx.rs Outdated
Comment thread crates/wasi/src/preview2/ctx.rs
@github-actions github-actions Bot added the wasi Issues pertaining to WASI label Dec 6, 2023
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
@rylev
Copy link
Copy Markdown
Contributor Author

rylev commented Dec 7, 2023

@alexcrichton I've addressed your feedback for everything but the test. Looking at the testing structure in wasmtime, I couldn't find a place where a test for this should clearly go. For example, I don't see a place where name lookups not being allowed is being tested. Any pointers on where to test this would be most appreciated.

@alexcrichton
Copy link
Copy Markdown
Member

Ah yes that's a good point that name lookups not being allowed isn't tested right now. Since that was added it'd become easier to add tests though so I think the best way to test this would be something along the lines of a cli_* tests in crates/test-programs/src/bin/*.rs and then a corresponding test function in this module. You could then pass -S ... flags to disable/enable the network and/or validate it's enabled/disabled by default.

Alternatively a preview2_* test could be added and this function could be modified to look at the name and configure disallowing options based on that.

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
@rylev
Copy link
Copy Markdown
Contributor Author

rylev commented Dec 7, 2023

@alexcrichton I added a test for TCP - I will add a test in #7648 for UDP since those tests would fail without the fixes there.

@alexcrichton alexcrichton added this pull request to the merge queue Dec 7, 2023
Merged via the queue into bytecodealliance:main with commit 576db01 Dec 7, 2023
@rylev rylev deleted the toggle-udp-tcp branch December 7, 2023 19:31
@vicspina
Copy link
Copy Markdown

vicspina commented Dec 8, 2023

Help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasi Issues pertaining to WASI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants