From 69477b5706449dd025874852e864edd05c4c418c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Jan 2022 01:01:45 +0100 Subject: [PATCH 01/15] Add a field for application data to TLS structures In structure types that are passed to user callbacks, add a field that the library won't ever care about. The application can use this field to either identify an instance of the structure with a handle, or store a pointer to extra data. Signed-off-by: Gilles Peskine --- ChangeLog.d/ssl_context-user_data.txt | 3 +++ include/mbedtls/ssl.h | 14 ++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 ChangeLog.d/ssl_context-user_data.txt diff --git a/ChangeLog.d/ssl_context-user_data.txt b/ChangeLog.d/ssl_context-user_data.txt new file mode 100644 index 000000000000..81df94aa631f --- /dev/null +++ b/ChangeLog.d/ssl_context-user_data.txt @@ -0,0 +1,3 @@ +Features + * The structures mbedtls_ssl_config and mbedtls_ssl_context have an + extra field user_data which is reserved for the application. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 7e5fb199c854..afbebfea123f 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1448,6 +1448,13 @@ struct mbedtls_ssl_config #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_SSL_CLI_C) unsigned int MBEDTLS_PRIVATE(dhm_min_bitlen); /*!< min. bit length of the DHM prime */ #endif + + /** User data pointer or handle. + * + * The library sets this to \p 0 when creating a context and does not + * access it afterwards. + */ + uintptr_t user_data; }; struct mbedtls_ssl_context @@ -1669,6 +1676,13 @@ struct mbedtls_ssl_context /** Callback to export key block and master secret */ mbedtls_ssl_export_keys_t *MBEDTLS_PRIVATE(f_export_keys); void *MBEDTLS_PRIVATE(p_export_keys); /*!< context for key export callback */ + + /** User data pointer or handle. + * + * The library sets this to \p 0 when creating a context and does not + * access it afterwards. + */ + uintptr_t user_data; }; /** From 915896f03c6fd7584a83347bf0d602f72a3ece0a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Jan 2022 01:05:10 +0100 Subject: [PATCH 02/15] Add accessor function from mbedtls_ssl_context to the configuration Signed-off-by: Gilles Peskine --- ChangeLog.d/ssl_context-user_data.txt | 2 ++ include/mbedtls/ssl.h | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/ChangeLog.d/ssl_context-user_data.txt b/ChangeLog.d/ssl_context-user_data.txt index 81df94aa631f..1198847963d8 100644 --- a/ChangeLog.d/ssl_context-user_data.txt +++ b/ChangeLog.d/ssl_context-user_data.txt @@ -1,3 +1,5 @@ Features * The structures mbedtls_ssl_config and mbedtls_ssl_context have an extra field user_data which is reserved for the application. + * Add an accessor function to get the configuration associated with + an SSL context. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index afbebfea123f..ecfcfc63dccc 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1850,6 +1850,22 @@ void mbedtls_ssl_conf_dbg( mbedtls_ssl_config *conf, void (*f_dbg)(void *, int, const char *, int, const char *), void *p_dbg ); +/** + * \brief Return the SSL configuration structure associated + * with the given SSL context. + * + * \note The pointer returned by this function is guaranteed to + * remain valid until the context is freed. + * + * \param ssl The SSL context to query. + * \return Pointer to the SSL configuration associated with \p ssl. + */ +static inline const mbedtls_ssl_config *mbedtls_ssl_context_get_config( + const mbedtls_ssl_context *ssl ) +{ + return( ssl->MBEDTLS_PRIVATE( conf ) ); +} + /** * \brief Set the underlying BIO callbacks for write, read and * read-with-timeout. From e1a0c25f71c7ee592492f12aa3abf61052dad1da Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Jan 2022 01:08:03 +0100 Subject: [PATCH 03/15] New function to access the TLS version from a context as an enum Signed-off-by: Gilles Peskine --- ChangeLog.d/ssl_context-version_number.txt | 3 +++ include/mbedtls/ssl.h | 19 +++++++++++++++++++ library/ssl_tls.c | 15 +++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 ChangeLog.d/ssl_context-version_number.txt diff --git a/ChangeLog.d/ssl_context-version_number.txt b/ChangeLog.d/ssl_context-version_number.txt new file mode 100644 index 000000000000..97395f435248 --- /dev/null +++ b/ChangeLog.d/ssl_context-version_number.txt @@ -0,0 +1,3 @@ +Features + * Add a function to access the TLS version from a context in a form that's + easy to compare. Fixes #5407. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index ecfcfc63dccc..d911b8f052b9 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1161,6 +1161,14 @@ struct mbedtls_ssl_session #endif }; +/** Human-friendly representation of the (D)TLS protocol version. */ +typedef enum +{ + MBEDTLS_SSL_VERSION_UNKNOWN, /*!< Context not in use or version not yet negotiated. */ + MBEDTLS_SSL_VERSION_1_2, /*!< (D)TLS 1.2 */ + MBEDTLS_SSL_VERSION_1_3, /*!< (D)TLS 1.3 */ +} mbedtls_ssl_protocol_version; + /* * Identifiers for PRFs used in various versions of TLS. */ @@ -3933,6 +3941,17 @@ int mbedtls_ssl_get_ciphersuite_id_from_ssl( const mbedtls_ssl_context *ssl ); */ const char *mbedtls_ssl_get_ciphersuite( const mbedtls_ssl_context *ssl ); + +/** + * \brief Return the (D)TLS protocol version negotiated in the + * given connection. + * + * \param ssl The SSL context to query. + * \return The negotiated protocol version. + */ +mbedtls_ssl_protocol_version mbedtls_ssl_get_version_number( + const mbedtls_ssl_context *ssl ); + /** * \brief Return the current TLS version * diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e80adb1551e3..436e15c14d2b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2206,6 +2206,21 @@ const char *mbedtls_ssl_get_ciphersuite( const mbedtls_ssl_context *ssl ) return mbedtls_ssl_get_ciphersuite_name( ssl->session->ciphersuite ); } +mbedtls_ssl_protocol_version mbedtls_ssl_get_version_number( + const mbedtls_ssl_context *ssl ) +{ + /* For major_ver, only 3 is supported, so skip checking it. */ + switch( ssl->minor_ver ) + { + case MBEDTLS_SSL_MINOR_VERSION_3: + return( MBEDTLS_SSL_VERSION_1_2 ); + case MBEDTLS_SSL_MINOR_VERSION_4: + return( MBEDTLS_SSL_VERSION_1_3 ); + default: + return( MBEDTLS_SSL_VERSION_UNKNOWN ); + } +} + const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) { #if defined(MBEDTLS_SSL_PROTO_DTLS) From 1255b0de98dcdf28d1365f2fda1a2a0f25d0c821 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Jan 2022 01:08:48 +0100 Subject: [PATCH 04/15] Positive unit testing for SSL context version functions Signed-off-by: Gilles Peskine --- tests/suites/test_suite_ssl.function | 52 +++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index cb66f3afc840..dd8c26209895 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1793,6 +1793,45 @@ int exchange_data( mbedtls_ssl_context *ssl_1, ssl_2, 256, 1 ); } +int check_ssl_version( int expected_negotiated_version, + const mbedtls_ssl_context *ssl ) +{ + const char *version_string = mbedtls_ssl_get_version( ssl ); + mbedtls_ssl_protocol_version version_number = + mbedtls_ssl_get_version_number( ssl ); + + TEST_EQUAL( ssl->major_ver, MBEDTLS_SSL_MAJOR_VERSION_3 ); + TEST_EQUAL( ssl->minor_ver, expected_negotiated_version ); + + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + TEST_EQUAL( version_string[0], 'D' ); + ++version_string; + } + + switch( expected_negotiated_version ) + { + case MBEDTLS_SSL_MINOR_VERSION_3: + TEST_EQUAL( version_number, MBEDTLS_SSL_VERSION_1_2 ); + TEST_ASSERT( strcmp( version_string, "TLSv1.2" ) == 0 ); + break; + + case MBEDTLS_SSL_MINOR_VERSION_4: + TEST_EQUAL( version_number, MBEDTLS_SSL_VERSION_1_3 ); + TEST_ASSERT( strcmp( version_string, "TLSv1.3" ) == 0 ); + break; + + default: + TEST_ASSERT( ! "Version check not implemented for this protocol version" ); + } + + return( 1 ); + +exit: + return( 0 ); +} + + #if defined(MBEDTLS_X509_CRT_PARSE_C) && \ defined(MBEDTLS_ENTROPY_C) && \ defined(MBEDTLS_CTR_DRBG_C) @@ -1984,11 +2023,16 @@ void perform_handshake( handshake_test_options* options ) TEST_ASSERT( client.ssl.state == MBEDTLS_SSL_HANDSHAKE_OVER ); TEST_ASSERT( server.ssl.state == MBEDTLS_SSL_HANDSHAKE_OVER ); - /* Check that we agree on the version... */ - TEST_ASSERT( client.ssl.minor_ver == server.ssl.minor_ver ); + /* Check that both sides have negotiated the expected version. */ + mbedtls_test_set_step( 0 ); + if( ! check_ssl_version( options->expected_negotiated_version, + &client.ssl ) ) + goto exit; - /* And check that the version negotiated is the expected one. */ - TEST_EQUAL( client.ssl.minor_ver, options->expected_negotiated_version ); + mbedtls_test_set_step( 1 ); + if( ! check_ssl_version( options->expected_negotiated_version, + &server.ssl ) ) + goto exit; #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) if( options->resize_buffers != 0 ) From c63a1e0e153989c2dddf83324c476026a204903e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Jan 2022 01:10:24 +0100 Subject: [PATCH 05/15] Fix mbedtls_ssl_get_version() for TLSv1.3 Test it in ssl-opt.sh. Signed-off-by: Gilles Peskine --- ChangeLog.d/ssl_get_version_1_3.txt | 2 ++ library/ssl_tls.c | 3 ++- tests/ssl-opt.sh | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 ChangeLog.d/ssl_get_version_1_3.txt diff --git a/ChangeLog.d/ssl_get_version_1_3.txt b/ChangeLog.d/ssl_get_version_1_3.txt new file mode 100644 index 000000000000..4436522b6e14 --- /dev/null +++ b/ChangeLog.d/ssl_get_version_1_3.txt @@ -0,0 +1,2 @@ +Bugfix + * Fix mbedtls_ssl_get_version() not reporting TLSv1.3. Fixes #5406. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 436e15c14d2b..adb18ab6c2f4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2241,7 +2241,8 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) { case MBEDTLS_SSL_MINOR_VERSION_3: return( "TLSv1.2" ); - + case MBEDTLS_SSL_MINOR_VERSION_4: + return( "TLSv1.3" ); default: return( "unknown" ); } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index dd05716eddfa..0548c14ba34f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -9668,6 +9668,7 @@ run_test "TLS 1.3: minimal feature sets - openssl" \ -c "<= parse certificate verify" \ -c "mbedtls_ssl_tls13_process_certificate_verify() returned 0" \ -c "<= parse finished message" \ + -c "Protocol is TLSv1.3" \ -c "HTTP/1.0 200 ok" requires_gnutls_tls1_3 From 66971f8ab1ba9cd06d20c8705a7aba1d3cfe7f67 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Jan 2022 13:46:05 +0100 Subject: [PATCH 06/15] Add prototype for automatically generated debug helper Signed-off-by: Gilles Peskine --- library/ssl_debug_helpers.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_debug_helpers.h b/library/ssl_debug_helpers.h index 2fc416351630..2ffc5f41fc26 100644 --- a/library/ssl_debug_helpers.h +++ b/library/ssl_debug_helpers.h @@ -33,6 +33,8 @@ const char *mbedtls_ssl_states_str( mbedtls_ssl_states in ); +const char *mbedtls_ssl_protocol_version_str( mbedtls_ssl_protocol_version in ); + const char *mbedtls_tls_prf_types_str( mbedtls_tls_prf_types in ); const char *mbedtls_ssl_key_export_type_str( mbedtls_ssl_key_export_type in ); From 80dae04f248bdbfb925128d333008ced3b7538cf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 21 Jan 2022 23:50:39 +0100 Subject: [PATCH 07/15] Make user_data fields private Add accessor functions. Add unit tests for the accessor functions. Signed-off-by: Gilles Peskine --- ChangeLog.d/ssl_context-user_data.txt | 5 +- include/mbedtls/ssl.h | 130 +++++++++++++++++++++++++- tests/suites/test_suite_ssl.function | 23 +++++ 3 files changed, 154 insertions(+), 4 deletions(-) diff --git a/ChangeLog.d/ssl_context-user_data.txt b/ChangeLog.d/ssl_context-user_data.txt index 1198847963d8..630d8f02cd35 100644 --- a/ChangeLog.d/ssl_context-user_data.txt +++ b/ChangeLog.d/ssl_context-user_data.txt @@ -1,5 +1,6 @@ Features - * The structures mbedtls_ssl_config and mbedtls_ssl_context have an - extra field user_data which is reserved for the application. + * The structures mbedtls_ssl_config and mbedtls_ssl_context now store + a piece of user data which is reserved for the application. The user + data can be either a pointer or an integer. * Add an accessor function to get the configuration associated with an SSL context. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d911b8f052b9..a96b5c87ea9e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1462,7 +1462,7 @@ struct mbedtls_ssl_config * The library sets this to \p 0 when creating a context and does not * access it afterwards. */ - uintptr_t user_data; + uintptr_t MBEDTLS_PRIVATE(user_data); }; struct mbedtls_ssl_context @@ -1690,7 +1690,7 @@ struct mbedtls_ssl_context * The library sets this to \p 0 when creating a context and does not * access it afterwards. */ - uintptr_t user_data; + uintptr_t MBEDTLS_PRIVATE(user_data); }; /** @@ -2301,6 +2301,132 @@ void mbedtls_ssl_set_export_keys_cb( mbedtls_ssl_context *ssl, mbedtls_ssl_export_keys_t *f_export_keys, void *p_export_keys ); +/** \brief Set the user data in an SSL configuration to a pointer. + * + * You can retrieve this value later with mbedtls_ssl_conf_get_user_data_p(). + * + * \note The library stores \c p without accessing it. It is the responsibility + * of the caller to ensure that the pointer remains valid. + * + * \param conf The SSL configuration context to modify. + * \param p The new value of the user data. + */ +static inline void mbedtls_ssl_conf_set_user_data_p( + mbedtls_ssl_config *conf, + void *p ) +{ + conf->MBEDTLS_PRIVATE(user_data) = (uintptr_t) p; +} + +/** \brief Set the user data in an SSL configuration to an integer. + * + * You can retrieve this value later with mbedtls_ssl_conf_get_user_data_n(). + * + * \param conf The SSL configuration context to modify. + * \param n The new value of the user data. + */ +static inline void mbedtls_ssl_conf_set_user_data_n( + mbedtls_ssl_config *conf, + uintptr_t n ) +{ + conf->MBEDTLS_PRIVATE(user_data) = n; +} + +/** \brief Retrieve the user data in an SSL configuration as a pointer. + * + * This is the value last set with mbedtls_ssl_conf_set_user_data_n(), or + * \c 0 if mbedtls_ssl_conf_set_user_data_n() has not previously been + * called. The value is undefined if mbedtls_ssl_conf_set_user_data_p() has + * been called without a subsequent call to mbedtls_ssl_conf_set_user_data_n(). + * + * \param conf The SSL configuration context to modify. + * \return The current value of the user data. + */ +static inline void *mbedtls_ssl_conf_get_user_data_p( + mbedtls_ssl_config *conf ) +{ + return( (void*) conf->MBEDTLS_PRIVATE(user_data) ); +} + +/** \brief Retrieve the user data in an SSL configuration as an integer. + * + * This is the value last set with mbedtls_ssl_conf_set_user_data_p(), or + * \c NULL if mbedtls_ssl_conf_set_user_data_p() has not previously been + * called. The value is undefined if mbedtls_ssl_conf_set_user_data_n() has + * been called without a subsequent call to mbedtls_ssl_conf_set_user_data_p(). + * + * \param conf The SSL configuration context to modify. + * \return The current value of the user data. + */ +static inline uintptr_t mbedtls_ssl_conf_get_user_data_n( + mbedtls_ssl_config *conf ) +{ + return( conf->MBEDTLS_PRIVATE(user_data) ); +} + +/** \brief Set the user data in an SSL context to a pointer. + * + * You can retrieve this value later with mbedtls_ssl_get_user_data_p(). + * + * \note The library stores \c p without accessing it. It is the responsibility + * of the caller to ensure that the pointer remains valid. + * + * \param ssl The SSL context context to modify. + * \param p The new value of the user data. + */ +static inline void mbedtls_ssl_set_user_data_p( + mbedtls_ssl_context *ssl, + void *p ) +{ + ssl->MBEDTLS_PRIVATE(user_data) = (uintptr_t) p; +} + +/** \brief Set the user data in an SSL context to an integer. + * + * You can retrieve this value later with mbedtls_ssl_get_user_data_n(). + * + * \param ssl The SSL context context to modify. + * \param n The new value of the user data. + */ +static inline void mbedtls_ssl_set_user_data_n( + mbedtls_ssl_context *ssl, + uintptr_t n ) +{ + ssl->MBEDTLS_PRIVATE(user_data) = n; +} + +/** \brief Retrieve the user data in an SSL context as a pointer. + * + * This is the value last set with mbedtls_ssl_set_user_data_n(), or + * \c 0 if mbedtls_ssl_set_user_data_n() has not previously been + * called. The value is undefined if mbedtls_ssl_set_user_data_p() has + * been called without a subsequent call to mbedtls_ssl_set_user_data_n(). + * + * \param ssl The SSL context context to modify. + * \return The current value of the user data. + */ +static inline void *mbedtls_ssl_get_user_data_p( + mbedtls_ssl_context *ssl ) +{ + return( (void*) ssl->MBEDTLS_PRIVATE(user_data) ); +} + +/** \brief Retrieve the user data in an SSL context as an integer. + * + * This is the value last set with mbedtls_ssl_set_user_data_p(), or + * \c NULL if mbedtls_ssl_set_user_data_p() has not previously been + * called. The value is undefined if mbedtls_ssl_set_user_data_n() has + * been called without a subsequent call to mbedtls_ssl_set_user_data_p(). + * + * \param ssl The SSL context context to modify. + * \return The current value of the user data. + */ +static inline uintptr_t mbedtls_ssl_get_user_data_n( + mbedtls_ssl_context *ssl ) +{ + return( ssl->MBEDTLS_PRIVATE(user_data) ); +} + #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) /** * \brief Configure asynchronous private key operation callbacks. diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index dd8c26209895..38a2602d905a 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -886,6 +886,7 @@ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, mbedtls_test_message_queue *output_queue ) { int ret = -1; + uintptr_t user_data_n; if( dtls_context != NULL && ( input_queue == NULL || output_queue == NULL ) ) return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; @@ -904,6 +905,18 @@ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, mbedtls_ctr_drbg_random, &( ep->ctr_drbg ) ); mbedtls_entropy_init( &( ep->entropy ) ); + + TEST_ASSERT( mbedtls_ssl_conf_get_user_data_p( &ep->conf ) == NULL ); + TEST_EQUAL( mbedtls_ssl_conf_get_user_data_n( &ep->conf ), 0 ); + TEST_ASSERT( mbedtls_ssl_get_user_data_p( &ep->ssl ) == NULL ); + TEST_EQUAL( mbedtls_ssl_get_user_data_n( &ep->ssl ), 0 ); + + (void) mbedtls_test_rnd_std_rand( NULL, + (void*) &user_data_n, + sizeof( user_data_n ) ); + mbedtls_ssl_conf_set_user_data_n( &ep->conf, user_data_n ); + mbedtls_ssl_set_user_data_n( &ep->ssl, user_data_n ); + if( dtls_context != NULL ) { TEST_ASSERT( mbedtls_message_socket_setup( input_queue, output_queue, @@ -954,6 +967,11 @@ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, ret = mbedtls_endpoint_certificate_init( ep, pk_alg ); TEST_ASSERT( ret == 0 ); + TEST_EQUAL( mbedtls_ssl_conf_get_user_data_n( &ep->conf ), user_data_n ); + mbedtls_ssl_conf_set_user_data_p( &ep->conf, ep ); + TEST_EQUAL( mbedtls_ssl_get_user_data_n( &ep->ssl ), user_data_n ); + mbedtls_ssl_set_user_data_p( &ep->ssl, ep ); + exit: return ret; } @@ -2194,6 +2212,11 @@ void perform_handshake( handshake_test_options* options ) } #endif /* MBEDTLS_SSL_RENEGOTIATION */ + TEST_ASSERT( mbedtls_ssl_conf_get_user_data_p( &client.conf ) == &client ); + TEST_ASSERT( mbedtls_ssl_get_user_data_p( &client.ssl ) == &client ); + TEST_ASSERT( mbedtls_ssl_conf_get_user_data_p( &server.conf ) == &server ); + TEST_ASSERT( mbedtls_ssl_get_user_data_p( &server.ssl ) == &server ); + exit: mbedtls_endpoint_free( &client, options->dtls != 0 ? &client_context : NULL ); mbedtls_endpoint_free( &server, options->dtls != 0 ? &server_context : NULL ); From 49d7ddf7f3a79b95d028eba357a9db8875ea512d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jan 2022 23:25:51 +0100 Subject: [PATCH 08/15] Serializing a context does not save the user data The user data is typically a pointer to a data structure or a handle which may no longer be valid after the session is restored. If the user data needs to be preserved, let the application do it. This way, it is a conscious decision for the application to save/restore either the pointer/handle itself or the object it refers to. Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 19 +++++++++++++++++-- tests/suites/test_suite_ssl.function | 2 ++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index a96b5c87ea9e..41ccd8700049 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1689,6 +1689,10 @@ struct mbedtls_ssl_context * * The library sets this to \p 0 when creating a context and does not * access it afterwards. + * + * \warning Serializing and restoring an SSL context with + * mbedtls_ssl_context_save() and mbedtls_ssl_context_load() + * does not currently restore the user data. */ uintptr_t MBEDTLS_PRIVATE(user_data); }; @@ -4509,6 +4513,14 @@ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ); * * \see mbedtls_ssl_context_load() * + * \note The serialized data only contains the data that is + * necessary to resume the connection: negotiated protocol + * options, session identifier, keys, etc. + * Loading a saved SSL context does not restore settings and + * state related to how the application accesses the context, + * such as configured callback functions, user data, pending + * incoming or outgoing data, etc. + * * \note This feature is currently only available under certain * conditions, see the documentation of the return value * #MBEDTLS_ERR_SSL_BAD_INPUT_DATA for details. @@ -4587,8 +4599,11 @@ int mbedtls_ssl_context_save( mbedtls_ssl_context *ssl, * (unless they were already set before calling * mbedtls_ssl_session_reset() and the values are suitable for * the present connection). Specifically, you want to call - * at least mbedtls_ssl_set_bio() and - * mbedtls_ssl_set_timer_cb(). All other SSL setter functions + * at least mbedtls_ssl_set_bio(), + * mbedtls_ssl_set_timer_cb(), and + * mbedtls_ssl_set_user_data_n() or + * mbedtls_ssl_set_user_data_p() if they were set originally. + * All other SSL setter functions * are not necessary to call, either because they're only used * in handshakes, or because the setting is already saved. You * might choose to call them anyway, for example in order to diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 38a2602d905a..4f5ee9762d2d 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2105,6 +2105,8 @@ void perform_handshake( handshake_test_options* options ) mbedtls_mock_tcp_recv_msg, NULL ); + mbedtls_ssl_set_user_data_p( &server.ssl, &server ); + #if defined(MBEDTLS_TIMING_C) mbedtls_ssl_set_timer_cb( &server.ssl, &timer_server, mbedtls_timing_set_delay, From 1e265d2e68fd0a9ff3cb10ece1353eef975cd31e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jan 2022 23:30:52 +0100 Subject: [PATCH 09/15] Fix swapped documentation of set_user_data_{n,p} Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 41ccd8700049..8ed3af443814 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2338,10 +2338,10 @@ static inline void mbedtls_ssl_conf_set_user_data_n( /** \brief Retrieve the user data in an SSL configuration as a pointer. * - * This is the value last set with mbedtls_ssl_conf_set_user_data_n(), or - * \c 0 if mbedtls_ssl_conf_set_user_data_n() has not previously been - * called. The value is undefined if mbedtls_ssl_conf_set_user_data_p() has - * been called without a subsequent call to mbedtls_ssl_conf_set_user_data_n(). + * This is the value last set with mbedtls_ssl_conf_set_user_data_p(), or + * \c NULL if mbedtls_ssl_conf_set_user_data_p() has not previously been + * called. The value is undefined if mbedtls_ssl_conf_set_user_data_n() has + * been called without a subsequent call to mbedtls_ssl_conf_set_user_data_p(). * * \param conf The SSL configuration context to modify. * \return The current value of the user data. @@ -2354,10 +2354,10 @@ static inline void *mbedtls_ssl_conf_get_user_data_p( /** \brief Retrieve the user data in an SSL configuration as an integer. * - * This is the value last set with mbedtls_ssl_conf_set_user_data_p(), or - * \c NULL if mbedtls_ssl_conf_set_user_data_p() has not previously been - * called. The value is undefined if mbedtls_ssl_conf_set_user_data_n() has - * been called without a subsequent call to mbedtls_ssl_conf_set_user_data_p(). + * This is the value last set with mbedtls_ssl_conf_set_user_data_n(), or + * \c 0 if mbedtls_ssl_conf_set_user_data_n() has not previously been + * called. The value is undefined if mbedtls_ssl_conf_set_user_data_p() has + * been called without a subsequent call to mbedtls_ssl_conf_set_user_data_n(). * * \param conf The SSL configuration context to modify. * \return The current value of the user data. @@ -2401,10 +2401,10 @@ static inline void mbedtls_ssl_set_user_data_n( /** \brief Retrieve the user data in an SSL context as a pointer. * - * This is the value last set with mbedtls_ssl_set_user_data_n(), or - * \c 0 if mbedtls_ssl_set_user_data_n() has not previously been - * called. The value is undefined if mbedtls_ssl_set_user_data_p() has - * been called without a subsequent call to mbedtls_ssl_set_user_data_n(). + * This is the value last set with mbedtls_ssl_set_user_data_p(), or + * \c NULL if mbedtls_ssl_set_user_data_p() has not previously been + * called. The value is undefined if mbedtls_ssl_set_user_data_n() has + * been called without a subsequent call to mbedtls_ssl_set_user_data_p(). * * \param ssl The SSL context context to modify. * \return The current value of the user data. @@ -2417,10 +2417,10 @@ static inline void *mbedtls_ssl_get_user_data_p( /** \brief Retrieve the user data in an SSL context as an integer. * - * This is the value last set with mbedtls_ssl_set_user_data_p(), or - * \c NULL if mbedtls_ssl_set_user_data_p() has not previously been - * called. The value is undefined if mbedtls_ssl_set_user_data_n() has - * been called without a subsequent call to mbedtls_ssl_set_user_data_p(). + * This is the value last set with mbedtls_ssl_set_user_data_n(), or + * \c 0 if mbedtls_ssl_set_user_data_n() has not previously been + * called. The value is undefined if mbedtls_ssl_set_user_data_p() has + * been called without a subsequent call to mbedtls_ssl_set_user_data_n(). * * \param ssl The SSL context context to modify. * \return The current value of the user data. From ded2a42ac1f473554c4d9516d2bd129f7146635f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 12 Feb 2022 00:20:08 +0100 Subject: [PATCH 10/15] Use a union instead of casts Same intended semantics, no casts. Limitation: this doesn't work on architectures where sizeof(uintptr_t) < sizeof(void*), which is somewhat weird but possible if pointers contain redundant information. Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8ed3af443814..a4d0dde0c7e7 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1218,6 +1218,18 @@ typedef void mbedtls_ssl_export_keys_t( void *p_expkey, const unsigned char server_random[32], mbedtls_tls_prf_types tls_prf_type ); +/* A type for storing user data in a library structure. + * + * The representation of type may change in future versions of the library. + * Only the behaviors guaranteed by documented accessor functions are + * guaranteed to remain stable. + */ +typedef union +{ + uintptr_t n; /* typically a handle to an associated object */ + void *p; /* typically a pointer to extra data */ +} mbedtls_ssl_user_data_t; + /** * SSL/TLS configuration to be shared between mbedtls_ssl_context structures. */ @@ -1462,7 +1474,7 @@ struct mbedtls_ssl_config * The library sets this to \p 0 when creating a context and does not * access it afterwards. */ - uintptr_t MBEDTLS_PRIVATE(user_data); + mbedtls_ssl_user_data_t MBEDTLS_PRIVATE(user_data); }; struct mbedtls_ssl_context @@ -1694,7 +1706,7 @@ struct mbedtls_ssl_context * mbedtls_ssl_context_save() and mbedtls_ssl_context_load() * does not currently restore the user data. */ - uintptr_t MBEDTLS_PRIVATE(user_data); + mbedtls_ssl_user_data_t MBEDTLS_PRIVATE(user_data); }; /** @@ -2319,7 +2331,7 @@ static inline void mbedtls_ssl_conf_set_user_data_p( mbedtls_ssl_config *conf, void *p ) { - conf->MBEDTLS_PRIVATE(user_data) = (uintptr_t) p; + conf->MBEDTLS_PRIVATE(user_data).p = p; } /** \brief Set the user data in an SSL configuration to an integer. @@ -2333,7 +2345,7 @@ static inline void mbedtls_ssl_conf_set_user_data_n( mbedtls_ssl_config *conf, uintptr_t n ) { - conf->MBEDTLS_PRIVATE(user_data) = n; + conf->MBEDTLS_PRIVATE(user_data).n = n; } /** \brief Retrieve the user data in an SSL configuration as a pointer. @@ -2349,7 +2361,7 @@ static inline void mbedtls_ssl_conf_set_user_data_n( static inline void *mbedtls_ssl_conf_get_user_data_p( mbedtls_ssl_config *conf ) { - return( (void*) conf->MBEDTLS_PRIVATE(user_data) ); + return( conf->MBEDTLS_PRIVATE(user_data).p ); } /** \brief Retrieve the user data in an SSL configuration as an integer. @@ -2365,7 +2377,7 @@ static inline void *mbedtls_ssl_conf_get_user_data_p( static inline uintptr_t mbedtls_ssl_conf_get_user_data_n( mbedtls_ssl_config *conf ) { - return( conf->MBEDTLS_PRIVATE(user_data) ); + return( conf->MBEDTLS_PRIVATE(user_data).n ); } /** \brief Set the user data in an SSL context to a pointer. @@ -2382,7 +2394,7 @@ static inline void mbedtls_ssl_set_user_data_p( mbedtls_ssl_context *ssl, void *p ) { - ssl->MBEDTLS_PRIVATE(user_data) = (uintptr_t) p; + ssl->MBEDTLS_PRIVATE(user_data).p = p; } /** \brief Set the user data in an SSL context to an integer. @@ -2396,7 +2408,7 @@ static inline void mbedtls_ssl_set_user_data_n( mbedtls_ssl_context *ssl, uintptr_t n ) { - ssl->MBEDTLS_PRIVATE(user_data) = n; + ssl->MBEDTLS_PRIVATE(user_data).n = n; } /** \brief Retrieve the user data in an SSL context as a pointer. @@ -2412,7 +2424,7 @@ static inline void mbedtls_ssl_set_user_data_n( static inline void *mbedtls_ssl_get_user_data_p( mbedtls_ssl_context *ssl ) { - return( (void*) ssl->MBEDTLS_PRIVATE(user_data) ); + return( ssl->MBEDTLS_PRIVATE(user_data).p ); } /** \brief Retrieve the user data in an SSL context as an integer. @@ -2428,7 +2440,7 @@ static inline void *mbedtls_ssl_get_user_data_p( static inline uintptr_t mbedtls_ssl_get_user_data_n( mbedtls_ssl_context *ssl ) { - return( ssl->MBEDTLS_PRIVATE(user_data) ); + return( ssl->MBEDTLS_PRIVATE(user_data).n ); } #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) From 9cb08822a16d19034d2a12ccb2bfe04ef0347317 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 12 Feb 2022 00:44:24 +0100 Subject: [PATCH 11/15] Minor clarification Signed-off-by: Gilles Peskine --- ChangeLog.d/ssl_context-version_number.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/ssl_context-version_number.txt b/ChangeLog.d/ssl_context-version_number.txt index 97395f435248..b5951d0b9076 100644 --- a/ChangeLog.d/ssl_context-version_number.txt +++ b/ChangeLog.d/ssl_context-version_number.txt @@ -1,3 +1,3 @@ Features - * Add a function to access the TLS version from a context in a form that's - easy to compare. Fixes #5407. + * Add a function to access the protocol version from an SSL context in a + form that's easy to compare. Fixes #5407. From 860429f8af5d4abf106c2c7d54619621de1a5698 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 12 Feb 2022 00:44:48 +0100 Subject: [PATCH 12/15] Add version number debug check to the GnuTLS interop test as well Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0548c14ba34f..2fe7a4016f8c 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -9702,6 +9702,7 @@ run_test "TLS 1.3: minimal feature sets - gnutls" \ -c "<= parse certificate verify" \ -c "mbedtls_ssl_tls13_process_certificate_verify() returned 0" \ -c "<= parse finished message" \ + -c "Protocol is TLSv1.3" \ -c "HTTP/1.0 200 OK" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 From d44e050339d46b849a7fb7d92c185a12df7ad7e8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 12 Feb 2022 00:45:21 +0100 Subject: [PATCH 13/15] get_version_number documentation: explicitly mention VERSION_UNKNOWN Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index a4d0dde0c7e7..a2b4cdd24251 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -4088,6 +4088,10 @@ const char *mbedtls_ssl_get_ciphersuite( const mbedtls_ssl_context *ssl ); * \brief Return the (D)TLS protocol version negotiated in the * given connection. * + * \note If you call this function too early during the initial + * handshake, before the two sides have agreed on a version, + * this function returns #MBEDTLS_SSL_VERSION_UNKNOWN. + * * \param ssl The SSL context to query. * \return The negotiated protocol version. */ From ce4f00de692c22506c64fc62659f748446c65b75 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 12 Feb 2022 00:47:23 +0100 Subject: [PATCH 14/15] Reference get_version_number from the conf_xxx_version documentation Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index a2b4cdd24251..b964c4e460ca 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3745,6 +3745,10 @@ void mbedtls_ssl_get_dtls_srtp_negotiation_result( const mbedtls_ssl_context *ss * * \note With DTLS, use MBEDTLS_SSL_MINOR_VERSION_3 for DTLS 1.2 * + * \note After the handhsake, you can call + * mbedtls_ssl_get_version_number() to see what version was + * negotiated. + * * \param conf SSL configuration * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) * \param minor Minor version number (only MBEDTLS_SSL_MINOR_VERSION_3 supported) @@ -3760,6 +3764,10 @@ void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int mino * * \note With DTLS, use MBEDTLS_SSL_MINOR_VERSION_3 for DTLS 1.2 * + * \note After the handhsake, you can call + * mbedtls_ssl_get_version_number() to see what version was + * negotiated. + * * \param conf SSL configuration * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) * \param minor Minor version number (only MBEDTLS_SSL_MINOR_VERSION_3 supported) From 57bf02bd587780b5401f873d71fa467705adde9e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 16 Feb 2022 12:06:22 +0100 Subject: [PATCH 15/15] ssl_conf_{min,max}_version documentation: update for 1.3 and improve Mention that TLS 1.3 is supported, in addition to (D)TLS 1.2. Improve and clarify the documentation. In particular, emphasise that the minor version numbers are the internal numbers which are off by one from the human numbers. Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b964c4e460ca..7544f42637f0 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3738,39 +3738,50 @@ void mbedtls_ssl_get_dtls_srtp_negotiation_result( const mbedtls_ssl_context *ss /** * \brief Set the maximum supported version sent from the client side - * and/or accepted at the server side - * (Default: MBEDTLS_SSL_MAX_MAJOR_VERSION, MBEDTLS_SSL_MAX_MINOR_VERSION) + * and/or accepted at the server side. * - * \note This ignores ciphersuites from higher versions. - * - * \note With DTLS, use MBEDTLS_SSL_MINOR_VERSION_3 for DTLS 1.2 + * See also the documentation of mbedtls_ssl_conf_min_version(). * - * \note After the handhsake, you can call - * mbedtls_ssl_get_version_number() to see what version was - * negotiated. + * \note This ignores ciphersuites from higher versions. * * \param conf SSL configuration - * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) - * \param minor Minor version number (only MBEDTLS_SSL_MINOR_VERSION_3 supported) + * \param major Major version number (#MBEDTLS_SSL_MAJOR_VERSION_3) + * \param minor Minor version number + * (#MBEDTLS_SSL_MINOR_VERSION_3 for (D)TLS 1.2, + * #MBEDTLS_SSL_MINOR_VERSION_4 for TLS 1.3) */ void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int minor ); /** * \brief Set the minimum accepted SSL/TLS protocol version - * (Default: TLS 1.2) + * + * \note By default, all supported versions are accepted. + * Future versions of the library may disable older + * protocol versions by default if they become deprecated. + * + * \note The following versions are supported (if enabled at + * compile time): + * - (D)TLS 1.2: \p major = #MBEDTLS_SSL_MAJOR_VERSION_3, + * \p minor = #MBEDTLS_SSL_MINOR_VERSION_3 + * - TLS 1.3: \p major = #MBEDTLS_SSL_MAJOR_VERSION_3, + * \p minor = #MBEDTLS_SSL_MINOR_VERSION_4 + * + * Note that the numbers in the constant names are the + * TLS internal protocol numbers, and the minor versions + * differ by one from the human-readable versions! * * \note Input outside of the SSL_MAX_XXXXX_VERSION and * SSL_MIN_XXXXX_VERSION range is ignored. * - * \note With DTLS, use MBEDTLS_SSL_MINOR_VERSION_3 for DTLS 1.2 - * - * \note After the handhsake, you can call + * \note After the handshake, you can call * mbedtls_ssl_get_version_number() to see what version was * negotiated. * * \param conf SSL configuration - * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) - * \param minor Minor version number (only MBEDTLS_SSL_MINOR_VERSION_3 supported) + * \param major Major version number (#MBEDTLS_SSL_MAJOR_VERSION_3) + * \param minor Minor version number + * (#MBEDTLS_SSL_MINOR_VERSION_3 for (D)TLS 1.2, + * #MBEDTLS_SSL_MINOR_VERSION_4 for TLS 1.3) */ void mbedtls_ssl_conf_min_version( mbedtls_ssl_config *conf, int major, int minor );