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 <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/8625)

3 files changed, 81 insertions, 55 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}
 };