commit 1510e460c646777346bebb9eb060d4c50e093813
author Bob Beck <bbe@google.com> Tue Aug 30 11:28:33 2022 -0600
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Sep 07 15:56:45 2022 +0000
tree 357f19a310e53dd3e0c390b01738b3862cd2855c
parent 2e295b91a3c441d32f985bef0dcff5e639f1f448

Add a poisoned field to EVP_CIPHER_CTX.

Poison the EVP_CIPHER_CTX structure on failures, and indicate
that it is an error to re-use an EVP_CIPHER_CTX context in another
call after a failure.

Bug: 494
Change-Id: Ibcdf28b83a2e690f7aab789d908c076d844231c6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54185
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>

crypto/fipsmodule/cipher/cipher.c

diff --git a/crypto/fipsmodule/cipher/cipher.c b/crypto/fipsmodule/cipher/cipher.c
index a126986..ee45578 100644
--- a/crypto/fipsmodule/cipher/cipher.c
+++ b/crypto/fipsmodule/cipher/cipher.c
@@ -104,6 +104,11 @@
     return 0;
   }
 
+  if (in->poisoned) {
+    OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    return 0;
+  }
+
   EVP_CIPHER_CTX_cleanup(out);
   OPENSSL_memcpy(out, in, sizeof(EVP_CIPHER_CTX));
 
@@ -226,6 +231,9 @@
 
   ctx->buf_len = 0;
   ctx->final_used = 0;
+  // Clear the poisoned flag to permit re-use of a CTX that previously had a
+  // failed operation.
+  ctx->poisoned = 0;
   return 1;
 }
 
@@ -250,6 +258,15 @@
 
 int EVP_EncryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,
                       const uint8_t *in, int in_len) {
+  if (ctx->poisoned) {
+    OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    return 0;
+  }
+  // If the first call to |cipher| succeeds and the second fails, |ctx| may be
+  // left in an indeterminate state. We set a poison flag on failure to ensure
+  // callers do not continue to use the object in that case.
+  ctx->poisoned = 1;
+
   // Ciphers that use blocks may write up to |bl| extra bytes. Ensure the output
   // does not overflow |*out_len|.
   int bl = ctx->cipher->block_size;
@@ -265,17 +282,23 @@
     } else {
       *out_len = ret;
     }
+    ctx->poisoned = 0;
     return 1;
   }
 
   if (in_len <= 0) {
     *out_len = 0;
-    return in_len == 0;
+    if (in_len == 0) {
+      ctx->poisoned = 0;
+      return 1;
+    }
+    return 0;
   }
 
   if (ctx->buf_len == 0 && block_remainder(ctx, in_len) == 0) {
     if (ctx->cipher->cipher(ctx, out, in, in_len)) {
       *out_len = in_len;
+      ctx->poisoned = 0;
       return 1;
     } else {
       *out_len = 0;
@@ -290,6 +313,7 @@
       OPENSSL_memcpy(&ctx->buf[i], in, in_len);
       ctx->buf_len += in_len;
       *out_len = 0;
+      ctx->poisoned = 0;
       return 1;
     } else {
       int j = bl - i;
@@ -319,6 +343,7 @@
     OPENSSL_memcpy(ctx->buf, &in[in_len], i);
   }
   ctx->buf_len = i;
+  ctx->poisoned = 0;
   return 1;
 }
 
@@ -326,6 +351,11 @@
   int n;
   unsigned int i, b, bl;
 
+  if (ctx->poisoned) {
+    OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    return 0;
+  }
+
   if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER) {
     // When EVP_CIPH_FLAG_CUSTOM_CIPHER is set, the return value of |cipher| is
     // the number of bytes written, or -1 on error. Otherwise the return value
@@ -371,6 +401,11 @@
 
 int EVP_DecryptUpdate(EVP_CIPHER_CTX *ctx, uint8_t *out, int *out_len,
                       const uint8_t *in, int in_len) {
+  if (ctx->poisoned) {
+    OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    return 0;
+  }
+
   // Ciphers that use blocks may write up to |bl| extra bytes. Ensure the output
   // does not overflow |*out_len|.
   unsigned int b = ctx->cipher->block_size;
@@ -433,6 +468,11 @@
   unsigned int b;
   *out_len = 0;
 
+  if (ctx->poisoned) {
+    OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+    return 0;
+  }
+
   if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER) {
     i = ctx->cipher->cipher(ctx, out, NULL, 0);
     if (i < 0) {

include/openssl/cipher.h

diff --git a/include/openssl/cipher.h b/include/openssl/cipher.h
index 380d25d..ba4b698 100644
--- a/include/openssl/cipher.h
+++ b/include/openssl/cipher.h
@@ -587,6 +587,9 @@
   int final_used;
 
   uint8_t final[EVP_MAX_BLOCK_LENGTH];  // possible final block
+
+  // Has this structure been rendered unusable by a failure.
+  int poisoned;
 } /* EVP_CIPHER_CTX */;
 
 typedef struct evp_cipher_info_st {