From 7a9e43fd1d1d4732397c799664761febd8212184 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 26 Sep 2016 22:03:55 +0100 Subject: [PATCH 01/10] Actually apply debug_level settings in cert_app --- programs/x509/cert_app.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/programs/x509/cert_app.c b/programs/x509/cert_app.c index 51d71aef2..3d175281a 100644 --- a/programs/x509/cert_app.c +++ b/programs/x509/cert_app.c @@ -55,6 +55,7 @@ int main( void ) #include "polarssl/net.h" #include "polarssl/ssl.h" #include "polarssl/x509.h" +#include "polarssl/debug.h" #include #include @@ -375,6 +376,10 @@ int main( int argc, char *argv[] ) polarssl_printf( " ok\n" ); +#if defined(POLARSSL_DEBUG_C) + debug_set_threshold( opt.debug_level ); +#endif + /* * 2. Start the connection */ From 160830312f7fd911cf61ea7f41f4f4d6ec7b257a Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 26 Sep 2016 22:06:16 +0100 Subject: [PATCH 02/10] Update for ChangeLog for fixes for cert_app --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 10ea41de7..d2af47420 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,7 @@ Bugfix when GCM is used. #441 * Fix for key exchanges based on ECDH-RSA or ECDH-ECDSA which weren't enabled unless others were also present. Found by David Fernandez. #428 + * Fixed configuration of debug output in cert_app sample program. = mbed TLS 1.3.17 branch 2016-06-28 From 7de149372818ccb538406cc23a5bb281fadb9a90 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 10 Oct 2016 23:23:41 +0100 Subject: [PATCH 03/10] Add extra compilation conditions to X.509 samples The sample applications programs/pkey/cert_req.c and programs/pkey/cert_write.c use the library functions mbedtls_pk_write_csr_pem() and mbedtls_pk_write_crt_pem() respectively and programs/pkey/gen_key.c uses the library function mbedtls_pk_write_key_pem(). These are dependent on the configuration option POLARSSL_PEM_WRITE_C. If the option isn't defined the build breaks. This change adds the compilation condition POLARSSL_PEM_WRITE_C to these sample applications. --- programs/pkey/gen_key.c | 16 ++++++++++------ programs/x509/cert_req.c | 12 +++++++----- programs/x509/cert_write.c | 15 +++++++++------ 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/programs/pkey/gen_key.c b/programs/pkey/gen_key.c index fca35e55a..275e541c2 100644 --- a/programs/pkey/gen_key.c +++ b/programs/pkey/gen_key.c @@ -33,8 +33,9 @@ #define polarssl_printf printf #endif -#if defined(POLARSSL_PK_WRITE_C) && defined(POLARSSL_FS_IO) && \ - defined(POLARSSL_ENTROPY_C) && defined(POLARSSL_CTR_DRBG_C) +#if defined(POLARSSL_PK_WRITE_C) && defined(POLARSSL_PEM_WRITE_C) && \ + defined(POLARSSL_FS_IO) && defined(POLARSSL_ENTROPY_C) && \ + defined(POLARSSL_CTR_DRBG_C) #include "polarssl/error.h" #include "polarssl/pk.h" #include "polarssl/ecdsa.h" @@ -121,12 +122,14 @@ int dev_random_entropy_poll( void *data, unsigned char *output, USAGE_DEV_RANDOM \ "\n" -#if !defined(POLARSSL_PK_WRITE_C) || !defined(POLARSSL_FS_IO) || \ - !defined(POLARSSL_ENTROPY_C) || !defined(POLARSSL_CTR_DRBG_C) +#if !defined(POLARSSL_PK_WRITE_C) || !defined(POLARSSL_PEM_WRITE_C) || \ + !defined(POLARSSL_FS_IO) || !defined(POLARSSL_ENTROPY_C) || \ + !defined(POLARSSL_CTR_DRBG_C) int main( void ) { polarssl_printf( "POLARSSL_PK_WRITE_C and/or POLARSSL_FS_IO and/or " - "POLARSSL_ENTROPY_C and/or POLARSSL_CTR_DRBG_C " + "POLARSSL_ENTROPY_C and/or POLARSSL_CTR_DRBG_C and/or " + "POLARSSL_PEM_WRITE_C " "not defined.\n" ); return( 0 ); } @@ -417,4 +420,5 @@ exit: return( ret ); } -#endif /* POLARSSL_PK_WRITE_C && POLARSSL_FS_IO */ +#endif /* POLARSSL_PK_WRITE_C && POLARSSL_PEM_WRITE_C && POLARSSL_FS_IO && + * POLARSSL_ENTROPY_C && POLARSSL_CTR_DRBG_C */ diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 01b8107a1..0e8862f37 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -35,13 +35,15 @@ #if !defined(POLARSSL_X509_CSR_WRITE_C) || !defined(POLARSSL_FS_IO) || \ !defined(POLARSSL_PK_PARSE_C) || !defined(POLARSSL_SHA256_C) || \ - !defined(POLARSSL_ENTROPY_C) || !defined(POLARSSL_CTR_DRBG_C) + !defined(POLARSSL_ENTROPY_C) || !defined(POLARSSL_CTR_DRBG_C) || \ + !defined(POLARSSL_PEM_WRITE_C) int main( void ) { polarssl_printf( "POLARSSL_X509_CSR_WRITE_C and/or POLARSSL_FS_IO and/or " - "POLARSSL_PK_PARSE_C and/or POLARSSL_SHA256_c and/or " - "POLARSSL_ENTROPY_C and/or POLARSSL_CTR_DRBG_C " - "not defined.\n"); + "POLARSSL_PK_PARSE_C and/or POLARSSL_SHA256_c and/or " + "POLARSSL_ENTROPY_C and/or POLARSSL_CTR_DRBG_C and/or " + "POLARSSL_PEM_WRITE_C " + "not defined.\n"); return( 0 ); } #else @@ -341,4 +343,4 @@ exit: return( ret ); } #endif /* POLARSSL_X509_CSR_WRITE_C && POLARSSL_PK_PARSE_C && POLARSSL_FS_IO && - POLARSSL_ENTROPY_C && POLARSSL_CTR_DRBG_C */ + POLARSSL_ENTROPY_C && POLARSSL_CTR_DRBG_C && POLARSSL_PEM_WRITE_C */ diff --git a/programs/x509/cert_write.c b/programs/x509/cert_write.c index 57cb6c7c0..290eebcf2 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -36,13 +36,16 @@ #if !defined(POLARSSL_X509_CRT_WRITE_C) || \ !defined(POLARSSL_X509_CRT_PARSE_C) || !defined(POLARSSL_FS_IO) || \ !defined(POLARSSL_ENTROPY_C) || !defined(POLARSSL_CTR_DRBG_C) || \ - !defined(POLARSSL_ERROR_C) || !defined(POLARSSL_SHA256_C) + !defined(POLARSSL_ERROR_C) || !defined(POLARSSL_SHA256_C) || \ + !defined(POLARSSL_PEM_WRITE_C) int main( void ) { - polarssl_printf( "POLARSSL_X509_CRT_WRITE_C and/or POLARSSL_X509_CRT_PARSE_C and/or " - "POLARSSL_FS_IO and/or POLARSSL_SHA256_C and_or " - "POLARSSL_ENTROPY_C and/or POLARSSL_CTR_DRBG_C and/or " - "POLARSSL_ERROR_C not defined.\n"); + polarssl_printf( "POLARSSL_X509_CRT_WRITE_C and/or " + "POLARSSL_X509_CRT_PARSE_C and/or " + "POLARSSL_FS_IO and/or POLARSSL_SHA256_C and_or " + "POLARSSL_ENTROPY_C and/or POLARSSL_CTR_DRBG_C and/or " + "POLARSSL_PEM_WRITE_C and/or " + "POLARSSL_ERROR_C not defined.\n"); return( 0 ); } #else @@ -665,4 +668,4 @@ exit: } #endif /* POLARSSL_X509_CRT_WRITE_C && POLARSSL_X509_CRT_PARSE_C && POLARSSL_FS_IO && POLARSSL_ENTROPY_C && POLARSSL_CTR_DRBG_C && - POLARSSL_ERROR_C */ + POLARSSL_ERROR_C && MBEDTLS_PEM_WRITE_C */ From 79f2e87f0cdb0083ebd2865d0f8d166254f1b6a0 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 10 Oct 2016 23:48:11 +0100 Subject: [PATCH 04/10] Update Changelog for fix #559 --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index d2af47420..61603c7d5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,9 @@ Bugfix * Fix for key exchanges based on ECDH-RSA or ECDH-ECDSA which weren't enabled unless others were also present. Found by David Fernandez. #428 * Fixed configuration of debug output in cert_app sample program. + * Fixed the sample applications gen_key.c, cert_req.c and cert_write.c for + builds where the configuration POLARSSL_PEM_WRITE_C is not defined. Found + by inestlerode. #559. = mbed TLS 1.3.17 branch 2016-06-28 From 15fdb7f9ff8985f236f78dbfef3e6201cba295b7 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 26 Sep 2016 09:52:41 +0100 Subject: [PATCH 05/10] Fix 1 byte overread in mbedtls_asn1_get_int() --- ChangeLog | 5 ++++- library/asn1parse.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 61603c7d5..2609aeefd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,7 +10,10 @@ Bugfix when GCM is used. #441 * Fix for key exchanges based on ECDH-RSA or ECDH-ECDSA which weren't enabled unless others were also present. Found by David Fernandez. #428 - * Fixed configuration of debug output in cert_app sample program. + * Fixed cert_app sample program for debug output and for use when no root + certificates are provided. + * Fix conditional statement that would cause a 1 byte overread in + mbedtls_asn1_get_int(). Found and fixed by Guido Vranken. * Fixed the sample applications gen_key.c, cert_req.c and cert_write.c for builds where the configuration POLARSSL_PEM_WRITE_C is not defined. Found by inestlerode. #559. diff --git a/library/asn1parse.c b/library/asn1parse.c index e4f46eb0a..8c167df5f 100644 --- a/library/asn1parse.c +++ b/library/asn1parse.c @@ -154,7 +154,7 @@ int asn1_get_int( unsigned char **p, if( ( ret = asn1_get_tag( p, end, &len, ASN1_INTEGER ) ) != 0 ) return( ret ); - if( len > sizeof( int ) || ( **p & 0x80 ) != 0 ) + if( len == 0 || len > sizeof( int ) || ( **p & 0x80 ) != 0 ) return( POLARSSL_ERR_ASN1_INVALID_LENGTH ); *val = 0; From de4b7e8256006942e0722ea5bb174ed96b4400bb Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 12 Oct 2016 18:31:29 +0100 Subject: [PATCH 06/10] Updated Changelog for fix #599 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 2609aeefd..ed6f22189 100644 --- a/ChangeLog +++ b/ChangeLog @@ -13,7 +13,7 @@ Bugfix * Fixed cert_app sample program for debug output and for use when no root certificates are provided. * Fix conditional statement that would cause a 1 byte overread in - mbedtls_asn1_get_int(). Found and fixed by Guido Vranken. + mbedtls_asn1_get_int(). Found and fixed by Guido Vranken. #599 * Fixed the sample applications gen_key.c, cert_req.c and cert_write.c for builds where the configuration POLARSSL_PEM_WRITE_C is not defined. Found by inestlerode. #559. From cdbcd2012d14c14b92b800d4fe3f4c52333231e1 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 26 Sep 2016 10:09:30 +0100 Subject: [PATCH 07/10] Fix documentation for mbedtls_gcm_finish() Fix implementation and documentation missmatch for the function arguments to mbedtls_gcm_finish(). Also, removed redundant if condition that always evaluates to true. --- ChangeLog | 2 ++ include/polarssl/gcm.h | 4 ++-- library/gcm.c | 3 +-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index ed6f22189..d64ce68f7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Bugfix * Fixed the sample applications gen_key.c, cert_req.c and cert_write.c for builds where the configuration POLARSSL_PEM_WRITE_C is not defined. Found by inestlerode. #559. + * Fix documentation and implementation missmatch for function arguments of + mbedtls_gcm_finish(). Found by cmiatpaar. = mbed TLS 1.3.17 branch 2016-06-28 diff --git a/include/polarssl/gcm.h b/include/polarssl/gcm.h index 5a9472298..3326919ff 100644 --- a/include/polarssl/gcm.h +++ b/include/polarssl/gcm.h @@ -186,8 +186,8 @@ int gcm_update( gcm_context *ctx, * 16 bytes. * * \param ctx GCM context - * \param tag buffer for holding the tag (may be NULL if tag_len is 0) - * \param tag_len length of the tag to generate + * \param tag buffer for holding the tag + * \param tag_len length of the tag to generate (must be at least 4) * * \return 0 if successful or POLARSSL_ERR_GCM_BAD_INPUT */ diff --git a/library/gcm.c b/library/gcm.c index 83f2fd2bd..33306e554 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -410,8 +410,7 @@ int gcm_finish( gcm_context *ctx, if( tag_len > 16 || tag_len < 4 ) return( POLARSSL_ERR_GCM_BAD_INPUT ); - if( tag_len != 0 ) - memcpy( tag, ctx->base_ectr, tag_len ); + memcpy( tag, ctx->base_ectr, tag_len ); if( orig_len || orig_add_len ) { From 2c73577d4a6f6af50cbfc9582325f430ac093cf4 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 12 Oct 2016 19:47:29 +0100 Subject: [PATCH 08/10] Clarified Changelog for fix #602 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index d64ce68f7..49fdbccb7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -18,7 +18,7 @@ Bugfix builds where the configuration POLARSSL_PEM_WRITE_C is not defined. Found by inestlerode. #559. * Fix documentation and implementation missmatch for function arguments of - mbedtls_gcm_finish(). Found by cmiatpaar. + mbedtls_gcm_finish(). Found by cmiatpaar. #602 = mbed TLS 1.3.17 branch 2016-06-28 From 3072458ec31604b5986b6ff36b9628ebe247925a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 21 Sep 2016 13:18:12 +0100 Subject: [PATCH 09/10] 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 | 16 +++++++--------- tests/suites/test_suite_rsa.data | 2 +- tests/suites/test_suite_rsa.function | 7 +++++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 49fdbccb7..ecd92d569 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,6 +19,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 = mbed TLS 1.3.17 branch 2016-06-28 diff --git a/library/rsa.c b/library/rsa.c index 26d69c522..bf77cb5b9 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -97,6 +97,9 @@ int rsa_gen_key( rsa_context *ctx, if( f_rng == NULL || nbits < 128 || exponent < 3 ) return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); + if( nbits % 2 ) + return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); + mpi_init( &P1 ); mpi_init( &Q1 ); mpi_init( &H ); mpi_init( &G ); @@ -111,16 +114,8 @@ int rsa_gen_key( rsa_context *ctx, MPI_CHK( mpi_gen_prime( &ctx->P, nbits >> 1, 0, f_rng, p_rng ) ); - if( nbits % 2 ) - { - MPI_CHK( mpi_gen_prime( &ctx->Q, ( nbits >> 1 ) + 1, 0, + MPI_CHK( mpi_gen_prime( &ctx->Q, nbits >> 1, 0, f_rng, p_rng ) ); - } - else - { - MPI_CHK( mpi_gen_prime( &ctx->Q, nbits >> 1, 0, - f_rng, p_rng ) ); - } if( mpi_cmp_mpi( &ctx->P, &ctx->Q ) == 0 ) continue; @@ -129,6 +124,9 @@ int rsa_gen_key( rsa_context *ctx, if( mpi_msb( &ctx->N ) != nbits ) continue; + if( mpi_cmp_mpi( &ctx->P, &ctx->Q ) < 0 ) + mpi_swap( &ctx->P, &ctx->Q ); + MPI_CHK( mpi_sub_int( &P1, &ctx->P, 1 ) ); MPI_CHK( mpi_sub_int( &Q1, &ctx->Q, 1 ) ); MPI_CHK( mpi_mul_mpi( &H, &P1, &Q1 ) ); diff --git a/tests/suites/test_suite_rsa.data b/tests/suites/test_suite_rsa.data index 720a5dd01..e4bc89eec 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 rsa_gen_key:2048:3:0 RSA Generate Key - 1025 bit key -rsa_gen_key:1025:3:0 +rsa_gen_key:1025:3:POLARSSL_ERR_RSA_BAD_INPUT_DATA RSA PKCS1 Encrypt Bad RNG depends_on:POLARSSL_PKCS1_V15 diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index 45d572330..d4f330805 100644 --- a/tests/suites/test_suite_rsa.function +++ b/tests/suites/test_suite_rsa.function @@ -668,14 +668,17 @@ void rsa_gen_key( int nrbits, int exponent, int result) entropy_init( &entropy ); TEST_ASSERT( ctr_drbg_init( &ctr_drbg, entropy_func, &entropy, - (const unsigned char *) pers, strlen( pers ) ) == 0 ); + (const unsigned char *) pers, + strlen( pers ) ) == 0 ); rsa_init( &ctx, 0, 0 ); - TEST_ASSERT( rsa_gen_key( &ctx, ctr_drbg_random, &ctr_drbg, nrbits, exponent ) == result ); + TEST_ASSERT( rsa_gen_key( &ctx, ctr_drbg_random, &ctr_drbg, nrbits, + exponent ) == result ); if( result == 0 ) { TEST_ASSERT( rsa_check_privkey( &ctx ) == 0 ); + TEST_ASSERT( mpi_cmp_mpi( &ctx.P, &ctx.Q ) > 0 ); } exit: From c371c435c10113da31e70732021d9f11f8b4e312 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 13 Oct 2016 09:34:25 +0100 Subject: [PATCH 10/10] Added credit to Changelog for fix #558 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index ecd92d569..f53530d37 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,7 +19,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 = mbed TLS 1.3.17 branch 2016-06-28