RT3425: constant-time evp_enc
authorEmilia Kasper <emilia@openssl.org>
Fri, 5 Sep 2014 12:47:33 +0000 (14:47 +0200)
committerEmilia Kasper <emilia@openssl.org>
Wed, 24 Sep 2014 14:25:04 +0000 (16:25 +0200)
Do the final padding check in EVP_DecryptFinal_ex in constant time to
avoid a timing leak from padding failure.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 4aac102f75b517bdb56b1bcfd0a856052d559f6e)

Conflicts:
crypto/evp/evp_enc.c

crypto/evp/Makefile
crypto/evp/evp_enc.c

index 30590d56bb724ae3773319d2c1276b6951408180..cacfea95f8bca1ee19a8d1ea8ddf5ca5816d802d 100644 (file)
@@ -406,7 +406,7 @@ evp_enc.o: ../../include/openssl/pkcs7.h ../../include/openssl/rand.h
 evp_enc.o: ../../include/openssl/safestack.h ../../include/openssl/sha.h
 evp_enc.o: ../../include/openssl/stack.h ../../include/openssl/symhacks.h
 evp_enc.o: ../../include/openssl/x509.h ../../include/openssl/x509_vfy.h
-evp_enc.o: ../cryptlib.h evp_enc.c evp_locl.h
+evp_enc.o: ../constant_time_locl.h ../cryptlib.h evp_enc.c evp_locl.h
 evp_err.o: ../../include/openssl/asn1.h ../../include/openssl/bio.h
 evp_err.o: ../../include/openssl/crypto.h ../../include/openssl/e_os2.h
 evp_err.o: ../../include/openssl/err.h ../../include/openssl/evp.h
index 84a3f864daba2e8f8f779014a4ee40446f66ec12..c06c83c0e33086294ee8b11235601d8d8970c2b1 100644 (file)
@@ -67,6 +67,7 @@
 #ifdef OPENSSL_FIPS
 #include <openssl/fips.h>
 #endif
+#include "../constant_time_locl.h"
 #include "evp_locl.h"
 
 #ifdef OPENSSL_FIPS
@@ -516,21 +517,21 @@ int EVP_DecryptFinal(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
 
 int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
        {
-       int i,n;
-       unsigned int b;
+       unsigned int i, b;
+        unsigned char pad, padding_good;
        *outl=0;
 
        if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER)
                {
-               i = M_do_cipher(ctx, out, NULL, 0);
-               if (i < 0)
+               int ret = M_do_cipher(ctx, out, NULL, 0);
+               if (ret < 0)
                        return 0;
                else
-                       *outl = i;
+                       *outl = ret;
                return 1;
                }
 
-       b=ctx->cipher->block_size;
+       b=(unsigned int)(ctx->cipher->block_size);
        if (ctx->flags & EVP_CIPH_NO_PADDING)
                {
                if(ctx->buf_len)
@@ -549,28 +550,34 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl)
                        return(0);
                        }
                OPENSSL_assert(b <= sizeof ctx->final);
-               n=ctx->final[b-1];
-               if (n == 0 || n > (int)b)
-                       {
-                       EVPerr(EVP_F_EVP_DECRYPTFINAL_EX,EVP_R_BAD_DECRYPT);
-                       return(0);
-                       }
-               for (i=0; i<n; i++)
+               pad=ctx->final[b-1];
+
+               padding_good = (unsigned char)(~constant_time_is_zero_8(pad));
+               padding_good &= constant_time_ge_8(b, pad);
+
+                for (i = 1; i < b; ++i)
                        {
-                       if (ctx->final[--b] != n)
-                               {
-                               EVPerr(EVP_F_EVP_DECRYPTFINAL_EX,EVP_R_BAD_DECRYPT);
-                               return(0);
-                               }
+                       unsigned char is_pad_index = constant_time_lt_8(i, pad);
+                       unsigned char pad_byte_good = constant_time_eq_8(ctx->final[b-i-1], pad);
+                       padding_good &= constant_time_select_8(is_pad_index, pad_byte_good, 0xff);
                        }
-               n=ctx->cipher->block_size-n;
-               for (i=0; i<n; i++)
-                       out[i]=ctx->final[i];
-               *outl=n;
+
+               /*
+                * At least 1 byte is always padding, so we always write b - 1
+                * bytes to avoid a timing leak. The caller is required to have |b|
+                * bytes space in |out| by the API contract.
+                */
+               for (i = 0; i < b - 1; ++i)
+                       out[i] = ctx->final[i] & padding_good;
+               /* Safe cast: for a good padding, EVP_MAX_IV_LENGTH >= b >= pad */
+               *outl = padding_good & ((unsigned char)(b - pad));
+               return padding_good & 1;
                }
        else
-               *outl=0;
-       return(1);
+               {
+               *outl = 0;
+               return 1;
+               }
        }
 
 void EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *ctx)
@@ -694,4 +701,3 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in)
                return in->cipher->ctrl((EVP_CIPHER_CTX *)in, EVP_CTRL_COPY, 0, out);
        return 1;
        }
-