RustCrypto/traits

`async-signature` on executors expecting `!Send` futures

QuinnWilton opened this issue · 4 comments

Hey there! I know it's just an experimental API, but I was hoping to make use of the AsyncSigner trait as part of a wasm-bindgen project, to support signing payloads using non-extractable WebCrypto keys.

I ran into two issues when trying that:

  1. The Send + Sync bounds on Self aren't satisfied by the types in web-sys, like CryptoKeyPair, which are explicitly !Send
  2. The Send bound on the future itself isn't satisfied by the !Send futures used when invoking SubtleCrypto.sign

Both of these problems arise because JsValue is !Send in wasm-bindgen, and so it's expected that all futures will also be !Send, unlike in more traditional runtimes like tokio.

I'm able to kludge around the first problem by manually implementing Send for my type that'll implement AsyncSigner<S>, which should be fine assuming web-workers aren't being used, but I'm not sure how to get around the second problem, and I suspect that an upstream change will be needed for that.

Do you have any ideas?

This is roughly what I'm trying to do:

struct JsSigner<K> {
    key_pair: CryptoKeyPair,
    _marker: std::marker::PhantomData<fn() -> K>,
}

#[async_trait]
impl AsyncSigner<rsa::pkcs1v15::Signature> for JsSigner<rsa::pkcs1v15::Signature> {
    async fn sign_async(
        &self,
        msg: &[u8],
    ) -> Result<rsa::pkcs1v15::Signature, async_signature::Error> {
        let subtle = Self::subtle_crypto().map_err(|e| async_signature::Error::from_source(e))?;

        // This can be done without copying using the unsafe `Uint8Array::view` method,
        // but I've opted to stick to safe APIs for now, until we benchmark signing.
        let data = Uint8Array::from(msg).buffer();

        let key = self
            .signing_key()
            .map_err(|e| async_signature::Error::from_source(e))?;

        let algorithm = HashMap::from_iter([("name", "RSASSA-PKCS1-v1_5")])
            .serialize(&serde_wasm_bindgen::Serializer::json_compatible())
            .map_err(|_| async_signature::Error::new())?
            .dyn_into::<js_sys::Object>()
            .map_err(|_| async_signature::Error::new())?;

        let promise = subtle
            .sign_with_object_and_buffer_source(&algorithm, &key, &data)
            .map_err(|_| async_signature::Error::new())?;

        let result = JsFuture::from(promise)
            .await
            .map_err(|_| async_signature::Error::new())?;

        let signature =
            rsa::pkcs1v15::Signature::try_from(Uint8Array::new(&result).to_vec().as_slice())
                .map_err(|_| async_signature::Error::new())?;

        Ok(signature)
    }
}

Which fails with the error:

future cannot be sent between threads safely
within `[async block@src/wasm.rs:71:67: 101:6]`, the trait `Send` is not implemented for `Rc<RefCell<wasm_bindgen_futures::Inner>>`
required for the cast from `Pin<Box<[async block@src/wasm.rs:71:67: 101:6]>>` to `Pin<Box<(dyn Future<Output = Result<rsa::pkcs1v15::Signature, signature::Error>> + Send + 'async_trait)>>`

If the bounds are problematic we can remove them. I'm not sure there's a good justification for them in the first place.

That would be a breaking change, but we can just bump the minor version number.

I think that probably makes sense! Do you want me to open up a PR doing that?

Sure, sounds good

I just tested, and the released version of the crate (0.4.0) works great in my application. Thank you so much!

Closed by #1375