From ec3f996b3066ecaaec87ba5ad29c606aeac0740d Mon Sep 17 00:00:00 2001 From: Andy Polyakov Date: Sun, 4 Feb 2018 15:24:54 +0100 Subject: [PATCH] rsa/*: switch to BN_bn2binpad. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/6889) (cherry picked from commit 582ad5d4d9b7703eb089016935133e3a18ea8205) Resolved conflicts: crypto/rsa/rsa_ossl.c crypto/rsa/rsa_pk1.c --- crypto/rsa/rsa_eay.c | 39 +++++++++------------------ crypto/rsa/rsa_oaep.c | 39 ++++++++++++++++----------- crypto/rsa/rsa_pk1.c | 62 ++++++++++++++++++++++++++++++------------- crypto/rsa/rsa_ssl.c | 8 ++++++ 4 files changed, 88 insertions(+), 60 deletions(-) diff --git a/crypto/rsa/rsa_eay.c b/crypto/rsa/rsa_eay.c index b147fff8bd..b9c6855caf 100644 --- a/crypto/rsa/rsa_eay.c +++ b/crypto/rsa/rsa_eay.c @@ -114,6 +114,7 @@ #include #include #include +#include "bn_int.h" #ifndef RSA_NULL @@ -156,7 +157,7 @@ static int RSA_eay_public_encrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) { BIGNUM *f, *ret; - int i, j, k, num = 0, r = -1; + int i, num = 0, r = -1; unsigned char *buf = NULL; BN_CTX *ctx = NULL; @@ -232,15 +233,10 @@ static int RSA_eay_public_encrypt(int flen, const unsigned char *from, goto err; /* - * put in leading 0 bytes if the number is less than the length of the - * modulus + * BN_bn2binpad puts in leading 0 bytes if the number is less than + * the length of the modulus. */ - j = BN_num_bytes(ret); - i = BN_bn2bin(ret, &(to[num - j])); - for (k = 0; k < (num - i); k++) - to[k] = 0; - - r = num; + r = bn_bn2binpad(ret, to, num); err: if (ctx != NULL) { BN_CTX_end(ctx); @@ -349,7 +345,7 @@ static int RSA_eay_private_encrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) { BIGNUM *f, *ret, *res; - int i, j, k, num = 0, r = -1; + int i, num = 0, r = -1; unsigned char *buf = NULL; BN_CTX *ctx = NULL; int local_blinding = 0; @@ -459,15 +455,10 @@ static int RSA_eay_private_encrypt(int flen, const unsigned char *from, res = ret; /* - * put in leading 0 bytes if the number is less than the length of the - * modulus + * BN_bn2binpad puts in leading 0 bytes if the number is less than + * the length of the modulus. */ - j = BN_num_bytes(res); - i = BN_bn2bin(res, &(to[num - j])); - for (k = 0; k < (num - i); k++) - to[k] = 0; - - r = num; + r = bn_bn2binpad(res, to, num); err: if (ctx != NULL) { BN_CTX_end(ctx); @@ -485,7 +476,6 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from, { BIGNUM *f, *ret; int j, num = 0, r = -1; - unsigned char *p; unsigned char *buf = NULL; BN_CTX *ctx = NULL; int local_blinding = 0; @@ -576,8 +566,7 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from, if (!rsa_blinding_invert(blinding, ret, unblind, ctx)) goto err; - p = buf; - j = BN_bn2bin(ret, p); /* j is only used with no-padding mode */ + j = bn_bn2binpad(ret, buf, num); switch (padding) { case RSA_PKCS1_PADDING: @@ -592,7 +581,7 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from, r = RSA_padding_check_SSLv23(to, num, buf, j, num); break; case RSA_NO_PADDING: - r = RSA_padding_check_none(to, num, buf, j, num); + memcpy(to, buf, (r = j)); break; default: RSAerr(RSA_F_RSA_EAY_PRIVATE_DECRYPT, RSA_R_UNKNOWN_PADDING_TYPE); @@ -619,7 +608,6 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from, { BIGNUM *f, *ret; int i, num = 0, r = -1; - unsigned char *p; unsigned char *buf = NULL; BN_CTX *ctx = NULL; @@ -684,8 +672,7 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from, if (!BN_sub(ret, rsa->n, ret)) goto err; - p = buf; - i = BN_bn2bin(ret, p); + i = bn_bn2binpad(ret, buf, num); switch (padding) { case RSA_PKCS1_PADDING: @@ -695,7 +682,7 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from, r = RSA_padding_check_X931(to, num, buf, i, num); break; case RSA_NO_PADDING: - r = RSA_padding_check_none(to, num, buf, i, num); + memcpy(to, buf, (r = i)); break; default: RSAerr(RSA_F_RSA_EAY_PUBLIC_DECRYPT, RSA_R_UNKNOWN_PADDING_TYPE); diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c index 9def7a0467..3fb8f6b33d 100644 --- a/crypto/rsa/rsa_oaep.c +++ b/crypto/rsa/rsa_oaep.c @@ -153,32 +153,41 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen, dblen = num - mdlen - 1; db = OPENSSL_malloc(dblen); - em = OPENSSL_malloc(num); - if (db == NULL || em == NULL) { + if (db == NULL) { RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP_MGF1, ERR_R_MALLOC_FAILURE); goto cleanup; } - /* - * Always do this zero-padding copy (even when num == flen) to avoid - * leaking that information. The copy still leaks some side-channel - * information, but it's impossible to have a fixed memory access - * pattern since we can't read out of the bounds of |from|. - * - * TODO(emilia): Consider porting BN_bn2bin_padded from BoringSSL. - */ - memset(em, 0, num); - memcpy(em + num - flen, from, flen); + if (flen != num) { + em = OPENSSL_malloc(num); + if (em == NULL) { + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP_MGF1, + ERR_R_MALLOC_FAILURE); + goto cleanup; + } + + /* + * Caller is encouraged to pass zero-padded message created with + * BN_bn2binpad, but if it doesn't, we do this zero-padding copy + * to avoid leaking that information. The copy still leaks some + * side-channel information, but it's impossible to have a fixed + * memory access pattern since we can't read out of the bounds of + * |from|. + */ + memset(em, 0, num); + memcpy(em + num - flen, from, flen); + from = em; + } /* * The first byte must be zero, however we must not leak if this is * true. See James H. Manger, "A Chosen Ciphertext Attack on RSA * Optimal Asymmetric Encryption Padding (OAEP) [...]", CRYPTO 2001). */ - good = constant_time_is_zero(em[0]); + good = constant_time_is_zero(from[0]); - maskedseed = em + 1; - maskeddb = em + 1 + mdlen; + maskedseed = from + 1; + maskeddb = from + 1 + mdlen; if (PKCS1_MGF1(seed, mdlen, maskeddb, dblen, mgf1md)) goto cleanup; diff --git a/crypto/rsa/rsa_pk1.c b/crypto/rsa/rsa_pk1.c index 50397c335a..5d7882a3bf 100644 --- a/crypto/rsa/rsa_pk1.c +++ b/crypto/rsa/rsa_pk1.c @@ -98,6 +98,27 @@ int RSA_padding_check_PKCS1_type_1(unsigned char *to, int tlen, const unsigned char *p; p = from; + + /* + * The format is + * 00 || 01 || PS || 00 || D + * PS - padding string, at least 8 bytes of FF + * D - data. + */ + + if (num < 11) + return -1; + + /* Accept inputs with and without the leading 0-byte. */ + if (num == flen) { + if ((*p++) != 0x00) { + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_1, + RSA_R_INVALID_PADDING); + return -1; + } + flen--; + } + if ((num != (flen + 1)) || (*(p++) != 01)) { RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_1, RSA_R_BLOCK_TYPE_IS_NOT_01); @@ -203,28 +224,31 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen, if (num < 11) goto err; - em = OPENSSL_malloc(num); - if (em == NULL) { - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE); - return -1; + if (flen != num) { + em = OPENSSL_malloc(num); + if (em == NULL) { + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE); + return -1; + } + /* + * Caller is encouraged to pass zero-padded message created with + * BN_bn2binpad, but if it doesn't, we do this zero-padding copy + * to avoid leaking that information. The copy still leaks some + * side-channel information, but it's impossible to have a fixed + * memory access pattern since we can't read out of the bounds of + * |from|. + */ + memset(em, 0, num); + memcpy(em + num - flen, from, flen); + from = em; } - memset(em, 0, num); - /* - * Always do this zero-padding copy (even when num == flen) to avoid - * leaking that information. The copy still leaks some side-channel - * information, but it's impossible to have a fixed memory access - * pattern since we can't read out of the bounds of |from|. - * - * TODO(emilia): Consider porting BN_bn2bin_padded from BoringSSL. - */ - memcpy(em + num - flen, from, flen); - good = constant_time_is_zero(em[0]); - good &= constant_time_eq(em[1], 2); + good = constant_time_is_zero(from[0]); + good &= constant_time_eq(from[1], 2); found_zero_byte = 0; for (i = 2; i < num; i++) { - unsigned int equals0 = constant_time_is_zero(em[i]); + unsigned int equals0 = constant_time_is_zero(from[i]); zero_index = constant_time_select_int(~found_zero_byte & equals0, i, zero_index); @@ -232,7 +256,7 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen, } /* - * PS must be at least 8 bytes long, and it starts two bytes into |em|. + * PS must be at least 8 bytes long, and it starts two bytes into |from|. * If we never found a 0-byte, then |zero_index| is 0 and the check * also fails. */ @@ -261,7 +285,7 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen, goto err; } - memcpy(to, em + msg_index, mlen); + memcpy(to, from + msg_index, mlen); err: if (em != NULL) { diff --git a/crypto/rsa/rsa_ssl.c b/crypto/rsa/rsa_ssl.c index 746e01f64a..831f75aaf4 100644 --- a/crypto/rsa/rsa_ssl.c +++ b/crypto/rsa/rsa_ssl.c @@ -112,6 +112,14 @@ int RSA_padding_check_SSLv23(unsigned char *to, int tlen, RSAerr(RSA_F_RSA_PADDING_CHECK_SSLV23, RSA_R_DATA_TOO_SMALL); return (-1); } + /* Accept even zero-padded input */ + if (flen == num) { + if (*(p++) != 0) { + RSAerr(RSA_F_RSA_PADDING_CHECK_SSLV23, RSA_R_BLOCK_TYPE_IS_NOT_02); + return -1; + } + flen--; + } if ((num != (flen + 1)) || (*(p++) != 02)) { RSAerr(RSA_F_RSA_PADDING_CHECK_SSLV23, RSA_R_BLOCK_TYPE_IS_NOT_02); return (-1); -- 2.25.1