From 3538b0f7ad7c4b67788f444827718a89ffb5b08d Mon Sep 17 00:00:00 2001 From: Pauli Date: Mon, 1 Apr 2019 10:04:57 +1000 Subject: [PATCH] Move the AES-XTS mode duplicated key check into the init_key function rather than the update call. The means an earlier error return at the cost of some duplicated code. Reviewed-by: Matthias St. Pierre (Merged from https://github.com/openssl/openssl/pull/8625) --- crypto/err/openssl.txt | 4 + crypto/evp/e_aes.c | 126 ++++++++++++---------- crypto/evp/evp_err.c | 6 ++ include/openssl/evperr.h | 4 + test/recipes/30-test_evp_data/evpciph.txt | 2 +- 5 files changed, 86 insertions(+), 56 deletions(-) diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 67829661ca..472413ae8f 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -754,13 +754,16 @@ ESS_F_ESS_SIGNING_CERT_NEW_INIT:102:ESS_SIGNING_CERT_new_init ESS_F_ESS_SIGNING_CERT_V2_ADD:105:ESS_SIGNING_CERT_V2_add ESS_F_ESS_SIGNING_CERT_V2_NEW_INIT:103:ESS_SIGNING_CERT_V2_new_init EVP_F_AESNI_INIT_KEY:165:aesni_init_key +EVP_F_AESNI_XTS_INIT_KEY:232:aesni_xts_init_key EVP_F_AES_GCM_CTRL:196:aes_gcm_ctrl EVP_F_AES_GCM_TLS_CIPHER:207:aes_gcm_tls_cipher EVP_F_AES_INIT_KEY:133:aes_init_key EVP_F_AES_OCB_CIPHER:169:aes_ocb_cipher EVP_F_AES_T4_INIT_KEY:178:aes_t4_init_key +EVP_F_AES_T4_XTS_INIT_KEY:233:aes_t4_xts_init_key EVP_F_AES_WRAP_CIPHER:170:aes_wrap_cipher EVP_F_AES_XTS_CIPHER:229:aes_xts_cipher +EVP_F_AES_XTS_INIT_KEY:234:aes_xts_init_key EVP_F_ALG_MODULE_INIT:177:alg_module_init EVP_F_ARIA_CCM_INIT_KEY:175:aria_ccm_init_key EVP_F_ARIA_GCM_CTRL:197:aria_gcm_ctrl @@ -2421,6 +2424,7 @@ EVP_R_UPDATE_ERROR:189:update error EVP_R_WRAP_MODE_NOT_ALLOWED:170:wrap mode not allowed EVP_R_WRONG_FINAL_BLOCK_LENGTH:109:wrong final block length EVP_R_XTS_DATA_UNIT_IS_TOO_LARGE:191:xts data unit is too large +EVP_R_XTS_DUPLICATED_KEYS:192:xts duplicated keys KDF_R_INVALID_DIGEST:100:invalid digest KDF_R_INVALID_MAC_TYPE:116:invalid mac type KDF_R_MISSING_ITERATION_COUNT:109:missing iteration count diff --git a/crypto/evp/e_aes.c b/crypto/evp/e_aes.c index b628c05f91..4f98cdc34d 100644 --- a/crypto/evp/e_aes.c +++ b/crypto/evp/e_aes.c @@ -390,22 +390,33 @@ static int aesni_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, return 1; if (key) { + /* The key is two half length keys in reality */ + const int bytes = EVP_CIPHER_CTX_key_length(ctx) / 2; + const int bits = bytes * 8; + + /* + * Verify that the two keys are different. + * + * This addresses Rogaway's vulnerability. + * See comment in aes_xts_init_key() below. + */ + if (memcmp(key, key + bytes, bytes) == 0) { + EVPerr(EVP_F_AESNI_XTS_INIT_KEY, EVP_R_XTS_DUPLICATED_KEYS); + return 0; + } + /* key_len is two AES keys */ if (enc) { - aesni_set_encrypt_key(key, EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + aesni_set_encrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) aesni_encrypt; xctx->stream = aesni_xts_encrypt; } else { - aesni_set_decrypt_key(key, EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + aesni_set_decrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) aesni_decrypt; xctx->stream = aesni_xts_decrypt; } - aesni_set_encrypt_key(key + EVP_CIPHER_CTX_key_length(ctx) / 2, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks2.ks); + aesni_set_encrypt_key(key + bytes, bits, &xctx->ks2.ks); xctx->xts.block2 = (block128_f) aesni_encrypt; xctx->xts.key1 = &xctx->ks1; @@ -796,7 +807,21 @@ static int aes_t4_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, return 1; if (key) { - int bits = EVP_CIPHER_CTX_key_length(ctx) * 4; + /* The key is two half length keys in reality */ + const int bytes = EVP_CIPHER_CTX_key_length(ctx) / 2; + const int bits = bytes * 8; + + /* + * Verify that the two keys are different. + * + * This addresses Rogaway's vulnerability. + * See comment in aes_xts_init_key() below. + */ + if (memcmp(key, key + bytes, bytes) == 0) { + EVPerr(EVP_F_AES_T4_XTS_INIT_KEY, EVP_R_XTS_DUPLICATED_KEYS); + return 0; + } + xctx->stream = NULL; /* key_len is two AES keys */ if (enc) { @@ -813,8 +838,7 @@ static int aes_t4_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, return 0; } } else { - aes_t4_set_decrypt_key(key, EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + aes_t4_set_decrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) aes_t4_decrypt; switch (bits) { case 128: @@ -828,9 +852,7 @@ static int aes_t4_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, } } - aes_t4_set_encrypt_key(key + EVP_CIPHER_CTX_key_length(ctx) / 2, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks2.ks); + aes_t4_set_encrypt_key(key + bytes, bits, &xctx->ks2.ks); xctx->xts.block2 = (block128_f) aes_t4_encrypt; xctx->xts.key1 = &xctx->ks1; @@ -3414,8 +3436,33 @@ static int aes_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, if (!iv && !key) return 1; - if (key) + if (key) { do { + /* The key is two half length keys in reality */ + const int bytes = EVP_CIPHER_CTX_key_length(ctx) / 2; + const int bits = bytes * 8; + + /* + * Verify that the two keys are different. + * + * This addresses the vulnerability described in Rogaway's + * September 2004 paper: + * + * "Efficient Instantiations of Tweakable Blockciphers and + * Refinements to Modes OCB and PMAC". + * (http://web.cs.ucdavis.edu/~rogaway/papers/offsets.pdf) + * + * FIPS 140-2 IG A.9 XTS-AES Key Generation Requirements states + * that: + * "The check for Key_1 != Key_2 shall be done at any place + * BEFORE using the keys in the XTS-AES algorithm to process + * data with them." + */ + if (memcmp(key, key + bytes, bytes) == 0) { + EVPerr(EVP_F_AES_XTS_INIT_KEY, EVP_R_XTS_DUPLICATED_KEYS); + return 0; + } + #ifdef AES_XTS_ASM xctx->stream = enc ? AES_xts_encrypt : AES_xts_decrypt; #else @@ -3425,26 +3472,20 @@ static int aes_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, #ifdef HWAES_CAPABLE if (HWAES_CAPABLE) { if (enc) { - HWAES_set_encrypt_key(key, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + HWAES_set_encrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) HWAES_encrypt; # ifdef HWAES_xts_encrypt xctx->stream = HWAES_xts_encrypt; # endif } else { - HWAES_set_decrypt_key(key, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + HWAES_set_decrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) HWAES_decrypt; # ifdef HWAES_xts_decrypt xctx->stream = HWAES_xts_decrypt; #endif } - HWAES_set_encrypt_key(key + EVP_CIPHER_CTX_key_length(ctx) / 2, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks2.ks); + HWAES_set_encrypt_key(key + bytes, bits, &xctx->ks2.ks); xctx->xts.block2 = (block128_f) HWAES_encrypt; xctx->xts.key1 = &xctx->ks1; @@ -3459,20 +3500,14 @@ static int aes_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, #ifdef VPAES_CAPABLE if (VPAES_CAPABLE) { if (enc) { - vpaes_set_encrypt_key(key, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + vpaes_set_encrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) vpaes_encrypt; } else { - vpaes_set_decrypt_key(key, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + vpaes_set_decrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) vpaes_decrypt; } - vpaes_set_encrypt_key(key + EVP_CIPHER_CTX_key_length(ctx) / 2, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks2.ks); + vpaes_set_encrypt_key(key + bytes, bits, &xctx->ks2.ks); xctx->xts.block2 = (block128_f) vpaes_encrypt; xctx->xts.key1 = &xctx->ks1; @@ -3482,22 +3517,19 @@ static int aes_xts_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key, (void)0; /* terminate potentially open 'else' */ if (enc) { - AES_set_encrypt_key(key, EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + AES_set_encrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) AES_encrypt; } else { - AES_set_decrypt_key(key, EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks1.ks); + AES_set_decrypt_key(key, bits, &xctx->ks1.ks); xctx->xts.block1 = (block128_f) AES_decrypt; } - AES_set_encrypt_key(key + EVP_CIPHER_CTX_key_length(ctx) / 2, - EVP_CIPHER_CTX_key_length(ctx) * 4, - &xctx->ks2.ks); + AES_set_encrypt_key(key + bytes, bits, &xctx->ks2.ks); xctx->xts.block2 = (block128_f) AES_encrypt; xctx->xts.key1 = &xctx->ks1; } while (0); + } if (iv) { xctx->xts.key2 = &xctx->ks2; @@ -3530,22 +3562,6 @@ static int aes_xts_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out, return 0; } - /* - * Verify that the two keys are different. - * - * This addresses the vulnerability described in Rogaway's September 2004 - * paper (http://web.cs.ucdavis.edu/~rogaway/papers/offsets.pdf): - * "Efficient Instantiations of Tweakable Blockciphers and Refinements - * to Modes OCB and PMAC". - * - * FIPS 140-2 IG A.9 XTS-AES Key Generation Requirements states that: - * "The check for Key_1 != Key_2 shall be done at any place BEFORE - * using the keys in the XTS-AES algorithm to process data with them." - */ - if (CRYPTO_memcmp(xctx->xts.key1, xctx->xts.key2, - EVP_CIPHER_CTX_key_length(ctx) / 2) == 0) - return 0; - if (xctx->stream) (*xctx->stream) (in, out, len, xctx->xts.key1, xctx->xts.key2, diff --git a/crypto/evp/evp_err.c b/crypto/evp/evp_err.c index a3e01fdd5d..1a4f3819b2 100644 --- a/crypto/evp/evp_err.c +++ b/crypto/evp/evp_err.c @@ -15,13 +15,17 @@ static const ERR_STRING_DATA EVP_str_functs[] = { {ERR_PACK(ERR_LIB_EVP, EVP_F_AESNI_INIT_KEY, 0), "aesni_init_key"}, + {ERR_PACK(ERR_LIB_EVP, EVP_F_AESNI_XTS_INIT_KEY, 0), "aesni_xts_init_key"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_GCM_CTRL, 0), "aes_gcm_ctrl"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_GCM_TLS_CIPHER, 0), "aes_gcm_tls_cipher"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_INIT_KEY, 0), "aes_init_key"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_OCB_CIPHER, 0), "aes_ocb_cipher"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_T4_INIT_KEY, 0), "aes_t4_init_key"}, + {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_T4_XTS_INIT_KEY, 0), + "aes_t4_xts_init_key"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_WRAP_CIPHER, 0), "aes_wrap_cipher"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_XTS_CIPHER, 0), "aes_xts_cipher"}, + {ERR_PACK(ERR_LIB_EVP, EVP_F_AES_XTS_INIT_KEY, 0), "aes_xts_init_key"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_ALG_MODULE_INIT, 0), "alg_module_init"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_ARIA_CCM_INIT_KEY, 0), "aria_ccm_init_key"}, {ERR_PACK(ERR_LIB_EVP, EVP_F_ARIA_GCM_CTRL, 0), "aria_gcm_ctrl"}, @@ -307,6 +311,8 @@ static const ERR_STRING_DATA EVP_str_reasons[] = { "wrong final block length"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_XTS_DATA_UNIT_IS_TOO_LARGE), "xts data unit is too large"}, + {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_XTS_DUPLICATED_KEYS), + "xts duplicated keys"}, {0, NULL} }; diff --git a/include/openssl/evperr.h b/include/openssl/evperr.h index 34d5e60c68..da16a1052b 100644 --- a/include/openssl/evperr.h +++ b/include/openssl/evperr.h @@ -24,13 +24,16 @@ int ERR_load_EVP_strings(void); * EVP function codes. */ # define EVP_F_AESNI_INIT_KEY 165 +# define EVP_F_AESNI_XTS_INIT_KEY 232 # define EVP_F_AES_GCM_CTRL 196 # define EVP_F_AES_GCM_TLS_CIPHER 207 # define EVP_F_AES_INIT_KEY 133 # define EVP_F_AES_OCB_CIPHER 169 # define EVP_F_AES_T4_INIT_KEY 178 +# define EVP_F_AES_T4_XTS_INIT_KEY 233 # define EVP_F_AES_WRAP_CIPHER 170 # define EVP_F_AES_XTS_CIPHER 229 +# define EVP_F_AES_XTS_INIT_KEY 234 # define EVP_F_ALG_MODULE_INIT 177 # define EVP_F_ARIA_CCM_INIT_KEY 175 # define EVP_F_ARIA_GCM_CTRL 197 @@ -228,5 +231,6 @@ int ERR_load_EVP_strings(void); # define EVP_R_WRAP_MODE_NOT_ALLOWED 170 # define EVP_R_WRONG_FINAL_BLOCK_LENGTH 109 # define EVP_R_XTS_DATA_UNIT_IS_TOO_LARGE 191 +# define EVP_R_XTS_DUPLICATED_KEYS 192 #endif diff --git a/test/recipes/30-test_evp_data/evpciph.txt b/test/recipes/30-test_evp_data/evpciph.txt index c6a117cf50..7c87a6fd08 100644 --- a/test/recipes/30-test_evp_data/evpciph.txt +++ b/test/recipes/30-test_evp_data/evpciph.txt @@ -1184,7 +1184,7 @@ Key = 0000000000000000000000000000000000000000000000000000000000000000 IV = 00000000000000000000000000000000 Plaintext = 0000000000000000000000000000000000000000000000000000000000000000 Ciphertext = 917cf69ebd68b2ec9b9fe9a3eadda692cd43d2f59598ed858c02c2652fbf922e -Result = CIPHERUPDATE_ERROR +Result = KEY_SET_ERROR Cipher = aes-128-xts Key = 1111111111111111111111111111111122222222222222222222222222222222 -- 2.25.1