From 582ad5d4d9b7703eb089016935133e3a18ea8205 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/5254) --- crypto/rsa/rsa_oaep.c | 38 +++++++++++++++++++++++--------------- crypto/rsa/rsa_ossl.c | 38 ++++++++++++-------------------------- crypto/rsa/rsa_pk1.c | 39 +++++++++++++++++++++------------------ crypto/rsa/rsa_ssl.c | 8 ++++++++ 4 files changed, 64 insertions(+), 59 deletions(-) diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c index d4de71dfde..dfea063454 100644 --- a/crypto/rsa/rsa_oaep.c +++ b/crypto/rsa/rsa_oaep.c @@ -150,32 +150,40 @@ 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_zalloc(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|. + */ + 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_ossl.c b/crypto/rsa/rsa_ossl.c index f7b5cdca2b..feb1ab4f07 100644 --- a/crypto/rsa/rsa_ossl.c +++ b/crypto/rsa/rsa_ossl.c @@ -68,7 +68,7 @@ static int rsa_ossl_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; @@ -142,15 +142,10 @@ static int rsa_ossl_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); @@ -239,7 +234,7 @@ static int rsa_ossl_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; @@ -354,15 +349,10 @@ static int rsa_ossl_private_encrypt(int flen, const unsigned char *from, } /* - * 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); @@ -376,7 +366,6 @@ static int rsa_ossl_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; @@ -472,8 +461,7 @@ static int rsa_ossl_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: @@ -486,7 +474,7 @@ static int rsa_ossl_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_OSSL_PRIVATE_DECRYPT, RSA_R_UNKNOWN_PADDING_TYPE); @@ -509,7 +497,6 @@ static int rsa_ossl_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; @@ -574,8 +561,7 @@ static int rsa_ossl_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: @@ -585,7 +571,7 @@ static int rsa_ossl_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_OSSL_PUBLIC_DECRYPT, RSA_R_UNKNOWN_PADDING_TYPE); diff --git a/crypto/rsa/rsa_pk1.c b/crypto/rsa/rsa_pk1.c index 7d453ff989..a1134f1d48 100644 --- a/crypto/rsa/rsa_pk1.c +++ b/crypto/rsa/rsa_pk1.c @@ -175,27 +175,30 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen, if (num < 11) goto err; - em = OPENSSL_zalloc(num); - if (em == NULL) { - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE); - return -1; + if (flen != num) { + em = OPENSSL_zalloc(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|. + */ + memcpy(em + num - flen, from, flen); + from = em; } - /* - * 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); @@ -203,7 +206,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. */ @@ -232,7 +235,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: OPENSSL_clear_free(em, num); diff --git a/crypto/rsa/rsa_ssl.c b/crypto/rsa/rsa_ssl.c index cd98584be5..97ae12dfbd 100644 --- a/crypto/rsa/rsa_ssl.c +++ b/crypto/rsa/rsa_ssl.c @@ -63,6 +63,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