Skip to content

WIP: Legacy ChaCha20Poly1305 Construction#304

Closed
kamulos wants to merge 1 commit intoRustCrypto:masterfrom
kamulos:legacy_chacha_construction
Closed

WIP: Legacy ChaCha20Poly1305 Construction#304
kamulos wants to merge 1 commit intoRustCrypto:masterfrom
kamulos:legacy_chacha_construction

Conversation

@kamulos
Copy link

@kamulos kamulos commented May 4, 2021

I just quickly copied the code from the xchacha20poly1305 module and adapted it to ChaCha20Legacy together with the LegacyNonce. I set the NonceSize to U8.

It is however not working and I could use some hints where to search. When I encrypt using libsodium and decrypt using my new module the mac verification fails (if self.mac.verify(tag).is_ok() in cipher.rs:87). When I do the same thing with XChaCha20Poly1305 everything works. So probably I just did not change something I am supposed to change, but I don't know what that is.

This is my test program:

Cargo.toml

[package]
name = "chacha"
version = "0.1.0"
edition = "2018"

[dependencies]
sodiumoxide = "0.2"
chacha20poly1305 = { path = "../AEADs/chacha20poly1305", features = ["legacy"] }

main.rs (comment in and out to switch to xchacha)

// use sodiumoxide::crypto::aead::xchacha20poly1305_ietf as sodium_algo;
// const NONCE: &[u8; 24] = b"123456781234567812345678";
// type RustCryptoChaCha = chacha20poly1305::XChaCha20Poly1305;
// type RustCryptoNonce = chacha20poly1305::XNonce;

use sodiumoxide::crypto::aead::chacha20poly1305 as sodium_algo;
const NONCE: &[u8; 8] = b"12345678";
type RustCryptoChaCha = chacha20poly1305::ChaCha20Poly1305Legacy;
type RustCryptoNonce = chacha20poly1305::LegacyNonce;

use chacha20poly1305::aead::{Aead, NewAead};

fn main() {
    sodiumoxide::init().unwrap();
    let k = sodium_algo::gen_key();
    let n = sodium_algo::Nonce::from_slice(NONCE).unwrap();

    let cipher = sodium_algo::seal(b"Hello World!", None, &n, &k);

    println!(
        "sodium: {}",
        std::str::from_utf8(&sodium_algo::open(&cipher, None, &n, &k).unwrap()).unwrap()
    );

    let decrypter = RustCryptoChaCha::new_from_slice(&k.0).unwrap();
    let plaintext = decrypter
        .decrypt(RustCryptoNonce::from_slice(&n.0), &cipher[..])
        .unwrap();

    println!(
        "rust crypto: {}",
        std::str::from_utf8(&plaintext[..]).unwrap()
    );
}

@tarcieri
Copy link
Member

tarcieri commented May 4, 2021

Unfortunately it looks like you won't be able to use Cipher because the two constructions are slightly different in ways that I wasn't suspecting. My apologies for claiming this would be easy.

Here's the "djb" version:

https://github.com/jedisct1/libsodium/blob/6d566070b48efd2fa099bbe9822914455150aba9/src/libsodium/crypto_aead/chacha20poly1305/sodium/aead_chacha20poly1305.c#L37-L52

Here's the IETF version:

https://github.com/jedisct1/libsodium/blob/6d566070b48efd2fa099bbe9822914455150aba9/src/libsodium/crypto_aead/chacha20poly1305/sodium/aead_chacha20poly1305.c#L108-L127

If I'm reading these correctly, the Poly1305 inputs differ in the following way:

  • djb version: aad || len_bytes(aad) || msg || len_bytes(msg)
  • IETF version: pad(aad) || pad(msg) || len_bits(aad) || len_bits(msg)

Where:

  • pad(...) adds zero bytes until the input is aligned with the Poly1305 block size. You don't need to do that.
  • The IETF version serializes lengths in terms of bits, whereas the djb version uses bytes.
  • The IETF version places both lengths at the end of the Poly1305 input, whereas the djb version interleaves them.

Here's an example of unpadded Poly1305:

https://github.com/RustCrypto/AEADs/blob/458a8a8/xsalsa20poly1305/src/lib.rs#L284

@kamulos
Copy link
Author

kamulos commented May 4, 2021

Thanks for the fast analysis, that helped a lot! I just hacked something into a commit, that seems to be working, but obviously is not any good.

My main problem is, that the compute_unpadded function you recommended consumes itself. I was not able to find anything for the Poly1305, that could be updated multiple times. Do you have a recommendation for that?

@tarcieri
Copy link
Member

tarcieri commented May 5, 2021

@kamulos unfortunately the poly1305 crate does not provide a buffered, non-block oriented API.

Sorry to stray even further from "this will be easy", but it seems like to implement this construction on top of the current API the poly1305 crate provides, you'll need to add a buffering wrapper.

@newpavlov has written a number of abstractions for this sort of thing and may be able to help.

My main problem is, that the compute_unpadded function you recommended consumes itself. I was not able to find anything for the Poly1305, that could be updated multiple times. Do you have a recommendation for that?

If you implement a block-sized buffer (16-bytes), you can pass data a block-at-a-time to Poly1305 using the UniversalHash::update method, and then use Poly1305::compute_unpadded to pass in any remaining data prior to finalization, regardless of whether or not it's aligned to the size of a block.

@kamulos
Copy link
Author

kamulos commented May 5, 2021

😄 no worries, that doesn't sound too bad. I think I have all the necessary information now. I will prepare something as soon as I find time and send it to you for a review.

@kamulos kamulos force-pushed the legacy_chacha_construction branch from 0bb63ea to e8ec2ae Compare May 11, 2021 19:22
@kamulos kamulos force-pushed the legacy_chacha_construction branch from e8ec2ae to b06ed94 Compare May 11, 2021 19:25
@kamulos
Copy link
Author

kamulos commented May 11, 2021

OK, I found a little bit of time and made progress. Can you have a look at the BufferedPoly1305 struct?

  • I don't know that much about the traits and GenericArrays in this repo, so I just did it as straight forward as possible
  • The BufferedPoly1305 just keeps the remainder of the buffer that was not enough to get a block
  • I did not find a different way to compute partial blocks, so I just used compute_unpadded for that. Otherwise the poly1305 crate would need to be changed?

@tarcieri tarcieri marked this pull request as ready for review May 11, 2021 23:20
@tarcieri
Copy link
Member

BufferedPoly1305 looks fine at first glance. Perhaps @newpavlov has some suggestions since he writes a lot of this sort of buffering code.

You don't need to worry about trying to impl any traits for it since it's a private implementation detail used only for this purpose.

@tarcieri
Copy link
Member

Closing this PR as stale. Please rebase and open an new PR if you'd like to continue it.

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.

2 participants