From 053022fe24c35fe082f4296ac9c2ac5428b499bf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Jun 2023 19:26:48 +0200 Subject: [PATCH 1/5] Reduce the size of mbedtls_mpi Reduce the size of mbedtls_mpi from 3 words to 2 on most architectures. This also reduces the code size significantly in bignum.o and ecp_curves.o, with negligible variations in other modules. This removes the ability to set MBEDTLS_MPI_MAX_LIMBS to a value >=65536, but we don't support customizing this value anyway (it's always 10000). Signed-off-by: Gilles Peskine --- include/mbedtls/bignum.h | 7 +++++-- library/bignum.c | 12 ++++++++---- library/ecp_curves.c | 5 +++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index e7f3131740..96cc656913 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -214,10 +214,13 @@ typedef struct mbedtls_mpi { * Note that this implies that calloc() or `... = {0}` does not create * a valid MPI representation. You must call mbedtls_mpi_init(). */ - int MBEDTLS_PRIVATE(s); + signed short MBEDTLS_PRIVATE(s); /** Total number of limbs in \c p. */ - size_t MBEDTLS_PRIVATE(n); + unsigned short MBEDTLS_PRIVATE(n); +#if MBEDTLS_MPI_MAX_LIMBS > 65535 +#error "MBEDTLS_MPI_MAX_LIMBS > 65535 is not supported" +#endif /** Pointer to limbs. * diff --git a/library/bignum.c b/library/bignum.c index 36effaf8da..5b9293293e 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -114,7 +114,9 @@ int mbedtls_mpi_grow(mbedtls_mpi *X, size_t nblimbs) mbedtls_free(X->p); } - X->n = nblimbs; + /* nblimbs fits in n because we ensure that MBEDTLS_MPI_MAX_LIMBS + * fits, and we've checked that nblimbs <= MBEDTLS_MPI_MAX_LIMBS. */ + X->n = (unsigned short) nblimbs; X->p = p; } @@ -162,7 +164,9 @@ int mbedtls_mpi_shrink(mbedtls_mpi *X, size_t nblimbs) mbedtls_free(X->p); } - X->n = i; + /* i fits in n because we ensure that MBEDTLS_MPI_MAX_LIMBS + * fits, and we've checked that i <= nblimbs <= MBEDTLS_MPI_MAX_LIMBS. */ + X->n = (unsigned short) i; X->p = p; return 0; @@ -1574,8 +1578,8 @@ static void mpi_montred(mbedtls_mpi *A, const mbedtls_mpi *N, { mbedtls_mpi_uint z = 1; mbedtls_mpi U; - - U.n = U.s = (int) z; + U.n = 1; + U.s = 1; U.p = &z; mpi_montmul(A, &U, N, mm, T); diff --git a/library/ecp_curves.c b/library/ecp_curves.c index a4fa663a56..9acf778aee 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -4512,12 +4512,13 @@ static const mbedtls_ecp_point brainpoolP512r1_T[32] = { defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) /* * Create an MPI from embedded constants - * (assumes len is an exact multiple of sizeof(mbedtls_mpi_uint)) + * (assumes len is an exact multiple of sizeof(mbedtls_mpi_uint) and + * len < 1048576) */ static inline void ecp_mpi_load(mbedtls_mpi *X, const mbedtls_mpi_uint *p, size_t len) { X->s = 1; - X->n = len / sizeof(mbedtls_mpi_uint); + X->n = (unsigned short) (len / sizeof(mbedtls_mpi_uint)); X->p = (mbedtls_mpi_uint *) p; } #endif From 84eaefa43e03181f2d75c411a22c6fe68746ba36 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 11 Jul 2023 09:52:31 +0100 Subject: [PATCH 2/5] Use designated initializers for mbedtls_mpi Signed-off-by: Dave Rodgman --- library/ecp.c | 2 +- library/ecp_curves.c | 2 +- tests/suites/test_suite_bignum_random.function | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 086acb35e3..538d2cfd63 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2930,7 +2930,7 @@ int mbedtls_ecp_muladd(mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED) #if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) -#define ECP_MPI_INIT(s, n, p) { s, (n), (mbedtls_mpi_uint *) (p) } +#define ECP_MPI_INIT(_s, _n, _p) { .s = (_s), .n = (_n), .p = (mbedtls_mpi_uint *) (_p) } #define ECP_MPI_INIT_ARRAY(x) \ ECP_MPI_INIT(1, sizeof(x) / sizeof(mbedtls_mpi_uint), x) /* diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 9acf778aee..1f9dc71869 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -44,7 +44,7 @@ #define ECP_VALIDATE(cond) \ MBEDTLS_INTERNAL_VALIDATE(cond) -#define ECP_MPI_INIT(s, n, p) { s, (n), (mbedtls_mpi_uint *) (p) } +#define ECP_MPI_INIT(_s, _n, _p) { .s = (_s), .n = (_n), .p = (mbedtls_mpi_uint *) (_p) } #define ECP_MPI_INIT_ARRAY(x) \ ECP_MPI_INIT(1, sizeof(x) / sizeof(mbedtls_mpi_uint), x) diff --git a/tests/suites/test_suite_bignum_random.function b/tests/suites/test_suite_bignum_random.function index e4db3d7acc..34221a796e 100644 --- a/tests/suites/test_suite_bignum_random.function +++ b/tests/suites/test_suite_bignum_random.function @@ -312,8 +312,8 @@ void mpi_random_many(int min, char *bound_hex, int iterations) /* Temporarily use a legacy MPI for analysis, because the * necessary auxiliary functions don't exist yet in core. */ - mbedtls_mpi B = { 1, limbs, upper_bound }; - mbedtls_mpi R = { 1, limbs, result }; + mbedtls_mpi B = { .s = 1, .n = limbs, .p = upper_bound }; + mbedtls_mpi R = { .s = 1, .n = limbs, .p = result }; TEST_ASSERT(mbedtls_mpi_cmp_mpi(&R, &B) < 0); TEST_ASSERT(mbedtls_mpi_cmp_int(&R, min) >= 0); From 98e632f2102ccc4faca93517342dfd067a9842f6 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 11 Jul 2023 15:59:14 +0100 Subject: [PATCH 3/5] Re-order mbedtls_mpi to save a few extra bytes with clang Signed-off-by: Dave Rodgman --- include/mbedtls/bignum.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index 96cc656913..a8422b1fce 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -203,6 +203,12 @@ extern "C" { * \brief MPI structure */ typedef struct mbedtls_mpi { + /** Pointer to limbs. + * + * This may be \c NULL if \c n is 0. + */ + mbedtls_mpi_uint *MBEDTLS_PRIVATE(p); + /** Sign: -1 if the mpi is negative, 1 otherwise. * * The number 0 must be represented with `s = +1`. Although many library @@ -221,12 +227,6 @@ typedef struct mbedtls_mpi { #if MBEDTLS_MPI_MAX_LIMBS > 65535 #error "MBEDTLS_MPI_MAX_LIMBS > 65535 is not supported" #endif - - /** Pointer to limbs. - * - * This may be \c NULL if \c n is 0. - */ - mbedtls_mpi_uint *MBEDTLS_PRIVATE(p); } mbedtls_mpi; From b8f18850c60dceb3750c105d56f9b57062b132bd Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 18 Jul 2023 12:45:17 +0100 Subject: [PATCH 4/5] Align ECP_MPI_INIT parameter order with mbedtls_mpi struct order Signed-off-by: Dave Rodgman --- library/ecp.c | 4 ++-- library/ecp_curves.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 538d2cfd63..24cd21b24c 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2930,9 +2930,9 @@ int mbedtls_ecp_muladd(mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED) #if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) -#define ECP_MPI_INIT(_s, _n, _p) { .s = (_s), .n = (_n), .p = (mbedtls_mpi_uint *) (_p) } +#define ECP_MPI_INIT(_p, _n) { .p = (mbedtls_mpi_uint *) (_p), .s = 1, .n = (_n) } #define ECP_MPI_INIT_ARRAY(x) \ - ECP_MPI_INIT(1, sizeof(x) / sizeof(mbedtls_mpi_uint), x) + ECP_MPI_INIT(x, sizeof(x) / sizeof(mbedtls_mpi_uint)) /* * Constants for the two points other than 0, 1, -1 (mod p) in * https://cr.yp.to/ecdh.html#validate diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 1f9dc71869..3d3ec60f94 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -44,15 +44,15 @@ #define ECP_VALIDATE(cond) \ MBEDTLS_INTERNAL_VALIDATE(cond) -#define ECP_MPI_INIT(_s, _n, _p) { .s = (_s), .n = (_n), .p = (mbedtls_mpi_uint *) (_p) } +#define ECP_MPI_INIT(_p, _n) { .p = (mbedtls_mpi_uint *) (_p), .s = 1, .n = (_n) } #define ECP_MPI_INIT_ARRAY(x) \ - ECP_MPI_INIT(1, sizeof(x) / sizeof(mbedtls_mpi_uint), x) + ECP_MPI_INIT(x, sizeof(x) / sizeof(mbedtls_mpi_uint)) #define ECP_POINT_INIT_XY_Z0(x, y) { \ - ECP_MPI_INIT_ARRAY(x), ECP_MPI_INIT_ARRAY(y), ECP_MPI_INIT(1, 0, NULL) } + ECP_MPI_INIT_ARRAY(x), ECP_MPI_INIT_ARRAY(y), ECP_MPI_INIT(NULL, 0) } #define ECP_POINT_INIT_XY_Z1(x, y) { \ - ECP_MPI_INIT_ARRAY(x), ECP_MPI_INIT_ARRAY(y), ECP_MPI_INIT(1, 1, mpi_one) } + ECP_MPI_INIT_ARRAY(x), ECP_MPI_INIT_ARRAY(y), ECP_MPI_INIT(mpi_one, 1) } #if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED) || \ defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED) || \ From 24a305ec22901ea58a28c8b82eedd4555316d944 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 Jul 2023 13:41:22 +0200 Subject: [PATCH 5/5] Explain why we check 65535 (not USHORT_MAX) Signed-off-by: Gilles Peskine --- include/mbedtls/bignum.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/mbedtls/bignum.h b/include/mbedtls/bignum.h index a8422b1fce..eb372cf1fd 100644 --- a/include/mbedtls/bignum.h +++ b/include/mbedtls/bignum.h @@ -224,6 +224,12 @@ typedef struct mbedtls_mpi { /** Total number of limbs in \c p. */ unsigned short MBEDTLS_PRIVATE(n); + /* Make sure that MBEDTLS_MPI_MAX_LIMBS fits in n. + * Use the same limit value on all platforms so that we don't have to + * think about different behavior on the rare platforms where + * unsigned short can store values larger than the minimum required by + * the C language, which is 65535. + */ #if MBEDTLS_MPI_MAX_LIMBS > 65535 #error "MBEDTLS_MPI_MAX_LIMBS > 65535 is not supported" #endif