From ef441784749d14ea9f8afad3f258489d2445c438 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 21 Sep 2016 13:18:12 +0100 Subject: [PATCH 1/5] Restore P>Q in RSA key generation (#558) The PKCS#1 standard says nothing about the relation between P and Q but many libraries guarantee P>Q and mbed TLS did so too in earlier versions. This commit restores this behaviour. --- ChangeLog | 1 + library/rsa.c | 18 ++++++++---------- tests/suites/test_suite_rsa.data | 2 +- tests/suites/test_suite_rsa.function | 1 + 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2e89c6afe..ab626b96a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -40,6 +40,7 @@ Bugfix by inestlerode. #559. * Fix documentation and implementation missmatch for function arguments of mbedtls_gcm_finish(). Found by cmiatpaar. #602 + * Guarantee that P>Q at RSA key generation. #558 Changes * Extended test coverage of special cases, and added new timing test suite. diff --git a/library/rsa.c b/library/rsa.c index 7a33689b2..40ef2a948 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -102,7 +102,10 @@ int mbedtls_rsa_gen_key( mbedtls_rsa_context *ctx, if( f_rng == NULL || nbits < 128 || exponent < 3 ) return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); - mbedtls_mpi_init( &P1 ); mbedtls_mpi_init( &Q1 ); + if( nbits % 2 ) + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + + mbedtls_mpi_init( &P1 ); mbedtls_mpi_init( &Q1 ); mbedtls_mpi_init( &H ); mbedtls_mpi_init( &G ); /* @@ -116,16 +119,8 @@ int mbedtls_rsa_gen_key( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->P, nbits >> 1, 0, f_rng, p_rng ) ); - if( nbits % 2 ) - { - MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->Q, ( nbits >> 1 ) + 1, 0, + MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->Q, nbits >> 1, 0, f_rng, p_rng ) ); - } - else - { - MBEDTLS_MPI_CHK( mbedtls_mpi_gen_prime( &ctx->Q, nbits >> 1, 0, - f_rng, p_rng ) ); - } if( mbedtls_mpi_cmp_mpi( &ctx->P, &ctx->Q ) == 0 ) continue; @@ -134,6 +129,9 @@ int mbedtls_rsa_gen_key( mbedtls_rsa_context *ctx, if( mbedtls_mpi_bitlen( &ctx->N ) != nbits ) continue; + if( mbedtls_mpi_cmp_mpi( &ctx->P, &ctx->Q ) < 0 ) + mbedtls_mpi_swap( &ctx->P, &ctx->Q ); + MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &P1, &ctx->P, 1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &Q1, &ctx->Q, 1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &H, &P1, &Q1 ) ); diff --git a/tests/suites/test_suite_rsa.data b/tests/suites/test_suite_rsa.data index d522332a2..af168805f 100644 --- a/tests/suites/test_suite_rsa.data +++ b/tests/suites/test_suite_rsa.data @@ -361,7 +361,7 @@ RSA Generate Key - 2048 bit key mbedtls_rsa_gen_key:2048:3:0 RSA Generate Key - 1025 bit key -mbedtls_rsa_gen_key:1025:3:0 +mbedtls_rsa_gen_key:1025:3:MBEDTLS_ERR_RSA_BAD_INPUT_DATA RSA PKCS1 Encrypt Bad RNG depends_on:MBEDTLS_PKCS1_V15 diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index 8837e3a83..d48bc8595 100644 --- a/tests/suites/test_suite_rsa.function +++ b/tests/suites/test_suite_rsa.function @@ -678,6 +678,7 @@ void mbedtls_rsa_gen_key( int nrbits, int exponent, int result) if( result == 0 ) { TEST_ASSERT( mbedtls_rsa_check_privkey( &ctx ) == 0 ); + TEST_ASSERT( mbedtls_mpi_cmp_mpi( &ctx.P, &ctx.Q ) > 0 ); } exit: From 4d69ecd9cb57cd77aee9519454618c98e4373b30 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 13 Oct 2016 00:14:37 +0100 Subject: [PATCH 2/5] Added credit to Changelog for fix #558 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index ab626b96a..b011ee42a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -40,7 +40,7 @@ Bugfix by inestlerode. #559. * Fix documentation and implementation missmatch for function arguments of mbedtls_gcm_finish(). Found by cmiatpaar. #602 - * Guarantee that P>Q at RSA key generation. #558 + * Guarantee that P>Q at RSA key generation. Found by inestlerode. #558 Changes * Extended test coverage of special cases, and added new timing test suite. From e019296ab7d0206dc6bc137a398b3f7f5c15f733 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 12 Oct 2016 23:07:30 +0100 Subject: [PATCH 3/5] Fix stdio redirection memory leak in test suites --- tests/suites/helpers.function | 5 +++++ tests/suites/main_test.function | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 5938447af..ff3ab99e4 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -140,6 +140,11 @@ static int restore_output( FILE** out_stream, int old_fd ) return 0; } + +static void close_output( FILE* stdout ) +{ + fclose( stdout ); +} #endif /* __unix__ || __APPLE__ __MACH__ */ static int unhexify( unsigned char *obuf, const char *ibuf ) diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 14209a576..afff5a482 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -261,7 +261,7 @@ int main(int argc, const char *argv[]) char buf[5000]; char *params[50]; void *pointer; - int stdout_fd = 0; + int stdout_fd = -1; #if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) && \ !defined(TEST_SUITE_MEMORY_BUFFER_ALLOC) @@ -499,6 +499,11 @@ int main(int argc, const char *argv[]) mbedtls_memory_buffer_alloc_free(); #endif +#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) + if( stdout_fd != -1 ) + close_output( stdout ); +#endif /* __unix__ || __APPLE__ __MACH__ */ + return( total_errors != 0 ); } From e709f7c9e07b54542f368297272b0fca36304300 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 13 Oct 2016 11:26:29 +0100 Subject: [PATCH 4/5] Fix global variable shadowing --- tests/suites/helpers.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index ff3ab99e4..63815df85 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -141,9 +141,9 @@ static int restore_output( FILE** out_stream, int old_fd ) return 0; } -static void close_output( FILE* stdout ) +static void close_output( FILE* out_stream ) { - fclose( stdout ); + fclose( out_stream ); } #endif /* __unix__ || __APPLE__ __MACH__ */ From bb83b42700cb74091ffc4c98b83b8889694616a6 Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Wed, 12 Oct 2016 17:36:50 -0500 Subject: [PATCH 5/5] Use allocated memory for SHA self tests Reduce the stack usage of the testing framework by dynamically allocating the memory used for the test. --- library/sha256.c | 15 ++++++++++++++- library/sha512.c | 15 ++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/library/sha256.c b/library/sha256.c index 4e82c0b79..ad25d3833 100644 --- a/library/sha256.c +++ b/library/sha256.c @@ -41,7 +41,10 @@ #include "mbedtls/platform.h" #else #include +#include #define mbedtls_printf printf +#define mbedtls_calloc calloc +#define mbedtls_free free #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ @@ -389,10 +392,19 @@ static const unsigned char sha256_test_sum[6][32] = int mbedtls_sha256_self_test( int verbose ) { int i, j, k, buflen, ret = 0; - unsigned char buf[1024]; + unsigned char *buf; unsigned char sha256sum[32]; mbedtls_sha256_context ctx; + buf = mbedtls_calloc( 1024, sizeof(unsigned char) ); + if( NULL == buf ) + { + if( verbose != 0 ) + mbedtls_printf( "Buffer allocation failed\n" ); + + return( 1 ); + } + mbedtls_sha256_init( &ctx ); for( i = 0; i < 6; i++ ) @@ -436,6 +448,7 @@ int mbedtls_sha256_self_test( int verbose ) exit: mbedtls_sha256_free( &ctx ); + mbedtls_free( buf ); return( ret ); } diff --git a/library/sha512.c b/library/sha512.c index 0f9e1e535..724522ac6 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -47,7 +47,10 @@ #include "mbedtls/platform.h" #else #include +#include #define mbedtls_printf printf +#define mbedtls_calloc calloc +#define mbedtls_free free #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ @@ -445,10 +448,19 @@ static const unsigned char sha512_test_sum[6][64] = int mbedtls_sha512_self_test( int verbose ) { int i, j, k, buflen, ret = 0; - unsigned char buf[1024]; + unsigned char *buf; unsigned char sha512sum[64]; mbedtls_sha512_context ctx; + buf = mbedtls_calloc( 1024, sizeof(unsigned char) ); + if( NULL == buf ) + { + if( verbose != 0 ) + mbedtls_printf( "Buffer allocation failed\n" ); + + return( 1 ); + } + mbedtls_sha512_init( &ctx ); for( i = 0; i < 6; i++ ) @@ -492,6 +504,7 @@ int mbedtls_sha512_self_test( int verbose ) exit: mbedtls_sha512_free( &ctx ); + mbedtls_free( buf ); return( ret ); }