diff --git a/ChangeLog.d/ecp_max_bits.txt b/ChangeLog.d/ecp_max_bits.txt new file mode 100644 index 0000000000..834dedaa56 --- /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/configs/config-suite-b.h b/configs/config-suite-b.h index 6eb03a97e4..9cad382739 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 47dd5e2227..8464fcb1bd 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 d0e61c5453..79aacf7b8a 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 b974e5984b..48eec61d5e 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -96,6 +96,7 @@ extern "C" { * - 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 @@ typedef struct mbedtls_ecp_point } 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 @@ mbedtls_ecp_group; * \{ */ -#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/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 934598dd37..3ca563e8c9 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -386,6 +386,8 @@ void mbedtls_ecp_curve_info( int id, int tls_id, int size, char * name ) 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 @@ void ecp_muladd( int id, 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 @@ void ecp_write_binary( int id, char * x, char * y, char * z, int format, if( ret == 0 ) { + TEST_ASSERT( olen <= MBEDTLS_ECP_MAX_PT_LEN ); TEST_ASSERT( mbedtls_test_hexcmp( buf, out->x, olen, out->len ) == 0 ); }