Search
j0ke.net Open Build Service
>
Projects
>
home:netmax
:
monitoring
>
openssl1
> 0008-rsa-rsa_oaep.c-remove-memcpy-calls-from-RSA_padding_.patch
Sign Up
|
Log In
Username
Password
Cancel
Overview
Repositories
Revisions
Requests
Users
Advanced
Attributes
Meta
File 0008-rsa-rsa_oaep.c-remove-memcpy-calls-from-RSA_padding_.patch of Package openssl1
From b29b91bc7ea8dfe47d873d7953175c57556a4488 Mon Sep 17 00:00:00 2001 From: Andy Polyakov <appro@openssl.org> Date: Thu, 6 Sep 2018 21:54:23 +0200 Subject: [PATCH 4/5] 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> (cherry picked from commit 75f5e944be97f28867e7c489823c889d89d0bd06) Resolved conflicts: crypto/rsa/rsa_oaep.c (Merged from https://github.com/openssl/openssl/pull/7737) --- crypto/rsa/rsa_oaep.c | 96 +++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 40 deletions(-) Index: openssl-1.0.1i/crypto/rsa/rsa_oaep.c =================================================================== --- openssl-1.0.1i.orig/crypto/rsa/rsa_oaep.c +++ openssl-1.0.1i/crypto/rsa/rsa_oaep.c @@ -121,7 +121,7 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(un const EVP_MD *md, const EVP_MD *mgf1md) { int i, dblen, 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 = Y || maskedSeed || maskedDB */ @@ -146,8 +146,11 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(un * 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); @@ -156,26 +159,25 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(un goto cleanup; } - if (flen != num) { - em = OPENSSL_malloc(num); - if (em == NULL) { + 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; } + /* + * 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 @@ -221,31 +223,46 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(un * 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: - if (db != NULL) OPENSSL_free(db); - if (em != NULL) OPENSSL_free(em); - return mlen; + OPENSSL_cleanse(seed, sizeof(seed)); + OPENSSL_cleanse(db, dblen); + OPENSSL_free(db); + OPENSSL_cleanse(em, num); + OPENSSL_free(em); + + return constant_time_select_int(good, mlen, -1); } int PKCS1_MGF1(unsigned char *mask, long len,