diff options
author | Andy Polyakov <appro@openssl.org> | 2018-09-06 21:54:23 +0200 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2018-11-30 12:32:25 +0000 |
commit | 75f5e944be97f28867e7c489823c889d89d0bd06 (patch) | |
tree | ee08ffb214a49a9944f5d42a49548dec35a7e30d /crypto/rsa | |
parent | e875b0cf2f10bf2adf73e0c2ec81428290f4660c (diff) |
rsa/rsa_oaep.c: remove memcpy calls from RSA_padding_check_PKCS1_OAEP.
And make RSAErr call unconditional.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Diffstat (limited to 'crypto/rsa')
-rw-r--r-- | crypto/rsa/rsa_oaep.c | 82 |
1 files changed, 51 insertions, 31 deletions
diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c index f13c6fc9e5..689e6dc222 100644 --- a/crypto/rsa/rsa_oaep.c +++ b/crypto/rsa/rsa_oaep.c @@ -120,7 +120,7 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen, const EVP_MD *mgf1md) { int i, dblen = 0, mlen = -1, one_index = 0, msg_index; - unsigned int good, found_one_byte; + unsigned int good = 0, found_one_byte, mask; const unsigned char *maskedseed, *maskeddb; /* * |em| is the encoded message, zero-padded to exactly |num| bytes: em = @@ -147,8 +147,11 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen, * the ciphertext, see PKCS #1 v2.2, section 7.1.2. * This does not leak any side-channel information. */ - if (num < flen || num < 2 * mdlen + 2) - goto decoding_err; + if (num < flen || num < 2 * mdlen + 2) { + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP_MGF1, + RSA_R_OAEP_DECODING_ERROR); + return -1; + } dblen = num - mdlen - 1; db = OPENSSL_malloc(dblen); @@ -157,25 +160,26 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen, goto cleanup; } - 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; - } + 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|. - */ - memcpy(em + num - flen, from, flen); - from = em; + /* + * Caller is encouraged to pass zero-padded message created with + * BN_bn2binpad. Trouble is that since we can't read out of |from|'s + * bounds, it's impossible to have an invariant memory access pattern + * in case |from| was not zero-padded in advance. + */ + for (from += flen, em += num, i = 0; i < num; i++) { + mask = ~constant_time_is_zero(flen); + flen -= 1 & mask; + from -= 1 & mask; + *--em = *from & mask; } + from = em; /* * The first byte must be zero, however we must not leak if this is @@ -222,32 +226,48 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen, * so plaintext-awareness ensures timing side-channels are no longer a * concern. */ - if (!good) - goto decoding_err; - msg_index = one_index + 1; mlen = dblen - msg_index; - if (tlen < mlen) { - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP_MGF1, RSA_R_DATA_TOO_LARGE); - mlen = -1; - } else { - memcpy(to, db + msg_index, mlen); - goto cleanup; + /* + * For good measure, do this check in constant tine as well. + */ + good &= constant_time_ge(tlen, mlen); + + /* + * Even though we can't fake result's length, we can pretend copying + * |tlen| bytes where |mlen| bytes would be real. Last |tlen| of |dblen| + * bytes are viewed as circular buffer with start at |tlen|-|mlen'|, + * where |mlen'| is "saturated" |mlen| value. Deducing information + * about failure or |mlen| would take attacker's ability to observe + * memory access pattern with byte granularity *as it occurs*. It + * should be noted that failure is indistinguishable from normal + * operation if |tlen| is fixed by protocol. + */ + tlen = constant_time_select_int(constant_time_lt(dblen, tlen), dblen, tlen); + msg_index = constant_time_select_int(good, msg_index, dblen - tlen); + mlen = dblen - msg_index; + for (from = db + msg_index, mask = good, i = 0; i < tlen; i++) { + unsigned int equals = constant_time_eq(i, mlen); + + from -= dblen & equals; /* if (i == dblen) rewind */ + mask &= mask ^ equals; /* if (i == dblen) mask = 0 */ + to[i] = constant_time_select_8(mask, from[i], to[i]); } - decoding_err: /* * To avoid chosen ciphertext attacks, the error message should not * reveal which kind of decoding error happened. */ RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP_MGF1, RSA_R_OAEP_DECODING_ERROR); + err_clear_last_constant_time(1 & good); cleanup: OPENSSL_cleanse(seed, sizeof(seed)); OPENSSL_clear_free(db, dblen); OPENSSL_clear_free(em, num); - return mlen; + + return constant_time_select_int(good, mlen, -1); } int PKCS1_MGF1(unsigned char *mask, long len, |