Zero cipher_data in EVP_CIPHER_CTX_cleanup

Add cleanup calls to evp_test.c

Allow reuse of cipher contexts by removing
automatic cleanup in EVP_*Final().

diff --git a/CHANGES b/CHANGES
index 6721f046e32a5098c257fbc66544f679eec1bc7c..c288d5419705946b53387dc50e63a881449fe785 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -55,6 +55,14 @@
  
  Changes between 0.9.6d and 0.9.7  [XX xxx 2002]
 
+  *) Remove most calls to EVP_CIPHER_CTX_cleanup() in evp_enc.c, this 
+     allows existing EVP_CIPHER_CTX structures to be reused after
+     calling EVP_*Final(). This behaviour is used by encryption
+     BIOs and some applications. This has the side effect that
+     applications must explicitly clean up cipher contexts with
+     EVP_CIPHER_CTX_cleanup() or they will leak memory.
+     [Steve Henson]
+
   *) Check the values of dna and dnb in bn_mul_recursive before calling
      bn_mul_comba (a non zero value means the a or b arrays do not contain
      n2 elements) and fallback to bn_mul_normal if either is not zero.

diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c
index d28a7d266e57993adefac7247e870b68d9f3b2a1..32a1c7a2e946a8ecfed44e1613af8797ba3661cf 100644 (file)
--- a/crypto/evp/evp_enc.c
+++ b/crypto/evp/evp_enc.c
@@ -102,11 +102,13 @@ int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, ENGINE *imp
                goto skip_to_init;
        if (cipher)
                {
-               /* Ensure an ENGINE left lying around from last time is cleared
+               /* Ensure a context left lying around from last time is cleared
                 * (the previous check attempted to avoid this if the same
                 * ENGINE and EVP_CIPHER could be used). */
-               if(ctx->engine)
-                       ENGINE_finish(ctx->engine);
+               EVP_CIPHER_CTX_cleanup(ctx);
+
+               /* Restore encrypt field: it is zeroed by cleanup */
+               ctx->encrypt = enc;
                if(impl)
                        {
                        if (!ENGINE_init(impl))
@@ -140,6 +142,7 @@ int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, ENGINE *imp
                        }
                else
                        ctx->engine = NULL;
+
                ctx->cipher=cipher;
                ctx->cipher_data=OPENSSL_malloc(ctx->cipher->ctx_size);
                ctx->key_len = cipher->key_len;
@@ -303,7 +306,6 @@ int EVP_EncryptFinal(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
        {
        int ret;
        ret = EVP_EncryptFinal_ex(ctx, out, outl);
-       EVP_CIPHER_CTX_cleanup(ctx);
        return ret;
        }
 
@@ -314,14 +316,12 @@ int EVP_EncryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
        b=ctx->cipher->block_size;
        if (b == 1)
                {
-               EVP_CIPHER_CTX_cleanup(ctx);
                *outl=0;
                return 1;
                }
        bl=ctx->buf_len;
        if (ctx->flags & EVP_CIPH_NO_PADDING)
                {
-               EVP_CIPHER_CTX_cleanup(ctx);
                if(bl)
                        {
                        EVPerr(EVP_F_EVP_ENCRYPTFINAL,EVP_R_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH);
@@ -336,7 +336,6 @@ int EVP_EncryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
                ctx->buf[i]=n;
        ret=ctx->cipher->do_cipher(ctx,out,ctx->buf,b);
 
-       EVP_CIPHER_CTX_cleanup(ctx);
 
        if(ret)
                *outl=b;
@@ -394,7 +393,6 @@ int EVP_DecryptFinal(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
        {
        int ret;
        ret = EVP_DecryptFinal_ex(ctx, out, outl);
-       EVP_CIPHER_CTX_cleanup(ctx);
        return ret;
        }
 
@@ -407,7 +405,6 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
        b=ctx->cipher->block_size;
        if (ctx->flags & EVP_CIPH_NO_PADDING)
                {
-               EVP_CIPHER_CTX_cleanup(ctx);
                if(ctx->buf_len)
                        {
                        EVPerr(EVP_F_EVP_DECRYPTFINAL,EVP_R_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH);
@@ -420,14 +417,12 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
                {
                if (ctx->buf_len || !ctx->final_used)
                        {
-                       EVP_CIPHER_CTX_cleanup(ctx);
                        EVPerr(EVP_F_EVP_DECRYPTFINAL,EVP_R_WRONG_FINAL_BLOCK_LENGTH);
                        return(0);
                        }
                n=ctx->final[b-1];
                if (n > b)
                        {
-                       EVP_CIPHER_CTX_cleanup(ctx);
                        EVPerr(EVP_F_EVP_DECRYPTFINAL,EVP_R_BAD_DECRYPT);
                        return(0);
                        }
@@ -435,7 +430,6 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
                        {
                        if (ctx->final[--b] != n)
                                {
-                               EVP_CIPHER_CTX_cleanup(ctx);
                                EVPerr(EVP_F_EVP_DECRYPTFINAL,EVP_R_BAD_DECRYPT);
                                return(0);
                                }
@@ -447,17 +441,21 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
                }
        else
                *outl=0;
-       EVP_CIPHER_CTX_cleanup(ctx);
        return(1);
        }
 
 int EVP_CIPHER_CTX_cleanup(EVP_CIPHER_CTX *c)
        {
-       if ((c->cipher != NULL) && (c->cipher->cleanup != NULL))
+       if (c->cipher != NULL)
                {
-               if(!c->cipher->cleanup(c)) return 0;
+               if(c->cipher->cleanup && !c->cipher->cleanup(c))
+                       return 0;
+               /* Zero cipher context data */
+               if (c->cipher_data)
+                       memset(c->cipher_data, 0, c->cipher->ctx_size);
                }
-       OPENSSL_free(c->cipher_data);
+       if (c->cipher_data)
+               OPENSSL_free(c->cipher_data);
        if (c->engine)
                /* The EVP_CIPHER we used belongs to an ENGINE, release the
                 * functional reference we held for this reason. */

diff --git a/crypto/evp/evp_test.c b/crypto/evp/evp_test.c
index 3607fe777677020f4663d5bb788a2b00e2885e17..decd0713d62bfa210e1163d1418e121bed3fccc2 100644 (file)
--- a/crypto/evp/evp_test.c
+++ b/crypto/evp/evp_test.c
@@ -209,6 +209,8 @@ static void test1(const EVP_CIPHER *c,const unsigned char *key,int kn,
        exit(9);
        }
 
+    EVP_CIPHER_CTX_cleanup(&ctx);
+
     printf("\n");
     }
 
@@ -279,6 +281,8 @@ static int test_digest(const char *digest,
 
     printf("\n");
 
+    EVP_MD_CTX_cleanup(&ctx);
+
     return 1;
     }