From 044a86bde8b0fd3b185253f52715655541dacc60 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Sun, 25 Oct 2015 10:58:03 +0100 Subject: [PATCH 1/4] Tests and fix added for #309 (inplace mpi doubling). --- library/bignum.c | 7 ++++++- tests/suites/test_suite_mpi.data | 6 ++++++ tests/suites/test_suite_mpi.function | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/library/bignum.c b/library/bignum.c index 628a6eedd..1b80200cb 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -862,7 +862,12 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi if( X == B ) { - const mbedtls_mpi *T = A; A = X; B = T; + const mbedtls_mpi *T; + + if( B == A) + return mbedtls_mpi_shift_l( X, 1 ); + + T = A; A = X; B = T; } if( X != A ) diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data index f838f3bda..2cfc212d2 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -295,6 +295,12 @@ mbedtls_mpi_add_mpi:10:"20395687835640197740576586692903457728019399331434826309 Test mbedtls_mpi_add_mpi #2 mbedtls_mpi_add_mpi:10:"643808006803554439230129854961492699151386107534013432918073439524138264842370630061369715394739134090922937332590384720397133335969549256322620979036686633213903952966175107096769180017646161851573147596390153":10:"56125680981752282333498088313568935051383833838594899821664631784577337171193624243181360054669678410455329112434552942717084003541384594864129940145043086760031292483340068923506115878221189886491132772739661669044958531131327771":10:"56125680981752282334141896320372489490613963693556392520816017892111350604111697682705498319512049040516698827829292076808006940873974979584527073481012636016353913462376755556720019831187364993587901952757307830896531678727717924" +Base test mbedtls_mpi_add_mpi inplace #1 +mbedtls_mpi_add_mpi_inplace:10:"12345678":10:"24691356" + +Test mbedtls_mpi_add_mpi inplace #2 +mbedtls_mpi_add_mpi_inplace:10:"643808006803554439230129854961492699151386107534013432918073439524138264842370630061369715394739134090922937332590384720397133335969549256322620979036686633213903952966175107096769180017646161851573147596390153":10:"1287616013607108878460259709922985398302772215068026865836146879048276529684741260122739430789478268181845874665180769440794266671939098512645241958073373266427807905932350214193538360035292323703146295192780306" + Test mbedtls_mpi_add_int #1 mbedtls_mpi_add_int:10:"2039568783564019774057658669290345772801939933143482630947726464532830627227012776329":9871232:10:"2039568783564019774057658669290345772801939933143482630947726464532830627227022647561" diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index 72b49408c..788893b35 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -442,6 +442,23 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mbedtls_mpi_add_mpi_inplace( int radix_X, char *input_X, int radix_A, char *input_A ) +{ + mbedtls_mpi X, A; + mbedtls_mpi_init( &X ); mbedtls_mpi_init( &A ); + + TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 ); + TEST_ASSERT( mbedtls_mpi_read_string( &A, radix_A, input_A ) == 0 ); + TEST_ASSERT( mbedtls_mpi_add_mpi( &X, &X, &X ) == 0 ); + TEST_ASSERT( mbedtls_mpi_cmp_mpi( &X, &A ) == 0 ); + +exit: + mbedtls_mpi_free( &X ); mbedtls_mpi_free( &A ); +} +/* END_CASE */ + + /* BEGIN_CASE */ void mbedtls_mpi_add_abs( int radix_X, char *input_X, int radix_Y, char *input_Y, int radix_A, char *input_A ) From 6cbacec3b339547df06b9f6c0ac5205a53c1f74a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Sun, 25 Oct 2015 12:29:13 +0100 Subject: [PATCH 2/4] Improved on the fix of #309 and extended the test to cover subroutines. --- library/bignum.c | 15 +++++++++++---- tests/suites/test_suite_mpi.function | 11 ++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 1b80200cb..7e35aa699 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -862,12 +862,19 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi if( X == B ) { - const mbedtls_mpi *T; + if( B == A ) + { + // Making a temporary copy instead of shifting by one to deny + // the possibility of corresponding side-channel attacks. + mbedtls_mpi TB; - if( B == A) - return mbedtls_mpi_shift_l( X, 1 ); + mbedtls_mpi_init( &TB ); + MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &TB, B ) ); + + return mbedtls_mpi_add_abs( X, A, &TB ); + } - T = A; A = X; B = T; + B = A; A = X; } if( X != A ) diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index 788893b35..2a709bc7b 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -448,8 +448,17 @@ void mbedtls_mpi_add_mpi_inplace( int radix_X, char *input_X, int radix_A, char mbedtls_mpi X, A; mbedtls_mpi_init( &X ); mbedtls_mpi_init( &A ); - TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 ); TEST_ASSERT( mbedtls_mpi_read_string( &A, radix_A, input_A ) == 0 ); + + TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 ); + TEST_ASSERT( mbedtls_mpi_sub_abs( &X, &X, &X ) == 0 ); + TEST_ASSERT( mbedtls_mpi_cmp_int( &X, 0 ) == 0 ); + + TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 ); + TEST_ASSERT( mbedtls_mpi_add_abs( &X, &X, &X ) == 0 ); + TEST_ASSERT( mbedtls_mpi_cmp_mpi( &X, &A ) == 0 ); + + TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 ); TEST_ASSERT( mbedtls_mpi_add_mpi( &X, &X, &X ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &X, &A ) == 0 ); From 3fc644f246ec88bcb2f1ace61206fd0199f4de3f Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Sun, 25 Oct 2015 14:24:10 +0100 Subject: [PATCH 3/4] Removed recursion from fix #309. --- library/bignum.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 5e2512343..9c38117b0 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -859,22 +859,21 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi int ret; size_t i, j; mbedtls_mpi_uint *o, *p, c; + mbedtls_mpi TB; if( X == B ) { + B = A; A = X; + if( B == A ) { // Making a temporary copy instead of shifting by one to deny // the possibility of corresponding side-channel attacks. - mbedtls_mpi TB; - mbedtls_mpi_init( &TB ); MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &TB, B ) ); - return mbedtls_mpi_add_abs( X, A, &TB ); + B = &TB; } - - B = A; A = X; } if( X != A ) @@ -911,6 +910,10 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi } cleanup: + if( &TB == B ) + { + mbedtls_mpi_free( &TB ); + } return( ret ); } From 6c9226809370f0fed4639ebc30766ef5a86987b4 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 30 Oct 2015 17:43:11 +0100 Subject: [PATCH 4/4] Improved on the previous fix and added a test case to cover both types of carries. --- library/bignum.c | 25 +++++++------------------ tests/suites/test_suite_mpi.data | 3 +++ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 9c38117b0..b587b6761 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -858,22 +858,11 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi { int ret; size_t i, j; - mbedtls_mpi_uint *o, *p, c; - mbedtls_mpi TB; + mbedtls_mpi_uint *o, *p, c, tmp; if( X == B ) { - B = A; A = X; - - if( B == A ) - { - // Making a temporary copy instead of shifting by one to deny - // the possibility of corresponding side-channel attacks. - mbedtls_mpi_init( &TB ); - MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &TB, B ) ); - - B = &TB; - } + const mbedtls_mpi *T = A; A = X; B = T; } if( X != A ) @@ -892,10 +881,14 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi o = B->p; p = X->p; c = 0; + /* + * tmp is used because it might happen that p == o + */ for( i = 0; i < j; i++, o++, p++ ) { + tmp= *o; *p += c; c = ( *p < c ); - *p += *o; c += ( *p < *o ); + *p += tmp; c += ( *p < tmp ); } while( c != 0 ) @@ -910,10 +903,6 @@ int mbedtls_mpi_add_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi } cleanup: - if( &TB == B ) - { - mbedtls_mpi_free( &TB ); - } return( ret ); } diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data index 2cfc212d2..3fd7f2d1b 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -301,6 +301,9 @@ mbedtls_mpi_add_mpi_inplace:10:"12345678":10:"24691356" Test mbedtls_mpi_add_mpi inplace #2 mbedtls_mpi_add_mpi_inplace:10:"643808006803554439230129854961492699151386107534013432918073439524138264842370630061369715394739134090922937332590384720397133335969549256322620979036686633213903952966175107096769180017646161851573147596390153":10:"1287616013607108878460259709922985398302772215068026865836146879048276529684741260122739430789478268181845874665180769440794266671939098512645241958073373266427807905932350214193538360035292323703146295192780306" +Test mbedtls_mpi_add_mpi inplace #3 +mbedtls_mpi_add_mpi_inplace:16:"ffffffffffffffffffffffffffffffff":16:"01fffffffffffffffffffffffffffffffe" + Test mbedtls_mpi_add_int #1 mbedtls_mpi_add_int:10:"2039568783564019774057658669290345772801939933143482630947726464532830627227012776329":9871232:10:"2039568783564019774057658669290345772801939933143482630947726464532830627227022647561"