Avoid double-dispatch with AES_* vs aes_nohw_*. In particular, consistently pair bsaes with aes_nohw. Ideally the aes_nohw_* calls in bsaes-*.pl would be patched out and bsaes grows its own constant-time key setup (https://crbug.com/boringssl/256), but I'll sort that out separately. In the meantime, avoid going through AES_* which now dispatch. This avoids several nuisances: 1. If we were to add, say, a vpaes-armv7.pl the ABI tests would break. Fundamentally, we cannot assume that an AES_KEY has one and only one representation and must keep everything matching up. 2. AES_* functions should enable vpaes. This makes AES_* faster and constant-time for vector-capable CPUs (https://crbug.com/boringssl/263), relevant for QUIC packet number encryption, allowing us to add vpaes-armv8.pl (https://crbug.com/boringssl/246) without carrying a (likely) mostly unused AES implementation. 3. It's silly to double-dispatch when the EVP layer has already dispatched. 4. We should avoid asm calling into C. Otherwise, we need to test asm for ABI compliance as both caller and callee. Currently we only test it for callee compliance. When asm calls into asm, it *should* comply with the ABI as caller too, but mistakes don't matter as long as the called function triggers it. If the function is asm, this is fixed. If it is C, we must care about arbitrary C compiler output. Bug: 263 Change-Id: Ic85af5c765fd57cbffeaf301c3872bad6c5bbf78 Reviewed-on: https://boringssl-review.googlesource.com/c/34874 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/fipsmodule/aes/asm/bsaes-armv7.pl b/crypto/fipsmodule/aes/asm/bsaes-armv7.pl index 5387d38..9df81c1 100644 --- a/crypto/fipsmodule/aes/asm/bsaes-armv7.pl +++ b/crypto/fipsmodule/aes/asm/bsaes-armv7.pl
@@ -1113,8 +1113,8 @@ my ($keysched)=("sp"); $code.=<<___; -.extern AES_cbc_encrypt -.extern AES_decrypt +.extern aes_nohw_cbc_encrypt +.extern aes_nohw_decrypt .global bsaes_cbc_encrypt .type bsaes_cbc_encrypt,%function @@ -1123,10 +1123,10 @@ #ifndef __KERNEL__ cmp $len, #128 #ifndef __thumb__ - blo AES_cbc_encrypt + blo aes_nohw_cbc_encrypt #else bhs 1f - b AES_cbc_encrypt + b aes_nohw_cbc_encrypt 1: #endif #endif @@ -1360,7 +1360,7 @@ mov r2, $key vmov @XMM[4],@XMM[15] @ just in case ensure that IV vmov @XMM[5],@XMM[0] @ and input are preserved - bl AES_decrypt + bl aes_nohw_decrypt vld1.8 {@XMM[0]}, [$fp] @ load result veor @XMM[0], @XMM[0], @XMM[4] @ ^= IV vmov @XMM[15], @XMM[5] @ @XMM[5] holds input @@ -1390,7 +1390,7 @@ my $keysched = "sp"; $code.=<<___; -.extern AES_encrypt +.extern aes_nohw_encrypt .global bsaes_ctr32_encrypt_blocks .type bsaes_ctr32_encrypt_blocks,%function .align 5 @@ -1596,7 +1596,7 @@ mov r1, sp @ output on the stack mov r2, r7 @ key - bl AES_encrypt + bl aes_nohw_encrypt vld1.8 {@XMM[0]}, [r4]! @ load input vld1.8 {@XMM[1]}, [sp] @ load encrypted counter @@ -1658,7 +1658,7 @@ ldr r0, [ip, #4] @ iv[] mov r1, sp ldr r2, [ip, #0] @ key2 - bl AES_encrypt + bl aes_nohw_encrypt mov r0,sp @ pointer to initial tweak #endif @@ -1976,7 +1976,7 @@ mov r2, $key mov r4, $fp @ preserve fp - bl AES_encrypt + bl aes_nohw_encrypt vld1.8 {@XMM[0]}, [sp,:128] veor @XMM[0], @XMM[0], @XMM[8] @@ -2008,7 +2008,7 @@ mov r2, $key mov r4, $fp @ preserve fp - bl AES_encrypt + bl aes_nohw_encrypt vld1.8 {@XMM[0]}, [sp,:128] veor @XMM[0], @XMM[0], @XMM[8] @@ -2062,7 +2062,7 @@ ldr r0, [ip, #4] @ iv[] mov r1, sp ldr r2, [ip, #0] @ key2 - bl AES_encrypt + bl aes_nohw_encrypt mov r0, sp @ pointer to initial tweak #endif @@ -2388,7 +2388,7 @@ mov r2, $key mov r4, $fp @ preserve fp - bl AES_decrypt + bl aes_nohw_decrypt vld1.8 {@XMM[0]}, [sp,:128] veor @XMM[0], @XMM[0], @XMM[8] @@ -2420,7 +2420,7 @@ mov r2, $key mov r4, $fp @ preserve fp - bl AES_decrypt + bl aes_nohw_decrypt vld1.8 {@XMM[0]}, [sp,:128] veor @XMM[0], @XMM[0], @XMM[9] @@ -2443,7 +2443,7 @@ vst1.8 {@XMM[0]}, [sp,:128] mov r2, $key - bl AES_decrypt + bl aes_nohw_decrypt vld1.8 {@XMM[0]}, [sp,:128] veor @XMM[0], @XMM[0], @XMM[8]
diff --git a/crypto/fipsmodule/cipher/e_aes.c b/crypto/fipsmodule/cipher/e_aes.c index 7745036..69bf435 100644 --- a/crypto/fipsmodule/cipher/e_aes.c +++ b/crypto/fipsmodule/cipher/e_aes.c
@@ -114,17 +114,22 @@ dat->stream.cbc = aes_hw_cbc_encrypt; } } else if (bsaes_capable() && mode == EVP_CIPH_CBC_MODE) { - ret = AES_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks); - dat->block = AES_decrypt; + ret = aes_nohw_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks); + dat->block = aes_nohw_decrypt; dat->stream.cbc = bsaes_cbc_encrypt; } else if (vpaes_capable()) { ret = vpaes_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks); dat->block = vpaes_decrypt; dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? vpaes_cbc_encrypt : NULL; } else { - ret = AES_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks); - dat->block = AES_decrypt; - dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? AES_cbc_encrypt : NULL; + ret = aes_nohw_set_decrypt_key(key, ctx->key_len * 8, &dat->ks.ks); + dat->block = aes_nohw_decrypt; + dat->stream.cbc = NULL; +#if defined(AES_NOHW_CBC) + if (mode == EVP_CIPH_CBC_MODE) { + dat->stream.cbc = aes_nohw_cbc_encrypt; + } +#endif } } else if (hwaes_capable()) { ret = aes_hw_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks); @@ -136,17 +141,22 @@ dat->stream.ctr = aes_hw_ctr32_encrypt_blocks; } } else if (bsaes_capable() && mode == EVP_CIPH_CTR_MODE) { - ret = AES_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks); - dat->block = AES_encrypt; + ret = aes_nohw_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks); + dat->block = aes_nohw_encrypt; dat->stream.ctr = bsaes_ctr32_encrypt_blocks; } else if (vpaes_capable()) { ret = vpaes_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks); dat->block = vpaes_encrypt; dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? vpaes_cbc_encrypt : NULL; } else { - ret = AES_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks); - dat->block = AES_encrypt; - dat->stream.cbc = mode == EVP_CIPH_CBC_MODE ? AES_cbc_encrypt : NULL; + ret = aes_nohw_set_encrypt_key(key, ctx->key_len * 8, &dat->ks.ks); + dat->block = aes_nohw_encrypt; + dat->stream.cbc = NULL; +#if defined(AES_NOHW_CBC) + if (mode == EVP_CIPH_CBC_MODE) { + dat->stream.cbc = aes_nohw_cbc_encrypt; + } +#endif } if (ret < 0) { @@ -229,12 +239,12 @@ const int bsaes_ok = bsaes_capable(); const int vpaes_ok = vpaes_capable(); if (bsaes_ok && (large_inputs || !vpaes_ok)) { - AES_set_encrypt_key(key, key_bytes * 8, aes_key); + aes_nohw_set_encrypt_key(key, key_bytes * 8, aes_key); if (gcm_key != NULL) { - CRYPTO_gcm128_init_key(gcm_key, aes_key, AES_encrypt, 0); + CRYPTO_gcm128_init_key(gcm_key, aes_key, aes_nohw_encrypt, 0); } if (out_block) { - *out_block = AES_encrypt; + *out_block = aes_nohw_encrypt; } return bsaes_ctr32_encrypt_blocks; } @@ -250,12 +260,12 @@ return NULL; } - AES_set_encrypt_key(key, key_bytes * 8, aes_key); + aes_nohw_set_encrypt_key(key, key_bytes * 8, aes_key); if (gcm_key != NULL) { - CRYPTO_gcm128_init_key(gcm_key, aes_key, AES_encrypt, 0); + CRYPTO_gcm128_init_key(gcm_key, aes_key, aes_nohw_encrypt, 0); } if (out_block) { - *out_block = AES_encrypt; + *out_block = aes_nohw_encrypt; } return NULL; }