From b6ce0b6cd819611585adc807b4f48f3a1bc7f2fd Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 9 Mar 2022 15:38:24 +0100 Subject: [PATCH 01/12] ssl_prepare_server_key_exchange(): generate a private/public key and write out the curve identifier and public key using PSA Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index e9febfd843..50715b5840 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3107,6 +3107,89 @@ curve_matching_done: return( ret ); } +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + psa_key_attributes_t key_attributes; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + size_t ecdh_bits = 0; + uint8_t *p = ssl->out_msg + ssl->out_msglen; + const size_t header_size = 4; // curve_type, namedcurve, data length + const size_t data_length_size = 1; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); + + /* Convert EC group to PSA key type. */ + if( ( handshake->ecdh_psa_type = + PSA_KEY_TYPE_ECC_KEY_PAIR( mbedtls_ecc_group_to_psa( + (*curve)->grp_id, &ecdh_bits ) ) ) == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Could not convert ECC group to PSA." ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + if( ecdh_bits > 0xffff ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid ecdh_bits." ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + handshake->ecdh_bits = (uint16_t) ecdh_bits; + + key_attributes = psa_key_attributes_init(); + psa_set_key_usage_flags( &key_attributes, PSA_KEY_USAGE_DERIVE ); + psa_set_key_algorithm( &key_attributes, PSA_ALG_ECDH ); + psa_set_key_type( &key_attributes, handshake->ecdh_psa_type ); + psa_set_key_bits( &key_attributes, handshake->ecdh_bits ); + + /* + * ECParameters curve_params + * + * First byte is curve_type, always named_curve + */ + *p++ = MBEDTLS_ECP_TLS_NAMED_CURVE; + + /* + * Next two bytes are the namedcurve value + */ + MBEDTLS_PUT_UINT16_BE( (*curve)->tls_id, p, 0 ); + p += 2; + + /* Generate ECDH private key. */ + status = psa_generate_key( &key_attributes, + &handshake->ecdh_psa_privkey ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_generate_key", ret ); + return( ret ); + } + + /* + * ECPoint public + * + * First byte is data length. + * It will be filled later. p holds now the data length location. + */ + + /* Export the public part of the ECDH private key from PSA. + * Make one byte space for the length. + */ + status = psa_export_public_key( handshake->ecdh_psa_privkey, + p + data_length_size, + (size_t)( MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen - header_size ), + &len ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_export_public_key", ret ); + return( ret ); + } + + /* Store the length of the exported public key. */ + *p = (uint8_t) len; + + /* Determine full message length. */ + len += header_size; +#else if( ( ret = mbedtls_ecdh_make_params( &ssl->handshake->ecdh_ctx, &len, ssl->out_msg + ssl->out_msglen, @@ -3116,6 +3199,7 @@ curve_matching_done: MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); return( ret ); } +#endif #if defined(MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) dig_signed = ssl->out_msg + ssl->out_msglen; From fd32e9609b88e327663a555252eb41ea8eb48d5c Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 9 Mar 2022 15:40:52 +0100 Subject: [PATCH 02/12] ssl_parse_client_key_exchange(): read the curve identifier and the peer's public key and compute the shared secret using PSA Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 50715b5840..55d7802086 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3878,6 +3878,56 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { +#if defined(MBEDTLS_USE_PSA_CRYPTO) + size_t data_len = (size_t)( *p++ ); + size_t buf_len = (size_t)( end - p ); + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Read the peer's public key." ) ); + + /* + * We must have at least two bytes (1 for length, at least 1 for data) + */ + if( buf_len < 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid buffer length" ) ); + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } + + if( data_len < 1 || data_len > buf_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid data length" ) ); + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } + + /* Store peer's ECDH public key. */ + memcpy( handshake->ecdh_psa_peerkey, p, data_len ); + handshake->ecdh_psa_peerkey_len = data_len; + + /* Compute ECDH shared secret. */ + status = psa_raw_key_agreement( + PSA_ALG_ECDH, handshake->ecdh_psa_privkey, + handshake->ecdh_psa_peerkey, handshake->ecdh_psa_peerkey_len, + handshake->premaster, sizeof( handshake->premaster ), + &handshake->pmslen ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_raw_key_agreement", ret ); + return( ret ); + } + + status = psa_destroy_key( handshake->ecdh_psa_privkey ); + + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); + return( ret ); + } + handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; +#else if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, p, end - p) ) != 0 ) { @@ -3900,6 +3950,7 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_Z ); +#endif } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || From 130c4b556727fae09978bdc5971a420f7740e118 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 14 Mar 2022 09:18:24 +0100 Subject: [PATCH 03/12] Use PSA version of key agreement only for ECDHE keys This should be cleaned when server-side static ECDH (1.2) support is added (#5320). Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 123 +++++++++++++++++++++++++++++----------------- 1 file changed, 77 insertions(+), 46 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 55d7802086..23fc49d32b 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3879,55 +3879,86 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { #if defined(MBEDTLS_USE_PSA_CRYPTO) - size_t data_len = (size_t)( *p++ ); - size_t buf_len = (size_t)( end - p ); - psa_status_t status = PSA_ERROR_GENERIC_ERROR; - mbedtls_ssl_handshake_params *handshake = ssl->handshake; - - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Read the peer's public key." ) ); - - /* - * We must have at least two bytes (1 for length, at least 1 for data) - */ - if( buf_len < 2 ) + // Handle only ECDHE keys using PSA crypto. + if ( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid buffer length" ) ); - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - } + size_t data_len = (size_t)( *p++ ); + size_t buf_len = (size_t)( end - p ); + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; - if( data_len < 1 || data_len > buf_len ) + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Read the peer's public key." ) ); + + /* + * We must have at least two bytes (1 for length, at least 1 for data) + */ + if( buf_len < 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid buffer length" ) ); + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } + + if( data_len < 1 || data_len > buf_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid data length" ) ); + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } + + /* Store peer's ECDH public key. */ + memcpy( handshake->ecdh_psa_peerkey, p, data_len ); + handshake->ecdh_psa_peerkey_len = data_len; + + /* Compute ECDH shared secret. */ + status = psa_raw_key_agreement( + PSA_ALG_ECDH, handshake->ecdh_psa_privkey, + handshake->ecdh_psa_peerkey, handshake->ecdh_psa_peerkey_len, + handshake->premaster, sizeof( handshake->premaster ), + &handshake->pmslen ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_raw_key_agreement", ret ); + return( ret ); + } + + status = psa_destroy_key( handshake->ecdh_psa_privkey ); + + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); + return( ret ); + } + handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; + + } + else { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid data length" ) ); - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + 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_DECODE_ERROR ); + } + + 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, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, + MBEDTLS_DEBUG_ECDH_Z ); } - - /* Store peer's ECDH public key. */ - memcpy( handshake->ecdh_psa_peerkey, p, data_len ); - handshake->ecdh_psa_peerkey_len = data_len; - - /* Compute ECDH shared secret. */ - status = psa_raw_key_agreement( - PSA_ALG_ECDH, handshake->ecdh_psa_privkey, - handshake->ecdh_psa_peerkey, handshake->ecdh_psa_peerkey_len, - handshake->premaster, sizeof( handshake->premaster ), - &handshake->pmslen ); - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_raw_key_agreement", ret ); - return( ret ); - } - - status = psa_destroy_key( handshake->ecdh_psa_privkey ); - - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); - return( ret ); - } - handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; -#else +#else /* MBEDTLS_USE_PSA_CRYPTO */ if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, p, end - p) ) != 0 ) { @@ -3950,7 +3981,7 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_Z ); -#endif +#endif /* MBEDTLS_USE_PSA_CRYPTO */ } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || From e9f00445bc3e02fa4713cf6215eb7cf9e793995d Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 14 Mar 2022 09:42:32 +0100 Subject: [PATCH 04/12] Destroy ecdh_psa_privkey on failure Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 23fc49d32b..a873753d89 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3181,6 +3181,8 @@ curve_matching_done: { ret = psa_ssl_status_to_mbedtls( status ); MBEDTLS_SSL_DEBUG_RET( 1, "psa_export_public_key", ret ); + (void) psa_destroy_key( handshake->ecdh_psa_privkey ); + handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; return( ret ); } @@ -3919,6 +3921,8 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) { ret = psa_ssl_status_to_mbedtls( status ); MBEDTLS_SSL_DEBUG_RET( 1, "psa_raw_key_agreement", ret ); + (void) psa_destroy_key( handshake->ecdh_psa_privkey ); + handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; return( ret ); } From 0a60c129deb74e4e276d8bc733bc286652cebae2 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 14 Mar 2022 09:54:39 +0100 Subject: [PATCH 05/12] Add intermediate variables to increase code readability Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index a873753d89..7218e3e105 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3173,10 +3173,12 @@ curve_matching_done: /* Export the public part of the ECDH private key from PSA. * Make one byte space for the length. */ + unsigned char *own_pubkey = p + data_length_size; + size_t own_pubkey_max_len = (size_t)( MBEDTLS_SSL_OUT_CONTENT_LEN + - ssl->out_msglen - header_size ); + status = psa_export_public_key( handshake->ecdh_psa_privkey, - p + data_length_size, - (size_t)( MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen - header_size ), - &len ); + own_pubkey, own_pubkey_max_len, &len ); if( status != PSA_SUCCESS ) { ret = psa_ssl_status_to_mbedtls( status ); From a21af3da0066ae64757004e48d90fd70f76f251d Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 14 Mar 2022 10:09:13 +0100 Subject: [PATCH 06/12] Use mbedtls_psa_parse_tls_ecc_group() instead PSA_KEY_TYPE_ECC_KEY_PAIR( mbedtls_ecc_group_to_psa() ) Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 7218e3e105..e78e816b05 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3119,17 +3119,12 @@ curve_matching_done: MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); /* Convert EC group to PSA key type. */ - if( ( handshake->ecdh_psa_type = - PSA_KEY_TYPE_ECC_KEY_PAIR( mbedtls_ecc_group_to_psa( - (*curve)->grp_id, &ecdh_bits ) ) ) == 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Could not convert ECC group to PSA." ) ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - } + handshake->ecdh_psa_type = mbedtls_psa_parse_tls_ecc_group( + (*curve)->tls_id, &ecdh_bits ); - if( ecdh_bits > 0xffff ) + if( handshake->ecdh_psa_type == 0 || ecdh_bits > 0xffff ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid ecdh_bits." ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid ecc group parse." ) ); return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } handshake->ecdh_bits = (uint16_t) ecdh_bits; From fc91a1f030d69cc1d1fcc16e9e33b92232e88fd1 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 14 Mar 2022 12:05:27 +0100 Subject: [PATCH 07/12] Use PSA for private key generation and public key export only for ECDHE keys This should be cleaned when server-side static ECDH (1.2) support is added (#5320). Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 169 +++++++++++++++++++++++++--------------------- 1 file changed, 93 insertions(+), 76 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index e78e816b05..be4159391f 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3108,86 +3108,103 @@ curve_matching_done: } #if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_status_t status = PSA_ERROR_GENERIC_ERROR; - psa_key_attributes_t key_attributes; - mbedtls_ssl_handshake_params *handshake = ssl->handshake; - size_t ecdh_bits = 0; - uint8_t *p = ssl->out_msg + ssl->out_msglen; - const size_t header_size = 4; // curve_type, namedcurve, data length - const size_t data_length_size = 1; - - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); - - /* Convert EC group to PSA key type. */ - handshake->ecdh_psa_type = mbedtls_psa_parse_tls_ecc_group( - (*curve)->tls_id, &ecdh_bits ); - - if( handshake->ecdh_psa_type == 0 || ecdh_bits > 0xffff ) + // Handle only ECDHE keys using PSA crypto. + if ( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid ecc group parse." ) ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + psa_key_attributes_t key_attributes; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + size_t ecdh_bits = 0; + uint8_t *p = ssl->out_msg + ssl->out_msglen; + const size_t header_size = 4; // curve_type, namedcurve, data length + const size_t data_length_size = 1; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); + + /* Convert EC group to PSA key type. */ + handshake->ecdh_psa_type = mbedtls_psa_parse_tls_ecc_group( + (*curve)->tls_id, &ecdh_bits ); + + if( handshake->ecdh_psa_type == 0 || ecdh_bits > 0xffff ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid ecc group parse." ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + handshake->ecdh_bits = (uint16_t) ecdh_bits; + + key_attributes = psa_key_attributes_init(); + psa_set_key_usage_flags( &key_attributes, PSA_KEY_USAGE_DERIVE ); + psa_set_key_algorithm( &key_attributes, PSA_ALG_ECDH ); + psa_set_key_type( &key_attributes, handshake->ecdh_psa_type ); + psa_set_key_bits( &key_attributes, handshake->ecdh_bits ); + + /* + * ECParameters curve_params + * + * First byte is curve_type, always named_curve + */ + *p++ = MBEDTLS_ECP_TLS_NAMED_CURVE; + + /* + * Next two bytes are the namedcurve value + */ + MBEDTLS_PUT_UINT16_BE( (*curve)->tls_id, p, 0 ); + p += 2; + + /* Generate ECDH private key. */ + status = psa_generate_key( &key_attributes, + &handshake->ecdh_psa_privkey ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_generate_key", ret ); + return( ret ); + } + + /* + * ECPoint public + * + * First byte is data length. + * It will be filled later. p holds now the data length location. + */ + + /* Export the public part of the ECDH private key from PSA. + * Make one byte space for the length. + */ + unsigned char *own_pubkey = p + data_length_size; + size_t own_pubkey_max_len = (size_t)( MBEDTLS_SSL_OUT_CONTENT_LEN + - ssl->out_msglen - header_size ); + + status = psa_export_public_key( handshake->ecdh_psa_privkey, + own_pubkey, own_pubkey_max_len, &len ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_export_public_key", ret ); + (void) psa_destroy_key( handshake->ecdh_psa_privkey ); + handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; + return( ret ); + } + + /* Store the length of the exported public key. */ + *p = (uint8_t) len; + + /* Determine full message length. */ + len += header_size; } - handshake->ecdh_bits = (uint16_t) ecdh_bits; - - key_attributes = psa_key_attributes_init(); - psa_set_key_usage_flags( &key_attributes, PSA_KEY_USAGE_DERIVE ); - psa_set_key_algorithm( &key_attributes, PSA_ALG_ECDH ); - psa_set_key_type( &key_attributes, handshake->ecdh_psa_type ); - psa_set_key_bits( &key_attributes, handshake->ecdh_bits ); - - /* - * ECParameters curve_params - * - * First byte is curve_type, always named_curve - */ - *p++ = MBEDTLS_ECP_TLS_NAMED_CURVE; - - /* - * Next two bytes are the namedcurve value - */ - MBEDTLS_PUT_UINT16_BE( (*curve)->tls_id, p, 0 ); - p += 2; - - /* Generate ECDH private key. */ - status = psa_generate_key( &key_attributes, - &handshake->ecdh_psa_privkey ); - if( status != PSA_SUCCESS ) + else { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_generate_key", ret ); - return( ret ); + if( ( ret = mbedtls_ecdh_make_params( + &ssl->handshake->ecdh_ctx, &len, + ssl->out_msg + ssl->out_msglen, + MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); + return( ret ); + } } - - /* - * ECPoint public - * - * First byte is data length. - * It will be filled later. p holds now the data length location. - */ - - /* Export the public part of the ECDH private key from PSA. - * Make one byte space for the length. - */ - unsigned char *own_pubkey = p + data_length_size; - size_t own_pubkey_max_len = (size_t)( MBEDTLS_SSL_OUT_CONTENT_LEN - - ssl->out_msglen - header_size ); - - status = psa_export_public_key( handshake->ecdh_psa_privkey, - own_pubkey, own_pubkey_max_len, &len ); - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_export_public_key", ret ); - (void) psa_destroy_key( handshake->ecdh_psa_privkey ); - handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; - return( ret ); - } - - /* Store the length of the exported public key. */ - *p = (uint8_t) len; - - /* Determine full message length. */ - len += header_size; #else if( ( ret = mbedtls_ecdh_make_params( &ssl->handshake->ecdh_ctx, &len, From ce1d7923151d6bfad2fde252b077dff2f4a62619 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 14 Mar 2022 16:16:25 +0100 Subject: [PATCH 08/12] Remove duplicated code Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 167 ++++++++++++++++++---------------------------- 1 file changed, 66 insertions(+), 101 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index be4159391f..4f91b8ba3d 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3108,9 +3108,8 @@ curve_matching_done: } #if defined(MBEDTLS_USE_PSA_CRYPTO) - // Handle only ECDHE keys using PSA crypto. - if ( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) + if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) { psa_status_t status = PSA_ERROR_GENERIC_ERROR; psa_key_attributes_t key_attributes; @@ -3194,28 +3193,16 @@ curve_matching_done: len += header_size; } else - { - if( ( ret = mbedtls_ecdh_make_params( +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + if( ( ret = mbedtls_ecdh_make_params( &ssl->handshake->ecdh_ctx, &len, ssl->out_msg + ssl->out_msglen, MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); - return( ret ); - } - } -#else - if( ( ret = mbedtls_ecdh_make_params( - &ssl->handshake->ecdh_ctx, &len, - ssl->out_msg + ssl->out_msglen, - MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); return( ret ); } -#endif #if defined(MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) dig_signed = ssl->out_msg + ssl->out_msglen; @@ -3885,6 +3872,68 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) } else #endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) && \ + ( defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) ) + if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || + ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) + { + size_t data_len = (size_t)( *p++ ); + size_t buf_len = (size_t)( end - p ); + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Read the peer's public key." ) ); + + /* + * We must have at least two bytes (1 for length, at least 1 for data) + */ + if( buf_len < 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid buffer length" ) ); + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } + + if( data_len < 1 || data_len > buf_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid data length" ) ); + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } + + /* Store peer's ECDH public key. */ + memcpy( handshake->ecdh_psa_peerkey, p, data_len ); + handshake->ecdh_psa_peerkey_len = data_len; + + /* Compute ECDH shared secret. */ + status = psa_raw_key_agreement( + PSA_ALG_ECDH, handshake->ecdh_psa_privkey, + handshake->ecdh_psa_peerkey, handshake->ecdh_psa_peerkey_len, + handshake->premaster, sizeof( handshake->premaster ), + &handshake->pmslen ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_raw_key_agreement", ret ); + (void) psa_destroy_key( handshake->ecdh_psa_privkey ); + handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; + return( ret ); + } + + status = psa_destroy_key( handshake->ecdh_psa_privkey ); + + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); + return( ret ); + } + handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; + } + else + +#endif /* MBEDTLS_USE_PSA_CRYPTO && + ( MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || + MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED ) */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ @@ -3894,89 +3943,6 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDH_RSA || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { -#if defined(MBEDTLS_USE_PSA_CRYPTO) - // Handle only ECDHE keys using PSA crypto. - if ( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) - { - size_t data_len = (size_t)( *p++ ); - size_t buf_len = (size_t)( end - p ); - psa_status_t status = PSA_ERROR_GENERIC_ERROR; - mbedtls_ssl_handshake_params *handshake = ssl->handshake; - - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Read the peer's public key." ) ); - - /* - * We must have at least two bytes (1 for length, at least 1 for data) - */ - if( buf_len < 2 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid buffer length" ) ); - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - } - - if( data_len < 1 || data_len > buf_len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid data length" ) ); - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - } - - /* Store peer's ECDH public key. */ - memcpy( handshake->ecdh_psa_peerkey, p, data_len ); - handshake->ecdh_psa_peerkey_len = data_len; - - /* Compute ECDH shared secret. */ - status = psa_raw_key_agreement( - PSA_ALG_ECDH, handshake->ecdh_psa_privkey, - handshake->ecdh_psa_peerkey, handshake->ecdh_psa_peerkey_len, - handshake->premaster, sizeof( handshake->premaster ), - &handshake->pmslen ); - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_raw_key_agreement", ret ); - (void) psa_destroy_key( handshake->ecdh_psa_privkey ); - handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; - return( ret ); - } - - status = psa_destroy_key( handshake->ecdh_psa_privkey ); - - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); - return( ret ); - } - handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; - - } - else - { - 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_DECODE_ERROR ); - } - - 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, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_Z ); - } -#else /* MBEDTLS_USE_PSA_CRYPTO */ if( ( ret = mbedtls_ecdh_read_public( &ssl->handshake->ecdh_ctx, p, end - p) ) != 0 ) { @@ -3999,7 +3965,6 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_Z ); -#endif /* MBEDTLS_USE_PSA_CRYPTO */ } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || From 338b61d6e42bc57bd5ca543ed0ec711ab79a6e3e Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 15 Mar 2022 08:03:43 +0100 Subject: [PATCH 09/12] Fix code style Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 4f91b8ba3d..ae1dd54eff 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3139,21 +3139,21 @@ curve_matching_done: psa_set_key_bits( &key_attributes, handshake->ecdh_bits ); /* - * ECParameters curve_params - * - * First byte is curve_type, always named_curve - */ + * ECParameters curve_params + * + * First byte is curve_type, always named_curve + */ *p++ = MBEDTLS_ECP_TLS_NAMED_CURVE; /* - * Next two bytes are the namedcurve value - */ + * Next two bytes are the namedcurve value + */ MBEDTLS_PUT_UINT16_BE( (*curve)->tls_id, p, 0 ); p += 2; /* Generate ECDH private key. */ status = psa_generate_key( &key_attributes, - &handshake->ecdh_psa_privkey ); + &handshake->ecdh_psa_privkey ); if( status != PSA_SUCCESS ) { ret = psa_ssl_status_to_mbedtls( status ); @@ -3162,21 +3162,22 @@ curve_matching_done: } /* - * ECPoint public - * - * First byte is data length. - * It will be filled later. p holds now the data length location. - */ + * ECPoint public + * + * First byte is data length. + * It will be filled later. p holds now the data length location. + */ /* Export the public part of the ECDH private key from PSA. - * Make one byte space for the length. - */ + * Make one byte space for the length. + */ unsigned char *own_pubkey = p + data_length_size; size_t own_pubkey_max_len = (size_t)( MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen - header_size ); status = psa_export_public_key( handshake->ecdh_psa_privkey, - own_pubkey, own_pubkey_max_len, &len ); + own_pubkey, own_pubkey_max_len, + &len ); if( status != PSA_SUCCESS ) { ret = psa_ssl_status_to_mbedtls( status ); @@ -3195,10 +3196,10 @@ curve_matching_done: else #endif /* MBEDTLS_USE_PSA_CRYPTO */ if( ( ret = mbedtls_ecdh_make_params( - &ssl->handshake->ecdh_ctx, &len, - ssl->out_msg + ssl->out_msglen, - MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + &ssl->handshake->ecdh_ctx, &len, + ssl->out_msg + ssl->out_msglen, + MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); return( ret ); @@ -3886,8 +3887,8 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 1, ( "Read the peer's public key." ) ); /* - * We must have at least two bytes (1 for length, at least 1 for data) - */ + * We must have at least two bytes (1 for length, at least 1 for data) + */ if( buf_len < 2 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid buffer length" ) ); @@ -3930,7 +3931,6 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; } else - #endif /* MBEDTLS_USE_PSA_CRYPTO && ( MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED ) */ From 855938e17d7eda5b20536fa535e9277e60de4a98 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 16 Mar 2022 11:29:29 +0100 Subject: [PATCH 10/12] Move mbedtls_ecdh_setup() to no-psa path Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index ae1dd54eff..128c01b094 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3100,13 +3100,6 @@ curve_matching_done: MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDHE curve: %s", (*curve)->name ) ); - if( ( ret = mbedtls_ecdh_setup( &ssl->handshake->ecdh_ctx, - (*curve)->grp_id ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecp_group_load", ret ); - return( ret ); - } - #if defined(MBEDTLS_USE_PSA_CRYPTO) if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA ) @@ -3195,14 +3188,23 @@ curve_matching_done: } else #endif /* MBEDTLS_USE_PSA_CRYPTO */ - if( ( ret = mbedtls_ecdh_make_params( - &ssl->handshake->ecdh_ctx, &len, - ssl->out_msg + ssl->out_msglen, - MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); - return( ret ); + if( ( ret = mbedtls_ecdh_setup( &ssl->handshake->ecdh_ctx, + (*curve)->grp_id ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecp_group_load", ret ); + return( ret ); + } + + if( ( ret = mbedtls_ecdh_make_params( + &ssl->handshake->ecdh_ctx, &len, + ssl->out_msg + ssl->out_msglen, + MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_params", ret ); + return( ret ); + } } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) From a4e15cc0d56e01dbf6dff9b4bb47c54e82a32bec Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 16 Mar 2022 11:32:42 +0100 Subject: [PATCH 11/12] Fix comment: add fields size Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 128c01b094..87e6b64bde 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3109,7 +3109,8 @@ curve_matching_done: mbedtls_ssl_handshake_params *handshake = ssl->handshake; size_t ecdh_bits = 0; uint8_t *p = ssl->out_msg + ssl->out_msglen; - const size_t header_size = 4; // curve_type, namedcurve, data length + const size_t header_size = 4; // curve_type(1), namedcurve(2), + // data length(1) const size_t data_length_size = 1; MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); From dd482bfd6a5194b6a5d83e1f9159053b971cc35f Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 16 Mar 2022 11:43:22 +0100 Subject: [PATCH 12/12] Modify own_pubkey_max_len calculation Signed-off-by: Przemek Stekiel --- library/ssl_srv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 87e6b64bde..dc5b8a550e 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3166,8 +3166,9 @@ curve_matching_done: * Make one byte space for the length. */ unsigned char *own_pubkey = p + data_length_size; + size_t own_pubkey_max_len = (size_t)( MBEDTLS_SSL_OUT_CONTENT_LEN - - ssl->out_msglen - header_size ); + - ( own_pubkey - ssl->out_msg ) ); status = psa_export_public_key( handshake->ecdh_psa_privkey, own_pubkey, own_pubkey_max_len,