From 254530f2e01a025ea4fae74d9a3864d5a3309305 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 17 May 2017 18:59:53 +0300 Subject: [PATCH 1/4] Documentation error in `mbedtls_ssl_get_session` Fix Documentation error in `mbedtls_ssl_get_session`. This function supports deep copying of the session, and the peer certificate is not lost anymore, Resolves #926 --- ChangeLog | 7 +++++++ include/mbedtls/ssl.h | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 588b833e82..f5c1789192 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * Remove wrong documentation for `mbedtls_ssl_get_session`. + This API has deep copy of the session, and the peer + certificate is not lost. #926 + = mbed TLS 2.1.13 branch released 2018-06-18 Bugfix diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8573283647..8e234ef34e 100755 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2081,7 +2081,6 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss * \brief Save session in order to resume it later (client-side only) * Session data is copied to presented session structure. * - * \warning Currently, peer certificate is lost in the operation. * * \param ssl SSL context * \param session session context @@ -2091,6 +2090,11 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss * MBEDTLS_ERR_SSL_BAD_INPUT_DATA if used server-side or * arguments are otherwise invalid * + * \note Only the server certificate is copied, and not the chain + * but this is not a problem because the result of the chain + * verification is stored in `verify_result` and can be checked + * with \c mbedtls_ssl_get_verify_result() + * * \sa mbedtls_ssl_set_session() */ int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, mbedtls_ssl_session *session ); From 5c8e588444d042f7b996ff40cc40ef05ffcf3355 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 4 Jul 2018 17:42:47 +0300 Subject: [PATCH 2/4] Minor fixes 1. Rephrase ChangeLog entry. 2. Add a full stop at the end of the fuinction documentation. --- ChangeLog | 15 +++++++++++++-- include/mbedtls/ssl.h | 4 ++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index f5c1789192..74a9d1b7c3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,9 +3,20 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx Bugfix - * Remove wrong documentation for `mbedtls_ssl_get_session`. + + * Fix the key_app_writer example which was writing a leading zero byte which + was creating an invalid ASN.1 tag. Found by Aryeh R. Fixes #1257. + * Fix compilation error on C++, because of a variable named new. + Found and fixed by Hirotaka Niisato in #1783. + * Fix "no symbols" warning issued by ranlib when building on Mac OS X. Fix + contributed by tabascoeye in pull request #1600. + * Clarify documentation for mbedtls_ssl_write() to include 0 as a valid + return value. Found by @davidwu2000. #839 + * Fix a memory leak in mbedtls_x509_csr_parse(), found by catenacyber, + Philippe Antoine. Fixes #1623. + * Correct the documentation for `mbedtls_ssl_get_session()`. This API has deep copy of the session, and the peer - certificate is not lost. #926 + certificate is not lost. Fixes #926. = mbed TLS 2.1.13 branch released 2018-06-18 diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8e234ef34e..2259717528 100755 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2088,12 +2088,12 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss * \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 + * arguments are otherwise invalid. * * \note Only the server certificate is copied, and not the chain * but this is not a problem because the result of the chain * verification is stored in `verify_result` and can be checked - * with \c mbedtls_ssl_get_verify_result() + * with \c mbedtls_ssl_get_verify_result(). * * \sa mbedtls_ssl_set_session() */ From 99b9f12a91170fde18d90df00991a317a59d8d32 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 17 Jul 2018 11:21:50 +0300 Subject: [PATCH 3/4] Repharse comments Rephrase comments to clarify them. --- include/mbedtls/ssl.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 2259717528..1ef3059ae0 100755 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2090,10 +2090,16 @@ const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ss * MBEDTLS_ERR_SSL_BAD_INPUT_DATA if used server-side or * arguments are otherwise invalid. * - * \note Only the server certificate is copied, and not the chain - * but this is not a problem because the result of the chain - * verification is stored in `verify_result` and can be checked - * with \c mbedtls_ssl_get_verify_result(). + * \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(). * * \sa mbedtls_ssl_set_session() */ @@ -2329,6 +2335,9 @@ void mbedtls_ssl_session_init( mbedtls_ssl_session *session ); * \brief Free referenced items in an SSL session including the * peer certificate and clear memory * + * \note A session object can be freed even if the SSL context + * that was used to retrieve the session is still in use. + * * \param session SSL session */ void mbedtls_ssl_session_free( mbedtls_ssl_session *session ); From 41273200a297101244d0930e107539f43d8a7652 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 17 Jul 2018 14:13:53 +0300 Subject: [PATCH 4/4] Update ChangeLog Remove extra entries added by a bad cherry-pick. --- ChangeLog | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index 74a9d1b7c3..35cb749211 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,17 +3,6 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx Bugfix - - * Fix the key_app_writer example which was writing a leading zero byte which - was creating an invalid ASN.1 tag. Found by Aryeh R. Fixes #1257. - * Fix compilation error on C++, because of a variable named new. - Found and fixed by Hirotaka Niisato in #1783. - * Fix "no symbols" warning issued by ranlib when building on Mac OS X. Fix - contributed by tabascoeye in pull request #1600. - * Clarify documentation for mbedtls_ssl_write() to include 0 as a valid - return value. Found by @davidwu2000. #839 - * Fix a memory leak in mbedtls_x509_csr_parse(), found by catenacyber, - Philippe Antoine. Fixes #1623. * Correct the documentation for `mbedtls_ssl_get_session()`. This API has deep copy of the session, and the peer certificate is not lost. Fixes #926.