From de7df1a409f766c6f98553d948c5eaaeb2ee641f Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 23 Apr 2013 18:22:26 -0400 Subject: [PATCH 10/11] constant_time_rsa_padding. This patch tweaks the OAEP padding check to be slightly more constant time and rewrites the PKCS#1 v1.5 padding check to the same end. --- crypto/rsa/rsa.h | 1 + crypto/rsa/rsa_err.c | 1 + crypto/rsa/rsa_oaep.c | 35 ++++++++++-------- crypto/rsa/rsa_pk1.c | 100 ++++++++++++++++++++++++++++++++++---------------- 4 files changed, 90 insertions(+), 47 deletions(-) diff --git a/crypto/rsa/rsa.h b/crypto/rsa/rsa.h index a3b1277..024e2f7 100644 --- a/crypto/rsa/rsa.h +++ b/crypto/rsa/rsa.h @@ -561,6 +561,7 @@ void ERR_load_RSA_strings(void); #define RSA_R_OAEP_DECODING_ERROR 121 #define RSA_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE 148 #define RSA_R_PADDING_CHECK_FAILED 114 +#define RSA_R_PKCS_DECODING_ERROR 159 #define RSA_R_P_NOT_PRIME 128 #define RSA_R_Q_NOT_PRIME 129 #define RSA_R_RSA_OPERATIONS_NOT_SUPPORTED 130 diff --git a/crypto/rsa/rsa_err.c b/crypto/rsa/rsa_err.c index db29ab0..0418f6a 100644 --- a/crypto/rsa/rsa_err.c +++ b/crypto/rsa/rsa_err.c @@ -169,6 +169,7 @@ static ERR_STRING_DATA RSA_str_reasons[]= {ERR_REASON(RSA_R_OAEP_DECODING_ERROR) ,"oaep decoding error"}, {ERR_REASON(RSA_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE),"operation not supported for this keytype"}, {ERR_REASON(RSA_R_PADDING_CHECK_FAILED) ,"padding check failed"}, +{ERR_REASON(RSA_R_PKCS_DECODING_ERROR) ,"pkcs decoding error"}, {ERR_REASON(RSA_R_P_NOT_PRIME) ,"p not prime"}, {ERR_REASON(RSA_R_Q_NOT_PRIME) ,"q not prime"}, {ERR_REASON(RSA_R_RSA_OPERATIONS_NOT_SUPPORTED),"rsa operations not supported"}, diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c index c57507d..fd6316e 100644 --- a/crypto/rsa/rsa_oaep.c +++ b/crypto/rsa/rsa_oaep.c @@ -97,30 +97,28 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen, const unsigned char *from, int flen, int num, const unsigned char *param, int plen) { - int i, dblen, mlen = -1; + int i, dblen, mlen = -1, bad; const unsigned char *maskeddb; int lzero; unsigned char *db = NULL, seed[SHA_DIGEST_LENGTH], phash[SHA_DIGEST_LENGTH]; unsigned char *padded_from; - int bad = 0; if (--num < 2 * SHA_DIGEST_LENGTH + 1) /* 'num' is the length of the modulus, i.e. does not depend on the * particular ciphertext. */ goto decoding_err; + /* lzero is the number of leading zeros. We must not leak in the case + * that this is negative. See James H. Manger, "A Chosen Ciphertext + * Attack on RSA Optimal Asymmetric Encryption Padding (OAEP) [...]", + * CRYPTO 2001). */ lzero = num - flen; - if (lzero < 0) - { - /* signalling this error immediately after detection might allow - * for side-channel attacks (e.g. timing if 'plen' is huge - * -- cf. James H. Manger, "A Chosen Ciphertext Attack on RSA Optimal - * Asymmetric Encryption Padding (OAEP) [...]", CRYPTO 2001), - * so we use a 'bad' flag */ - bad = 1; - lzero = 0; - flen = num; /* don't overflow the memcpy to padded_from */ - } + /* If lzero is negative then the MSB will be set and this arithmetic + * right shift will set bad to all ones. Otherwise it'll be all + * zeros. */ + bad = lzero >> (sizeof(int)*8 - 1); + lzero &= ~bad; + flen = (bad & num) | (~bad & flen); dblen = num - SHA_DIGEST_LENGTH; db = OPENSSL_malloc(dblen + num); @@ -130,10 +128,13 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen, return -1; } - /* Always do this zero-padding copy (even when lzero == 0) - * to avoid leaking timing info about the value of lzero. */ + /* Always do this zero-padding copy (even when lzero == 0) to avoid + * leaking timing info about the value of lzero. This sadly leaks + * side-channel information, but it's not possible to have a fixed + * memory access pattern since we can't read out of the bounds of + * |from|. */ padded_from = db + dblen; - memset(padded_from, 0, lzero); + memset(padded_from, 0, num); memcpy(padded_from + lzero, from, flen); maskeddb = padded_from + SHA_DIGEST_LENGTH; @@ -155,6 +156,8 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen, goto decoding_err; else { + /* At this point we consider timing side-channels to be moot + * because the plaintext contained the correct phash. */ for (i = SHA_DIGEST_LENGTH; i < dblen; i++) if (db[i] != 0x00) break; diff --git a/crypto/rsa/rsa_pk1.c b/crypto/rsa/rsa_pk1.c index 0cce4bf..4d1a538 100644 --- a/crypto/rsa/rsa_pk1.c +++ b/crypto/rsa/rsa_pk1.c @@ -180,47 +180,85 @@ int RSA_padding_add_PKCS1_type_2(unsigned char *to, int tlen, return(1); } +/* constant_time_byte_eq returns 1 if x == y and 0 otherwise. */ +static int constant_time_byte_eq(unsigned char a, unsigned char b) { + unsigned char z = ~(a ^ b); + z &= z >> 4; + z &= z >> 2; + z &= z >> 1; + + return z; +} + +/* constant_time_select returns x if v is 1 and y if v is 0. + * Its behavior is undefined if v takes any other value. */ +static int constant_time_select(int v, int x, int y) { + return ((~(v-1)) & x) | ((v-1) & y); +} + +/* constant_time_le returns 1 if x < y and 0 otherwise. + * x and y must be positive. */ +static int constant_time_le(int x, int y) { + return ((x - y - 1) >> (sizeof(int)*8 - 1)) & 1; +} + int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen, const unsigned char *from, int flen, int num) { - int i,j; - const unsigned char *p; + int i; + unsigned char *em = NULL; + int ret = -1; + int first_byte_is_zero, second_byte_is_two, looking_for_index; + int valid_index, zero_index = 0, msg_index; - p=from; - if ((num != (flen+1)) || (*(p++) != 02)) - { - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_BLOCK_TYPE_IS_NOT_02); - return(-1); - } -#ifdef PKCS1_CHECK - return(num-11); -#endif + /* PKCS#1 v1.5 decryption. See "PKCS #1 v2.2: RSA Cryptography + * Standard", section 7.2.2. */ - /* scan over padding data */ - j=flen-1; /* one for type. */ - for (i=0; i num) + goto err; - if (i == j) - { - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_NULL_BEFORE_BLOCK_MISSING); - return(-1); - } + if (num < 11) + goto err; - if (i < 8) + em = OPENSSL_malloc(num); + if (em == NULL) { - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_BAD_PAD_BYTE_COUNT); - return(-1); + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE); + return -1; } - i++; /* Skip over the '\0' */ - j-=i; - if (j > tlen) + memset(em, 0, num); + /* This unavoidably leaks timing information about |flen| because we + * cannot have a constant memory access pattern without accessing + * outside the bounds of |from|. */ + memcpy(em + num - flen, from, flen); + + first_byte_is_zero = constant_time_byte_eq(em[0], 0); + second_byte_is_two = constant_time_byte_eq(em[1], 2); + + looking_for_index = 1; + for (i = 2; i < num; i++) { - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_DATA_TOO_LARGE); - return(-1); + int equals0 = constant_time_byte_eq(em[i], 0); + zero_index = constant_time_select(looking_for_index&equals0, i, zero_index); + looking_for_index = constant_time_select(equals0, 0, looking_for_index); } - memcpy(to,p,(unsigned int)j); - return(j); - } + /* PS must be at least 8 bytes long, and it starts two bytes into |em|. */ + valid_index = constant_time_le(2 + 8, zero_index); + /* Skip the zero byte. */ + msg_index = zero_index + 1; + valid_index &= constant_time_le(num - msg_index, tlen); + if (!(first_byte_is_zero & second_byte_is_two & ~looking_for_index & valid_index)) + goto err; + + ret = num - msg_index; + memcpy(to, &em[msg_index], ret); + +err: + if (em != NULL) + OPENSSL_free(em); + if (ret == -1) + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, RSA_R_PKCS_DECODING_ERROR); + return ret; + } -- 1.8.2.1