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

Wasm wrapper #114

Merged
merged 6 commits into from
Jul 17, 2024
Merged

Wasm wrapper #114

merged 6 commits into from
Jul 17, 2024

Conversation

skaunov
Copy link
Collaborator

@skaunov skaunov commented Jul 3, 2024

Replacement for NPM package.

# Subtle
I was really late to understand that Subtle crypto supports the different curve `secp256r`, *and* it doesn't provide a facility to store secret values. So implementation for `web_sys::SecretKey` turned out to be just extra miles leading nowhere.
```toml
web-sys = { version = "0.3", features = ["CryptoKey", "SubtleCrypto", "Crypto", "EcKeyImportParams"] }
wasm-bindgen-futures = "0.4"
```
```rust
#[wasm_bindgen]
extern "C" {
    // Return type of js_sys::global()
    type Global;
    // // Web Crypto API: Crypto interface (https://www.w3.org/TR/WebCryptoAPI/)
    // type WebCrypto;
    // Getters for the WebCrypto API
    #[wasm_bindgen(method, getter)]
    fn crypto(this: &Global) -> web_sys::Crypto;
}

// `fn sign`
if sk.type_() != "secret" {return Err(JsError::new("`sk` must be secret key"))}
if !js_sys::Object::values(&sk.algorithm().map_err(
    |er|
        JsError::new(er.as_string().expect("TODO check this failing").as_str())
)?).includes(&JsValue::from_str("P-256"), 0) {return Err(JsError::new("`sk` must be from `secp256`"))}

// this was my approach, but seems I got what they did at <https://github.com/rust-random/getrandom/blob/master/src/js.rs>
// js_sys::global().entries().find(); // TODO throw if no Crypto in global

let global_the: Global = js_sys::global().unchecked_into();
let crypto_the: web_sys::Crypto = global_the.crypto();
let subtle_the = crypto_the.subtle();
let sk = JsFuture::from(subtle_the.export_key("pkcs8", &sk)?).await?;

// ...
::from_pkcs8_der(js_sys::ArrayBuffer::from(sk).try_into()?)?;
    zeroize::Zeroizing::new(js_sys::Uint8Array::from(JsFuture::from(subtle_the.export_key("pkcs8", &sk).map_err(
        |er|
            Err(JsError::new(er.as_string().expect("TODO check this failing").as_str()))
        )?).await?).to_vec());

// ...

// `fn try_into`

// ...

// zeroization protection ommitted here due to deprecation // <#112>
// mostly boilerplate from signing; also some excessive ops left for the same reason
// TODO align error-handling in this part
if self.c.type_() != "secret" {return Err(JsError::new("`c` must be secret key"))}
if !js_sys::Object::values(&self.c.algorithm()?).includes(js_sys::JsString::from("P-256").into(), 0) {return Err(JsError::new("`c` must be from `secp256`"))}
this was my approach, but seems I got what they did at <https://github.com/rust-random/getrandom/blob/master/src/js.rs>
js_sys::global().entries().find(); // TODO throw if no Crypto in global
let global_the: Global = js_sys::global().unchecked_into();
let crypto_the: web_sys::Crypto = global_the.crypto();
let subtle_the = crypto_the.subtle();
let c_pkcs = //zeroize::Zeroizing::new(
    js_sys::Uint8Array::from(JsFuture::from(subtle_the.export_key("pkcs8", &self.c)?).await?).to_vec();
// );
let c_scalar = &plume_rustcrypto::SecretKey::from_pkcs8_der(&c_pkcs)?.to_nonzero_scalar();
sk_z.zeroize();

// ...
```

# randomness
Somehow I thought Wasm doesn't have access to RNG, so I used a seedable one and required the seed. Here's how `sign` `fn` was different.
```rust
// Wasm environment doesn't have a suitable way to get randomness for the signing process, so this instantiates ChaCha20 RNG with the provided seed.
// @throws a "crypto error" in case of a problem with the secret key, and a verbal error on a problem with `seed`
// @param {Uint8Array} seed - must be exactly 32 bytes.
pub fn sign(seed: &mut [u8], v1: bool, sk: &mut [u8], msg: &[u8]) -> Result<PlumeSignature, JsError> {
    // ...

    let seed_z: zeroize::Zeroizing<[u8; 32]> = zeroize::Zeroizing::new(seed.try_into()?);
    seed.zeroize();

    // TODO switch to `wasi-random` when that will be ready for crypto
    let sig = match v1 {
        true => plume_rustcrypto::PlumeSignature::sign_v1(
            &sk_z, msg, &mut rand_chacha::ChaCha20Rng::from_seed(seed_z)
        ),
        false => plume_rustcrypto::PlumeSignature::sign_v2(
            &sk_z, msg, &mut rand_chacha::ChaCha20Rng::from_seed(seed_z)
        ),
    };

    let sig = signer.sign_with_rng(
        &mut rand_chacha::ChaCha20Rng::from_seed(*seed_z), msg
    );

    // ...
}
```

# `BigInt` conversion
It was appealing to leave `s` as `BigInt` (see the comments), but that seems to be confusing and hinder downstream code reusage. There's an util function left for anybody who would want to have it as `BigInt`, but leaving the contraty function makes less sense and also makes the thing larger. So let me left it here for reference.
```rust
let scalar_from_bigint =
    |n: js_sys::BigInt| -> Result<plume_rustcrypto::NonZeroScalar, anyhow::Error> {
        let result = plume_rustcrypto::NonZeroScalar::from_repr(k256::FieldBytes::from_slice(
            hex::decode({
                let hexstring_freelen = n.to_string(16).map_err(
                    |er|
                        anyhow::Error::msg(er.as_string().expect("`RangeError` can be printed out"))
                )?.as_string().expect("on `JsString` this always produce a `String`");
                let l = hexstring_freelen.len();
                if l > 32*2 {return Err(anyhow::Error::msg("too many digits"))}
                else {["0".repeat(64-l), hexstring_freelen].concat()}
            })?.as_slice()
        ).to_owned());
        if result.is_none().into() {Err(anyhow::Error::msg("isn't valid `secp256` non-zero scalar"))}
        else {Ok(result.expect(EXPECT_NONEALREADYCHECKED))}
    };
```
will give it a look like tomorrow
I should check that `rc` makes it opt-in and publish to NPM
@skaunov skaunov marked this pull request as ready for review July 8, 2024 20:54
@skaunov skaunov requested a review from Divide-By-0 July 8, 2024 20:55
this one should be tested like a package I guess,
ideas for such tests are welcome as issues
@skaunov skaunov merged commit 83e3ed9 into main Jul 17, 2024
5 of 8 checks passed
@skaunov skaunov deleted the wasm branch July 17, 2024 09:29
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

Successfully merging this pull request may close these issues.

1 participant