Fix EVP_CIPHER_CTX_copy for AES-GCM. 7578f3f0dea091a3392f0be3216989bdc7355ad2 made it work, but 26ba48a6fbbf7b25bbfb521d3f5591e2d5a0b4bd regressed it by losing the EVP_CIPH_CUSTOM_COPY flag. Additionally, we've since added an alignment requirement to EVP_AES_GCM_CTX, which complicates things. Thanks to Guido Vranken for catching this! Bug: 270 Change-Id: I71784593dc5a34d1334c92a4daa93546ec0ee2c3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35624 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/cipher_extra/cipher_test.cc b/crypto/cipher_extra/cipher_test.cc index 62ef939..0a96bf3 100644 --- a/crypto/cipher_extra/cipher_test.cc +++ b/crypto/cipher_extra/cipher_test.cc
@@ -150,7 +150,8 @@ } static void TestOperation(FileTest *t, const EVP_CIPHER *cipher, bool encrypt, - size_t chunk_size, const std::vector<uint8_t> &key, + bool copy, size_t chunk_size, + const std::vector<uint8_t> &key, const std::vector<uint8_t> &iv, const std::vector<uint8_t> &plaintext, const std::vector<uint8_t> &ciphertext, @@ -167,45 +168,52 @@ bool is_aead = EVP_CIPHER_mode(cipher) == EVP_CIPH_GCM_MODE; - bssl::ScopedEVP_CIPHER_CTX ctx; - ASSERT_TRUE(EVP_CipherInit_ex(ctx.get(), cipher, nullptr, nullptr, nullptr, - encrypt ? 1 : 0)); + bssl::ScopedEVP_CIPHER_CTX ctx1; + ASSERT_TRUE(EVP_CipherInit_ex(ctx1.get(), cipher, nullptr, nullptr, nullptr, + encrypt ? 1 : 0)); if (t->HasAttribute("IV")) { if (is_aead) { - ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IVLEN, + ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx1.get(), EVP_CTRL_AEAD_SET_IVLEN, iv.size(), 0)); } else { - ASSERT_EQ(iv.size(), EVP_CIPHER_CTX_iv_length(ctx.get())); + ASSERT_EQ(iv.size(), EVP_CIPHER_CTX_iv_length(ctx1.get())); } } + + bssl::ScopedEVP_CIPHER_CTX ctx2; + EVP_CIPHER_CTX *ctx = ctx1.get(); + if (copy) { + ASSERT_TRUE(EVP_CIPHER_CTX_copy(ctx2.get(), ctx1.get())); + ctx = ctx2.get(); + } + if (is_aead && !encrypt) { - ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_TAG, - tag.size(), + ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, tag.size(), const_cast<uint8_t *>(tag.data()))); } // The ciphers are run with no padding. For each of the ciphers we test, the // output size matches the input size. ASSERT_EQ(in->size(), out->size()); - ASSERT_TRUE(EVP_CIPHER_CTX_set_key_length(ctx.get(), key.size())); - ASSERT_TRUE(EVP_CipherInit_ex(ctx.get(), nullptr, nullptr, key.data(), - iv.data(), -1)); + ASSERT_TRUE(EVP_CIPHER_CTX_set_key_length(ctx, key.size())); + ASSERT_TRUE( + EVP_CipherInit_ex(ctx, nullptr, nullptr, key.data(), iv.data(), -1)); // Note: the deprecated |EVP_CIPHER|-based AEAD API is sensitive to whether // parameters are NULL, so it is important to skip the |in| and |aad| // |EVP_CipherUpdate| calls when empty. if (!aad.empty()) { int unused; ASSERT_TRUE( - EVP_CipherUpdate(ctx.get(), nullptr, &unused, aad.data(), aad.size())); + EVP_CipherUpdate(ctx, nullptr, &unused, aad.data(), aad.size())); } - ASSERT_TRUE(EVP_CIPHER_CTX_set_padding(ctx.get(), 0)); + ASSERT_TRUE(EVP_CIPHER_CTX_set_padding(ctx, 0)); std::vector<uint8_t> result; - ASSERT_TRUE(DoCipher(ctx.get(), &result, *in, chunk_size)); + ASSERT_TRUE(DoCipher(ctx, &result, *in, chunk_size)); EXPECT_EQ(Bytes(*out), Bytes(result)); if (encrypt && is_aead) { uint8_t rtag[16]; ASSERT_LE(tag.size(), sizeof(rtag)); - ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_TAG, - tag.size(), rtag)); + ASSERT_TRUE( + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, tag.size(), rtag)); EXPECT_EQ(Bytes(tag), Bytes(rtag, tag.size())); } } @@ -252,14 +260,18 @@ // By default, both directions are run, unless overridden by the operation. if (operation != kDecrypt) { SCOPED_TRACE("encrypt"); - TestOperation(t, cipher, true /* encrypt */, chunk_size, key, iv, - plaintext, ciphertext, aad, tag); + TestOperation(t, cipher, true /* encrypt */, false /* no copy */, + chunk_size, key, iv, plaintext, ciphertext, aad, tag); + TestOperation(t, cipher, true /* encrypt */, true /* copy */, chunk_size, + key, iv, plaintext, ciphertext, aad, tag); } if (operation != kEncrypt) { SCOPED_TRACE("decrypt"); - TestOperation(t, cipher, false /* decrypt */, chunk_size, key, iv, - plaintext, ciphertext, aad, tag); + TestOperation(t, cipher, false /* decrypt */, false /* no copy */, + chunk_size, key, iv, plaintext, ciphertext, aad, tag); + TestOperation(t, cipher, false /* decrypt */, true /* copy */, chunk_size, + key, iv, plaintext, ciphertext, aad, tag); } } }
diff --git a/crypto/fipsmodule/cipher/e_aes.c b/crypto/fipsmodule/cipher/e_aes.c index dc94166..1ea012d 100644 --- a/crypto/fipsmodule/cipher/e_aes.c +++ b/crypto/fipsmodule/cipher/e_aes.c
@@ -456,6 +456,9 @@ case EVP_CTRL_COPY: { EVP_CIPHER_CTX *out = ptr; EVP_AES_GCM_CTX *gctx_out = aes_gcm_from_cipher_ctx(out); + // |EVP_CIPHER_CTX_copy| copies this generically, but we must redo it in + // case |out->cipher_data| and |in->cipher_data| are differently aligned. + OPENSSL_memcpy(gctx_out, gctx, sizeof(EVP_AES_GCM_CTX)); if (gctx->iv == c->iv) { gctx_out->iv = out->iv; } else { @@ -590,7 +593,7 @@ out->key_len = 16; out->iv_len = 12; out->ctx_size = sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING; - out->flags = EVP_CIPH_GCM_MODE | EVP_CIPH_CUSTOM_IV | + out->flags = EVP_CIPH_GCM_MODE | EVP_CIPH_CUSTOM_IV | EVP_CIPH_CUSTOM_COPY | EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_ALWAYS_CALL_INIT | EVP_CIPH_CTRL_INIT | EVP_CIPH_FLAG_AEAD_CIPHER; out->init = aes_gcm_init_key; @@ -658,7 +661,7 @@ out->key_len = 24; out->iv_len = 12; out->ctx_size = sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING; - out->flags = EVP_CIPH_GCM_MODE | EVP_CIPH_CUSTOM_IV | + out->flags = EVP_CIPH_GCM_MODE | EVP_CIPH_CUSTOM_IV | EVP_CIPH_CUSTOM_COPY | EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_ALWAYS_CALL_INIT | EVP_CIPH_CTRL_INIT | EVP_CIPH_FLAG_AEAD_CIPHER; out->init = aes_gcm_init_key; @@ -726,7 +729,7 @@ out->key_len = 32; out->iv_len = 12; out->ctx_size = sizeof(EVP_AES_GCM_CTX) + EVP_AES_GCM_CTX_PADDING; - out->flags = EVP_CIPH_GCM_MODE | EVP_CIPH_CUSTOM_IV | + out->flags = EVP_CIPH_GCM_MODE | EVP_CIPH_CUSTOM_IV | EVP_CIPH_CUSTOM_COPY | EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_ALWAYS_CALL_INIT | EVP_CIPH_CTRL_INIT | EVP_CIPH_FLAG_AEAD_CIPHER; out->init = aes_gcm_init_key;