Skip to content
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

symmetric_state_clone not implemented for wasi-crypto-guest #65

Closed
rjzak opened this issue Jul 18, 2022 · 12 comments
Closed

symmetric_state_clone not implemented for wasi-crypto-guest #65

rjzak opened this issue Jul 18, 2022 · 12 comments

Comments

@rjzak
Copy link
Contributor

rjzak commented Jul 18, 2022

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
@rjzak rjzak changed the title symmetric_state_clone not implemented for symmetric_state_clone not implemented for wasi-crypto-guest Jul 19, 2022
rjzak added a commit to rjzak/wasi-crypto that referenced this issue Jul 19, 2022
Signed-off-by: Richard Zak <richard@profian.com>
@sonder-joker sonder-joker self-assigned this Jul 19, 2022
@jedisct1
Copy link
Member

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
Copy link
Collaborator

sonder-joker commented Jul 19, 2022

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
Copy link
Member

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
Copy link
Contributor Author

rjzak commented Jul 19, 2022

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
Copy link
Member

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
Copy link
Contributor Author

rjzak commented Jul 19, 2022

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
Copy link
Member

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

@sonder-joker sonder-joker removed their assignment Jul 21, 2022
@rjzak
Copy link
Contributor Author

rjzak commented Jul 26, 2022

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
Copy link
Contributor Author

rjzak commented Jul 27, 2022

deleted

@rjzak
Copy link
Contributor Author

rjzak commented Jul 27, 2022

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
Copy link
Member

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

@jedisct1
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants