guidovranken / cryptofuzz

Fuzzing cryptographic libraries. Magic bug printer go brrrr.
https://guidovranken.com/2019/05/14/differential-fuzzing-of-cryptographic-libraries/
GNU General Public License v3.0
663 stars 74 forks source link

Symmetric ciphers with invalid key sizes #69

Open gilles-peskine-arm opened 4 months ago

gilles-peskine-arm commented 4 months ago

I'm adding support for CMAC and ciphers for TF-PSA-Crypto and I've run into something which I'm not sure is a feature or a bug. In OpCMAC(operation::CMAC& op), op.cipher.key.GetSize() can be incompatible with op.cipher.cipherType, e.g. a 5-byte key with CF_CIPHER("AES_128_ECB"). This would make sense if the operation was fuzzing an interface that includes validation. But as far as I understand, OpCMAC(operation::CMAC& op) returns either nullopt to mean “not supported” or the result of a successful operation.

I've looked at a couple of existing modules and if I understand correctly, they just return nullopt if the cipher is invalid. That works but it's likely to add errors, e.g. if the validation is too strict then we could silently be fuzzing nothing at all. Also it seems that every module has to do this validation.

So my understanding is:

But maybe I've missed something!

My current work: adding CMAC, running /cryptofuzz --force-module=PSA-Crypto --operations=cmac aborts with high probability on CF_ASSERT_PSA(operation.set_key(…)) because psa_import_key complains that the key size (e.g. 5 bytes) is not valid for the key type (e.g. AES).

gilles-peskine-arm commented 3 months ago

As I dig deeper, I think what's happening is

they just return nullopt if the cipher is invalid. That works but it's likely to add errors, e.g. if the validation is too strict then we could silently be fuzzing nothing at all.

And it's not just for invalid ciphers, it's for many invalid things.

I'm seeing this now in the mbedtls module, where currently unauthenticated ciphers are not being fuzzed at all. The culprit is the line

        CF_CHECK_EQ(mbedtls_cipher_update_ad(&cipher_ctx, nullptr, 0), 0);

which means that if mbedtls_cipher_update_ad returns an error, the SymmetricDecrypt or SymmetricEncrypt operation returns std::nullopt which is treated as a pass. This used to work, but in November 2022 we tightened mbedtls_cipher_update_ad to return an error on non-AEAD ciphers, and since then unauthenticated ciphers have not been fuzzed.

(I'll submit a patch for that soon. It'll also add a call to mbedtls_cipher_set_padding_mode for CBC, which became enforced in Mbed TLS 3.5.0.)