Search
j0ke.net Open Build Service
>
Projects
>
home:netmax
:
monitoring
>
openssl1
> 0007-rsa-rsa_pk1.c-remove-memcpy-calls-from-RSA_padding_c.patch
Sign Up
|
Log In
Username
Password
Cancel
Overview
Repositories
Revisions
Requests
Users
Advanced
Attributes
Meta
File 0007-rsa-rsa_pk1.c-remove-memcpy-calls-from-RSA_padding_c.patch of Package openssl1
From 47f8fba64353a637cacdd8751cab25a9f3be3715 Mon Sep 17 00:00:00 2001 From: Andy Polyakov <appro@openssl.org> Date: Sat, 1 Sep 2018 12:00:33 +0200 Subject: [PATCH 3/5] rsa/rsa_pk1.c: remove memcpy calls from RSA_padding_check_PKCS1_type_2. And make RSAErr call unconditional. Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (cherry picked from commit e875b0cf2f10bf2adf73e0c2ec81428290f4660c) Resolved conflicts: crypto/rsa/rsa_pk1.c (Merged from https://github.com/openssl/openssl/pull/7737) --- crypto/rsa/rsa_pk1.c | 98 +++++++++++---------- doc/crypto/RSA_padding_add_PKCS1_type_1.pod | 7 +- 2 files changed, 58 insertions(+), 47 deletions(-) Index: openssl-1.0.1i/crypto/rsa/rsa_pk1.c =================================================================== --- openssl-1.0.1i.orig/crypto/rsa/rsa_pk1.c +++ openssl-1.0.1i/crypto/rsa/rsa_pk1.c @@ -207,7 +207,7 @@ int RSA_padding_check_PKCS1_type_2(unsig int i; /* |em| is the encoded message, zero-padded to exactly |num| bytes */ unsigned char *em = NULL; - unsigned int good, found_zero_byte; + unsigned int good, found_zero_byte, mask; int zero_index = 0, msg_index, mlen = -1; if (tlen < 0 || flen < 0) @@ -216,39 +216,41 @@ int RSA_padding_check_PKCS1_type_2(unsig /* PKCS#1 v1.5 decryption. See "PKCS #1 v2.2: RSA Cryptography * Standard", section 7.2.2. */ - if (flen > num) - goto err; - - if (num < 11) - goto err; + if (flen > num || num < 11) { + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, + RSA_R_PKCS_DECODING_ERROR); + 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; + 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. 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; good = constant_time_is_zero(from[0]); good &= constant_time_eq(from[1], 2); + /* scan over padding data */ found_zero_byte = 0; for (i = 2; i < num; i++) { unsigned int equals0 = constant_time_is_zero(from[i]); - zero_index = constant_time_select_int(~found_zero_byte & equals0, i, zero_index); + zero_index = constant_time_select_int(~found_zero_byte & equals0, + i, zero_index); found_zero_byte |= equals0; } @@ -257,36 +259,42 @@ int RSA_padding_check_PKCS1_type_2(unsig * If we never found a 0-byte, then |zero_index| is 0 and the check * also fails. */ - good &= constant_time_ge((unsigned int)(zero_index), 2 + 8); + good &= constant_time_ge(zero_index, 2 + 8); /* Skip the zero byte. This is incorrect if we never found a zero-byte * but in this case we also do not copy the message out. */ msg_index = zero_index + 1; mlen = num - msg_index; - /* For good measure, do this check in constant time as well; it could - * leak something if |tlen| was assuming valid padding. */ - good &= constant_time_ge((unsigned int)(tlen), (unsigned int)(mlen)); + /* + * For good measure, do this check in constant time as well. + */ + good &= constant_time_ge(tlen, mlen); /* - * We can't continue in constant-time because we need to copy the result - * and we cannot fake its length. This unavoidably leaks timing - * information at the API boundary. - * TODO(emilia): this could be addressed at the call site, - * see BoringSSL commit 0aa0767340baf925bda4804882aab0cb974b2d26. + * Even though we can't fake result's length, we can pretend copying + * |tlen| bytes where |mlen| bytes would be real. Last |tlen| of |num| + * 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. */ - if (!good) - { - mlen = -1; - goto err; + tlen = constant_time_select_int(constant_time_lt(num, tlen), num, tlen); + msg_index = constant_time_select_int(good, msg_index, num - tlen); + mlen = num - msg_index; + for (from += msg_index, mask = good, i = 0; i < tlen; i++) { + unsigned int equals = constant_time_eq(i, mlen); + from -= tlen & equals; /* if (i == mlen) rewind */ + mask &= mask ^ equals; /* if (i == mlen) mask = 0 */ + to[i] = constant_time_select_8(mask, from[i], to[i]); } - memcpy(to, from + msg_index, mlen); + OPENSSL_cleanse(em, num); + OPENSSL_free(em); + RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, RSA_R_PKCS_DECODING_ERROR); + err_clear_last_constant_time(1 & good); -err: - if (em != NULL) - OPENSSL_free(em); - if (mlen == -1) - RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, RSA_R_PKCS_DECODING_ERROR); - return mlen; + return constant_time_select_int(good, mlen, -1); }