Stream: wasi

Topic: Fine-grained network policies

Ryan Levick (rylev) (Dec 18 2023 at 10:47):

@Badeend I really love how things are coming together in https://github.com/bytecodealliance/wasmtime/issues/7681 and https://github.com/bytecodealliance/wasmtime/issues/7694. How do you want to go about implementing this since the design (at least at a high-level) seems pretty clear? I'm keen on diving into this, but I don't want to step on toes or increase risk of duplicate work.

Dave Bakker (badeend) (Dec 18 2023 at 15:44):

I think the first step will be to introduce all the required extension points. I posted an idea for that in https://github.com/bytecodealliance/wasmtime/issues/7681#issuecomment-1860560055

Ryan Levick (rylev) (Dec 19 2023 at 11:16):

@Dave Bakker (badeend) your approach laid out here seems reasonable. Would you mind if I started on trying out the implementation?

Dave Bakker (badeend) (Dec 19 2023 at 11:33):

Yeah, sure. No problem!
Maybe start out with UDP and/or domain name lookup? I already started tinkering with TCP

Ryan Levick (rylev) (Dec 19 2023 at 11:38):

Cool. I’ll start with domain names then.

Ryan Levick (rylev) (Dec 19 2023 at 15:49):

https://github.com/bytecodealliance/wasmtime/pull/7705

Dave Bakker (badeend) (Dec 20 2023 at 15:24):

@Ryan Levick (rylev) I've spent a day trying to get wasi-io's InputStream & OutputStream working on top of tokio's AsyncRead & AsyncWrite, but getting that to work would require a lot more work than I'm willing to put in right now. So if you don't mind I'm going to continue on with custom traits:

#[async_trait]
pub trait TcpReader {
    fn try_read(&mut self, size: usize) -> io::Result<Bytes>;
    async fn readable(&mut self);
}

#[async_trait]
pub trait TcpWriter {
    fn try_write(&mut self, bytes: Bytes) -> io::Result<usize>;
    async fn writable(&mut self);
}

Ryan Levick (rylev) (Dec 20 2023 at 15:58):

Works for me! Just out of curiosity, do you imagine you would run into the same issues with the futures crate’s AsyncRead and AsyncWrite?

Dave Bakker (badeend) (Dec 20 2023 at 16:48):

Yeah, they're the same general design

Ryan Levick (rylev) (Jan 03 2024 at 13:57):

@Dave Bakker (badeend) Happy New Year - I wanted to check in with you on this. Are you still hoping to have a PR for TCP support similar to the one I have for IP name lookups? I was thinking about jumping into UDP soon.

Dave Bakker (badeend) (Jan 03 2024 at 14:25):

Happy new year to you too. :fireworks: I haven't worked on WASI at all the past week, but expect to continue somewhere this week

Dave Bakker (badeend) (Jan 11 2024 at 19:27):

@Ryan Levick (rylev) To keep you up to date on my progress, I've opened a draft PR. To my estimation it is about ~70% done.

Dave Bakker (badeend) (Jan 14 2024 at 21:32):

Alright, I'm done. I've updated the PR. Let me know what you think

Ryan Levick (rylev) (Jan 15 2024 at 16:26):

Looks great! I think we should merge this ASAP so that I can get in a PR for UDP.

Ryan Levick (rylev) (Jan 19 2024 at 17:14):

@Dave Bakker (badeend) I finally had time to sit down today and do the UDP work. It's not quite done, and I haven't actually tested it, but if you're curious for a preview, you can find it here: https://github.com/rylev/wasmtime/pull/4. I'll move the PR out of draft state early next week.

Dave Bakker (badeend) (Jan 19 2024 at 19:15):

Already looking great! :tada: I only skimmed the PR , but I have two thoughts so far:

1. You've implemented UdpSocketResource::inner as an Arc. This forces implementors to use a Mutex if they want to maintain additional state. Is there any way to get around this?
2. Perhaps we can merge send_data_to, send_data & await_writable into a single fn poll_send(&self, cx: &mut Context<'_>, buf: &[u8], target: SocketAddr) -> Poll<Result<usize>> similar to how tokio does it?

Dave Bakker (badeend) (Jan 19 2024 at 19:17):

and the same for receive.
That would be similar to TcpSocket::poll_accept

Ryan Levick (rylev) (Jan 22 2024 at 12:35):

You've implemented UdpSocketResource::inner as an Arc. This forces implementors to use a Mutex if they want to maintain additional state. Is there any way to get around this?

Yea, that's not ideal. Unfortunately, at least in the naive implementation I don't know if we can avoid it. The dyn UdpSocket lives both in the SocketResource, the IncomingDatagramStream, and the OutgoingDatagramStream. So in order to support multiple owners we need to use reference counting. We could of course change things to that the two stream types don't own a dyn UdpSocket but rather get some ownership over different handle types.

Perhaps we can merge send_data_to, send_data & await_writable into a single fn poll_send(&self, cx: &mut Context<'_>, buf: &[u8], target: SocketAddr) -> Poll<Result<usize>> similar to how tokio does it?

I thought the same about this and only avoided it since it's not quite a simple as the current naive implementation. I'll play around with this and the idea of different handle types discussed above, and we'll see what happens.

Ryan Levick (rylev) (Jan 22 2024 at 17:53):

@Dave Bakker (badeend) I unfortunately couldn't come up with a way to both signal readiness and allow for reading and writing data within the same function. Perhaps you have an idea for how to do this elegantly. In any case, it might be best to have you review https://github.com/rylev/wasmtime/pull/4 and if you're generally ok with the direction, we can merge it, and then start doing final tweaks before we get Alex to review the whole thing for merging into Wasmtime main.

Ryan Levick (rylev) (Jan 29 2024 at 16:02):

@Dave Bakker (badeend) I tried to do a rebase against the latest main and it was pretty painful. I'm thinking I might squash all the commits on the branch so that only one commit needs to be rebased. Would you object to this?

Dave Bakker (badeend) (Jan 29 2024 at 19:17):

Go ahead

Ryan Levick (rylev) (Mar 28 2024 at 08:59):

@Dave Bakker (badeend) I'd like to try to see if we can land all of this work. Unless you object I'll start breaking up https://github.com/bytecodealliance/wasmtime/pull/7705 (like you suggested here)

Dave Bakker (badeend) (Mar 28 2024 at 09:04):

I won't be able to work on WASI for at least a couple of weeks, so go ahead! :smile:

Ryan Levick (rylev) (Mar 28 2024 at 09:58):

Absolutely no worries! That's why there's one than one of us :-)

Ryan Levick (rylev) (Apr 02 2024 at 13:03):

Started this process with https://github.com/bytecodealliance/wasmtime/pull/8282

Last updated: Nov 22 2024 at 17:03 UTC