From 494dc71de8aa461fb44ba7af7f44eecc3a6dd02a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 14 May 2021 15:08:51 +0100 Subject: [PATCH 1/7] Remove mbedtls_ssl_get_session_pointer() Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 18 ------------------ library/ssl_tls.c | 8 -------- programs/ssl/ssl_client2.c | 21 ++++++++++++++++++--- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b9b2e045f3..98b873beae 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2433,7 +2433,6 @@ int mbedtls_ssl_session_load( mbedtls_ssl_session *session, * of session cache or session tickets. * * \see mbedtls_ssl_session_load() - * \see mbedtls_ssl_get_session_pointer() * * \param session The session structure to be saved. * \param buf The buffer to write the serialized data to. It must be a @@ -2456,23 +2455,6 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, size_t buf_len, size_t *olen ); -/** - * \brief Get a pointer to the current session structure, for example - * to serialize it. - * - * \warning Ownership of the session remains with the SSL context, and - * the returned pointer is only guaranteed to be valid until - * the next API call operating on the same \p ssl context. - * - * \see mbedtls_ssl_session_save() - * - * \param ssl The SSL context. - * - * \return A pointer to the current session if successful. - * \return \c NULL if no session is active. - */ -const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer( const mbedtls_ssl_context *ssl ); - /** * \brief Set the list of allowed ciphersuites and the preference * order. First in the list has the highest preference. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bc2f269a9c..1decc9fce5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4801,14 +4801,6 @@ int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_CLI_C */ -const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer( const mbedtls_ssl_context *ssl ) -{ - if( ssl == NULL ) - return( NULL ); - - return( ssl->session ); -} - /* * Define ticket header determining Mbed TLS version * and structure of the ticket. diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 24c859e388..eac6356255 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -2224,6 +2224,8 @@ int main( int argc, char *argv[] ) if( opt.reco_mode == 1 ) { + mbedtls_ssl_session exported_session; + /* free any previously saved data */ if( session_data != NULL ) { @@ -2232,27 +2234,40 @@ int main( int argc, char *argv[] ) session_data = NULL; } + mbedtls_ssl_session_init( &exported_session ); + ret = mbedtls_ssl_get_session( &ssl, &exported_session ); + if( ret != 0 ) + { + mbedtls_printf( + "failed\n ! mbedtls_ssl_get_session() returned -%#02x\n", + (unsigned) -ret ); + goto exit; + } + /* get size of the buffer needed */ - mbedtls_ssl_session_save( mbedtls_ssl_get_session_pointer( &ssl ), - NULL, 0, &session_data_len ); + mbedtls_ssl_session_save( &exported_session, NULL, 0, &session_data_len ); session_data = mbedtls_calloc( 1, session_data_len ); if( session_data == NULL ) { mbedtls_printf( " failed\n ! alloc %u bytes for session data\n", (unsigned) session_data_len ); + mbedtls_ssl_session_free( &exported_session ); ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; goto exit; } /* actually save session data */ - if( ( ret = mbedtls_ssl_session_save( mbedtls_ssl_get_session_pointer( &ssl ), + if( ( ret = mbedtls_ssl_session_save( &exported_session, session_data, session_data_len, &session_data_len ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_ssl_session_saved returned -0x%04x\n\n", (unsigned int) -ret ); + mbedtls_ssl_session_free( &exported_session ); goto exit; } + + mbedtls_ssl_session_free( &exported_session ); } else { From e810bbc1aca89b9775f7a78535f21eb154026fa6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 14 May 2021 16:01:05 +0100 Subject: [PATCH 2/7] Implement 3.0-semantics for mbedtls_ssl_{get,set}_session() mbedtls_ssl_{get,set}_session() exhibited idempotent behaviour in Mbed TLS 2.x. Multiple calls to those functions are not useful in TLS 1.2, and the idempotent nature is unsuitable for support of TLS 1.3 which introduces the availabilty to offer multiple tickets for resumption, as well as receive multiple tickets. In preparation for TLS 1.3 support, this commit relaxes the semantics of `mbedtls_ssl_{get,set}_session()` by allowing implementations to fail gracefully, and leveraging this freedom by modifying the existing TLS 1.2 implementation to only accept one call to `mbedtls_ssl_{get,set}_session()` per context, and non-fatally failing all subsequent invocations. For TLS 1.3, it will be leveraged by making multiple calls to `mbedtls_ssl_get_session()` issue one ticket a time until no more tickets are available, and by using multiple calls to `mbedtls_ssl_set_session()` to allow the client to offer multiple tickets to the server. Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 96 +++++++++++++++++++++++++++++++------------ library/ssl_tls.c | 27 +++++++++++- 2 files changed, 95 insertions(+), 28 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 98b873beae..6fa380d499 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -897,6 +897,8 @@ struct mbedtls_ssl_session unsigned char id[32]; /*!< session identifier */ unsigned char master[48]; /*!< the master secret */ + unsigned char exported; + #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) mbedtls_x509_crt *peer_cert; /*!< peer X.509 cert chain */ @@ -2373,18 +2375,49 @@ void mbedtls_ssl_conf_session_cache( mbedtls_ssl_config *conf, #if defined(MBEDTLS_SSL_CLI_C) /** - * \brief Request resumption of session (client-side only) - * Session data is copied from presented session structure. + * \brief Load a session for session resumption. * - * \param ssl SSL context - * \param session session context + * Sessions loaded through this call will be considered + * for session resumption in the next handshake. * - * \return 0 if successful, - * MBEDTLS_ERR_SSL_ALLOC_FAILED if memory allocation failed, - * MBEDTLS_ERR_SSL_BAD_INPUT_DATA if used server-side or - * arguments are otherwise invalid + * \note Even if this call succeeds, it is not guaranteed that + * the next handshake will indeed be shortened through the + * use of session resumption: The server is always free + * to reject any attempt for resumption and fall back to + * a full handshake. + * + * \note The mechanism of session resumption is opaque to this + * call: For TLS 1.2, both session ID-based resumption and + * ticket-based resumption will be considered. For TLS 1.3, + * once implemented, sessions equate to tickets, and loading + * one or more sessions via this call will lead to their + * corresponding tickets being advertised as resumption PSKs + * by the client. + * + * \note Calling this function multiple times will only be useful + * once TLS 1.3 is supported. For TLS 1.2 connections, this + * function should be called at most once. + * + * \param ssl The SSL context representing the connection which should + * be attempted to be setup using session resumption. This + * must be initialized via mbedtls_ssl_init() and bound to + * an SSL configuration via mbedtls_ssl_setup(), but + * the handshake must not yet have been started. + * \param session The session to be considered for session resumption. + * This must be a session previously exported via + * mbedtls_ssl_get_session(), and potentially serialized and + * deserialized through mbedtls_ssl_session_save() and + * mbedtls_ssl_session_load() in the meantime. + * + * \return \c 0 if successful. + * \return \c MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE if the session + * could not be loaded because of an implementation limitation. + * This error is non-fatal, and has no observable effect on + * the SSL context or the session that was attempted to be loaded. + * \return Another negative error code on other kinds of failure. * * \sa mbedtls_ssl_get_session() + * \sa mbedtls_ssl_session_load() */ int mbedtls_ssl_set_session( mbedtls_ssl_context *ssl, const mbedtls_ssl_session *session ); #endif /* MBEDTLS_SSL_CLI_C */ @@ -3677,32 +3710,41 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss #if defined(MBEDTLS_SSL_CLI_C) /** - * \brief Save session in order to resume it later (client-side only) - * Session data is copied to presented session structure. + * \brief Export a session in order to resume it later. * + * \param ssl The SSL context representing the connection for which to + * to export a session structure for later resumption. + * \param session The target structure in which to store the exported session. + * This must have been initialized with mbedtls_ssl_init_session() + * but otherwise be unused. * - * \param ssl SSL context - * \param session session context + * \note The mechanism of session resumption is opaque to this + * call: For TLS 1.2, both session ID-based resumption and + * ticket-based resumption will be considered. For TLS 1.3, + * once implemented, sessions equate to tickets, and calling + * this function multiple times will export the available + * tickets one a time until no further tickets are available, + * in which case MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE will + * be returned. * - * \return 0 if successful, - * MBEDTLS_ERR_SSL_ALLOC_FAILED if memory allocation failed, - * MBEDTLS_ERR_SSL_BAD_INPUT_DATA if used server-side or - * arguments are otherwise invalid. + * \note Calling this function multiple times will only be useful + * once TLS 1.3 is supported. For TLS 1.2 connections, this + * function should be called at most once. * - * \note Only the server certificate is copied, and not the full chain, - * so you should not attempt to validate the certificate again - * by calling \c mbedtls_x509_crt_verify() on it. - * Instead, you should use the results from the verification - * in the original handshake by calling \c mbedtls_ssl_get_verify_result() - * after loading the session again into a new SSL context - * using \c mbedtls_ssl_set_session(). - * - * \note Once the session object is not needed anymore, you should - * free it by calling \c mbedtls_ssl_session_free(). + * \return \c 0 if successful. In this case, \p session can be used for + * session resumption by passing it to mbedtls_ssl_set_session(), + * and serialized for storage via mbedtls_ssl_session_save(). + * \return #MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE if no further session + * is available for export. + * This error is a non-fatal, and has no observable effect on + * the SSL context or the destination session. + * \return Another negative error code on other kinds of failure. * * \sa mbedtls_ssl_set_session() + * \sa mbedtls_ssl_session_save() */ -int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, mbedtls_ssl_session *session ); +int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, + mbedtls_ssl_session *session ); #endif /* MBEDTLS_SSL_CLI_C */ /** diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1decc9fce5..6e32b564ed 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3788,6 +3788,9 @@ int mbedtls_ssl_set_session( mbedtls_ssl_context *ssl, const mbedtls_ssl_session return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } + if( ssl->handshake->resume == 1 ) + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + if( ( ret = mbedtls_ssl_session_copy( ssl->session_negotiate, session ) ) != 0 ) return( ret ); @@ -4789,6 +4792,8 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, mbedtls_ssl_session *dst ) { + int ret; + if( ssl == NULL || dst == NULL || ssl->session == NULL || @@ -4797,7 +4802,27 @@ int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - return( mbedtls_ssl_session_copy( dst, ssl->session ) ); + /* Since Mbed TLS 3.0, mbedtls_ssl_get_session() is no longer + * idempotent: Each session can only be exported once. + * + * (This is in preparation for TLS 1.3 support where we will + * need the ability to export multiple sessions (aka tickets), + * which will be achieved by calling mbedtls_ssl_get_session() + * multiple times until it fails.) + * + * Check whether we have already exported the current session, + * and fail if so. + */ + if( ssl->session->exported == 1 ) + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + + ret = mbedtls_ssl_session_copy( dst, ssl->session ); + if( ret != 0 ) + return( ret ); + + /* Remember that we've exported the session. */ + ssl->session->exported = 1; + return( 0 ); } #endif /* MBEDTLS_SSL_CLI_C */ From 548b136e8f974f0fdee4a27f3c266beea0349535 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 14 May 2021 16:09:15 +0100 Subject: [PATCH 3/7] Add migration guide for removal of mbedtls_ssl_get_session_pointer() Signed-off-by: Hanno Becker --- .../remove-ssl-get-session_pointer.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 docs/3.0-migration-guide.d/remove-ssl-get-session_pointer.md diff --git a/docs/3.0-migration-guide.d/remove-ssl-get-session_pointer.md b/docs/3.0-migration-guide.d/remove-ssl-get-session_pointer.md new file mode 100644 index 0000000000..a4a4895a8f --- /dev/null +++ b/docs/3.0-migration-guide.d/remove-ssl-get-session_pointer.md @@ -0,0 +1,23 @@ +Remove the SSL API mbedtls_ssl_get_session_pointer() +----------------------------------------------------------------- + +This affects two classes of users: + +1. Users who manually inspect parts of the current session through + direct structure field access. + +2. Users of session resumption who query the current session + via `mbedtls_ssl_get_session_pointer()` prior to saving or exporting + it via `mbedtls_ssl_session_copy()` or `mbedtls_ssl_session_save()`, + respectively. + +Migration paths: + +1. Mbed TLS 3.0 does not offer a migration path for the usecase 1: Like many + other Mbed TLS structures, the structure of `mbedtls_ssl_session` is no + longer part of the public API in Mbed TLS 3.0, and direct structure field + access is no longer supported. Please see the corresponding migration guide. + +2. Users should replace calls to `mbedtls_ssl_get_session_pointer()` by + calls to `mbedtls_ssl_get_session()` as demonstrated in the example + program `programs/ssl/ssl_client2.c`. From 699d4d7df75161dc5b35d67db9ebbbaaee9cae6e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 14 May 2021 16:21:36 +0100 Subject: [PATCH 4/7] Add migration guide for new SSL ticket API Signed-off-by: Hanno Becker --- docs/3.0-migration-guide.d/ssl-ticket-api.md | 30 ++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 docs/3.0-migration-guide.d/ssl-ticket-api.md diff --git a/docs/3.0-migration-guide.d/ssl-ticket-api.md b/docs/3.0-migration-guide.d/ssl-ticket-api.md new file mode 100644 index 0000000000..ce95e2f24b --- /dev/null +++ b/docs/3.0-migration-guide.d/ssl-ticket-api.md @@ -0,0 +1,30 @@ +Modified semantics of mbedtls_ssl_{get,set}_session() +----------------------------------------------------------------- + +This affects users who call `mbedtls_ssl_get_session()` or +`mbedtls_ssl_session_set()` multiple times on the same SSL context +representing an established TLS 1.2 connection. +Those users will now observe the second call to fail with +`MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE`. + +Migration path: +- Exporting the same TLS 1.2 connection multiple times via + `mbedtls_ssl_get_session()` leads to multiple copies of + the same session. This use of `mbedtls_ssl_get_session()` + is discouraged, and the following should be considered: + * If the various session copies are later loaded into + fresh SSL contexts via `mbedtls_ssl_set_session()`, + export via `mbedtls_ssl_get_session()` only once and + load the same session into different contexts via + `mbedtls_ssl_set_session()`. Since `mbedtls_ssl_set_session()` + makes a copy of the session that's being loaded, this + is functionally equivalent. + * If the various session copies are later serialized + via `mbedtls_ssl_session_save()`, export and serialize + the session only once via `mbedtls_ssl_get_session()` and + `mbedtls_ssl_session_save()` and make copies of the raw + data instead. +- Calling `mbedtls_ssl_set_session()` multiple times in Mbed TLS 2.x + is not useful since subsequent calls overwrite the effect of previous + calls. Applications achieve equivalent functional behaviour by + issuing only the very last call to `mbedtls_ssl_set_session()`. From 3bbf4c058fed6b27a469ae50f779f8250b9b1ee6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 May 2021 06:20:23 +0100 Subject: [PATCH 5/7] Fix typo in migration guide for ticket API change Signed-off-by: Hanno Becker --- docs/3.0-migration-guide.d/ssl-ticket-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/3.0-migration-guide.d/ssl-ticket-api.md b/docs/3.0-migration-guide.d/ssl-ticket-api.md index ce95e2f24b..23c53d671d 100644 --- a/docs/3.0-migration-guide.d/ssl-ticket-api.md +++ b/docs/3.0-migration-guide.d/ssl-ticket-api.md @@ -2,7 +2,7 @@ Modified semantics of mbedtls_ssl_{get,set}_session() ----------------------------------------------------------------- This affects users who call `mbedtls_ssl_get_session()` or -`mbedtls_ssl_session_set()` multiple times on the same SSL context +`mbedtls_ssl_set_session()` multiple times on the same SSL context representing an established TLS 1.2 connection. Those users will now observe the second call to fail with `MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE`. From ab4ecfcc2c01cbf178dd0d4b3cf21e1b7573761f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 17 Jun 2021 05:12:16 +0100 Subject: [PATCH 6/7] Improve wording of documentation of new session resumption API Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 6fa380d499..1e8976ed14 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2386,8 +2386,8 @@ void mbedtls_ssl_conf_session_cache( mbedtls_ssl_config *conf, * to reject any attempt for resumption and fall back to * a full handshake. * - * \note The mechanism of session resumption is opaque to this - * call: For TLS 1.2, both session ID-based resumption and + * \note This function can handle a variety of mechanisms for session + * resumption: For TLS 1.2, both session ID-based resumption and * ticket-based resumption will be considered. For TLS 1.3, * once implemented, sessions equate to tickets, and loading * one or more sessions via this call will lead to their @@ -3718,8 +3718,8 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss * This must have been initialized with mbedtls_ssl_init_session() * but otherwise be unused. * - * \note The mechanism of session resumption is opaque to this - * call: For TLS 1.2, both session ID-based resumption and + * \note This function can handle a variety of mechanism for session + * resumption: For TLS 1.2, both session ID-based resumption and * ticket-based resumption will be considered. For TLS 1.3, * once implemented, sessions equate to tickets, and calling * this function multiple times will export the available From 5c5b820352295a2726a5562af3261eb493efdc52 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 18 Jun 2021 12:21:25 +0100 Subject: [PATCH 7/7] Fix typo in doc'n of session resumption API Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 1e8976ed14..68fee03a77 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3718,7 +3718,7 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss * This must have been initialized with mbedtls_ssl_init_session() * but otherwise be unused. * - * \note This function can handle a variety of mechanism for session + * \note This function can handle a variety of mechanisms for session * resumption: For TLS 1.2, both session ID-based resumption and * ticket-based resumption will be considered. For TLS 1.3, * once implemented, sessions equate to tickets, and calling