Initial implementation of Wasi-tls (Transport Layer Security) #10249
Conversation
This crate provides the Wasmtime host implementation for the [wasi-tls] API. The [wasi-tls] world allows WebAssembly modules to perform SSL/TLS operations, such as establishing secure connections to servers. TLS often relies on other wasi networking systems to provide the stream so it will be common to enable the [wasi:cli] world as well with the networking features enabled. The initial implemntation is using rustls. Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
Signed-off-by: James Sturtevant <jstur@microsoft.com>
Signed-off-by: James Sturtevant <jstur@microsoft.com>
|
Looking great! CI fails because Clippy being Clippy, and I suspect the build fails because there's some missing conditional compilation. Aside from that, I;ve left behind some remarks: |
crates/wasi-tls/src/lib.rs
Outdated
| /// Represents the ClientHandshake which will be used to configure the handshake | ||
| pub struct ClientHandShake { | ||
| server_name: String, | ||
| streams: TcpStreams, |
There was a problem hiding this comment.
Might be nice to rename references to the term "TCP" within this file to something more generic. Since this TLS implementation doesn't actually depend on TCP, and the inner streams may come from anywhere.
crates/wasi-tls/src/lib.rs
Outdated
| let StreamState::Pending(future) = mem::replace(&mut self.0, StreamState::Closed) else { | ||
| unreachable!() | ||
| }; | ||
| self.0 = StreamState::Ready(future.await); |
There was a problem hiding this comment.
I don't think this is cancel-safe. If the .await is canceled, the streamState will be left behind in the Closed state forever. Cancellation may happen when waiting for multiple Pollables at once.
There was a problem hiding this comment.
Will take a look, is there an example of how to test that?
There was a problem hiding this comment.
To reproduce, you could try adjusting the timeout of blocking_finish in crates/test-programs/src/tls.rs to something non-0, but very small, so that both pollables are polled at least once, but the monotonic_clock pollable will finish first. After that, I suspect that the ClientHandshake::finish pollable will never resolve.
There was a problem hiding this comment.
you are indeed correct.
There was a problem hiding this comment.
I've pushed an update for this, I tested it locally but working getting a test written for this scenario. Let me know if the change I made makes more sense.
There was a problem hiding this comment.
added test that covers this in f13938e
crates/wasi-tls/src/lib.rs
Outdated
| output: StreamState<BoxOutputStream>, | ||
| } | ||
|
|
||
| impl AsyncWrite for TcpStreams { |
There was a problem hiding this comment.
Regarding this impl AsyncWrite for TcpStreams block and the impl AsyncRead for TcpStreams block below: I'm fine with them existing here, but AFAIK these have nothing to do with TLS right? If so, we could move them into the wasi-io subcrate at some point.
There was a problem hiding this comment.
correct, not specific to TLS it just enables passing wasi-io streams to libraries that require those traits (i.e. in this case the connect() method for rustls)
| } | ||
| ", | ||
| path: [ | ||
| "../wasi-http/wit", | ||
| "../wasi-config/wit", | ||
| "../wasi-keyvalue/wit", | ||
| "../wasi-tls/wit/world.wit", |
There was a problem hiding this comment.
The reason for this being the wit file directly is bytecodealliance/wasm-tools#1996 (comment)
Cargo.toml
Outdated
| wit-parser = { git = "https://github.com/jsturtevant/wasm-tools.git", branch="unstable-refrence-stable" } | ||
| wit-component = { git = "https://github.com/jsturtevant/wasm-tools.git", branch="unstable-refrence-stable" } | ||
| wasm-wave = { git = "https://github.com/jsturtevant/wasm-tools.git", branch="unstable-refrence-stable" } | ||
| wasm-metadata = { git = "https://github.com/jsturtevant/wasm-tools.git", branch="unstable-refrence-stable" } |
There was a problem hiding this comment.
this branch contains the temp fix for bytecodealliance/wasm-tools#1995
Signed-off-by: James Sturtevant <jstur@microsoft.com>
Signed-off-by: James Sturtevant <jstur@microsoft.com>
Signed-off-by: James Sturtevant <jstur@microsoft.com>
jsturtevant
force-pushed
the
wasi-tls-initial
branch
from
7c8d5ba to
bfea678
Compare
Signed-off-by: James Sturtevant <jstur@microsoft.com>
jsturtevant
force-pushed
the
wasi-tls-initial
branch
from
bfea678 to
f13938e
Compare
fixes: #10089
This adds a crate that provides the Wasmtime host implementation for the wasi-tls API.
The wasi-tls world allows WebAssembly modules to perform SSL/TLS operations,
such as establishing secure connections to servers.
This initially implements it with rustls which was already included in the repository.
There are a few items that still need to be resolved but are not specially related to the implementation but wanted to start the review to make sure things moving in the correct direction.
close-notifyfunction to the wasi-tls wit in the proposal to properly close down the Add close function that allows for cleaning closing tls connection WebAssembly/wasi-tls#5Unstableattribute in wit wasm-tools#1995). Write now I have a workaround on a branch that is being used in rust cargo patch.Thanks goes out to @badeend and @dicej as much of this was based on their prior work.