From 1aaf66940ef7dbc68e39be7f8938fa8aaad7701c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 10 Jul 2019 14:14:05 +0200 Subject: [PATCH] Implement usage checks in context_save() Enforce restrictions indicated in the documentation. This allows to make some simplifying assumptions (no need to worry about saving IVs for CBC in TLS < 1.1, nor about saving handshake data) and guarantees that all values marked as "forced" in the design document have the intended values and can be skipped when serialising. Some of the "forced" values are not checked because their value is a consequence of other checks (for example, session_negotiated == NULL outside handshakes). We do however check that session and transform are not NULL (even if that's also a consequence of the initial handshake being over) as we're going to dereference them and static analyzers may appreciate the info. --- include/mbedtls/ssl.h | 3 ++- include/mbedtls/ssl_internal.h | 15 +++++++++++++++ library/ssl_tls.c | 29 +++++++++++++++++++++++++++-- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 3767804748..13320145f1 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3918,13 +3918,14 @@ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ); * \return #MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL if \p buf is too small. * \return #MBEDTLS_ERR_SSL_BAD_INPUT_DATA if a handshake is in * progress, or there is pending data for reading or sending, - * or the connection does not use DTLS 1.2 with and AEAD + * or the connection does not use DTLS 1.2 with an AEAD * ciphersuite, or renegotiation is enabled. */ int mbedtls_ssl_context_save( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buf_len, size_t *olen ); + /** * \brief Load serialized connection data to an SSL context. * diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 11d66eec41..f703da99b4 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -650,6 +650,21 @@ struct mbedtls_ssl_transform #endif /* MBEDTLS_SSL_CONTEXT_SERIALIZATION */ }; +/* + * Return 1 if the transform uses an AEAD cipher, 0 otherwise. + * Equivalently, return 0 if a separate MAC is used, 1 otherwise. + */ +static inline int mbedtls_ssl_transform_uses_aead( + const mbedtls_ssl_transform *transform ) +{ +#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) + return( transform->maclen == 0 && transform->taglen != 0 ); +#else + (void) transform; + return( 1 ); +#endif +} + /* * Internal representation of record frames * diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 07201478b7..247459f120 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -11292,9 +11292,34 @@ int mbedtls_ssl_context_save( mbedtls_ssl_context *ssl, size_t buf_len, size_t *olen ) { - /* Unimplemented */ - (void) ssl; + /* + * Enforce current usage restrictions + */ + if( /* The initial handshake is over ... */ + ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER || + ssl->handshake != NULL || + /* ... and the various sub-structures are indeed ready. */ + ssl->transform == NULL || + ssl->session == NULL || + /* There is no pending incoming or outgoing data ... */ + mbedtls_ssl_check_pending( ssl ) != 0 || + ssl->out_left != 0 || + /* We're using DTLS 1.2 ... */ + ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM || + ssl->major_ver != MBEDTLS_SSL_MAJOR_VERSION_3 || + ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 || + /* ... with an AEAD ciphersuite. */ + mbedtls_ssl_transform_uses_aead( ssl->transform ) != 1 || + /* Renegotation is disabled. */ +#if defined(MBEDTLS_SSL_RENEGOTIATION) + ssl->conf->disable_renegotiation != MBEDTLS_SSL_RENEGOTIATION_DISABLED +#endif + ) + { + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + /* Unimplemented */ if( buf != NULL ) memset( buf, 0, buf_len );