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

replace sodiumoxide with chacha20-poly1305-aead or other RFC7539 crate? #58

Closed
warner opened this issue Aug 21, 2018 · 10 comments
Closed

Comments

@warner
Copy link
Collaborator

warner commented Aug 21, 2018

I've been updating our dependencies, and moving to the current sodiumoxide-0.1.0 crate is a hassle because it requires that the OS have a pre-installed libsodium (of a newer version that what came in debian/stretch). Getting that installed is frustrating, and it means that cargo install magic-wormhole won't work out-of-the-box.

We don't need much from libsodium: we only use the SecretBox construction, to encrypt the post-PAKE records in the Mailbox/rendezvous protocol (and again for the Transit records). We don't need the more general Box construction (the public-key thing), or any of the lower-level tools.

It looks like SecretBox might have been standardized in RFC7539, at least there's an AEAD construction that uses ChaCha20 and Poly1305. If the layout is in fact the same, then we've got some pure-Rust options for our symmetric crypto. The chacha20-poly1305-aead crate looks promising, although it hasn't been updated in a couple years and says that it needs Nightly rust. The well-respected ring crate has an RFC7539 AEAD construction; I can't immediately tell if it's implemented in C or in Rust, but I believe it all builds with a simple cargo build so it doesn't matter too much. Also I see a RustCrypto experiment that seems to have the right pieces in it, which would be ideal (we're using several RustCrypto libraries already).

So the task here is to identify the smallest easy-to-build ideally-pure-Rust-but-at-least-self-contained most-respected crate we can find, port our use of SecretBox to its API, and then drop our dependency on sodiumoxide.

@warner
Copy link
Collaborator Author

warner commented Aug 21, 2018

ring is looking promising. Here's what I've got for the encryption side.. any thoughts?

fn gen_nonce(buf: &mut [u8]) {
    let sr = rand::SystemRandom::new();
    sr.fill(buf).unwrap();
}

pub fn encrypt_data(key: &[u8], plaintext: &[u8]) -> (Vec<u8>, Vec<u8>) {
    let box_key = aead::SealingKey::new(&aead::CHACHA20_POLY1305, &key).unwrap();
    let alg = box_key.algorithm();
    let mut nonce = vec![0u8; alg.nonce_len()];
    gen_nonce(&mut nonce);
    let mut buf = vec![0u8; alg.nonce_len() + plaintext.len() + alg.tag_len()];
    let ad = vec![];
    aead::seal_in_place(&box_key, &nonce, &ad, &mut buf[alg.nonce_len()..], alg.tag_len()).unwrap();
    buf[0..alg.nonce_len()].copy_from_slice(&nonce);
    (nonce, buf)
}

I'm still trying to figure out the decryption side: the ring API decrypts data in-place, so I'm having to juggle the buffers in an unfamilar way.

@warner
Copy link
Collaborator Author

warner commented Aug 21, 2018

I've got a prototype in https://github.com/warner/magic-wormhole.rs/tree/ring , although it is failing the known-answer-tests (which I just added to the python version and then ported here). So maybe the formats aren't really compatible. Hopefully it's something easy to fix.

@warner
Copy link
Collaborator Author

warner commented Aug 21, 2018

Hrmph, it looks like RFC7539 is ChaCha20/Poly1305, whereas the default SecretBox cipher is XSalsa20/Poly1305. ChaCha20 takes a 12-byte nonce, XSalsa20 takes a 24-byte nonce.

@warner
Copy link
Collaborator Author

warner commented Aug 21, 2018

Zaki tells me that XSalsa20 is pretty much only in NaCl/libsodium variants, and that ChaCha20 is being used everywhere else. Bummer.

@copyninja
Copy link
Contributor

I'm not crypto expert but was just curious about your ring experiment branch. I tried to build it on Windows and it worked which is good. But the bad part is this does not work with SecretBox from NaCl. I think this you already noted in your above comment. So use of ring means original python version also needs to be changed.

@warner
Copy link
Collaborator Author

warner commented Oct 30, 2018

Yeah, what it really means is a compatibility break, or rather an obligation to add a backwards-compatibility layer. I only really want to do that once, so I'm gonna put it on the list of crypto improvements we'd like to make and defer it for a while.

I might be interested in writing a pure-rust crate that just does the SecretBox-compatible XSalsa20/Poly1305, to drop our dependency on libsodium. If anyone sees one of those floating around, please let me know.

@warner
Copy link
Collaborator Author

warner commented Sep 1, 2019

I think that the RustCrypto chacha20poly1305 crate now includes a libsodium-compatible XChaCha20Poly1305 implementation, as of two days ago. So it's worth a try to replace our use of libsodium with that (pure-rust) crate.

@warner
Copy link
Collaborator Author

warner commented Sep 1, 2019

Ah, nope. That crate implements an algorithm (XChaCha20Poly1305) which is probably compatible with one of the libsodium options, but it is not compatible with the default secretbox algorithm that the python code uses (XSalsa20Poly1305). Since I want to maintain interoperability with the python client, I've gotta stick with the xsalsa20 variant, so that crate doesn't help.

We could imagine a breaking change that switched to XChaCha20 (if pynacl were to expose it, which I think it doesn't), but really if we were willing to break backwards compatibility, I'd rather move to an RFC-specified cipher like ChaCha20.

warner added a commit that referenced this issue Sep 2, 2019
This isn't what we want for #58, but if/when a real XSalsa20Poly1305 crate
exists, this patch does 90% of the work to use it. Just s/ChaCha/Salsa/g and
it should probably work.

refs #58
@warner
Copy link
Collaborator Author

warner commented Sep 23, 2019

Removing sodiumoxide (which uses a big C library) in favor of something pure-Rust will also help with compiling to WASM (#2).

@warner
Copy link
Collaborator Author

warner commented Oct 3, 2019

The xsalsa20poly1305 crate just got published.. thanks @tarcieri ! I'm going to update the 58-xchacha20 branch to use xsalsa20 instead and see how well it works.

warner added a commit that referenced this issue Oct 12, 2019
Tests pass, still need to check interop.

refs #58
warner added a commit that referenced this issue Oct 15, 2019
@warner warner closed this as completed in a3ee43a Oct 15, 2019
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

2 participants