From 8d36696e1fdb5228938a6cfe9c60ad3772a769dc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 25 Feb 2020 19:51:07 +0100 Subject: [PATCH 1/4] Fix fuzz_pubkey failure on valid RSA keys On a valid RSA public key, mbedtls_rsa_export should succeed if you ask for the public fields, but fail if you ask for private fields. The code was expecting to succeed when asking for private fields, so failed on every valid RSA public key. --- programs/fuzz/fuzz_pubkey.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/programs/fuzz/fuzz_pubkey.c b/programs/fuzz/fuzz_pubkey.c index 38eacfb61..c0571c47d 100644 --- a/programs/fuzz/fuzz_pubkey.c +++ b/programs/fuzz/fuzz_pubkey.c @@ -21,7 +21,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { mbedtls_mpi_init( &DQ ); mbedtls_mpi_init( &QP ); rsa = mbedtls_pk_rsa( pk ); - if ( mbedtls_rsa_export( rsa, &N, &P, &Q, &D, &E ) != 0 ) { + if ( mbedtls_rsa_export( rsa, &N, NULL, NULL, NULL, &E ) != 0 ) { + abort(); + } + if ( mbedtls_rsa_export( rsa, &N, &P, &Q, &D, &E ) != MBEDTLS_ERR_RSA_BAD_INPUT_DATA ) { abort(); } if ( mbedtls_rsa_export_crt( rsa, &DP, &DQ, &QP ) != MBEDTLS_ERR_RSA_BAD_INPUT_DATA ) { From f02b984f863c1dd0013e12cb9b2c715a63c1823f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 25 Feb 2020 19:52:44 +0100 Subject: [PATCH 2/4] Sanity check on elliptic curve keys: check that the group is known --- programs/fuzz/fuzz_privkey.c | 13 ++++++++----- programs/fuzz/fuzz_pubkey.c | 19 +++++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/programs/fuzz/fuzz_privkey.c b/programs/fuzz/fuzz_privkey.c index 178d17bbc..8f1295e1e 100644 --- a/programs/fuzz/fuzz_privkey.c +++ b/programs/fuzz/fuzz_privkey.c @@ -46,12 +46,15 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { #if defined(MBEDTLS_ECP_C) if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY ) { - mbedtls_ecp_keypair *ecp; + mbedtls_ecp_keypair *ecp = mbedtls_pk_ec( pk ); + mbedtls_ecp_group_id grp_id = ecp->grp.id; + const mbedtls_ecp_curve_info *curve_info = + mbedtls_ecp_curve_info_from_grp_id( grp_id ); - ecp = mbedtls_pk_ec( pk ); - if (ecp) { - ret = 0; - } + /* If the curve is not supported, the key should not have been + * accepted. */ + if( curve_info == NULL ) + abort( ); } else #endif diff --git a/programs/fuzz/fuzz_pubkey.c b/programs/fuzz/fuzz_pubkey.c index c0571c47d..265ee2ae6 100644 --- a/programs/fuzz/fuzz_pubkey.c +++ b/programs/fuzz/fuzz_pubkey.c @@ -41,13 +41,20 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { #if defined(MBEDTLS_ECP_C) if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY ) { - mbedtls_ecp_keypair *ecp; + mbedtls_ecp_keypair *ecp = mbedtls_pk_ec( pk ); + mbedtls_ecp_group_id grp_id = ecp->grp.id; + const mbedtls_ecp_curve_info *curve_info = + mbedtls_ecp_curve_info_from_grp_id( grp_id ); - ecp = mbedtls_pk_ec( pk ); - //dummy use of value - if (ecp) { - ret = 0; - } + /* If the curve is not supported, the key should not have been + * accepted. */ + if( curve_info == NULL ) + abort( ); + + /* It's a public key, so the private value should not have + * been changed from its initialization to 0. */ + if( mbedtls_mpi_cmp_int( &ecp->d, 0 ) != 0 ) + abort( ); } else #endif From e60b365a5e4a991a843d85b749632628b0b585e5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 25 Feb 2020 19:54:07 +0100 Subject: [PATCH 3/4] EC keys can have the type MBEDTLS_PK_ECKEY_DH too --- programs/fuzz/fuzz_privkey.c | 3 ++- programs/fuzz/fuzz_pubkey.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/programs/fuzz/fuzz_privkey.c b/programs/fuzz/fuzz_privkey.c index 8f1295e1e..2a64f57d0 100644 --- a/programs/fuzz/fuzz_privkey.c +++ b/programs/fuzz/fuzz_privkey.c @@ -44,7 +44,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { else #endif #if defined(MBEDTLS_ECP_C) - if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY ) + if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY || + mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY_DH ) { mbedtls_ecp_keypair *ecp = mbedtls_pk_ec( pk ); mbedtls_ecp_group_id grp_id = ecp->grp.id; diff --git a/programs/fuzz/fuzz_pubkey.c b/programs/fuzz/fuzz_pubkey.c index 265ee2ae6..95b75211c 100644 --- a/programs/fuzz/fuzz_pubkey.c +++ b/programs/fuzz/fuzz_pubkey.c @@ -39,7 +39,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { else #endif #if defined(MBEDTLS_ECP_C) - if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY ) + if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY || + mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY_DH ) { mbedtls_ecp_keypair *ecp = mbedtls_pk_ec( pk ); mbedtls_ecp_group_id grp_id = ecp->grp.id; From d7fb66fd138380b9bb2dab5acd5b5d91de00a9ef Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 25 Feb 2020 19:54:27 +0100 Subject: [PATCH 4/4] If a key is not of a supported type, something went wrong --- programs/fuzz/fuzz_privkey.c | 8 +++++--- programs/fuzz/fuzz_pubkey.c | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/programs/fuzz/fuzz_privkey.c b/programs/fuzz/fuzz_privkey.c index 2a64f57d0..6c968fd54 100644 --- a/programs/fuzz/fuzz_privkey.c +++ b/programs/fuzz/fuzz_privkey.c @@ -59,9 +59,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { } else #endif - { - ret = 0; - } + { + /* The key is valid but is not of a supported type. + * This should not happen. */ + abort( ); + } } mbedtls_pk_free( &pk ); #else diff --git a/programs/fuzz/fuzz_pubkey.c b/programs/fuzz/fuzz_pubkey.c index 95b75211c..9e8035045 100644 --- a/programs/fuzz/fuzz_pubkey.c +++ b/programs/fuzz/fuzz_pubkey.c @@ -60,7 +60,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { else #endif { - ret = 0; + /* The key is valid but is not of a supported type. + * This should not happen. */ + abort( ); } } mbedtls_pk_free( &pk );