WebAssembly / wasi-crypto

WASI Cryptography API Proposal
162 stars 25 forks source link

`symmetric_state_clone` not implemented for `wasi-crypto-guest` #65

Closed rjzak closed 1 year ago

rjzak commented 2 years ago

proposal_symmetric.witx defines a function symmetric_state_clone which isn't implemented in rust/src/symmetric/low/state.rs in the binding crate. This causes compilation errors for Wasmtime when trying to use the latest wasi-crypto, which would be nice to use to leverage the P384 support and updated RustCrypto dependencies.

I'd work on this myself, but I don't understand how the data types defined in the witx format translate into Rust types, or the members of the raw::SymmetricState object.

I commended it out in the witx file for now, but otherwise it generates this error:

error[E0046]: not all trait items implemented, missing: `symmetric_state_clone`
 --> crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs:6:1
  |
6 |   impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for WasiCryptoCtx {
  |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `symmetric_state_clone` in implementation
  |
 ::: crates/wasi-crypto/src/wiggle_interfaces/mod.rs:3:1
  |
3 | / wiggle::from_witx!({
4 | |     witx: ["$CARGO_MANIFEST_DIR/spec/witx/wasi_ephemeral_crypto.witx"],
5 | | });
  | |__- `symmetric_state_clone` from trait
jedisct1 commented 2 years ago

That function was recently added to the spec, but implementations may be a little bit behind, so you need to use a previous version of the witx files.

I'm going to update the Rust implementation; looks like sonder-joker will be handling this for wasmedge.

sonder-joker commented 2 years ago

That function was recently added to the spec, but implementations may be a little bit behind, so you need to use a previous version of the witx files.

I'm going to update the Rust implementation; looks like sonder-joker will be handling this for wasmedge.

I was going to implement clone and some other features for wasi-crypto-guest a few hours ago, but delayed due to some things. WasmEdge already implement symmetric_state_clone

jedisct1 commented 2 years ago

WasmEdge already implement symmetric_state_clone

Nice!

For the rust implementation of hostcalls, I tried to implement symmetric_state_clone() but miserably lost the battle against the borrow checker. I've no idea how to do it.

Individual primitives are cloneable, but the generic SymmetricStateLike is not and I couldn't find the Rust magical incantation to make it work.

@rjzak can you help here?

rjzak commented 2 years ago

I'm happy to take a shot! I'm reviewing https://github.com/jedisct1/witx-codegen to see how to make sense of witx to learn about the types.

jedisct1 commented 2 years ago

This is not really about the generated code. The current one should be fine.

But making SymmetricState or SymmetricStateLike cloneable doesn't look trivial.

rjzak commented 2 years ago

I'm not sure where to start since my IDE isn't able to see into anything generated from the witx files, and the witx files are lacking in data type information.

jedisct1 commented 2 years ago

That's the joy of Rust macros. They are write-only.

rjzak commented 2 years ago

For the rust implementation of hostcalls, I tried to implement symmetric_state_clone() but miserably lost the battle against the borrow checker. I've no idea how to do it.

Individual primitives are cloneable, but the generic SymmetricStateLike is not and I couldn't find the Rust magical incantation to make it work.

@jedisct1 Could you submit a draft PR that I could work from? I'm not sure how to get started since it seems to all be pointers, and I'm not sure of the underlying structure.

rjzak commented 2 years ago

deleted

rjzak commented 2 years ago

Why is mutability required? Some the functions don't need to change the state, yet it seems the trait requires it.

error[E0053]: method `symmetric_state_clone` has an incompatible type for trait
   --> crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs:135:30
    |
135 |     fn symmetric_state_clone(&self, state: guest_types::SymmetricState) -> Result<guest_types::SymmetricState, guest_types::CryptoErrno> {
    |                              ^^^^^
    |                              |
    |                              types differ in mutability
    |                              help: change the self-receiver type to match the trait: `self: &mut CryptoCtx`
    |
note: type in trait
   --> crates/wasi-crypto/src/wiggle_interfaces/mod.rs:3:1
    |
3   | / wiggle::from_witx!({
4   | |     witx: ["$CARGO_MANIFEST_DIR/spec/witx/wasi_ephemeral_crypto.witx"],
5   | | });
    | |__^
    = note: expected fn pointer `fn(&mut CryptoCtx, types::SymmetricState) -> Result<_, _>`
               found fn pointer `fn(&CryptoCtx, types::SymmetricState) -> Result<_, _>`
    = note: this error originates in the macro `wiggle::from_witx` (in Nightly builds, run with -Z macro-backtrace for more info)
jedisct1 commented 2 years ago

That's what wiggle emits unconditionally. I guess the idea is that the WASI context can change with memory allocations, preemption, etc.

jedisct1 commented 1 year ago

This is done.

Not sure why, maybe a bug was fixed in the Rust compiler or in dependencies, but the .clone() method on symmetric states works, so the symmetric_state_clone() function has been added.