From fd367b4ce37d8f8353deb93fd7677ca636881d81 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 9 Apr 2019 15:32:33 +0100 Subject: [PATCH] Deprecate AES_ige_encrypt() and AES_bi_ige_encrypt() These undocumented functions were never integrated into the EVP layer and implement the AES Infinite Garble Extension (IGE) mode and AES Bi-directional IGE mode. These modes were never formally standardised and usage of these functions is believed to be very small. In particular AES_bi_ige_encrypt() has a known bug. It accepts 2 AES keys, but only one is ever used. The security implications are believed to be minimal, but this issue was never fixed for backwards compatibility reasons. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/8710) --- CHANGES | 11 +++++++++++ apps/speed.c | 10 +++++++++- crypto/aes/aes_ige.c | 14 ++++++++++++++ include/openssl/aes.h | 2 ++ test/igetest.c | 17 +++++++++++------ util/libcrypto.num | 4 ++-- 6 files changed, 49 insertions(+), 9 deletions(-) diff --git a/CHANGES b/CHANGES index 11c80b762f..164787c45d 100644 --- a/CHANGES +++ b/CHANGES @@ -9,6 +9,17 @@ Changes between 1.1.1 and 3.0.0 [xx XXX xxxx] + *) The functions AES_ige_encrypt() and AES_bi_ige_encrypt() have been + deprecated. These undocumented functions were never integrated into the EVP + layer and implement the AES Infinite Garble Extension (IGE) mode and AES + Bi-directional IGE mode. These modes were never formally standardised and + usage of these functions is believed to be very small. In particular + AES_bi_ige_encrypt() has a known bug. It accepts 2 AES keys, but only one + is ever used. The security implications are believed to be minimal, but + this issue was never fixed for backwards compatibility reasons. New code + should not use these modes. + [Matt Caswell] + *) Add prediction resistance to the DRBG reseeding process. [Paul Dale] diff --git a/apps/speed.c b/apps/speed.c index e9ed8b54f7..72826f821c 100644 --- a/apps/speed.c +++ b/apps/speed.c @@ -166,10 +166,12 @@ static int DES_ede3_cbc_encrypt_loop(void *args); #endif static int AES_cbc_128_encrypt_loop(void *args); static int AES_cbc_192_encrypt_loop(void *args); -static int AES_ige_128_encrypt_loop(void *args); static int AES_cbc_256_encrypt_loop(void *args); +#if !OPENSSL_API_3 +static int AES_ige_128_encrypt_loop(void *args); static int AES_ige_192_encrypt_loop(void *args); static int AES_ige_256_encrypt_loop(void *args); +#endif static int CRYPTO_gcm128_aad_loop(void *args); static int RAND_bytes_loop(void *args); static int EVP_Update_loop(void *args); @@ -428,9 +430,11 @@ static const OPT_PAIR doit_choices[] = { {"aes-128-cbc", D_CBC_128_AES}, {"aes-192-cbc", D_CBC_192_AES}, {"aes-256-cbc", D_CBC_256_AES}, +#if !OPENSSL_API_3 {"aes-128-ige", D_IGE_128_AES}, {"aes-192-ige", D_IGE_192_AES}, {"aes-256-ige", D_IGE_256_AES}, +#endif #ifndef OPENSSL_NO_RC2 {"rc2-cbc", D_CBC_RC2}, {"rc2", D_CBC_RC2}, @@ -869,6 +873,7 @@ static int AES_cbc_256_encrypt_loop(void *args) return count; } +#if !OPENSSL_API_3 static int AES_ige_128_encrypt_loop(void *args) { loopargs_t *tempargs = *(loopargs_t **) args; @@ -904,6 +909,7 @@ static int AES_ige_256_encrypt_loop(void *args) (size_t)lengths[testnum], &aes_ks3, iv, AES_ENCRYPT); return count; } +#endif static int CRYPTO_gcm128_aad_loop(void *args) { @@ -2429,6 +2435,7 @@ int speed_main(int argc, char **argv) } } +#if !OPENSSL_API_3 if (doit[D_IGE_128_AES]) { for (testnum = 0; testnum < size_num; testnum++) { print_message(names[D_IGE_128_AES], c[D_IGE_128_AES][testnum], @@ -2462,6 +2469,7 @@ int speed_main(int argc, char **argv) print_result(D_IGE_256_AES, testnum, count, d); } } +#endif if (doit[D_GHASH]) { for (i = 0; i < loopargs_len; i++) { loopargs[i].gcm_ctx = diff --git a/crypto/aes/aes_ige.c b/crypto/aes/aes_ige.c index e19922a1c4..351c173459 100644 --- a/crypto/aes/aes_ige.c +++ b/crypto/aes/aes_ige.c @@ -9,6 +9,10 @@ #include "internal/cryptlib.h" +#if OPENSSL_API_3 +NON_EMPTY_TRANSLATION_UNIT +#else + #include #include "aes_locl.h" @@ -34,6 +38,7 @@ typedef struct { /* N.B. The IV for this mode is _twice_ the block size */ +/* Use of this function is deprecated. */ void AES_ige_encrypt(const unsigned char *in, unsigned char *out, size_t length, const AES_KEY *key, unsigned char *ivec, const int enc) @@ -162,6 +167,14 @@ void AES_ige_encrypt(const unsigned char *in, unsigned char *out, /* * Note that its effectively impossible to do biIGE in anything other * than a single pass, so no provision is made for chaining. + * + * NB: The implementation of AES_bi_ige_encrypt has a bug. It is supposed to use + * 2 AES keys, but in fact only one is ever used. This bug has been present + * since this code was first implemented. It is believed to have minimal + * security impact in practice and has therefore not been fixed for backwards + * compatibility reasons. + * + * Use of this function is deprecated. */ /* N.B. The IV for this mode is _four times_ the block size */ @@ -282,3 +295,4 @@ void AES_bi_ige_encrypt(const unsigned char *in, unsigned char *out, } } } +#endif diff --git a/include/openssl/aes.h b/include/openssl/aes.h index e0e5ff3bae..060aa0f67c 100644 --- a/include/openssl/aes.h +++ b/include/openssl/aes.h @@ -67,6 +67,7 @@ void AES_cfb8_encrypt(const unsigned char *in, unsigned char *out, void AES_ofb128_encrypt(const unsigned char *in, unsigned char *out, size_t length, const AES_KEY *key, unsigned char *ivec, int *num); +# if !OPENSSL_API_3 /* NB: the IV is _two_ blocks long */ void AES_ige_encrypt(const unsigned char *in, unsigned char *out, size_t length, const AES_KEY *key, @@ -76,6 +77,7 @@ void AES_bi_ige_encrypt(const unsigned char *in, unsigned char *out, size_t length, const AES_KEY *key, const AES_KEY *key2, const unsigned char *ivec, const int enc); +# endif int AES_wrap_key(AES_KEY *key, const unsigned char *iv, unsigned char *out, diff --git a/test/igetest.c b/test/igetest.c index 9cc551f48e..a4b8cfab4a 100644 --- a/test/igetest.c +++ b/test/igetest.c @@ -15,19 +15,21 @@ #include "internal/nelem.h" #include "testutil.h" -#define TEST_SIZE 128 -#define BIG_TEST_SIZE 10240 +#if !OPENSSL_API_3 -#if BIG_TEST_SIZE < TEST_SIZE -#error BIG_TEST_SIZE is smaller than TEST_SIZE -#endif +# define TEST_SIZE 128 +# define BIG_TEST_SIZE 10240 + +# if BIG_TEST_SIZE < TEST_SIZE +# error BIG_TEST_SIZE is smaller than TEST_SIZE +# endif static unsigned char rkey[16]; static unsigned char rkey2[16]; static unsigned char plaintext[BIG_TEST_SIZE]; static unsigned char saved_iv[AES_BLOCK_SIZE * 4]; -#define MAX_VECTOR_SIZE 64 +# define MAX_VECTOR_SIZE 64 struct ige_test { const unsigned char key[16]; @@ -432,9 +434,11 @@ static int test_bi_ige_garble3(void) /* Fail if there is more than 1% matching bytes */ return TEST_size_t_le(matches, sizeof(checktext) / 100); } +#endif int setup_tests(void) { +#if !OPENSSL_API_3 RAND_bytes(rkey, sizeof(rkey)); RAND_bytes(rkey2, sizeof(rkey2)); RAND_bytes(plaintext, sizeof(plaintext)); @@ -450,5 +454,6 @@ int setup_tests(void) ADD_TEST(test_bi_ige_garble3); ADD_ALL_TESTS(test_ige_vectors, OSSL_NELEM(ige_test_vectors)); ADD_ALL_TESTS(test_bi_ige_vectors, OSSL_NELEM(bi_ige_test_vectors)); +#endif return 1; } diff --git a/util/libcrypto.num b/util/libcrypto.num index 3704a63556..c14523ea20 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -684,7 +684,7 @@ PKCS7_SIGNER_INFO_it 683 3_0_0 EXIST:!EXPORT_VAR_AS_FUNCTION: PKCS7_SIGNER_INFO_it 683 3_0_0 EXIST:EXPORT_VAR_AS_FUNCTION:FUNCTION: CRYPTO_ocb128_copy_ctx 684 3_0_0 EXIST::FUNCTION:OCB TS_REQ_get_ext_d2i 685 3_0_0 EXIST::FUNCTION:TS -AES_ige_encrypt 686 3_0_0 EXIST::FUNCTION: +AES_ige_encrypt 686 3_0_0 EXIST::FUNCTION:DEPRECATEDIN_3 d2i_SXNET 687 3_0_0 EXIST::FUNCTION: CTLOG_get0_log_id 688 3_0_0 EXIST::FUNCTION:CT CMS_RecipientInfo_ktri_get0_signer_id 689 3_0_0 EXIST::FUNCTION:CMS @@ -1456,7 +1456,7 @@ EVP_PKEY_get0_DH 1442 3_0_0 EXIST::FUNCTION:DH d2i_OCSP_CRLID 1443 3_0_0 EXIST::FUNCTION:OCSP EVP_CIPHER_CTX_set_padding 1444 3_0_0 EXIST::FUNCTION: CTLOG_new_from_base64 1445 3_0_0 EXIST::FUNCTION:CT -AES_bi_ige_encrypt 1446 3_0_0 EXIST::FUNCTION: +AES_bi_ige_encrypt 1446 3_0_0 EXIST::FUNCTION:DEPRECATEDIN_3 ERR_pop_to_mark 1447 3_0_0 EXIST::FUNCTION: CRL_DIST_POINTS_new 1449 3_0_0 EXIST::FUNCTION: EVP_PKEY_get0_asn1 1450 3_0_0 EXIST::FUNCTION: