-
Notifications
You must be signed in to change notification settings - Fork 83
Add a new IO abstraction #237
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
Conversation
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.
LGTM.
Thank you for this PR for provide an better unified interface of IO. Learn a lot
It make it easier to add the async support.
Will refactor #266 based on this. It seems I could follow same path to add maker type of Async AnyIo
, Steam
and implement related traits
If I understand it correctly, you did two things at once in this PR:
That is a nice and safe abstraction over the C interface and removes the necessity to manually implement a C callback interface. This part is a huge step forward 👍
It took me a while to wrap my head around this but I guess it is meant to be an interface which the SSL context can wrap transparently like I see the motivation here because it would theoretically allow consumers of this interface to work independently from the underlying transport layer but since this trait is defined in this library, this would only work for consumers which explicitly have mbedtls in mind. So it would only work for users which want to use mbedtls anyways. For this idea to work in a generic way, the trait would need to be defined outside of this library but this would just be like the start of another Regarding the fact that there are now three similarly named traits, I think there is a loss of readibility. Additionally, I suspect that it might get harder to use If you stick to the |
Thanks
Right.
Agreed but the ecosystem hasn't really developed something like this. Especially also considering the datagram-based abstraction. My thought was that by providing this trait, users can always interface their own traits with it as they see fit. |
Yes, that's right. Personally, I would not do it like this but I do not object too strongly here. So feel free to merge this. |
f4a6a5d
to
3bde9bb
Compare
Added some more documentation on the bors r=Taowyoo |
Build succeeded:
|
239: Add async IO & async support for TLS r=jethrogb a=Taowyoo # TLDR This PR added: - Async version of IO abstraction ( only stream part, IO abstraction is introduced on PR: [Add a new IO abstraction #237](237) ) - Implementation for TLS # Detailed Explanation ## Background We use `mbedtls` for crypto and SSL handshake in rust. `mbedtls::ssl` module contains most FFI wrapper for c mbedtls API. ### How `mbedtls::ssl` works To setup a TLS client or a TLS server with `mbedtls`, mainly you need to provide two things to `mbedtls`: - [Config](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/config.rs#L148): a config object containing settings and parameters needed for TLS handshake - [`BIO`][1]: similar to `openssl`, `mbedtls` need you to provide a `BIO` to it. `BIO` stands for **Basic I/O abstraction**, at here you could simply treat them as source & destination of data such as a TCP socket or file i/o. In code level, it represent as two callbacks in rust: ```rust // representation of the `mbedtls_ssl_send_t` and `mbedtls_ssl_recv_t` callback function pointers. unsafe extern "C" fn call_recv(user_data: *mut c_void, data: *mut c_uchar, len: size_t) -> c_int where Self: Sized; unsafe extern "C" fn call_send(user_data: *mut c_void, data: *const c_uchar, len: size_t) -> c_int where Self: Sized; ``` In `mbedtls`, we pass function pointers and user_data pointer to c mbedtls lib through FFI function `ssl_set_bio`. During handshake and data transfer, the c mbedtls lib will utilize those two callbacks to send or receive data from underlying IO. ### IO abstraction The IO abstraction PR #237 created and implemented safe rust abstraction of `BIO`: <details> <summary>Details</summary> - Introduce unsafe trait [`IoCallbackUnsafe`](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/io.rs#L36) to represent `mbedtls_ssl_send_t` and `mbedtls_ssl_recv_t` callback function pointers. - Introduce safe trait [`IoCallback`](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/io.rs#L48) : A safe representation of above trait. Also provides generic implementation of `IoCallback` for `IoCallbackUnsafe`. So user only need to implement this trait. - Introduce trait [`Io`](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/io.rs#L102) to represent a duplex socket or file descriptor that can be read from or written to in case you cannot use `std::io::Read` and `std::io::Write`. - To integrate TCP, `std::net::TcpStream` implemented `std::io::Read` and `std::io::Write`: - Implemented `IoCallback` with Marker types: `Stream` for `T: std::io::Read + std::io::Write` - Implemented `IoCallbackUnsafe` with Marker types: `Stream` for `Contex<T>` - So you can pass `std::net::TcpStream` directly to [`mbedtls::ssl::context::Context::establish`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L234) - To integrate UDP, since `std::net::UdpSocket` does not implement and reader/writer trait: - Introduced `ConnectedUdpSocket` as wrapper of `std::net::UdpSocket` and implemented `Io` for it - Implemented `IoCallback` with Marker types: `AnyIo` for `T: Io` - Implemented `IoCallbackUnsafe` with Marker types: `AnyIo` for `Contex<T>` - So you can pass `ConnectedUdpSocket` directly to [`mbedtls::ssl::context::Context::establish`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L234) </details> ## Async Support for TLS ### Follow same path To add async support, we could use `tokio::net::TcpStream` and traits `tokio::io::Read` and `tokio::io::Write` and follow the same path of what we did for TCP in [IO abstraction](###IO_abstraction): <details> <summary>Details</summary> - (new) Add marker type for async Steam IO: `AsyncStream` - (new) Add [`establish_async`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/async_io.rs#L144) for `Context<T>`, this async function is implemented by wrapping the original blocking function [`handshake`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L271) in a custom `Future` implementation. - Implemented `IoCallback` with Marker types: `AsyncStream` for `T: tokio::io::Read + tokio::io::Write` - Implemented `IoCallbackUnsafe` with Marker types: `AsyncStream` for `Contex<T>` - So you can pass `tokio::net::TcpStream` directly to [`Context::establish_async`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/async_io.rs#L144) </details> ### Async need Context But the key obstacle here is async code need context. The functions needed by [`tokio::io::Read`](https://docs.rs/tokio/latest/tokio/io/trait.AsyncRead.html) and [`tokio::io::Write` ](https://docs.rs/tokio/latest/tokio/io/trait.AsyncWrite.html) both need a parameter: `cx` whose type is `std::task::Context`. To address this, implementation need to adjust: - `IoCallback` `impl`'s now are for a tuple `(&'a mut TaskContext<'b>, &'c mut T)` so that `Context` is accessible in the callback - Introduced function [`with_bio_async`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L199) to use a callback function to wrap the `Context` and callback functions' data pointer together and pass it to `mbedtls` through `set_bio_raw` **Note**: The async support for DTLS will be added in another PR: #241 [1]: https://www.openssl.org/docs/man1.1.1/man7/bio.html Co-authored-by: Yuxiang Cao <yuxiang.cao@fortanix.com>
239: Add async IO & async support for TLS r=[jethrogb] a=Taowyoo # TLDR This PR added: - Async version of IO abstraction ( only stream part, IO abstraction is introduced on PR: [Add a new IO abstraction #237](237) ) - Implementation for TLS # Detailed Explanation ## Background We use `mbedtls` for crypto and SSL handshake in rust. `mbedtls::ssl` module contains most FFI wrapper for c mbedtls API. ### How `mbedtls::ssl` works To setup a TLS client or a TLS server with `mbedtls`, mainly you need to provide two things to `mbedtls`: - [Config](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/config.rs#L148): a config object containing settings and parameters needed for TLS handshake - [`BIO`][1]: similar to `openssl`, `mbedtls` need you to provide a `BIO` to it. `BIO` stands for **Basic I/O abstraction**, at here you could simply treat them as source & destination of data such as a TCP socket or file i/o. In code level, it represent as two callbacks in rust: ```rust // representation of the `mbedtls_ssl_send_t` and `mbedtls_ssl_recv_t` callback function pointers. unsafe extern "C" fn call_recv(user_data: *mut c_void, data: *mut c_uchar, len: size_t) -> c_int where Self: Sized; unsafe extern "C" fn call_send(user_data: *mut c_void, data: *const c_uchar, len: size_t) -> c_int where Self: Sized; ``` In `mbedtls`, we pass function pointers and user_data pointer to c mbedtls lib through FFI function `ssl_set_bio`. During handshake and data transfer, the c mbedtls lib will utilize those two callbacks to send or receive data from underlying IO. ### IO abstraction The IO abstraction PR #237 created and implemented safe rust abstraction of `BIO`: <details> <summary>Details</summary> - Introduce unsafe trait [`IoCallbackUnsafe`](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/io.rs#L36) to represent `mbedtls_ssl_send_t` and `mbedtls_ssl_recv_t` callback function pointers. - Introduce safe trait [`IoCallback`](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/io.rs#L48) : A safe representation of above trait. Also provides generic implementation of `IoCallback` for `IoCallbackUnsafe`. So user only need to implement this trait. - Introduce trait [`Io`](https://github.com/fortanix/rust-mbedtls/blob/c39d93a19d043587dc2346fb4c54501e71190162/mbedtls/src/ssl/io.rs#L102) to represent a duplex socket or file descriptor that can be read from or written to in case you cannot use `std::io::Read` and `std::io::Write`. - To integrate TCP, `std::net::TcpStream` implemented `std::io::Read` and `std::io::Write`: - Implemented `IoCallback` with Marker types: `Stream` for `T: std::io::Read + std::io::Write` - Implemented `IoCallbackUnsafe` with Marker types: `Stream` for `Contex<T>` - So you can pass `std::net::TcpStream` directly to [`mbedtls::ssl::context::Context::establish`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L234) - To integrate UDP, since `std::net::UdpSocket` does not implement and reader/writer trait: - Introduced `ConnectedUdpSocket` as wrapper of `std::net::UdpSocket` and implemented `Io` for it - Implemented `IoCallback` with Marker types: `AnyIo` for `T: Io` - Implemented `IoCallbackUnsafe` with Marker types: `AnyIo` for `Contex<T>` - So you can pass `ConnectedUdpSocket` directly to [`mbedtls::ssl::context::Context::establish`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L234) </details> ## Async Support for TLS ### Follow same path To add async support, we could use `tokio::net::TcpStream` and traits `tokio::io::Read` and `tokio::io::Write` and follow the same path of what we did for TCP in [IO abstraction](###IO_abstraction): <details> <summary>Details</summary> - (new) Add marker type for async Steam IO: `AsyncStream` - (new) Add [`establish_async`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/async_io.rs#L144) for `Context<T>`, this async function is implemented by wrapping the original blocking function [`handshake`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L271) in a custom `Future` implementation. - Implemented `IoCallback` with Marker types: `AsyncStream` for `T: tokio::io::Read + tokio::io::Write` - Implemented `IoCallbackUnsafe` with Marker types: `AsyncStream` for `Contex<T>` - So you can pass `tokio::net::TcpStream` directly to [`Context::establish_async`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/async_io.rs#L144) </details> ### Async need Context But the key obstacle here is async code need context. The functions needed by [`tokio::io::Read`](https://docs.rs/tokio/latest/tokio/io/trait.AsyncRead.html) and [`tokio::io::Write` ](https://docs.rs/tokio/latest/tokio/io/trait.AsyncWrite.html) both need a parameter: `cx` whose type is `std::task::Context`. To address this, implementation need to adjust: - `IoCallback` `impl`'s now are for a tuple `(&'a mut TaskContext<'b>, &'c mut T)` so that `Context` is accessible in the callback - Introduced function [`with_bio_async`](https://github.com/fortanix/rust-mbedtls/blob/yx/async-support_master/mbedtls/src/ssl/context.rs#L199) to use a callback function to wrap the `Context` and callback functions' data pointer together and pass it to `mbedtls` through `set_bio_raw` **Note**: The async support for DTLS will be added in another PR: #241 [1]: https://www.openssl.org/docs/man1.1.1/man7/bio.html Co-authored-by: Yuxiang Cao <yuxiang.cao@fortanix.com>
Implement an IO abstraction as previously suggested to replace Read+Write if you don't have that available. This allows you to layer any
Io
types like you would Read+Write.cc @DrTobe hope you can live with the API change.