From dd2e4683d0bdfada6a40e49154b3c02f67f62bfa Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 17 Oct 2022 10:16:56 +0100 Subject: [PATCH 1/6] Bignum Core: add limb size specific test generation In Bignum Core the result also involves a carry and both the result and the carry depend on the size of the limbs. Before this change both 32 and 64 bit specific result have been passed to the test functions. Moving this decision out of the tests makes the test functions easier to write and read and the test cases easier to read and debug. The change doesn't make writing the generator script any harder and might even make reading it easier. Signed-off-by: Janos Follath --- scripts/mbedtls_dev/bignum_core.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/scripts/mbedtls_dev/bignum_core.py b/scripts/mbedtls_dev/bignum_core.py index 3652ac20a..23db980e8 100644 --- a/scripts/mbedtls_dev/bignum_core.py +++ b/scripts/mbedtls_dev/bignum_core.py @@ -72,6 +72,24 @@ class BignumCoreOperation(bignum_common.OperationCommon, BignumCoreTarget, metac yield cls(a_value, b_value).create_test_case() +class BignumCoreOperationArchSplit(BignumCoreOperation): + #pylint: disable=abstract-method + """Common features for bignum core operations where the result depends on + the limb size.""" + + def __init__(self, val_a: str, val_b: str, bits_in_limb: int) -> None: + super().__init__(val_a, val_b) + self.bits_in_limb = bits_in_limb + if self.bits_in_limb == 32: + self.dependencies = ["MBEDTLS_HAVE_INT32"] + elif self.bits_in_limb == 64: + self.dependencies = ["MBEDTLS_HAVE_INT64"] + + @classmethod + def generate_function_tests(cls) -> Iterator[test_case.TestCase]: + for a_value, b_value in cls.get_value_pairs(): + yield cls(a_value, b_value, 32).create_test_case() + yield cls(a_value, b_value, 64).create_test_case() class BignumCoreAddIf(BignumCoreOperation): """Test cases for bignum core add if.""" From e153a715f0c6093dd0a8ee0c0d649a80970aa1b1 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 17 Oct 2022 10:25:29 +0100 Subject: [PATCH 2/6] mpi_core_add_if: simplify tests Use the new, limb size aware base class to generate tests for mpi_core_add_if(). Signed-off-by: Janos Follath --- scripts/mbedtls_dev/bignum_core.py | 18 ++++++-------- tests/suites/test_suite_bignum_core.function | 26 ++++++-------------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/scripts/mbedtls_dev/bignum_core.py b/scripts/mbedtls_dev/bignum_core.py index 23db980e8..0b6ee9717 100644 --- a/scripts/mbedtls_dev/bignum_core.py +++ b/scripts/mbedtls_dev/bignum_core.py @@ -91,7 +91,7 @@ class BignumCoreOperationArchSplit(BignumCoreOperation): yield cls(a_value, b_value, 32).create_test_case() yield cls(a_value, b_value, 64).create_test_case() -class BignumCoreAddIf(BignumCoreOperation): +class BignumCoreAddIf(BignumCoreOperationArchSplit): """Test cases for bignum core add if.""" count = 0 symbol = "+" @@ -99,17 +99,15 @@ class BignumCoreAddIf(BignumCoreOperation): test_name = "mbedtls_mpi_core_add_if" def result(self) -> List[str]: - tmp = self.int_a + self.int_b + result = self.int_a + self.int_b bound_val = max(self.int_a, self.int_b) - bound_4 = bignum_common.bound_mpi(bound_val, 32) - bound_8 = bignum_common.bound_mpi(bound_val, 64) - carry_4, remainder_4 = divmod(tmp, bound_4) - carry_8, remainder_8 = divmod(tmp, bound_8) + + bound = bignum_common.bound_mpi(bound_val, self.bits_in_limb) + carry, result = divmod(result, bound) + return [ - "\"{:x}\"".format(remainder_4), - str(carry_4), - "\"{:x}\"".format(remainder_8), - str(carry_8) + "\"{:x}\"".format(result), + str(carry) ] diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index de8b7f194..5bc955ca7 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -340,10 +340,9 @@ exit: /* BEGIN_CASE */ void mpi_core_add_if( char * input_A, char * input_B, - char * input_S4, int carry4, - char * input_S8, int carry8 ) + char * input_S, int carry ) { - mbedtls_mpi S4, S8, A, B; + mbedtls_mpi S, A, B; mbedtls_mpi_uint *a = NULL; /* first value to add */ mbedtls_mpi_uint *b = NULL; /* second value to add */ mbedtls_mpi_uint *sum = NULL; @@ -351,28 +350,20 @@ void mpi_core_add_if( char * input_A, char * input_B, mbedtls_mpi_init( &A ); mbedtls_mpi_init( &B ); - mbedtls_mpi_init( &S4 ); - mbedtls_mpi_init( &S8 ); + mbedtls_mpi_init( &S ); TEST_EQUAL( 0, mbedtls_test_read_mpi( &A, input_A ) ); TEST_EQUAL( 0, mbedtls_test_read_mpi( &B, input_B ) ); - TEST_EQUAL( 0, mbedtls_test_read_mpi( &S4, input_S4 ) ); - TEST_EQUAL( 0, mbedtls_test_read_mpi( &S8, input_S8 ) ); - - /* We only need to work with one of (S4, carry4) or (S8, carry8) depending - * on sizeof(mbedtls_mpi_uint) - */ - mbedtls_mpi *X = ( sizeof(mbedtls_mpi_uint) == 4 ) ? &S4 : &S8; - mbedtls_mpi_uint carry = ( sizeof(mbedtls_mpi_uint) == 4 ) ? carry4 : carry8; + TEST_EQUAL( 0, mbedtls_test_read_mpi( &S, input_S ) ); /* All of the inputs are +ve (or zero) */ TEST_EQUAL( 1, A.s ); TEST_EQUAL( 1, B.s ); - TEST_EQUAL( 1, X->s ); + TEST_EQUAL( 1, S.s ); /* Test cases are such that A <= B, so #limbs should be <= */ TEST_LE_U( A.n, B.n ); - TEST_LE_U( X->n, B.n ); + TEST_LE_U( S.n, B.n ); /* Now let's get arrays of mbedtls_mpi_uints, rather than MPI structures */ @@ -400,7 +391,7 @@ void mpi_core_add_if( char * input_A, char * input_B, */ memcpy( a, A.p, A.n * sizeof(mbedtls_mpi_uint) ); memcpy( b, B.p, B.n * sizeof(mbedtls_mpi_uint) ); - memcpy( sum, X->p, X->n * sizeof(mbedtls_mpi_uint) ); + memcpy( sum, S.p, S.n * sizeof(mbedtls_mpi_uint) ); /* The test cases have a <= b to avoid repetition, so we test a + b then, * if a != b, b + a. If a == b, we can test when a and b are aliased */ @@ -448,8 +439,7 @@ exit: mbedtls_free( sum ); mbedtls_free( d ); - mbedtls_mpi_free( &S4 ); - mbedtls_mpi_free( &S8 ); + mbedtls_mpi_free( &S ); mbedtls_mpi_free( &A ); mbedtls_mpi_free( &B ); } From 5ff03d49c0be5d1dea0e13cb803016fa210faf17 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 17 Oct 2022 11:21:22 +0100 Subject: [PATCH 3/6] Bignum Core test: move bound to constructor We will need it to pad parameters in the base class, but it is useful because every child class would need to calculate it anyway. Signed-off-by: Janos Follath --- scripts/mbedtls_dev/bignum_core.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/mbedtls_dev/bignum_core.py b/scripts/mbedtls_dev/bignum_core.py index 0b6ee9717..8327df1b8 100644 --- a/scripts/mbedtls_dev/bignum_core.py +++ b/scripts/mbedtls_dev/bignum_core.py @@ -79,11 +79,13 @@ class BignumCoreOperationArchSplit(BignumCoreOperation): def __init__(self, val_a: str, val_b: str, bits_in_limb: int) -> None: super().__init__(val_a, val_b) + bound_val = max(self.int_a, self.int_b) self.bits_in_limb = bits_in_limb + self.bound = bignum_common.bound_mpi(bound_val, self.bits_in_limb) if self.bits_in_limb == 32: self.dependencies = ["MBEDTLS_HAVE_INT32"] elif self.bits_in_limb == 64: - self.dependencies = ["MBEDTLS_HAVE_INT64"] + self.dependencies = ["MBDTLS_HAVE_INT64"] @classmethod def generate_function_tests(cls) -> Iterator[test_case.TestCase]: @@ -100,10 +102,8 @@ class BignumCoreAddIf(BignumCoreOperationArchSplit): def result(self) -> List[str]: result = self.int_a + self.int_b - bound_val = max(self.int_a, self.int_b) - bound = bignum_common.bound_mpi(bound_val, self.bits_in_limb) - carry, result = divmod(result, bound) + carry, result = divmod(result, self.bound) return [ "\"{:x}\"".format(result), From ba516f752436dbe23c62ced96f526f04d62faa97 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 17 Oct 2022 13:47:13 +0100 Subject: [PATCH 4/6] mpi_core_add_if test: Remove dependency on old API Signed-off-by: Janos Follath --- scripts/mbedtls_dev/bignum_core.py | 19 ++- tests/suites/test_suite_bignum_core.function | 123 +++++++------------ 2 files changed, 59 insertions(+), 83 deletions(-) diff --git a/scripts/mbedtls_dev/bignum_core.py b/scripts/mbedtls_dev/bignum_core.py index 8327df1b8..ab582d3a7 100644 --- a/scripts/mbedtls_dev/bignum_core.py +++ b/scripts/mbedtls_dev/bignum_core.py @@ -61,8 +61,8 @@ class BignumCoreOperation(bignum_common.OperationCommon, BignumCoreTarget, metac generated to provide some context to the test case. """ if not self.case_description: - self.case_description = "{} {} {}".format( - self.arg_a, self.symbol, self.arg_b + self.case_description = "{:x} {} {:x}".format( + self.int_a, self.symbol, self.int_b ) return super().description() @@ -82,10 +82,20 @@ class BignumCoreOperationArchSplit(BignumCoreOperation): bound_val = max(self.int_a, self.int_b) self.bits_in_limb = bits_in_limb self.bound = bignum_common.bound_mpi(bound_val, self.bits_in_limb) + limbs = bignum_common.limbs_mpi(bound_val, self.bits_in_limb) + byte_len = limbs*self.bits_in_limb//8 + self.hex_digits = 2*byte_len if self.bits_in_limb == 32: self.dependencies = ["MBEDTLS_HAVE_INT32"] elif self.bits_in_limb == 64: - self.dependencies = ["MBDTLS_HAVE_INT64"] + self.dependencies = ["MBEDTLS_HAVE_INT64"] + else: + raise ValueError("Invalid number of bits in limb!") + self.arg_a = self.arg_a.zfill(self.hex_digits) + self.arg_b = self.arg_b.zfill(self.hex_digits) + + def pad_to_limbs(self, val) -> str: + return "{:x}".format(val).zfill(self.hex_digits) @classmethod def generate_function_tests(cls) -> Iterator[test_case.TestCase]: @@ -106,11 +116,10 @@ class BignumCoreAddIf(BignumCoreOperationArchSplit): carry, result = divmod(result, self.bound) return [ - "\"{:x}\"".format(result), + bignum_common.quote_str(self.pad_to_limbs(result)), str(carry) ] - class BignumCoreSub(BignumCoreOperation): """Test cases for bignum core sub.""" count = 0 diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 5bc955ca7..ab4f5b564 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -342,106 +342,73 @@ exit: void mpi_core_add_if( char * input_A, char * input_B, char * input_S, int carry ) { - mbedtls_mpi S, A, B; - mbedtls_mpi_uint *a = NULL; /* first value to add */ - mbedtls_mpi_uint *b = NULL; /* second value to add */ - mbedtls_mpi_uint *sum = NULL; - mbedtls_mpi_uint *d = NULL; /* destination - the in/out first operand */ + mbedtls_mpi_uint *A = NULL; /* first value to add */ + size_t A_limbs; + mbedtls_mpi_uint *B = NULL; /* second value to add */ + size_t B_limbs; + mbedtls_mpi_uint *S = NULL; /* expected result */ + size_t S_limbs; + mbedtls_mpi_uint *X = NULL; /* destination - the in/out first operand */ + size_t X_limbs; - mbedtls_mpi_init( &A ); - mbedtls_mpi_init( &B ); - mbedtls_mpi_init( &S ); + TEST_EQUAL( 0, mbedtls_test_read_mpi_core( &A, &A_limbs, input_A ) ); + TEST_EQUAL( 0, mbedtls_test_read_mpi_core( &B, &B_limbs, input_B ) ); + TEST_EQUAL( 0, mbedtls_test_read_mpi_core( &S, &S_limbs, input_S ) ); + X_limbs = S_limbs; + ASSERT_ALLOC( X, X_limbs ); - TEST_EQUAL( 0, mbedtls_test_read_mpi( &A, input_A ) ); - TEST_EQUAL( 0, mbedtls_test_read_mpi( &B, input_B ) ); - TEST_EQUAL( 0, mbedtls_test_read_mpi( &S, input_S ) ); + /* add_if expects all operands to be the same length */ + TEST_EQUAL( A_limbs, B_limbs ); + TEST_EQUAL( A_limbs, S_limbs ); + size_t limbs = A_limbs; + size_t bytes = limbs * sizeof( *A ); - /* All of the inputs are +ve (or zero) */ - TEST_EQUAL( 1, A.s ); - TEST_EQUAL( 1, B.s ); - TEST_EQUAL( 1, S.s ); + /* The test cases have A <= B to avoid repetition, so we test A + B then, + * if A != B, B + A. If A == B, we can test when A and B are aliased */ - /* Test cases are such that A <= B, so #limbs should be <= */ - TEST_LE_U( A.n, B.n ); - TEST_LE_U( S.n, B.n ); + /* A + B */ - /* Now let's get arrays of mbedtls_mpi_uints, rather than MPI structures */ - - /* mbedtls_mpi_core_add_if() uses input arrays of mbedtls_mpi_uints which - * must be the same size. The MPIs we've read in will only have arrays - * large enough for the number they represent. Therefore we create new - * raw arrays of mbedtls_mpi_uints and populate them from the MPIs we've - * just read in. - * - * We generated test data such that B was always >= A, so that's how many - * limbs each of these need. - */ - size_t limbs = B.n; - size_t bytes = limbs * sizeof(mbedtls_mpi_uint); - - /* ASSERT_ALLOC() uses calloc() under the hood, so these do get zeroed */ - ASSERT_ALLOC( a, bytes ); - ASSERT_ALLOC( b, bytes ); - ASSERT_ALLOC( sum, bytes ); - ASSERT_ALLOC( d, bytes ); - - /* Populate the arrays. As the mbedtls_mpi_uint[]s in mbedtls_mpis (and as - * processed by mbedtls_mpi_core_add_if()) are little endian, we can just - * copy what we have as long as MSBs are 0 (which they are from ASSERT_ALLOC()) - */ - memcpy( a, A.p, A.n * sizeof(mbedtls_mpi_uint) ); - memcpy( b, B.p, B.n * sizeof(mbedtls_mpi_uint) ); - memcpy( sum, S.p, S.n * sizeof(mbedtls_mpi_uint) ); - - /* The test cases have a <= b to avoid repetition, so we test a + b then, - * if a != b, b + a. If a == b, we can test when a and b are aliased */ - - /* a + b */ - - /* cond = 0 => d unchanged, no carry */ - memcpy( d, a, bytes ); - TEST_EQUAL( 0, mbedtls_mpi_core_add_if( d, b, limbs, 0 ) ); - ASSERT_COMPARE( d, bytes, a, bytes ); + /* cond = 0 => X unchanged, no carry */ + memcpy( X, A, bytes ); + TEST_EQUAL( 0, mbedtls_mpi_core_add_if( X, B, limbs, 0 ) ); + ASSERT_COMPARE( X, bytes, A, bytes ); /* cond = 1 => correct result and carry */ - TEST_EQUAL( carry, mbedtls_mpi_core_add_if( d, b, limbs, 1 ) ); - ASSERT_COMPARE( d, bytes, sum, bytes ); + TEST_EQUAL( carry, mbedtls_mpi_core_add_if( X, B, limbs, 1 ) ); + ASSERT_COMPARE( X, bytes, S, bytes ); - if ( A.n == B.n && memcmp( A.p, B.p, bytes ) == 0 ) + if ( memcmp( A, B, bytes ) == 0 ) { - /* a == b, so test where a and b are aliased */ + /* A == B, so test where A and B are aliased */ - /* cond = 0 => d unchanged, no carry */ - TEST_EQUAL( 0, mbedtls_mpi_core_add_if( b, b, limbs, 0 ) ); - ASSERT_COMPARE( b, bytes, B.p, bytes ); + /* cond = 0 => X unchanged, no carry */ + memcpy( X, B, bytes ); + TEST_EQUAL( 0, mbedtls_mpi_core_add_if( X, B, limbs, 0 ) ); + ASSERT_COMPARE( X, bytes, B, bytes ); /* cond = 1 => correct result and carry */ - TEST_EQUAL( carry, mbedtls_mpi_core_add_if( b, b, limbs, 1 ) ); - ASSERT_COMPARE( b, bytes, sum, bytes ); + TEST_EQUAL( carry, mbedtls_mpi_core_add_if( X, X, limbs, 1 ) ); + ASSERT_COMPARE( X, bytes, S, bytes ); } else { - /* a != b, so test b + a */ + /* A != B, so test B + A */ /* cond = 0 => d unchanged, no carry */ - memcpy( d, b, bytes ); - TEST_EQUAL( 0, mbedtls_mpi_core_add_if( d, a, limbs, 0 ) ); - ASSERT_COMPARE( d, bytes, b, bytes ); + memcpy( X, B, bytes ); + TEST_EQUAL( 0, mbedtls_mpi_core_add_if( X, A, limbs, 0 ) ); + ASSERT_COMPARE( X, bytes, B, bytes ); /* cond = 1 => correct result and carry */ - TEST_EQUAL( carry, mbedtls_mpi_core_add_if( d, a, limbs, 1 ) ); - ASSERT_COMPARE( d, bytes, sum, bytes ); + TEST_EQUAL( carry, mbedtls_mpi_core_add_if( X, A, limbs, 1 ) ); + ASSERT_COMPARE( X, bytes, S, bytes ); } exit: - mbedtls_free( a ); - mbedtls_free( b ); - mbedtls_free( sum ); - mbedtls_free( d ); - - mbedtls_mpi_free( &S ); - mbedtls_mpi_free( &A ); - mbedtls_mpi_free( &B ); + mbedtls_free( A ); + mbedtls_free( B ); + mbedtls_free( S ); + mbedtls_free( X ); } /* END_CASE */ From 560805d665ac8de7f016e5fa1af96a5646deaf23 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 20 Oct 2022 12:04:40 +0100 Subject: [PATCH 5/6] Fix mbedtls_mpi_core_add_if test aliasing Signed-off-by: Janos Follath --- tests/suites/test_suite_bignum_core.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index ab4f5b564..9803587bc 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -383,7 +383,7 @@ void mpi_core_add_if( char * input_A, char * input_B, /* cond = 0 => X unchanged, no carry */ memcpy( X, B, bytes ); - TEST_EQUAL( 0, mbedtls_mpi_core_add_if( X, B, limbs, 0 ) ); + TEST_EQUAL( 0, mbedtls_mpi_core_add_if( X, X, limbs, 0 ) ); ASSERT_COMPARE( X, bytes, B, bytes ); /* cond = 1 => correct result and carry */ From 78e3c9b5747b49e6821172927f6479a23d28278b Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 20 Oct 2022 12:09:30 +0100 Subject: [PATCH 6/6] Fix style in bignum_core.py Signed-off-by: Janos Follath --- scripts/mbedtls_dev/bignum_core.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/mbedtls_dev/bignum_core.py b/scripts/mbedtls_dev/bignum_core.py index ab582d3a7..2e641953d 100644 --- a/scripts/mbedtls_dev/bignum_core.py +++ b/scripts/mbedtls_dev/bignum_core.py @@ -82,9 +82,9 @@ class BignumCoreOperationArchSplit(BignumCoreOperation): bound_val = max(self.int_a, self.int_b) self.bits_in_limb = bits_in_limb self.bound = bignum_common.bound_mpi(bound_val, self.bits_in_limb) - limbs = bignum_common.limbs_mpi(bound_val, self.bits_in_limb) - byte_len = limbs*self.bits_in_limb//8 - self.hex_digits = 2*byte_len + limbs = bignum_common.limbs_mpi(bound_val, self.bits_in_limb) + byte_len = limbs * self.bits_in_limb // 8 + self.hex_digits = 2 * byte_len if self.bits_in_limb == 32: self.dependencies = ["MBEDTLS_HAVE_INT32"] elif self.bits_in_limb == 64: