Merge branch 'mbedtls-2.16' into mbedtls-2.16-restricted
* mbedtls-2.16: (21 commits)
Reword changelog - Test Resource Leak
Update changelog formatting - Missing Free Context
Fix fd range for select on Windows
Refactor file descriptor checks into a common function
Update changelog formatting - Missing Free Context
Update changelog formatting - Missing Free Context
Changelog entry for Free Context in test_suite_aes fix
Free context at the end of aes_crypt_xts_size()
Add changelog entry for non-uniform MPI random generation
ecp: Fix bias in the generation of blinding values
DHM: add test case with x_size < 0
DHM tests: add some explanations
DHM: add notes about leading zeros
dhm: Fix bias in private key generation and blinding
dhm_check_range: microoptimization
DHM refactoring: use dhm_random_below in dhm_make_common
DHM blinding: don't accept P-1 as a blinding value
DHM refactoring: unify mbedtls_dhm_make_{params,public}
Test mbedtls_dhm_make_params with different x_size
Repeat a few DH tests
...
diff --git a/ChangeLog.d/ecdsa-random-leading-zeros.txt b/ChangeLog.d/ecdsa-random-leading-zeros.txt
new file mode 100644
index 0000000..793b371
--- /dev/null
+++ b/ChangeLog.d/ecdsa-random-leading-zeros.txt
@@ -0,0 +1,7 @@
+Security
+* Fix a potential side channel vulnerability in ECDSA ephemeral key generation.
+ An adversary who is capable of very precise timing measurements could
+ learn partial information about the leading bits of the nonce used for the
+ signature, allowing the recovery of the private key after observing a
+ large number of signature operations. This completes a partial fix in
+ Mbed TLS 2.16.4.
diff --git a/ChangeLog.d/ecp_max_bits.txt b/ChangeLog.d/ecp_max_bits.txt
new file mode 100644
index 0000000..bfbe11f
--- /dev/null
+++ b/ChangeLog.d/ecp_max_bits.txt
@@ -0,0 +1,4 @@
+Security
+ * It was possible to configure MBEDTLS_ECP_MAX_BITS to a value that is
+ too small, leading to buffer overflows in ECC operations. Fail the build
+ in such a case.
diff --git a/ChangeLog.d/fix-rsa-leak.txt b/ChangeLog.d/fix-rsa-leak.txt
new file mode 100644
index 0000000..b7d3e3e
--- /dev/null
+++ b/ChangeLog.d/fix-rsa-leak.txt
@@ -0,0 +1,6 @@
+Security
+ * An adversary with access to precise enough information about memory
+ accesses (typically, an untrusted operating system attacking a secure
+ enclave) could recover an RSA private key after observing the victim
+ performing a single private-key operation. Found and reported by
+ Zili KOU, Wenjian HE, Sharad Sinha, and Wei ZHANG.
diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h
index bdc750e..e4a1ed8 100644
--- a/include/mbedtls/ecp.h
+++ b/include/mbedtls/ecp.h
@@ -154,6 +154,40 @@
}
mbedtls_ecp_point;
+/* Determine the minimum safe value of MBEDTLS_ECP_MAX_BITS. */
+#if !defined(MBEDTLS_ECP_C)
+#define MBEDTLS_ECP_MAX_BITS_MIN 0
+/* Note: the curves must be listed in DECREASING size! */
+#elif defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 521
+#elif defined(MBEDTLS_ECP_DP_BP512R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 512
+#elif defined(MBEDTLS_ECP_DP_CURVE448_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 448
+#elif defined(MBEDTLS_ECP_DP_BP384R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 384
+#elif defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 384
+#elif defined(MBEDTLS_ECP_DP_BP256R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 256
+#elif defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 256
+#elif defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 256
+#elif defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 255
+#elif defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 225 // n is slightly above 2^224
+#elif defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 224
+#elif defined(MBEDTLS_ECP_DP_SECP192K1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 192
+#elif defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED)
+#define MBEDTLS_ECP_MAX_BITS_MIN 192
+#else
+#error "MBEDTLS_ECP_C enabled, but no curve?"
+#endif
+
#if !defined(MBEDTLS_ECP_ALT)
/*
* default mbed TLS elliptic curve arithmetic implementation
@@ -228,7 +262,13 @@
* \{
*/
-#if !defined(MBEDTLS_ECP_MAX_BITS)
+#if defined(MBEDTLS_ECP_MAX_BITS)
+
+#if MBEDTLS_ECP_MAX_BITS < MBEDTLS_ECP_MAX_BITS_MIN
+#error "MBEDTLS_ECP_MAX_BITS is smaller than the largest supported curve"
+#endif
+
+#else
/**
* The maximum size of the groups, that is, of \c N and \c P.
*/
diff --git a/library/bignum.c b/library/bignum.c
index bd352e1..8a2acdb 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -267,6 +267,36 @@
memcpy( Y, &T, sizeof( mbedtls_mpi ) );
}
+/**
+ * Select between two sign values in constant-time.
+ *
+ * This is functionally equivalent to second ? a : b but uses only bit
+ * operations in order to avoid branches.
+ *
+ * \param[in] a The first sign; must be either +1 or -1.
+ * \param[in] b The second sign; must be either +1 or -1.
+ * \param[in] second Must be either 1 (return b) or 0 (return a).
+ *
+ * \return The selected sign value.
+ */
+static int mpi_safe_cond_select_sign( int a, int b, unsigned char second )
+{
+ /* In order to avoid questions about what we can reasonnably assume about
+ * the representations of signed integers, move everything to unsigned
+ * by taking advantage of the fact that a and b are either +1 or -1. */
+ unsigned ua = a + 1;
+ unsigned ub = b + 1;
+
+ /* second was 0 or 1, mask is 0 or 2 as are ua and ub */
+ const unsigned mask = second << 1;
+
+ /* select ua or ub */
+ unsigned ur = ( ua & ~mask ) | ( ub & mask );
+
+ /* ur is now 0 or 2, convert back to -1 or +1 */
+ return( (int) ur - 1 );
+}
+
/*
* Conditionally assign dest = src, without leaking information
* about whether the assignment was made or not.
@@ -279,8 +309,23 @@
unsigned char assign )
{
size_t i;
+
+ /* MSVC has a warning about unary minus on unsigned integer types,
+ * but this is well-defined and precisely what we want to do here. */
+#if defined(_MSC_VER)
+#pragma warning( push )
+#pragma warning( disable : 4146 )
+#endif
+
+ /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */
+ const mbedtls_mpi_uint mask = -assign;
+
+#if defined(_MSC_VER)
+#pragma warning( pop )
+#endif
+
for( i = 0; i < n; i++ )
- dest[i] = dest[i] * ( 1 - assign ) + src[i] * assign;
+ dest[i] = ( src[i] & mask ) | ( dest[i] & ~mask );
}
/*
@@ -292,20 +337,34 @@
{
int ret = 0;
size_t i;
+ mbedtls_mpi_uint limb_mask;
MPI_VALIDATE_RET( X != NULL );
MPI_VALIDATE_RET( Y != NULL );
+ /* MSVC has a warning about unary minus on unsigned integer types,
+ * but this is well-defined and precisely what we want to do here. */
+#if defined(_MSC_VER)
+#pragma warning( push )
+#pragma warning( disable : 4146 )
+#endif
+
/* make sure assign is 0 or 1 in a time-constant manner */
- assign = (assign | (unsigned char)-assign) >> 7;
+ assign = (assign | (unsigned char)-assign) >> (sizeof( assign ) * 8 - 1);
+ /* all-bits 1 if assign is 1, all-bits 0 if assign is 0 */
+ limb_mask = -assign;
+
+#if defined(_MSC_VER)
+#pragma warning( pop )
+#endif
MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) );
- X->s = X->s * ( 1 - assign ) + Y->s * assign;
+ X->s = mpi_safe_cond_select_sign( X->s, Y->s, assign );
mpi_safe_cond_assign( Y->n, X->p, Y->p, assign );
for( i = Y->n; i < X->n; i++ )
- X->p[i] *= ( 1 - assign );
+ X->p[i] &= ~limb_mask;
cleanup:
return( ret );
@@ -321,6 +380,7 @@
{
int ret, s;
size_t i;
+ mbedtls_mpi_uint limb_mask;
mbedtls_mpi_uint tmp;
MPI_VALIDATE_RET( X != NULL );
MPI_VALIDATE_RET( Y != NULL );
@@ -328,22 +388,35 @@
if( X == Y )
return( 0 );
+ /* MSVC has a warning about unary minus on unsigned integer types,
+ * but this is well-defined and precisely what we want to do here. */
+#if defined(_MSC_VER)
+#pragma warning( push )
+#pragma warning( disable : 4146 )
+#endif
+
/* make sure swap is 0 or 1 in a time-constant manner */
- swap = (swap | (unsigned char)-swap) >> 7;
+ swap = (swap | (unsigned char)-swap) >> (sizeof( swap ) * 8 - 1);
+ /* all-bits 1 if swap is 1, all-bits 0 if swap is 0 */
+ limb_mask = -swap;
+
+#if defined(_MSC_VER)
+#pragma warning( pop )
+#endif
MBEDTLS_MPI_CHK( mbedtls_mpi_grow( X, Y->n ) );
MBEDTLS_MPI_CHK( mbedtls_mpi_grow( Y, X->n ) );
s = X->s;
- X->s = X->s * ( 1 - swap ) + Y->s * swap;
- Y->s = Y->s * ( 1 - swap ) + s * swap;
+ X->s = mpi_safe_cond_select_sign( X->s, Y->s, swap );
+ Y->s = mpi_safe_cond_select_sign( Y->s, s, swap );
for( i = 0; i < X->n; i++ )
{
tmp = X->p[i];
- X->p[i] = X->p[i] * ( 1 - swap ) + Y->p[i] * swap;
- Y->p[i] = Y->p[i] * ( 1 - swap ) + tmp * swap;
+ X->p[i] = ( X->p[i] & ~limb_mask ) | ( Y->p[i] & limb_mask );
+ Y->p[i] = ( Y->p[i] & ~limb_mask ) | ( tmp & limb_mask );
}
cleanup:
@@ -2032,6 +2105,72 @@
}
/*
+ * Constant-flow boolean "equal" comparison:
+ * return x == y
+ *
+ * This function can be used to write constant-time code by replacing branches
+ * with bit operations - it can be used in conjunction with
+ * mbedtls_ssl_cf_mask_from_bit().
+ *
+ * This function is implemented without using comparison operators, as those
+ * might be translated to branches by some compilers on some platforms.
+ */
+static size_t mbedtls_mpi_cf_bool_eq( size_t x, size_t y )
+{
+ /* diff = 0 if x == y, non-zero otherwise */
+ const size_t diff = x ^ y;
+
+ /* MSVC has a warning about unary minus on unsigned integer types,
+ * but this is well-defined and precisely what we want to do here. */
+#if defined(_MSC_VER)
+#pragma warning( push )
+#pragma warning( disable : 4146 )
+#endif
+
+ /* diff_msb's most significant bit is equal to x != y */
+ const size_t diff_msb = ( diff | (size_t) -diff );
+
+#if defined(_MSC_VER)
+#pragma warning( pop )
+#endif
+
+ /* diff1 = (x != y) ? 1 : 0 */
+ const size_t diff1 = diff_msb >> ( sizeof( diff_msb ) * 8 - 1 );
+
+ return( 1 ^ diff1 );
+}
+
+/**
+ * Select an MPI from a table without leaking the index.
+ *
+ * This is functionally equivalent to mbedtls_mpi_copy(R, T[idx]) except it
+ * reads the entire table in order to avoid leaking the value of idx to an
+ * attacker able to observe memory access patterns.
+ *
+ * \param[out] R Where to write the selected MPI.
+ * \param[in] T The table to read from.
+ * \param[in] T_size The number of elements in the table.
+ * \param[in] idx The index of the element to select;
+ * this must satisfy 0 <= idx < T_size.
+ *
+ * \return \c 0 on success, or a negative error code.
+ */
+static int mpi_select( mbedtls_mpi *R, const mbedtls_mpi *T, size_t T_size, size_t idx )
+{
+ int ret = MBEDTLS_ERR_MPI_BAD_INPUT_DATA;
+ size_t i;
+
+ for( i = 0; i < T_size; i++ )
+ {
+ MBEDTLS_MPI_CHK( mbedtls_mpi_safe_cond_assign( R, &T[i],
+ (unsigned char) mbedtls_mpi_cf_bool_eq( i, idx ) ) );
+ }
+
+cleanup:
+ return( ret );
+}
+
+/*
* Sliding-window exponentiation: X = A^E mod N (HAC 14.85)
*/
int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A,
@@ -2043,7 +2182,7 @@
size_t i, j, nblimbs;
size_t bufsize, nbits;
mbedtls_mpi_uint ei, mm, state;
- mbedtls_mpi RR, T, W[ 1 << MBEDTLS_MPI_WINDOW_SIZE ], Apos;
+ mbedtls_mpi RR, T, W[ 1 << MBEDTLS_MPI_WINDOW_SIZE ], WW, Apos;
int neg;
MPI_VALIDATE_RET( X != NULL );
@@ -2067,6 +2206,7 @@
mpi_montg_init( &mm, N );
mbedtls_mpi_init( &RR ); mbedtls_mpi_init( &T );
mbedtls_mpi_init( &Apos );
+ mbedtls_mpi_init( &WW );
memset( W, 0, sizeof( W ) );
i = mbedtls_mpi_bitlen( E );
@@ -2207,7 +2347,8 @@
/*
* X = X * W[wbits] R^-1 mod N
*/
- mpi_montmul( X, &W[wbits], N, mm, &T );
+ MBEDTLS_MPI_CHK( mpi_select( &WW, W, (size_t) 1 << wsize, wbits ) );
+ mpi_montmul( X, &WW, N, mm, &T );
state--;
nbits = 0;
@@ -2245,6 +2386,7 @@
mbedtls_mpi_free( &W[i] );
mbedtls_mpi_free( &W[1] ); mbedtls_mpi_free( &T ); mbedtls_mpi_free( &Apos );
+ mbedtls_mpi_free( &WW );
if( _RR == NULL || _RR->p == NULL )
mbedtls_mpi_free( &RR );
diff --git a/library/ecp.c b/library/ecp.c
index d3c6ccc..285c363 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -3057,6 +3057,11 @@
{
int ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
size_t n_size;
+#if defined(ECP_SHORTWEIERSTRASS)
+ mbedtls_mpi one;
+
+ mbedtls_mpi_init( &one );
+#endif
ECP_VALIDATE_RET( grp != NULL );
ECP_VALIDATE_RET( d != NULL );
@@ -3097,7 +3102,10 @@
{
/* SEC1 3.2.1: Generate d such that 1 <= n < N */
int count = 0;
- unsigned cmp = 0;
+ unsigned lt_lower = 1, lt_upper = 0;
+
+ MBEDTLS_MPI_CHK( mbedtls_mpi_grow( &one, grp->N.n ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &one, 1 ) );
/*
* Match the procedure given in RFC 6979 (deterministic ECDSA):
@@ -3121,19 +3129,22 @@
* such as secp224k1 are actually very close to the worst case.
*/
if( ++count > 30 )
- return( MBEDTLS_ERR_ECP_RANDOM_FAILED );
-
- ret = mbedtls_mpi_lt_mpi_ct( d, &grp->N, &cmp );
- if( ret != 0 )
{
+ ret = MBEDTLS_ERR_ECP_RANDOM_FAILED;
goto cleanup;
}
+
+ MBEDTLS_MPI_CHK( mbedtls_mpi_lt_mpi_ct( d, &grp->N, <_upper ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_lt_mpi_ct( d, &one, <_lower ) );
}
- while( mbedtls_mpi_cmp_int( d, 1 ) < 0 || cmp != 1 );
+ while( lt_lower != 0 || lt_upper == 0 );
}
#endif /* ECP_SHORTWEIERSTRASS */
cleanup:
+#if defined(ECP_SHORTWEIERSTRASS)
+ mbedtls_mpi_free( &one );
+#endif
return( ret );
}
diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function
index 3e901f7..9ae84ed 100644
--- a/tests/suites/test_suite_ecp.function
+++ b/tests/suites/test_suite_ecp.function
@@ -393,6 +393,8 @@
TEST_ASSERT( by_id == by_name );
TEST_ASSERT( by_id->bit_size == size );
+ TEST_ASSERT( size <= MBEDTLS_ECP_MAX_BITS );
+ TEST_ASSERT( size <= MBEDTLS_ECP_MAX_BYTES * 8 );
}
/* END_CASE */
@@ -750,6 +752,7 @@
TEST_EQUAL( 0, mbedtls_ecp_point_write_binary(
&grp, &R, MBEDTLS_ECP_PF_UNCOMPRESSED,
&len, actual_result, sizeof( actual_result ) ) );
+ TEST_ASSERT( len <= MBEDTLS_ECP_MAX_PT_LEN );
ASSERT_COMPARE( expected_result->x, expected_result->len,
actual_result, len );
@@ -821,6 +824,7 @@
if( ret == 0 )
{
+ TEST_ASSERT( olen <= MBEDTLS_ECP_MAX_PT_LEN );
TEST_ASSERT( mbedtls_test_hexcmp( buf, out->x, olen, out->len ) == 0 );
}