From e40a2f7a9971963ae2a3cb29bc6ae19173df1048 Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Wed, 25 Mar 2020 16:57:00 +0000 Subject: [PATCH 1/9] Improve documentation about PSK configuration Signed-off-by: Guilhem Bryant --- include/mbedtls/ssl.h | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 1a071fc30..e9d1c1ea7 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2682,6 +2682,9 @@ int mbedtls_ssl_conf_own_cert( mbedtls_ssl_config *conf, * \note This is mainly useful for clients. Servers will usually * want to use \c mbedtls_ssl_conf_psk_cb() instead. * + * \note A PSK set by \c mbedtls_ssl_set_hs_psk() in the PSK callback + * takes precedence over a PSK configured by this function. + * * \warning Currently, clients can only register a single pre-shared key. * Calling this function or mbedtls_ssl_conf_psk_opaque() more * than once will overwrite values configured in previous calls. @@ -2715,6 +2718,10 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf, * \note This is mainly useful for clients. Servers will usually * want to use \c mbedtls_ssl_conf_psk_cb() instead. * + * \note An opaque PSK set by \c mbedtls_ssl_set_hs_psk_opaque() in + * the PSK callback takes precedence over an opaque PSK + * configured by this function. + * * \warning Currently, clients can only register a single pre-shared key. * Calling this function or mbedtls_ssl_conf_psk() more than * once will overwrite values configured in previous calls. @@ -2752,6 +2759,9 @@ int mbedtls_ssl_conf_psk_opaque( mbedtls_ssl_config *conf, * \note This should only be called inside the PSK callback, * i.e. the function passed to \c mbedtls_ssl_conf_psk_cb(). * + * \note A PSK set by this function takes precedence over a PSK + * configured by \c mbedtls_ssl_conf_psk(). + * * \param ssl The SSL context to configure a PSK for. * \param psk The pointer to the pre-shared key. * \param psk_len The length of the pre-shared key in bytes. @@ -2769,6 +2779,9 @@ int mbedtls_ssl_set_hs_psk( mbedtls_ssl_context *ssl, * \note This should only be called inside the PSK callback, * i.e. the function passed to \c mbedtls_ssl_conf_psk_cb(). * + * \note An opaque PSK set by this function takes precedence over an + * opaque PSK configured by \c mbedtls_ssl_conf_psk_opaque(). + * * \param ssl The SSL context to configure a PSK for. * \param psk The identifier of the key slot holding the PSK. * For the duration of the current handshake, the key slot @@ -2807,9 +2820,14 @@ int mbedtls_ssl_set_hs_psk_opaque( mbedtls_ssl_context *ssl, * on the SSL context to set the correct PSK and return \c 0. * Any other return value will result in a denied PSK identity. * - * \note If you set a PSK callback using this function, then you - * don't need to set a PSK key and identity using - * \c mbedtls_ssl_conf_psk(). + * \note A dynamic PSK (i.e. set by the PSK callback) takes + * precedence over a static PSK (i.e. set by + * \c mbedtls_ssl_conf_psk() or + * \c mbedtls_ssl_conf_psk_opaque()). + * This means that if you set a PSK callback using this + * function, you don't need to set a PSK using + * \c mbedtls_ssl_conf_psk() or + * \c mbedtls_ssl_conf_psk_opaque()). * * \param conf The SSL configuration to register the callback with. * \param f_psk The callback for selecting and setting the PSK based From d511ac34193eb10ef400a0ec9b4c4c4230062b38 Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Wed, 25 Mar 2020 17:06:37 +0000 Subject: [PATCH 2/9] Define internal PSK getter Signed-off-by: Guilhem Bryant --- include/mbedtls/ssl_internal.h | 53 +++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 9ff61fd3c..ed852e8dd 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -921,7 +921,58 @@ void mbedtls_ssl_optimize_checksum( 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 + +/* + * Get the first defined PSK by order of precedence: + * 1. handshake PSK set by \c mbedtls_ssl_set_hs_psk() in the PSK callback + * 2. static PSK configured by \c mbedtls_ssl_conf_psk() + * Return a code and update the pair (PSK, PSK length) passed to this function + */ +static inline int mbedtls_ssl_get_psk( const mbedtls_ssl_context *ssl, + const unsigned char **psk, size_t *psk_len ) +{ + if( ssl->handshake->psk != NULL && ssl->handshake->psk_len > 0 ) + { + *psk = ssl->handshake->psk; + *psk_len = ssl->handshake->psk_len; + } + + else if( ssl->conf->psk != NULL && ssl->conf->psk_len > 0 ) + { + *psk = ssl->conf->psk; + *psk_len = ssl->conf->psk_len; + } + + else + { + return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); + } + + return( 0 ); +} + +#if defined(MBEDTLS_USE_PSA_CRYPTO) +/* + * Get the first defined opaque PSK by order of precedence: + * 1. handshake PSK set by \c mbedtls_ssl_set_hs_psk_opaque() in the PSK + * callback + * 2. static PSK configured by \c mbedtls_ssl_conf_psk_opaque() + * Return an opaque PSK + */ +static inline psa_key_handle_t mbedtls_ssl_get_opaque_psk( + const mbedtls_ssl_context *ssl ) +{ + if( ssl->handshake->psk_opaque != 0 ) + return( ssl->handshake->psk_opaque ); + + if( ssl->conf->psk_opaque != 0 ) + return( ssl->conf->psk_opaque ); + + return( 0 ); +} +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + +#endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ #if defined(MBEDTLS_PK_C) unsigned char mbedtls_ssl_sig_from_pk( mbedtls_pk_context *pk ); From c5285d8c40119ed3f518ac4b0820c64851561d4d Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Wed, 25 Mar 2020 17:08:15 +0000 Subject: [PATCH 3/9] Use internal PSK getter Signed-off-by: Guilhem Bryant --- library/ssl_tls.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 990fa5c0c..4abf02a20 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1514,9 +1514,7 @@ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, MBEDTLS_SSL_DEBUG_MSG( 2, ( "perform PSA-based PSK-to-MS expansion" ) ); - psk = ssl->conf->psk_opaque; - if( handshake->psk_opaque != 0 ) - psk = handshake->psk_opaque; + psk = mbedtls_ssl_get_opaque_psk( ssl ); if( hash_alg == MBEDTLS_MD_SHA384 ) alg = PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_384); @@ -1850,15 +1848,16 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch { unsigned char *p = ssl->handshake->premaster; unsigned char *end = p + sizeof( ssl->handshake->premaster ); - const unsigned char *psk = ssl->conf->psk; - size_t psk_len = ssl->conf->psk_len; + const unsigned char *psk; + size_t psk_len; - /* If the psk callback was called, use its result */ - if( ssl->handshake->psk != NULL ) - { - psk = ssl->handshake->psk; - psk_len = ssl->handshake->psk_len; - } + if( mbedtls_ssl_get_psk( ssl, &psk, &psk_len ) + == MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ) + /* + * This should never happen because the existence of a PSK is always + * checked before calling this function + */ + MBEDTLS_SSL_DEBUG_MSG(1, ("should never happen")); /* * PMS = struct { From 82194c888d1b0c6471b333d91f764ff8da5d045f Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Thu, 26 Mar 2020 17:04:31 +0000 Subject: [PATCH 4/9] Fix bracketing style in ssl_tls.c Signed-off-by: Guilhem Bryant --- 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 4abf02a20..8dbae7aaa 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1857,7 +1857,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch * This should never happen because the existence of a PSK is always * checked before calling this function */ - MBEDTLS_SSL_DEBUG_MSG(1, ("should never happen")); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); /* * PMS = struct { From 61b0fe617e51877bb51a86d6e01b78db435029da Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Fri, 27 Mar 2020 11:12:12 +0000 Subject: [PATCH 5/9] Initialise psk_len in mbedtls_ssl_psk_derive_premaster() Signed-off-by: Guilhem Bryant --- 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 8dbae7aaa..6dabd3409 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1849,7 +1849,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch unsigned char *p = ssl->handshake->premaster; unsigned char *end = p + sizeof( ssl->handshake->premaster ); const unsigned char *psk; - size_t psk_len; + size_t psk_len = 0; if( mbedtls_ssl_get_psk( ssl, &psk, &psk_len ) == MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ) From 8a69ddd7ad7eb17b8daf9a7b0352314183c54c96 Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Fri, 27 Mar 2020 11:13:39 +0000 Subject: [PATCH 6/9] Fix Doxygen comments Signed-off-by: Guilhem Bryant --- include/mbedtls/ssl_internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index ed852e8dd..fb8c23fee 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -922,7 +922,7 @@ void mbedtls_ssl_optimize_checksum( 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 ); -/* +/** * Get the first defined PSK by order of precedence: * 1. handshake PSK set by \c mbedtls_ssl_set_hs_psk() in the PSK callback * 2. static PSK configured by \c mbedtls_ssl_conf_psk() @@ -952,7 +952,7 @@ static inline int mbedtls_ssl_get_psk( const mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_USE_PSA_CRYPTO) -/* +/** * Get the first defined opaque PSK by order of precedence: * 1. handshake PSK set by \c mbedtls_ssl_set_hs_psk_opaque() in the PSK * callback From b5f04e4d84966610a291483765cbd6871b7a6f43 Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Wed, 1 Apr 2020 11:23:58 +0100 Subject: [PATCH 7/9] Properly initialise psk and psk_len Signed-off-by: Guilhem Bryant --- include/mbedtls/ssl_internal.h | 2 ++ library/ssl_tls.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index fb8c23fee..1e58ca3eb 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -945,6 +945,8 @@ static inline int mbedtls_ssl_get_psk( const mbedtls_ssl_context *ssl, else { + *psk = NULL; + *psk_len = 0; return( MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ); } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6dabd3409..4a8686cc4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1848,7 +1848,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch { unsigned char *p = ssl->handshake->premaster; unsigned char *end = p + sizeof( ssl->handshake->premaster ); - const unsigned char *psk; + const unsigned char *psk = NULL; size_t psk_len = 0; if( mbedtls_ssl_get_psk( ssl, &psk, &psk_len ) From 03d3711bb109011b62456eafe30cc6586bd879dc Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Fri, 3 Apr 2020 18:21:19 +0100 Subject: [PATCH 8/9] Fix bracket style Signed-off-by: Guilhem Bryant --- library/ssl_tls.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4a8686cc4..c0ebfcb67 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1853,11 +1853,13 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch if( mbedtls_ssl_get_psk( ssl, &psk, &psk_len ) == MBEDTLS_ERR_SSL_PRIVATE_KEY_REQUIRED ) + { /* * This should never happen because the existence of a PSK is always * checked before calling this function */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + } /* * PMS = struct { From 0c9b1955cacc59556370911b3713a6f316766623 Mon Sep 17 00:00:00 2001 From: Guilhem Bryant Date: Wed, 8 Apr 2020 11:02:38 +0100 Subject: [PATCH 9/9] Return internal error if no PSK is found when deriving the premaster secret Signed-off-by: Guilhem Bryant --- library/ssl_tls.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c0ebfcb67..9a523f5b0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1859,6 +1859,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch * checked before calling this function */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } /*