WebAssembly / wasi-sockets

WASI API proposal for managing sockets
243 stars 22 forks source link

TLS #104

Open badeend opened 2 months ago

badeend commented 2 months ago

Not intended to be merged (anytime soon)

badeend commented 1 month ago

@rvolosatovs Thanks for your feedback. I've been trying out different variations of your suggestions. Aside from some details, I think that overall they indeed yield a better solution. Would you mind reviewing the PR again? Basically, everything has changed, sorry about that ¯\(ツ)/¯ I've included some examples in the markdown file.

Remarks:

alexcrichton commented 1 month ago

I was reading over this today and overall this looks quite good to me. While not necessarily the "perfect" interface we might have hoped for type-wise I think it's a good reflection of what we can do today with the component model and uses what we've got well.

One other possible point worth comparing to (or just drawing inspiration from) is the native-tls crate in Rust which tries to span multiple TLS implementations and give a "least common denominator" of sorts while still being by-default secure. This API will likely be used to implement that crate one day. The only major incompatibility I see though is the requirement to take an input-stream and output-stream. Along these lines I'm not sure whether pipe would solve the problem since using that typically requires some sort of concurrency or parallelism.

I didn't happen to catch this before the latest refactoring, though, so I don't know how different the API would look like to support arbitrary I/O sources.

badeend commented 1 month ago

I didn't happen to catch this before the latest refactoring, though, so I don't know how different the API would look like to support arbitrary I/O sources.

The current version consumes a pair of streams and provides a pair of streams. The previous version didn't consume any stream, but instead provided two pairs of streams:

https://github.com/badeend/wasi-sockets/blob/c30022b558e8bd0c86f6533e2644748807c188c6/wit/tls.wit#L198-L229

    /// The I/O streams that represent both sides of the transform.
    ///
    /// The application side interacts with the cleartext "private" streams.
    /// The network side interacts with the encrypted "public" streams.
    ///
    /// A typical setup looks something like this:
    /// 
    /// ```text
    /// :    TCP Socket                              TLS Client/Server
    /// +-----------------+            +---------------------------------------------+
    /// |                 |   splice   |                 decryption                  |   read
    /// |  `input-stream` | ========>> | `public-output` =========>> `private-input` | ======>>   your
    /// |                 |            |                                             |            app
    /// |                 |            |                                             |            lives
    /// | `output-stream` | <<======== | `public-input` <<========= `private-output` | <<======   here
    /// |                 |   splice   |                 encryption                  |   write
    /// +-----------------+            +---------------------------------------------+
    /// ```
    ///
    /// The user of this interface is responsible for continually forwarding
    /// data from the socket into the `public-output` stream and
    /// data from the `public-input` into the socket.
    ///
    /// # Caution
    /// Because the guest acts as both the producer and the consumer for these
    /// streams, do not use the `blocking_*` methods as that will deadlock yourself.
    record io-streams {
        public-input: input-stream,
        public-output: output-stream,
        private-output: output-stream,
        private-input: input-stream,
    }

As noted in the # Caution section, this is subject to the same concurrency/parallelism issue you mentioned on poll.

alexcrichton commented 1 month ago

Do you think it'd be too onerous to bring back such a construct? That feels like it would work quite well in terms of supporting for example in-memory tests and such which aren't required to be tied to I/O

badeend commented 1 month ago

Yes, we definitely could. That being said, assuming a pipe-like construct exists, I quite like the current design as it is simpler while also being a more general-purpose solution that's not limited to just TLS.

If we were to revert to returning 4 streams, we'd preferably also need to introduce some kind of automatic forwarding mechanism to get back to the same level of ergonomics as the current draft.

I flipped my opinion to the current design, because in the end it seems like the least-worst solution.

alexcrichton commented 1 month ago

Those are good points yeah, and if pipe supported async I/O (which is almost certainly would) then I think everything comes out to be more-or-less equivalent. One thing that I think might be helpful is for errors to have a wasi:sockets/tls accessor to return more rich information such as "I'm waiting on the read end" or the write end or something to help guide what the host needs to do in terms of its original pipes perhaps. It'll be a bit wonky for libraries like native-tls to support everything in-memory but not impossible I think.

badeend commented 1 month ago

Thinking about it some more, forward and pipe are probably equally powerful at the theoretical level. Yet, forward has the advantage of being usable on all existing stream sources. E.g. it can "pipe" a file stream into a socket stream, without the socket API needing to consume the file stream.

Edit: looks like we responded at the same time :)