cloudflare / sslconfig

Cloudflare's Internet facing SSL configuration
BSD 3-Clause "New" or "Revised" License
1.3k stars 132 forks source link

Tag mismatch when using asm chacha20_poly1305_{seal, open} directly for rfc7539 #58

Closed anirudhvr closed 7 years ago

anirudhvr commented 7 years ago

Hi @vkrasnov -

Since the EVP_CIPHER_CTX_ctrl API for c20p is specifically for TLS (with the 13-byte AAD + seq number mixing), I'm trying to use the underlying ASM seal/open APIs to get rfc7539 behavior. From looking at e_chacha20_poly1305.c, there doesn't seem to be any TLS-specific details in the ASM implementation. Code snippet adapted from e_chacha20_poly1305.c behavior per https://github.com/cloudflare/sslconfig/blob/master/patches/openssl__chacha20_poly1305_draft_and_rfc_ossl102j.patch#L4159-L4163

unsigned char key_iv[32 + 4 + 12]; memcpy(key_iv, key, 32); memset(key_iv + 32, 0, 4); memcpy(key_iv + 32 + 4, iv, 12); chacha20_poly1305_seal(ciphertext, plaintext, plaintext_len + 16, (uint8_t*)aad, aad_len, key_iv); return plaintext_len + 16;

Since we're not doing this for TLS, there are no seq numbers to XOR into the IV. However, on testing this with the input here: https://tools.ietf.org/html/rfc7539#appendix-A.5 - the encryption and decryption work correctly but the tag is wrong. Any ideas how to fix this? Thanks

vkrasnov commented 7 years ago

The length argument should be the length of the plaintext without the tag. The tag length is implicit.

anirudhvr commented 7 years ago

Wow, that was indeed simple; I also had to do ciphertext_len - 16 and it works now! Thanks!

I was misled by this snippet :) /* Fix for MAC */ inl -= POLY1305_MAC_LEN;