From 5d397686a9ac2361a38b9f9a2545af631e54058e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 12:50:34 +0100 Subject: [PATCH 01/38] Restructure outgoing CliKeyExch: Add frame for new structure This commit adds declarations and dummy implementations for the restructured outgoing client key exchange handling that will replace the previous ssl_write_client_key_exchange(). The entry point for the CliKeyExchange handling that is called from the handshake state machine is `ssl_process_client_key_exchange()`, splitting the processing into the following steps: - Preparation Compute the keying material to be sent. * For (EC)DH: Pick parameters and compute PMS. * For ECJPAKE: Run round 2 * For RSA: Encrypt PMS - Writing: Prepare the writing of a new messae. - Postprocessing: Update handstate state machine. The subsequent commits will scatter the code from the previous monolithic function ssl_write_client_key_exchange() among those dedicated functions, commenting out each part of ssl_write_client_key_exchange() that has already been dealt with. This gradual progression is meant to ease reviewing. Once all code has been moved and all changes explained, ssl_write_client_key_exchange() will be removed. --- include/mbedtls/ssl_internal.h | 2 + library/ssl_cli.c | 93 +++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index ee0fa297b9..f53d747d4a 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1806,4 +1806,6 @@ MBEDTLS_ALWAYS_INLINE static inline void mbedtls_ssl_pend_fatal_alert( ssl->pending_fatal_alert_msg = message; } +#define SSL_PROC_CHK(f) do { if( ( ret = f ) < 0 ) goto cleanup; } while( 0 ) + #endif /* ssl_internal.h */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 003aa10c9f..63596c46b7 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3187,6 +3187,97 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) return( 0 ); } +/* + * + * STATE HANDLING: Client Key Exchange + * + */ + +/* + * Overview + */ + +/* Main entry point; orchestrates the other functions */ +static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ); + +/* Preparation + * - For ECDH: Generate client params and derive premater secret + * - For RSA-suites: Encrypt PMS + * - For ECJPAKE: Do Round 2 + */ +static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ); +static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen, + size_t *olen ); +static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ); + +/* + * Implementation + */ + +static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> process client key exchange" ) ); + + SSL_PROC_CHK( ssl_client_key_exchange_prepare( ssl ) ); + + /* Prepare CertificateVerify message in output buffer. */ + SSL_PROC_CHK( ssl_client_key_exchange_write( ssl, ssl->out_msg, + MBEDTLS_SSL_MAX_CONTENT_LEN, + &ssl->out_msglen ) ); + + ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; + ssl->out_msg[0] = MBEDTLS_SSL_HS_CLIENT_KEY_EXCHANGE; + + /* Update state */ + SSL_PROC_CHK( ssl_client_key_exchange_postprocess( ssl ) ); + + /* Dispatch message */ + + SSL_PROC_CHK( mbedtls_ssl_write_handshake_msg( ssl ) ); + + /* NOTE: For the new messaging layer, the postprocessing step + * might come after the dispatching step if the latter + * doesn't send the message immediately. + * At the moment, we must do the postprocessing + * prior to the dispatching because if the latter + * returns WANT_WRITE, we want the handshake state + * to be updated in order to not enter + * this function again on retry. */ + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= process client key exchange" ) ); + return( ret ); +} + +static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) +{ + /* TBD */ +} + +static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen, + size_t *olen ) +{ + /* TBD */ +} + +static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) +{ + /* TBD */ +} + +/* OLD CODE + * + * Temporarily included to gradually move it to the correct + * place in the restructured code. + * + */ + static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret; @@ -3892,7 +3983,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_CLIENT_KEY_EXCHANGE: - ret = ssl_write_client_key_exchange( ssl ); + ret = ssl_process_client_key_exchange( ssl ); break; case MBEDTLS_SSL_CERTIFICATE_VERIFY: From 6fb638b2fb6b5f7474476437fc71c3f94b1d1f46 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 17:55:44 +0100 Subject: [PATCH 02/38] Restructure outgoing CliKeyExch: Move PMS assembly code After and performing key generation operations, the client-side outgoing ClientKeyExchange handling includes code-paths to assembly the PreMasterSecret (PMS) from the available keying material, the exact assembly procedure depending on which ciphersuite is in use. E.g., in an (EC)DHE-PSK ciphersuite, the (EC)DHE secret would be concatenated with the PSK to form the PMS. This assembly of the PMS logically can be done after the ClientKeyExchange has been written and the respective keying material has been generated, and this commit moves it to the new postprocessing function ssl_client_key_exchange_postprocess(). Ideally, the PMS assembly could be done prior to writing the ClientKeyExchange message, but the (EC)DHE API does currently not allow splitting secret-generation and secret-export; as long as that's the case, we to generation and exporting in the message writing function, forcing PMS assembly to be done in the postprocessing. --- include/mbedtls/ssl_internal.h | 1 - library/ssl_cli.c | 226 +++++++++++++++++++++++++-------- 2 files changed, 172 insertions(+), 55 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index f53d747d4a..c16a238cc9 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -409,7 +409,6 @@ struct mbedtls_ssl_handshake_params ssl_ecrs_crt_vrfy_sign, /*!< CertificateVerify: pk_sign() */ } ecrs_state; /*!< current (or last) operation */ mbedtls_x509_crt *ecrs_peer_cert; /*!< The peer's CRT chain. */ - size_t ecrs_n; /*!< place for saving a length */ #endif #if defined(MBEDTLS_X509_CRT_PARSE_C) && \ !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 63596c46b7..1c580264f2 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3221,6 +3221,16 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) int ret = 0; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> process client key exchange" ) ); +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ssl->handshake->ecrs_enabled ) + { + if( ssl->handshake->ecrs_state == ssl_ecrs_cke_ecdh_calc_secret ) + goto cli_key_exchange_postprocess; + + mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); + } +#endif + SSL_PROC_CHK( ssl_client_key_exchange_prepare( ssl ) ); /* Prepare CertificateVerify message in output buffer. */ @@ -3231,7 +3241,10 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; ssl->out_msg[0] = MBEDTLS_SSL_HS_CLIENT_KEY_EXCHANGE; - /* Update state */ + /* Calculate secrets and update state */ +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) +cli_key_exchange_postprocess: +#endif SSL_PROC_CHK( ssl_client_key_exchange_postprocess( ssl ) ); /* Dispatch message */ @@ -3268,7 +3281,112 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { - /* TBD */ + int ret; +/* size_t i, n; */ + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) + { + if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, + ssl->handshake->premaster, + MBEDTLS_PREMASTER_SIZE, + &ssl->handshake->pmslen, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + { +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ssl->handshake->ecrs_enabled ) + ssl->handshake->ecrs_state = ssl_ecrs_cke_ecdh_calc_secret; +#endif /* MBEDTLS_SSL__ECP_RESTARTABLE */ + + if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, + &ssl->handshake->pmslen, + ssl->handshake->premaster, + MBEDTLS_MPI_MAX_SIZE, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_MPI( 3, "ECDH: z", &ssl->handshake->ecdh_ctx.z ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) + if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) + { + if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == + MBEDTLS_KEY_EXCHANGE_ECJPAKE ) + { + ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, + ssl->handshake->premaster, 32, &ssl->handshake->pmslen, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) + if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA ) + { + ((void) ret); + /* The premaster secret has already been by + * ssl_rsa_generate_partial_pms(). Only the + * PMS length needs to be set. */ + ssl->handshake->pmslen = 48; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + ssl->state = MBEDTLS_SSL_CERTIFICATE_VERIFY; + return( 0 ); } /* OLD CODE @@ -3313,18 +3431,18 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: X ", &ssl->handshake->dhm_ctx.X ); MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); - if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, - ssl->handshake->premaster, - MBEDTLS_PREMASTER_SIZE, - &ssl->handshake->pmslen, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, */ + /* ssl->handshake->premaster, */ + /* MBEDTLS_PREMASTER_SIZE, */ + /* &ssl->handshake->pmslen, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); */ + /* return( ret ); */ + /* } */ - MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); + /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); */ } else #endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ @@ -3374,34 +3492,34 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_Q ); -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_enabled ) - { - ssl->handshake->ecrs_n = n; - ssl->handshake->ecrs_state = ssl_ecrs_cke_ecdh_calc_secret; - } +/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ +/* if( ssl->handshake->ecrs_enabled ) */ +/* { */ +/* ssl->handshake->ecrs_n = n; */ +/* ssl->handshake->ecrs_state = ssl_ecrs_cke_ecdh_calc_secret; */ +/* } */ -ecdh_calc_secret: - if( ssl->handshake->ecrs_enabled ) - n = ssl->handshake->ecrs_n; -#endif - if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, - &ssl->handshake->pmslen, - ssl->handshake->premaster, - MBEDTLS_MPI_MAX_SIZE, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; -#endif - return( ret ); - } +/* ecdh_calc_secret: */ +/* if( ssl->handshake->ecrs_enabled ) */ +/* n = ssl->handshake->ecrs_n; */ +/* #endif */ +/* if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, */ +/* &ssl->handshake->pmslen, */ +/* ssl->handshake->premaster, */ +/* MBEDTLS_MPI_MAX_SIZE, */ +/* mbedtls_ssl_conf_get_frng( ssl->conf ), */ +/* ssl->conf->p_rng ) ) != 0 ) */ +/* { */ +/* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); */ +/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ +/* if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) */ +/* ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; */ +/* #endif */ +/* return( ret ); */ +/* } */ - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_Z ); +/* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ +/* MBEDTLS_DEBUG_ECDH_Z ); */ } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || @@ -3512,12 +3630,12 @@ ecdh_calc_secret: return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ + /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ @@ -3547,15 +3665,15 @@ ecdh_calc_secret: return( ret ); } - ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, - ssl->handshake->premaster, 32, &ssl->handshake->pmslen, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); - return( ret ); - } + /* ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, */ + /* ssl->handshake->premaster, 32, &ssl->handshake->pmslen, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ); */ + /* if( ret != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ From 01290c72407125ec6bf53e3de32b3b09ca8de4ef Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 23 May 2018 13:08:07 +0100 Subject: [PATCH 03/38] Restructure outgoing CliKeyExch: Move RSA/RSA-PSK PMS generation For RSA or RSA-PSK exchanges, the PMS contains 46 random bytes picked by the client. These bytes are generated prior to the writing of the ClientKeyExchange message. This commit splits the previous function ssl_write_encrypted_pms() into PPMS-GEN: ssl_rsa_generate_partial_pms() PPMS-ENC: ssl_rsa_encrypt_partial_pms(). The prefix 'partial' is meant to emphasize that the generation of the PMS is not always entirely done by these functions: For RSA-PSK e.g., the PSK still needs to be added. The two calls of ssl_write_encrypted_pms() in ssl_write_client_key_exchange() will split in calls of the functions PPMS-GEN and PPMS-ENC each, with PPMS-GEN being moved to the new preparation function ssl_client_key_exchange_prepare() in this commit, and PPMS-ENC being moved to ssl_client_key_exchange_write() in the next commit. --- library/ssl_cli.c | 128 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 26 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 1c580264f2..35542fbd4c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2337,43 +2337,62 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) -/* - * Generate a pre-master secret and encrypt it with the server's RSA key - */ -static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, - size_t offset, size_t *olen, - size_t pms_offset ) + +static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, + unsigned char* out, + unsigned add_length_tag ) { int ret; - size_t len_bytes = mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; - unsigned char *p = ssl->handshake->premaster + pms_offset; - mbedtls_pk_context *peer_pk = NULL; - - if( offset + len_bytes > MBEDTLS_SSL_OUT_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for encrypted pms" ) ); - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); - } /* - * Generate (part of) the pre-master as + * Generate (part of) the pre-master secret as * struct { + * [ uint16 length(48) ] * ProtocolVersion client_version; * opaque random[46]; * } PreMasterSecret; */ + + if( add_length_tag ) + { + out[0] = 0; + out[1] = 48; + out += 2; + } + mbedtls_ssl_write_version( mbedtls_ssl_conf_get_max_major_ver( ssl->conf ), mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ), - ssl->conf->transport, p ); + ssl->conf->transport, out ); if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, p + 2, 46 ) ) != 0 ) + ( ssl->conf->p_rng, out + 2, 46 ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); return( ret ); } - ssl->handshake->pmslen = 48; + return( 0 ); +} + +/* + * Encrypt the Premaster Secret it with the server's RSA key and + * write it to the provided buffer. + */ +static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, + unsigned char const *ppms, + unsigned char *out, size_t buflen, + size_t *olen ) +{ + int ret; + size_t len_bytes = mbedtls_ssl_get_minor_ver( ssl ) == + MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; + mbedtls_pk_context *peer_pk = NULL; + + if( buflen < len_bytes ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for encrypted pms" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) /* Because the peer CRT pubkey is embedded into the handshake @@ -2403,7 +2422,7 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, } /* - * Now write it out, encrypted + * Encrypt the part of the premaster secret and write it out. */ if( ! mbedtls_pk_can_do( peer_pk, MBEDTLS_PK_RSA ) ) { @@ -2413,9 +2432,8 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, } if( ( ret = mbedtls_pk_encrypt( peer_pk, - p, ssl->handshake->pmslen, - ssl->out_msg + offset + len_bytes, olen, - MBEDTLS_SSL_OUT_CONTENT_LEN - offset - len_bytes, + ppms, 48, out + len_bytes, + olen, buflen - len_bytes, mbedtls_ssl_conf_get_frng( ssl->conf ), ssl->conf->p_rng ) ) != 0 ) { @@ -2427,8 +2445,8 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, defined(MBEDTLS_SSL_PROTO_TLS1_2) if( len_bytes == 2 ) { - ssl->out_msg[offset+0] = (unsigned char)( *olen >> 8 ); - ssl->out_msg[offset+1] = (unsigned char)( *olen ); + out[0] = (unsigned char)( *olen >> 8 ); + out[1] = (unsigned char)( *olen ); *olen += 2; } #endif @@ -3266,9 +3284,63 @@ cleanup: return( ret ); } + + static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) { - /* TBD */ + int ret; + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + + ((void) ret); + ((void) ciphersuite_info); + + /* TODO: The current API for DH and ECDH does not allow + * to separate public key generation from public key export. + * + * Ideally, we would like to pick the private (EC)DH keys + * in this preparation step, exporting the corresponding + * public key in the writing step only. + * + * The necessary extension of the (EC)DH API is being + * considered, but until then we perform the public + * generation + export in the writing step. + * + */ + +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) + { + /* For RSA-PSK, the premaster secret is composed of + * - Length tag with value 48, encoded as a uint16 + * - 2 bytes indicating the TLS version + * - 46 randomly chosen bytes + * - the chosen PSK. + * The following call takes care of all but the PSK. */ + ret = ssl_rsa_generate_partial_pms( ssl, ssl->handshake->premaster, + 1 /* Add length tag */ ); + if( ret != 0 ) + return( ret ); + } +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ + +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA ) + { + /* For RSA-PSK, the premaster secret is composed of + * - 2 bytes indicating the TLS version + * - 46 randomly chosen bytes + * which the following call generates. */ + ret = ssl_rsa_generate_partial_pms( ssl, ssl->handshake->premaster, + 0 /* Omit length tag */ ); + if( ret != 0 ) + return( ret ); + } +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ + + return( 0 ); } static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, @@ -3566,6 +3638,8 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) { + /* Code for PMS generation has been moved, + * code for encryption and writing it hasn't. */ if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 2 ) ) != 0 ) return( ret ); } @@ -3644,6 +3718,8 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_KEY_EXCHANGE_RSA ) { i = 4; + /* Code for PMS generation has been moved, + * code for encryption and writing it hasn't. */ if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 0 ) ) != 0 ) return( ret ); } From 87e3c9aae8180296b668d60c0a877885a37f9477 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 23 May 2018 13:13:04 +0100 Subject: [PATCH 04/38] Restructure outgoing CliKeyExch: Move writing code This commit moves the code responsible for (a) generating the client's private and public (EC)DHE keys (b) writing it to the message buffer to the new writing function ssl_client_key_exchange_write(). As mentioned in the previous commit message, (a) and (b) are currently inseparable at the (EC)DHE API level, which is why (a) can't be moved to the preparation step. --- library/ssl_cli.c | 510 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 379 insertions(+), 131 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 35542fbd4c..38efd23765 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3348,7 +3348,252 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, size_t buflen, size_t *olen ) { - /* TBD */ + int ret; + unsigned char *p, *end; + size_t n; + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + + /* NOTE: This function will generate different messages + * when it's called multiple times, because it currently + * includes private/public key generation in case of + * (EC)DH ciphersuites. + * + * It is therefore not suitable to be registered as a callback + * for retransmission, if such get introduced at some point. + * + * Also see the documentation of ssl_client_key_exchange_prepare(). + */ + + p = buf + 4; + end = buf + buflen; + +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) + { + /* + * DHM key exchange -- send G^X mod P + */ + n = ssl->handshake->dhm_ctx.len; + if( (size_t)( end - p ) < n + 2 ) + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + + p[0] = (unsigned char)( n >> 8 ); + p[1] = (unsigned char)( n ); + p += 2; + + ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, + (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), + p, n, mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); + return( ret ); + } + MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: X ", &ssl->handshake->dhm_ctx.X ); + MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); + + p += n; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + { + /* + * ECDH key exchange -- generate and send client public value + */ + + /* NOTE: If we ever switch the ECDH stack/API to allow + * independent key generation and export, we should have + * generated our key share in the preparation step, and + * we'd only do the export here. */ + ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, + &n, p, end - p, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) + ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; +#endif + return( ret ); + } + + MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, + MBEDTLS_DEBUG_ECDH_Q ); + + p += n; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) + if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) + { + /* + * opaque psk_identity<0..2^16-1>; + */ + if( ssl->conf->psk == NULL || ssl->conf->psk_identity == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no private key for PSK" ) ); + return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); + } + + n = ssl->conf->psk_identity_len; + if( buflen < n + 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity too long or " + "SSL buffer too short" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + + p[0] = (unsigned char)( n >> 8 ); + p[1] = (unsigned char)( n ); + p += 2; + + memcpy( p, ssl->conf->psk_identity, n ); + p += ssl->conf->psk_identity_len; + +#if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_PSK ) + { + ((void) ret); + ((void) end); + n = 0; + } + else +#endif +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) + { + if( ( ret = ssl_rsa_encrypt_partial_pms( ssl, + ssl->handshake->premaster + 2, + p, end - p, &n ) ) != 0 ) + return( ret ); + p += n; + } + else +#endif +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) + { + /* + * ClientDiffieHellmanPublic public (DHM send G^X mod P) + */ + ((void) end); + + n = ssl->handshake->dhm_ctx.len; + + if( buflen < n + 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity or DHM size too long" + " or SSL buffer too short" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + + p[0] = (unsigned char)( n >> 8 ); + p[1] = (unsigned char)( n ); + p += 2; + + ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, + (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), + p, n, mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); + return( ret ); + } + + p += n; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) + { + /* + * ClientECDiffieHellmanPublic public; + */ + ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, &n, + p, buflen, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); + return( ret ); + } + MBEDTLS_SSL_DEBUG_ECP( 3, "ECDH: Q", &ssl->handshake->ecdh_ctx.Q ); + + p += n; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA ) + { + if( ( ret = ssl_rsa_encrypt_partial_pms( ssl, ssl->handshake->premaster, + p, end - p, &n ) ) != 0 ) + return( ret ); + p += n; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECJPAKE ) + { + ret = mbedtls_ecjpake_write_round_two( &ssl->handshake->ecjpake_ctx, + p, end - p, &n, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_write_round_two", ret ); + return( ret ); + } + p += n; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ + { + ((void) ciphersuite_info); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + *olen = p - buf; + return( 0 ); } static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) @@ -3483,25 +3728,26 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) /* * DHM key exchange -- send G^X mod P */ - n = ssl->handshake->dhm_ctx.len; - ssl->out_msg[4] = (unsigned char)( n >> 8 ); - ssl->out_msg[5] = (unsigned char)( n ); - i = 6; + /* n = ssl->handshake->dhm_ctx.len; */ - ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, - (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), - &ssl->out_msg[i], n, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); - return( ret ); - } + /* ssl->out_msg[4] = (unsigned char)( n >> 8 ); */ + /* ssl->out_msg[5] = (unsigned char)( n ); */ + /* i = 6; */ - MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: X ", &ssl->handshake->dhm_ctx.X ); - MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); + /* ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, */ + /* (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), */ + /* &ssl->out_msg[i], n, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ); */ + /* if( ret != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); */ + /* return( ret ); */ + /* } */ + + /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: X ", &ssl->handshake->dhm_ctx.X ); */ + /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); */ /* if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, */ /* ssl->handshake->premaster, */ @@ -3534,35 +3780,35 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) /* * ECDH key exchange -- send client public value */ - i = 4; +/* i = 4; */ -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_enabled ) - { - if( ssl->handshake->ecrs_state == ssl_ecrs_cke_ecdh_calc_secret ) - goto ecdh_calc_secret; +/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ +/* if( ssl->handshake->ecrs_enabled ) */ +/* { */ +/* if( ssl->handshake->ecrs_state == ssl_ecrs_cke_ecdh_calc_secret ) */ +/* goto ecdh_calc_secret; */ - mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); - } -#endif +/* mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); */ +/* } */ +/* #endif */ - ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, - &n, - &ssl->out_msg[i], 1000, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; -#endif - return( ret ); - } +/* ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, */ +/* &n, */ +/* &ssl->out_msg[i], 1000, */ +/* mbedtls_ssl_conf_get_frng( ssl->conf ), */ +/* ssl->conf->p_rng ); */ +/* if( ret != 0 ) */ +/* { */ +/* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); */ +/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ +/* if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) */ +/* ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; */ +/* #endif */ +/* return( ret ); */ +/* } */ - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_Q ); +/* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ +/* MBEDTLS_DEBUG_ECDH_Q ); */ /* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ /* if( ssl->handshake->ecrs_enabled ) */ @@ -3604,78 +3850,79 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) /* * opaque psk_identity<0..2^16-1>; */ - if( ssl->conf->psk == NULL || ssl->conf->psk_identity == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no private key for PSK" ) ); - return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); - } - i = 4; - n = ssl->conf->psk_identity_len; + /* if( ssl->conf->psk == NULL || ssl->conf->psk_identity == NULL ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no private key for PSK" ) ); */ + /* return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); */ + /* } */ - if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity too long or " - "SSL buffer too short" ) ); - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); - } + /* i = 4; */ + /* n = ssl->conf->psk_identity_len; */ - ssl->out_msg[i++] = (unsigned char)( n >> 8 ); - ssl->out_msg[i++] = (unsigned char)( n ); + /* if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity too long or " */ + /* "SSL buffer too short" ) ); */ + /* return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); */ + /* } */ - memcpy( ssl->out_msg + i, ssl->conf->psk_identity, ssl->conf->psk_identity_len ); - i += ssl->conf->psk_identity_len; + /* ssl->out_msg[i++] = (unsigned char)( n >> 8 ); */ + /* ssl->out_msg[i++] = (unsigned char)( n ); */ + + /* memcpy( ssl->out_msg + i, ssl->conf->psk_identity, ssl->conf->psk_identity_len ); */ + /* i += ssl->conf->psk_identity_len; */ #if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_PSK ) - { - n = 0; - } - else + /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ + /* == MBEDTLS_KEY_EXCHANGE_PSK ) */ + /* { */ + /* n = 0; */ + /* } */ + /* else */ #endif #if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) - { - /* Code for PMS generation has been moved, - * code for encryption and writing it hasn't. */ - if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 2 ) ) != 0 ) - return( ret ); - } - else + /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ + /* == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) */ + /* { */ + /* /\* Code for PMS generation has been moved, */ + /* * code for encryption and writing it hasn't. *\/ */ + /* if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 2 ) ) != 0 ) */ + /* return( ret ); */ + /* } */ + /* else */ #endif #if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) - { - /* - * ClientDiffieHellmanPublic public (DHM send G^X mod P) - */ - n = ssl->handshake->dhm_ctx.len; + /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ + /* == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) */ + /* { */ + /* /\* */ + /* * ClientDiffieHellmanPublic public (DHM send G^X mod P) */ + /* *\/ */ + /* n = ssl->handshake->dhm_ctx.len; */ - if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity or DHM size too long" - " or SSL buffer too short" ) ); - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); - } + /* if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity or DHM size too long" */ + /* " or SSL buffer too short" ) ); */ + /* return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); */ + /* } */ - ssl->out_msg[i++] = (unsigned char)( n >> 8 ); - ssl->out_msg[i++] = (unsigned char)( n ); + /* ssl->out_msg[i++] = (unsigned char)( n >> 8 ); */ + /* ssl->out_msg[i++] = (unsigned char)( n ); */ - ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, - (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), - &ssl->out_msg[i], n, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); - return( ret ); - } - } - else + /* ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, */ + /* (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), */ + /* &ssl->out_msg[i], n, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ); */ + /* if( ret != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); */ + /* return( ret ); */ + /* } */ + /* } */ + /* else */ #endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) @@ -3684,18 +3931,19 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) /* * ClientECDiffieHellmanPublic public; */ - ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, &n, - &ssl->out_msg[i], MBEDTLS_SSL_OUT_CONTENT_LEN - i, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); - return( ret ); - } - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_Q ); + /* ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, &n, */ + /* &ssl->out_msg[i], MBEDTLS_SSL_OUT_CONTENT_LEN - i, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ); */ + /* if( ret != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); */ + /* return( ret ); */ + /* } */ + + /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ + /* MBEDTLS_DEBUG_ECDH_Q ); */ } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ @@ -3714,32 +3962,32 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) else #endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_RSA ) - { - i = 4; - /* Code for PMS generation has been moved, - * code for encryption and writing it hasn't. */ - if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 0 ) ) != 0 ) - return( ret ); - } + /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ + /* MBEDTLS_KEY_EXCHANGE_RSA ) */ + /* { */ + /* i = 4; */ + /* /\* Code for PMS generation has been moved, */ + /* * code for encryption and writing it hasn't. *\/ */ + /* if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 0 ) ) != 0 ) */ + /* return( ret ); */ + /* } */ else #endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECJPAKE ) { - i = 4; + /* i = 4; */ - ret = mbedtls_ecjpake_write_round_two( &ssl->handshake->ecjpake_ctx, - ssl->out_msg + i, MBEDTLS_SSL_OUT_CONTENT_LEN - i, &n, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_write_round_two", ret ); - return( ret ); - } + /* ret = mbedtls_ecjpake_write_round_two( &ssl->handshake->ecjpake_ctx, */ + /* ssl->out_msg + i, MBEDTLS_SSL_OUT_CONTENT_LEN - i, &n, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ); */ + /* if( ret != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_write_round_two", ret ); */ + /* return( ret ); */ + /* } */ /* ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, */ /* ssl->handshake->premaster, 32, &ssl->handshake->pmslen, */ From 4f68b04018a35fe6aff4298565cdfc1f0c781809 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 22 Jul 2019 15:58:19 +0100 Subject: [PATCH 05/38] Restructure outgoing CliKeyExch: Remove old code The code from the previous function ssl_write_client_key_exchange() has been entirely moved to one of the newly introduced subroutines and is no longer needed. This commit removes it. --- library/ssl_cli.c | 318 ---------------------------------------------- 1 file changed, 318 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 38efd23765..ded93a6666 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3706,324 +3706,6 @@ static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) return( 0 ); } -/* OLD CODE - * - * Temporarily included to gradually move it to the correct - * place in the restructured code. - * - */ - -static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) -{ - int ret; - size_t i, n; - mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = - mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write client key exchange" ) ); - -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) - { - /* - * DHM key exchange -- send G^X mod P - */ - - /* n = ssl->handshake->dhm_ctx.len; */ - - /* ssl->out_msg[4] = (unsigned char)( n >> 8 ); */ - /* ssl->out_msg[5] = (unsigned char)( n ); */ - /* i = 6; */ - - /* ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, */ - /* (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), */ - /* &ssl->out_msg[i], n, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ); */ - /* if( ret != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); */ - /* return( ret ); */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: X ", &ssl->handshake->dhm_ctx.X ); */ - /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); */ - - /* if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, */ - /* ssl->handshake->premaster, */ - /* MBEDTLS_PREMASTER_SIZE, */ - /* &ssl->handshake->pmslen, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); */ - /* return( ret ); */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) - { - /* - * ECDH key exchange -- send client public value - */ -/* i = 4; */ - -/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ -/* if( ssl->handshake->ecrs_enabled ) */ -/* { */ -/* if( ssl->handshake->ecrs_state == ssl_ecrs_cke_ecdh_calc_secret ) */ -/* goto ecdh_calc_secret; */ - -/* mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); */ -/* } */ -/* #endif */ - -/* ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, */ -/* &n, */ -/* &ssl->out_msg[i], 1000, */ -/* mbedtls_ssl_conf_get_frng( ssl->conf ), */ -/* ssl->conf->p_rng ); */ -/* if( ret != 0 ) */ -/* { */ -/* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); */ -/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ -/* if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) */ -/* ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; */ -/* #endif */ -/* return( ret ); */ -/* } */ - -/* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ -/* MBEDTLS_DEBUG_ECDH_Q ); */ - -/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ -/* if( ssl->handshake->ecrs_enabled ) */ -/* { */ -/* ssl->handshake->ecrs_n = n; */ -/* ssl->handshake->ecrs_state = ssl_ecrs_cke_ecdh_calc_secret; */ -/* } */ - -/* ecdh_calc_secret: */ -/* if( ssl->handshake->ecrs_enabled ) */ -/* n = ssl->handshake->ecrs_n; */ -/* #endif */ -/* if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, */ -/* &ssl->handshake->pmslen, */ -/* ssl->handshake->premaster, */ -/* MBEDTLS_MPI_MAX_SIZE, */ -/* mbedtls_ssl_conf_get_frng( ssl->conf ), */ -/* ssl->conf->p_rng ) ) != 0 ) */ -/* { */ -/* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); */ -/* #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) */ -/* if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) */ -/* ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; */ -/* #endif */ -/* return( ret ); */ -/* } */ - -/* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ -/* MBEDTLS_DEBUG_ECDH_Z ); */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) - if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) - { - /* - * opaque psk_identity<0..2^16-1>; - */ - - /* if( ssl->conf->psk == NULL || ssl->conf->psk_identity == NULL ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no private key for PSK" ) ); */ - /* return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); */ - /* } */ - - /* i = 4; */ - /* n = ssl->conf->psk_identity_len; */ - - /* if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity too long or " */ - /* "SSL buffer too short" ) ); */ - /* return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); */ - /* } */ - - /* ssl->out_msg[i++] = (unsigned char)( n >> 8 ); */ - /* ssl->out_msg[i++] = (unsigned char)( n ); */ - - /* memcpy( ssl->out_msg + i, ssl->conf->psk_identity, ssl->conf->psk_identity_len ); */ - /* i += ssl->conf->psk_identity_len; */ - -#if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) - /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ - /* == MBEDTLS_KEY_EXCHANGE_PSK ) */ - /* { */ - /* n = 0; */ - /* } */ - /* else */ -#endif -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) - /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ - /* == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) */ - /* { */ - /* /\* Code for PMS generation has been moved, */ - /* * code for encryption and writing it hasn't. *\/ */ - /* if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 2 ) ) != 0 ) */ - /* return( ret ); */ - /* } */ - /* else */ -#endif -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ - /* == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) */ - /* { */ - /* /\* */ - /* * ClientDiffieHellmanPublic public (DHM send G^X mod P) */ - /* *\/ */ - /* n = ssl->handshake->dhm_ctx.len; */ - - /* if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity or DHM size too long" */ - /* " or SSL buffer too short" ) ); */ - /* return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); */ - /* } */ - - /* ssl->out_msg[i++] = (unsigned char)( n >> 8 ); */ - /* ssl->out_msg[i++] = (unsigned char)( n ); */ - - /* ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, */ - /* (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), */ - /* &ssl->out_msg[i], n, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ); */ - /* if( ret != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); */ - /* return( ret ); */ - /* } */ - /* } */ - /* else */ -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) - { - /* - * ClientECDiffieHellmanPublic public; - */ - - /* ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, &n, */ - /* &ssl->out_msg[i], MBEDTLS_SSL_OUT_CONTENT_LEN - i, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ); */ - /* if( ret != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); */ - /* return( ret ); */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ - /* MBEDTLS_DEBUG_ECDH_Q ); */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ - /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) - /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ - /* MBEDTLS_KEY_EXCHANGE_RSA ) */ - /* { */ - /* i = 4; */ - /* /\* Code for PMS generation has been moved, */ - /* * code for encryption and writing it hasn't. *\/ */ - /* if( ( ret = ssl_write_encrypted_pms( ssl, i, &n, 0 ) ) != 0 ) */ - /* return( ret ); */ - /* } */ - else -#endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_ECJPAKE ) - { - /* i = 4; */ - - /* ret = mbedtls_ecjpake_write_round_two( &ssl->handshake->ecjpake_ctx, */ - /* ssl->out_msg + i, MBEDTLS_SSL_OUT_CONTENT_LEN - i, &n, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ); */ - /* if( ret != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_write_round_two", ret ); */ - /* return( ret ); */ - /* } */ - - /* ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, */ - /* ssl->handshake->premaster, 32, &ssl->handshake->pmslen, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ); */ - /* if( ret != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ - { - ((void) ciphersuite_info); - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - ssl->out_msglen = i + n; - ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; - ssl->out_msg[0] = MBEDTLS_SSL_HS_CLIENT_KEY_EXCHANGE; - - ssl->state++; - - if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); - return( ret ); - } - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write client key exchange" ) ); - - return( 0 ); -} - #if !defined(MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED) static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) { From 7ec345d95f0babf84292aa72b11b80536339f090 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 17:16:42 +0100 Subject: [PATCH 06/38] Restructure incoming CliKeyExch: Add frame for restructuring This commit adds declarations and dummy implementations for the restructured incoming client key exchange handling that will replace the previous ssl_parse_client_key_exchange(). The entry point for the CliKeyExchange handling that is called from the handshake state machine is `ssl_process_client_key_exchange()`, splitting the processing into the following steps: - Fetching: Read next message from the messaging layer and check that it has the correct type. The ClientKeyExchange message is never omitted, so there is no ambiguity in what to expect, and hence no dedicated preparation step as for other handshake states. - Parsing: Parse the ClientKeyExchange message and use the information in it to derive keying material such as the shared (EC)DHE secret. - Postprocessing: Compute the session keys from the available keying material. This splits in two steps: (1) Build the PreMasterSecret (PMS) from the available keying material, e.g. concatenate the (EC)DHE secret with a PSK, if used. (2) Extract the MasterSecret and Session Keys from the PreMasterSecret. The subsequent commits will scatter the code from the previous monolithic function ssl_parse_client_key_exchange() among those dedicated functions, commenting out each part of ssl_parse_client_key_exchange() that has already been dealt with. This gradual progression is meant to ease reviewing. Once all code has been moved and all changes explained, ssl_parse_client_key_exchange() will be removed. --- library/ssl_srv.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 937ee0b52f..5f10e249f4 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4040,6 +4040,86 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha } #endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ +/* + * + * STATE HANDLING: Client Key Exchange + * + */ + +/* + * Overview + */ + +/* Main entry point; orchestrates the other functions. */ +static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ); + +static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen ); +/* Update the handshake state */ +static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ); + +/* + * Implementation + */ + +static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) +{ + int ret; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> process client key exchange" ) ); + + /* The ClientKeyExchange message is never skipped. */ + + /* Reading step */ + if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + + if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE || + ssl->in_msg[0] != MBEDTLS_SSL_HS_CLIENT_KEY_EXCHANGE ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) ); + mbedtls_ssl_pend_fatal_alert( ssl, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE; + goto cleanup; + } + + SSL_PROC_CHK( ssl_client_key_exchange_parse( ssl, ssl->in_msg, + ssl->in_hslen ) ); + + /* Update state */ + SSL_PROC_CHK( ssl_client_key_exchange_postprocess( ssl ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= process client key exchange" ) ); + return( ret ); +} + +static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen ) +{ + /* TBD */ +} + +/* Update the handshake state */ +static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) +{ + ssl->state = MBEDTLS_SSL_CERTIFICATE_VERIFY; + return( 0 ); +} + +/* OLD CODE + * + * Temporarily included to gradually move it to the correct + * place in the restructured code. + * + */ + static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret; @@ -4705,7 +4785,7 @@ int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_CLIENT_KEY_EXCHANGE: - ret = ssl_parse_client_key_exchange( ssl ); + ret = ssl_process_client_key_exchange( ssl ); break; case MBEDTLS_SSL_CERTIFICATE_VERIFY: From dc8bfb9001bd0b1acacaf80eeaf17c44d6d66773 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 17:24:27 +0100 Subject: [PATCH 07/38] Restructure incoming CliKeyExch: Move key derivation code This commit moves the generation of the master secret and session keys from the premaster secret (done in mbedtlsssl_derive_keys()) from the previous ClientKeyExchange parsing function ssl_parse_client_key_exchange() to the new postprocessing function ssl_client_key_exchange_postprocess(). --- library/ssl_srv.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 5f10e249f4..5737411f41 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4109,6 +4109,14 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, /* Update the handshake state */ static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { + int ret; + + if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_derive_keys", ret ); + return( ret ); + } + ssl->state = MBEDTLS_SSL_CERTIFICATE_VERIFY; return( 0 ); } @@ -4400,11 +4408,11 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } - if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_derive_keys", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_derive_keys", ret ); */ + /* return( ret ); */ + /* } */ ssl->state++; From 1e23af8fa8b648147c426ec4ac8600f0b5848666 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 17:26:17 +0100 Subject: [PATCH 08/38] Restructure incoming CliKeyExch: Move PMS assembly code After parsing and performing key generation operations, the server-side incoming ClientKeyExchange handling includes code-paths to assembly the PreMasterSecret (PMS) from the available keying material, the exact assembly procedure depending on which ciphersuite is in use. E.g., in an (EC)DHE-PSK ciphersuite, the (EC)DHE secret would be concatenated with the PSK to form the PMS. This assembly of the PMS logically comes done after the ClientKeyExchange has been parsed and the respective keying material has been generated, and this commit moves it to the new postprocessing function ssl_client_key_exchange_postprocess(). --- library/ssl_srv.c | 239 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 182 insertions(+), 57 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 5737411f41..26964924c1 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4071,7 +4071,8 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) /* The ClientKeyExchange message is never skipped. */ /* Reading step */ - if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_read_record( ssl, + 1 /* update checksum */ ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); return( ret ); @@ -4110,6 +4111,130 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { int ret; + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) + { + if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, + ssl->handshake->premaster, + MBEDTLS_PREMASTER_SIZE, + &ssl->handshake->pmslen, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); + } + + MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + { + if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, + &ssl->handshake->pmslen, + ssl->handshake->premaster, + MBEDTLS_MPI_MAX_SIZE, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); + } + + MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, + MBEDTLS_DEBUG_ECDH_Z ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_PSK ) + { + if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) + { + if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) + { + if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) + { + if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECJPAKE ) + { + ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, + ssl->handshake->premaster, 32, &ssl->handshake->pmslen, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) { @@ -4188,18 +4313,18 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); } - if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, - ssl->handshake->premaster, - MBEDTLS_PREMASTER_SIZE, - &ssl->handshake->pmslen, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); - } + /* if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, */ + /* ssl->handshake->premaster, */ + /* MBEDTLS_PREMASTER_SIZE, */ + /* &ssl->handshake->pmslen, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); */ + /* } */ - MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); + /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); */ } else #endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ @@ -4226,19 +4351,19 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_QP ); - if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, - &ssl->handshake->pmslen, - ssl->handshake->premaster, - MBEDTLS_MPI_MAX_SIZE, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); - } + /* if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, */ + /* &ssl->handshake->pmslen, */ + /* ssl->handshake->premaster, */ + /* MBEDTLS_MPI_MAX_SIZE, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); */ + /* } */ - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_Z ); + /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ + /* MBEDTLS_DEBUG_ECDH_Z ); */ } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || @@ -4261,12 +4386,12 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); } - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ + /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ @@ -4299,12 +4424,12 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) return( ret ); } - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ + /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ @@ -4329,12 +4454,12 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); } - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ + /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ @@ -4358,12 +4483,12 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_QP ); - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ + /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ @@ -4391,15 +4516,15 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } - ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, - ssl->handshake->premaster, 32, &ssl->handshake->pmslen, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); - return( ret ); - } + /* ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, */ + /* ssl->handshake->premaster, 32, &ssl->handshake->pmslen, */ + /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ + /* ssl->conf->p_rng ); */ + /* if( ret != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ From e7c4eed9b80dd9dfacd3357bb44e1a42968ce121 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 17:30:11 +0100 Subject: [PATCH 09/38] Restructure incoming CliKeyExch: Parsing code --- library/ssl_srv.c | 408 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 303 insertions(+), 105 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 26964924c1..0053398a24 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4071,6 +4071,22 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) /* The ClientKeyExchange message is never skipped. */ /* Reading step */ +#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) && \ + ( defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ) + if( ( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA_PSK || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA ) && + ( ssl->handshake->async_in_progress != 0 ) ) + { + /* We've already read a record and there is an asynchronous + * operation in progress to decrypt it. So skip reading the + * record. */ + MBEDTLS_SSL_DEBUG_MSG( 3, ( "will resume decryption of previously-read record" ) ); + } + else +#endif if( ( ret = mbedtls_ssl_read_record( ssl, 1 /* update checksum */ ) ) != 0 ) { @@ -4104,7 +4120,185 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) { - /* TBD */ + int ret; + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + unsigned char *p, *end; + + p = buf + mbedtls_ssl_hs_hdr_len( ssl ); + end = buf + buflen; + +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) + { + if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); + return( ret ); + } + + if( p != end ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + { + if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, + p, end - p) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); + } + + MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, + MBEDTLS_DEBUG_ECDH_QP ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_PSK ) + { + if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); + return( ret ); + } + + if( p != end ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) + { +#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) + if ( ssl->handshake->async_in_progress != 0 ) + { + /* There is an asynchronous operation in progress to + * decrypt the encrypted premaster secret, so skip + * directly to resuming this operation. */ + MBEDTLS_SSL_DEBUG_MSG( 3, ( "PSK identity already parsed" ) ); + /* Update p to skip the PSK identity. ssl_parse_encrypted_pms + * won't actually use it, but maintain p anyway for robustness. */ + p += ssl->conf->psk_identity_len + 2; + } + else +#endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ + if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); + return( ret ); + } + + if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 2 ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_encrypted_pms" ), ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) + { + if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); + return( ret ); + } + if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); + return( ret ); + } + + if( p != end ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) + { + if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); + return( ret ); + } + + if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, + p, end - p ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); + return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); + } + + MBEDTLS_SSL_DEBUG_ECP( 3, "ECDH: Qp ", &ssl->handshake->ecdh_ctx.Qp ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA ) + { + if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 0 ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_parse_encrypted_pms_secret" ), ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECJPAKE ) + { + ret = mbedtls_ecjpake_read_round_two( &ssl->handshake->ecjpake_ctx, + p, end - p ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_two", ret ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + return( 0 ); } /* Update the handshake state */ @@ -4114,6 +4308,8 @@ static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client key exchange" ) ); + #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) @@ -4262,19 +4458,21 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client key exchange" ) ); -#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) && \ - ( defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ) - if( ( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_RSA_PSK || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_RSA ) && - ( ssl->handshake->async_in_progress != 0 ) ) - { - /* We've already read a record and there is an asynchronous - * operation in progress to decrypt it. So skip reading the - * record. */ - MBEDTLS_SSL_DEBUG_MSG( 3, ( "will resume decryption of previously-read record" ) ); - } - else +/* #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) && \ */ +/* ( defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ */ +/* defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ) */ +/* if( ( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ +/* == MBEDTLS_KEY_EXCHANGE_RSA_PSK || */ +/* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ +/* == MBEDTLS_KEY_EXCHANGE_RSA ) && */ +/* ( ssl->handshake->async_in_progress != 0 ) ) */ +/* { */ +/* /\* We've already read a record and there is an asynchronous */ +/* * operation in progress to decrypt it. So skip reading the */ +/* * record. *\/ */ +/* MBEDTLS_SSL_DEBUG_MSG( 3, ( "will resume decryption of previously-read record" ) ); */ +/* } */ +/* else */ #endif if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) { @@ -4301,17 +4499,17 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) { - if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); - return( ret ); - } + /* if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); */ + /* return( ret ); */ + /* } */ - if( p != end ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); - } + /* if( p != end ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); */ + /* } */ /* if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, */ /* ssl->handshake->premaster, */ @@ -4341,15 +4539,15 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { - if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, - p, end - p) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); - } + /* if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, */ + /* p, end - p) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); */ + /* } */ - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_QP ); + /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ + /* MBEDTLS_DEBUG_ECDH_QP ); */ /* if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, */ /* &ssl->handshake->pmslen, */ @@ -4374,17 +4572,17 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_PSK ) { - if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); - return( ret ); - } + /* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ + /* return( ret ); */ + /* } */ - if( p != end ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); - } + /* if( p != end ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); */ + /* } */ /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ @@ -4399,30 +4597,30 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) { -#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) - if ( ssl->handshake->async_in_progress != 0 ) - { - /* There is an asynchronous operation in progress to - * decrypt the encrypted premaster secret, so skip - * directly to resuming this operation. */ - MBEDTLS_SSL_DEBUG_MSG( 3, ( "PSK identity already parsed" ) ); - /* Update p to skip the PSK identity. ssl_parse_encrypted_pms - * won't actually use it, but maintain p anyway for robustness. */ - p += ssl->conf->psk_identity_len + 2; - } - else -#endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ - if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); - return( ret ); - } +/* #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) */ +/* if ( ssl->handshake->async_in_progress != 0 ) */ +/* { */ +/* /\* There is an asynchronous operation in progress to */ +/* * decrypt the encrypted premaster secret, so skip */ +/* * directly to resuming this operation. *\/ */ +/* MBEDTLS_SSL_DEBUG_MSG( 3, ( "PSK identity already parsed" ) ); */ +/* /\* Update p to skip the PSK identity. ssl_parse_encrypted_pms */ +/* * won't actually use it, but maintain p anyway for robustness. *\/ */ +/* p += ssl->conf->psk_identity_len + 2; */ +/* } */ +/* else */ +/* #endif /\* MBEDTLS_SSL_ASYNC_PRIVATE *\/ */ +/* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ +/* { */ +/* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ +/* return( ret ); */ +/* } */ - if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 2 ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_encrypted_pms" ), ret ); - return( ret ); - } + /* if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 2 ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_encrypted_pms" ), ret ); */ + /* return( ret ); */ + /* } */ /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ @@ -4437,22 +4635,22 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) { - if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); - return( ret ); - } - if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); - return( ret ); - } + /* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ + /* return( ret ); */ + /* } */ + /* if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); */ + /* return( ret ); */ + /* } */ - if( p != end ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); - } + /* if( p != end ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); */ + /* } */ /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ @@ -4467,21 +4665,21 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) { - if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); - return( ret ); - } + /* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ + /* return( ret ); */ + /* } */ - if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, - p, end - p ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); - } + /* if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, */ + /* p, end - p ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); */ + /* } */ - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_QP ); + /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ + /* MBEDTLS_DEBUG_ECDH_QP ); */ /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ @@ -4496,11 +4694,11 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_RSA ) { - if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 0 ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_parse_encrypted_pms_secret" ), ret ); - return( ret ); - } + /* if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 0 ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_parse_encrypted_pms_secret" ), ret ); */ + /* return( ret ); */ + /* } */ } else #endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ @@ -4508,13 +4706,13 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECJPAKE ) { - ret = mbedtls_ecjpake_read_round_two( &ssl->handshake->ecjpake_ctx, - p, end - p ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_two", ret ); - return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); - } + /* ret = mbedtls_ecjpake_read_round_two( &ssl->handshake->ecjpake_ctx, */ + /* p, end - p ); */ + /* if( ret != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_two", ret ); */ + /* return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); */ + /* } */ /* ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, */ /* ssl->handshake->premaster, 32, &ssl->handshake->pmslen, */ From 2eb716d62661fd5048a0e1671220cbb21b2bc051 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 22 Jul 2019 17:07:50 +0100 Subject: [PATCH 10/38] Restructure incoming CliKeyExch: Remove old code The code from the previous function ssl_parse_client_key_exchange() has been entirely moved to one of the newly introduced subroutines and is no longer needed. This commit removes it. --- library/ssl_srv.c | 302 ---------------------------------------------- 1 file changed, 302 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 0053398a24..4ab8a3ed6f 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4442,308 +4442,6 @@ static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) return( 0 ); } -/* OLD CODE - * - * Temporarily included to gradually move it to the correct - * place in the restructured code. - * - */ - -static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) -{ - int ret; - mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = - mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); - unsigned char *p, *end; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client key exchange" ) ); - -/* #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) && \ */ -/* ( defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ */ -/* defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ) */ -/* if( ( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ -/* == MBEDTLS_KEY_EXCHANGE_RSA_PSK || */ -/* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ -/* == MBEDTLS_KEY_EXCHANGE_RSA ) && */ -/* ( ssl->handshake->async_in_progress != 0 ) ) */ -/* { */ -/* /\* We've already read a record and there is an asynchronous */ -/* * operation in progress to decrypt it. So skip reading the */ -/* * record. *\/ */ -/* MBEDTLS_SSL_DEBUG_MSG( 3, ( "will resume decryption of previously-read record" ) ); */ -/* } */ -/* else */ -#endif - if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); - return( ret ); - } - - p = ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ); - end = ssl->in_msg + ssl->in_hslen; - - if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); - } - - if( ssl->in_msg[0] != MBEDTLS_SSL_HS_CLIENT_KEY_EXCHANGE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); - } - -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) - { - /* if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); */ - /* return( ret ); */ - /* } */ - - /* if( p != end ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); */ - /* } */ - - /* if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, */ - /* ssl->handshake->premaster, */ - /* MBEDTLS_PREMASTER_SIZE, */ - /* &ssl->handshake->pmslen, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) - { - /* if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, */ - /* p, end - p) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ - /* MBEDTLS_DEBUG_ECDH_QP ); */ - - /* if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, */ - /* &ssl->handshake->pmslen, */ - /* ssl->handshake->premaster, */ - /* MBEDTLS_MPI_MAX_SIZE, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ - /* MBEDTLS_DEBUG_ECDH_Z ); */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_PSK ) - { - /* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ - /* return( ret ); */ - /* } */ - - /* if( p != end ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); */ - /* } */ - - /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ - /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_RSA_PSK ) - { -/* #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) */ -/* if ( ssl->handshake->async_in_progress != 0 ) */ -/* { */ -/* /\* There is an asynchronous operation in progress to */ -/* * decrypt the encrypted premaster secret, so skip */ -/* * directly to resuming this operation. *\/ */ -/* MBEDTLS_SSL_DEBUG_MSG( 3, ( "PSK identity already parsed" ) ); */ -/* /\* Update p to skip the PSK identity. ssl_parse_encrypted_pms */ -/* * won't actually use it, but maintain p anyway for robustness. *\/ */ -/* p += ssl->conf->psk_identity_len + 2; */ -/* } */ -/* else */ -/* #endif /\* MBEDTLS_SSL_ASYNC_PRIVATE *\/ */ -/* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ -/* { */ -/* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ -/* return( ret ); */ -/* } */ - - /* if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 2 ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_encrypted_pms" ), ret ); */ - /* return( ret ); */ - /* } */ - - /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ - /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_DHE_PSK ) - { - /* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ - /* return( ret ); */ - /* } */ - /* if( ( ret = ssl_parse_client_dh_public( ssl, &p, end ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_dh_public" ), ret ); */ - /* return( ret ); */ - /* } */ - - /* if( p != end ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange" ) ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); */ - /* } */ - - /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ - /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) - { - /* if( ( ret = ssl_parse_client_psk_identity( ssl, &p, end ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_client_psk_identity" ), ret ); */ - /* return( ret ); */ - /* } */ - - /* if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, */ - /* p, end - p ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, */ - /* MBEDTLS_DEBUG_ECDH_QP ); */ - - /* if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, */ - /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_RSA ) - { - /* if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 0 ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_parse_encrypted_pms_secret" ), ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_RSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_ECJPAKE ) - { - /* ret = mbedtls_ecjpake_read_round_two( &ssl->handshake->ecjpake_ctx, */ - /* p, end - p ); */ - /* if( ret != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_two", ret ); */ - /* return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); */ - /* } */ - - /* ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, */ - /* ssl->handshake->premaster, 32, &ssl->handshake->pmslen, */ - /* mbedtls_ssl_conf_get_frng( ssl->conf ), */ - /* ssl->conf->p_rng ); */ - /* if( ret != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); */ - /* return( ret ); */ - /* } */ - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_derive_keys", ret ); */ - /* return( ret ); */ - /* } */ - - ssl->state++; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse client key exchange" ) ); - - return( 0 ); -} - #if !defined(MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED) static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) { From d116e82268b0901f426e7cb14bf59044a1b518e7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 18:17:00 +0100 Subject: [PATCH 11/38] Restructure incoming CliKeyExch: Shorten postprocessing This commit subsumes multiple branches of ssl_client_key_exchange_postprocess() that call mbedtls_ssl_psk_derive_premaster(). --- library/ssl_srv.c | 46 +++------------------------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 4ab8a3ed6f..559024458c 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4360,48 +4360,8 @@ static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_PSK ) - { - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) - { - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) - { - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) + if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) { if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) @@ -4411,7 +4371,7 @@ static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) } } else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECJPAKE ) { From 09d236419e79638eda2aa575a75aaddbf0ef663c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 22 Jul 2019 17:18:18 +0100 Subject: [PATCH 12/38] Share code between In-CliKeyExch and Out-CliKeyExch The postprocessing code for the server-side incoming client key exchange and the client-side outgoing client key exchange both contain the same code-paths for building the premaster secret depending on the chosen ciphersuite (e.g., for ECDHE-PSK, concatenating the ECDHE secret with the chosen PSK). This commit moves this common code to ssl_tls.c, allowing client- and server-side to share it. --- include/mbedtls/ssl_internal.h | 2 + library/ssl_cli.c | 103 +++----------------------------- library/ssl_srv.c | 86 +-------------------------- library/ssl_tls.c | 105 +++++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 178 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index c16a238cc9..25ded986b7 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -987,6 +987,8 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ); void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl, mbedtls_ssl_ciphersuite_handle_t ciphersuite_info ); +int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ); + #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exchange_type_t key_ex ); #endif diff --git a/library/ssl_cli.c b/library/ssl_cli.c index ded93a6666..56ce907a35 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3599,107 +3599,20 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { int ret; -/* size_t i, n; */ - mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = - mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) + if( ( ret = mbedtls_ssl_build_pms( ssl ) ) != 0 ) { - if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, - ssl->handshake->premaster, - MBEDTLS_PREMASTER_SIZE, - &ssl->handshake->pmslen, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); - return( ret ); - } + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_build_pms", ret ); - MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) - { #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_enabled ) + if( ssl->handshake->ecrs_enabled && + ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) + { + ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; ssl->handshake->ecrs_state = ssl_ecrs_cke_ecdh_calc_secret; + } #endif /* MBEDTLS_SSL__ECP_RESTARTABLE */ - - if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, - &ssl->handshake->pmslen, - ssl->handshake->premaster, - MBEDTLS_MPI_MAX_SIZE, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); - return( ret ); - } - - MBEDTLS_SSL_DEBUG_MPI( 3, "ECDH: z", &ssl->handshake->ecdh_ctx.z ); - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) - if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) - { - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } - } - else -#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_ECJPAKE ) - { - ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, - ssl->handshake->premaster, 32, &ssl->handshake->pmslen, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); - return( ret ); - } - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA ) - { - ((void) ret); - /* The premaster secret has already been by - * ssl_rsa_generate_partial_pms(). Only the - * PMS length needs to be set. */ - ssl->handshake->pmslen = 48; - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + return( ret ); } ssl->state = MBEDTLS_SSL_CERTIFICATE_VERIFY; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 559024458c..63760c5e68 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4305,91 +4305,11 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { int ret; - mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = - mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client key exchange" ) ); - -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) + if( ( ret = mbedtls_ssl_build_pms( ssl ) ) != 0 ) { - if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, - ssl->handshake->premaster, - MBEDTLS_PREMASTER_SIZE, - &ssl->handshake->pmslen, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); - } - - MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) - { - if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, - &ssl->handshake->pmslen, - ssl->handshake->premaster, - MBEDTLS_MPI_MAX_SIZE, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); - } - - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_Z ); - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) - if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) - { - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); - } - } - else -#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECJPAKE ) - { - ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, - ssl->handshake->premaster, 32, &ssl->handshake->pmslen, - mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); - return( ret ); - } - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_build_pms", ret ); + return( ret ); } if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6187eb01ab..e651f29c1c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1653,6 +1653,111 @@ void ssl_calc_verify_tls_sha384( const mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SHA512_C */ #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ +int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) +{ + int ret; + + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) + { + if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, + ssl->handshake->premaster, + MBEDTLS_PREMASTER_SIZE, + &ssl->handshake->pmslen, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: K ", &ssl->handshake->dhm_ctx.K ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + { + if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, + &ssl->handshake->pmslen, + ssl->handshake->premaster, + MBEDTLS_MPI_MAX_SIZE, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_MPI( 3, "ECDH: z", &ssl->handshake->ecdh_ctx.z ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) + if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) + { + if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == + MBEDTLS_KEY_EXCHANGE_ECJPAKE ) + { + ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, + ssl->handshake->premaster, 32, &ssl->handshake->pmslen, + mbedtls_ssl_conf_get_frng( ssl->conf ), + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); + return( ret ); + } + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA ) + { + ((void) ret); + /* The premaster secret has already been by + * ssl_rsa_generate_partial_pms(). Only the + * PMS length needs to be set. */ + ssl->handshake->pmslen = 48; + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + return( 0 ); +} + #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exchange_type_t key_ex ) { From 04769ddb8439296bfbfb4e4158e61c058a6125b8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 09:03:36 +0100 Subject: [PATCH 13/38] Restructure SrvKeyExchange: Add frame for structure This commit adds declarations and dummy implementations for the restructured incoming server key exchange handling that will replace the previous ssl_parse_server_key_exchange(). The entry point for the SrvKeyExchange handling that is called from the handshake state machine is `ssl_process_server_key_exchange()`, splitting the processing into the following steps: - Preparation: For a static DH key exchange, extract DH parameters from the server's CRT. - Coordination: Check if a SrvKeyExchange message is expected (e.g., it isn't for a RSA-based key exchange) - Reading: Fetch and check content and handshake type of incoming message. - Parsing: Parse and store the ServerKeyExchange message. - Postprocessing: Update handstate state machine. The subsequent commits will scatter the code from the previous monolithic function ssl_parse_server_key_exchange() among those dedicated functions, commenting out each part of ssl_parse_server_key_exchange() that has already been dealt with. This gradual progression is meant to ease reviewing. Once all code has been moved and all changes explained, ssl_parse_server_key_exchange() will be removed. --- library/ssl_cli.c | 130 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 129 insertions(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 56ce907a35..0305a58653 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2595,6 +2595,134 @@ cleanup: #endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +/* + * + * STATE HANDLING: ServerKeyExchange + * + */ + +/* + * Overview + */ + +/* Main entry point; orchestrates the other functions. */ +static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ); + +/* Coordination: + * Check if a ServerKeyExchange message is expected, and skip if not. + * Returns a negative code on failure, or + * - SSL_SRV_KEY_EXCHANGE_SKIP + * - SSL_SRV_KEY_EXCHANGE_EXPECTED + * indicating if a ServerKeyExchange message is expected or not. + */ +#define SSL_SRV_KEY_EXCHANGE_SKIP 0 +#define SSL_SRV_KEY_EXCHANGE_EXPECTED 1 +static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ); +/* Preparation + * If applicable, prepare DH parameters from Server certificate. */ +static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ); +/* Parse SrvKeyExchange message and store contents + * (PSK or DH parameters) in handshake structure. */ +static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen ); +/* Update the handshake state */ +static int ssl_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ); + +/* + * Implementation + */ + +static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) +{ + int ret; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server key exchange" ) ); + + /* Preparation: + * Potentially extract DH parameters from Server's certificate. + * + * Consider: Why don't we do this as post-processing after + * the server certificate has been read? + */ + SSL_PROC_CHK( ssl_server_key_exchange_prepare( ssl ) ); + + /* Coordination: + * Check if we expect a ServerKeyExchange */ + SSL_PROC_CHK( ssl_server_key_exchange_coordinate( ssl ) ); + + if( ret == SSL_SRV_KEY_EXCHANGE_EXPECTED ) + { + /* Reading step */ + if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + + if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE || + ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); + mbedtls_ssl_pend_alert_message( ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE; + goto cleanup; + } + else + { + SSL_PROC_CHK( ssl_server_key_exchange_parse( ssl, ssl->in_msg, + ssl->in_hslen ) ); + } + } + else if( ret == SSL_SRV_KEY_EXCHANGE_SKIP ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); + } + + /* Update state */ + SSL_PROC_CHK( ssl_server_key_exchange_postprocess( ssl ) ); + +cleanup: + +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS ) + ssl->keep_current_message = 1; +#endif + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse server key exchange" ) ); + return( ret ); +} + +static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) +{ + /* TBD */ +} + +static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) +{ + /* TBD */ +} + +static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen ) +{ + /* TBD */ +} + +static int ssl_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ) +{ + /* TBD */ +} + +/* OLD CODE + * + * Temporarily included to gradually move it to the correct + * place in the restructured code. + * + */ + static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret; @@ -3997,7 +4125,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_SERVER_KEY_EXCHANGE: - ret = ssl_parse_server_key_exchange( ssl ); + ret = ssl_process_server_key_exchange( ssl ); break; case MBEDTLS_SSL_CERTIFICATE_REQUEST: From fca604d355c46f6288bbf5e6ad846c27f558ef69 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 09:12:05 +0100 Subject: [PATCH 14/38] Restructure SrvKeyExchange: Move static DH parameter extraction This code moves the code-path that extracts static DH parameters from the server's CRT (if applicable) to the new function ssl_server_key_exchange_prepare(). --- library/ssl_cli.c | 67 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 0305a58653..bc5eb5f6a5 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2663,8 +2663,7 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_pend_alert_message( ssl, - MBEDTLS_SSL_ALERT_LEVEL_FATAL, + mbedtls_ssl_pend_fatal_alert( ssl, MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE; goto cleanup; @@ -2696,7 +2695,33 @@ cleanup: static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) { - /* TBD */ + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = + ssl->transform_negotiate->ciphersuite_info; + ((void) ciphersuite_info); + + /* If applicable, extract static DH parameters from Server CRT. */ + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == + MBEDTLS_KEY_EXCHANGE_ECDH_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + { + int ret; + + if( ( ret = ssl_get_ecdh_params_from_cert( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); + mbedtls_ssl_pend_fatal_alert( ssl, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + return( ret ); + } + } +#endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ + + return( 0 ); } static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) @@ -2746,25 +2771,25 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_ECDH_RSA || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) - { - if( ( ret = ssl_get_ecdh_params_from_cert( ssl ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); - mbedtls_ssl_pend_fatal_alert( ssl, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); - return( ret ); - } +/* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ +/* MBEDTLS_KEY_EXCHANGE_ECDH_RSA || */ +/* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ +/* MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) */ +/* { */ +/* if( ( ret = ssl_get_ecdh_params_from_cert( ssl ) ) != 0 ) */ +/* { */ +/* MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); */ +/* mbedtls_ssl_pend_fatal_alert( ssl, */ +/* MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); */ +/* return( ret ); */ +/* } */ - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); - ssl->state++; - return( 0 ); - } - ((void) p); - ((void) end); +/* MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); */ +/* ssl->state++; */ +/* return( 0 ); */ +/* } */ +/* ((void) p); */ +/* ((void) end); */ #endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ From eb76c20496ade16aa0b4fc9512c589990c9afd13 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 09:14:46 +0100 Subject: [PATCH 15/38] Restructure SrvKeyExchange: Move code for skipping SrvKeyExchange This commit moves the code checking whether a SrvKeyExchange message is expected or not to the new function ssl_srv_key_exchange_coordinate(). Note that the potential static DH extraction is done prior to the coordination step. --- library/ssl_cli.c | 52 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index bc5eb5f6a5..d1f5d74db7 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2695,8 +2695,8 @@ cleanup: static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) { - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = - ssl->transform_negotiate->ciphersuite_info; + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); ((void) ciphersuite_info); /* If applicable, extract static DH parameters from Server CRT. */ @@ -2726,7 +2726,31 @@ static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) { - /* TBD */ + int ret; + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = + mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == + MBEDTLS_KEY_EXCHANGE_RSA ) + { + return( SSL_SRV_KEY_EXCHANGE_SKIP ); + } +#endif + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == + MBEDTLS_KEY_EXCHANGE_ECDH_RSA || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) + { + return( SSL_SRV_KEY_EXCHANGE_SKIP ); + } +#endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ + + return( SSL_SRV_KEY_EXCHANGE_EXPECTED ); } static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, @@ -2757,17 +2781,17 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server key exchange" ) ); -#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == - MBEDTLS_KEY_EXCHANGE_RSA ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); - ssl->state++; - return( 0 ); - } - ((void) p); - ((void) end); -#endif +/* #if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) */ +/* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ +/* MBEDTLS_KEY_EXCHANGE_RSA ) */ +/* { */ +/* MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); */ +/* ssl->state++; */ +/* return( 0 ); */ +/* } */ +/* ((void) p); */ +/* ((void) end); */ +/* #endif */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) From 8b7b879143beedea68da37fb29fac29e4177625c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 May 2018 09:30:17 +0100 Subject: [PATCH 16/38] Restructure SrvKeyExchange: Move msg skipping for PSK and RSA-PSK In the PSK and RSA-PSK ciphersuites, the ServerKeyExchange message MAY be skipped. This commit moves the code-path peeking at the incoming message to decide whether it's probably a ServerKeyExchange to the new coordination function ssl_server_key_exchange_coordinate(). --- library/ssl_cli.c | 102 ++++++++++++++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 34 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index d1f5d74db7..767620725c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2730,6 +2730,13 @@ static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); + /* The ServerKeyExchange message is not used for + * - RSA or + * - static ECDH + * ciphersuites. + * It MAY be used in PSK or RSA-PSK. + */ + #if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_RSA ) @@ -2750,6 +2757,33 @@ static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ + /* + * ServerKeyExchange may be skipped with PSK and RSA-PSK when the server + * doesn't use a psk_identity_hint. Peek at next message to decide whether + * the ServerKeyExchange is being skipped or not. + */ + + if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_PSK || + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) + == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) + { + if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + ssl->keep_current_message = 1; + + if( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE && + ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) + { + /* Current message is probably either + * CertificateRequest or ServerHelloDone */ + return( SSL_SRV_KEY_EXCHANGE_SKIP ); + } + } + return( SSL_SRV_KEY_EXCHANGE_EXPECTED ); } @@ -2825,44 +2859,44 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) } #endif - if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); - return( ret ); - } + /* if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); */ + /* return( ret ); */ + /* } */ - if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_pend_fatal_alert( ssl, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); - return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); - } + /* if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) */ + /* { */ + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); */ + /* mbedtls_ssl_pend_fatal_alert( ssl, */ + /* MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); */ + /* return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); */ + /* } */ - /* - * ServerKeyExchange may be skipped with PSK and RSA-PSK when the server - * doesn't use a psk_identity_hint - */ - if( ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) - { - if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_PSK || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) - { - /* Current message is probably either - * CertificateRequest or ServerHelloDone */ - ssl->keep_current_message = 1; - goto exit; - } + /* /\* */ + /* * ServerKeyExchange may be skipped with PSK and RSA-PSK when the server */ + /* * doesn't use a psk_identity_hint */ + /* *\/ */ + /* if( ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) */ + /* { */ + /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ + /* == MBEDTLS_KEY_EXCHANGE_PSK || */ + /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ + /* == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) */ + /* { */ + /* /\* Current message is probably either */ + /* * CertificateRequest or ServerHelloDone *\/ */ + /* ssl->keep_current_message = 1; */ + /* goto exit; */ + /* } */ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key exchange message must " - "not be skipped" ) ); - mbedtls_ssl_pend_fatal_alert( ssl, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key exchange message must " */ + /* "not be skipped" ) ); */ + /* mbedtls_ssl_pend_fatal_alert( ssl, */ + /* MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); */ - return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); - } + /* return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); */ + /* } */ #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) if( ssl->handshake->ecrs_enabled ) From 4ec73cb2516bfd1d430edbd9657f1292578868c9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 23 Jul 2019 09:28:03 +0100 Subject: [PATCH 17/38] Restructure SrvKeyExchange: Move parsing code --- library/ssl_cli.c | 126 ++++------------------------------------------ library/ssl_srv.c | 6 +++ 2 files changed, 17 insertions(+), 115 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 767620725c..8ddeec08d1 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2790,123 +2790,18 @@ static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) -{ - /* TBD */ -} - -static int ssl_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ) -{ - /* TBD */ -} - -/* OLD CODE - * - * Temporarily included to gradually move it to the correct - * place in the restructured code. - * - */ - -static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret; + unsigned char *p; + unsigned char *end; + mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); - unsigned char *p = NULL, *end = NULL; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server key exchange" ) ); + p = buf + mbedtls_ssl_hs_hdr_len( ssl ); + end = buf + buflen; -/* #if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) */ -/* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ -/* MBEDTLS_KEY_EXCHANGE_RSA ) */ -/* { */ -/* MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); */ -/* ssl->state++; */ -/* return( 0 ); */ -/* } */ -/* ((void) p); */ -/* ((void) end); */ -/* #endif */ - -#if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) -/* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ -/* MBEDTLS_KEY_EXCHANGE_ECDH_RSA || */ -/* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == */ -/* MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) */ -/* { */ -/* if( ( ret = ssl_get_ecdh_params_from_cert( ssl ) ) != 0 ) */ -/* { */ -/* MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); */ -/* mbedtls_ssl_pend_fatal_alert( ssl, */ -/* MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); */ -/* return( ret ); */ -/* } */ - -/* MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); */ -/* ssl->state++; */ -/* return( 0 ); */ -/* } */ -/* ((void) p); */ -/* ((void) end); */ -#endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ - -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_enabled && - ssl->handshake->ecrs_state == ssl_ecrs_ske_start_processing ) - { - goto start_processing; - } -#endif - - /* if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); */ - /* return( ret ); */ - /* } */ - - /* if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) */ - /* { */ - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); */ - /* mbedtls_ssl_pend_fatal_alert( ssl, */ - /* MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); */ - /* return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); */ - /* } */ - - /* /\* */ - /* * ServerKeyExchange may be skipped with PSK and RSA-PSK when the server */ - /* * doesn't use a psk_identity_hint */ - /* *\/ */ - /* if( ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) */ - /* { */ - /* if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ - /* == MBEDTLS_KEY_EXCHANGE_PSK || */ - /* mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) */ - /* == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) */ - /* { */ - /* /\* Current message is probably either */ - /* * CertificateRequest or ServerHelloDone *\/ */ - /* ssl->keep_current_message = 1; */ - /* goto exit; */ - /* } */ - - /* MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key exchange message must " */ - /* "not be skipped" ) ); */ - /* mbedtls_ssl_pend_fatal_alert( ssl, */ - /* MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); */ - - /* return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); */ - /* } */ - -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_enabled ) - ssl->handshake->ecrs_state = ssl_ecrs_ske_start_processing; - -start_processing: -#endif - p = ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ); - end = ssl->in_msg + ssl->in_hslen; - MBEDTLS_SSL_DEBUG_BUF( 3, "server key exchange", p, end - p ); + MBEDTLS_SSL_DEBUG_BUF( 3, "server key exchange", p, end - p ); #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) @@ -3186,11 +3081,12 @@ start_processing: } #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ -exit: - ssl->state++; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse server key exchange" ) ); + return( 0 ); +} +static int ssl_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ) +{ + ssl->state = MBEDTLS_SSL_CERTIFICATE_REQUEST; return( 0 ); } diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 63760c5e68..922196720b 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4218,6 +4218,12 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 2 ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_encrypted_pms" ), ret ); + +#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) + if ( ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ) + return( ret ); +#endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ + return( ret ); } } From 9db697e8c66af5eeae9beb7c353478168c911a1a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 23 Jul 2019 12:19:48 +0100 Subject: [PATCH 18/38] Async operations: Simplify restart logic --- library/ssl_srv.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 922196720b..51068caedb 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4071,22 +4071,6 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) /* The ClientKeyExchange message is never skipped. */ /* Reading step */ -#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) && \ - ( defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ) - if( ( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_RSA_PSK || - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) - == MBEDTLS_KEY_EXCHANGE_RSA ) && - ( ssl->handshake->async_in_progress != 0 ) ) - { - /* We've already read a record and there is an asynchronous - * operation in progress to decrypt it. So skip reading the - * record. */ - MBEDTLS_SSL_DEBUG_MSG( 3, ( "will resume decryption of previously-read record" ) ); - } - else -#endif if( ( ret = mbedtls_ssl_read_record( ssl, 1 /* update checksum */ ) ) != 0 ) { @@ -4112,6 +4096,11 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) cleanup: +#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) + if ( ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ) + ssl->keep_current_message = 1; +#endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= process client key exchange" ) ); return( ret ); } @@ -4218,12 +4207,6 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 2 ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "ssl_parse_encrypted_pms" ), ret ); - -#if defined(MBEDTLS_SSL_ASYNC_PRIVATE) - if ( ret == MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS ) - return( ret ); -#endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ - return( ret ); } } From 9a12243b012493043fdda9cc4e5fc21cc5c3ac7b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 23 Jul 2019 13:24:02 +0100 Subject: [PATCH 19/38] Introduce getter function for RNG context --- include/mbedtls/ssl_internal.h | 4 ++++ library/ssl_cli.c | 24 ++++++++++++------------ library/ssl_srv.c | 20 ++++++++++---------- library/ssl_tls.c | 12 ++++++------ 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 25ded986b7..e5c37eadee 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1560,6 +1560,10 @@ static inline mbedtls_ssl_recv_timeout_t* mbedtls_ssl_get_recv_timeout( typedef int mbedtls_frng_t( void*, unsigned char*, size_t ); +static inline void* mbedtls_ssl_conf_get_prng( mbedtls_ssl_config const *conf ) +{ + return( conf->p_rng ); +} #if !defined(MBEDTLS_SSL_CONF_RNG) static inline mbedtls_frng_t* mbedtls_ssl_conf_get_frng( mbedtls_ssl_config const *conf ) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 8ddeec08d1..c36a735078 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -375,7 +375,7 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, ret = mbedtls_ecjpake_write_round_one( &ssl->handshake->ecjpake_ctx, p + 2, end - p - 2, &kkpp_len, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1 , "mbedtls_ecjpake_write_round_one", ret ); @@ -735,7 +735,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, current time: %lu", t ) ); #else if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, p, 4 ) ) != 0 ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 4 ) ) != 0 ) { return( ret ); } @@ -744,7 +744,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_HAVE_TIME */ if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, p, 28 ) ) != 0 ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 28 ) ) != 0 ) { return( ret ); } @@ -911,7 +911,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) ssl->session_negotiate->ticket_len != 0 ) { ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, ssl->session_negotiate->id, 32 ); + ( mbedtls_ssl_conf_get_prng( ssl->conf ), ssl->session_negotiate->id, 32 ); if( ret != 0 ) return( ret ); @@ -2365,7 +2365,7 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, ssl->conf->transport, out ); if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, out + 2, 46 ) ) != 0 ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), out + 2, 46 ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); return( ret ); @@ -2435,7 +2435,7 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, ppms, 48, out + len_bytes, olen, buflen - len_bytes, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_rsa_pkcs1_encrypt", ret ); goto cleanup; @@ -3493,7 +3493,7 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), p, n, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); @@ -3530,7 +3530,7 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, &n, p, end - p, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); @@ -3625,7 +3625,7 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), p, n, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); @@ -3646,7 +3646,7 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, &n, p, buflen, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_public", ret ); @@ -3683,7 +3683,7 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, ret = mbedtls_ecjpake_write_round_two( &ssl->handshake->ecjpake_ctx, p, end - p, &n, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_write_round_two", ret ); @@ -3895,7 +3895,7 @@ sign: md_alg, hash_start, hashlen, ssl->out_msg + 6 + offset, &n, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng, rs_ctx ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ), rs_ctx ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_sign", ret ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 51068caedb..a31eda438a 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2603,7 +2603,7 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, ret = mbedtls_ecjpake_write_round_one( &ssl->handshake->ecjpake_ctx, p + 2, end - p - 2, &kkpp_len, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1 , "mbedtls_ecjpake_write_round_one", ret ); @@ -2783,7 +2783,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, current time: %lu", t ) ); #else if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, p, 4 ) ) != 0 ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 4 ) ) != 0 ) { return( ret ); } @@ -2792,7 +2792,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_HAVE_TIME */ if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, p, 28 ) ) != 0 ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 28 ) ) != 0 ) { return( ret ); } @@ -2859,7 +2859,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) { ssl->session_negotiate->id_len = n = 32; if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, ssl->session_negotiate->id, n ) ) != 0 ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), ssl->session_negotiate->id, n ) ) != 0 ) { return( ret ); } @@ -3272,7 +3272,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, ssl->out_msg + ssl->out_msglen, MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, &len, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_write_round_two", ret ); @@ -3336,7 +3336,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), ssl->out_msg + ssl->out_msglen, &len, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_params", ret ); return( ret ); @@ -3393,7 +3393,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, ssl->out_msg + ssl->out_msglen, MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); return( ret ); @@ -3579,7 +3579,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, ssl->out_msg + ssl->out_msglen + 2, signature_len, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_sign", ret ); return( ret ); @@ -3876,7 +3876,7 @@ static int ssl_decrypt_encrypted_pms( mbedtls_ssl_context *ssl, ret = mbedtls_pk_decrypt( private_key, p, len, peer_pms, peer_pmslen, peer_pmssize, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); return( ret ); } @@ -3946,7 +3946,7 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, * regardless of whether it will ultimately influence the output or not. */ ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( ssl->conf->p_rng, fake_pms, sizeof( fake_pms ) ); + ( mbedtls_ssl_conf_get_prng( ssl->conf ), fake_pms, sizeof( fake_pms ) ); if( ret != 0 ) { /* It's ok to abort on an RNG failure, since this does not reveal diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e651f29c1c..c63e08090e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1669,7 +1669,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) MBEDTLS_PREMASTER_SIZE, &ssl->handshake->pmslen, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); return( ret ); @@ -1697,7 +1697,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) ssl->handshake->premaster, MBEDTLS_MPI_MAX_SIZE, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); return( ret ); @@ -1729,7 +1729,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) ret = mbedtls_ecjpake_derive_secret( &ssl->handshake->ecjpake_ctx, ssl->handshake->premaster, 32, &ssl->handshake->pmslen, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ); + mbedtls_ssl_conf_get_prng( ssl->conf ) ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); @@ -1823,7 +1823,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, p + 2, end - ( p + 2 ), &len, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); return( ret ); @@ -1845,7 +1845,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, &zlen, p + 2, end - ( p + 2 ), mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); return( ret ); @@ -4189,7 +4189,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) if( ( ret = mbedtls_ssl_encrypt_buf( ssl, ssl->transform_out, &rec, mbedtls_ssl_conf_get_frng( ssl->conf ), - ssl->conf->p_rng ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_encrypt_buf", ret ); return( ret ); From 572d448ab234e8edaa3e7dc0dd95c49cf8c86382 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 23 Jul 2019 13:47:53 +0100 Subject: [PATCH 20/38] Enforce NULL context for hardcoded RNG --- configs/baremetal.h | 2 +- configs/baremetal_test.h | 2 -- include/mbedtls/config.h | 5 +++-- include/mbedtls/ssl.h | 11 +---------- include/mbedtls/ssl_internal.h | 16 +++++++++++----- library/ssl_tls.c | 6 ------ programs/ssl/dtls_client.c | 20 ++++++++++++++++++++ programs/ssl/dtls_server.c | 19 +++++++++++++++++++ programs/ssl/mini_client.c | 15 ++++++++++++++- programs/ssl/ssl_client1.c | 20 ++++++++++++++++++++ programs/ssl/ssl_client2.c | 16 +++++++++++++++- programs/ssl/ssl_fork_server.c | 19 +++++++++++++++++++ programs/ssl/ssl_mail_client.c | 19 +++++++++++++++++++ programs/ssl/ssl_pthread_server.c | 19 +++++++++++++++++++ programs/ssl/ssl_server.c | 19 +++++++++++++++++++ programs/ssl/ssl_server2.c | 16 +++++++++++++++- programs/x509/cert_app.c | 21 ++++++++++++++++++++- tests/suites/helpers.function | 12 ++++++++++++ 18 files changed, 227 insertions(+), 30 deletions(-) diff --git a/configs/baremetal.h b/configs/baremetal.h index 9929d56570..e4e3a2ac89 100644 --- a/configs/baremetal.h +++ b/configs/baremetal.h @@ -106,7 +106,7 @@ #define MBEDTLS_SSL_CONF_RECV mbedtls_net_recv #define MBEDTLS_SSL_CONF_SEND mbedtls_net_send #define MBEDTLS_SSL_CONF_RECV_TIMEOUT mbedtls_net_recv_timeout -#define MBEDTLS_SSL_CONF_RNG mbedtls_hmac_drbg_random +#define MBEDTLS_SSL_CONF_RNG rng_wrap #define MBEDTLS_SSL_CONF_MIN_MINOR_VER MBEDTLS_SSL_MINOR_VERSION_3 #define MBEDTLS_SSL_CONF_MAX_MINOR_VER MBEDTLS_SSL_MINOR_VERSION_3 #define MBEDTLS_SSL_CONF_MIN_MAJOR_VER MBEDTLS_SSL_MAJOR_VERSION_3 diff --git a/configs/baremetal_test.h b/configs/baremetal_test.h index 1b87474fc8..82c0ed17cf 100644 --- a/configs/baremetal_test.h +++ b/configs/baremetal_test.h @@ -51,8 +51,6 @@ /* ssl_client2 and ssl_server2 use CTR-DRBG so far. */ #define MBEDTLS_CTR_DRBG_C -#undef MBEDTLS_SSL_CONF_RNG -#define MBEDTLS_SSL_CONF_RNG mbedtls_ctr_drbg_random /* The ticket implementation hardcodes AES-GCM */ #define MBEDTLS_GCM_C diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 243678c62c..96413f850f 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3668,9 +3668,10 @@ /* The PRNG to use by the SSL module. If defined, this must * evaluate to the name on externally defined function with signature * int f_rng(void *, unsigned char *, size_t), - * e.g. mbedtls_ctr_drbg_random or mbedtls_hmac_drbg_random. + * which ignores its first parameter (the stack will always + * pass NULL to this function). */ -//#define MBEDTLS_SSL_CONF_RNG mbedtls_ctr_drbg_random +//#define MBEDTLS_SSL_CONF_RNG rng_wrap /* TLS version */ //#define MBEDTLS_SSL_CONF_MIN_MINOR_VER MBEDTLS_SSL_MINOR_VERSION_3 diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index ec2cf4574d..1f0997784a 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1000,8 +1000,8 @@ struct mbedtls_ssl_config #if !defined(MBEDTLS_SSL_CONF_RNG) /** Callback for getting (pseudo-)random numbers */ int (*f_rng)(void *, unsigned char *, size_t); -#endif /* !MBEDTLS_SSL_CONF_RNG */ void *p_rng; /*!< context for the RNG function */ +#endif /* !MBEDTLS_SSL_CONF_RNG */ #if defined(MBEDTLS_SSL_SRV_C) && !defined(MBEDTLS_SSL_NO_SESSION_CACHE) /** Callback to retrieve a session from the cache */ @@ -1601,15 +1601,6 @@ void mbedtls_ssl_conf_verify( mbedtls_ssl_config *conf, void mbedtls_ssl_conf_rng( mbedtls_ssl_config *conf, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); -#else /* !MBEDTLS_SSL_CONF_RNG */ -/** - * \brief Set the random number generator callback context. - * - * \param conf SSL configuration - * \param p_rng RNG parameter - */ -void mbedtls_ssl_conf_rng_ctx( mbedtls_ssl_config *conf, - void *p_rng ); #endif /* MBEDTLS_SSL_CONF_RNG */ #if defined(MBEDTLS_DEBUG_C) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index e5c37eadee..9cf80efb47 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1560,18 +1560,18 @@ static inline mbedtls_ssl_recv_timeout_t* mbedtls_ssl_get_recv_timeout( typedef int mbedtls_frng_t( void*, unsigned char*, size_t ); -static inline void* mbedtls_ssl_conf_get_prng( mbedtls_ssl_config const *conf ) -{ - return( conf->p_rng ); -} #if !defined(MBEDTLS_SSL_CONF_RNG) static inline mbedtls_frng_t* mbedtls_ssl_conf_get_frng( mbedtls_ssl_config const *conf ) { return( conf->f_rng ); } -#else /* !MBEDTLS_SSL_CONF_RNG */ +static inline void* mbedtls_ssl_conf_get_prng( mbedtls_ssl_config const *conf ) +{ + return( conf->p_rng ); +} +#else /* !MBEDTLS_SSL_CONF_RNG */ #define mbedtls_ssl_conf_rng_func MBEDTLS_SSL_CONF_RNG extern int mbedtls_ssl_conf_rng_func( void*, unsigned char*, size_t ); @@ -1581,6 +1581,12 @@ static inline mbedtls_frng_t* mbedtls_ssl_conf_get_frng( ((void) conf); return ((mbedtls_frng_t*) mbedtls_ssl_conf_rng_func); } + +static inline void* mbedtls_ssl_conf_get_prng( mbedtls_ssl_config const *conf ) +{ + ((void) conf); + return( NULL ); +} #endif /* MBEDTLS_SSL_CONF_RNG */ static inline int mbedtls_ssl_conf_get_max_major_ver( diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c63e08090e..2bb4fcd3cf 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8510,12 +8510,6 @@ void mbedtls_ssl_conf_rng( mbedtls_ssl_config *conf, conf->f_rng = f_rng; conf->p_rng = p_rng; } -#else -void mbedtls_ssl_conf_rng_ctx( mbedtls_ssl_config *conf, - void *p_rng ) -{ - conf->p_rng = p_rng; -} #endif #if defined(MBEDTLS_DEBUG_C) diff --git a/programs/ssl/dtls_client.c b/programs/ssl/dtls_client.c index 5bd5cbedd5..39a9f769f9 100644 --- a/programs/ssl/dtls_client.c +++ b/programs/ssl/dtls_client.c @@ -95,6 +95,20 @@ static void my_debug( void *ctx, int level, } #endif /* MBEDTLS_DEBUG_C */ +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( int argc, char *argv[] ) { int ret, len; @@ -192,7 +206,13 @@ int main( int argc, char *argv[] ) * Production code should set a proper ca chain and use REQUIRED. */ mbedtls_ssl_conf_authmode( &conf, MBEDTLS_SSL_VERIFY_OPTIONAL ); mbedtls_ssl_conf_ca_chain( &conf, &cacert, NULL ); + +#if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); #endif diff --git a/programs/ssl/dtls_server.c b/programs/ssl/dtls_server.c index 84b905d70d..1dddf8e1f1 100644 --- a/programs/ssl/dtls_server.c +++ b/programs/ssl/dtls_server.c @@ -104,6 +104,20 @@ static void my_debug( void *ctx, int level, } #endif /* MBEDTLS_DEBUG_C */ +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( void ) { int ret, len; @@ -224,7 +238,12 @@ int main( void ) goto exit; } +#if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); #endif diff --git a/programs/ssl/mini_client.c b/programs/ssl/mini_client.c index 4231c0c8fc..87c48ff4af 100644 --- a/programs/ssl/mini_client.c +++ b/programs/ssl/mini_client.c @@ -166,6 +166,19 @@ enum exit_codes ssl_write_failed, }; +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ int main( void ) { @@ -212,7 +225,7 @@ int main( void ) #if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); #else - mbedtls_ssl_conf_rng_ctx( &conf, &ctr_drbg ); + rng_ctx_global = &ctr_drbg; #endif #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) diff --git a/programs/ssl/ssl_client1.c b/programs/ssl/ssl_client1.c index 5f453dcb51..8629980f87 100644 --- a/programs/ssl/ssl_client1.c +++ b/programs/ssl/ssl_client1.c @@ -85,6 +85,20 @@ static void my_debug( void *ctx, int level, } #endif /* MBEDTLS_DEBUG_C */ +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( void ) { int ret = 1, len; @@ -179,7 +193,13 @@ int main( void ) * but makes interop easier in this simplified example */ mbedtls_ssl_conf_authmode( &conf, MBEDTLS_SSL_VERIFY_OPTIONAL ); mbedtls_ssl_conf_ca_chain( &conf, &cacert, NULL ); + +#if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); #endif diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 716263b1ae..a7fd2595de 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -889,6 +889,20 @@ int report_cid_usage( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( int argc, char *argv[] ) { int ret = 0, len, tail_len, i, written, frags, retry_left; @@ -1942,7 +1956,7 @@ int main( int argc, char *argv[] ) #if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); #else - mbedtls_ssl_conf_rng_ctx( &conf, &ctr_drbg ); + rng_ctx_global = &ctr_drbg; #endif #if defined(MBEDTLS_DEBUG_C) diff --git a/programs/ssl/ssl_fork_server.c b/programs/ssl/ssl_fork_server.c index 098761cc90..7033b86ce8 100644 --- a/programs/ssl/ssl_fork_server.c +++ b/programs/ssl/ssl_fork_server.c @@ -102,6 +102,20 @@ static void my_debug( void *ctx, int level, } #endif /* MBEDTLS_DEBUG_C */ +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( void ) { int ret = 1, len, cnt = 0, pid; @@ -196,7 +210,12 @@ int main( void ) goto exit; } +#if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); #endif diff --git a/programs/ssl/ssl_mail_client.c b/programs/ssl/ssl_mail_client.c index b2e4f7f399..205c93ec6d 100644 --- a/programs/ssl/ssl_mail_client.c +++ b/programs/ssl/ssl_mail_client.c @@ -361,6 +361,20 @@ static int write_and_get_response( mbedtls_net_context *sock_fd, unsigned char * while( 1 ); } +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( int argc, char *argv[] ) { int ret = 1, len; @@ -620,7 +634,12 @@ int main( int argc, char *argv[] ) * but makes interop easier in this simplified example */ mbedtls_ssl_conf_authmode( &conf, MBEDTLS_SSL_VERIFY_OPTIONAL ); +#if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); #endif diff --git a/programs/ssl/ssl_pthread_server.c b/programs/ssl/ssl_pthread_server.c index fd6ca26137..80bb049b9f 100644 --- a/programs/ssl/ssl_pthread_server.c +++ b/programs/ssl/ssl_pthread_server.c @@ -325,6 +325,20 @@ static int thread_create( mbedtls_net_context *client_fd ) return( 0 ); } +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( void ) { int ret; @@ -439,7 +453,12 @@ int main( void ) goto exit; } +#if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_mutexed_debug, stdout ); #endif diff --git a/programs/ssl/ssl_server.c b/programs/ssl/ssl_server.c index bf502a5a3a..e13af918f4 100644 --- a/programs/ssl/ssl_server.c +++ b/programs/ssl/ssl_server.c @@ -97,6 +97,20 @@ static void my_debug( void *ctx, int level, } #endif /* MBEDTLS_DEBUG_C */ +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( void ) { int ret, len; @@ -212,7 +226,12 @@ int main( void ) goto exit; } +#if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); #endif diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 3fa2b15023..e0e433714d 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1505,6 +1505,20 @@ int report_cid_usage( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( int argc, char *argv[] ) { int ret = 0, len, written, frags, exchanges_left; @@ -2754,7 +2768,7 @@ int main( int argc, char *argv[] ) #if !defined(MBEDTLS_SSL_CONF_RNG) mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); #else - mbedtls_ssl_conf_rng_ctx( &conf, &ctr_drbg ); + rng_ctx_global = &ctr_drbg; #endif #if defined(MBEDTLS_DEBUG_C) diff --git a/programs/x509/cert_app.c b/programs/x509/cert_app.c index cdd77f2732..9b40fa71ba 100644 --- a/programs/x509/cert_app.c +++ b/programs/x509/cert_app.c @@ -149,6 +149,20 @@ static int my_verify( void *data, mbedtls_x509_crt *crt, int depth, uint32_t *fl return( 0 ); } +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ); + +mbedtls_ctr_drbg_context *rng_ctx_global = NULL; +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + /* We expect the NULL parameter here. */ + if( ctx != NULL ) + return( -1 ); + + return( mbedtls_ctr_drbg_random( rng_ctx_global, dst, len ) ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + int main( int argc, char *argv[] ) { int ret = 1; @@ -424,7 +438,12 @@ int main( int argc, char *argv[] ) else mbedtls_ssl_conf_authmode( &conf, MBEDTLS_SSL_VERIFY_NONE ); - mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#if !defined(MBEDTLS_SSL_CONF_RNG) + mbedtls_ssl_conf_rng( &conf, mbedtls_ctr_drbg_random, &ctr_drbg ); +#else + rng_ctx_global = &ctr_drbg; +#endif + #if defined(MBEDTLS_DEBUG_C) mbedtls_ssl_conf_dbg( &conf, my_debug, stdout ); #endif diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index ccd4d42552..f5b61f31cd 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -356,6 +356,18 @@ void mbedtls_param_failed( const char *failure_condition, } #endif +#if defined(MBEDTLS_SSL_CONF_RNG) +int rng_wrap( void *ctx, unsigned char *dst, size_t len ) +{ + ((void) ctx); + ((void) dst); + ((void) len); + /* We don't expect test suites to use SSL functionality + * that would trigger the hardcoded RNG. */ + return( -1 ); +} +#endif /* MBEDTLS_SSL_CONF_RNG */ + #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) static int redirect_output( FILE** out_stream, const char* path ) { From 868cb586cc9c91480523504dc58c13c7256a1757 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 23 Jul 2019 14:29:13 +0100 Subject: [PATCH 21/38] Rename SSL_PROC_CHK -> MBEDTLS_SSL_CHK --- include/mbedtls/ssl_internal.h | 2 +- library/ssl_cli.c | 16 ++++++++-------- library/ssl_srv.c | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 9cf80efb47..a5c4ca1228 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1817,6 +1817,6 @@ MBEDTLS_ALWAYS_INLINE static inline void mbedtls_ssl_pend_fatal_alert( ssl->pending_fatal_alert_msg = message; } -#define SSL_PROC_CHK(f) do { if( ( ret = f ) < 0 ) goto cleanup; } while( 0 ) +#define MBEDTLS_SSL_CHK(f) do { if( ( ret = f ) < 0 ) goto cleanup; } while( 0 ) #endif /* ssl_internal.h */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index c36a735078..e672af1299 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2644,11 +2644,11 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) * Consider: Why don't we do this as post-processing after * the server certificate has been read? */ - SSL_PROC_CHK( ssl_server_key_exchange_prepare( ssl ) ); + MBEDTLS_SSL_CHK( ssl_server_key_exchange_prepare( ssl ) ); /* Coordination: * Check if we expect a ServerKeyExchange */ - SSL_PROC_CHK( ssl_server_key_exchange_coordinate( ssl ) ); + MBEDTLS_SSL_CHK( ssl_server_key_exchange_coordinate( ssl ) ); if( ret == SSL_SRV_KEY_EXCHANGE_EXPECTED ) { @@ -2670,7 +2670,7 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) } else { - SSL_PROC_CHK( ssl_server_key_exchange_parse( ssl, ssl->in_msg, + MBEDTLS_SSL_CHK( ssl_server_key_exchange_parse( ssl, ssl->in_msg, ssl->in_hslen ) ); } } @@ -2680,7 +2680,7 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) } /* Update state */ - SSL_PROC_CHK( ssl_server_key_exchange_postprocess( ssl ) ); + MBEDTLS_SSL_CHK( ssl_server_key_exchange_postprocess( ssl ) ); cleanup: @@ -3356,10 +3356,10 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) } #endif - SSL_PROC_CHK( ssl_client_key_exchange_prepare( ssl ) ); + MBEDTLS_SSL_CHK( ssl_client_key_exchange_prepare( ssl ) ); /* Prepare CertificateVerify message in output buffer. */ - SSL_PROC_CHK( ssl_client_key_exchange_write( ssl, ssl->out_msg, + MBEDTLS_SSL_CHK( ssl_client_key_exchange_write( ssl, ssl->out_msg, MBEDTLS_SSL_MAX_CONTENT_LEN, &ssl->out_msglen ) ); @@ -3370,11 +3370,11 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) cli_key_exchange_postprocess: #endif - SSL_PROC_CHK( ssl_client_key_exchange_postprocess( ssl ) ); + MBEDTLS_SSL_CHK( ssl_client_key_exchange_postprocess( ssl ) ); /* Dispatch message */ - SSL_PROC_CHK( mbedtls_ssl_write_handshake_msg( ssl ) ); + MBEDTLS_SSL_CHK( mbedtls_ssl_write_handshake_msg( ssl ) ); /* NOTE: For the new messaging layer, the postprocessing step * might come after the dispatching step if the latter diff --git a/library/ssl_srv.c b/library/ssl_srv.c index a31eda438a..413dfc2d13 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4088,11 +4088,11 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) goto cleanup; } - SSL_PROC_CHK( ssl_client_key_exchange_parse( ssl, ssl->in_msg, + MBEDTLS_SSL_CHK( ssl_client_key_exchange_parse( ssl, ssl->in_msg, ssl->in_hslen ) ); /* Update state */ - SSL_PROC_CHK( ssl_client_key_exchange_postprocess( ssl ) ); + MBEDTLS_SSL_CHK( ssl_client_key_exchange_postprocess( ssl ) ); cleanup: From a855cb635d3e22cab70c9e817b9e3455928d876a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 24 Jul 2019 14:57:54 +0100 Subject: [PATCH 22/38] Avoid unused variable warning in ServerKeyExchange parsing ssl_server_key_exchange_parse() is compiled even if there's no ciphersuite enabled which uses it (for example, that's the case in RSA-only builds). The rationale for that is to avoid cluttering the code with numerous compile-time guards. A consequence, however, is the top of ssl_server_key_exchange_parse() contains declarations for variables which are never put to use, and rightfully leading to compiler warnings. This commit silences these warnings by putting `((void) VAR);` statements in the branch which detects if we ever happen to call the function in an unexpected ciphersuite. --- library/ssl_cli.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index e672af1299..e8c4a09b4c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2892,6 +2892,10 @@ static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, else #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ { + ((void) ret); + ((void) p); + ((void) end); + ((void) ciphersuite_info); MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } From 7ba0a886bd5581a6a2f586e8cf5bd5ee301ae932 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 24 Jul 2019 17:07:41 +0100 Subject: [PATCH 23/38] Fix 'set but not used' ARM compiler warning --- library/ssl_srv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 413dfc2d13..3915315f5d 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4151,6 +4151,7 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, p, end - p) ) != 0 ) { + ((void) ret); MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_read_public", ret ); return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_RP ); } From 587c1ae1955374f5d6b0686c8bd3bf4af4fd341d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 25 Jul 2019 09:50:25 +0100 Subject: [PATCH 24/38] Make IAR happy by dummy-initializing an unused variable IAR doesn't like `((void) var);` as a means to indicate an unused variable if that variable hasn't been initialized before. Make it happy by initializing the variable before. --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index e8c4a09b4c..60bec57ddb 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3399,7 +3399,7 @@ cleanup: static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) { - int ret; + int ret = 0; mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); From 8793fab6358d776c53b7b1c27844ee8ff533524c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 1 Aug 2019 10:44:07 +0200 Subject: [PATCH 25/38] Fix two typos in comments --- library/ssl_cli.c | 2 +- library/ssl_tls.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 60bec57ddb..9c859eb332 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3330,7 +3330,7 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ); /* Preparation - * - For ECDH: Generate client params and derive premater secret + * - For ECDH: Generate client params and derive premaster secret * - For RSA-suites: Encrypt PMS * - For ECJPAKE: Do Round 2 */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2bb4fcd3cf..2d9c591631 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1743,7 +1743,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) == MBEDTLS_KEY_EXCHANGE_RSA ) { ((void) ret); - /* The premaster secret has already been by + /* The premaster secret has already been set by * ssl_rsa_generate_partial_pms(). Only the * PMS length needs to be set. */ ssl->handshake->pmslen = 48; From fb02e96cf5e1231be3e4211d46d0378aba924f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 1 Aug 2019 10:48:49 +0200 Subject: [PATCH 26/38] Fix indentation to a multiple of 4 spaces --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2d9c591631..2a50db6168 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1714,7 +1714,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) { if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); return( ret ); From 91cf7693b9fe3f9bbd36115d6e3784f3957c6cf6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 13:57:04 +0100 Subject: [PATCH 27/38] Remove restartable ECP return code check from ECDH suite handling mbedtls_ecdh_read_params() is not restartable. --- library/ssl_cli.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 9c859eb332..e898206939 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2274,10 +2274,6 @@ static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, (const unsigned char **) p, end ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ecdh_read_params" ), ret ); -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; -#endif return( ret ); } From 2fc84cebf647cb420a1232d535232f4bb21c4fd9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 13:58:17 +0100 Subject: [PATCH 28/38] Remove unused restartable ECC state for SrvKeyExchange Previously, ssl_ecrs_ske_start_processing was used to indicate that the ServerKeyExchange has been fetched from the record layer, but that parsing its ECDHE parameter component has been preempted by the restartable ECP feature. On re-entry of ssl_parse_server_key_exchange() in this state, the code would directly jump into the parsing routine. However, the only non-reentrant code that's jumped over this way is the record fetching routine mbedtls_ssl_parse_record(), which is now made re-entrant by setting `ssl->keep_current_message = 1` in case of pre-emption due to restartable ECC. The ssl_ecrs_ske_start_processing state is therefore redundant and can be removed, which is what this commit does. --- include/mbedtls/ssl_internal.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index a5c4ca1228..df02800215 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -404,7 +404,6 @@ struct mbedtls_ssl_handshake_params enum { /* this complements ssl->state with info on intra-state operations */ ssl_ecrs_none = 0, /*!< nothing going on (yet) */ ssl_ecrs_crt_verify, /*!< Certificate: crt_verify() */ - ssl_ecrs_ske_start_processing, /*!< ServerKeyExchange: pk_verify() */ ssl_ecrs_cke_ecdh_calc_secret, /*!< ClientKeyExchange: ECDH step 2 */ ssl_ecrs_crt_vrfy_sign, /*!< CertificateVerify: pk_sign() */ } ecrs_state; /*!< current (or last) operation */ From ae22dd3820ea81f5039f5042f4b015acd8948c3e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 14:43:22 +0100 Subject: [PATCH 29/38] Simplify logic of restartable ECDHE in CliKeyExch writing --- library/ssl_cli.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index e898206939..07ee2bbb0b 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3347,13 +3347,11 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> process client key exchange" ) ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_enabled ) - { - if( ssl->handshake->ecrs_state == ssl_ecrs_cke_ecdh_calc_secret ) - goto cli_key_exchange_postprocess; + if( ssl->handshake->ecrs_state == ssl_ecrs_cke_ecdh_calc_secret ) + goto cli_key_exchange_postprocess; + if( ssl->handshake->ecrs_enabled ) mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); - } #endif MBEDTLS_SSL_CHK( ssl_client_key_exchange_prepare( ssl ) ); @@ -3368,9 +3366,18 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) /* Calculate secrets and update state */ #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ssl->handshake->ecrs_enabled ) + ssl->handshake->ecrs_state = ssl_ecrs_cke_ecdh_calc_secret; + cli_key_exchange_postprocess: #endif - MBEDTLS_SSL_CHK( ssl_client_key_exchange_postprocess( ssl ) ); + + ret = ssl_client_key_exchange_postprocess( ssl ); +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) + ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; +#endif /* MBEDTLS_SSL__ECP_RESTARTABLE */ + MBEDTLS_SSL_CHK( ret ); /* Dispatch message */ @@ -3710,15 +3717,6 @@ static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) if( ( ret = mbedtls_ssl_build_pms( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_build_pms", ret ); - -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_enabled && - ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - { - ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; - ssl->handshake->ecrs_state = ssl_ecrs_cke_ecdh_calc_secret; - } -#endif /* MBEDTLS_SSL__ECP_RESTARTABLE */ return( ret ); } From 44a29f4a6f07afc7503fdd4bc9a1571c9c48e804 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 14:47:43 +0100 Subject: [PATCH 30/38] Remove redundant use of local variable in CliKeyExch writing --- library/ssl_cli.c | 1 - 1 file changed, 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 07ee2bbb0b..8e865924a1 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3591,7 +3591,6 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, { ((void) ret); ((void) end); - n = 0; } else #endif From 084694dd5774e7fb9fa2073242b41f4082e816fa Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 14:50:46 +0100 Subject: [PATCH 31/38] Fix copy-pasta in documentation for outgoing CliKeyExchange --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 8e865924a1..b38c2a3ed5 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3443,7 +3443,7 @@ static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_RSA ) { - /* For RSA-PSK, the premaster secret is composed of + /* For RSA, the premaster secret is composed of * - 2 bytes indicating the TLS version * - 46 randomly chosen bytes * which the following call generates. */ From aa49620f6e7501b555225bfa0dbac978c78df2c0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 14:55:33 +0100 Subject: [PATCH 32/38] Minor documentation improvement to ssl_rsa_encrypt_partial_pms() --- library/ssl_cli.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index b38c2a3ed5..aea5f12d5a 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2371,8 +2371,8 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, } /* - * Encrypt the Premaster Secret it with the server's RSA key and - * write it to the provided buffer. + * Encrypt the randomly chosen part of the Premaster Secret with the + * server's RSA key and write it to the provided buffer. */ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, unsigned char const *ppms, @@ -2418,7 +2418,7 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, } /* - * Encrypt the part of the premaster secret and write it out. + * Encrypt the partial premaster secret and write it out. */ if( ! mbedtls_pk_can_do( peer_pk, MBEDTLS_PK_RSA ) ) { From 48e526b3801e1f0e4fad12382e6d83d3203ba3b7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 14:59:32 +0100 Subject: [PATCH 33/38] Document parameter precondition for ssl_rsa_generate_partial_pms() --- library/ssl_cli.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index aea5f12d5a..5771035a5c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2334,6 +2334,9 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) +/* This function assumes that `out` points to a writable buffer + * of size 48 Bytes if `add_length_tag == 0` and size 50 Bytes + * if `add_length_tag != 0`. */ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, unsigned char* out, unsigned add_length_tag ) From 7d552fad652574dd8969fa8059ae2d5f794423fb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 15:04:59 +0100 Subject: [PATCH 34/38] Avoid collision of ssl_xxx_key_exchange_yyy() func names in cli/srv This makes grepping the functions more difficult, and also leads to compilation failures when trying to build the library from a single source file (which might be useful for code-size reasons). --- library/ssl_cli.c | 56 +++++++++++++++++++++++------------------------ library/ssl_srv.c | 24 ++++++++++---------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 5771035a5c..fd6b90d2ce 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2605,7 +2605,7 @@ cleanup: */ /* Main entry point; orchestrates the other functions. */ -static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ); +static int ssl_process_in_server_key_exchange( mbedtls_ssl_context *ssl ); /* Coordination: * Check if a ServerKeyExchange message is expected, and skip if not. @@ -2616,23 +2616,23 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ); */ #define SSL_SRV_KEY_EXCHANGE_SKIP 0 #define SSL_SRV_KEY_EXCHANGE_EXPECTED 1 -static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ); +static int ssl_in_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ); /* Preparation * If applicable, prepare DH parameters from Server certificate. */ -static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ); +static int ssl_in_server_key_exchange_prepare( mbedtls_ssl_context *ssl ); /* Parse SrvKeyExchange message and store contents * (PSK or DH parameters) in handshake structure. */ -static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, +static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ); /* Update the handshake state */ -static int ssl_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ); +static int ssl_in_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ); /* * Implementation */ -static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) +static int ssl_process_in_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server key exchange" ) ); @@ -2643,11 +2643,11 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) * Consider: Why don't we do this as post-processing after * the server certificate has been read? */ - MBEDTLS_SSL_CHK( ssl_server_key_exchange_prepare( ssl ) ); + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_prepare( ssl ) ); /* Coordination: * Check if we expect a ServerKeyExchange */ - MBEDTLS_SSL_CHK( ssl_server_key_exchange_coordinate( ssl ) ); + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_coordinate( ssl ) ); if( ret == SSL_SRV_KEY_EXCHANGE_EXPECTED ) { @@ -2669,7 +2669,7 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) } else { - MBEDTLS_SSL_CHK( ssl_server_key_exchange_parse( ssl, ssl->in_msg, + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_parse( ssl, ssl->in_msg, ssl->in_hslen ) ); } } @@ -2679,7 +2679,7 @@ static int ssl_process_server_key_exchange( mbedtls_ssl_context *ssl ) } /* Update state */ - MBEDTLS_SSL_CHK( ssl_server_key_exchange_postprocess( ssl ) ); + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_postprocess( ssl ) ); cleanup: @@ -2692,7 +2692,7 @@ cleanup: return( ret ); } -static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) +static int ssl_in_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) { mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); @@ -2723,7 +2723,7 @@ static int ssl_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) +static int ssl_in_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) { int ret; mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = @@ -2786,7 +2786,7 @@ static int ssl_server_key_exchange_coordinate( mbedtls_ssl_context *ssl ) return( SSL_SRV_KEY_EXCHANGE_EXPECTED ); } -static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, +static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) { @@ -3087,7 +3087,7 @@ static int ssl_server_key_exchange_parse( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ) +static int ssl_in_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_CERTIFICATE_REQUEST; return( 0 ); @@ -3326,25 +3326,25 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) */ /* Main entry point; orchestrates the other functions */ -static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ); +static int ssl_process_out_client_key_exchange( mbedtls_ssl_context *ssl ); /* Preparation * - For ECDH: Generate client params and derive premaster secret * - For RSA-suites: Encrypt PMS * - For ECJPAKE: Do Round 2 */ -static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ); -static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, +static int ssl_out_client_key_exchange_prepare( mbedtls_ssl_context *ssl ); +static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen, size_t *olen ); -static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ); +static int ssl_out_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ); /* * Implementation */ -static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) +static int ssl_process_out_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret = 0; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> process client key exchange" ) ); @@ -3357,10 +3357,10 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); #endif - MBEDTLS_SSL_CHK( ssl_client_key_exchange_prepare( ssl ) ); + MBEDTLS_SSL_CHK( ssl_out_client_key_exchange_prepare( ssl ) ); /* Prepare CertificateVerify message in output buffer. */ - MBEDTLS_SSL_CHK( ssl_client_key_exchange_write( ssl, ssl->out_msg, + MBEDTLS_SSL_CHK( ssl_out_client_key_exchange_write( ssl, ssl->out_msg, MBEDTLS_SSL_MAX_CONTENT_LEN, &ssl->out_msglen ) ); @@ -3375,7 +3375,7 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) cli_key_exchange_postprocess: #endif - ret = ssl_client_key_exchange_postprocess( ssl ); + ret = ssl_out_client_key_exchange_postprocess( ssl ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; @@ -3403,7 +3403,7 @@ cleanup: -static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) +static int ssl_out_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) { int ret = 0; mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = @@ -3460,7 +3460,7 @@ static int ssl_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, +static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen, size_t *olen ) @@ -3479,7 +3479,7 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, * It is therefore not suitable to be registered as a callback * for retransmission, if such get introduced at some point. * - * Also see the documentation of ssl_client_key_exchange_prepare(). + * Also see the documentation of ssl_out_client_key_exchange_prepare(). */ p = buf + 4; @@ -3712,7 +3712,7 @@ static int ssl_client_key_exchange_write( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) +static int ssl_out_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { int ret; @@ -4104,7 +4104,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_SERVER_KEY_EXCHANGE: - ret = ssl_process_server_key_exchange( ssl ); + ret = ssl_process_in_server_key_exchange( ssl ); break; case MBEDTLS_SSL_CERTIFICATE_REQUEST: @@ -4127,7 +4127,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_CLIENT_KEY_EXCHANGE: - ret = ssl_process_client_key_exchange( ssl ); + ret = ssl_process_out_client_key_exchange( ssl ); break; case MBEDTLS_SSL_CERTIFICATE_VERIFY: diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 3915315f5d..7aaea95d0b 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4051,19 +4051,19 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha */ /* Main entry point; orchestrates the other functions. */ -static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ); +static int ssl_process_in_client_key_exchange( mbedtls_ssl_context *ssl ); -static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t buflen ); +static int ssl_in_client_key_exchange_parse( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen ); /* Update the handshake state */ -static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ); +static int ssl_in_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ); /* * Implementation */ -static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) +static int ssl_process_in_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> process client key exchange" ) ); @@ -4088,11 +4088,11 @@ static int ssl_process_client_key_exchange( mbedtls_ssl_context *ssl ) goto cleanup; } - MBEDTLS_SSL_CHK( ssl_client_key_exchange_parse( ssl, ssl->in_msg, - ssl->in_hslen ) ); + MBEDTLS_SSL_CHK( ssl_in_client_key_exchange_parse( ssl, ssl->in_msg, + ssl->in_hslen ) ); /* Update state */ - MBEDTLS_SSL_CHK( ssl_client_key_exchange_postprocess( ssl ) ); + MBEDTLS_SSL_CHK( ssl_in_client_key_exchange_postprocess( ssl ) ); cleanup: @@ -4105,7 +4105,7 @@ cleanup: return( ret ); } -static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, +static int ssl_in_client_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) { @@ -4292,7 +4292,7 @@ static int ssl_client_key_exchange_parse( mbedtls_ssl_context *ssl, } /* Update the handshake state */ -static int ssl_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) +static int ssl_in_client_key_exchange_postprocess( mbedtls_ssl_context *ssl ) { int ret; @@ -4684,7 +4684,7 @@ int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_CLIENT_KEY_EXCHANGE: - ret = ssl_process_client_key_exchange( ssl ); + ret = ssl_process_in_client_key_exchange( ssl ); break; case MBEDTLS_SSL_CERTIFICATE_VERIFY: From 4e467098000dafa7b9ce0a55dd6243d117f6af90 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 15:12:35 +0100 Subject: [PATCH 35/38] Document precoditions on some HS parsing/writing functions Eventually, all HS parsing/writing functions should take an arbitrary buffer + length pair as their argument, and return MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL if the provided buffer is too short. So far, we've only made a first step by allowing to pass an arbitrary buffer, but don't yet add bounds checks throughout. While deliberate for now, this must be clearly documented. --- library/ssl_cli.c | 5 +++++ library/ssl_srv.c | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index fd6b90d2ce..83e7f252cd 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3460,6 +3460,11 @@ static int ssl_out_client_key_exchange_prepare( mbedtls_ssl_context *ssl ) return( 0 ); } +/* Warning: Despite accepting a length argument, this function is currently + * still lacking some bounds checks and assumes that `buf` has length + * `MBEDTLS_SSL_OUT_CONTENT_LEN`. Eventually, it should be rewritten to work + * with any buffer + length pair, returning MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL + * on insufficient writing space. */ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen, diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 7aaea95d0b..8ffbf7c0ba 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4105,6 +4105,11 @@ cleanup: return( ret ); } +/* Warning: Despite accepting a length argument, this function is currently + * still lacking some bounds checks and assumes that `buf` has length + * `MBEDTLS_SSL_IN_CONTENT_LEN`. Eventually, it should be rewritten to work + * with any buffer + length pair, returning MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL + * on insufficient parsing space. */ static int ssl_in_client_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) From 3328b1822ad4be9fae77c0c564f10de83d0759bf Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Aug 2019 15:18:29 +0100 Subject: [PATCH 36/38] Move ssl_process_in_server_key_exchange to avoid func use-before-def --- library/ssl_cli.c | 120 +++++++++++++++++++++++----------------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 83e7f252cd..c729194965 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2632,66 +2632,6 @@ static int ssl_in_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ); * Implementation */ -static int ssl_process_in_server_key_exchange( mbedtls_ssl_context *ssl ) -{ - int ret; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server key exchange" ) ); - - /* Preparation: - * Potentially extract DH parameters from Server's certificate. - * - * Consider: Why don't we do this as post-processing after - * the server certificate has been read? - */ - MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_prepare( ssl ) ); - - /* Coordination: - * Check if we expect a ServerKeyExchange */ - MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_coordinate( ssl ) ); - - if( ret == SSL_SRV_KEY_EXCHANGE_EXPECTED ) - { - /* Reading step */ - if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); - return( ret ); - } - - if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE || - ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_pend_fatal_alert( ssl, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); - ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE; - goto cleanup; - } - else - { - MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_parse( ssl, ssl->in_msg, - ssl->in_hslen ) ); - } - } - else if( ret == SSL_SRV_KEY_EXCHANGE_SKIP ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); - } - - /* Update state */ - MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_postprocess( ssl ) ); - -cleanup: - -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS ) - ssl->keep_current_message = 1; -#endif - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse server key exchange" ) ); - return( ret ); -} - static int ssl_in_server_key_exchange_prepare( mbedtls_ssl_context *ssl ) { mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = @@ -3093,6 +3033,66 @@ static int ssl_in_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ) return( 0 ); } +static int ssl_process_in_server_key_exchange( mbedtls_ssl_context *ssl ) +{ + int ret; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server key exchange" ) ); + + /* Preparation: + * Potentially extract DH parameters from Server's certificate. + * + * Consider: Why don't we do this as post-processing after + * the server certificate has been read? + */ + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_prepare( ssl ) ); + + /* Coordination: + * Check if we expect a ServerKeyExchange */ + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_coordinate( ssl ) ); + + if( ret == SSL_SRV_KEY_EXCHANGE_EXPECTED ) + { + /* Reading step */ + if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + + if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE || + ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); + mbedtls_ssl_pend_fatal_alert( ssl, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + ret = MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE; + goto cleanup; + } + else + { + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_parse( ssl, ssl->in_msg, + ssl->in_hslen ) ); + } + } + else if( ret == SSL_SRV_KEY_EXCHANGE_SKIP ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse server key exchange" ) ); + } + + /* Update state */ + MBEDTLS_SSL_CHK( ssl_in_server_key_exchange_postprocess( ssl ) ); + +cleanup: + +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS ) + ssl->keep_current_message = 1; +#endif + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse server key exchange" ) ); + return( ret ); +} + #if ! defined(MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED) static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { From 665d6989725633846d36ee795c9736ab323e011b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 13 Aug 2019 11:06:41 +0100 Subject: [PATCH 37/38] Fix typo in documentation of MBEDTLS_SSL_CONF_RNG in config.h --- include/mbedtls/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 96413f850f..69601de6a1 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3666,7 +3666,7 @@ //#define MBEDTLS_SSL_CONF_RECV_TIMEOUT mbedtls_net_recv_timeout /* The PRNG to use by the SSL module. If defined, this must - * evaluate to the name on externally defined function with signature + * evaluate to the name of an externally defined function with signature * int f_rng(void *, unsigned char *, size_t), * which ignores its first parameter (the stack will always * pass NULL to this function). From 0f36e7bfa39b3e888be86e0d653513ce7eb7e2fb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 13 Aug 2019 11:06:51 +0100 Subject: [PATCH 38/38] Explain why RNG ctx parameter is kept in MBEDTLS_SSL_CONF_RNG --- include/mbedtls/config.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 69601de6a1..930f7c7400 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3670,6 +3670,9 @@ * int f_rng(void *, unsigned char *, size_t), * which ignores its first parameter (the stack will always * pass NULL to this function). + * NB: The reason not to drop the context parameter entirely + * is for compatibility with the Crypto and X.509 libraries + * which use RNG callbacks including context parameters. */ //#define MBEDTLS_SSL_CONF_RNG rng_wrap