Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix memory overrun in rsa padding check functions #8438

Closed
wants to merge 1 commit into from
Closed

Fix memory overrun in rsa padding check functions #8438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 12 additions & 12 deletions crypto/rsa/rsa_oaep.c
View file
Open in desktop
Expand Up @@ -144,7 +144,7 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen,
* |num| is the length of the modulus; |flen| is the length of the
* encoded message. Therefore, for any |from| that was obtained by
* decrypting a ciphertext, we must have |flen| <= |num|. Similarly,
* num < 2 * mdlen + 2 must hold for the modulus irrespective of
* |num| >= 2 * |mdlen| + 2 must hold for the modulus irrespective of
* the ciphertext, see PKCS #1 v2.2, section 7.1.2.
* This does not leak any side-channel information.
*/
Expand Down Expand Up @@ -180,17 +180,16 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen,
from -= 1 & mask;
*--em = *from & mask;
}
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(from[0]);
good = constant_time_is_zero(em[0]);

maskedseed = from + 1;
maskeddb = from + 1 + mdlen;
maskedseed = em + 1;
maskeddb = em + 1 + mdlen;

if (PKCS1_MGF1(seed, mdlen, maskeddb, dblen, mgf1md))
goto cleanup;
Expand Down Expand Up @@ -231,7 +230,7 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen,
mlen = dblen - msg_index;

/*
* For good measure, do this check in constant tine as well.
* For good measure, do this check in constant time as well.
*/
good &= constant_time_ge(tlen, mlen);

Expand All @@ -245,15 +244,16 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen,
* 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);
tlen = constant_time_select_int(constant_time_lt(dblen - mdlen - 1, tlen),
dblen - mdlen - 1, 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);
for (mask = good, i = 0; i < tlen; i++) {
unsigned int equals = constant_time_eq(msg_index, dblen);

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]);
msg_index -= tlen & equals; /* rewind at EOF */
mask &= ~equals; /* mask = 0 at EOF */
to[i] = constant_time_select_8(mask, db[msg_index++], to[i]);
}

/*
Expand Down
22 changes: 11 additions & 11 deletions crypto/rsa/rsa_pk1.c
View file
Open in desktop
Expand Up @@ -241,23 +241,22 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
from -= 1 & mask;
*--em = *from & mask;
}
from = em;

good = constant_time_is_zero(from[0]);
good &= constant_time_eq(from[1], 2);
good = constant_time_is_zero(em[0]);
good &= constant_time_eq(em[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]);
unsigned int equals0 = constant_time_is_zero(em[i]);

zero_index = constant_time_select_int(~found_zero_byte & equals0,
i, zero_index);
found_zero_byte |= equals0;
}

/*
* PS must be at least 8 bytes long, and it starts two bytes into |from|.
* PS must be at least 8 bytes long, and it starts two bytes into |em|.
* If we never found a 0-byte, then |zero_index| is 0 and the check
* also fails.
*/
Expand Down Expand Up @@ -285,15 +284,16 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
* should be noted that failure is indistinguishable from normal
* operation if |tlen| is fixed by protocol.
*/
tlen = constant_time_select_int(constant_time_lt(num, tlen), num, tlen);
tlen = constant_time_select_int(constant_time_lt(num - 11, tlen),
num - 11, 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);
for (mask = good, i = 0; i < tlen; i++) {
unsigned int equals = constant_time_eq(msg_index, num);

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]);
msg_index -= tlen & equals; /* rewind at EOF */
mask &= ~equals; /* mask = 0 at EOF */
to[i] = constant_time_select_8(mask, em[msg_index++], to[i]);
}

OPENSSL_cleanse(em, num);
Expand Down
33 changes: 18 additions & 15 deletions crypto/rsa/rsa_ssl.c
View file
Open in desktop
Expand Up @@ -104,7 +104,7 @@ int RSA_padding_add_SSLv23(unsigned char *to, int tlen,

/*
* Copy of RSA_padding_check_PKCS1_type_2 with a twist that rejects padding
* if nul delimiter is preceded by 8 consecutive 0x03 bytes. It also
* if nul delimiter is not preceded by 8 consecutive 0x03 bytes. It also
* preserves error code reporting for backward compatibility.
*/
int RSA_padding_check_SSLv23(unsigned char *to, int tlen,
Expand All @@ -116,7 +116,10 @@ int RSA_padding_check_SSLv23(unsigned char *to, int tlen,
unsigned int good, found_zero_byte, mask, threes_in_row;
int zero_index = 0, msg_index, mlen = -1, err;

if (flen < 10) {
if (tlen <= 0 || flen <= 0)
return -1;

if (flen > num || num < 11) {
RSAerr(RSA_F_RSA_PADDING_CHECK_SSLV23, RSA_R_DATA_TOO_SMALL);
return (-1);
}
Expand All @@ -138,29 +141,28 @@ int RSA_padding_check_SSLv23(unsigned char *to, int tlen,
from -= 1 & mask;
*--em = *from & mask;
}
from = em;

good = constant_time_is_zero(from[0]);
good &= constant_time_eq(from[1], 2);
good = constant_time_is_zero(em[0]);
good &= constant_time_eq(em[1], 2);
err = constant_time_select_int(good, 0, RSA_R_BLOCK_TYPE_IS_NOT_02);
mask = ~good;

/* scan over padding data */
found_zero_byte = 0;
threes_in_row = 0;
for (i = 2; i < num; i++) {
unsigned int equals0 = constant_time_is_zero(from[i]);
unsigned int equals0 = constant_time_is_zero(em[i]);

zero_index = constant_time_select_int(~found_zero_byte & equals0,
i, zero_index);
found_zero_byte |= equals0;

threes_in_row += 1 & ~found_zero_byte;
threes_in_row &= found_zero_byte | constant_time_eq(from[i], 3);
threes_in_row &= found_zero_byte | constant_time_eq(em[i], 3);
}

/*
* PS must be at least 8 bytes long, and it starts two bytes into |from|.
* PS must be at least 8 bytes long, and it starts two bytes into |em|.
* If we never found a 0-byte, then |zero_index| is 0 and the check
* also fails.
*/
Expand All @@ -169,7 +171,7 @@ int RSA_padding_check_SSLv23(unsigned char *to, int tlen,
RSA_R_NULL_BEFORE_BLOCK_MISSING);
mask = ~good;

good &= constant_time_lt(threes_in_row, 8);
good &= constant_time_ge(threes_in_row, 8);
err = constant_time_select_int(mask | good, err,
RSA_R_SSLV3_ROLLBACK_ATTACK);
mask = ~good;
Expand Down Expand Up @@ -197,15 +199,16 @@ int RSA_padding_check_SSLv23(unsigned char *to, int tlen,
* should be noted that failure is indistinguishable from normal
* operation if |tlen| is fixed by protocol.
*/
tlen = constant_time_select_int(constant_time_lt(num, tlen), num, tlen);
tlen = constant_time_select_int(constant_time_lt(num - 11, tlen),
num - 11, 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);
for (mask = good, i = 0; i < tlen; i++) {
unsigned int equals = constant_time_eq(msg_index, num);

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]);
msg_index -= tlen & equals; /* rewind at EOF */
mask &= ~equals; /* mask = 0 at EOF */
to[i] = constant_time_select_8(mask, em[msg_index++], to[i]);
}

OPENSSL_cleanse(em, num);
Expand Down