From 4fa8334bae7a29e4d4b541cf5f0ebf2ac1b6b7d7 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 1 May 2023 22:30:54 +0100 Subject: [PATCH 1/7] Convert curve 448 to use ecp core functions Signed-off-by: Paul Elliott --- library/ecp_curves.c | 100 ++++++++++++++++++--------- library/ecp_invasive.h | 2 +- tests/suites/test_suite_ecp.function | 2 +- 3 files changed, 71 insertions(+), 33 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index b07753a074..094b25c675 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -22,6 +22,7 @@ #if defined(MBEDTLS_ECP_LIGHT) #include "mbedtls/ecp.h" +#include "mbedtls/platform.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" @@ -4608,7 +4609,7 @@ static int ecp_mod_p255(mbedtls_mpi *); #if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) static int ecp_mod_p448(mbedtls_mpi *); MBEDTLS_STATIC_TESTABLE -int mbedtls_ecp_mod_p448(mbedtls_mpi *); +int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *, size_t); #endif #if defined(MBEDTLS_ECP_DP_SECP192K1_ENABLED) static int ecp_mod_p192k1(mbedtls_mpi *); @@ -5455,7 +5456,18 @@ static int ecp_mod_p255(mbedtls_mpi *N) static int ecp_mod_p448(mbedtls_mpi *N) { - return mbedtls_ecp_mod_p448(N); + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t expected_width = 2 * ((448 + biL - 1) / biL); + + /* This is required as some tests and use cases do not pass in a Bignum of + * the correct size, and expect the growth to be done automatically, which + * will no longer happen. */ + MBEDTLS_MPI_CHK(mbedtls_mpi_grow(N, expected_width)); + + ret = mbedtls_ecp_mod_p448(N->p, N->n); + +cleanup: + return ret; } /* @@ -5470,56 +5482,82 @@ static int ecp_mod_p448(mbedtls_mpi *N) * the reduction with 224-bit limbs, since mpi_add_mpi will then use 64-bit adds. */ MBEDTLS_STATIC_TESTABLE -int mbedtls_ecp_mod_p448(mbedtls_mpi *N) +int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *N, size_t N_limbs) { - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t i; - mbedtls_mpi M, Q; - mbedtls_mpi_uint Mp[P448_WIDTH + 1], Qp[P448_WIDTH]; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if (N->n <= P448_WIDTH) { + if (N_limbs <= P448_WIDTH) { return 0; } - /* M = A1 */ - M.s = 1; - M.n = N->n - (P448_WIDTH); - if (M.n > P448_WIDTH) { + size_t M_limbs = N_limbs - (P448_WIDTH); + size_t Q_limbs = M_limbs; + + if (M_limbs > P448_WIDTH) { /* Shouldn't be called with N larger than 2^896! */ return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; } - M.p = Mp; - memset(Mp, 0, sizeof(Mp)); - memcpy(Mp, N->p + P448_WIDTH, M.n * sizeof(mbedtls_mpi_uint)); - /* N = A0 */ - for (i = P448_WIDTH; i < N->n; i++) { - N->p[i] = 0; + /* Extra limb for carry below. */ + M_limbs++; + + mbedtls_mpi_uint *M = mbedtls_calloc(M_limbs, ciL); + + if (M == NULL) { + return MBEDTLS_ERR_ECP_ALLOC_FAILED; } - /* N += A1 */ - MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(N, N, &M)); + /* M = A1 */ + memset(M, 0, (M_limbs * ciL)); + + /* Do not copy into the overflow limb, as this would read past the end of + * N. */ + memcpy(M, N + P448_WIDTH, ((M_limbs - 1) * ciL)); + + /* N = A0 */ + for (i = P448_WIDTH; i < N_limbs; i++) { + N[i] = 0; + } + + /* N += A1 - Carry here dealt with by oversize M and N. */ + (void) mbedtls_mpi_core_add(N, N, M, M_limbs); /* Q = B1, N += B1 */ - Q = M; - Q.p = Qp; - memcpy(Qp, Mp, sizeof(Qp)); - MBEDTLS_MPI_CHK(mbedtls_mpi_shift_r(&Q, 224)); - MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(N, N, &Q)); + mbedtls_mpi_uint *Q = mbedtls_calloc(Q_limbs, ciL); + + if (Q == NULL) { + ret = MBEDTLS_ERR_ECP_ALLOC_FAILED; + goto cleanup; + } + + memcpy(Q, M, (Q_limbs * ciL)); + + mbedtls_mpi_core_shift_r(Q, Q_limbs, 224); + + /* No carry here - only max 224 bits */ + (void) mbedtls_mpi_core_add(N, N, Q, Q_limbs); /* M = (B0 + B1) * 2^224, N += M */ if (sizeof(mbedtls_mpi_uint) > 4) { - Mp[P224_WIDTH_MIN] &= ((mbedtls_mpi_uint)-1) >> (P224_UNUSED_BITS); + M[P224_WIDTH_MIN] &= ((mbedtls_mpi_uint)-1) >> (P224_UNUSED_BITS); } - for (i = P224_WIDTH_MAX; i < M.n; ++i) { - Mp[i] = 0; + for (i = P224_WIDTH_MAX; i < M_limbs; ++i) { + M[i] = 0; } - MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(&M, &M, &Q)); - M.n = P448_WIDTH + 1; /* Make room for shifted carry bit from the addition */ - MBEDTLS_MPI_CHK(mbedtls_mpi_shift_l(&M, 224)); - MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(N, N, &M)); + + (void) mbedtls_mpi_core_add(M, M, Q, Q_limbs); + + /* Shifted carry bit from the addition is dealt with by oversize M */ + mbedtls_mpi_core_shift_l(M, M_limbs, 224); + (void) mbedtls_mpi_core_add(N, N, M, M_limbs); + + ret = 0; cleanup: + mbedtls_free(M); + mbedtls_free(Q); + return ret; } #endif /* MBEDTLS_ECP_DP_CURVE448_ENABLED */ diff --git a/library/ecp_invasive.h b/library/ecp_invasive.h index 68187acbc8..4cf4f6e4b4 100644 --- a/library/ecp_invasive.h +++ b/library/ecp_invasive.h @@ -196,7 +196,7 @@ int mbedtls_ecp_mod_p256k1(mbedtls_mpi *N); #if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) MBEDTLS_STATIC_TESTABLE -int mbedtls_ecp_mod_p448(mbedtls_mpi *N); +int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *N, size_t N_limbs); #endif /* MBEDTLS_ECP_DP_CURVE448_ENABLED */ diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index f034d6fc81..95aaef2a37 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1499,7 +1499,7 @@ void ecp_mod_p448(char *input_N, TEST_LE_U(X.n, 2 * limbs); TEST_EQUAL(res.n, limbs); - TEST_EQUAL(mbedtls_ecp_mod_p448(&X), 0); + TEST_EQUAL(mbedtls_ecp_mod_p448(X.p, X.n), 0); TEST_EQUAL(mbedtls_mpi_mod_mpi(&X, &X, &N), 0); TEST_LE_U(mbedtls_mpi_core_bitlen(X.p, X.n), 448); ASSERT_COMPARE(X.p, bytes, res.p, bytes); From 34b08e5005a41d2066707f9dc31f85e6caea772c Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 16 May 2023 15:28:30 +0100 Subject: [PATCH 2/7] Convert over to using X, X_limbs Signed-off-by: Paul Elliott --- library/ecp_curves.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 094b25c675..6c588f713a 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5482,16 +5482,16 @@ cleanup: * the reduction with 224-bit limbs, since mpi_add_mpi will then use 64-bit adds. */ MBEDTLS_STATIC_TESTABLE -int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *N, size_t N_limbs) +int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) { size_t i; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if (N_limbs <= P448_WIDTH) { + if (X_limbs <= P448_WIDTH) { return 0; } - size_t M_limbs = N_limbs - (P448_WIDTH); + size_t M_limbs = X_limbs - (P448_WIDTH); size_t Q_limbs = M_limbs; if (M_limbs > P448_WIDTH) { @@ -5513,15 +5513,15 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *N, size_t N_limbs) /* Do not copy into the overflow limb, as this would read past the end of * N. */ - memcpy(M, N + P448_WIDTH, ((M_limbs - 1) * ciL)); + memcpy(M, X + P448_WIDTH, ((M_limbs - 1) * ciL)); /* N = A0 */ - for (i = P448_WIDTH; i < N_limbs; i++) { - N[i] = 0; + for (i = P448_WIDTH; i < X_limbs; i++) { + X[i] = 0; } /* N += A1 - Carry here dealt with by oversize M and N. */ - (void) mbedtls_mpi_core_add(N, N, M, M_limbs); + (void) mbedtls_mpi_core_add(X, X, M, M_limbs); /* Q = B1, N += B1 */ mbedtls_mpi_uint *Q = mbedtls_calloc(Q_limbs, ciL); @@ -5536,7 +5536,7 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *N, size_t N_limbs) mbedtls_mpi_core_shift_r(Q, Q_limbs, 224); /* No carry here - only max 224 bits */ - (void) mbedtls_mpi_core_add(N, N, Q, Q_limbs); + (void) mbedtls_mpi_core_add(X, X, Q, Q_limbs); /* M = (B0 + B1) * 2^224, N += M */ if (sizeof(mbedtls_mpi_uint) > 4) { @@ -5550,7 +5550,7 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *N, size_t N_limbs) /* Shifted carry bit from the addition is dealt with by oversize M */ mbedtls_mpi_core_shift_l(M, M_limbs, 224); - (void) mbedtls_mpi_core_add(N, N, M, M_limbs); + (void) mbedtls_mpi_core_add(X, X, M, M_limbs); ret = 0; From 235c1947fb668a58646f13ddd3e0efdecb35a527 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 16 May 2023 15:51:23 +0100 Subject: [PATCH 3/7] Group memory allocations earlier Signed-off-by: Paul Elliott --- library/ecp_curves.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 6c588f713a..2bbec41e24 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5508,6 +5508,13 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) return MBEDTLS_ERR_ECP_ALLOC_FAILED; } + mbedtls_mpi_uint *Q = mbedtls_calloc(Q_limbs, ciL); + + if (Q == NULL) { + ret = MBEDTLS_ERR_ECP_ALLOC_FAILED; + goto cleanup; + } + /* M = A1 */ memset(M, 0, (M_limbs * ciL)); @@ -5524,13 +5531,6 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) (void) mbedtls_mpi_core_add(X, X, M, M_limbs); /* Q = B1, N += B1 */ - mbedtls_mpi_uint *Q = mbedtls_calloc(Q_limbs, ciL); - - if (Q == NULL) { - ret = MBEDTLS_ERR_ECP_ALLOC_FAILED; - goto cleanup; - } - memcpy(Q, M, (Q_limbs * ciL)); mbedtls_mpi_core_shift_r(Q, Q_limbs, 224); From 6b1f7f101fdb5ed1fb61ae9771c5808df1e91076 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 16 May 2023 15:59:56 +0100 Subject: [PATCH 4/7] Use const where appropriate Signed-off-by: Paul Elliott --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 2bbec41e24..c4ae8f9e35 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5492,7 +5492,7 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) } size_t M_limbs = X_limbs - (P448_WIDTH); - size_t Q_limbs = M_limbs; + const size_t Q_limbs = M_limbs; if (M_limbs > P448_WIDTH) { /* Shouldn't be called with N larger than 2^896! */ From c05f51ded9c0a2d3500a3cc3922befc52c0c013b Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 16 May 2023 17:55:44 +0100 Subject: [PATCH 5/7] Convert comments over to X rather than N Signed-off-by: Paul Elliott --- library/ecp_curves.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index c4ae8f9e35..d34eea2f92 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5472,14 +5472,14 @@ cleanup: /* * Fast quasi-reduction modulo p448 = 2^448 - 2^224 - 1 - * Write N as A0 + 2^448 A1 and A1 as B0 + 2^224 B1, and return - * A0 + A1 + B1 + (B0 + B1) * 2^224. This is different to the reference - * implementation of Curve448, which uses its own special 56-bit limbs rather - * than a generic bignum library. We could squeeze some extra speed out on - * 32-bit machines by splitting N up into 32-bit limbs and doing the - * arithmetic using the limbs directly as we do for the NIST primes above, - * but for 64-bit targets it should use half the number of operations if we do - * the reduction with 224-bit limbs, since mpi_add_mpi will then use 64-bit adds. + * Write X as A0 + 2^448 A1 and A1 as B0 + 2^224 B1, and return A0 + A1 + B1 + + * (B0 + B1) * 2^224. This is different to the reference implementation of + * Curve448, which uses its own special 56-bit limbs rather than a generic + * bignum library. We could squeeze some extra speed out on 32-bit machines by + * splitting N up into 32-bit limbs and doing the arithmetic using the limbs + * directly as we do for the NIST primes above, but for 64-bit targets it should + * use half the number of operations if we do the reduction with 224-bit limbs, + * since mpi_add_mpi will then use 64-bit adds. */ MBEDTLS_STATIC_TESTABLE int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) @@ -5495,7 +5495,7 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) const size_t Q_limbs = M_limbs; if (M_limbs > P448_WIDTH) { - /* Shouldn't be called with N larger than 2^896! */ + /* Shouldn't be called with X larger than 2^896! */ return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; } @@ -5519,7 +5519,7 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) memset(M, 0, (M_limbs * ciL)); /* Do not copy into the overflow limb, as this would read past the end of - * N. */ + * X. */ memcpy(M, X + P448_WIDTH, ((M_limbs - 1) * ciL)); /* N = A0 */ @@ -5527,10 +5527,10 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) X[i] = 0; } - /* N += A1 - Carry here dealt with by oversize M and N. */ + /* X += A1 - Carry here dealt with by oversize M and X. */ (void) mbedtls_mpi_core_add(X, X, M, M_limbs); - /* Q = B1, N += B1 */ + /* Q = B1, X += B1 */ memcpy(Q, M, (Q_limbs * ciL)); mbedtls_mpi_core_shift_r(Q, Q_limbs, 224); From 3b6bf105d1271ba624b22cc1bedd951b9c2be8bf Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 23 May 2023 17:51:52 +0100 Subject: [PATCH 6/7] Fix missed renames from N to X Signed-off-by: Paul Elliott --- library/ecp_curves.c | 4 ++-- library/ecp_invasive.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index d34eea2f92..42d151a4c6 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5522,7 +5522,7 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) * X. */ memcpy(M, X + P448_WIDTH, ((M_limbs - 1) * ciL)); - /* N = A0 */ + /* X = A0 */ for (i = P448_WIDTH; i < X_limbs; i++) { X[i] = 0; } @@ -5538,7 +5538,7 @@ int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs) /* No carry here - only max 224 bits */ (void) mbedtls_mpi_core_add(X, X, Q, Q_limbs); - /* M = (B0 + B1) * 2^224, N += M */ + /* M = (B0 + B1) * 2^224, X += M */ if (sizeof(mbedtls_mpi_uint) > 4) { M[P224_WIDTH_MIN] &= ((mbedtls_mpi_uint)-1) >> (P224_UNUSED_BITS); } diff --git a/library/ecp_invasive.h b/library/ecp_invasive.h index 4cf4f6e4b4..379e022b87 100644 --- a/library/ecp_invasive.h +++ b/library/ecp_invasive.h @@ -196,7 +196,7 @@ int mbedtls_ecp_mod_p256k1(mbedtls_mpi *N); #if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) MBEDTLS_STATIC_TESTABLE -int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *N, size_t N_limbs); +int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs); #endif /* MBEDTLS_ECP_DP_CURVE448_ENABLED */ From 7050662a48a1eb46f32509c182f5087c1139759e Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 24 May 2023 17:31:57 +0100 Subject: [PATCH 7/7] Correct comment header block Signed-off-by: Paul Elliott --- library/ecp_curves.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 42d151a4c6..597a340447 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5479,7 +5479,7 @@ cleanup: * splitting N up into 32-bit limbs and doing the arithmetic using the limbs * directly as we do for the NIST primes above, but for 64-bit targets it should * use half the number of operations if we do the reduction with 224-bit limbs, - * since mpi_add_mpi will then use 64-bit adds. + * since mpi_core_add will then use 64-bit adds. */ MBEDTLS_STATIC_TESTABLE int mbedtls_ecp_mod_p448(mbedtls_mpi_uint *X, size_t X_limbs)