Also tweak s3_cbc.c to use new constant-time methods.
Also fix memory leaks from internal errors in RSA_padding_check_PKCS1_OAEP_mgf1
This patch is based on the original RT submission by Adam Langley <agl@chromium.org>,
as well as code from BoringSSL and OpenSSL.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Conflicts:
crypto/rsa/rsa_oaep.c
#endif
/*
- * The following methods return a bitmask of all ones (0xff...f) for true
+ * The boolean methods return a bitmask of all ones (0xff...f) for true
* and 0 for false. This is useful for choosing a value based on the result
* of a conditional in constant time. For example,
*
* can be written as
*
* unsigned int lt = constant_time_lt(a, b);
- * c = a & lt | b & ~lt;
+ * c = constant_time_select(lt, a, b);
*/
/*
/* Convenience method for getting an 8-bit mask. */
static inline unsigned char constant_time_eq_8(unsigned int a, unsigned int b);
+/*
+ * Returns (mask & a) | (~mask & b).
+ *
+ * When |mask| is all 1s or all 0s (as returned by the methods above),
+ * the select methods return either |a| (if |mask| is nonzero) or |b|
+ * (if |mask| is zero).
+ */
+static inline unsigned int constant_time_select(unsigned int mask,
+ unsigned int a, unsigned int b);
+/* Convenience method for unsigned chars. */
+static inline unsigned char constant_time_select_8(unsigned char mask,
+ unsigned char a, unsigned char b);
+/* Convenience method for signed integers. */
+static inline int constant_time_select_int(unsigned int mask, int a, int b);
+
static inline unsigned int constant_time_msb(unsigned int a)
{
return (unsigned int)((int)(a) >> (sizeof(int) * 8 - 1));
return (unsigned char)(constant_time_eq(a, b));
}
+static inline unsigned int constant_time_select(unsigned int mask,
+ unsigned int a, unsigned int b)
+ {
+ return (mask & a) | (~mask & b);
+ }
+
+static inline unsigned char constant_time_select_8(unsigned char mask,
+ unsigned char a, unsigned char b)
+ {
+ return (unsigned char)(constant_time_select(mask, a, b));
+ }
+
+inline int constant_time_select_int(unsigned int mask, int a, int b)
+ {
+ return (int)(constant_time_select(mask, (unsigned)(a), (unsigned)(b)));
+ }
+
#ifdef __cplusplus
}
#endif
#include <stdio.h>
#include <stdlib.h>
-static const unsigned int CONSTTIME_TRUE = ~0;
+static const unsigned int CONSTTIME_TRUE = (unsigned)(~0);
static const unsigned int CONSTTIME_FALSE = 0;
-static const unsigned char CONSTTIME_TRUE_8 = ~0;
+static const unsigned char CONSTTIME_TRUE_8 = 0xff;
static const unsigned char CONSTTIME_FALSE_8 = 0;
static int test_binary_op(unsigned int (*op)(unsigned int a, unsigned int b),
return 0;
}
+static int test_select(unsigned int a, unsigned int b)
+ {
+ unsigned int selected = constant_time_select(CONSTTIME_TRUE, a, b);
+ if (selected != a)
+ {
+ fprintf(stderr, "Test failed for constant_time_select(%du, %du,"
+ "%du): expected %du(first value), got %du\n",
+ CONSTTIME_TRUE, a, b, a, selected);
+ return 1;
+ }
+ selected = constant_time_select(CONSTTIME_FALSE, a, b);
+ if (selected != b)
+ {
+ fprintf(stderr, "Test failed for constant_time_select(%du, %du,"
+ "%du): expected %du(second value), got %du\n",
+ CONSTTIME_FALSE, a, b, b, selected);
+ return 1;
+ }
+ return 0;
+ }
+
+static int test_select_8(unsigned char a, unsigned char b)
+ {
+ unsigned char selected = constant_time_select_8(CONSTTIME_TRUE_8, a, b);
+ if (selected != a)
+ {
+ fprintf(stderr, "Test failed for constant_time_select(%u, %u,"
+ "%u): expected %u(first value), got %u\n",
+ CONSTTIME_TRUE, a, b, a, selected);
+ return 1;
+ }
+ selected = constant_time_select_8(CONSTTIME_FALSE_8, a, b);
+ if (selected != b)
+ {
+ fprintf(stderr, "Test failed for constant_time_select(%u, %u,"
+ "%u): expected %u(second value), got %u\n",
+ CONSTTIME_FALSE, a, b, b, selected);
+ return 1;
+ }
+ return 0;
+ }
+
+static int test_select_int(int a, int b)
+ {
+ int selected = constant_time_select_int(CONSTTIME_TRUE, a, b);
+ if (selected != a)
+ {
+ fprintf(stderr, "Test failed for constant_time_select(%du, %d,"
+ "%d): expected %d(first value), got %d\n",
+ CONSTTIME_TRUE, a, b, a, selected);
+ return 1;
+ }
+ selected = constant_time_select_int(CONSTTIME_FALSE, a, b);
+ if (selected != b)
+ {
+ fprintf(stderr, "Test failed for constant_time_select(%du, %d,"
+ "%d): expected %d(second value), got %d\n",
+ CONSTTIME_FALSE, a, b, b, selected);
+ return 1;
+ }
+ return 0;
+ }
+
+
static unsigned int test_values[] = {0, 1, 1024, 12345, 32000, UINT_MAX/2-1,
UINT_MAX/2, UINT_MAX/2+1, UINT_MAX-1,
UINT_MAX};
+static unsigned char test_values_8[] = {0, 1, 2, 20, 32, 127, 128, 129, 255};
+
+static int signed_test_values[] = {0, 1, -1, 1024, -1024, 12345, -12345,
+ 32000, -32000, INT_MAX, INT_MIN, INT_MAX-1,
+ INT_MIN+1};
+
+
int main(int argc, char *argv[])
{
unsigned int a, b, i, j;
+ int c, d;
+ unsigned char e, f;
int num_failed = 0, num_all = 0;
fprintf(stdout, "Testing constant time operations...\n");
a = test_values[i];
num_failed += test_is_zero(a);
num_failed += test_is_zero_8(a);
- num_failed += test_binary_op(&constant_time_lt,
- "constant_time_lt", a, a, 0);
- num_failed += test_binary_op_8(&constant_time_lt_8,
- "constant_time_lt_8", a, a, 0);
- num_failed += test_binary_op(&constant_time_ge,
- "constant_time_ge", a, a, 1);
- num_failed += test_binary_op_8(&constant_time_ge_8,
- "constant_time_ge_8", a, a, 1);
- num_failed += test_binary_op(&constant_time_eq,
- "constant_time_eq", a, a, 1);
- num_failed += test_binary_op_8(&constant_time_eq_8,
- "constant_time_eq_8", a, a, 1);
- num_all += 8;
- for (j = i + 1; j < sizeof(test_values)/sizeof(int); ++j)
+ num_all += 2;
+ for (j = 0; j < sizeof(test_values)/sizeof(int); ++j)
{
b = test_values[j];
num_failed += test_binary_op(&constant_time_lt,
"constant_time_eq", b, a, b == a);
num_failed += test_binary_op_8(&constant_time_eq_8,
"constant_time_eq_8", b, a, b == a);
- num_all += 12;
+ num_failed += test_select(a, b);
+ num_all += 13;
+ }
+ }
+
+ for (i = 0; i < sizeof(signed_test_values)/sizeof(int); ++i)
+ {
+ c = signed_test_values[i];
+ for (j = 0; j < sizeof(signed_test_values)/sizeof(int); ++j)
+ {
+ d = signed_test_values[j];
+ num_failed += test_select_int(c, d);
+ num_all += 1;
+ }
+ }
+
+ for (i = 0; i < sizeof(test_values_8); ++i)
+ {
+ e = test_values_8[i];
+ for (j = 0; j < sizeof(test_values_8); ++j)
+ {
+ f = test_values_8[j];
+ num_failed += test_select_8(e, f);
+ num_all += 1;
}
}
rsa_oaep.o: ../../include/openssl/rand.h ../../include/openssl/rsa.h
rsa_oaep.o: ../../include/openssl/safestack.h ../../include/openssl/sha.h
rsa_oaep.o: ../../include/openssl/stack.h ../../include/openssl/symhacks.h
-rsa_oaep.o: ../cryptlib.h rsa_oaep.c
+rsa_oaep.o: ../constant_time_locl.h ../cryptlib.h rsa_oaep.c
rsa_pk1.o: ../../e_os.h ../../include/openssl/asn1.h
rsa_pk1.o: ../../include/openssl/bio.h ../../include/openssl/bn.h
rsa_pk1.o: ../../include/openssl/buffer.h ../../include/openssl/crypto.h
rsa_pk1.o: ../../include/openssl/opensslv.h ../../include/openssl/ossl_typ.h
rsa_pk1.o: ../../include/openssl/rand.h ../../include/openssl/rsa.h
rsa_pk1.o: ../../include/openssl/safestack.h ../../include/openssl/stack.h
-rsa_pk1.o: ../../include/openssl/symhacks.h ../cryptlib.h rsa_pk1.c
+rsa_pk1.o: ../../include/openssl/symhacks.h ../constant_time_locl.h
+rsa_pk1.o: ../cryptlib.h rsa_pk1.c
rsa_pmeth.o: ../../e_os.h ../../include/openssl/asn1.h
rsa_pmeth.o: ../../include/openssl/asn1t.h ../../include/openssl/bio.h
rsa_pmeth.o: ../../include/openssl/bn.h ../../include/openssl/buffer.h
#define RSA_R_OAEP_DECODING_ERROR 121
#define RSA_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE 148
#define RSA_R_PADDING_CHECK_FAILED 114
+#define RSA_R_PKCS_DECODING_ERROR 159
#define RSA_R_P_NOT_PRIME 128
#define RSA_R_Q_NOT_PRIME 129
#define RSA_R_RSA_OPERATIONS_NOT_SUPPORTED 130
{ERR_REASON(RSA_R_OAEP_DECODING_ERROR) ,"oaep decoding error"},
{ERR_REASON(RSA_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE),"operation not supported for this keytype"},
{ERR_REASON(RSA_R_PADDING_CHECK_FAILED) ,"padding check failed"},
+{ERR_REASON(RSA_R_PKCS_DECODING_ERROR) ,"pkcs decoding error"},
{ERR_REASON(RSA_R_P_NOT_PRIME) ,"p not prime"},
{ERR_REASON(RSA_R_Q_NOT_PRIME) ,"q not prime"},
{ERR_REASON(RSA_R_RSA_OPERATIONS_NOT_SUPPORTED),"rsa operations not supported"},
* an equivalent notion.
*/
+#include "../constant_time_locl.h"
#if !defined(OPENSSL_NO_SHA) && !defined(OPENSSL_NO_SHA1)
#include <stdio.h>
const unsigned char *from, int flen, int num,
const unsigned char *param, int plen)
{
- int i, dblen, mlen = -1;
- const unsigned char *maskeddb;
- int lzero;
- unsigned char *db = NULL, seed[SHA_DIGEST_LENGTH], phash[SHA_DIGEST_LENGTH];
- unsigned char *padded_from;
- int bad = 0;
-
- if (--num < 2 * SHA_DIGEST_LENGTH + 1)
- /* 'num' is the length of the modulus, i.e. does not depend on the
- * particular ciphertext. */
- goto decoding_err;
+ int i, dblen, mlen = -1, one_index = 0, msg_index;
+ unsigned int good, found_one_byte;
+ const unsigned char *maskedseed, *maskeddb;
+ /* |em| is the encoded message, zero-padded to exactly |num| bytes:
+ * em = Y || maskedSeed || maskedDB */
+ unsigned char *db = NULL, *em = NULL, seed[EVP_MAX_MD_SIZE],
+ phash[EVP_MAX_MD_SIZE];
- lzero = num - flen;
- if (lzero < 0)
- {
- /* signalling this error immediately after detection might allow
- * for side-channel attacks (e.g. timing if 'plen' is huge
- * -- cf. James H. Manger, "A Chosen Ciphertext Attack on RSA Optimal
- * Asymmetric Encryption Padding (OAEP) [...]", CRYPTO 2001),
- * so we use a 'bad' flag */
- bad = 1;
- lzero = 0;
- flen = num; /* don't overflow the memcpy to padded_from */
- }
+ if (tlen <= 0 || flen <= 0)
+ return -1;
- dblen = num - SHA_DIGEST_LENGTH;
- db = OPENSSL_malloc(dblen + num);
- if (db == NULL)
+ /*
+ * |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 * SHA_DIGEST_LENGTH + 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.
+ */
+ if (num < flen || num < 2 * SHA_DIGEST_LENGTH + 2)
+ goto decoding_err;
+
+ dblen = num - SHA_DIGEST_LENGTH - 1;
+ db = OPENSSL_malloc(dblen);
+ em = OPENSSL_malloc(num);
+ if (db == NULL || em == NULL)
{
RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP, ERR_R_MALLOC_FAILURE);
- return -1;
+ goto cleanup;
}
- /* Always do this zero-padding copy (even when lzero == 0)
- * to avoid leaking timing info about the value of lzero. */
- padded_from = db + dblen;
- memset(padded_from, 0, lzero);
- memcpy(padded_from + lzero, from, flen);
+ /*
+ * 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);
- maskeddb = padded_from + SHA_DIGEST_LENGTH;
+ /*
+ * 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]);
+
+ maskedseed = em + 1;
+ maskeddb = em + 1 + SHA_DIGEST_LENGTH;
if (MGF1(seed, SHA_DIGEST_LENGTH, maskeddb, dblen))
- return -1;
+ goto cleanup;
for (i = 0; i < SHA_DIGEST_LENGTH; i++)
- seed[i] ^= padded_from[i];
-
+ seed[i] ^= maskedseed[i];
+
if (MGF1(db, dblen, seed, SHA_DIGEST_LENGTH))
- return -1;
+ goto cleanup;
for (i = 0; i < dblen; i++)
db[i] ^= maskeddb[i];
EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL);
- if (CRYPTO_memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad)
+ good &= constant_time_is_zero(CRYPTO_memcmp(db, phash, SHA_DIGEST_LENGTH));
+
+ found_one_byte = 0;
+ for (i = SHA_DIGEST_LENGTH; i < dblen; i++)
+ {
+ /* Padding consists of a number of 0-bytes, followed by a 1. */
+ unsigned int equals1 = constant_time_eq(db[i], 1);
+ unsigned int equals0 = constant_time_is_zero(db[i]);
+ one_index = constant_time_select_int(~found_one_byte & equals1,
+ i, one_index);
+ found_one_byte |= equals1;
+ good &= (found_one_byte | equals0);
+ }
+
+ good &= found_one_byte;
+
+ /*
+ * At this point |good| is zero unless the plaintext was valid,
+ * 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, RSA_R_DATA_TOO_LARGE);
+ mlen = -1;
+ }
else
{
- for (i = SHA_DIGEST_LENGTH; i < dblen; i++)
- if (db[i] != 0x00)
- break;
- if (i == dblen || db[i] != 0x01)
- goto decoding_err;
- else
- {
- /* everything looks OK */
-
- mlen = dblen - ++i;
- if (tlen < mlen)
- {
- RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP, RSA_R_DATA_TOO_LARGE);
- mlen = -1;
- }
- else
- memcpy(to, db + i, mlen);
- }
+ memcpy(to, db + msg_index, mlen);
+ goto cleanup;
}
- OPENSSL_free(db);
- return mlen;
decoding_err:
- /* to avoid chosen ciphertext attacks, the error message should not reveal
- * which kind of decoding error happened */
+ /* 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, RSA_R_OAEP_DECODING_ERROR);
+cleanup:
if (db != NULL) OPENSSL_free(db);
- return -1;
+ if (em != NULL) OPENSSL_free(em);
+ return mlen;
}
int PKCS1_MGF1(unsigned char *mask, long len,
* [including the GNU Public Licence.]
*/
+#include "../constant_time_locl.h"
+
#include <stdio.h>
#include "cryptlib.h"
#include <openssl/bn.h>
int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
const unsigned char *from, int flen, int num)
{
- int i,j;
- const unsigned char *p;
+ int i;
+ /* |em| is the encoded message, zero-padded to exactly |num| bytes */
+ unsigned char *em = NULL;
+ unsigned int good, found_zero_byte;
+ int zero_index = 0, msg_index, mlen = -1;
- p=from;
- if ((num != (flen+1)) || (*(p++) != 02))
- {
- RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_BLOCK_TYPE_IS_NOT_02);
- return(-1);
- }
-#ifdef PKCS1_CHECK
- return(num-11);
-#endif
+ if (tlen < 0 || flen < 0)
+ return -1;
- /* scan over padding data */
- j=flen-1; /* one for type. */
- for (i=0; i<j; i++)
- if (*(p++) == 0) break;
+ /* PKCS#1 v1.5 decryption. See "PKCS #1 v2.2: RSA Cryptography
+ * Standard", section 7.2.2. */
- if (i == j)
+ if (flen > num)
+ goto err;
+
+ if (num < 11)
+ goto err;
+
+ em = OPENSSL_malloc(num);
+ if (em == NULL)
{
- RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_NULL_BEFORE_BLOCK_MISSING);
- return(-1);
+ RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE);
+ return -1;
}
+ memset(em, 0, num);
+ /*
+ * 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);
- if (i < 8)
+ good = constant_time_is_zero(em[0]);
+ good &= constant_time_eq(em[1], 2);
+
+ found_zero_byte = 0;
+ for (i = 2; i < num; i++)
{
- RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_BAD_PAD_BYTE_COUNT);
- return(-1);
+ 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;
}
- i++; /* Skip over the '\0' */
- j-=i;
- if (j > tlen)
+
+ /*
+ * 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.
+ */
+ good &= constant_time_ge((unsigned int)(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));
+
+ /*
+ * 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.
+ */
+ if (!good)
{
- RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_DATA_TOO_LARGE);
- return(-1);
+ mlen = -1;
+ goto err;
}
- memcpy(to,p,(unsigned int)j);
- return(j);
- }
+ memcpy(to, em + msg_index, mlen);
+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;
+ }
padding_length = good & (padding_length+1);
rec->length -= padding_length;
rec->type |= padding_length<<8; /* kludge: pass padding length */
- return (int)((good & 1) | (~good & -1));
+ return constant_time_select_int(good, 1, -1);
}
/* tls1_cbc_remove_padding removes the CBC padding from the decrypted, TLS, CBC
rec->length -= padding_length;
rec->type |= padding_length<<8; /* kludge: pass padding length */
- return (int)((good & 1) | (~good & -1));
+ return constant_time_select_int(good, 1, -1);
}
/* ssl3_cbc_copy_mac copies |md_size| bytes from the end of |rec| to |out| in
/* If this is the block containing the end of the
* application data, and we are at the offset for the
* 0x80 value, then overwrite b with 0x80. */
- b = (b&~is_past_c) | (0x80&is_past_c);
+ b = constant_time_select_8(is_past_c, 0x80, b);
/* If this the the block containing the end of the
* application data and we're past the 0x80 value then
* just write zero. */
if (j >= md_block_size - md_length_size)
{
/* If this is index_b, write a length byte. */
- b = (b&~is_block_b) | (is_block_b&length_bytes[j-(md_block_size-md_length_size)]);
+ b = constant_time_select_8(
+ is_block_b, length_bytes[j-(md_block_size-md_length_size)], b);
}
block[j] = b;
}
jpaketest.o: ../include/openssl/opensslv.h ../include/openssl/ossl_typ.h
jpaketest.o: ../include/openssl/safestack.h ../include/openssl/stack.h
jpaketest.o: ../include/openssl/symhacks.h jpaketest.c
-md2test.o: ../e_os.h ../include/openssl/e_os2.h
-md2test.o: ../include/openssl/opensslconf.h md2test.c
+md2test.o: ../include/openssl/buffer.h ../include/openssl/crypto.h
+md2test.o: ../include/openssl/e_os2.h ../include/openssl/opensslconf.h
+md2test.o: ../include/openssl/opensslv.h ../include/openssl/ossl_typ.h
+md2test.o: ../include/openssl/safestack.h ../include/openssl/stack.h
+md2test.o: ../include/openssl/symhacks.h md2test.c
md4test.o: ../e_os.h ../include/openssl/asn1.h ../include/openssl/bio.h
md4test.o: ../include/openssl/crypto.h ../include/openssl/e_os2.h
md4test.o: ../include/openssl/evp.h ../include/openssl/md4.h