Merge branch 'development_2.x' into development_2.x-restricted
* development_2.x:
Reword changelog - Test Resource Leak
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
Update changelog formatting - Missing Free Context
Changelog entry for Free Context in test_suite_aes fix
Free context in at the end of aes_crypt_xts_size()
Fix copypasta in test data
Use UNUSED wherever applicable in derive_input tests
Fix missing state check for tls12_prf output
Key derivation: add test cases where the secret is missing
Add bad-workflow key derivation tests
More explicit names for some bad-workflow key derivation 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..cbc674b
--- /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.20.0.
diff --git a/ChangeLog.d/ecp_max_bits.txt b/ChangeLog.d/ecp_max_bits.txt
new file mode 100644
index 0000000..834deda
--- /dev/null
+++ b/ChangeLog.d/ecp_max_bits.txt
@@ -0,0 +1,8 @@
+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.
+
+Features
+ * MBEDTLS_ECP_MAX_BITS is now determined automatically from the configured
+ curves and no longer needs to be configured explicitly to save RAM.
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/configs/config-suite-b.h b/configs/config-suite-b.h
index 6eb03a9..9cad382 100644
--- a/configs/config-suite-b.h
+++ b/configs/config-suite-b.h
@@ -80,8 +80,7 @@
#define MBEDTLS_AES_ROM_TABLES
/* Save RAM by adjusting to our exact needs */
-#define MBEDTLS_ECP_MAX_BITS 384
-#define MBEDTLS_MPI_MAX_SIZE 48 // 384 bits is 48 bytes
+#define MBEDTLS_MPI_MAX_SIZE 48 // 48 bytes for a 384-bit elliptic curve
/* Save RAM at the expense of speed, see ecp.h */
#define MBEDTLS_ECP_WINDOW_SIZE 2
diff --git a/configs/config-thread.h b/configs/config-thread.h
index 47dd5e2..8464fcb 100644
--- a/configs/config-thread.h
+++ b/configs/config-thread.h
@@ -81,8 +81,7 @@
#define MBEDTLS_AES_ROM_TABLES
/* Save RAM by adjusting to our exact needs */
-#define MBEDTLS_ECP_MAX_BITS 256
-#define MBEDTLS_MPI_MAX_SIZE 32 // 256 bits is 32 bytes
+#define MBEDTLS_MPI_MAX_SIZE 32 // 32 bytes for a 256-bit elliptic curve
/* Save ROM and a few bytes of RAM by specifying our own ciphersuite list */
#define MBEDTLS_SSL_CIPHERSUITES MBEDTLS_TLS_ECJPAKE_WITH_AES_128_CCM_8
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index d0e61c5..79aacf7 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -3616,7 +3616,7 @@
//#define MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT 384 /**< Maximum size of (re)seed buffer */
/* ECP options */
-//#define MBEDTLS_ECP_MAX_BITS 521 /**< Maximum bit size of groups */
+//#define MBEDTLS_ECP_MAX_BITS 521 /**< Maximum bit size of groups. Normally determined automatically from the configured curves. */
//#define MBEDTLS_ECP_WINDOW_SIZE 6 /**< Maximum window size used */
//#define MBEDTLS_ECP_FIXED_POINT_OPTIM 1 /**< Enable fixed-point speed-up */
diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h
index b974e59..48eec61 100644
--- a/include/mbedtls/ecp.h
+++ b/include/mbedtls/ecp.h
@@ -96,6 +96,7 @@
* - Add it at the end of this enum, otherwise you'll break the ABI by
* changing the numerical value for existing curves.
* - Increment MBEDTLS_ECP_DP_MAX below if needed.
+ * - Update the calculation of MBEDTLS_ECP_MAX_BITS_MIN below.
* - Add the corresponding MBEDTLS_ECP_DP_xxx_ENABLED macro definition to
* config.h.
* - List the curve as a dependency of MBEDTLS_ECP_C and
@@ -171,6 +172,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
@@ -245,11 +280,23 @@
* \{
*/
-#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
+
+#elif defined(MBEDTLS_ECP_C)
/**
* The maximum size of the groups, that is, of \c N and \c P.
*/
-#define MBEDTLS_ECP_MAX_BITS 521 /**< The maximum size of groups, in bits. */
+#define MBEDTLS_ECP_MAX_BITS MBEDTLS_ECP_MAX_BITS_MIN
+
+#else
+/* MBEDTLS_ECP_MAX_BITS is not relevant without MBEDTLS_ECP_C, but set it
+ * to a nonzero value so that code that unconditionally allocates an array
+ * of a size based on it keeps working if built without ECC support. */
+#define MBEDTLS_ECP_MAX_BITS 1
#endif
#define MBEDTLS_ECP_MAX_BYTES ( ( MBEDTLS_ECP_MAX_BITS + 7 ) / 8 )
diff --git a/library/bignum.c b/library/bignum.c
index 3acc4b9..dd80eb4 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -258,6 +258,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.
@@ -270,8 +300,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 );
}
/*
@@ -283,20 +328,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 );
@@ -312,6 +371,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 );
@@ -319,22 +379,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:
@@ -2127,6 +2200,71 @@
}
/*
+ * 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_ERROR_CORRUPTION_DETECTED;
+
+ for( size_t 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,
@@ -2138,7 +2276,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 );
@@ -2162,6 +2300,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 );
@@ -2302,7 +2441,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;
@@ -2340,6 +2480,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 );
@@ -2466,9 +2607,10 @@
{
int ret = MBEDTLS_ERR_MPI_BAD_INPUT_DATA;
int count;
- unsigned cmp = 0;
+ unsigned lt_lower = 1, lt_upper = 0;
size_t n_bits = mbedtls_mpi_bitlen( N );
size_t n_bytes = ( n_bits + 7 ) / 8;
+ mbedtls_mpi lower_bound;
if( min < 0 )
return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA );
@@ -2494,10 +2636,14 @@
*/
count = ( n_bytes > 4 ? 30 : 250 );
+ mbedtls_mpi_init( &lower_bound );
+
/* Ensure that target MPI has exactly the same number of limbs
* as the upper bound, even if the upper bound has leading zeros.
* This is necessary for the mbedtls_mpi_lt_mpi_ct() check. */
MBEDTLS_MPI_CHK( mbedtls_mpi_resize_clear( X, N->n ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_grow( &lower_bound, N->n ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &lower_bound, min ) );
/*
* Match the procedure given in RFC 6979 §3.3 (deterministic ECDSA)
@@ -2518,11 +2664,13 @@
goto cleanup;
}
- MBEDTLS_MPI_CHK( mbedtls_mpi_lt_mpi_ct( X, N, &cmp ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_lt_mpi_ct( X, &lower_bound, <_lower ) );
+ MBEDTLS_MPI_CHK( mbedtls_mpi_lt_mpi_ct( X, N, <_upper ) );
}
- while( mbedtls_mpi_cmp_int( X, min ) < 0 || cmp != 1 );
+ while( lt_lower != 0 || lt_upper == 0 );
cleanup:
+ mbedtls_mpi_free( &lower_bound );
return( ret );
}
diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function
index 934598d..3ca563e 100644
--- a/tests/suites/test_suite_ecp.function
+++ b/tests/suites/test_suite_ecp.function
@@ -386,6 +386,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 */
@@ -794,6 +796,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 );
@@ -865,6 +868,7 @@
if( ret == 0 )
{
+ TEST_ASSERT( olen <= MBEDTLS_ECP_MAX_PT_LEN );
TEST_ASSERT( mbedtls_test_hexcmp( buf, out->x, olen, out->len ) == 0 );
}