From 26f982f50e6a7f55e41d2e7e5dec17a8e7162839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 May 2019 11:01:32 +0200 Subject: [PATCH] Improve save API by always updating olen This allows callers to discover what an appropriate size is. Otherwise they'd have to either try repeatedly, or allocate an overly large buffer (or some combination of those). Adapt documentation an example usage in ssl_client2. --- include/mbedtls/ssl.h | 12 ++++-- library/ssl_tls.c | 81 +++++++++++++++++++------------------- programs/ssl/ssl_client2.c | 32 +++++++++++++-- 3 files changed, 79 insertions(+), 46 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 072204553..92ce32463 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2391,10 +2391,16 @@ int mbedtls_ssl_session_load( mbedtls_ssl_session *session, * * \param session The session structure to be saved. * \param buf The buffer to write the serialized data to. It must be a - * writeable buffer of at least \p len bytes. + * writeable buffer of at least \p len bytes, or may be \c + * NULL if \p len is \c 0. * \param buf_len The number of bytes available for writing in \p buf. - * \param olen The size of the written data in bytes. It must point to a - * valid \c size_t. + * \param olen The size in bytes of the data that has been or would have + * been written. It must point to a valid \c size_t. + * + * \note \p olen is updated to the correct value regardless of + * whether \p buf_len was large enough. This makes it possible + * to determine the necessary size by calling this function + * with \p buf set to \c NULL and \p buf_len to \c 0. * * \return \c 0 if successful. * \return #MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL if \p buf is too small. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index f1ba99a02..6f9203daa 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -9862,7 +9862,7 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, size_t *olen ) { unsigned char *p = buf; - size_t left = buf_len; + size_t used = 0; #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) size_t cert_len; @@ -9874,15 +9874,16 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, /* * Shallow copy of the session structure */ - if( left < sizeof( mbedtls_ssl_session ) ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); - left -= sizeof( mbedtls_ssl_session ); + used += sizeof( mbedtls_ssl_session ); /* This also copies the values of pointer fields in the * session to be serialized, but they'll be ignored when * loading the session through ssl_load_session(). */ - memcpy( p, session, sizeof( mbedtls_ssl_session ) ); - p += sizeof( mbedtls_ssl_session ); + if( used <= buf_len ) + { + memcpy( p, session, sizeof( mbedtls_ssl_session ) ); + p += sizeof( mbedtls_ssl_session ); + } /* * Copy of the peer's end-entity certificate @@ -9894,40 +9895,37 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, else cert_len = session->peer_cert->raw.len; - if( left < 3 + cert_len ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); - left -= 3 + cert_len; + used += 3 + cert_len; - *p++ = (unsigned char)( ( cert_len >> 16 ) & 0xFF ); - *p++ = (unsigned char)( ( cert_len >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( cert_len ) & 0xFF ); - left -= 3; - - if( session->peer_cert != NULL ) + if( used <= buf_len ) { - memcpy( p, session->peer_cert->raw.p, cert_len ); - p += cert_len; - } + *p++ = (unsigned char)( ( cert_len >> 16 ) & 0xFF ); + *p++ = (unsigned char)( ( cert_len >> 8 ) & 0xFF ); + *p++ = (unsigned char)( ( cert_len ) & 0xFF ); - p += cert_len; - left -= cert_len; + if( session->peer_cert != NULL ) + { + memcpy( p, session->peer_cert->raw.p, cert_len ); + p += cert_len; + } + } #else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ if( session->peer_cert_digest != NULL ) cert_digest_len = 0; else cert_digest_len = session->peer_cert_digest_len; - if( left < 1 + cert_digest_len ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + used += 1 + cert_digest_len; - *p++ = (unsigned char) cert_digest_len; - left--; + if( used <= buf_len ) + { + *p++ = (unsigned char) cert_digest_len; - if( session->peer_cert_digest != NULL ) - memcpy( p, session->peer_cert_digest, cert_digest_len ); + if( session->peer_cert_digest != NULL ) + memcpy( p, session->peer_cert_digest, cert_digest_len ); - p += cert_digest_len; - left -= cert_digest_len; + p += cert_digest_len; + } #endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ #endif /* MBEDTLS_X509_CRT_PARSE_C */ @@ -9935,24 +9933,27 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, * Copy of the session ticket if any */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) - if( left < 3 + session->ticket_len ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); - left -= 3 + session->ticket_len; + used += 3 + session->ticket_len; - *p++ = (unsigned char)( ( session->ticket_len >> 16 ) & 0xFF ); - *p++ = (unsigned char)( ( session->ticket_len >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( session->ticket_len ) & 0xFF ); - - if( session->ticket != NULL ) + if( used <= buf_len ) { - memcpy( p, session->ticket, session->ticket_len ); - p += session->ticket_len; + *p++ = (unsigned char)( ( session->ticket_len >> 16 ) & 0xFF ); + *p++ = (unsigned char)( ( session->ticket_len >> 8 ) & 0xFF ); + *p++ = (unsigned char)( ( session->ticket_len ) & 0xFF ); + + if( session->ticket != NULL ) + { + memcpy( p, session->ticket, session->ticket_len ); + p += session->ticket_len; + } } #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ /* Done */ - (void) left; - *olen = p - buf; + *olen = used; + + if( used > buf_len ) + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); return( 0 ); } diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 36b78e590..317fea373 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -38,6 +38,8 @@ #define mbedtls_calloc calloc #define mbedtls_free free #define mbedtls_exit exit +#define mbedtls_calloc calloc +#define mbedtls_free free #define MBEDTLS_EXIT_SUCCESS EXIT_SUCCESS #define MBEDTLS_EXIT_FAILURE EXIT_FAILURE #endif @@ -1052,8 +1054,8 @@ int main( int argc, char *argv[] ) mbedtls_ssl_context ssl; mbedtls_ssl_config conf; mbedtls_ssl_session saved_session; - unsigned char session_data[MBEDTLS_SSL_MAX_CONTENT_LEN]; - size_t session_data_len; + unsigned char *session_data = NULL; + size_t session_data_len = 0; #if defined(MBEDTLS_TIMING_C) mbedtls_timing_delay_context timer; #endif @@ -2456,8 +2458,25 @@ int main( int argc, char *argv[] ) if( opt.reco_mode == 1 ) { + /* free any previously saved data */ + mbedtls_free( session_data ); + session_data = NULL; + + /* get size of the buffer needed */ + mbedtls_ssl_session_save( mbedtls_ssl_get_session_pointer( &ssl ), + 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 ); + ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; + goto exit; + } + + /* actually save session data */ if( ( ret = mbedtls_ssl_session_save( mbedtls_ssl_get_session_pointer( &ssl ), - session_data, sizeof( session_data ), + session_data, session_data_len, &session_data_len ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_ssl_session_saved returned -0x%04x\n\n", @@ -2476,6 +2495,12 @@ int main( int argc, char *argv[] ) } mbedtls_printf( " ok\n" ); + + if( opt.reco_mode == 1 ) + { + mbedtls_printf( " [ Saved %u bytes of session data]\n", + (unsigned) session_data_len ); + } } #if defined(MBEDTLS_X509_CRT_PARSE_C) @@ -2999,6 +3024,7 @@ exit: mbedtls_ssl_config_free( &conf ); mbedtls_ctr_drbg_free( &ctr_drbg ); mbedtls_entropy_free( &entropy ); + mbedtls_free( session_data ); #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) && \ defined(MBEDTLS_USE_PSA_CRYPTO)