jedisct1 / libsodium

A modern, portable, easy to use crypto library.
https://libsodium.org
Other
12.06k stars 1.72k forks source link

Authentication tag inconsistency between different ChaPoly constructions. #1152

Closed LoupVaillant closed 2 years ago

LoupVaillant commented 2 years ago

Hi,

I was trying to use crypto_aead_chacha20poly1305_encrypt_detached() crypto_aead_chacha20poly1305_ietf_encrypt_detached(), and crypto_aead_xchacha20poly1305_ietf_encrypt_detached(), to generate various test vectors, and I noticed an inconsistency in the way they handle the authentication tag: while the latter two follow RFC 8439 to the letter, the first one omits the padding from the authentication step.

This discrepancy does not look voluntary. It may have been caused by a copy pasta fumble.

When removing everything except the authentication code, here's what we get for the original chacha20:

crypto_onetimeauth_poly1305_init(&state, block0);
crypto_onetimeauth_poly1305_update(&state, ad, adlen);
STORE64_LE(slen, (uint64_t) adlen);
crypto_onetimeauth_poly1305_update(&state, slen, sizeof slen);
crypto_onetimeauth_poly1305_update(&state, c, mlen);
STORE64_LE(slen, (uint64_t) mlen);
crypto_onetimeauth_poly1305_update(&state, slen, sizeof slen);
crypto_onetimeauth_poly1305_final(&state, mac);

No padding there. The other two functions however authenticate the padding, just like RFC 8439:

crypto_onetimeauth_poly1305_init(&state, block0);
crypto_onetimeauth_poly1305_update(&state, ad, adlen);
crypto_onetimeauth_poly1305_update(&state, _pad0, (0x10 - adlen) & 0xf);
crypto_onetimeauth_poly1305_update(&state, c, mlen);
crypto_onetimeauth_poly1305_update(&state, _pad0, (0x10 - mlen) & 0xf);
STORE64_LE(slen, (uint64_t) adlen);
crypto_onetimeauth_poly1305_update(&state, slen, sizeof slen);
STORE64_LE(slen, (uint64_t) mlen);
crypto_onetimeauth_poly1305_update(&state, slen, sizeof slen);
crypto_onetimeauth_poly1305_final(&state, mac);

I see three way this could be fixed:

jedisct1 commented 2 years ago

This is voluntary.

crypto_aead_chacha20poly1305 predates the RFC. It was originally called secretbox_chacha20poly1305, and was later renamed crypto_aead_chacha20poly1305.

The RFC doesn't cover the DJB variant, so crypto_aead_chacha20poly1305 is not meant to be compatible with anything but by how it was originally implemented.