From 60b7d3bbb509e2be874824c8fa1a0fa2379ed5ff Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Wed, 17 Dec 2014 14:49:28 +0100 Subject: [PATCH] Revert "RT3425: constant-time evp_enc" Causes more problems than it fixes: even though error codes are not part of the stable API, several users rely on the specific error code, and the change breaks them. Conversely, we don't have any concrete use-cases for constant-time behaviour here. This reverts commit f2df488a1c7402e48c21c83e937955dfe9f40bee. Reviewed-by: Andy Polyakov --- crypto/evp/Makefile | 2 +- crypto/evp/evp_enc.c | 55 +++++++++++++++++++------------------------- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/crypto/evp/Makefile b/crypto/evp/Makefile index fa25bff6b5..5d0c6b7db4 100644 --- a/crypto/evp/Makefile +++ b/crypto/evp/Makefile @@ -383,7 +383,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: ../constant_time_locl.h ../cryptlib.h evp_enc.c evp_locl.h +evp_enc.o: ../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 diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c index 7b1842a52a..e3e1147969 100644 --- a/crypto/evp/evp_enc.c +++ b/crypto/evp/evp_enc.c @@ -67,7 +67,6 @@ #ifdef OPENSSL_FIPS #include #endif -#include "constant_time_locl.h" #include "evp_locl.h" #ifdef OPENSSL_FIPS @@ -501,21 +500,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) { - unsigned int i, b; - unsigned char pad, padding_good; + int i,n; + unsigned int b; *outl=0; if (ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_CIPHER) { - int ret = M_do_cipher(ctx, out, NULL, 0); - if (ret < 0) + i = M_do_cipher(ctx, out, NULL, 0); + if (i < 0) return 0; else - *outl = ret; + *outl = i; return 1; } - b=(unsigned int)(ctx->cipher->block_size); + b=ctx->cipher->block_size; if (ctx->flags & EVP_CIPH_NO_PADDING) { if(ctx->buf_len) @@ -534,34 +533,28 @@ int EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl) return(0); } OPENSSL_assert(b <= sizeof ctx->final); - 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) + n=ctx->final[b-1]; + if (n == 0 || n > (int)b) { - 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); + EVPerr(EVP_F_EVP_DECRYPTFINAL_EX,EVP_R_BAD_DECRYPT); + return(0); } - - /* - * 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; + for (i=0; ifinal[--b] != n) + { + EVPerr(EVP_F_EVP_DECRYPTFINAL_EX,EVP_R_BAD_DECRYPT); + return(0); + } + } + n=ctx->cipher->block_size-n; + for (i=0; ifinal[i]; + *outl=n; } else - { - *outl = 0; - return 1; - } + *outl=0; + return(1); } void EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *ctx) -- 2.25.1