deprecate rust-crypto / wasm

[aep]

whats your opinion on the state of rust-crypto? it doesnt look well maintained and doesnt compile for webassembly. there's an unpopular but maintained fork here that does https://github.com/buttercup/rust-crypto-wasm

this stuff is maintained https://github.com/RustCrypto but would require picking libraries for individual parts.

• CSPRNG https://github.com/RustCrypto/CSRNGs
• 25519 https://github.com/dalek-cryptography/ed25519-dalek
• curve 448: couldnt find anything
• AES https://github.com/RustCrypto/block-ciphers
• chacha20: this looks dead, but slightly less dead than crypto-wasm https://github.com/cesarb/chacha20-poly1305-aead
• SHA256/SHA512 https://github.com/RustCrypto/hashes
• BLAKE2 https://github.com/RustCrypto/hashes

[mcginty]

yeah I'd really like to phase out rust-crypto as rust implementations become available. I'm very interested in replacing the remaining pieces with competent implementations (ex. curve25519-dalek).

I'll have to check out this wasm fork to see what kinds of changes they made, haven't heard of it up until now.

[aep]

in the meantime the PR might be an OK quickfix. it enables the fork only for wasm.

[aep]

@mcginty seems to work for me. should we go ahead with that PR?

[mcginty]

Hey sorry for being less responsive, I'm traveling more intensely for the next couple weeks. Let's take a look at that PR.

[mcginty]

Your code seems fine, but there's some smell in the rust-crypto-wasm crate that makes me on edge.

First, they haven't updated the README or crate metadata, which makes this crate seem more like a dirty workaround hack than a maintained repository.

Second, the constant-time comparison implemented isn't convincingly constant-time given how complicated it can be in Rust with its clever optimizer. In a nicer world it'd rely on something like subtle rather than implementing in an way that's I'd imagine does not actually fix timing attacks.

Overall, I don't get the feeling that this crate should be included in its current state since it feels half-baked.

[aep]

makes sense, thanks for checking.

[mcginty]

No problem! By the way, unfortunately I think the AES checkmark in your list isn't there yet either, because the RustCrypto project doesn't implement GCM.

Also, we already use https://github.com/cesarb/chacha20-poly1305-aead for chacha20poly1305, and the same author of that wrote the BLAKE library we are using as well.

[mcginty]

It seems like maybe we could switch to RustCrypto's SHA implementations though at least? Then all we'd use rust-crypto for is its AES-GCM implementation.

I'll look into putting a PR in to migrate rust-crypto's GHASH and GCM implementation to RustCrypto's repos but not sure if I have the time at the moment.

[aep]

could you open a bug in the meantime? Maybe someone else could work on the GCM. If thats the only part missing, that sounds pretty good!

[mcginty]

There's already a tracking issue open for their missing MACs.

[aep]

this is currently blocking me from porting to aaarch64 and mipsel. any idea what the state is?

the issue here RustCrypto/block-ciphers#1 doesnt seem to mention GCM

is using a tls crate like rusttls an option?

[mcginty]

rustls uses ring as its crypto provider. does ring not compile for aarch64 and mipsel I take it?

[aep]

ah, that makes sense of course. no ring isnt that portable unfortunately.

my best option seems to be fixing hacl

[aep]

hacl compiles, however:

Init { reason: GetRngImpl }

looks like thats just missing:

snow/src/resolvers/hacl_star.rs

Lines 19 to 21 in da3675f

	fn resolve_rng(&self) -> Option<Box<Random>> {
	None
	}

should i add this using the rand crate?

[mcginty]

You can do this using the existing snow just by using FallbackResolver and a custom resolver that only resolves the RNG to an instance from the rand crate (can just copy that part from the default resolver).

I'd like to keep the HACL* resolver as-is because it would only make sense to resolve the RNG in that impl if it's a HACL*-specific RNG, in my opinion.

[aep]

got it. unfortunately hacl has no aesgcm either. how do you feel about this one:

https://github.com/miscreant/miscreant-rs

(sorry i did misread, thats not AES, which the author claims to be less misuse resistant. i'm going to post this to the noise ml)

[aep]

how do we proceed here? My proposal would be to add a new resolver that simply doesnt have aesgcm. I can do a PR

[mcginty]

Yeah, let's just remove our dependency on rust-crypto and not support AES-GCM in the default resolver for now, and I will look into spinning off a separate crate with just the AES-GCM support + fixes.

If you want to submit a PR, I think it's time to remove our dependency on an abandoned crypto crate.

[BlackHoleFox]

@mcginty I spent a bit of time and replaced rust-crypto. Its probably not the cleanest job ever, but any tips are appreciated. All the tests pass and the benchmarks run as well. Currently I can't cross-build Snow because of a error inside rust-crypto's symbols. You can see my work over here. If it looks alright to start then I'd be happy to open a PR.

[mcginty]

@BlackHoleFox just skimmed it - off to a good start, left you some comments. Feel free to open a PR and I can help you finish iterating on it.

[mcginty]

#76 and #79 are related.

[mcginty]

In the mean time though, rust-crypto is no longer in this project so I'm going to go ahead and close this issue.