From ad5390fc4f2f7274b0311f5398b9bd4cdd89d49b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 01/22] Clarify that RNG parameters are mandatory in SSL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No change here, these were already mandatory, it just wasn't explicit in the documentation. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/ssl.h | 2 +- include/mbedtls/ssl_ticket.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b2f5c67a2..8c442871b 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1535,7 +1535,7 @@ void mbedtls_ssl_conf_verify( mbedtls_ssl_config *conf, * \brief Set the random number generator callback * * \param conf SSL configuration - * \param f_rng RNG function + * \param f_rng RNG function (mandatory) * \param p_rng RNG parameter */ void mbedtls_ssl_conf_rng( mbedtls_ssl_config *conf, diff --git a/include/mbedtls/ssl_ticket.h b/include/mbedtls/ssl_ticket.h index 871eec379..1047dbb0d 100644 --- a/include/mbedtls/ssl_ticket.h +++ b/include/mbedtls/ssl_ticket.h @@ -90,7 +90,7 @@ void mbedtls_ssl_ticket_init( mbedtls_ssl_ticket_context *ctx ); * \brief Prepare context to be actually used * * \param ctx Context to be set up - * \param f_rng RNG callback function + * \param f_rng RNG callback function (mandatory) * \param p_rng RNG callback context * \param cipher AEAD cipher to use for ticket protection. * Recommended value: MBEDTLS_CIPHER_AES_256_GCM. From c305b72ed1d54955b81a5a8ad402cc5c05cdacdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 02/22] Make RNG parameters mandatory in X.509 functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not adding a check in the code here, as this will be checked by the lower-level modules. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/x509_crt.h | 14 ++++---------- include/mbedtls/x509_csr.h | 14 ++++---------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index d383168d2..5c5509c79 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -1108,16 +1108,13 @@ void mbedtls_x509write_crt_free( mbedtls_x509write_cert *ctx ); * \param ctx certificate to write away * \param buf buffer to write to * \param size size of the buffer - * \param f_rng RNG function (for signature, see note) + * \param f_rng RNG function. This must not be \c NULL. * \param p_rng RNG parameter * * \return length of data written if successful, or a specific * error code * - * \note f_rng may be NULL if RSA is used for signature and the - * signature is made offline (otherwise f_rng is desirable - * for countermeasures against timing attacks). - * ECDSA signatures always require a non-NULL f_rng. + * \note \p f_rng is used for the signature operation. */ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, size_t size, int (*f_rng)(void *, unsigned char *, size_t), @@ -1130,15 +1127,12 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, * \param ctx certificate to write away * \param buf buffer to write to * \param size size of the buffer - * \param f_rng RNG function (for signature, see note) + * \param f_rng RNG function. This must not be \c NULL. * \param p_rng RNG parameter * * \return 0 if successful, or a specific error code * - * \note f_rng may be NULL if RSA is used for signature and the - * signature is made offline (otherwise f_rng is desirable - * for countermeasures against timing attacks). - * ECDSA signatures always require a non-NULL f_rng. + * \note \p f_rng is used for the signature operation. */ int mbedtls_x509write_crt_pem( mbedtls_x509write_cert *ctx, unsigned char *buf, size_t size, int (*f_rng)(void *, unsigned char *, size_t), diff --git a/include/mbedtls/x509_csr.h b/include/mbedtls/x509_csr.h index 0a069df93..a0f1278e2 100644 --- a/include/mbedtls/x509_csr.h +++ b/include/mbedtls/x509_csr.h @@ -264,16 +264,13 @@ void mbedtls_x509write_csr_free( mbedtls_x509write_csr *ctx ); * \param ctx CSR to write away * \param buf buffer to write to * \param size size of the buffer - * \param f_rng RNG function (for signature, see note) + * \param f_rng RNG function. This must not be \c NULL. * \param p_rng RNG parameter * * \return length of data written if successful, or a specific * error code * - * \note f_rng may be NULL if RSA is used for signature and the - * signature is made offline (otherwise f_rng is desirable - * for countermeasures against timing attacks). - * ECDSA signatures always require a non-NULL f_rng. + * \note \p f_rng is used for the signature operation. */ int mbedtls_x509write_csr_der( mbedtls_x509write_csr *ctx, unsigned char *buf, size_t size, int (*f_rng)(void *, unsigned char *, size_t), @@ -287,15 +284,12 @@ int mbedtls_x509write_csr_der( mbedtls_x509write_csr *ctx, unsigned char *buf, s * \param ctx CSR to write away * \param buf buffer to write to * \param size size of the buffer - * \param f_rng RNG function (for signature, see note) + * \param f_rng RNG function. This must not be \c NULL. * \param p_rng RNG parameter * * \return 0 if successful, or a specific error code * - * \note f_rng may be NULL if RSA is used for signature and the - * signature is made offline (otherwise f_rng is desirable - * for countermeasures against timing attacks). - * ECDSA signatures always require a non-NULL f_rng. + * \note \p f_rng is used for the signature operation. */ int mbedtls_x509write_csr_pem( mbedtls_x509write_csr *ctx, unsigned char *buf, size_t size, int (*f_rng)(void *, unsigned char *, size_t), From 34d3756457c4787e9194b408968950c4e18e9041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 03/22] Make RNG parameters mandatory in PK functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Again, no check here, will be checked by lower-level modules. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 85bf7c906..36af5d98d 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -498,7 +498,7 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options, * given the key type. * \param sig_len On successful return, * the number of bytes written to \p sig. - * \param f_rng RNG function + * \param f_rng RNG function, must not be \c NULL. * \param p_rng RNG parameter * * \return 0 on success, or a specific error code. @@ -538,7 +538,7 @@ int mbedtls_pk_sign( mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg, * given the key type. * \param sig_len On successful return, * the number of bytes written to \p sig. - * \param f_rng RNG function + * \param f_rng RNG function, must not be \c NULL. * \param p_rng RNG parameter * \param rs_ctx Restart context (NULL to disable restart) * @@ -563,7 +563,7 @@ int mbedtls_pk_sign_restartable( mbedtls_pk_context *ctx, * \param output Decrypted output * \param olen Decrypted message length * \param osize Size of the output buffer - * \param f_rng RNG function + * \param f_rng RNG function, must not be \c NULL. * \param p_rng RNG parameter * * \note For RSA keys, the default padding type is PKCS#1 v1.5. @@ -584,9 +584,11 @@ int mbedtls_pk_decrypt( mbedtls_pk_context *ctx, * \param output Encrypted output * \param olen Encrypted output length * \param osize Size of the output buffer - * \param f_rng RNG function + * \param f_rng RNG function, must not be \c NULL. * \param p_rng RNG parameter * + * \note \p f_rng is used for padding generation. + * * \note For RSA keys, the default padding type is PKCS#1 v1.5. * * \return 0 on success, or a specific error code. From f0359040604503f63f01fcb3ccdc3f147eae82bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 04/22] Check for mandatory RNG parameters in RSA private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (This commit is best reviewed using `git show -b` as indentation levels have changed.) The documentation already states that the RNG parameter is mandatory, since PRs #4488 and #4515. There are several families of functions to consider here: - private-key operations (sign, decrypt) all call mbedtls_rsa_private() where this commit adds a non-NULL check; - encrypt operations need an RNG for masking/padding and already had a non-NULL check since #4515 (conditional on \p mode before that) - verify operations no longer take an RNG parameter since #4515 So, after this commit, all RSA functions that accept an RNG will reach a non-NULL check before the RNG is used. Signed-off-by: Manuel Pégourié-Gonnard --- library/rsa.c | 121 +++++++++++++++++++++++--------------------------- 1 file changed, 56 insertions(+), 65 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index f7274eab1..d4e63b65c 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -929,8 +929,11 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, RSA_VALIDATE_RET( input != NULL ); RSA_VALIDATE_RET( output != NULL ); - if( rsa_check_context( ctx, 1 /* private key checks */, - f_rng != NULL /* blinding y/n */ ) != 0 ) + if( f_rng == NULL ) + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + + if( rsa_check_context( ctx, 1 /* private key checks */, + 1 /* blinding on */ ) != 0 ) { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } @@ -947,15 +950,12 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, mbedtls_mpi_init( &Q1 ); mbedtls_mpi_init( &R ); - if( f_rng != NULL ) - { #if defined(MBEDTLS_RSA_NO_CRT) - mbedtls_mpi_init( &D_blind ); + mbedtls_mpi_init( &D_blind ); #else - mbedtls_mpi_init( &DP_blind ); - mbedtls_mpi_init( &DQ_blind ); + mbedtls_mpi_init( &DP_blind ); + mbedtls_mpi_init( &DQ_blind ); #endif - } #if !defined(MBEDTLS_RSA_NO_CRT) mbedtls_mpi_init( &TP ); mbedtls_mpi_init( &TQ ); @@ -975,57 +975,54 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &I, &T ) ); - if( f_rng != NULL ) - { - /* - * Blinding - * T = T * Vi mod N - */ - MBEDTLS_MPI_CHK( rsa_prepare_blinding( ctx, f_rng, p_rng ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T, &T, &ctx->Vi ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); + /* + * Blinding + * T = T * Vi mod N + */ + MBEDTLS_MPI_CHK( rsa_prepare_blinding( ctx, f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T, &T, &ctx->Vi ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); - /* - * Exponent blinding - */ - MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &P1, &ctx->P, 1 ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &Q1, &ctx->Q, 1 ) ); + /* + * Exponent blinding + */ + MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &P1, &ctx->P, 1 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &Q1, &ctx->Q, 1 ) ); #if defined(MBEDTLS_RSA_NO_CRT) - /* - * D_blind = ( P - 1 ) * ( Q - 1 ) * R + D - */ - MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, RSA_EXPONENT_BLINDING, - f_rng, p_rng ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &D_blind, &P1, &Q1 ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &D_blind, &D_blind, &R ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &D_blind, &D_blind, &ctx->D ) ); + /* + * D_blind = ( P - 1 ) * ( Q - 1 ) * R + D + */ + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, RSA_EXPONENT_BLINDING, + f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &D_blind, &P1, &Q1 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &D_blind, &D_blind, &R ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &D_blind, &D_blind, &ctx->D ) ); - D = &D_blind; + D = &D_blind; #else - /* - * DP_blind = ( P - 1 ) * R + DP - */ - MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, RSA_EXPONENT_BLINDING, - f_rng, p_rng ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &DP_blind, &P1, &R ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &DP_blind, &DP_blind, - &ctx->DP ) ); + /* + * DP_blind = ( P - 1 ) * R + DP + */ + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, RSA_EXPONENT_BLINDING, + f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &DP_blind, &P1, &R ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &DP_blind, &DP_blind, + &ctx->DP ) ); - DP = &DP_blind; + DP = &DP_blind; - /* - * DQ_blind = ( Q - 1 ) * R + DQ - */ - MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, RSA_EXPONENT_BLINDING, - f_rng, p_rng ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &DQ_blind, &Q1, &R ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &DQ_blind, &DQ_blind, - &ctx->DQ ) ); + /* + * DQ_blind = ( Q - 1 ) * R + DQ + */ + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, RSA_EXPONENT_BLINDING, + f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &DQ_blind, &Q1, &R ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &DQ_blind, &DQ_blind, + &ctx->DQ ) ); - DQ = &DQ_blind; + DQ = &DQ_blind; #endif /* MBEDTLS_RSA_NO_CRT */ - } #if defined(MBEDTLS_RSA_NO_CRT) MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &T, &T, D, &ctx->N, &ctx->RN ) ); @@ -1054,15 +1051,12 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &T, &TQ, &TP ) ); #endif /* MBEDTLS_RSA_NO_CRT */ - if( f_rng != NULL ) - { - /* - * Unblind - * T = T * Vf mod N - */ - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T, &T, &ctx->Vf ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); - } + /* + * Unblind + * T = T * Vf mod N + */ + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T, &T, &ctx->Vf ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); /* Verify the result to prevent glitching attacks. */ MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E, @@ -1086,15 +1080,12 @@ cleanup: mbedtls_mpi_free( &Q1 ); mbedtls_mpi_free( &R ); - if( f_rng != NULL ) - { #if defined(MBEDTLS_RSA_NO_CRT) - mbedtls_mpi_free( &D_blind ); + mbedtls_mpi_free( &D_blind ); #else - mbedtls_mpi_free( &DP_blind ); - mbedtls_mpi_free( &DQ_blind ); + mbedtls_mpi_free( &DP_blind ); + mbedtls_mpi_free( &DQ_blind ); #endif - } mbedtls_mpi_free( &T ); From 1a87722bb69540bbb621a42f54c5cb4302b01bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 05/22] Make RNG parameters mandatory in DHM functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/dhm.h | 8 ++++---- library/dhm.c | 21 ++++++++------------- tests/suites/test_suite_dhm.function | 10 ++++++++-- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/include/mbedtls/dhm.h b/include/mbedtls/dhm.h index e8c8a82f5..850813e37 100644 --- a/include/mbedtls/dhm.h +++ b/include/mbedtls/dhm.h @@ -279,10 +279,10 @@ int mbedtls_dhm_make_public( mbedtls_dhm_context *ctx, int x_size, * \param output_size The size of the destination buffer. This must be at * least the size of \c ctx->len (the size of \c P). * \param olen On exit, holds the actual number of Bytes written. - * \param f_rng The RNG function, for blinding purposes. This may - * b \c NULL if blinding isn't needed. - * \param p_rng The RNG context. This may be \c NULL if \p f_rng - * doesn't need a context argument. + * \param f_rng The RNG function. Must not be \c NULL. Used for + * blinding. + * \param p_rng The RNG context to be passed to \p f_rng. This may be + * \c NULL if \p f_rng doesn't need a context parameter. * * \return \c 0 on success. * \return An \c MBEDTLS_ERR_DHM_XXX error code on failure. diff --git a/library/dhm.c b/library/dhm.c index e88f3a2c7..29ce75598 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -444,6 +444,9 @@ int mbedtls_dhm_calc_secret( mbedtls_dhm_context *ctx, DHM_VALIDATE_RET( output != NULL ); DHM_VALIDATE_RET( olen != NULL ); + if( f_rng == NULL ) + return( MBEDTLS_ERR_DHM_BAD_INPUT_DATA ); + if( output_size < mbedtls_dhm_get_len( ctx ) ) return( MBEDTLS_ERR_DHM_BAD_INPUT_DATA ); @@ -453,25 +456,17 @@ int mbedtls_dhm_calc_secret( mbedtls_dhm_context *ctx, mbedtls_mpi_init( &GYb ); /* Blind peer's value */ - if( f_rng != NULL ) - { - MBEDTLS_MPI_CHK( dhm_update_blinding( ctx, f_rng, p_rng ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &GYb, &ctx->GY, &ctx->Vi ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &GYb, &GYb, &ctx->P ) ); - } - else - MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &GYb, &ctx->GY ) ); + MBEDTLS_MPI_CHK( dhm_update_blinding( ctx, f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &GYb, &ctx->GY, &ctx->Vi ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &GYb, &GYb, &ctx->P ) ); /* Do modular exponentiation */ MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &ctx->K, &GYb, &ctx->X, &ctx->P, &ctx->RP ) ); /* Unblind secret value */ - if( f_rng != NULL ) - { - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->K, &ctx->K, &ctx->Vf ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->K, &ctx->K, &ctx->P ) ); - } + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->K, &ctx->K, &ctx->Vf ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->K, &ctx->K, &ctx->P ) ); /* Output the secret without any leading zero byte. This is mandatory * for TLS per RFC 5246 §8.1.2. */ diff --git a/tests/suites/test_suite_dhm.function b/tests/suites/test_suite_dhm.function index 62e634a7f..5286bc7b8 100644 --- a/tests/suites/test_suite_dhm.function +++ b/tests/suites/test_suite_dhm.function @@ -150,7 +150,10 @@ void dhm_do_dhm( int radix_P, char *input_P, int x_size, &sec_srv_len, &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); - TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, sizeof( sec_cli ), &sec_cli_len, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, sizeof( sec_cli ), + &sec_cli_len, + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ) == 0 ); TEST_ASSERT( sec_srv_len == sec_cli_len ); TEST_ASSERT( sec_srv_len != 0 ); @@ -206,7 +209,10 @@ void dhm_do_dhm( int radix_P, char *input_P, int x_size, &sec_srv_len, &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); - TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, sizeof( sec_cli ), &sec_cli_len, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, sizeof( sec_cli ), + &sec_cli_len, + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ) == 0 ); TEST_ASSERT( sec_srv_len == sec_cli_len ); TEST_ASSERT( sec_srv_len != 0 ); From 7861ecf8384e6d3eb75568b113cb5de2c56fb06d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 06/22] Make RNG parameters mandatory in ECDH functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Again, no check in the code - will be checked by ECP Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/ecdh.h | 8 ++------ tests/suites/test_suite_ecdh.function | 24 ++++++++++++++++++------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/include/mbedtls/ecdh.h b/include/mbedtls/ecdh.h index 2a0980b39..587035aad 100644 --- a/include/mbedtls/ecdh.h +++ b/include/mbedtls/ecdh.h @@ -222,10 +222,7 @@ int mbedtls_ecdh_gen_public( mbedtls_ecp_group *grp, mbedtls_mpi *d, mbedtls_ecp * This must be initialized. * \param d Our secret exponent (private key). * This must be initialized. - * \param f_rng The RNG function. This may be \c NULL if randomization - * of intermediate results during the ECP computations is - * not needed (discouraged). See the documentation of - * mbedtls_ecp_mul() for more. + * \param f_rng The RNG function to use. This must not be \c NULL. * \param p_rng The RNG context to be passed to \p f_rng. This may be * \c NULL if \p f_rng is \c NULL or doesn't need a * context argument. @@ -428,8 +425,7 @@ int mbedtls_ecdh_read_public( mbedtls_ecdh_context *ctx, * \param buf The buffer to write the generated shared key to. This * must be a writable buffer of size \p blen Bytes. * \param blen The length of the destination buffer \p buf in Bytes. - * \param f_rng The RNG function, for blinding purposes. This may - * b \c NULL if blinding isn't needed. + * \param f_rng The RNG function to use. This must not be \c NULL. * \param p_rng The RNG context. This may be \c NULL if \p f_rng * doesn't need a context argument. * diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function index 6e8459dcb..94030d89d 100644 --- a/tests/suites/test_suite_ecdh.function +++ b/tests/suites/test_suite_ecdh.function @@ -85,7 +85,8 @@ void ecdh_primitive_random( int id ) &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_ecdh_compute_shared( &grp, &zB, &qA, &dB, - NULL, NULL ) == 0 ); + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &zA, &zB ) == 0 ); @@ -106,11 +107,13 @@ void ecdh_primitive_testvec( int id, data_t * rnd_buf_A, char * xA_str, mbedtls_ecp_point qA, qB; mbedtls_mpi dA, dB, zA, zB, check; mbedtls_test_rnd_buf_info rnd_info_A, rnd_info_B; + mbedtls_test_rnd_pseudo_info rnd_info; mbedtls_ecp_group_init( &grp ); mbedtls_ecp_point_init( &qA ); mbedtls_ecp_point_init( &qB ); mbedtls_mpi_init( &dA ); mbedtls_mpi_init( &dB ); mbedtls_mpi_init( &zA ); mbedtls_mpi_init( &zB ); mbedtls_mpi_init( &check ); + memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) ); TEST_ASSERT( mbedtls_ecp_group_load( &grp, id ) == 0 ); @@ -169,9 +172,13 @@ void ecdh_primitive_testvec( int id, data_t * rnd_buf_A, char * xA_str, TEST_ASSERT( mbedtls_mpi_cmp_mpi( &qB.Y, &check ) == 0 ); TEST_ASSERT( mbedtls_mpi_read_string( &check, 16, z_str ) == 0 ); - TEST_ASSERT( mbedtls_ecdh_compute_shared( &grp, &zA, &qB, &dA, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_ecdh_compute_shared( &grp, &zA, &qB, &dA, + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &zA, &check ) == 0 ); - TEST_ASSERT( mbedtls_ecdh_compute_shared( &grp, &zB, &qA, &dB, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_ecdh_compute_shared( &grp, &zB, &qA, &dB, + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &zB, &check ) == 0 ); exit: @@ -215,7 +222,8 @@ void ecdh_exchange( int id ) &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_ecdh_calc_secret( &cli, &res_len, res_buf, 1000, - NULL, NULL ) == 0 ); + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ) == 0 ); TEST_ASSERT( len == res_len ); TEST_ASSERT( memcmp( buf, res_buf, len ) == 0 ); @@ -235,12 +243,14 @@ void ecdh_restart( int id, data_t *dA, data_t *dB, data_t *z, const unsigned char *vbuf; size_t len; mbedtls_test_rnd_buf_info rnd_info_A, rnd_info_B; + mbedtls_test_rnd_pseudo_info rnd_info; int cnt_restart; mbedtls_ecp_group grp; mbedtls_ecp_group_init( &grp ); mbedtls_ecdh_init( &srv ); mbedtls_ecdh_init( &cli ); + memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) ); rnd_info_A.fallback_f_rng = mbedtls_test_rnd_std_rand; rnd_info_A.fallback_p_rng = NULL; @@ -315,7 +325,8 @@ void ecdh_restart( int id, data_t *dA, data_t *dB, data_t *z, cnt_restart = 0; do { ret = mbedtls_ecdh_calc_secret( &srv, &len, buf, sizeof( buf ), - NULL, NULL ); + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ); } while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restart ); TEST_ASSERT( ret == 0 ); @@ -332,7 +343,8 @@ void ecdh_restart( int id, data_t *dA, data_t *dB, data_t *z, cnt_restart = 0; do { ret = mbedtls_ecdh_calc_secret( &cli, &len, buf, sizeof( buf ), - NULL, NULL ); + &mbedtls_test_rnd_pseudo_rand, + &rnd_info ); } while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restart ); TEST_ASSERT( ret == 0 ); From aa3ed6f9872e3def71a557a940ab6f81364f274b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 07/22] Make RNG parameters mandatory in ECP functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix trivial faulty calls in ECP test suite and ECP/ECJPAKE self-tests (by adding a dummy RNG). Several tests suites are not passing yet, as a couple of library function do call ecp_mul() with a NULL RNG. The complexity of the fixes range from "simple refactoring" to "requires API changes", so these will be addressed in separate commits. This makes the option MBEDTLS_ECP_NO_INTERNAL_RNG, as well as the whole "internal RNG" code, obsolete. This will be addressed in a future commit, after getting the test suites to pass again. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/ecp.h | 23 +++++++-------------- library/ecjpake.c | 26 +++++++++++++++++++++-- library/ecp.c | 31 +++++++++++++++++++++++++--- tests/suites/test_suite_ecp.function | 23 ++++++++++++++------- 4 files changed, 76 insertions(+), 27 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 49e85d941..f203a7b25 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -911,15 +911,8 @@ int mbedtls_ecp_tls_write_group( const mbedtls_ecp_group *grp, * \note To prevent timing attacks, this function * executes the exact same sequence of base-field * operations for any valid \p m. It avoids any if-branch or - * array index depending on the value of \p m. - * - * \note If \p f_rng is not NULL, it is used to randomize - * intermediate results to prevent potential timing attacks - * targeting these results. We recommend always providing - * a non-NULL \p f_rng. The overhead is negligible. - * Note: unless #MBEDTLS_ECP_NO_INTERNAL_RNG is defined, when - * \p f_rng is NULL, an internal RNG (seeded from the value - * of \p m) will be used instead. + * array index depending on the value of \p m. If also uses + * \p f_rng to randomize some intermediate results. * * \param grp The ECP group to use. * This must be initialized and have group parameters @@ -928,9 +921,9 @@ int mbedtls_ecp_tls_write_group( const mbedtls_ecp_group *grp, * This must be initialized. * \param m The integer by which to multiply. This must be initialized. * \param P The point to multiply. This must be initialized. - * \param f_rng The RNG function. This may be \c NULL if randomization - * of intermediate results isn't desired (discouraged). - * \param p_rng The RNG context to be passed to \p p_rng. + * \param f_rng The RNG function. This must not be \c NULL. + * \param p_rng The RNG context to be passed to \p f_rng. This may be \c + * NULL if \p f_rng doesn't need a context. * * \return \c 0 on success. * \return #MBEDTLS_ERR_ECP_INVALID_KEY if \p m is not a valid private @@ -959,9 +952,9 @@ int mbedtls_ecp_mul( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, * This must be initialized. * \param m The integer by which to multiply. This must be initialized. * \param P The point to multiply. This must be initialized. - * \param f_rng The RNG function. This may be \c NULL if randomization - * of intermediate results isn't desired (discouraged). - * \param p_rng The RNG context to be passed to \p p_rng. + * \param f_rng The RNG function. This must not be \c NULL. + * \param p_rng The RNG context to be passed to \p f_rng. This may be \c + * NULL if \p f_rng doesn't need a context. * \param rs_ctx The restart context (NULL disables restart). * * \return \c 0 on success. diff --git a/library/ecjpake.c b/library/ecjpake.c index de43ddb70..d22931142 100644 --- a/library/ecjpake.c +++ b/library/ecjpake.c @@ -962,6 +962,28 @@ static const unsigned char ecjpake_test_pms[] = { 0xb4, 0x38, 0xf7, 0x19, 0xd3, 0xc4, 0xf3, 0x51 }; +/* + * PRNG for test - !!!INSECURE NEVER USE IN PRODUCTION!!! + * + * This is the linear congruential generator from numerical recipes, + * except we only use the low byte as the output. See + * https://en.wikipedia.org/wiki/Linear_congruential_generator#Parameters_in_common_use + */ +static int self_test_rng( void *ctx, unsigned char *out, size_t len ) +{ + static uint32_t state = 42; + + (void) ctx; + + for( size_t i = 0; i < len; i++ ) + { + state = state * 1664525u + 1013904223u; + out[i] = (unsigned char) state; + } + + return( 0 ); +} + /* Load my private keys and generate the corresponding public keys */ static int ecjpake_test_load( mbedtls_ecjpake_context *ctx, const unsigned char *xm1, size_t len1, @@ -972,9 +994,9 @@ static int ecjpake_test_load( mbedtls_ecjpake_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &ctx->xm1, xm1, len1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &ctx->xm2, xm2, len2 ) ); MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &ctx->grp, &ctx->Xm1, &ctx->xm1, - &ctx->grp.G, NULL, NULL ) ); + &ctx->grp.G, self_test_rng, NULL ) ); MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &ctx->grp, &ctx->Xm2, &ctx->xm2, - &ctx->grp.G, NULL, NULL ) ); + &ctx->grp.G, self_test_rng, NULL ) ); cleanup: return( ret ); diff --git a/library/ecp.c b/library/ecp.c index 044bbe1d1..873b4c839 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2684,6 +2684,9 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, ECP_VALIDATE_RET( m != NULL ); ECP_VALIDATE_RET( P != NULL ); + if( f_rng == NULL ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + #if defined(MBEDTLS_ECP_RESTARTABLE) /* reset ops count for this call if top-level */ if( rs_ctx != NULL && rs_ctx->depth++ == 0 ) @@ -3315,6 +3318,28 @@ cleanup: #if defined(MBEDTLS_SELF_TEST) +/* + * PRNG for test - !!!INSECURE NEVER USE IN PRODUCTION!!! + * + * This is the linear congruential generator from numerical recipes, + * except we only use the low byte as the output. See + * https://en.wikipedia.org/wiki/Linear_congruential_generator#Parameters_in_common_use + */ +static int self_test_rng( void *ctx, unsigned char *out, size_t len ) +{ + static uint32_t state = 42; + + (void) ctx; + + for( size_t i = 0; i < len; i++ ) + { + state = state * 1664525u + 1013904223u; + out[i] = (unsigned char) state; + } + + return( 0 ); +} + /* Adjust the exponent to be a valid private point for the specified curve. * This is sometimes necessary because we use a single set of exponents * for all curves but the validity of values depends on the curve. */ @@ -3370,7 +3395,7 @@ static int self_test_point( int verbose, MBEDTLS_MPI_CHK( mbedtls_mpi_read_string( m, 16, exponents[0] ) ); MBEDTLS_MPI_CHK( self_test_adjust_exponent( grp, m ) ); - MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, NULL, NULL ) ); + MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, self_test_rng, NULL ) ); for( i = 1; i < n_exponents; i++ ) { @@ -3383,7 +3408,7 @@ static int self_test_point( int verbose, MBEDTLS_MPI_CHK( mbedtls_mpi_read_string( m, 16, exponents[i] ) ); MBEDTLS_MPI_CHK( self_test_adjust_exponent( grp, m ) ); - MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, NULL, NULL ) ); + MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, self_test_rng, NULL ) ); if( add_count != add_c_prev || dbl_count != dbl_c_prev || @@ -3461,7 +3486,7 @@ int mbedtls_ecp_self_test( int verbose ) mbedtls_printf( " ECP SW test #1 (constant op_count, base point G): " ); /* Do a dummy multiplication first to trigger precomputation */ MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &m, 2 ) ); - MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &grp, &P, &m, &grp.G, NULL, NULL ) ); + MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &grp, &P, &m, &grp.G, self_test_rng, NULL ) ); ret = self_test_point( verbose, &grp, &R, &m, &grp.G, sw_exponents, diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index f2b637614..e820067a7 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -124,12 +124,14 @@ void ecp_test_vect_restart( int id, mbedtls_mpi dA, xA, yA, dB, xZ, yZ; int cnt_restarts; int ret; + mbedtls_test_rnd_pseudo_info rnd_info; mbedtls_ecp_restart_init( &ctx ); mbedtls_ecp_group_init( &grp ); mbedtls_ecp_point_init( &R ); mbedtls_ecp_point_init( &P ); mbedtls_mpi_init( &dA ); mbedtls_mpi_init( &xA ); mbedtls_mpi_init( &yA ); mbedtls_mpi_init( &dB ); mbedtls_mpi_init( &xZ ); mbedtls_mpi_init( &yZ ); + memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) ); TEST_ASSERT( mbedtls_ecp_group_load( &grp, id ) == 0 ); @@ -147,7 +149,8 @@ void ecp_test_vect_restart( int id, cnt_restarts = 0; do { ECP_PT_RESET( &R ); - ret = mbedtls_ecp_mul_restartable( &grp, &R, &dA, &grp.G, NULL, NULL, &ctx ); + ret = mbedtls_ecp_mul_restartable( &grp, &R, &dA, &grp.G, + &mbedtls_test_rnd_pseudo_rand, &rnd_info, &ctx ); } while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restarts ); TEST_ASSERT( ret == 0 ); @@ -162,7 +165,8 @@ void ecp_test_vect_restart( int id, cnt_restarts = 0; do { ECP_PT_RESET( &R ); - ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P, NULL, NULL, &ctx ); + ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P, + &mbedtls_test_rnd_pseudo_rand, &rnd_info, &ctx ); } while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restarts ); TEST_ASSERT( ret == 0 ); @@ -176,7 +180,8 @@ void ecp_test_vect_restart( int id, * This test only makes sense when we actually restart */ if( min_restarts > 0 ) { - ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P, NULL, NULL, &ctx ); + ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P, + &mbedtls_test_rnd_pseudo_rand, &rnd_info, &ctx ); TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); } @@ -294,12 +299,14 @@ void ecp_test_vect( int id, char * dA_str, char * xA_str, char * yA_str, TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xA ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yA ) == 0 ); TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 ); - TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &R, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &R, + &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xZ ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yZ ) == 0 ); TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 ); - TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G, + &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xB ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yB ) == 0 ); TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 ); @@ -351,11 +358,13 @@ void ecp_test_vec_x( int id, char * dA_hex, char * xA_hex, char * dB_hex, TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xS ) == 0 ); - TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G, + &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xB ) == 0 ); - TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dA, &R, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dA, &R, + &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xS ) == 0 ); From 75525aec527bc9d3bba2cd3214c3c8fc2d9961af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 08/22] Fix mbedtls_ecp_muladd() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was indirectly calling ecp_mul() without an RNG. That's actually the rare case where this should be allowed, as ecp_muladd() is typically used on non-secret data (to verify signatures or ZKPs) and documented as not being constant-time. Refactor a bit in order to keep the ability to call ecp_mul() without a RNG, but not exposed publicly (except though muladd). Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 873b4c839..bd560b574 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2669,8 +2669,11 @@ cleanup: /* * Restartable multiplication R = m * P + * + * This internal function can be called without an RNG in case where we know + * the inputs are not sensitive. */ -int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, +static int ecp_mul_restartable_internal( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, const mbedtls_mpi *m, const mbedtls_ecp_point *P, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, mbedtls_ecp_restart_ctx *rs_ctx ) @@ -2679,13 +2682,6 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #if defined(MBEDTLS_ECP_INTERNAL_ALT) char is_grp_capable = 0; #endif - ECP_VALIDATE_RET( grp != NULL ); - ECP_VALIDATE_RET( R != NULL ); - ECP_VALIDATE_RET( m != NULL ); - ECP_VALIDATE_RET( P != NULL ); - - if( f_rng == NULL ) - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); #if defined(MBEDTLS_ECP_RESTARTABLE) /* reset ops count for this call if top-level */ @@ -2738,6 +2734,25 @@ cleanup: return( ret ); } +/* + * Restartable multiplication R = m * P + */ +int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, + const mbedtls_mpi *m, const mbedtls_ecp_point *P, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, + mbedtls_ecp_restart_ctx *rs_ctx ) +{ + ECP_VALIDATE_RET( grp != NULL ); + ECP_VALIDATE_RET( R != NULL ); + ECP_VALIDATE_RET( m != NULL ); + ECP_VALIDATE_RET( P != NULL ); + + if( f_rng == NULL ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + + return( ecp_mul_restartable_internal( grp, R, m, P, f_rng, p_rng, rs_ctx ) ); +} + /* * Multiplication R = m * P */ @@ -2831,8 +2846,8 @@ static int mbedtls_ecp_mul_shortcuts( mbedtls_ecp_group *grp, } else { - MBEDTLS_MPI_CHK( mbedtls_ecp_mul_restartable( grp, R, m, P, - NULL, NULL, rs_ctx ) ); + MBEDTLS_MPI_CHK( ecp_mul_restartable_internal( grp, R, m, P, + NULL, NULL, rs_ctx ) ); } cleanup: From f8c24bf507089b5e90721cb59be399d359459411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 09/22] Fix signature of check_pub_priv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/ecp.h | 8 ++++++-- library/ecp.c | 6 ++++-- tests/suites/test_suite_ecp.function | 5 ++++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index f203a7b25..75ad8087d 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1258,14 +1258,18 @@ int mbedtls_ecp_write_key( mbedtls_ecp_keypair *key, * part is ignored. * \param prv The keypair structure holding the full keypair. * This must be initialized. + * \param f_rng The RNG function. This must not be \c NULL. + * \param p_rng The RNG context to be passed to \p f_rng. This may be \c + * NULL if \p f_rng doesn't need a context. * * \return \c 0 on success, meaning that the keys are valid and match. * \return #MBEDTLS_ERR_ECP_BAD_INPUT_DATA if the keys are invalid or do not match. * \return An \c MBEDTLS_ERR_ECP_XXX or an \c MBEDTLS_ERR_MPI_XXX * error code on calculation failure. */ -int mbedtls_ecp_check_pub_priv( const mbedtls_ecp_keypair *pub, - const mbedtls_ecp_keypair *prv ); +int mbedtls_ecp_check_pub_priv( + const mbedtls_ecp_keypair *pub, const mbedtls_ecp_keypair *prv, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); #if defined(MBEDTLS_SELF_TEST) diff --git a/library/ecp.c b/library/ecp.c index bd560b574..1a78a8f32 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3290,7 +3290,9 @@ cleanup: /* * Check a public-private key pair */ -int mbedtls_ecp_check_pub_priv( const mbedtls_ecp_keypair *pub, const mbedtls_ecp_keypair *prv ) +int mbedtls_ecp_check_pub_priv( + const mbedtls_ecp_keypair *pub, const mbedtls_ecp_keypair *prv, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ecp_point Q; @@ -3314,7 +3316,7 @@ int mbedtls_ecp_check_pub_priv( const mbedtls_ecp_keypair *pub, const mbedtls_ec mbedtls_ecp_group_copy( &grp, &prv->grp ); /* Also checks d is valid */ - MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &grp, &Q, &prv->d, &prv->grp.G, NULL, NULL ) ); + MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &grp, &Q, &prv->d, &prv->grp.G, f_rng, p_rng ) ); if( mbedtls_mpi_cmp_mpi( &Q.X, &prv->Q.X ) || mbedtls_mpi_cmp_mpi( &Q.Y, &prv->Q.Y ) || diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index e820067a7..d795fe214 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -776,9 +776,11 @@ void mbedtls_ecp_check_pub_priv( int id_pub, char * Qx_pub, char * Qy_pub, int ret ) { mbedtls_ecp_keypair pub, prv; + mbedtls_test_rnd_pseudo_info rnd_info; mbedtls_ecp_keypair_init( &pub ); mbedtls_ecp_keypair_init( &prv ); + memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) ); if( id_pub != MBEDTLS_ECP_DP_NONE ) TEST_ASSERT( mbedtls_ecp_group_load( &pub.grp, id_pub ) == 0 ); @@ -789,7 +791,8 @@ void mbedtls_ecp_check_pub_priv( int id_pub, char * Qx_pub, char * Qy_pub, TEST_ASSERT( mbedtls_ecp_point_read_string( &prv.Q, 16, Qx, Qy ) == 0 ); TEST_ASSERT( mbedtls_mpi_read_string( &prv.d, 16, d ) == 0 ); - TEST_ASSERT( mbedtls_ecp_check_pub_priv( &pub, &prv ) == ret ); + TEST_ASSERT( mbedtls_ecp_check_pub_priv( &pub, &prv, + &mbedtls_test_rnd_pseudo_rand, &rnd_info ) == ret ); exit: mbedtls_ecp_keypair_free( &pub ); From 39be1410fdad87998cc345a6b808410ded100dd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 10/22] Add RNG parameter to check_pair functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - mbedtls_ecp_check_pub_priv() because it calls ecp_mul() - mbedtls_pk_check_pair() because it calls the former Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 7 ++++++- library/pk.c | 10 ++++++++-- library/pk_wrap.c | 19 ++++++++++++++----- library/pk_wrap.h | 4 +++- programs/x509/cert_write.c | 3 ++- tests/suites/test_suite_pk.function | 11 ++++++++--- 6 files changed, 41 insertions(+), 13 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 36af5d98d..5c7b2f646 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -603,6 +603,8 @@ int mbedtls_pk_encrypt( mbedtls_pk_context *ctx, * * \param pub Context holding a public key. * \param prv Context holding a private (and public) key. + * \param f_rng RNG function, must not be \c NULL. + * \param p_rng RNG parameter * * \return \c 0 on success (keys were checked and match each other). * \return #MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE if the keys could not @@ -610,7 +612,10 @@ int mbedtls_pk_encrypt( mbedtls_pk_context *ctx, * \return #MBEDTLS_ERR_PK_BAD_INPUT_DATA if a context is invalid. * \return Another non-zero value if the keys do not match. */ -int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_context *prv ); +int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, + const mbedtls_pk_context *prv, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ); /** * \brief Export debug information diff --git a/library/pk.c b/library/pk.c index 06021e26c..275d34bb1 100644 --- a/library/pk.c +++ b/library/pk.c @@ -500,7 +500,10 @@ int mbedtls_pk_encrypt( mbedtls_pk_context *ctx, /* * Check public-private key pair */ -int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_context *prv ) +int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, + const mbedtls_pk_context *prv, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { PK_VALIDATE_RET( pub != NULL ); PK_VALIDATE_RET( prv != NULL ); @@ -511,6 +514,9 @@ int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_conte return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); } + if( f_rng == NULL ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + if( prv->pk_info->check_pair_func == NULL ) return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); @@ -525,7 +531,7 @@ int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_conte return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); } - return( prv->pk_info->check_pair_func( pub->pk_ctx, prv->pk_ctx ) ); + return( prv->pk_info->check_pair_func( pub->pk_ctx, prv->pk_ctx, f_rng, p_rng ) ); } /* diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 7c317c52d..864e495b3 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -154,8 +154,12 @@ static int rsa_encrypt_wrap( void *ctx, ilen, input, output ) ); } -static int rsa_check_pair_wrap( const void *pub, const void *prv ) +static int rsa_check_pair_wrap( const void *pub, const void *prv, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { + (void) f_rng; + (void) p_rng; return( mbedtls_rsa_check_pub_priv( (const mbedtls_rsa_context *) pub, (const mbedtls_rsa_context *) prv ) ); } @@ -388,10 +392,13 @@ cleanup: #endif /* MBEDTLS_ECP_RESTARTABLE */ #endif /* MBEDTLS_ECDSA_C */ -static int eckey_check_pair( const void *pub, const void *prv ) +static int eckey_check_pair( const void *pub, const void *prv, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { return( mbedtls_ecp_check_pub_priv( (const mbedtls_ecp_keypair *) pub, - (const mbedtls_ecp_keypair *) prv ) ); + (const mbedtls_ecp_keypair *) prv, + f_rng, p_rng ) ); } static void *eckey_alloc_wrap( void ) @@ -799,7 +806,9 @@ static int rsa_alt_decrypt_wrap( void *ctx, } #if defined(MBEDTLS_RSA_C) -static int rsa_alt_check_pair( const void *pub, const void *prv ) +static int rsa_alt_check_pair( const void *pub, const void *prv, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { unsigned char sig[MBEDTLS_MPI_MAX_SIZE]; unsigned char hash[32]; @@ -813,7 +822,7 @@ static int rsa_alt_check_pair( const void *pub, const void *prv ) if( ( ret = rsa_alt_sign_wrap( (void *) prv, MBEDTLS_MD_NONE, hash, sizeof( hash ), - sig, &sig_len, NULL, NULL ) ) != 0 ) + sig, &sig_len, f_rng, p_rng ) ) != 0 ) { return( ret ); } diff --git a/library/pk_wrap.h b/library/pk_wrap.h index f7f938a88..b2db63739 100644 --- a/library/pk_wrap.h +++ b/library/pk_wrap.h @@ -85,7 +85,9 @@ struct mbedtls_pk_info_t void *p_rng ); /** Check public-private key pair */ - int (*check_pair_func)( const void *pub, const void *prv ); + int (*check_pair_func)( const void *pub, const void *prv, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ); /** Allocate a new context */ void * (*ctx_alloc_func)( void ); diff --git a/programs/x509/cert_write.c b/programs/x509/cert_write.c index ff7cf9807..041f459cf 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -606,7 +606,8 @@ int main( int argc, char *argv[] ) // if( strlen( opt.issuer_crt ) ) { - if( mbedtls_pk_check_pair( &issuer_crt.MBEDTLS_PRIVATE(pk), issuer_key ) != 0 ) + if( mbedtls_pk_check_pair( &issuer_crt.MBEDTLS_PRIVATE(pk), issuer_key, + mbedtls_ctr_drbg_random, &ctr_drbg ) != 0 ) { mbedtls_printf( " failed\n ! issuer_key does not match " "issuer certificate\n\n" ); diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 573c9d430..b46cf05cf 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -177,7 +177,8 @@ void pk_psa_utils( ) /* unsupported functions: check_pair, debug */ TEST_ASSERT( mbedtls_pk_setup( &pk2, mbedtls_pk_info_from_type( MBEDTLS_PK_ECKEY ) ) == 0 ); - TEST_ASSERT( mbedtls_pk_check_pair( &pk, &pk2 ) + TEST_ASSERT( mbedtls_pk_check_pair( &pk, &pk2, + mbedtls_test_rnd_std_rand, NULL ) == MBEDTLS_ERR_PK_TYPE_MISMATCH ); TEST_ASSERT( mbedtls_pk_debug( &pk, &dbg ) == MBEDTLS_ERR_PK_TYPE_MISMATCH ); @@ -350,7 +351,9 @@ void mbedtls_pk_check_pair( char * pub_file, char * prv_file, int ret ) TEST_ASSERT( mbedtls_pk_parse_public_keyfile( &pub, pub_file ) == 0 ); TEST_ASSERT( mbedtls_pk_parse_keyfile( &prv, prv_file, NULL ) == 0 ); - TEST_ASSERT( mbedtls_pk_check_pair( &pub, &prv ) == ret ); + TEST_ASSERT( mbedtls_pk_check_pair( &pub, &prv, + mbedtls_test_rnd_std_rand, NULL ) + == ret ); #if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_PK_RSA_ALT_SUPPORT) if( mbedtls_pk_get_type( &prv ) == MBEDTLS_PK_RSA ) @@ -358,7 +361,9 @@ void mbedtls_pk_check_pair( char * pub_file, char * prv_file, int ret ) TEST_ASSERT( mbedtls_pk_setup_rsa_alt( &alt, mbedtls_pk_rsa( prv ), mbedtls_rsa_decrypt_func, mbedtls_rsa_sign_func, mbedtls_rsa_key_len_func ) == 0 ); - TEST_ASSERT( mbedtls_pk_check_pair( &pub, &alt ) == ret ); + TEST_ASSERT( mbedtls_pk_check_pair( &pub, &alt, + mbedtls_test_rnd_std_rand, NULL ) + == ret ); } #endif From 84dea01f36240592c162cd3724dc89abb25b3080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 11/22] Add RNG params to private key parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is necessary for the case where the public part of an EC keypair needs to be computed from the private part - either because it was not included (it's an optional component) or because it was compressed (a format we can't parse). This changes the API of two public functions: mbedtls_pk_parse_key() and mbedtls_pk_parse_keyfile(). Tests and programs have been adapted. Some programs use a non-secure RNG (from the test library) just to get things to compile and run; in a future commit this should be improved in order to demonstrate best practice. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 12 +++-- library/pkparse.c | 52 ++++++++++++---------- library/psa_crypto_rsa.c | 3 +- programs/fuzz/fuzz_dtlsserver.c | 4 +- programs/fuzz/fuzz_privkey.c | 4 +- programs/fuzz/fuzz_server.c | 3 +- programs/pkey/key_app.c | 5 ++- programs/pkey/key_app_writer.c | 6 ++- programs/pkey/pk_decrypt.c | 3 +- programs/pkey/pk_sign.c | 3 +- programs/pkey/rsa_sign_pss.c | 3 +- programs/ssl/dtls_server.c | 40 +++++++++-------- programs/ssl/ssl_client2.c | 4 +- programs/ssl/ssl_fork_server.c | 3 +- programs/ssl/ssl_mail_client.c | 9 +++- programs/ssl/ssl_pthread_server.c | 37 +++++++-------- programs/ssl/ssl_server.c | 39 ++++++++-------- programs/ssl/ssl_server2.c | 14 +++--- programs/x509/cert_req.c | 3 +- programs/x509/cert_write.c | 4 +- tests/suites/test_suite_pk.function | 11 +++-- tests/suites/test_suite_pkparse.function | 9 ++-- tests/suites/test_suite_pkwrite.function | 3 +- tests/suites/test_suite_ssl.function | 12 +++-- tests/suites/test_suite_x509write.function | 10 +++-- 25 files changed, 175 insertions(+), 121 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 5c7b2f646..dec511112 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -667,6 +667,8 @@ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); * The empty password is not supported. * \param pwdlen Size of the password in bytes. * Ignored if \p pwd is \c NULL. + * \param f_rng RNG function, must not be \c NULL. Used for blinding. + * \param p_rng RNG parameter * * \note On entry, ctx must be empty, either freshly initialised * with mbedtls_pk_init() or reset with mbedtls_pk_free(). If you need a @@ -677,8 +679,9 @@ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); * \return 0 if successful, or a specific PK or PEM error code */ int mbedtls_pk_parse_key( mbedtls_pk_context *ctx, - const unsigned char *key, size_t keylen, - const unsigned char *pwd, size_t pwdlen ); + const unsigned char *key, size_t keylen, + const unsigned char *pwd, size_t pwdlen, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); /** \ingroup pk_module */ /** @@ -718,6 +721,8 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, * Pass a null-terminated string if expecting an encrypted * key; a non-encrypted key will also be accepted. * The empty password is not supported. + * \param f_rng RNG function, must not be \c NULL. Used for blinding. + * \param p_rng RNG parameter * * \note On entry, ctx must be empty, either freshly initialised * with mbedtls_pk_init() or reset with mbedtls_pk_free(). If you need a @@ -728,7 +733,8 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, * \return 0 if successful, or a specific PK or PEM error code */ int mbedtls_pk_parse_keyfile( mbedtls_pk_context *ctx, - const char *path, const char *password ); + const char *path, const char *password, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); /** \ingroup pk_module */ /** diff --git a/library/pkparse.c b/library/pkparse.c index 3222ca20f..5438ee4a0 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -123,7 +123,8 @@ int mbedtls_pk_load_file( const char *path, unsigned char **buf, size_t *n ) * Load and parse a private key */ int mbedtls_pk_parse_keyfile( mbedtls_pk_context *ctx, - const char *path, const char *pwd ) + const char *path, const char *pwd, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t n; @@ -136,10 +137,10 @@ int mbedtls_pk_parse_keyfile( mbedtls_pk_context *ctx, return( ret ); if( pwd == NULL ) - ret = mbedtls_pk_parse_key( ctx, buf, n, NULL, 0 ); + ret = mbedtls_pk_parse_key( ctx, buf, n, NULL, 0, f_rng, p_rng ); else ret = mbedtls_pk_parse_key( ctx, buf, n, - (const unsigned char *) pwd, strlen( pwd ) ); + (const unsigned char *) pwd, strlen( pwd ), f_rng, p_rng ); mbedtls_platform_zeroize( buf, n ); mbedtls_free( buf ); @@ -859,8 +860,8 @@ cleanup: * Parse a SEC1 encoded private EC key */ static int pk_parse_key_sec1_der( mbedtls_ecp_keypair *eck, - const unsigned char *key, - size_t keylen ) + const unsigned char *key, size_t keylen, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int version, pubkey_done; @@ -967,7 +968,7 @@ static int pk_parse_key_sec1_der( mbedtls_ecp_keypair *eck, if( ! pubkey_done && ( ret = mbedtls_ecp_mul( &eck->grp, &eck->Q, &eck->d, &eck->grp.G, - NULL, NULL ) ) != 0 ) + f_rng, p_rng ) ) != 0 ) { mbedtls_ecp_keypair_free( eck ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret ) ); @@ -997,9 +998,9 @@ static int pk_parse_key_sec1_der( mbedtls_ecp_keypair *eck, * */ static int pk_parse_key_pkcs8_unencrypted_der( - mbedtls_pk_context *pk, - const unsigned char* key, - size_t keylen ) + mbedtls_pk_context *pk, + const unsigned char* key, size_t keylen, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret, version; size_t len; @@ -1071,7 +1072,7 @@ static int pk_parse_key_pkcs8_unencrypted_der( if( pk_alg == MBEDTLS_PK_ECKEY || pk_alg == MBEDTLS_PK_ECKEY_DH ) { if( ( ret = pk_use_ecparams( ¶ms, &mbedtls_pk_ec( *pk )->grp ) ) != 0 || - ( ret = pk_parse_key_sec1_der( mbedtls_pk_ec( *pk ), p, len ) ) != 0 ) + ( ret = pk_parse_key_sec1_der( mbedtls_pk_ec( *pk ), p, len, f_rng, p_rng ) ) != 0 ) { mbedtls_pk_free( pk ); return( ret ); @@ -1094,9 +1095,10 @@ static int pk_parse_key_pkcs8_unencrypted_der( */ #if defined(MBEDTLS_PKCS12_C) || defined(MBEDTLS_PKCS5_C) static int pk_parse_key_pkcs8_encrypted_der( - mbedtls_pk_context *pk, - unsigned char *key, size_t keylen, - const unsigned char *pwd, size_t pwdlen ) + mbedtls_pk_context *pk, + unsigned char *key, size_t keylen, + const unsigned char *pwd, size_t pwdlen, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret, decrypted = 0; size_t len; @@ -1206,7 +1208,7 @@ static int pk_parse_key_pkcs8_encrypted_der( if( decrypted == 0 ) return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); - return( pk_parse_key_pkcs8_unencrypted_der( pk, buf, len ) ); + return( pk_parse_key_pkcs8_unencrypted_der( pk, buf, len, f_rng, p_rng ) ); } #endif /* MBEDTLS_PKCS12_C || MBEDTLS_PKCS5_C */ @@ -1215,7 +1217,8 @@ static int pk_parse_key_pkcs8_encrypted_der( */ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, const unsigned char *key, size_t keylen, - const unsigned char *pwd, size_t pwdlen ) + const unsigned char *pwd, size_t pwdlen, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const mbedtls_pk_info_t *pk_info; @@ -1278,7 +1281,8 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, if( ( ret = mbedtls_pk_setup( pk, pk_info ) ) != 0 || ( ret = pk_parse_key_sec1_der( mbedtls_pk_ec( *pk ), - pem.buf, pem.buflen ) ) != 0 ) + pem.buf, pem.buflen, + f_rng, p_rng ) ) != 0 ) { mbedtls_pk_free( pk ); } @@ -1305,7 +1309,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, if( ret == 0 ) { if( ( ret = pk_parse_key_pkcs8_unencrypted_der( pk, - pem.buf, pem.buflen ) ) != 0 ) + pem.buf, pem.buflen, f_rng, p_rng ) ) != 0 ) { mbedtls_pk_free( pk ); } @@ -1327,9 +1331,8 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, key, NULL, 0, &len ); if( ret == 0 ) { - if( ( ret = pk_parse_key_pkcs8_encrypted_der( pk, - pem.buf, pem.buflen, - pwd, pwdlen ) ) != 0 ) + if( ( ret = pk_parse_key_pkcs8_encrypted_der( pk, pem.buf, pem.buflen, + pwd, pwdlen, f_rng, p_rng ) ) != 0 ) { mbedtls_pk_free( pk ); } @@ -1362,7 +1365,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, memcpy( key_copy, key, keylen ); ret = pk_parse_key_pkcs8_encrypted_der( pk, key_copy, keylen, - pwd, pwdlen ); + pwd, pwdlen, f_rng, p_rng ); mbedtls_platform_zeroize( key_copy, keylen ); mbedtls_free( key_copy ); @@ -1380,8 +1383,11 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, } #endif /* MBEDTLS_PKCS12_C || MBEDTLS_PKCS5_C */ - if( ( ret = pk_parse_key_pkcs8_unencrypted_der( pk, key, keylen ) ) == 0 ) + if( ( ret = pk_parse_key_pkcs8_unencrypted_der( + pk, key, keylen, f_rng, p_rng ) ) == 0 ) + { return( 0 ); + } mbedtls_pk_free( pk ); mbedtls_pk_init( pk ); @@ -1403,7 +1409,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_ECKEY ); if( mbedtls_pk_setup( pk, pk_info ) == 0 && pk_parse_key_sec1_der( mbedtls_pk_ec( *pk ), - key, keylen ) == 0 ) + key, keylen, f_rng, p_rng ) == 0 ) { return( 0 ); } diff --git a/library/psa_crypto_rsa.c b/library/psa_crypto_rsa.c index f2e9a1c05..ef2adc134 100644 --- a/library/psa_crypto_rsa.c +++ b/library/psa_crypto_rsa.c @@ -108,7 +108,8 @@ psa_status_t mbedtls_psa_rsa_load_representation( /* Parse the data. */ if( PSA_KEY_TYPE_IS_KEY_PAIR( type ) ) status = mbedtls_to_psa_error( - mbedtls_pk_parse_key( &ctx, data, data_length, NULL, 0 ) ); + mbedtls_pk_parse_key( &ctx, data, data_length, NULL, 0, + mbedtls_psa_get_random, MBEDTLS_PSA_RANDOM_STATE ) ); else status = mbedtls_to_psa_error( mbedtls_pk_parse_public_key( &ctx, data, data_length ) ); diff --git a/programs/fuzz/fuzz_dtlsserver.c b/programs/fuzz/fuzz_dtlsserver.c index 34ff63ede..a64eef979 100644 --- a/programs/fuzz/fuzz_dtlsserver.c +++ b/programs/fuzz/fuzz_dtlsserver.c @@ -6,6 +6,7 @@ #include "common.h" #include "mbedtls/ssl.h" #include "test/certs.h" +#include "test/random.h" #if defined(MBEDTLS_SSL_PROTO_DTLS) #include "mbedtls/entropy.h" #include "mbedtls/ctr_drbg.h" @@ -55,7 +56,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { mbedtls_test_cas_pem_len ) != 0) return 1; if (mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, - mbedtls_test_srv_key_len, NULL, 0 ) != 0) + mbedtls_test_srv_key_len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ) != 0) return 1; #endif dummy_init(); diff --git a/programs/fuzz/fuzz_privkey.c b/programs/fuzz/fuzz_privkey.c index f76afd1c5..a06187562 100644 --- a/programs/fuzz/fuzz_privkey.c +++ b/programs/fuzz/fuzz_privkey.c @@ -3,6 +3,7 @@ #include #include #include "mbedtls/pk.h" +#include "test/random.h" //4 Kb should be enough for every bug ;-) #define MAX_LEN 0x1000 @@ -19,7 +20,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { } mbedtls_pk_init( &pk ); - ret = mbedtls_pk_parse_key( &pk, Data, Size, NULL, 0 ); + ret = mbedtls_pk_parse_key( &pk, Data, Size, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ); if (ret == 0) { #if defined(MBEDTLS_RSA_C) if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_RSA ) diff --git a/programs/fuzz/fuzz_server.c b/programs/fuzz/fuzz_server.c index 5480e3e87..d4480c5c8 100644 --- a/programs/fuzz/fuzz_server.c +++ b/programs/fuzz/fuzz_server.c @@ -66,7 +66,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { mbedtls_test_cas_pem_len ) != 0) return 1; if (mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, - mbedtls_test_srv_key_len, NULL, 0 ) != 0) + mbedtls_test_srv_key_len, NULL, 0, + mbedtls_ctr_drbg_random, &ctr_drbg ) != 0) return 1; #endif diff --git a/programs/pkey/key_app.c b/programs/pkey/key_app.c index 7bd93c72b..0e30be4b2 100644 --- a/programs/pkey/key_app.c +++ b/programs/pkey/key_app.c @@ -40,6 +40,8 @@ #include "mbedtls/rsa.h" #include "mbedtls/pk.h" +#include "test/random.h" + #include #endif @@ -181,7 +183,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n . Loading the private key ..." ); fflush( stdout ); - ret = mbedtls_pk_parse_keyfile( &pk, opt.filename, opt.password ); + ret = mbedtls_pk_parse_keyfile( &pk, opt.filename, opt.password, + mbedtls_test_rnd_std_rand, NULL ); if( ret != 0 ) { diff --git a/programs/pkey/key_app_writer.c b/programs/pkey/key_app_writer.c index 4b65262d0..c7f974118 100644 --- a/programs/pkey/key_app_writer.c +++ b/programs/pkey/key_app_writer.c @@ -39,6 +39,8 @@ #include "mbedtls/pk.h" #include "mbedtls/error.h" +#include "test/random.h" + #include #include #endif @@ -292,8 +294,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n . Loading the private key ..." ); fflush( stdout ); - ret = mbedtls_pk_parse_keyfile( &key, opt.filename, NULL ); - + ret = mbedtls_pk_parse_keyfile( &key, opt.filename, NULL, + mbedtls_test_rnd_std_rand, NULL ); if( ret != 0 ) { mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); diff --git a/programs/pkey/pk_decrypt.c b/programs/pkey/pk_decrypt.c index 810d6fb3e..e01f5d558 100644 --- a/programs/pkey/pk_decrypt.c +++ b/programs/pkey/pk_decrypt.c @@ -106,7 +106,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n . Reading private key from '%s'", argv[1] ); fflush( stdout ); - if( ( ret = mbedtls_pk_parse_keyfile( &pk, argv[1], "" ) ) != 0 ) + if( ( ret = mbedtls_pk_parse_keyfile( &pk, argv[1], "", + mbedtls_ctr_drbg_random, &ctr_drbg ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_keyfile returned -0x%04x\n", (unsigned int) -ret ); goto exit; diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 451e3de9b..422fa257e 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -101,7 +101,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n . Reading private key from '%s'", argv[1] ); fflush( stdout ); - if( ( ret = mbedtls_pk_parse_keyfile( &pk, argv[1], "" ) ) != 0 ) + if( ( ret = mbedtls_pk_parse_keyfile( &pk, argv[1], "", + mbedtls_ctr_drbg_random, &ctr_drbg ) ) != 0 ) { mbedtls_printf( " failed\n ! Could not parse '%s'\n", argv[1] ); goto exit; diff --git a/programs/pkey/rsa_sign_pss.c b/programs/pkey/rsa_sign_pss.c index 26056dd9b..bbbe0a9bd 100644 --- a/programs/pkey/rsa_sign_pss.c +++ b/programs/pkey/rsa_sign_pss.c @@ -102,7 +102,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n . Reading private key from '%s'", argv[1] ); fflush( stdout ); - if( ( ret = mbedtls_pk_parse_keyfile( &pk, argv[1], "" ) ) != 0 ) + if( ( ret = mbedtls_pk_parse_keyfile( &pk, argv[1], "", + mbedtls_ctr_drbg_random, &ctr_drbg ) ) != 0 ) { mbedtls_printf( " failed\n ! Could not read key from '%s'\n", argv[1] ); mbedtls_printf( " ! mbedtls_pk_parse_public_keyfile returned %d\n\n", ret ); diff --git a/programs/ssl/dtls_server.c b/programs/ssl/dtls_server.c index de47aab25..857671ff4 100644 --- a/programs/ssl/dtls_server.c +++ b/programs/ssl/dtls_server.c @@ -79,7 +79,9 @@ int main( void ) #include "mbedtls/error.h" #include "mbedtls/debug.h" #include "mbedtls/timing.h" + #include "test/certs.h" +#include "test/random.h" #if defined(MBEDTLS_SSL_CACHE_C) #include "mbedtls/ssl_cache.h" @@ -138,7 +140,23 @@ int main( void ) #endif /* - * 1. Load the certificates and private RSA key + * 1. Seed the RNG + */ + printf( " . Seeding the random number generator..." ); + fflush( stdout ); + + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, + (const unsigned char *) pers, + strlen( pers ) ) ) != 0 ) + { + printf( " failed\n ! mbedtls_ctr_drbg_seed returned %d\n", ret ); + goto exit; + } + + printf( " ok\n" ); + + /* + * 2. Load the certificates and private RSA key */ printf( "\n . Loading the server cert. and key..." ); fflush( stdout ); @@ -165,7 +183,7 @@ int main( void ) } ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, - mbedtls_test_srv_key_len, NULL, 0 ); + mbedtls_test_srv_key_len, NULL, 0, mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { printf( " failed\n ! mbedtls_pk_parse_key returned %d\n\n", ret ); @@ -175,7 +193,7 @@ int main( void ) printf( " ok\n" ); /* - * 2. Setup the "listening" UDP socket + * 3. Setup the "listening" UDP socket */ printf( " . Bind on udp/*/4433 ..." ); fflush( stdout ); @@ -188,22 +206,6 @@ int main( void ) printf( " ok\n" ); - /* - * 3. Seed the RNG - */ - printf( " . Seeding the random number generator..." ); - fflush( stdout ); - - if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, - (const unsigned char *) pers, - strlen( pers ) ) ) != 0 ) - { - printf( " failed\n ! mbedtls_ctr_drbg_seed returned %d\n", ret ); - goto exit; - } - - printf( " ok\n" ); - /* * 4. Setup stuff */ diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 98a304868..6501c4927 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1548,12 +1548,12 @@ int main( int argc, char *argv[] ) else #if defined(MBEDTLS_FS_IO) if( strlen( opt.key_file ) ) - ret = mbedtls_pk_parse_keyfile( &pkey, opt.key_file, opt.key_pwd ); + ret = mbedtls_pk_parse_keyfile( &pkey, opt.key_file, opt.key_pwd, rng_get, &rng ); else #endif ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_cli_key, - mbedtls_test_cli_key_len, NULL, 0 ); + mbedtls_test_cli_key_len, NULL, 0, rng_get, &rng ); if( ret != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned -0x%x\n\n", diff --git a/programs/ssl/ssl_fork_server.c b/programs/ssl/ssl_fork_server.c index 573210870..74190103e 100644 --- a/programs/ssl/ssl_fork_server.c +++ b/programs/ssl/ssl_fork_server.c @@ -166,7 +166,8 @@ int main( void ) } ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, - mbedtls_test_srv_key_len, NULL, 0 ); + mbedtls_test_srv_key_len, NULL, 0, + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { mbedtls_printf( " failed! mbedtls_pk_parse_key returned %d\n\n", ret ); diff --git a/programs/ssl/ssl_mail_client.c b/programs/ssl/ssl_mail_client.c index 09bbc3d69..f223977a8 100644 --- a/programs/ssl/ssl_mail_client.c +++ b/programs/ssl/ssl_mail_client.c @@ -556,12 +556,17 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_FS_IO) if( strlen( opt.key_file ) ) - ret = mbedtls_pk_parse_keyfile( &pkey, opt.key_file, "" ); + { + ret = mbedtls_pk_parse_keyfile( &pkey, opt.key_file, "", + mbedtls_ctr_drbg_random, &ctr_drbg ); + } else #endif #if defined(MBEDTLS_PEM_PARSE_C) + { ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_cli_key, - mbedtls_test_cli_key_len, NULL, 0 ); + mbedtls_test_cli_key_len, NULL, 0, mbedtls_ctr_drbg_random, &ctr_drbg ); + } #else { mbedtls_printf("MBEDTLS_PEM_PARSE_C not defined."); diff --git a/programs/ssl/ssl_pthread_server.c b/programs/ssl/ssl_pthread_server.c index 93eab4620..a083e4b64 100644 --- a/programs/ssl/ssl_pthread_server.c +++ b/programs/ssl/ssl_pthread_server.c @@ -360,7 +360,23 @@ int main( void ) mbedtls_entropy_init( &entropy ); /* - * 1. Load the certificates and private RSA key + * 1a. Seed the random number generator + */ + mbedtls_printf( " . Seeding the random number generator..." ); + + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, + (const unsigned char *) pers, + strlen( pers ) ) ) != 0 ) + { + mbedtls_printf( " failed: mbedtls_ctr_drbg_seed returned -0x%04x\n", + ( unsigned int ) -ret ); + goto exit; + } + + mbedtls_printf( " ok\n" ); + + /* + * 1b. Load the certificates and private RSA key */ mbedtls_printf( "\n . Loading the server cert. and key..." ); fflush( stdout ); @@ -388,7 +404,8 @@ int main( void ) mbedtls_pk_init( &pkey ); ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, - mbedtls_test_srv_key_len, NULL, 0 ); + mbedtls_test_srv_key_len, NULL, 0, + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned %d\n\n", ret ); @@ -397,22 +414,6 @@ int main( void ) mbedtls_printf( " ok\n" ); - /* - * 1b. Seed the random number generator - */ - mbedtls_printf( " . Seeding the random number generator..." ); - - if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, - (const unsigned char *) pers, - strlen( pers ) ) ) != 0 ) - { - mbedtls_printf( " failed: mbedtls_ctr_drbg_seed returned -0x%04x\n", - ( unsigned int ) -ret ); - goto exit; - } - - mbedtls_printf( " ok\n" ); - /* * 1c. Prepare SSL configuration */ diff --git a/programs/ssl/ssl_server.c b/programs/ssl/ssl_server.c index 42196ffc0..aaccb58ec 100644 --- a/programs/ssl/ssl_server.c +++ b/programs/ssl/ssl_server.c @@ -125,7 +125,23 @@ int main( void ) #endif /* - * 1. Load the certificates and private RSA key + * 1. Seed the RNG + */ + mbedtls_printf( " . Seeding the random number generator..." ); + fflush( stdout ); + + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, + (const unsigned char *) pers, + strlen( pers ) ) ) != 0 ) + { + mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned %d\n", ret ); + goto exit; + } + + mbedtls_printf( " ok\n" ); + + /* + * 2. Load the certificates and private RSA key */ mbedtls_printf( "\n . Loading the server cert. and key..." ); fflush( stdout ); @@ -152,7 +168,8 @@ int main( void ) } ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, - mbedtls_test_srv_key_len, NULL, 0 ); + mbedtls_test_srv_key_len, NULL, 0, + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned %d\n\n", ret ); @@ -162,7 +179,7 @@ int main( void ) mbedtls_printf( " ok\n" ); /* - * 2. Setup the listening TCP socket + * 3. Setup the listening TCP socket */ mbedtls_printf( " . Bind on https://localhost:4433/ ..." ); fflush( stdout ); @@ -175,22 +192,6 @@ int main( void ) mbedtls_printf( " ok\n" ); - /* - * 3. Seed the RNG - */ - mbedtls_printf( " . Seeding the random number generator..." ); - fflush( stdout ); - - if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, - (const unsigned char *) pers, - strlen( pers ) ) ) != 0 ) - { - mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned %d\n", ret ); - goto exit; - } - - mbedtls_printf( " ok\n" ); - /* * 4. Setup stuff */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index de4eb6d87..37f4348ed 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -20,6 +20,7 @@ #define MBEDTLS_ALLOW_PRIVATE_ACCESS #include "ssl_test_lib.h" +#include "test/random.h" #if defined(MBEDTLS_SSL_TEST_IMPOSSIBLE) int main( void ) @@ -727,7 +728,8 @@ sni_entry *sni_parse( char *sni_string ) mbedtls_pk_init( new->key ); if( mbedtls_x509_crt_parse_file( new->cert, crt_file ) != 0 || - mbedtls_pk_parse_keyfile( new->key, key_file, "" ) != 0 ) + mbedtls_pk_parse_keyfile( new->key, key_file, "", + mbedtls_test_rnd_std_rand, NULL ) != 0 ) goto error; if( strcmp( ca_file, "-" ) != 0 ) @@ -2257,7 +2259,7 @@ int main( int argc, char *argv[] ) { key_cert_init++; if( ( ret = mbedtls_pk_parse_keyfile( &pkey, opt.key_file, - opt.key_pwd ) ) != 0 ) + opt.key_pwd, rng_get, &rng ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_keyfile returned -0x%x\n\n", (unsigned int) -ret ); goto exit; @@ -2283,7 +2285,7 @@ int main( int argc, char *argv[] ) { key_cert_init2++; if( ( ret = mbedtls_pk_parse_keyfile( &pkey2, opt.key_file2, - opt.key_pwd2 ) ) != 0 ) + opt.key_pwd2, rng_get, &rng ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_keyfile(2) returned -0x%x\n\n", (unsigned int) -ret ); @@ -2314,7 +2316,8 @@ int main( int argc, char *argv[] ) } if( ( ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key_rsa, - mbedtls_test_srv_key_rsa_len, NULL, 0 ) ) != 0 ) + mbedtls_test_srv_key_rsa_len, NULL, 0, + rng_get, &rng ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned -0x%x\n\n", (unsigned int) -ret ); @@ -2333,7 +2336,8 @@ int main( int argc, char *argv[] ) } if( ( ret = mbedtls_pk_parse_key( &pkey2, (const unsigned char *) mbedtls_test_srv_key_ec, - mbedtls_test_srv_key_ec_len, NULL, 0 ) ) != 0 ) + mbedtls_test_srv_key_ec_len, NULL, 0, + rng_get, &rng ) ) != 0 ) { mbedtls_printf( " failed\n ! pk_parse_key2 returned -0x%x\n\n", (unsigned int) -ret ); diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index e8241a320..09414583e 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -346,7 +346,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( " . Loading the private key ..." ); fflush( stdout ); - ret = mbedtls_pk_parse_keyfile( &key, opt.filename, opt.password ); + ret = mbedtls_pk_parse_keyfile( &key, opt.filename, opt.password, + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { diff --git a/programs/x509/cert_write.c b/programs/x509/cert_write.c index 041f459cf..4b8fba968 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -577,7 +577,7 @@ int main( int argc, char *argv[] ) fflush( stdout ); ret = mbedtls_pk_parse_keyfile( &loaded_subject_key, opt.subject_key, - opt.subject_pwd ); + opt.subject_pwd, mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { mbedtls_strerror( ret, buf, 1024 ); @@ -593,7 +593,7 @@ int main( int argc, char *argv[] ) fflush( stdout ); ret = mbedtls_pk_parse_keyfile( &loaded_issuer_key, opt.issuer_key, - opt.issuer_pwd ); + opt.issuer_pwd, mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { mbedtls_strerror( ret, buf, 1024 ); diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index b46cf05cf..5ccb072e7 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -279,7 +279,8 @@ void valid_parameters( ) MBEDTLS_ERR_PK_BAD_INPUT_DATA ); #if defined(MBEDTLS_PK_PARSE_C) - TEST_ASSERT( mbedtls_pk_parse_key( &pk, NULL, 0, NULL, 1 ) == + TEST_ASSERT( mbedtls_pk_parse_key( &pk, NULL, 0, NULL, 1, + mbedtls_test_rnd_std_rand, NULL ) == MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); TEST_ASSERT( mbedtls_pk_parse_public_key( &pk, NULL, 0 ) == @@ -296,8 +297,8 @@ void valid_parameters_pkwrite( data_t *key_data ) /* For the write tests to be effective, we need a valid key pair. */ mbedtls_pk_init( &pk ); TEST_ASSERT( mbedtls_pk_parse_key( &pk, - key_data->x, key_data->len, - NULL, 0 ) == 0 ); + key_data->x, key_data->len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ) == 0 ); TEST_ASSERT( mbedtls_pk_write_key_der( &pk, NULL, 0 ) == MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); @@ -349,7 +350,9 @@ void mbedtls_pk_check_pair( char * pub_file, char * prv_file, int ret ) mbedtls_pk_init( &alt ); TEST_ASSERT( mbedtls_pk_parse_public_keyfile( &pub, pub_file ) == 0 ); - TEST_ASSERT( mbedtls_pk_parse_keyfile( &prv, prv_file, NULL ) == 0 ); + TEST_ASSERT( mbedtls_pk_parse_keyfile( &prv, prv_file, NULL, + mbedtls_test_rnd_std_rand, NULL ) + == 0 ); TEST_ASSERT( mbedtls_pk_check_pair( &pub, &prv, mbedtls_test_rnd_std_rand, NULL ) diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index 4650d3311..4c7f3d2ca 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -21,7 +21,8 @@ void pk_parse_keyfile_rsa( char * key_file, char * password, int result ) if( strcmp( pwd, "NULL" ) == 0 ) pwd = NULL; - res = mbedtls_pk_parse_keyfile( &ctx, key_file, pwd ); + res = mbedtls_pk_parse_keyfile( &ctx, key_file, pwd, + mbedtls_test_rnd_std_rand, NULL ); TEST_ASSERT( res == result ); @@ -96,7 +97,8 @@ void pk_parse_keyfile_ec( char * key_file, char * password, int result ) mbedtls_pk_init( &ctx ); - res = mbedtls_pk_parse_keyfile( &ctx, key_file, password ); + res = mbedtls_pk_parse_keyfile( &ctx, key_file, password, + mbedtls_test_rnd_std_rand, NULL ); TEST_ASSERT( res == result ); @@ -120,7 +122,8 @@ void pk_parse_key( data_t * buf, int result ) mbedtls_pk_init( &pk ); - TEST_ASSERT( mbedtls_pk_parse_key( &pk, buf->x, buf->len, NULL, 0 ) == result ); + TEST_ASSERT( mbedtls_pk_parse_key( &pk, buf->x, buf->len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ) == result ); exit: mbedtls_pk_free( &pk ); diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index 2bad4ed13..d1e029abb 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -63,7 +63,8 @@ void pk_write_key_check( char * key_file ) memset( check_buf, 0, sizeof( check_buf ) ); mbedtls_pk_init( &key ); - TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL ) == 0 ); + TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL, + mbedtls_test_rnd_std_rand, NULL ) == 0 ); ret = mbedtls_pk_write_key_pem( &key, buf, sizeof( buf )); TEST_ASSERT( ret == 0 ); diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index c555d74a2..d4aad60f8 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -800,7 +800,8 @@ int mbedtls_endpoint_certificate_init( mbedtls_endpoint *ep, int pk_alg ) ret = mbedtls_pk_parse_key( &( cert->pkey ), (const unsigned char*) mbedtls_test_srv_key_rsa_der, - mbedtls_test_srv_key_rsa_der_len, NULL, 0 ); + mbedtls_test_srv_key_rsa_der_len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ); TEST_ASSERT( ret == 0 ); } else @@ -812,7 +813,8 @@ int mbedtls_endpoint_certificate_init( mbedtls_endpoint *ep, int pk_alg ) ret = mbedtls_pk_parse_key( &( cert->pkey ), (const unsigned char*) mbedtls_test_srv_key_ec_der, - mbedtls_test_srv_key_ec_der_len, NULL, 0 ); + mbedtls_test_srv_key_ec_der_len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ); TEST_ASSERT( ret == 0 ); } } @@ -827,7 +829,8 @@ int mbedtls_endpoint_certificate_init( mbedtls_endpoint *ep, int pk_alg ) ret = mbedtls_pk_parse_key( &( cert->pkey ), (const unsigned char *) mbedtls_test_cli_key_rsa_der, - mbedtls_test_cli_key_rsa_der_len, NULL, 0 ); + mbedtls_test_cli_key_rsa_der_len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ); TEST_ASSERT( ret == 0 ); } else @@ -839,7 +842,8 @@ int mbedtls_endpoint_certificate_init( mbedtls_endpoint *ep, int pk_alg ) ret = mbedtls_pk_parse_key( &( cert->pkey ), (const unsigned char *) mbedtls_test_cli_key_ec_der, - mbedtls_test_cli_key_ec_der_len, NULL, 0 ); + mbedtls_test_cli_key_ec_der_len, NULL, 0, + mbedtls_test_rnd_std_rand, NULL ); TEST_ASSERT( ret == 0 ); } } diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 44f846fd3..c9b7cf9c5 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -94,7 +94,8 @@ void x509_csr_check( char * key_file, char * cert_req_check_file, int md_type, memset( &rnd_info, 0x2a, sizeof( mbedtls_test_rnd_pseudo_info ) ); mbedtls_pk_init( &key ); - TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL ) == 0 ); + TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL, + mbedtls_test_rnd_std_rand, NULL ) == 0 ); mbedtls_x509write_csr_init( &req ); mbedtls_x509write_csr_set_md_alg( &req, md_type ); @@ -163,7 +164,8 @@ void x509_csr_check_opaque( char *key_file, int md_type, int key_usage, TEST_ASSERT( md_alg_psa != MBEDTLS_MD_NONE ); mbedtls_pk_init( &key ); - TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL ) == 0 ); + TEST_ASSERT( mbedtls_pk_parse_keyfile( &key, key_file, NULL, + mbedtls_test_rnd_std_rand, NULL ) == 0 ); TEST_ASSERT( mbedtls_pk_wrap_as_opaque( &key, &key_id, md_alg_psa ) == 0 ); mbedtls_x509write_csr_init( &req ); @@ -225,10 +227,10 @@ void x509_crt_check( char *subject_key_file, char *subject_pwd, mbedtls_x509write_crt_init( &crt ); TEST_ASSERT( mbedtls_pk_parse_keyfile( &subject_key, subject_key_file, - subject_pwd ) == 0 ); + subject_pwd, mbedtls_test_rnd_std_rand, NULL ) == 0 ); TEST_ASSERT( mbedtls_pk_parse_keyfile( &issuer_key, issuer_key_file, - issuer_pwd ) == 0 ); + issuer_pwd, mbedtls_test_rnd_std_rand, NULL ) == 0 ); #if defined(MBEDTLS_RSA_C) /* For RSA PK contexts, create a copy as an alternative RSA context. */ From d51aaad4c9172ca8fccad83b58e1f4030bc864c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 12/22] Remove config option MBEDTLS_ECP_NO_INTERNAL_RNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was used to remove the code used when mbedtls_ecp_mul() received a NULL RNG parameter. This code is no longer relevant (as the RNG may no longer be NULL) and will be unconditionally removed in the next commit. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/check_config.h | 8 ------- include/mbedtls/config.h | 22 ------------------- scripts/config.py | 1 - tests/scripts/all.sh | 39 ---------------------------------- 4 files changed, 70 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 90dee6c1a..85f7efde2 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -150,14 +150,6 @@ #error "MBEDTLS_ECP_C defined, but not all prerequisites" #endif -#if defined(MBEDTLS_ECP_C) && !( \ - defined(MBEDTLS_ECP_ALT) || \ - defined(MBEDTLS_CTR_DRBG_C) || \ - defined(MBEDTLS_HMAC_DRBG_C) || \ - defined(MBEDTLS_ECP_NO_INTERNAL_RNG)) -#error "MBEDTLS_ECP_C requires a DRBG module unless MBEDTLS_ECP_NO_INTERNAL_RNG is defined or an alternative implementation is used" -#endif - #if defined(MBEDTLS_PK_PARSE_C) && !defined(MBEDTLS_ASN1_PARSE_C) #error "MBEDTLS_PK_PARSE_C defined, but not all prerequesites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 16f8f8b35..10370f37b 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -651,28 +651,6 @@ */ #define MBEDTLS_ECP_NIST_OPTIM -/** - * \def MBEDTLS_ECP_NO_INTERNAL_RNG - * - * When this option is disabled, mbedtls_ecp_mul() will make use of an - * internal RNG when called with a NULL \c f_rng argument, in order to protect - * against some side-channel attacks. - * - * This protection introduces a dependency of the ECP module on one of the - * DRBG modules. For very constrained implementations that don't require this - * protection (for example, because you're only doing signature verification, - * so not manipulating any secret, or because local/physical side-channel - * attacks are outside your threat model), it might be desirable to get rid of - * that dependency. - * - * \warning Enabling this option makes some uses of ECP vulnerable to some - * side-channel attacks. Only enable it if you know that's not a problem for - * your use case. - * - * Uncomment this macro to disable some counter-measures in ECP. - */ -//#define MBEDTLS_ECP_NO_INTERNAL_RNG - /** * \def MBEDTLS_ECP_RESTARTABLE * diff --git a/scripts/config.py b/scripts/config.py index e27f32270..cbce1eb47 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -181,7 +181,6 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_DEPRECATED_WARNING', # conflicts with deprecated options 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', # influences the use of ECDH in TLS 'MBEDTLS_ECP_NO_FALLBACK', # removes internal ECP implementation - 'MBEDTLS_ECP_NO_INTERNAL_RNG', # removes a feature 'MBEDTLS_ECP_RESTARTABLE', # incompatible with USE_PSA_CRYPTO 'MBEDTLS_ENTROPY_FORCE_SHA256', # interacts with CTR_DRBG_128_BIT_KEY 'MBEDTLS_HAVE_SSE2', # hardware dependency diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 142309957..2f3573f57 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1000,7 +1000,6 @@ component_test_psa_external_rng_no_drbg_classic () { scripts/config.py unset MBEDTLS_CTR_DRBG_C scripts/config.py unset MBEDTLS_HMAC_DRBG_C scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG - scripts/config.py set MBEDTLS_ECP_NO_INTERNAL_RNG # When MBEDTLS_USE_PSA_CRYPTO is disabled and there is no DRBG, # the SSL test programs don't have an RNG and can't work. Explicitly # make them use the PSA RNG with -DMBEDTLS_TEST_USE_PSA_CRYPTO_RNG. @@ -1023,7 +1022,6 @@ component_test_psa_external_rng_no_drbg_use_psa () { scripts/config.py unset MBEDTLS_CTR_DRBG_C scripts/config.py unset MBEDTLS_HMAC_DRBG_C scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG - scripts/config.py set MBEDTLS_ECP_NO_INTERNAL_RNG make CFLAGS="$ASAN_CFLAGS -O2" LDFLAGS="$ASAN_CFLAGS" msg "test: PSA_CRYPTO_EXTERNAL_RNG minus *_DRBG, PSA crypto - main suites" @@ -1048,43 +1046,6 @@ component_test_psa_external_rng_use_psa_crypto () { if_build_succeeded tests/ssl-opt.sh -f 'Default\|opaque' } -component_test_ecp_no_internal_rng () { - msg "build: Default plus ECP_NO_INTERNAL_RNG minus DRBG modules" - scripts/config.py set MBEDTLS_ECP_NO_INTERNAL_RNG - scripts/config.py unset MBEDTLS_CTR_DRBG_C - scripts/config.py unset MBEDTLS_HMAC_DRBG_C - scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG - scripts/config.py unset MBEDTLS_PSA_CRYPTO_C # requires a DRBG - scripts/config.py unset MBEDTLS_PSA_CRYPTO_STORAGE_C # requires PSA Crypto - - CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . - make - - msg "test: ECP_NO_INTERNAL_RNG, no DRBG module" - make test - - # no SSL tests as they all depend on having a DRBG -} - -component_test_ecp_restartable_no_internal_rng () { - msg "build: Default plus ECP_RESTARTABLE and ECP_NO_INTERNAL_RNG, no DRBG" - scripts/config.py set MBEDTLS_ECP_NO_INTERNAL_RNG - scripts/config.py set MBEDTLS_ECP_RESTARTABLE - scripts/config.py unset MBEDTLS_CTR_DRBG_C - scripts/config.py unset MBEDTLS_HMAC_DRBG_C - scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG - scripts/config.py unset MBEDTLS_PSA_CRYPTO_C # requires CTR_DRBG - scripts/config.py unset MBEDTLS_PSA_CRYPTO_STORAGE_C # requires PSA Crypto - - CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . - make - - msg "test: ECP_RESTARTABLE and ECP_NO_INTERNAL_RNG, no DRBG module" - make test - - # no SSL tests as they all depend on having a DRBG -} - component_test_everest () { msg "build: Everest ECDH context (ASan build)" # ~ 6 min scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED From 7962bfaa798b09598516a81efe32a5eea2ac520b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 13/22] Remove "internal RNG" code from ECP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is no longer needed, as the RNG param is now mandatory. Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 224 -------------------------------------------------- 1 file changed, 224 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 1a78a8f32..b7a385475 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -101,16 +101,6 @@ #include "ecp_internal_alt.h" -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) -#if defined(MBEDTLS_HMAC_DRBG_C) -#include "mbedtls/hmac_drbg.h" -#elif defined(MBEDTLS_CTR_DRBG_C) -#include "mbedtls/ctr_drbg.h" -#else -#error "Invalid configuration detected. Include check_config.h to ensure that the configuration is valid." -#endif -#endif /* MBEDTLS_ECP_NO_INTERNAL_RNG */ - #if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \ !defined(inline) && !defined(__cplusplus) #define inline __inline @@ -124,144 +114,6 @@ static unsigned long add_count, dbl_count, mul_count; #endif -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) -/* - * Currently ecp_mul() takes a RNG function as an argument, used for - * side-channel protection, but it can be NULL. The initial reasoning was - * that people will pass non-NULL RNG when they care about side-channels, but - * unfortunately we have some APIs that call ecp_mul() with a NULL RNG, with - * no opportunity for the user to do anything about it. - * - * The obvious strategies for addressing that include: - * - change those APIs so that they take RNG arguments; - * - require a global RNG to be available to all crypto modules. - * - * Unfortunately those would break compatibility. So what we do instead is - * have our own internal DRBG instance, seeded from the secret scalar. - * - * The following is a light-weight abstraction layer for doing that with - * HMAC_DRBG (first choice) or CTR_DRBG. - */ - -#if defined(MBEDTLS_HMAC_DRBG_C) - -/* DRBG context type */ -typedef mbedtls_hmac_drbg_context ecp_drbg_context; - -/* DRBG context init */ -static inline void ecp_drbg_init( ecp_drbg_context *ctx ) -{ - mbedtls_hmac_drbg_init( ctx ); -} - -/* DRBG context free */ -static inline void ecp_drbg_free( ecp_drbg_context *ctx ) -{ - mbedtls_hmac_drbg_free( ctx ); -} - -/* DRBG function */ -static inline int ecp_drbg_random( void *p_rng, - unsigned char *output, size_t output_len ) -{ - return( mbedtls_hmac_drbg_random( p_rng, output, output_len ) ); -} - -/* DRBG context seeding */ -static int ecp_drbg_seed( ecp_drbg_context *ctx, - const mbedtls_mpi *secret, size_t secret_len ) -{ - int ret; - unsigned char secret_bytes[MBEDTLS_ECP_MAX_BYTES]; - /* The list starts with strong hashes */ - const mbedtls_md_type_t md_type = mbedtls_md_list()[0]; - const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( md_type ); - - if( secret_len > MBEDTLS_ECP_MAX_BYTES ) - { - ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; - goto cleanup; - } - - MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( secret, - secret_bytes, secret_len ) ); - - ret = mbedtls_hmac_drbg_seed_buf( ctx, md_info, secret_bytes, secret_len ); - -cleanup: - mbedtls_platform_zeroize( secret_bytes, secret_len ); - - return( ret ); -} - -#elif defined(MBEDTLS_CTR_DRBG_C) - -/* DRBG context type */ -typedef mbedtls_ctr_drbg_context ecp_drbg_context; - -/* DRBG context init */ -static inline void ecp_drbg_init( ecp_drbg_context *ctx ) -{ - mbedtls_ctr_drbg_init( ctx ); -} - -/* DRBG context free */ -static inline void ecp_drbg_free( ecp_drbg_context *ctx ) -{ - mbedtls_ctr_drbg_free( ctx ); -} - -/* DRBG function */ -static inline int ecp_drbg_random( void *p_rng, - unsigned char *output, size_t output_len ) -{ - return( mbedtls_ctr_drbg_random( p_rng, output, output_len ) ); -} - -/* - * Since CTR_DRBG doesn't have a seed_buf() function the way HMAC_DRBG does, - * we need to pass an entropy function when seeding. So we use a dummy - * function for that, and pass the actual entropy as customisation string. - * (During seeding of CTR_DRBG the entropy input and customisation string are - * concatenated before being used to update the secret state.) - */ -static int ecp_ctr_drbg_null_entropy(void *ctx, unsigned char *out, size_t len) -{ - (void) ctx; - memset( out, 0, len ); - return( 0 ); -} - -/* DRBG context seeding */ -static int ecp_drbg_seed( ecp_drbg_context *ctx, - const mbedtls_mpi *secret, size_t secret_len ) -{ - int ret; - unsigned char secret_bytes[MBEDTLS_ECP_MAX_BYTES]; - - if( secret_len > MBEDTLS_ECP_MAX_BYTES ) - { - ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; - goto cleanup; - } - - MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( secret, - secret_bytes, secret_len ) ); - - ret = mbedtls_ctr_drbg_seed( ctx, ecp_ctr_drbg_null_entropy, NULL, - secret_bytes, secret_len ); - -cleanup: - mbedtls_platform_zeroize( secret_bytes, secret_len ); - - return( ret ); -} - -#else -#error "Invalid configuration detected. Include check_config.h to ensure that the configuration is valid." -#endif /* DRBG modules */ -#endif /* MBEDTLS_ECP_NO_INTERNAL_RNG */ - #if defined(MBEDTLS_ECP_RESTARTABLE) /* * Maximum number of "basic operations" to be done in a row. @@ -309,10 +161,6 @@ struct mbedtls_ecp_restart_mul ecp_rsm_comb_core, /* ecp_mul_comb_core() */ ecp_rsm_final_norm, /* do the final normalization */ } state; -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_context drbg_ctx; - unsigned char drbg_seeded; -#endif }; /* @@ -325,10 +173,6 @@ static void ecp_restart_rsm_init( mbedtls_ecp_restart_mul_ctx *ctx ) ctx->T = NULL; ctx->T_size = 0; ctx->state = ecp_rsm_init; -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_init( &ctx->drbg_ctx ); - ctx->drbg_seeded = 0; -#endif } /* @@ -350,10 +194,6 @@ static void ecp_restart_rsm_free( mbedtls_ecp_restart_mul_ctx *ctx ) mbedtls_free( ctx->T ); } -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_free( &ctx->drbg_ctx ); -#endif - ecp_restart_rsm_init( ctx ); } @@ -2068,9 +1908,7 @@ static int ecp_mul_comb_core( const mbedtls_ecp_group *grp, mbedtls_ecp_point *R i = d; MBEDTLS_MPI_CHK( ecp_select_comb( grp, R, T, T_size, x[i] ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &R->Z, 1 ) ); -#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != 0 ) -#endif MBEDTLS_MPI_CHK( ecp_randomize_jac( grp, R, f_rng, p_rng ) ); } @@ -2204,9 +2042,7 @@ final_norm: * * Avoid the leak by randomizing coordinates before we normalize them. */ -#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != 0 ) -#endif MBEDTLS_MPI_CHK( ecp_randomize_jac( grp, RR, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( ecp_normalize_jac( grp, RR ) ); @@ -2286,42 +2122,9 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, size_t d; unsigned char T_size = 0, T_ok = 0; mbedtls_ecp_point *T = NULL; -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_context drbg_ctx; - - ecp_drbg_init( &drbg_ctx ); -#endif ECP_RS_ENTER( rsm ); -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - if( f_rng == NULL ) - { - /* Adjust pointers */ - f_rng = &ecp_drbg_random; -#if defined(MBEDTLS_ECP_RESTARTABLE) - if( rs_ctx != NULL && rs_ctx->rsm != NULL ) - p_rng = &rs_ctx->rsm->drbg_ctx; - else -#endif - p_rng = &drbg_ctx; - - /* Initialize internal DRBG if necessary */ -#if defined(MBEDTLS_ECP_RESTARTABLE) - if( rs_ctx == NULL || rs_ctx->rsm == NULL || - rs_ctx->rsm->drbg_seeded == 0 ) -#endif - { - const size_t m_len = ( grp->nbits + 7 ) / 8; - MBEDTLS_MPI_CHK( ecp_drbg_seed( p_rng, m, m_len ) ); - } -#if defined(MBEDTLS_ECP_RESTARTABLE) - if( rs_ctx != NULL && rs_ctx->rsm != NULL ) - rs_ctx->rsm->drbg_seeded = 1; -#endif - } -#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ - /* Is P the base point ? */ #if MBEDTLS_ECP_FIXED_POINT_OPTIM == 1 p_eq_g = ( mbedtls_mpi_cmp_mpi( &P->Y, &grp->G.Y ) == 0 && @@ -2393,10 +2196,6 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, cleanup: -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_free( &drbg_ctx ); -#endif - /* does T belong to the group? */ if( T == grp->T ) T = NULL; @@ -2583,23 +2382,8 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, unsigned char b; mbedtls_ecp_point RP; mbedtls_mpi PX; -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_context drbg_ctx; - - ecp_drbg_init( &drbg_ctx ); -#endif mbedtls_ecp_point_init( &RP ); mbedtls_mpi_init( &PX ); -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - if( f_rng == NULL ) - { - const size_t m_len = ( grp->nbits + 7 ) / 8; - MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m, m_len ) ); - f_rng = &ecp_drbg_random; - p_rng = &drbg_ctx; - } -#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ - /* Save PX and read from P before writing to R, in case P == R */ MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &PX, &P->X ) ); MBEDTLS_MPI_CHK( mbedtls_ecp_copy( &RP, P ) ); @@ -2613,9 +2397,7 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MOD_ADD( RP.X ); /* Randomize coordinates of the starting point */ -#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != NULL ) -#endif MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, &RP, f_rng, p_rng ) ); /* Loop invariant: R = result so far, RP = R + P */ @@ -2648,18 +2430,12 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, * * Avoid the leak by randomizing coordinates before we normalize them. */ -#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != NULL ) -#endif MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, R, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( ecp_normalize_mxz( grp, R ) ); cleanup: -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_free( &drbg_ctx ); -#endif - mbedtls_ecp_point_free( &RP ); mbedtls_mpi_free( &PX ); return( ret ); From 02b5705aa37613d83c97aa2f84b8d66b10b60d3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 14/22] Simplify internal code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We know that Montgomery multiplication will never be called without an RNG, so make that clear from the beginning of the function. Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index b7a385475..8f6e9886b 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2384,6 +2384,9 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, mbedtls_mpi PX; mbedtls_ecp_point_init( &RP ); mbedtls_mpi_init( &PX ); + if( f_rng == NULL ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + /* Save PX and read from P before writing to R, in case P == R */ MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &PX, &P->X ) ); MBEDTLS_MPI_CHK( mbedtls_ecp_copy( &RP, P ) ); @@ -2397,8 +2400,7 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MOD_ADD( RP.X ); /* Randomize coordinates of the starting point */ - if( f_rng != NULL ) - MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, &RP, f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, &RP, f_rng, p_rng ) ); /* Loop invariant: R = result so far, RP = R + P */ i = mbedtls_mpi_bitlen( m ); /* one past the (zero-based) most significant bit */ @@ -2430,9 +2432,7 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, * * Avoid the leak by randomizing coordinates before we normalize them. */ - if( f_rng != NULL ) - MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, R, f_rng, p_rng ) ); - + MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, R, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( ecp_normalize_mxz( grp, R ) ); cleanup: From e6e51aab55fd002ddc89f5b1b2579f768c6eb606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:29:26 +0200 Subject: [PATCH 15/22] Add ChangeLog and migration guide entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge part of the RSA entries into this one, as I think it's easier for users to have all similar changes in one place regardless of whether they were introduce in the same PR or not. Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/mandatory-rng-param.txt | 12 +++++++ ChangeLog.d/remove-rsa-mode-parameter.txt | 1 - .../mandatory-rng-param.md | 36 +++++++++++++++++++ .../remove-rsa-mode-parameter.md | 8 ----- 4 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 ChangeLog.d/mandatory-rng-param.txt create mode 100644 docs/3.0-migration-guide.d/mandatory-rng-param.md diff --git a/ChangeLog.d/mandatory-rng-param.txt b/ChangeLog.d/mandatory-rng-param.txt new file mode 100644 index 000000000..4e04248fa --- /dev/null +++ b/ChangeLog.d/mandatory-rng-param.txt @@ -0,0 +1,12 @@ +API changes + * For all functions that take an RNG parameter, this parameter is now + mandatory (that is, NULL is not an acceptable value). Functions which + previously accepted NULL and now reject it are: the X.509 CRT and CSR + writing functions; the PK sign and decrypt function; the RSA encrypt, + decrypt, sign and private functions; the function in DHM and ECDH that + compute the share secret; the scalar multiplication functions in ECP. + * The following functions now require an RNG parameter: + mbedtls_ecp_check_pub_priv(), mbedtls_pk_check_pair(), + mbedtls_pk_parse_key(), mbedtls_pk_parse_keyfile(). + * The configuration option MBEDTLS_ECP_NO_INTERNAL_RNG has been removed as + it no longer had any effect. diff --git a/ChangeLog.d/remove-rsa-mode-parameter.txt b/ChangeLog.d/remove-rsa-mode-parameter.txt index 854dda34b..2590d3a94 100644 --- a/ChangeLog.d/remove-rsa-mode-parameter.txt +++ b/ChangeLog.d/remove-rsa-mode-parameter.txt @@ -6,4 +6,3 @@ API changes decryption functions now always use the private key and verification and encryption use the public key. Verification functions also no longer have RNG parameters. - * The RNG is now mandatory for all private-key RSA operations. diff --git a/docs/3.0-migration-guide.d/mandatory-rng-param.md b/docs/3.0-migration-guide.d/mandatory-rng-param.md new file mode 100644 index 000000000..3cbc35695 --- /dev/null +++ b/docs/3.0-migration-guide.d/mandatory-rng-param.md @@ -0,0 +1,36 @@ +The RNG parameter is now mandatory for all functions that accept one +-------------------------------------------------------------------- + +This change affects all users who called a function accepting a `f_rng` +parameter with `NULL` as the value of this argument; this is no longer +supported. + +The changed functions are: the X.509 CRT and CSR writing functions; the PK +sign and decrypt function; the RSA encrypt, decrypt, sign and private +functions; the functions in DHM and ECDH that compute the share secret; the +scalar multiplication functions in ECP. + +You now need to pass a properly seeded, cryptographically secure RNG to all +functions that accept a `f_rng` parameter. It is of course still possible to +pass `NULL` as the context pointer `p_rng` if your RNG function doesn't need a +context. + +Some functions gained an RNG parameter +-------------------------------------- + +This affects users of the following functions: `mbedtls_ecp_check_pub_priv()`, +`mbedtls_pk_check_pair()`, `mbedtls_pk_parse_key()`, and +`mbedtls_pk_parse_keyfile()`. + +You now need to pass a properly seeded, cryptographically secure RNG when +calling these functions. It is used for blinding, a counter-measure against +side-channel attacks. + +The configuration option `MBEDTLS_ECP_NO_INTERNAL_RNG` was removed +------------------------------------------------------------------ + +This doesn't affect users of the default configuration; it only affects people +who were explicitly setting this option. + +This was a trade-off between code size and counter-measures; it is no longer +relevant as the counter-measure is now always on at no cost in code size. diff --git a/docs/3.0-migration-guide.d/remove-rsa-mode-parameter.md b/docs/3.0-migration-guide.d/remove-rsa-mode-parameter.md index e400650dd..d21d5ed85 100644 --- a/docs/3.0-migration-guide.d/remove-rsa-mode-parameter.md +++ b/docs/3.0-migration-guide.d/remove-rsa-mode-parameter.md @@ -19,11 +19,3 @@ RSA verification functions also no longer take random generator arguments (this was only needed when using a private key). This affects all applications using the RSA verify functions. -RNG is now mandatory in all RSA private key operations ------------------------------------------------------- - -The random generator is now mandatory for blinding in all RSA private-key -operations (`mbedtls_rsa_private`, `mbedtls_rsa_xxx_sign`, -`mbedtls_rsa_xxx_decrypt`) as well as for encryption -(`mbedtls_rsa_xxx_encrypt`). This means that passing a null `f_rng` is no longer -supported. From 36a8963b3b1c073eb9420d75ec45dc59fcfcd809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jun 2021 11:43:33 +0200 Subject: [PATCH 16/22] Fix cmake build of programs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- programs/aes/CMakeLists.txt | 1 + programs/hash/CMakeLists.txt | 1 + programs/pkey/CMakeLists.txt | 2 ++ programs/random/CMakeLists.txt | 1 + programs/util/CMakeLists.txt | 1 + programs/x509/CMakeLists.txt | 1 + 6 files changed, 7 insertions(+) diff --git a/programs/aes/CMakeLists.txt b/programs/aes/CMakeLists.txt index 62a54c768..85bcd5fca 100644 --- a/programs/aes/CMakeLists.txt +++ b/programs/aes/CMakeLists.txt @@ -5,6 +5,7 @@ set(executables foreach(exe IN LISTS executables) add_executable(${exe} ${exe}.c $) target_link_libraries(${exe} ${mbedcrypto_target}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) endforeach() install(TARGETS ${executables} diff --git a/programs/hash/CMakeLists.txt b/programs/hash/CMakeLists.txt index b2f2a1f5c..729474c03 100644 --- a/programs/hash/CMakeLists.txt +++ b/programs/hash/CMakeLists.txt @@ -6,6 +6,7 @@ set(executables foreach(exe IN LISTS executables) add_executable(${exe} ${exe}.c $) target_link_libraries(${exe} ${mbedcrypto_target}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) endforeach() install(TARGETS ${executables} diff --git a/programs/pkey/CMakeLists.txt b/programs/pkey/CMakeLists.txt index 9c6fe7d49..3ad56436e 100644 --- a/programs/pkey/CMakeLists.txt +++ b/programs/pkey/CMakeLists.txt @@ -6,6 +6,7 @@ set(executables_mbedtls foreach(exe IN LISTS executables_mbedtls) add_executable(${exe} ${exe}.c $) target_link_libraries(${exe} ${mbedtls_target}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) endforeach() set(executables_mbedcrypto @@ -32,6 +33,7 @@ set(executables_mbedcrypto foreach(exe IN LISTS executables_mbedcrypto) add_executable(${exe} ${exe}.c $) target_link_libraries(${exe} ${mbedcrypto_target}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) endforeach() install(TARGETS ${executables_mbedtls} ${executables_mbedcrypto} diff --git a/programs/random/CMakeLists.txt b/programs/random/CMakeLists.txt index f32dc31ee..e5edf7b58 100644 --- a/programs/random/CMakeLists.txt +++ b/programs/random/CMakeLists.txt @@ -6,6 +6,7 @@ set(executables foreach(exe IN LISTS executables) add_executable(${exe} ${exe}.c $) target_link_libraries(${exe} ${mbedcrypto_target}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) endforeach() install(TARGETS ${executables} diff --git a/programs/util/CMakeLists.txt b/programs/util/CMakeLists.txt index 2a11212ec..7fc58cbcf 100644 --- a/programs/util/CMakeLists.txt +++ b/programs/util/CMakeLists.txt @@ -10,6 +10,7 @@ set(executables foreach(exe IN LISTS executables) add_executable(${exe} ${exe}.c $) target_link_libraries(${exe} ${libs}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) endforeach() install(TARGETS ${executables} diff --git a/programs/x509/CMakeLists.txt b/programs/x509/CMakeLists.txt index cf57ca431..a04fa8bcf 100644 --- a/programs/x509/CMakeLists.txt +++ b/programs/x509/CMakeLists.txt @@ -13,6 +13,7 @@ set(executables foreach(exe IN LISTS executables) add_executable(${exe} ${exe}.c $) target_link_libraries(${exe} ${libs}) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) endforeach() target_link_libraries(cert_app ${mbedtls_target}) From 7f93da1265c9798d81aed3d766bef6acbfe2f0fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jun 2021 10:20:30 +0200 Subject: [PATCH 17/22] Use the dedicated dummy_random in fuzzing programs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also make sure to initialize the DRBG before using it in fuzz_server (dummy_random uses ctr_drbg internally). Signed-off-by: Manuel Pégourié-Gonnard --- programs/fuzz/fuzz_dtlsserver.c | 3 +-- programs/fuzz/fuzz_privkey.c | 4 ++-- programs/fuzz/fuzz_server.c | 16 ++++++++-------- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/programs/fuzz/fuzz_dtlsserver.c b/programs/fuzz/fuzz_dtlsserver.c index a64eef979..9a6e894a8 100644 --- a/programs/fuzz/fuzz_dtlsserver.c +++ b/programs/fuzz/fuzz_dtlsserver.c @@ -6,7 +6,6 @@ #include "common.h" #include "mbedtls/ssl.h" #include "test/certs.h" -#include "test/random.h" #if defined(MBEDTLS_SSL_PROTO_DTLS) #include "mbedtls/entropy.h" #include "mbedtls/ctr_drbg.h" @@ -57,7 +56,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { return 1; if (mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, mbedtls_test_srv_key_len, NULL, 0, - mbedtls_test_rnd_std_rand, NULL ) != 0) + dummy_random, NULL ) != 0) return 1; #endif dummy_init(); diff --git a/programs/fuzz/fuzz_privkey.c b/programs/fuzz/fuzz_privkey.c index a06187562..b9a160e1e 100644 --- a/programs/fuzz/fuzz_privkey.c +++ b/programs/fuzz/fuzz_privkey.c @@ -3,7 +3,7 @@ #include #include #include "mbedtls/pk.h" -#include "test/random.h" +#include "common.h" //4 Kb should be enough for every bug ;-) #define MAX_LEN 0x1000 @@ -21,7 +21,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { mbedtls_pk_init( &pk ); ret = mbedtls_pk_parse_key( &pk, Data, Size, NULL, 0, - mbedtls_test_rnd_std_rand, NULL ); + dummy_random, NULL ); if (ret == 0) { #if defined(MBEDTLS_RSA_C) if( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_RSA ) diff --git a/programs/fuzz/fuzz_server.c b/programs/fuzz/fuzz_server.c index d4480c5c8..c35b42523 100644 --- a/programs/fuzz/fuzz_server.c +++ b/programs/fuzz/fuzz_server.c @@ -56,6 +56,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { options = Data[Size - 1]; if (initialized == 0) { + mbedtls_ctr_drbg_init( &ctr_drbg ); + mbedtls_entropy_init( &entropy ); + + if( mbedtls_ctr_drbg_seed( &ctr_drbg, dummy_entropy, &entropy, + (const unsigned char *) pers, strlen( pers ) ) != 0 ) + return 1; + #if defined(MBEDTLS_X509_CRT_PARSE_C) && defined(MBEDTLS_PEM_PARSE_C) mbedtls_x509_crt_init( &srvcert ); mbedtls_pk_init( &pkey ); @@ -67,7 +74,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { return 1; if (mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key, mbedtls_test_srv_key_len, NULL, 0, - mbedtls_ctr_drbg_random, &ctr_drbg ) != 0) + dummy_random, &ctr_drbg ) != 0) return 1; #endif @@ -81,17 +88,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { } mbedtls_ssl_init( &ssl ); mbedtls_ssl_config_init( &conf ); - mbedtls_ctr_drbg_init( &ctr_drbg ); - mbedtls_entropy_init( &entropy ); #if defined(MBEDTLS_SSL_SESSION_TICKETS) mbedtls_ssl_ticket_init( &ticket_ctx ); #endif - if( mbedtls_ctr_drbg_seed( &ctr_drbg, dummy_entropy, &entropy, - (const unsigned char *) pers, strlen( pers ) ) != 0 ) - goto exit; - - if( mbedtls_ssl_config_defaults( &conf, MBEDTLS_SSL_IS_SERVER, MBEDTLS_SSL_TRANSPORT_STREAM, From 1503a9adab19434dbf9daf23bfeb19468ba14411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jun 2021 10:35:56 +0200 Subject: [PATCH 18/22] Use a proper DRBG in programs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- programs/pkey/key_app.c | 36 +++++++++++++++++++++++++------- programs/pkey/key_app_writer.c | 38 ++++++++++++++++++++++++++++------ programs/ssl/dtls_server.c | 1 - programs/ssl/ssl_server2.c | 8 +++---- 4 files changed, 64 insertions(+), 19 deletions(-) diff --git a/programs/pkey/key_app.c b/programs/pkey/key_app.c index 0e30be4b2..2145e072a 100644 --- a/programs/pkey/key_app.c +++ b/programs/pkey/key_app.c @@ -35,12 +35,13 @@ #endif /* MBEDTLS_PLATFORM_C */ #if defined(MBEDTLS_BIGNUM_C) && \ - defined(MBEDTLS_PK_PARSE_C) && defined(MBEDTLS_FS_IO) + defined(MBEDTLS_PK_PARSE_C) && defined(MBEDTLS_FS_IO) && \ + defined(MBEDTLS_ENTROPY_C) && defined(MBEDTLS_CTR_DRBG_C) #include "mbedtls/error.h" #include "mbedtls/rsa.h" #include "mbedtls/pk.h" - -#include "test/random.h" +#include "mbedtls/entropy.h" +#include "mbedtls/ctr_drbg.h" #include #endif @@ -65,11 +66,13 @@ "\n" #if !defined(MBEDTLS_BIGNUM_C) || \ - !defined(MBEDTLS_PK_PARSE_C) || !defined(MBEDTLS_FS_IO) + !defined(MBEDTLS_PK_PARSE_C) || !defined(MBEDTLS_FS_IO) || \ + !defined(MBEDTLS_ENTROPY_C) || !defined(MBEDTLS_CTR_DRBG_C) int main( void ) { mbedtls_printf("MBEDTLS_BIGNUM_C and/or " - "MBEDTLS_PK_PARSE_C and/or MBEDTLS_FS_IO not defined.\n"); + "MBEDTLS_PK_PARSE_C and/or MBEDTLS_FS_IO and/or " + "MBEDTLS_ENTROPY_C and/or MBEDTLS_CTR_DRBG_C not defined.\n"); mbedtls_exit( 0 ); } #else @@ -94,12 +97,19 @@ int main( int argc, char *argv[] ) int i; char *p, *q; + const char *pers = "pkey/key_app"; + mbedtls_entropy_context entropy; + mbedtls_ctr_drbg_context ctr_drbg; + mbedtls_pk_context pk; mbedtls_mpi N, P, Q, D, E, DP, DQ, QP; /* * Set to sane values */ + mbedtls_entropy_init( &entropy ); + mbedtls_ctr_drbg_init( &ctr_drbg ); + mbedtls_pk_init( &pk ); memset( buf, 0, sizeof(buf) ); @@ -183,8 +193,16 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n . Loading the private key ..." ); fflush( stdout ); + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, + (const unsigned char *) pers, + strlen( pers ) ) ) != 0 ) + { + mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned -0x%04x\n", (unsigned int) -ret ); + goto cleanup; + } + ret = mbedtls_pk_parse_keyfile( &pk, opt.filename, opt.password, - mbedtls_test_rnd_std_rand, NULL ); + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { @@ -302,6 +320,9 @@ cleanup: } #endif + mbedtls_ctr_drbg_free( &ctr_drbg ); + mbedtls_entropy_free( &entropy ); + mbedtls_pk_free( &pk ); mbedtls_mpi_free( &N ); mbedtls_mpi_free( &P ); mbedtls_mpi_free( &Q ); mbedtls_mpi_free( &D ); mbedtls_mpi_free( &E ); mbedtls_mpi_free( &DP ); @@ -314,4 +335,5 @@ cleanup: mbedtls_exit( exit_code ); } -#endif /* MBEDTLS_BIGNUM_C && MBEDTLS_PK_PARSE_C && MBEDTLS_FS_IO */ +#endif /* MBEDTLS_BIGNUM_C && MBEDTLS_PK_PARSE_C && MBEDTLS_FS_IO && + MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C */ diff --git a/programs/pkey/key_app_writer.c b/programs/pkey/key_app_writer.c index c7f974118..89c67ed9e 100644 --- a/programs/pkey/key_app_writer.c +++ b/programs/pkey/key_app_writer.c @@ -34,12 +34,15 @@ #define MBEDTLS_EXIT_FAILURE EXIT_FAILURE #endif /* MBEDTLS_PLATFORM_C */ -#if defined(MBEDTLS_PK_WRITE_C) && defined(MBEDTLS_FS_IO) +#if defined(MBEDTLS_PK_PARSE_C) && defined(MBEDTLS_PK_WRITE_C) && \ + defined(MBEDTLS_FS_IO) && \ + defined(MBEDTLS_ENTROPY_C) && defined(MBEDTLS_CTR_DRBG_C) #include "mbedtls/error.h" #include "mbedtls/pk.h" #include "mbedtls/error.h" -#include "test/random.h" +#include "mbedtls/entropy.h" +#include "mbedtls/ctr_drbg.h" #include #include @@ -90,10 +93,14 @@ #if !defined(MBEDTLS_PK_PARSE_C) || \ !defined(MBEDTLS_PK_WRITE_C) || \ - !defined(MBEDTLS_FS_IO) + !defined(MBEDTLS_FS_IO) || \ + !defined(MBEDTLS_ENTROPY_C) || \ + !defined(MBEDTLS_CTR_DRBG_C) int main( void ) { - mbedtls_printf( "MBEDTLS_PK_PARSE_C and/or MBEDTLS_PK_WRITE_C and/or MBEDTLS_FS_IO not defined.\n" ); + mbedtls_printf( "MBEDTLS_PK_PARSE_C and/or MBEDTLS_PK_WRITE_C and/or " + "MBEDTLS_ENTROPY_C and/or MBEDTLS_CTR_DRBG_C and/or " + "MBEDTLS_FS_IO not defined.\n" ); mbedtls_exit( 0 ); } #else @@ -203,12 +210,19 @@ int main( int argc, char *argv[] ) int i; char *p, *q; + const char *pers = "pkey/key_app"; + mbedtls_entropy_context entropy; + mbedtls_ctr_drbg_context ctr_drbg; + mbedtls_pk_context key; mbedtls_mpi N, P, Q, D, E, DP, DQ, QP; /* * Set to sane values */ + mbedtls_entropy_init( &entropy ); + mbedtls_ctr_drbg_init( &ctr_drbg ); + mbedtls_pk_init( &key ); memset( buf, 0, sizeof( buf ) ); @@ -294,8 +308,16 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n . Loading the private key ..." ); fflush( stdout ); + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, + (const unsigned char *) pers, + strlen( pers ) ) ) != 0 ) + { + mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned -0x%04x\n", (unsigned int) -ret ); + goto exit; + } + ret = mbedtls_pk_parse_keyfile( &key, opt.filename, NULL, - mbedtls_test_rnd_std_rand, NULL ); + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); @@ -431,6 +453,9 @@ exit: mbedtls_pk_free( &key ); + mbedtls_ctr_drbg_free( &ctr_drbg ); + mbedtls_entropy_free( &entropy ); + #if defined(_WIN32) mbedtls_printf( " + Press Enter to exit this program.\n" ); fflush( stdout ); getchar(); @@ -438,4 +463,5 @@ exit: mbedtls_exit( exit_code ); } -#endif /* MBEDTLS_PK_PARSE_C && MBEDTLS_PK_WRITE_C && MBEDTLS_FS_IO */ +#endif /* MBEDTLS_PK_PARSE_C && MBEDTLS_PK_WRITE_C && MBEDTLS_FS_IO && + MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C */ diff --git a/programs/ssl/dtls_server.c b/programs/ssl/dtls_server.c index 857671ff4..d2cc4509d 100644 --- a/programs/ssl/dtls_server.c +++ b/programs/ssl/dtls_server.c @@ -81,7 +81,6 @@ int main( void ) #include "mbedtls/timing.h" #include "test/certs.h" -#include "test/random.h" #if defined(MBEDTLS_SSL_CACHE_C) #include "mbedtls/ssl_cache.h" diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 37f4348ed..68cc0275d 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -20,7 +20,6 @@ #define MBEDTLS_ALLOW_PRIVATE_ACCESS #include "ssl_test_lib.h" -#include "test/random.h" #if defined(MBEDTLS_SSL_TEST_IMPOSSIBLE) int main( void ) @@ -689,7 +688,7 @@ void sni_free( sni_entry *head ) * * Modifies the input string! This is not production quality! */ -sni_entry *sni_parse( char *sni_string ) +sni_entry *sni_parse( char *sni_string, rng_context_t *p_rng ) { sni_entry *cur = NULL, *new = NULL; char *p = sni_string; @@ -728,8 +727,7 @@ sni_entry *sni_parse( char *sni_string ) mbedtls_pk_init( new->key ); if( mbedtls_x509_crt_parse_file( new->cert, crt_file ) != 0 || - mbedtls_pk_parse_keyfile( new->key, key_file, "", - mbedtls_test_rnd_std_rand, NULL ) != 0 ) + mbedtls_pk_parse_keyfile( new->key, key_file, "", rng_get, p_rng ) != 0 ) goto error; if( strcmp( ca_file, "-" ) != 0 ) @@ -2373,7 +2371,7 @@ int main( int argc, char *argv[] ) mbedtls_printf( " . Setting up SNI information..." ); fflush( stdout ); - if( ( sni_info = sni_parse( opt.sni ) ) == NULL ) + if( ( sni_info = sni_parse( opt.sni, &rng ) ) == NULL ) { mbedtls_printf( " failed\n" ); goto exit; From 8707259318618bb6d1b972b470cb21b3b730a8f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jun 2021 11:02:38 +0200 Subject: [PATCH 19/22] Improve ChangeLog and migration guide entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/mandatory-rng-param.txt | 14 ++++++++------ docs/3.0-migration-guide.d/mandatory-rng-param.md | 12 ++++++++---- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/ChangeLog.d/mandatory-rng-param.txt b/ChangeLog.d/mandatory-rng-param.txt index 4e04248fa..39ee33533 100644 --- a/ChangeLog.d/mandatory-rng-param.txt +++ b/ChangeLog.d/mandatory-rng-param.txt @@ -1,12 +1,14 @@ API changes - * For all functions that take an RNG parameter, this parameter is now - mandatory (that is, NULL is not an acceptable value). Functions which - previously accepted NULL and now reject it are: the X.509 CRT and CSR - writing functions; the PK sign and decrypt function; the RSA encrypt, - decrypt, sign and private functions; the function in DHM and ECDH that - compute the share secret; the scalar multiplication functions in ECP. + * For all functions that take a random number generator (RNG) as a + parameter, this parameter is now mandatory (that is, NULL is not an + acceptable value). Functions which previously accepted NULL and now + reject it are: the X.509 CRT and CSR writing functions; the PK and RSA + sign and decrypt function; mbedtls_rsa_private(); the functions + in DHM and ECDH that compute the shared secret; the scalar multiplication + functions in ECP. * The following functions now require an RNG parameter: mbedtls_ecp_check_pub_priv(), mbedtls_pk_check_pair(), mbedtls_pk_parse_key(), mbedtls_pk_parse_keyfile(). +Removals * The configuration option MBEDTLS_ECP_NO_INTERNAL_RNG has been removed as it no longer had any effect. diff --git a/docs/3.0-migration-guide.d/mandatory-rng-param.md b/docs/3.0-migration-guide.d/mandatory-rng-param.md index 3cbc35695..f6aba08b1 100644 --- a/docs/3.0-migration-guide.d/mandatory-rng-param.md +++ b/docs/3.0-migration-guide.d/mandatory-rng-param.md @@ -5,16 +5,20 @@ This change affects all users who called a function accepting a `f_rng` parameter with `NULL` as the value of this argument; this is no longer supported. -The changed functions are: the X.509 CRT and CSR writing functions; the PK -sign and decrypt function; the RSA encrypt, decrypt, sign and private -functions; the functions in DHM and ECDH that compute the share secret; the -scalar multiplication functions in ECP. +The changed functions are: the X.509 CRT and CSR writing functions; the PK and +RSA sign and decrypt functions; `mbedtls_rsa_private()`; the functions in DHM +and ECDH that compute the shared secret; the scalar multiplication functions in +ECP. You now need to pass a properly seeded, cryptographically secure RNG to all functions that accept a `f_rng` parameter. It is of course still possible to pass `NULL` as the context pointer `p_rng` if your RNG function doesn't need a context. +Alternative implementations of a module (enabled with the `MBEDTLS_module_ALT` +configuration options) may have their own internal and are free to ignore the +`f_rng` argument but must allow users to pass one anyway. + Some functions gained an RNG parameter -------------------------------------- From 6f19ce317bd68c4656b802529f7df0ef56546d27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jun 2021 12:08:34 +0200 Subject: [PATCH 20/22] Fix async support in ssl_server2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- programs/ssl/ssl_server2.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 68cc0275d..51125bdb6 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -525,6 +525,8 @@ int main( void ) (out_be)[(i) + 7] = (unsigned char)( ( (in_le) >> 0 ) & 0xFF ); \ } +/* This is global so it can be easily accessed by callback functions */ +rng_context_t rng; /* * global options @@ -688,7 +690,7 @@ void sni_free( sni_entry *head ) * * Modifies the input string! This is not production quality! */ -sni_entry *sni_parse( char *sni_string, rng_context_t *p_rng ) +sni_entry *sni_parse( char *sni_string ) { sni_entry *cur = NULL, *new = NULL; char *p = sni_string; @@ -727,7 +729,7 @@ sni_entry *sni_parse( char *sni_string, rng_context_t *p_rng ) mbedtls_pk_init( new->key ); if( mbedtls_x509_crt_parse_file( new->cert, crt_file ) != 0 || - mbedtls_pk_parse_keyfile( new->key, key_file, "", rng_get, p_rng ) != 0 ) + mbedtls_pk_parse_keyfile( new->key, key_file, "", rng_get, &rng ) != 0 ) goto error; if( strcmp( ca_file, "-" ) != 0 ) @@ -1045,7 +1047,8 @@ static int ssl_async_start( mbedtls_ssl_context *ssl, for( slot = 0; slot < config_data->slots_used; slot++ ) { if( mbedtls_pk_check_pair( &cert->pk, - config_data->slots[slot].pk ) == 0 ) + config_data->slots[slot].pk, + rng_get, &rng ) == 0 ) break; } if( slot == config_data->slots_used ) @@ -1271,7 +1274,6 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_X509_CRT_PARSE_C) mbedtls_x509_crt_profile crt_profile_for_test = mbedtls_x509_crt_profile_default; #endif - rng_context_t rng; mbedtls_ssl_context ssl; mbedtls_ssl_config conf; #if defined(MBEDTLS_TIMING_C) @@ -2371,7 +2373,7 @@ int main( int argc, char *argv[] ) mbedtls_printf( " . Setting up SNI information..." ); fflush( stdout ); - if( ( sni_info = sni_parse( opt.sni, &rng ) ) == NULL ) + if( ( sni_info = sni_parse( opt.sni ) ) == NULL ) { mbedtls_printf( " failed\n" ); goto exit; From 6ff9ef56a507a02cb338f8de70acca3f53de913b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jun 2021 12:37:10 +0200 Subject: [PATCH 21/22] Fix cmake build of fuzz_privkey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- programs/fuzz/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/fuzz/CMakeLists.txt b/programs/fuzz/CMakeLists.txt index 4f35d7663..c7fcd356b 100644 --- a/programs/fuzz/CMakeLists.txt +++ b/programs/fuzz/CMakeLists.txt @@ -8,7 +8,6 @@ if(FUZZINGENGINE_LIB) endif() set(executables_no_common_c - fuzz_privkey fuzz_pubkey fuzz_x509crl fuzz_x509crt @@ -16,6 +15,7 @@ set(executables_no_common_c ) set(executables_with_common_c + fuzz_privkey fuzz_client fuzz_dtlsclient fuzz_dtlsserver From 609ab6478be8bf6ebd62add5ebf93cef7113ab0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 16 Jun 2021 14:29:11 +0200 Subject: [PATCH 22/22] Fix warning in some configurations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/pkparse.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/pkparse.c b/library/pkparse.c index 5438ee4a0..25fc1fecd 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1010,6 +1010,11 @@ static int pk_parse_key_pkcs8_unencrypted_der( mbedtls_pk_type_t pk_alg = MBEDTLS_PK_NONE; const mbedtls_pk_info_t *pk_info; +#if !defined(MBEDTLS_ECP_C) + (void) f_rng; + (void) p_rng; +#endif + /* * This function parses the PrivateKeyInfo object (PKCS#8 v1.2 = RFC 5208) *