From c221e794e2628a44b5de441b019a4b29c7d6ff8b Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 1 Feb 2023 13:58:04 +0100 Subject: [PATCH 01/13] Extract Secp521r1 from the prototype Signed-off-by: Gabor Mezei --- library/ecp_curves.c | 65 +++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 727283f73a98..be542d0c972a 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5162,11 +5162,6 @@ static int ecp_mod_p384(mbedtls_mpi *N) MBEDTLS_ECP_DP_SECP384R1_ENABLED */ #if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) -/* - * Here we have an actual Mersenne prime, so things are more straightforward. - * However, chunks are aligned on a 'weird' boundary (521 bits). - */ - /* Size of p521 in terms of mbedtls_mpi_uint */ #define P521_WIDTH (521 / 8 / sizeof(mbedtls_mpi_uint) + 1) @@ -5174,48 +5169,56 @@ static int ecp_mod_p384(mbedtls_mpi *N) #define P521_MASK 0x01FF /* - * Fast quasi-reduction modulo p521 (FIPS 186-3 D.2.5) - * Write N as A1 + 2^521 A0, return A0 + A1 + * Fast quasi-reduction modulo p521 = 2^521 - 1 (FIPS 186-3 D.2.5) */ static int ecp_mod_p521(mbedtls_mpi *N) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t i; - mbedtls_mpi M; - mbedtls_mpi_uint Mp[P521_WIDTH + 1]; - /* Worst case for the size of M is when mbedtls_mpi_uint is 16 bits: - * we need to hold bits 513 to 1056, which is 34 limbs, that is - * P521_WIDTH + 1. Otherwise P521_WIDTH is enough. */ + size_t expected_width = 2 * ((521 + biL - 1) / biL); + MBEDTLS_MPI_CHK(mbedtls_mpi_grow(N, expected_width)); + ret = ecp_mod_p521_raw(N->p, expected_width); +cleanup: + return ret; +} +static int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) +{ + mbedtls_mpi_uint carry = 0; - if (N->n < P521_WIDTH) { + if (N_n > 2*P521_WIDTH) { + N_n = 2*P521_WIDTH; + } + if (N_n < P521_WIDTH) { return 0; } - /* M = A1 */ - M.s = 1; - M.n = N->n - (P521_WIDTH - 1); - if (M.n > P521_WIDTH + 1) { - M.n = P521_WIDTH + 1; - } - M.p = Mp; - memcpy(Mp, N->p + P521_WIDTH - 1, M.n * sizeof(mbedtls_mpi_uint)); - MBEDTLS_MPI_CHK(mbedtls_mpi_shift_r(&M, 521 % (8 * sizeof(mbedtls_mpi_uint)))); + /* Step 1: Reduction to P521_WIDTH limbs */ + if (N_n > P521_WIDTH) { + /* Helper references for top part of N */ + mbedtls_mpi_uint *NT_p = N_p + P521_WIDTH; + size_t NT_n = N_n - P521_WIDTH; - /* N = A0 */ - N->p[P521_WIDTH - 1] &= P521_MASK; - for (i = P521_WIDTH; i < N->n; i++) { - N->p[i] = 0; + /* Split N as A0 + 2^(512 + biL) A1 and compute A0 + 2^(biL - 9) * A1. + * This can be done in place. */ + mbedtls_mpi_uint shift = ((mbedtls_mpi_uint) 1u) << (biL - 9); + carry = MPI_CORE(mla)(N_p, P521_WIDTH, NT_p, NT_n, shift); + + /* Clear top part */ + memset(NT_p, 0, sizeof(mbedtls_mpi_uint) * NT_n); } - /* N = A0 + A1 */ - MBEDTLS_MPI_CHK(mbedtls_mpi_add_abs(N, N, &M)); + /* Step 2: Reduction to < 2p. + * Now split as A0 + 2^521 * c, with c a scalar, and compute A0 + c. */ + carry <<= (biL - 9); + carry += (N_p[P521_WIDTH-1] >> 9); + N_p[P521_WIDTH-1] &= P521_MASK; + (void) mbedtls_core_add_int(N_p, N_p, carry, P521_WIDTH); -cleanup: - return ret; + return 0; } #undef P521_WIDTH #undef P521_MASK + #endif /* MBEDTLS_ECP_DP_SECP521R1_ENABLED */ #endif /* MBEDTLS_ECP_NIST_OPTIM */ From 32fa5ccb0d6af0a8161eee2722f2bcebcdb227fb Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 6 Feb 2023 15:47:00 +0100 Subject: [PATCH 02/13] Fix Secp521r1 reduction The prototype calculated with wrong limb size and not taken into account the overflow in the shared limb. Signed-off-by: Gabor Mezei --- library/ecp_curves.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index be542d0c972a..1565b2d5a821 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5184,14 +5184,19 @@ static int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) { mbedtls_mpi_uint carry = 0; - if (N_n > 2*P521_WIDTH) { - N_n = 2*P521_WIDTH; + if (N_n > 2 * P521_WIDTH - 1) { + N_n = 2 * P521_WIDTH - 1; } if (N_n < P521_WIDTH) { return 0; } - /* Step 1: Reduction to P521_WIDTH limbs */ + /* Save and clear the A1 content of the shared limb to prevent it + from overwrite. */ + mbedtls_mpi_uint remainder[P521_WIDTH] = {0}; + remainder[0] = N_p[P521_WIDTH - 1] >> 9; + N_p[P521_WIDTH - 1] &= P521_MASK; + if (N_n > P521_WIDTH) { /* Helper references for top part of N */ mbedtls_mpi_uint *NT_p = N_p + P521_WIDTH; @@ -5200,18 +5205,14 @@ static int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) /* Split N as A0 + 2^(512 + biL) A1 and compute A0 + 2^(biL - 9) * A1. * This can be done in place. */ mbedtls_mpi_uint shift = ((mbedtls_mpi_uint) 1u) << (biL - 9); - carry = MPI_CORE(mla)(N_p, P521_WIDTH, NT_p, NT_n, shift); + carry = mbedtls_mpi_core_mla(N_p, P521_WIDTH - 1, NT_p, NT_n, shift); /* Clear top part */ memset(NT_p, 0, sizeof(mbedtls_mpi_uint) * NT_n); } - /* Step 2: Reduction to < 2p. - * Now split as A0 + 2^521 * c, with c a scalar, and compute A0 + c. */ - carry <<= (biL - 9); - carry += (N_p[P521_WIDTH-1] >> 9); - N_p[P521_WIDTH-1] &= P521_MASK; - (void) mbedtls_core_add_int(N_p, N_p, carry, P521_WIDTH); + (void)mbedtls_mpi_core_add(N_p, N_p, remainder, P521_WIDTH); + N_p[P521_WIDTH - 1] += carry; return 0; } From 23c47f5b54fe0758aea231a175e8b83a8f4c8ea0 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 1 Feb 2023 14:02:16 +0100 Subject: [PATCH 03/13] Change the ecp_mod_p521_raw to be testable Signed-off-by: Gabor Mezei --- library/ecp_curves.c | 6 +++++- library/ecp_invasive.h | 7 +++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 1565b2d5a821..a1f3ffecd610 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -4582,6 +4582,8 @@ static int ecp_mod_p384(mbedtls_mpi *); #endif #if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) static int ecp_mod_p521(mbedtls_mpi *); +MBEDTLS_STATIC_TESTABLE +int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n); #endif #define NIST_MODP(P) grp->modp = ecp_mod_ ## P; @@ -5180,7 +5182,9 @@ static int ecp_mod_p521(mbedtls_mpi *N) cleanup: return ret; } -static int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) + +MBEDTLS_STATIC_TESTABLE +int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) { mbedtls_mpi_uint carry = 0; diff --git a/library/ecp_invasive.h b/library/ecp_invasive.h index 18815be08911..5b19d6eba1a1 100644 --- a/library/ecp_invasive.h +++ b/library/ecp_invasive.h @@ -76,6 +76,13 @@ int mbedtls_ecp_gen_privkey_mx(size_t n_bits, #endif /* MBEDTLS_ECP_MONTGOMERY_ENABLED */ +#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) + +MBEDTLS_STATIC_TESTABLE +int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n); + +#endif /* MBEDTLS_ECP_DP_SECP521R1_ENABLED */ + #endif /* MBEDTLS_TEST_HOOKS && MBEDTLS_ECP_C */ #endif /* MBEDTLS_ECP_INVASIVE_H */ From 23b58e5654c73e7ae05a4a7657eaea2210eec59b Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 1 Feb 2023 14:13:57 +0100 Subject: [PATCH 04/13] Add test generation support for the ecp module Signed-off-by: Gabor Mezei --- scripts/make_generated_files.bat | 1 + scripts/mbedtls_dev/ecp.py | 24 +++++++++ tests/CMakeLists.txt | 51 +++++++++++++++++++- tests/Makefile | 21 +++++++- tests/scripts/check-generated-files.sh | 1 + tests/scripts/generate_ecp_tests.py | 67 ++++++++++++++++++++++++++ 6 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 scripts/mbedtls_dev/ecp.py create mode 100755 tests/scripts/generate_ecp_tests.py diff --git a/scripts/make_generated_files.bat b/scripts/make_generated_files.bat index e9d92758a641..9cf34f6fd220 100644 --- a/scripts/make_generated_files.bat +++ b/scripts/make_generated_files.bat @@ -11,4 +11,5 @@ python scripts\generate_ssl_debug_helpers.py || exit /b 1 perl scripts\generate_visualc_files.pl || exit /b 1 python scripts\generate_psa_constants.py || exit /b 1 python tests\scripts\generate_bignum_tests.py || exit /b 1 +python tests\scripts\generate_ecp_tests.py || exit /b 1 python tests\scripts\generate_psa_tests.py || exit /b 1 diff --git a/scripts/mbedtls_dev/ecp.py b/scripts/mbedtls_dev/ecp.py new file mode 100644 index 000000000000..8874c42968ea --- /dev/null +++ b/scripts/mbedtls_dev/ecp.py @@ -0,0 +1,24 @@ +"""Framework classes for generation of ecp test cases.""" +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from typing import List + +from . import test_data_generation + +class EcpTarget(test_data_generation.BaseTarget): + #pylint: disable=abstract-method, too-few-public-methods + """Target for ecp test case generation.""" + target_basename = 'test_suite_ecp.generated' diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4a7de820aa45..4549a7a2b67d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -29,6 +29,18 @@ execute_process( string(REGEX REPLACE "[^;]*/" "" base_bignum_generated_data_files "${base_bignum_generated_data_files}") +execute_process( + COMMAND + ${MBEDTLS_PYTHON_EXECUTABLE} + ${CMAKE_CURRENT_SOURCE_DIR}/../tests/scripts/generate_ecp_tests.py + --list-for-cmake + WORKING_DIRECTORY + ${CMAKE_CURRENT_SOURCE_DIR}/.. + OUTPUT_VARIABLE + base_ecp_generated_data_files) +string(REGEX REPLACE "[^;]*/" "" + base_ecp_generated_data_files "${base_ecp_generated_data_files}") + execute_process( COMMAND ${MBEDTLS_PYTHON_EXECUTABLE} @@ -44,14 +56,18 @@ string(REGEX REPLACE "[^;]*/" "" # Derive generated file paths in the build directory. The generated data # files go into the suites/ subdirectory. set(base_generated_data_files - ${base_bignum_generated_data_files} ${base_psa_generated_data_files}) + ${base_bignum_generated_data_files} ${base_ecp_generated_data_files} ${base_psa_generated_data_files}) string(REGEX REPLACE "([^;]+)" "suites/\\1" all_generated_data_files "${base_generated_data_files}") set(bignum_generated_data_files "") +set(ecp_generated_data_files "") set(psa_generated_data_files "") foreach(file ${base_bignum_generated_data_files}) list(APPEND bignum_generated_data_files ${CMAKE_CURRENT_BINARY_DIR}/suites/${file}) endforeach() +foreach(file ${base_ecp_generated_data_files}) + list(APPEND ecp_generated_data_files ${CMAKE_CURRENT_BINARY_DIR}/suites/${file}) +endforeach() foreach(file ${base_psa_generated_data_files}) list(APPEND psa_generated_data_files ${CMAKE_CURRENT_BINARY_DIR}/suites/${file}) endforeach() @@ -75,6 +91,22 @@ if(GEN_FILES) ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_case.py ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_data_generation.py ) + add_custom_command( + OUTPUT + ${ecp_generated_data_files} + WORKING_DIRECTORY + ${CMAKE_CURRENT_SOURCE_DIR}/.. + COMMAND + ${MBEDTLS_PYTHON_EXECUTABLE} + ${CMAKE_CURRENT_SOURCE_DIR}/../tests/scripts/generate_ecp_tests.py + --directory ${CMAKE_CURRENT_BINARY_DIR}/suites + DEPENDS + ${CMAKE_CURRENT_SOURCE_DIR}/../tests/scripts/generate_ecp_tests.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/bignum_common.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/ecp.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_case.py + ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_data_generation.py + ) add_custom_command( OUTPUT ${psa_generated_data_files} @@ -107,6 +139,7 @@ endif() # With this line, only 4 sub-makefiles include the above command, that reduces # the risk of a race. add_custom_target(test_suite_bignum_generated_data DEPENDS ${bignum_generated_data_files}) +add_custom_target(test_suite_ecp_generated_data DEPENDS ${ecp_generated_data_files}) add_custom_target(test_suite_psa_generated_data DEPENDS ${psa_generated_data_files}) # If SKIP_TEST_SUITES is not defined with -D, get it from the environment. if((NOT DEFINED SKIP_TEST_SUITES) AND (DEFINED ENV{SKIP_TEST_SUITES})) @@ -129,6 +162,7 @@ function(add_test_suite suite_name) # Get the test names of the tests with generated .data files # from the generated_data_files list in parent scope. set(bignum_generated_data_names "") + set(ecp_generated_data_names "") set(psa_generated_data_names "") foreach(generated_data_file ${bignum_generated_data_files}) # Get the plain filename @@ -139,6 +173,15 @@ function(add_test_suite suite_name) string(SUBSTRING ${generated_data_name} 11 -1 generated_data_name) list(APPEND bignum_generated_data_names ${generated_data_name}) endforeach() + foreach(generated_data_file ${ecp_generated_data_files}) + # Get the plain filename + get_filename_component(generated_data_name ${generated_data_file} NAME) + # Remove the ".data" extension + get_name_without_last_ext(generated_data_name ${generated_data_name}) + # Remove leading "test_suite_" + string(SUBSTRING ${generated_data_name} 11 -1 generated_data_name) + list(APPEND ecp_generated_data_names ${generated_data_name}) + endforeach() foreach(generated_data_file ${psa_generated_data_files}) # Get the plain filename get_filename_component(generated_data_name ${generated_data_file} NAME) @@ -153,6 +196,10 @@ function(add_test_suite suite_name) set(data_file ${CMAKE_CURRENT_BINARY_DIR}/suites/test_suite_${data_name}.data) set(dependency test_suite_bignum_generated_data) + elseif(";${ecp_generated_data_names};" MATCHES ";${data_name};") + set(data_file + ${CMAKE_CURRENT_BINARY_DIR}/suites/test_suite_${data_name}.data) + set(dependency test_suite_ecp_generated_data) elseif(";${psa_generated_data_names};" MATCHES ";${data_name};") set(data_file ${CMAKE_CURRENT_BINARY_DIR}/suites/test_suite_${data_name}.data) @@ -160,7 +207,7 @@ function(add_test_suite suite_name) else() set(data_file ${CMAKE_CURRENT_SOURCE_DIR}/suites/test_suite_${data_name}.data) - set(dependency test_suite_bignum_generated_data test_suite_psa_generated_data) + set(dependency test_suite_bignum_generated_data test_suite_ecp_generated_data test_suite_psa_generated_data) endif() add_custom_command( diff --git a/tests/Makefile b/tests/Makefile index 312607ee3024..c9283c984ff3 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -73,6 +73,13 @@ GENERATED_BIGNUM_DATA_FILES := $(patsubst tests/%,%,$(shell \ ifeq ($(GENERATED_BIGNUM_DATA_FILES),FAILED) $(error "$(PYTHON) scripts/generate_bignum_tests.py --list" failed) endif +GENERATED_ECP_DATA_FILES := $(patsubst tests/%,%,$(shell \ + $(PYTHON) scripts/generate_ecp_tests.py --list || \ + echo FAILED \ +)) +ifeq ($(GENERATED_ECP_DATA_FILES),FAILED) +$(error "$(PYTHON) scripts/generate_ecp_tests.py --list" failed) +endif GENERATED_PSA_DATA_FILES := $(patsubst tests/%,%,$(shell \ $(PYTHON) scripts/generate_psa_tests.py --list || \ echo FAILED \ @@ -80,7 +87,7 @@ GENERATED_PSA_DATA_FILES := $(patsubst tests/%,%,$(shell \ ifeq ($(GENERATED_PSA_DATA_FILES),FAILED) $(error "$(PYTHON) scripts/generate_psa_tests.py --list" failed) endif -GENERATED_FILES := $(GENERATED_PSA_DATA_FILES) $(GENERATED_BIGNUM_DATA_FILES) +GENERATED_FILES := $(GENERATED_PSA_DATA_FILES) $(GENERATED_ECP_DATA_FILES) $(GENERATED_BIGNUM_DATA_FILES) generated_files: $(GENERATED_FILES) # generate_bignum_tests.py and generate_psa_tests.py spend more time analyzing @@ -89,7 +96,7 @@ generated_files: $(GENERATED_FILES) # It's rare not to want all the outputs. So always generate all of its outputs. # Use an intermediate phony dependency so that parallel builds don't run # a separate instance of the recipe for each output file. -.SECONDARY: generated_bignum_test_data generated_psa_test_data +.SECONDARY: generated_bignum_test_data generated_ecp_test_data generated_psa_test_data $(GENERATED_BIGNUM_DATA_FILES): generated_bignum_test_data generated_bignum_test_data: scripts/generate_bignum_tests.py generated_bignum_test_data: ../scripts/mbedtls_dev/bignum_common.py @@ -102,6 +109,16 @@ generated_bignum_test_data: echo " Gen $(GENERATED_BIGNUM_DATA_FILES)" $(PYTHON) scripts/generate_bignum_tests.py +$(GENERATED_ECP_DATA_FILES): generated_ecp_test_data +generated_ecp_test_data: scripts/generate_ecp_tests.py +generated_ecp_test_data: ../scripts/mbedtls_dev/bignum_common.py +generated_ecp_test_data: ../scripts/mbedtls_dev/ecp.py +generated_ecp_test_data: ../scripts/mbedtls_dev/test_case.py +generated_ecp_test_data: ../scripts/mbedtls_dev/test_data_generation.py +generated_ecp_test_data: + echo " Gen $(GENERATED_ECP_DATA_FILES)" + $(PYTHON) scripts/generate_ecp_tests.py + $(GENERATED_PSA_DATA_FILES): generated_psa_test_data generated_psa_test_data: scripts/generate_psa_tests.py generated_psa_test_data: ../scripts/mbedtls_dev/crypto_knowledge.py diff --git a/tests/scripts/check-generated-files.sh b/tests/scripts/check-generated-files.sh index 2bb9fea7ceca..4d6f93079c88 100755 --- a/tests/scripts/check-generated-files.sh +++ b/tests/scripts/check-generated-files.sh @@ -137,4 +137,5 @@ check scripts/generate_ssl_debug_helpers.py library/ssl_debug_helpers_generated. check scripts/generate_visualc_files.pl visualc/VS2013 check scripts/generate_psa_constants.py programs/psa/psa_constant_names_generated.c check tests/scripts/generate_bignum_tests.py $(tests/scripts/generate_bignum_tests.py --list) +check tests/scripts/generate_ecp_tests.py $(tests/scripts/generate_ecp_tests.py --list) check tests/scripts/generate_psa_tests.py $(tests/scripts/generate_psa_tests.py --list) diff --git a/tests/scripts/generate_ecp_tests.py b/tests/scripts/generate_ecp_tests.py new file mode 100755 index 000000000000..b3c1d10cfc4f --- /dev/null +++ b/tests/scripts/generate_ecp_tests.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python3 +"""Generate test data for ecp functions. + +With no arguments, generate all test data. With non-option arguments, +generate only the specified files. + +Class structure: + +Child classes of test_data_generation.BaseTarget (file targets) represent an output +file. These indicate where test cases will be written to, for all subclasses of +this target. Multiple file targets should not reuse a `target_basename`. + +Each subclass derived from a file target can either be: + - A concrete class, representing a test function, which generates test cases. + - An abstract class containing shared methods and attributes, not associated + with a test function. + +Both concrete and abstract subclasses can be derived from, to implement +additional test cases (see BignumCmp and BignumCmpAbs for examples of deriving +from abstract and concrete classes). + + +Adding test case generation for a function: + +A subclass representing the test function should be added, deriving from a +file target such as BignumTarget. This test class must set/implement the +following: + - test_function: the function name from the associated .function file. + - test_name: a descriptive name or brief summary to refer to the test + function. + - arguments(): a method to generate the list of arguments required for the + test_function. + - generate_function_tests(): a method to generate TestCases for the function. + This should create instances of the class with required input data, and + call `.create_test_case()` to yield the TestCase. + +Additional details and other attributes/methods are given in the documentation +of BaseTarget in test_data_generation.py. +""" + +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys + +import scripts_path # pylint: disable=unused-import +from mbedtls_dev import test_data_generation +# Import modules containing additional test classes +# Test function classes in these modules will be registered by +# the framework +from mbedtls_dev import ecp # pylint: disable=unused-import + +if __name__ == '__main__': + # Use the section of the docstring relevant to the CLI as description + test_data_generation.main(sys.argv[1:], "\n".join(__doc__.splitlines()[:4])) From 309801c1dde09887e1a254fddadde03a9a2e45a7 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 6 Feb 2023 15:49:42 +0100 Subject: [PATCH 05/13] Add test generation for ecp_mod_p521_raw Signed-off-by: Gabor Mezei --- scripts/mbedtls_dev/ecp.py | 84 ++++++++++++++++++++++++++++ tests/suites/test_suite_ecp.function | 44 +++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/scripts/mbedtls_dev/ecp.py b/scripts/mbedtls_dev/ecp.py index 8874c42968ea..09dcba281920 100644 --- a/scripts/mbedtls_dev/ecp.py +++ b/scripts/mbedtls_dev/ecp.py @@ -17,8 +17,92 @@ from typing import List from . import test_data_generation +from . import bignum_common class EcpTarget(test_data_generation.BaseTarget): #pylint: disable=abstract-method, too-few-public-methods """Target for ecp test case generation.""" target_basename = 'test_suite_ecp.generated' + +class EcpP521R1Raw(bignum_common.ModOperationCommon, + EcpTarget): + """Test cases for ecp quasi_reduction().""" + test_function = "ecp_mod_p521_raw" + test_name = "ecp_mod_p521_raw" + input_style = "fixed" + arity = 1 + + moduli = [("01ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff") + ] # type: List[str] + + input_values = [ + "0", "1", + + # Test case for overflow during addition + ("0001efffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "000001ef" + "0000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000f000000"), + + # First 8 number generated by random.getrandbits(1042) - seed(2,2) + ("0003cc2e82523e86feac7eb7dc38f519b91751dacdbd47d364be8049a372db8f" + "6e405d93ffed9235288bc781ae66267594c9c9500925e4749b575bd13653f8dd" + "9b1f282e" + "4067c3584ee207f8da94e3e8ab73738fcf1822ffbc6887782b491044d5e34124" + "5c6e433715ba2bdd177219d30e7a269fd95bafc8f2a4d27bdcf4bb99f4bea973"), + ("00017052829e07b0829a48d422fe99a22c70501e533c91352d3d854e061b9030" + "3b08c6e33c7295782d6c797f8f7d9b782a1be9cd8697bbd0e2520e33e44c5055" + "6c71c4a6" + "6148a86fe8624fab5186ee32ee8d7ee9770348a05d300cb90706a045defc044a" + "09325626e6b58de744ab6cce80877b6f71e1f6d2ef8acd128b4f2fc15f3f57eb"), + ("00021f15a7a83ee0761ebfd2bd143fa9b714210c665d7435c1066932f4767f26" + "294365b2721dea3bf63f23d0dbe53fcafb2147df5ca495fa5a91c89b97eeab64" + "ca2ce6bc" + "5d3fd983c34c769fe89204e2e8168561867e5e15bc01bfce6a27e0dfcbf87544" + "72154e76e4c11ab2fec3f6b32e8d4b8a8f54f8ceacaab39e83844b40ffa9b9f1"), + ("000381bc2a838af8d5c44a4eb3172062d08f1bb2531d6460f0caeef038c89b38" + "a8acb5137c9260dc74e088a9b9492f258ebdbfe3eb9ac688b9d39cca91551e82" + "59cc60b1" + "7604e4b4e73695c3e652c71a74667bffe202849da9643a295a9ac6decbd4d3e2" + "d4dec9ef83f0be4e80371eb97f81375eecc1cb6347733e847d718d733ff98ff3"), + ("00034816c8c69069134bccd3e1cf4f589f8e4ce0af29d115ef24bd625dd961e6" + "830b54fa7d28f93435339774bb1e386c4fd5079e681b8f5896838b769da59b74" + "a6c3181c" + "81e220df848b1df78feb994a81167346d4c0dca8b4c9e755cc9c3adcf515a823" + "4da4daeb4f3f87777ad1f45ae9500ec9c5e2486c44a4a8f69dc8db48e86ec9c6"), + ("000397846c4454b90f756132e16dce72f18e859835e1f291d322a7353ead4efe" + "440e2b4fda9c025a22f1a83185b98f5fc11e60de1b343f52ea748db9e020307a" + "aeb6db2c" + "3a038a709779ac1f45e9dd320c855fdfa7251af0930cdbd30f0ad2a81b2d19a2" + "beaa14a7ff3fe32a30ffc4eed0a7bd04e85bfcdd0227eeb7b9d7d01f5769da05"), + ("00002c3296e6bc4d62b47204007ee4fab105d83e85e951862f0981aebc1b00d9" + "2838e766ef9b6bf2d037fe2e20b6a8464174e75a5f834da70569c018eb2b5693" + "babb7fbb" + "0a76c196067cfdcb11457d9cf45e2fa01d7f4275153924800600571fac3a5b26" + "3fdf57cd2c0064975c3747465cc36c270e8a35b10828d569c268a20eb78ac332"), + ("00009d23b4917fc09f20dbb0dcc93f0e66dfe717c17313394391b6e2e6eacb0f" + "0bb7be72bd6d25009aeb7fa0c4169b148d2f527e72daf0a54ef25c0707e33868" + "7d1f7157" + "5653a45c49390aa51cf5192bbf67da14be11d56ba0b4a2969d8055a9f03f2d71" + "581d8e830112ff0f0948eccaf8877acf26c377c13f719726fd70bddacb4deeec"), + + # Next 2 number generated by random.getrandbits(521) + ("12b84ae65e920a63ac1f2b64df6dff07870c9d531ae72a47403063238da1a1fe" + "3f9d6a179fa50f96cd4aff9261aa92c0e6f17ec940639bc2ccdf572df00790813e3"), + ("166049dd332a73fa0b26b75196cf87eb8a09b27ec714307c68c425424a1574f1" + "eedf5b0f16cdfdb839424d201e653f53d6883ca1c107ca6e706649889c0c7f38608") + ] + + @property + def arg_a(self) -> str: + return super().format_arg('{:x}'.format(self.int_a)).zfill(2 * self.hex_digits - 2 * self.bits_in_limb // 8) + + def result(self) -> List[str]: + result = self.int_a % self.int_n + return [self.format_result(result)] + + @property + def is_valid(self) -> bool: + return True diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 394253de53f2..d90d7cbef1b4 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -4,6 +4,7 @@ #include "mbedtls/ecdh.h" #include "ecp_invasive.h" +#include "bignum_mod_raw_invasive.h" #if defined(MBEDTLS_TEST_HOOKS) && \ (defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED) || \ @@ -1295,3 +1296,46 @@ exit: mbedtls_mpi_free(&expected_n); } /* END_CASE */ + + +/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS */ +void ecp_mod_p521_raw(char *input_N, + char *input_X, + char *result) +{ + mbedtls_mpi_uint *X = NULL; + mbedtls_mpi_uint *N = NULL; + mbedtls_mpi_uint *res = NULL; + size_t limbs_X; + size_t limbs_N; + size_t limbs_res; + + mbedtls_mpi_mod_modulus m; + mbedtls_mpi_mod_modulus_init(&m); + + TEST_EQUAL(mbedtls_test_read_mpi_core(&X, &limbs_X, input_X), 0); + TEST_EQUAL(mbedtls_test_read_mpi_core(&N, &limbs_N, input_N), 0); + TEST_EQUAL(mbedtls_test_read_mpi_core(&res, &limbs_res, result), 0); + + size_t limbs = limbs_N; + size_t bytes = limbs * sizeof(mbedtls_mpi_uint); + + TEST_EQUAL(limbs_X, 2 * limbs - 1); + TEST_EQUAL(limbs_res, limbs); + + TEST_EQUAL(mbedtls_mpi_mod_modulus_setup( + &m, N, limbs, + MBEDTLS_MPI_MOD_REP_MONTGOMERY), 0); + + TEST_EQUAL(ecp_mod_p521_raw(X, limbs_X), 0); + mbedtls_mpi_mod_raw_fix_quasi_reduction(X, &m); + ASSERT_COMPARE(X, bytes, res, bytes); + +exit: + mbedtls_free(X); + mbedtls_free(res); + + mbedtls_mpi_mod_modulus_free(&m); + mbedtls_free(N); +} +/* END_CASE */ From 14eb50263e851a0afbb6a9cb901ee6612f749de2 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 6 Feb 2023 16:02:05 +0100 Subject: [PATCH 06/13] Add documentation Signed-off-by: Gabor Mezei --- library/ecp_invasive.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/ecp_invasive.h b/library/ecp_invasive.h index 5b19d6eba1a1..efaf85fb2e6d 100644 --- a/library/ecp_invasive.h +++ b/library/ecp_invasive.h @@ -78,6 +78,12 @@ int mbedtls_ecp_gen_privkey_mx(size_t n_bits, #if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) +/** Fast quasi-reduction modulo p521 = 2^521 - 1 (FIPS 186-3 D.2.5) + * + * \param[in,out] N_p The address of the MPI to be converted. + * Must have 2 * N - 1 limbs, where N is the modulus. + * \param[in] N_n The length of \p N_p in limbs. + */ MBEDTLS_STATIC_TESTABLE int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n); From aa11ef4870ee09a7cdf295700f8d43bb82b280aa Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 6 Feb 2023 17:13:02 +0100 Subject: [PATCH 07/13] Rename function to follow naming convention Signed-off-by: Gabor Mezei --- library/ecp_curves.c | 6 +++--- library/ecp_invasive.h | 2 +- tests/suites/test_suite_ecp.function | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index a1f3ffecd610..45720ab30199 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -4583,7 +4583,7 @@ static int ecp_mod_p384(mbedtls_mpi *); #if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) static int ecp_mod_p521(mbedtls_mpi *); MBEDTLS_STATIC_TESTABLE -int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n); +int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n); #endif #define NIST_MODP(P) grp->modp = ecp_mod_ ## P; @@ -5178,13 +5178,13 @@ static int ecp_mod_p521(mbedtls_mpi *N) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t expected_width = 2 * ((521 + biL - 1) / biL); MBEDTLS_MPI_CHK(mbedtls_mpi_grow(N, expected_width)); - ret = ecp_mod_p521_raw(N->p, expected_width); + ret = mbedtls_ecp_mod_p521_raw(N->p, expected_width); cleanup: return ret; } MBEDTLS_STATIC_TESTABLE -int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) +int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) { mbedtls_mpi_uint carry = 0; diff --git a/library/ecp_invasive.h b/library/ecp_invasive.h index efaf85fb2e6d..e22a9de1e108 100644 --- a/library/ecp_invasive.h +++ b/library/ecp_invasive.h @@ -85,7 +85,7 @@ int mbedtls_ecp_gen_privkey_mx(size_t n_bits, * \param[in] N_n The length of \p N_p in limbs. */ MBEDTLS_STATIC_TESTABLE -int ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n); +int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n); #endif /* MBEDTLS_ECP_DP_SECP521R1_ENABLED */ diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index d90d7cbef1b4..90257a149b63 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1327,7 +1327,7 @@ void ecp_mod_p521_raw(char *input_N, &m, N, limbs, MBEDTLS_MPI_MOD_REP_MONTGOMERY), 0); - TEST_EQUAL(ecp_mod_p521_raw(X, limbs_X), 0); + TEST_EQUAL(mbedtls_ecp_mod_p521_raw(X, limbs_X), 0); mbedtls_mpi_mod_raw_fix_quasi_reduction(X, &m); ASSERT_COMPARE(X, bytes, res, bytes); From 0f83e15e670565147daa32fd1fac510759520e26 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 6 Feb 2023 18:03:39 +0100 Subject: [PATCH 08/13] Fix pylint issues Create a new function for calculating the number of hex digits needed for a certain amount of limbs. Signed-off-by: Gabor Mezei --- scripts/mbedtls_dev/bignum_common.py | 6 +++++- scripts/mbedtls_dev/ecp.py | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/mbedtls_dev/bignum_common.py b/scripts/mbedtls_dev/bignum_common.py index 24221755415f..5319ec68b285 100644 --- a/scripts/mbedtls_dev/bignum_common.py +++ b/scripts/mbedtls_dev/bignum_common.py @@ -74,6 +74,10 @@ def combination_pairs(values: List[T]) -> List[Tuple[T, T]]: """Return all pair combinations from input values.""" return [(x, y) for x in values for y in values] +def hex_digits_for_limb(limbs: int, bits_in_limb: int) -> int: + """ Retrun the hex digits need for a number of limbs. """ + return 2 * (limbs * bits_in_limb // 8) + class OperationCommon(test_data_generation.BaseTest): """Common features for bignum binary operations. @@ -138,7 +142,7 @@ def limbs(self) -> int: @property def hex_digits(self) -> int: - return 2 * (self.limbs * self.bits_in_limb // 8) + return hex_digits_for_limb(self.limbs, self.bits_in_limb) def format_arg(self, val: str) -> str: if self.input_style not in self.input_styles: diff --git a/scripts/mbedtls_dev/ecp.py b/scripts/mbedtls_dev/ecp.py index 09dcba281920..fbdea9958746 100644 --- a/scripts/mbedtls_dev/ecp.py +++ b/scripts/mbedtls_dev/ecp.py @@ -97,7 +97,9 @@ class EcpP521R1Raw(bignum_common.ModOperationCommon, @property def arg_a(self) -> str: - return super().format_arg('{:x}'.format(self.int_a)).zfill(2 * self.hex_digits - 2 * self.bits_in_limb // 8) + # Number of limbs: 2 * N - 1 + hex_digits = bignum_common.hex_digits_for_limb(2 * self.limbs - 1, self.bits_in_limb) + return super().format_arg('{:x}'.format(self.int_a)).zfill(hex_digits) def result(self) -> List[str]: result = self.int_a % self.int_n From 27977fc741ee9a1300d36b17c5c6bc110e631e1b Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 6 Feb 2023 18:06:54 +0100 Subject: [PATCH 09/13] Fix coding style issues Signed-off-by: Gabor Mezei --- library/ecp_curves.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 45720ab30199..5a59d386b4b5 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5197,7 +5197,7 @@ int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) /* Save and clear the A1 content of the shared limb to prevent it from overwrite. */ - mbedtls_mpi_uint remainder[P521_WIDTH] = {0}; + mbedtls_mpi_uint remainder[P521_WIDTH] = { 0 }; remainder[0] = N_p[P521_WIDTH - 1] >> 9; N_p[P521_WIDTH - 1] &= P521_MASK; @@ -5215,7 +5215,7 @@ int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) memset(NT_p, 0, sizeof(mbedtls_mpi_uint) * NT_n); } - (void)mbedtls_mpi_core_add(N_p, N_p, remainder, P521_WIDTH); + (void) mbedtls_mpi_core_add(N_p, N_p, remainder, P521_WIDTH); N_p[P521_WIDTH - 1] += carry; return 0; From c04f8ecb527d5ab71bf0de821c2b7a83a0c86edd Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 7 Feb 2023 15:24:57 +0000 Subject: [PATCH 10/13] Revert changes to mod_p521 flow It is not necessary to save the middle limb upfront as overwriting it is the desired result: in the first step we are reducing modulo 2^{512+biL}. Arguably, the original flow is more intuitive and easier to see the idea behind it. Signed-off-by: Janos Follath --- library/ecp_curves.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 5a59d386b4b5..a975f40c7a44 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5195,12 +5195,6 @@ int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) return 0; } - /* Save and clear the A1 content of the shared limb to prevent it - from overwrite. */ - mbedtls_mpi_uint remainder[P521_WIDTH] = { 0 }; - remainder[0] = N_p[P521_WIDTH - 1] >> 9; - N_p[P521_WIDTH - 1] &= P521_MASK; - if (N_n > P521_WIDTH) { /* Helper references for top part of N */ mbedtls_mpi_uint *NT_p = N_p + P521_WIDTH; @@ -5209,14 +5203,17 @@ int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) /* Split N as A0 + 2^(512 + biL) A1 and compute A0 + 2^(biL - 9) * A1. * This can be done in place. */ mbedtls_mpi_uint shift = ((mbedtls_mpi_uint) 1u) << (biL - 9); - carry = mbedtls_mpi_core_mla(N_p, P521_WIDTH - 1, NT_p, NT_n, shift); + carry = mbedtls_mpi_core_mla(N_p, P521_WIDTH, NT_p, NT_n, shift); /* Clear top part */ memset(NT_p, 0, sizeof(mbedtls_mpi_uint) * NT_n); } + mbedtls_mpi_uint remainder[P521_WIDTH] = { 0 }; + remainder[0] = carry << (biL - 9); + remainder[0] += (N_p[P521_WIDTH-1] >> 9); + N_p[P521_WIDTH-1] &= P521_MASK; (void) mbedtls_mpi_core_add(N_p, N_p, remainder, P521_WIDTH); - N_p[P521_WIDTH - 1] += carry; return 0; } From 1a2647d6e9a3d7e91f7e974eafb22bc33e5d481f Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 7 Feb 2023 15:27:44 +0000 Subject: [PATCH 11/13] Add corner case to mod_p521 tests Signed-off-by: Janos Follath --- scripts/mbedtls_dev/ecp.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/mbedtls_dev/ecp.py b/scripts/mbedtls_dev/ecp.py index fbdea9958746..632235ae5fed 100644 --- a/scripts/mbedtls_dev/ecp.py +++ b/scripts/mbedtls_dev/ecp.py @@ -39,6 +39,13 @@ class EcpP521R1Raw(bignum_common.ModOperationCommon, input_values = [ "0", "1", + # Corner case: maximum canonical P521 multiplication result + ("3fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "ff80000000000000000000000000000000000000000000000000000000000000" + "0000000000000000000000000000000000000000000000000000000000000000" + "00004"), + # Test case for overflow during addition ("0001efffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" From 1194ece7b1b89c913886d98627b28901715b41e4 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 7 Feb 2023 15:49:15 +0000 Subject: [PATCH 12/13] modp521: apply naming conventions Apply the usual parameter name and align the local variables and comments. This naming diverges from the standard notation, but this is beneficial as our variable meanings diverge as well and the difference can help avoiding confusion. Signed-off-by: Janos Follath --- library/ecp_curves.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index a975f40c7a44..58fed9a6ab11 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5184,36 +5184,39 @@ static int ecp_mod_p521(mbedtls_mpi *N) } MBEDTLS_STATIC_TESTABLE -int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *N_p, size_t N_n) +int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *X, size_t X_limbs) { mbedtls_mpi_uint carry = 0; - if (N_n > 2 * P521_WIDTH - 1) { - N_n = 2 * P521_WIDTH - 1; + if (X_limbs > 2 * P521_WIDTH - 1) { + X_limbs = 2 * P521_WIDTH - 1; } - if (N_n < P521_WIDTH) { + if (X_limbs < P521_WIDTH) { return 0; } - if (N_n > P521_WIDTH) { - /* Helper references for top part of N */ - mbedtls_mpi_uint *NT_p = N_p + P521_WIDTH; - size_t NT_n = N_n - P521_WIDTH; + if (X_limbs > P521_WIDTH) { + /* Helper references for bottom part of X */ + mbedtls_mpi_uint *X0 = X; + size_t X0_limbs = P521_WIDTH; + /* Helper references for top part of X */ + mbedtls_mpi_uint *X1 = X + X0_limbs; + size_t X1_limbs = X_limbs - X0_limbs; - /* Split N as A0 + 2^(512 + biL) A1 and compute A0 + 2^(biL - 9) * A1. + /* Split X as X0 + 2^(512 + biL) X1 and compute X0 + 2^(biL - 9) * X1. * This can be done in place. */ mbedtls_mpi_uint shift = ((mbedtls_mpi_uint) 1u) << (biL - 9); - carry = mbedtls_mpi_core_mla(N_p, P521_WIDTH, NT_p, NT_n, shift); + carry = mbedtls_mpi_core_mla(X0, X0_limbs, X1, X1_limbs, shift); /* Clear top part */ - memset(NT_p, 0, sizeof(mbedtls_mpi_uint) * NT_n); + memset(X1, 0, X1_limbs * sizeof(mbedtls_mpi_uint)); } - mbedtls_mpi_uint remainder[P521_WIDTH] = { 0 }; - remainder[0] = carry << (biL - 9); - remainder[0] += (N_p[P521_WIDTH-1] >> 9); - N_p[P521_WIDTH-1] &= P521_MASK; - (void) mbedtls_mpi_core_add(N_p, N_p, remainder, P521_WIDTH); + mbedtls_mpi_uint addend[P521_WIDTH] = { 0 }; + addend[0] = carry << (biL - 9); + addend[0] += (X[P521_WIDTH-1] >> 9); + X[P521_WIDTH-1] &= P521_MASK; + (void) mbedtls_mpi_core_add(X, X, addend, P521_WIDTH); return 0; } From fc9c0d7717819d5eb45966b3874ec93947af0c69 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 8 Feb 2023 10:14:21 +0000 Subject: [PATCH 13/13] mod_p521: document reduction algorithm Signed-off-by: Janos Follath --- library/ecp_curves.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index 58fed9a6ab11..fb0c35c19cba 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -5195,6 +5195,7 @@ int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *X, size_t X_limbs) return 0; } + /* Step 1: Reduction to P521_WIDTH limbs */ if (X_limbs > P521_WIDTH) { /* Helper references for bottom part of X */ mbedtls_mpi_uint *X0 = X; @@ -5203,20 +5204,43 @@ int mbedtls_ecp_mod_p521_raw(mbedtls_mpi_uint *X, size_t X_limbs) mbedtls_mpi_uint *X1 = X + X0_limbs; size_t X1_limbs = X_limbs - X0_limbs; - /* Split X as X0 + 2^(512 + biL) X1 and compute X0 + 2^(biL - 9) * X1. - * This can be done in place. */ + /* Split X as X0 + 2^P521_WIDTH X1 and compute X0 + 2^(biL - 9) X1. + * (We are using that 2^P521_WIDTH = 2^(512 + biL) and that + * 2^(512 + biL) X1 = 2^(biL - 9) X1 mod P521.) + * The high order limb of the result will be held in carry and the rest + * in X0 (that is the result will be represented as + * 2^P521_WIDTH carry + X0). + * + * Also, note that the resulting carry is either 0 or 1: + * X0 < 2^P521_WIDTH = 2^(512 + biL) and X1 < 2^(P521_WIDTH-biL) = 2^512 + * therefore + * X0 + 2^(biL - 9) X1 < 2^(512 + biL) + 2^(512 + biL - 9) + * which in turn is less than 2 * 2^(512 + biL). + */ mbedtls_mpi_uint shift = ((mbedtls_mpi_uint) 1u) << (biL - 9); carry = mbedtls_mpi_core_mla(X0, X0_limbs, X1, X1_limbs, shift); - /* Clear top part */ + /* Set X to X0 (by clearing the top part). */ memset(X1, 0, X1_limbs * sizeof(mbedtls_mpi_uint)); } - mbedtls_mpi_uint addend[P521_WIDTH] = { 0 }; - addend[0] = carry << (biL - 9); - addend[0] += (X[P521_WIDTH-1] >> 9); + /* Step 2: Reduction modulo P521 + * + * At this point X is reduced to P521_WIDTH limbs. What remains is to add + * the carry (that is 2^P521_WIDTH carry) and to reduce mod P521. */ + + /* 2^P521_WIDTH carry = 2^(512 + biL) carry = 2^(biL - 9) carry mod P521. + * Also, recall that carry is either 0 or 1. */ + mbedtls_mpi_uint addend = carry << (biL - 9); + /* Keep the top 9 bits and reduce the rest, using 2^521 = 1 mod P521. */ + addend += (X[P521_WIDTH-1] >> 9); X[P521_WIDTH-1] &= P521_MASK; - (void) mbedtls_mpi_core_add(X, X, addend, P521_WIDTH); + /* Declare a helper array for carrying out the addition. */ + mbedtls_mpi_uint addend_arr[P521_WIDTH] = { 0 }; + addend_arr[0] = addend; + (void) mbedtls_mpi_core_add(X, X, addend_arr, P521_WIDTH); + /* Both addends were less than P521 therefore X < 2 P521. (This also means + * that the result fit in P521_WIDTH limbs and there won't be any carry.) */ return 0; }