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

CHACHA20 POLY1305 succeeds with IV size of 16 (OpenSSL CVE-2019-1543) #4301

Closed
guidovranken opened this issue Apr 6, 2021 · 5 comments · Fixed by #5253
Closed

CHACHA20 POLY1305 succeeds with IV size of 16 (OpenSSL CVE-2019-1543) #4301

guidovranken opened this issue Apr 6, 2021 · 5 comments · Fixed by #5253
Assignees
Labels
bug component-crypto Crypto primitives and low-level interfaces size-s Estimated task size: small (~2d)

Comments

@guidovranken
Copy link
Contributor

OpenSSL addressed this: https://www.openssl.org/news/secadv/20190306.txt

I've been working around this in my fuzzer (https://github.com/guidovranken/cryptofuzz/blob/5a41fd9293b818a8094ca9c9a0b491f9eb5e2b76/modules/mbedtls/module.cpp#L390-L395) but maybe this is something you'd like to address?

@chris-jones-arm
Copy link
Contributor

Thanks for the report @guidovranken, we will try to schedule this work in soon.

Note: Having checked this in the code I can verify that the nonce is documented to only be 96 bits however it is never checked by the library.

@chris-jones-arm chris-jones-arm added bug Community component-crypto Crypto primitives and low-level interfaces size-s Estimated task size: small (~2d) labels Apr 6, 2021
@laumor01 laumor01 moved this from Incoming Items to Prioritised Backlog in OBSOLETE - SEE https://github.com/orgs/Mbed-TLS/projects/3 Apr 6, 2021
@gilles-peskine-arm
Copy link
Contributor

This affects CHACHA20 as well. The cipher module takes an iv_len parameter but does not validate it. The low-level modules (chacha20.c and chachapoly.c) both expect a fixed IV length and don't take a length parameter.

This is only a problem in the classic cipher interface. The PSA code calls cipher for CHACHA20 and has its own nonce validation for CHACAHA20_POLY1305.

@gilles-peskine-arm
Copy link
Contributor

This issue affects modes with a fixed IV size (all except GCM and CCM), and only if the fixed size is neither 0 nor MBEDTLS_MAX_IV_LENGTH (which is 16). As a consequence, the only affected modes are ChaCha20 and Chacha20-Poly1305.

@AndrzejKurek
Copy link
Contributor

The cipher behavior fix is trivial, but the way we handle iv in our tests is a problem.
We use the same size of iv (16) for our enc/dec tests using test_suite_cipher.function, and I don't see a trivial way to fix it apart from the cumbersome way of adding an iv_len parameter to all of the tests and make an exception to use 12 for ChaCha20 and ChaCha20-Poly1305.
Do you have anything better in mind, @gilles-peskine-arm?

@gilles-peskine-arm
Copy link
Contributor

@AndrzejKurek For the functions that have a hard-coded IV, I think making the IV length a function of the cipher ID is a reasonable solution.

For negative tests, I guess we have to add a new function anyway?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces size-s Estimated task size: small (~2d)
Projects
None yet
4 participants