From 02f2609551c6cd42edc8d2f8ff9cb24361795df4 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 Jul 2019 16:13:00 +0100 Subject: [PATCH 01/10] Introduce configuration option and API for SSL record checking --- include/mbedtls/config.h | 13 ++++++++++ include/mbedtls/ssl.h | 48 +++++++++++++++++++++++++++++++++++++ library/ssl_tls.c | 12 ++++++++++ programs/ssl/query_config.c | 8 +++++++ 4 files changed, 81 insertions(+) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index f8a99f2e5..243678c62 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1293,6 +1293,19 @@ */ #define MBEDTLS_SSL_ALL_ALERT_MESSAGES +/** + * \def MBEDTLS_SSL_RECORD_CHECKING + * + * Enable the API mbedtls_ssl_check_record() which allows to check the + * validity, freshness and authenticity of an incoming record without + * modifying the externally visible state of the SSL context. + * + * See mbedtls_ssl_check_record() for more information. + * + * Uncomment to enable support for record checking. + */ +#define MBEDTLS_SSL_RECORD_CHECKING + /** * \def MBEDTLS_SSL_DTLS_CONNECTION_ID * diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 9e5fb2dbb..d5378df57 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1898,6 +1898,54 @@ void mbedtls_ssl_set_mtu( mbedtls_ssl_context *ssl, uint16_t mtu ); void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ); #endif /* !MBEDTLS_SSL_CONF_READ_TIMEOUT */ +#if defined(MBEDTLS_SSL_RECORD_CHECKING) +/** + * \brief Check whether a buffer contains a valid, fresh + * and authentic application data record (DTLS only). + * + * This function does not change the user-visible state + * of the SSL context. It's sole purpose is to provide + * an indication of the legitimacy of an incoming record. + * + * This can be useful e.g. in distributed server environments + * using the DTLS Connection ID feature, in which connections + * might need to be passed between service instances on a change + * of peer address, but where such disruptive operations should + * only happen after the validity of incoming records has been + * confirmed. + * + * \param ssl The SSL context to use. + * \param buf The address of the buffer holding the record to be checked. + * This must be an R/W buffer of length \p buflen Bytes. + * \param buflen The length of \p buf in Bytes. + * + * \note This routine only checks whether the provided buffer begins + * with a valid, fresh and authentic record, but does not check + * potential data following the initial record. In particular, + * it is possible to pass DTLS datagrams containing records, + * in which case only the first record is checked. + * + * \note This function modifies the input buffer \p buf. If you need + * to preserve the original record, you have to maintain a copy. + * + * \return \c 0 if the record is valid, fresh (DTLS only) and authentic. + * \return MBEDTLS_ERR_SSL_INVALID_MAC if the check completed + * successfully but the record was found to be not authentic. + * \return MBEDTLS_ERR_SSL_INVALID_RECORD if the check completed + * successfully but the record was found to be invalid for + * a reason different from authenticity checking. + * \return MBEDTLS_ERR_SSL_UNEXPECTED_RECORD if the check completed + * successfully but the record was found to be unexpected + * in the state of the SSL context, including replayed records. + * \return Another negative error code on different kinds of failure. + * In this case, the SSL context becomes unusable and needs + * to be freed or reset before reuse. + */ +int mbedtls_ssl_check_record( mbedtls_ssl_context const *ssl, + unsigned char *buf, + size_t buflen ); +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ + #if !defined(MBEDTLS_SSL_CONF_SET_TIMER) && \ !defined(MBEDTLS_SSL_CONF_GET_TIMER) /** diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3e78e5d95..d04602027 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -122,6 +122,18 @@ static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform ); static void ssl_update_in_pointers( mbedtls_ssl_context *ssl ); +#if defined(MBEDTLS_SSL_RECORD_CHECKING) +int mbedtls_ssl_check_record( mbedtls_ssl_context const *ssl, + unsigned char *buf, + size_t buflen ) +{ + ((void) ssl); + ((void) buf); + ((void) buflen); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +} +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ + #define SSL_DONT_FORCE_FLUSH 0 #define SSL_FORCE_FLUSH 1 diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c index 0f7251d45..cf10ba18d 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -1210,6 +1210,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_SSL_ALL_ALERT_MESSAGES */ +#if defined(MBEDTLS_SSL_RECORD_CHECKING) + if( strcmp( "MBEDTLS_SSL_RECORD_CHECKING", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_RECORD_CHECKING ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ + #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) if( strcmp( "MBEDTLS_SSL_DTLS_CONNECTION_ID", config ) == 0 ) { From 14219feb278508d71e592b133f3f8dd54864d9b7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 Jul 2019 17:02:43 +0100 Subject: [PATCH 02/10] Add IO wrappers to ssl_client2 as interm's between NET and SSL layer --- programs/ssl/ssl_client2.c | 89 +++++++++++++++++++++++++++++++------- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index f0de38153..ad1936f43 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -490,7 +490,8 @@ static void my_debug( void *ctx, int level, * Test recv/send functions that make sure each try returns * WANT_READ/WANT_WRITE at least once before sucesseding */ -static int my_recv( void *ctx, unsigned char *buf, size_t len ) + +static int delayed_recv( void *ctx, unsigned char *buf, size_t len ) { static int first_try = 1; int ret; @@ -507,7 +508,7 @@ static int my_recv( void *ctx, unsigned char *buf, size_t len ) return( ret ); } -static int my_send( void *ctx, const unsigned char *buf, size_t len ) +static int delayed_send( void *ctx, const unsigned char *buf, size_t len ) { static int first_try = 1; int ret; @@ -527,6 +528,70 @@ static int my_send( void *ctx, const unsigned char *buf, size_t len ) MBEDTLS_SSL_CONF_SEND && MBEDTLS_SSL_CONF_RECV_TIMEOUT */ +typedef struct +{ + mbedtls_ssl_context *ssl; + mbedtls_net_context *net; +} io_ctx_t; + +static int recv_cb( void *ctx, unsigned char *buf, size_t len ) +{ + io_ctx_t *io_ctx = (io_ctx_t*) ctx; + size_t recv_len; + int ret; + + if( opt.nbio == 2 ) + ret = delayed_recv( io_ctx->net, buf, len ); + else + ret = mbedtls_net_recv( io_ctx->net, buf, len ); + if( ret < 0 ) + return( ret ); + recv_len = (size_t) ret; + + if( opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + /* Here's the place to do any datagram/record checking + * in between receiving the packet from the underlying + * transport and passing it on to the TLS stack. */ + mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); + } + + return( (int) recv_len ); +} + +static int recv_timeout_cb( void *ctx, unsigned char *buf, size_t len, + uint32_t timeout ) +{ + io_ctx_t *io_ctx = (io_ctx_t*) ctx; + int ret; + size_t recv_len; + + ret = mbedtls_net_recv_timeout( io_ctx->net, buf, len, timeout ); + if( ret < 0 ) + return( ret ); + recv_len = (size_t) ret; + + if( opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + /* Here's the place to do any datagram/record checking + * in between receiving the packet from the underlying + * transport and passing it on to the TLS stack. */ + mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); + } + + return( (int) recv_len ); +} + +static int send_cb( void *ctx, unsigned char const *buf, size_t len ) +{ + io_ctx_t *io_ctx = (io_ctx_t*) ctx; + + if( opt.nbio == 2 ) + return( delayed_send( io_ctx->net, buf, len ) ); + + return( mbedtls_net_send( io_ctx->net, buf, len ) ); +} + #if defined(MBEDTLS_X509_CRT_PARSE_C) static unsigned char peer_crt_info[1024]; @@ -761,6 +826,7 @@ int main( int argc, char *argv[] ) { int ret = 0, len, tail_len, i, written, frags, retry_left; mbedtls_net_context server_fd; + io_ctx_t io_ctx; unsigned char buf[MAX_REQUEST_SIZE + 1]; @@ -1928,12 +1994,10 @@ int main( int argc, char *argv[] ) #if !defined(MBEDTLS_SSL_CONF_RECV) && \ !defined(MBEDTLS_SSL_CONF_SEND) && \ !defined(MBEDTLS_SSL_CONF_RECV_TIMEOUT) - if( opt.nbio == 2 ) - mbedtls_ssl_set_bio( &ssl, &server_fd, my_send, my_recv, NULL ); - else - mbedtls_ssl_set_bio( &ssl, &server_fd, - mbedtls_net_send, mbedtls_net_recv, - opt.nbio == 0 ? mbedtls_net_recv_timeout : NULL ); + io_ctx.ssl = &ssl; + io_ctx.net = &server_fd; + mbedtls_ssl_set_bio( &ssl, &io_ctx, send_cb, recv_cb, + opt.nbio == 0 ? recv_timeout_cb : NULL ); #else mbedtls_ssl_set_bio_ctx( &ssl, &server_fd ); #endif @@ -2550,13 +2614,8 @@ send_request: goto exit; } - if( opt.nbio == 2 ) - mbedtls_ssl_set_bio( &ssl, &server_fd, my_send, my_recv, - NULL ); - else - mbedtls_ssl_set_bio( &ssl, &server_fd, mbedtls_net_send, - mbedtls_net_recv, - opt.nbio == 0 ? mbedtls_net_recv_timeout : NULL ); + mbedtls_ssl_set_bio( &ssl, &io_ctx, send_cb, recv_cb, + opt.nbio == 0 ? recv_timeout_cb : NULL ); #if defined(MBEDTLS_TIMING_C) #if !defined(MBEDTLS_SSL_CONF_SET_TIMER) && \ From fe24b3b269e164f90ef54a62add2ecf8adb03f0d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 Jul 2019 17:05:43 +0100 Subject: [PATCH 03/10] Add IO wrappers to ssl_server2 as interm's between NET and SSL layer --- programs/ssl/ssl_server2.c | 88 +++++++++++++++++++++++++++++++------- 1 file changed, 73 insertions(+), 15 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 747523b5f..fe9ed6b2e 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -621,7 +621,7 @@ static void my_debug( void *ctx, int level, * Test recv/send functions that make sure each try returns * WANT_READ/WANT_WRITE at least once before sucesseding */ -static int my_recv( void *ctx, unsigned char *buf, size_t len ) +static int delayed_recv( void *ctx, unsigned char *buf, size_t len ) { static int first_try = 1; int ret; @@ -638,7 +638,7 @@ static int my_recv( void *ctx, unsigned char *buf, size_t len ) return( ret ); } -static int my_send( void *ctx, const unsigned char *buf, size_t len ) +static int delayed_send( void *ctx, const unsigned char *buf, size_t len ) { static int first_try = 1; int ret; @@ -658,6 +658,70 @@ static int my_send( void *ctx, const unsigned char *buf, size_t len ) MBEDTLS_SSL_CONF_SEND && MBEDTLS_SSL_CONF_RECV_TIMEOUT */ +typedef struct +{ + mbedtls_ssl_context *ssl; + mbedtls_net_context *net; +} io_ctx_t; + +static int recv_cb( void *ctx, unsigned char *buf, size_t len ) +{ + io_ctx_t *io_ctx = (io_ctx_t*) ctx; + size_t recv_len; + int ret; + + if( opt.nbio == 2 ) + ret = delayed_recv( io_ctx->net, buf, len ); + else + ret = mbedtls_net_recv( io_ctx->net, buf, len ); + if( ret < 0 ) + return( ret ); + recv_len = (size_t) ret; + + if( opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + /* Here's the place to do any datagram/record checking + * in between receiving the packet from the underlying + * transport and passing it on to the TLS stack. */ + mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); + } + + return( (int) recv_len ); +} + +static int recv_timeout_cb( void *ctx, unsigned char *buf, size_t len, + uint32_t timeout ) +{ + io_ctx_t *io_ctx = (io_ctx_t*) ctx; + int ret; + size_t recv_len; + + ret = mbedtls_net_recv_timeout( io_ctx->net, buf, len, timeout ); + if( ret < 0 ) + return( ret ); + recv_len = (size_t) ret; + + if( opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + /* Here's the place to do any datagram/record checking + * in between receiving the packet from the underlying + * transport and passing it on to the TLS stack. */ + mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); + } + + return( (int) recv_len ); +} + +static int send_cb( void *ctx, unsigned char const *buf, size_t len ) +{ + io_ctx_t *io_ctx = (io_ctx_t*) ctx; + + if( opt.nbio == 2 ) + return( delayed_send( io_ctx->net, buf, len ) ); + + return( mbedtls_net_send( io_ctx->net, buf, len ) ); +} + #if !defined(MBEDTLS_SSL_CONF_AUTHMODE) /* * Return authmode from string, or -1 on error @@ -1376,6 +1440,7 @@ int main( int argc, char *argv[] ) { int ret = 0, len, written, frags, exchanges_left; int version_suites[4][2]; + io_ctx_t io_ctx; unsigned char* buf = 0; #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) unsigned char psk[MBEDTLS_PSK_MAX_LEN]; @@ -2918,12 +2983,10 @@ int main( int argc, char *argv[] ) #if !defined(MBEDTLS_SSL_CONF_RECV) && \ !defined(MBEDTLS_SSL_CONF_SEND) && \ !defined(MBEDTLS_SSL_CONF_RECV_TIMEOUT) - if( opt.nbio == 2 ) - mbedtls_ssl_set_bio( &ssl, &client_fd, my_send, my_recv, NULL ); - else - mbedtls_ssl_set_bio( &ssl, &client_fd, - mbedtls_net_send, mbedtls_net_recv, - opt.nbio == 0 ? mbedtls_net_recv_timeout : NULL ); + io_ctx.ssl = &ssl; + io_ctx.net = &client_fd; + mbedtls_ssl_set_bio( &ssl, &io_ctx, send_cb, recv_cb, + opt.nbio == 0 ? recv_timeout_cb : NULL ); #else mbedtls_ssl_set_bio_ctx( &ssl, &client_fd ); #endif @@ -3586,13 +3649,8 @@ data_exchange: * if you want to share your set up code between the case of * establishing a new connection and this case. */ - if( opt.nbio == 2 ) - mbedtls_ssl_set_bio( &ssl, &client_fd, my_send, my_recv, - NULL ); - else - mbedtls_ssl_set_bio( &ssl, &client_fd, mbedtls_net_send, - mbedtls_net_recv, - opt.nbio == 0 ? mbedtls_net_recv_timeout : NULL ); + mbedtls_ssl_set_bio( &ssl, &io_ctx, send_cb, recv_cb, + opt.nbio == 0 ? recv_timeout_cb : NULL ); #if defined(MBEDTLS_TIMING_C) #if !defined(MBEDTLS_SSL_CONF_SET_TIMER) && \ From de9e36e6b3729d79e38b77aafd78bfcfafee4f76 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 Jul 2019 17:14:41 +0100 Subject: [PATCH 04/10] Pass dgrams to mbedtls_ssl_check_record in ssl_client2/server2 --- programs/ssl/ssl_client2.c | 72 +++++++++++++++++++++++++++++++++++-- programs/ssl/ssl_server2.c | 74 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 142 insertions(+), 4 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index ad1936f43..cc1816cef 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -534,6 +534,68 @@ typedef struct mbedtls_net_context *net; } io_ctx_t; +#if defined(MBEDTLS_SSL_RECORD_CHECKING) +static int ssl_check_record( mbedtls_ssl_context const *ssl, + unsigned char const *buf, size_t len ) +{ + int ret; + unsigned char *tmp_buf; + + tmp_buf = mbedtls_calloc( 1, len ); + if( tmp_buf == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + memcpy( tmp_buf, buf, len ); + + ret = mbedtls_ssl_check_record( ssl, tmp_buf, len ); + if( ret != MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ) + { + int ret_repeated; + + /* Test-only: Make sure that mbedtls_ssl_check_record() + * doesn't alter state. */ + memcpy( tmp_buf, buf, len ); /* Restore buffer */ + ret_repeated = mbedtls_ssl_check_record( ssl, tmp_buf, len ); + if( ret != ret_repeated ) + { + ret = -1; + goto exit; + } + + switch( ret ) + { + case 0: + break; + + case MBEDTLS_ERR_SSL_INVALID_RECORD: + if( opt.debug_level > 1 ) + mbedtls_printf( "mbedtls_ssl_check_record() detected invalid record.\n" ); + break; + + case MBEDTLS_ERR_SSL_INVALID_MAC: + if( opt.debug_level > 1 ) + mbedtls_printf( "mbedtls_ssl_check_record() detected unauthentic record.\n" ); + break; + + case MBEDTLS_ERR_SSL_UNEXPECTED_RECORD: + if( opt.debug_level > 1 ) + mbedtls_printf( "mbedtls_ssl_check_record() detected unexpected record.\n" ); + break; + + default: + mbedtls_printf( "mbedtls_ssl_check_record() failed fatally with -%#04x.\n", -ret ); + return( -1 ); + } + + /* Regardless of the outcome, forward the record to the stack. */ + } + +exit: + mbedtls_free( tmp_buf ); + + return( 0 ); +} +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ + static int recv_cb( void *ctx, unsigned char *buf, size_t len ) { io_ctx_t *io_ctx = (io_ctx_t*) ctx; @@ -553,7 +615,10 @@ static int recv_cb( void *ctx, unsigned char *buf, size_t len ) /* Here's the place to do any datagram/record checking * in between receiving the packet from the underlying * transport and passing it on to the TLS stack. */ - mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); +#if defined(MBEDTLS_SSL_RECORD_CHECKING) + if( ssl_check_record( io_ctx->ssl, buf, recv_len ) != 0 ) + return( -1 ); +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ } return( (int) recv_len ); @@ -576,7 +641,10 @@ static int recv_timeout_cb( void *ctx, unsigned char *buf, size_t len, /* Here's the place to do any datagram/record checking * in between receiving the packet from the underlying * transport and passing it on to the TLS stack. */ - mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); +#if defined(MBEDTLS_SSL_RECORD_CHECKING) + if( ssl_check_record( io_ctx->ssl, buf, recv_len ) != 0 ) + return( -1 ); +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ } return( (int) recv_len ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index fe9ed6b2e..ddad7102a 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -664,6 +664,70 @@ typedef struct mbedtls_net_context *net; } io_ctx_t; +#if defined(MBEDTLS_SSL_RECORD_CHECKING) +static int ssl_check_record( mbedtls_ssl_context const *ssl, + unsigned char const *buf, size_t len ) +{ + int ret; + unsigned char *tmp_buf; + + /* Record checking may modify the input buffer, + * so make a copy. */ + tmp_buf = mbedtls_calloc( 1, len ); + if( tmp_buf == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + memcpy( tmp_buf, buf, len ); + + ret = mbedtls_ssl_check_record( ssl, tmp_buf, len ); + if( ret != MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ) + { + int ret_repeated; + + /* Test-only: Make sure that mbedtls_ssl_check_record() + * doesn't alter state. */ + memcpy( tmp_buf, buf, len ); /* Restore buffer */ + ret_repeated = mbedtls_ssl_check_record( ssl, tmp_buf, len ); + if( ret != ret_repeated ) + { + ret = -1; + goto exit; + } + + switch( ret ) + { + case 0: + break; + + case MBEDTLS_ERR_SSL_INVALID_RECORD: + if( opt.debug_level > 1 ) + mbedtls_printf( "mbedtls_ssl_check_record() detected invalid record.\n" ); + break; + + case MBEDTLS_ERR_SSL_INVALID_MAC: + if( opt.debug_level > 1 ) + mbedtls_printf( "mbedtls_ssl_check_record() detected unauthentic record.\n" ); + break; + + case MBEDTLS_ERR_SSL_UNEXPECTED_RECORD: + if( opt.debug_level > 1 ) + mbedtls_printf( "mbedtls_ssl_check_record() detected unexpected record.\n" ); + break; + + default: + mbedtls_printf( "mbedtls_ssl_check_record() failed fatally with -%#04x.\n", -ret ); + return( -1 ); + } + + /* Regardless of the outcome, forward the record to the stack. */ + } + +exit: + mbedtls_free( tmp_buf ); + + return( 0 ); +} +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ + static int recv_cb( void *ctx, unsigned char *buf, size_t len ) { io_ctx_t *io_ctx = (io_ctx_t*) ctx; @@ -683,7 +747,10 @@ static int recv_cb( void *ctx, unsigned char *buf, size_t len ) /* Here's the place to do any datagram/record checking * in between receiving the packet from the underlying * transport and passing it on to the TLS stack. */ - mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); +#if defined(MBEDTLS_SSL_RECORD_CHECKING) + if( ssl_check_record( io_ctx->ssl, buf, recv_len ) != 0 ) + return( -1 ); +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ } return( (int) recv_len ); @@ -706,7 +773,10 @@ static int recv_timeout_cb( void *ctx, unsigned char *buf, size_t len, /* Here's the place to do any datagram/record checking * in between receiving the packet from the underlying * transport and passing it on to the TLS stack. */ - mbedtls_printf( "[RECV] Datagram of size %u\n", (unsigned) recv_len ); +#if defined(MBEDTLS_SSL_RECORD_CHECKING) + if( ssl_check_record( io_ctx->ssl, buf, recv_len ) != 0 ) + return( -1 ); +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ } return( (int) recv_len ); From 82ff6f1e17b636e80cc359d69c4125e3df5cbd0b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 4 Jul 2019 17:05:10 +0100 Subject: [PATCH 05/10] Update version_features.c --- library/version_features.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/version_features.c b/library/version_features.c index 51ce7801d..102b52109 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -438,6 +438,9 @@ static const char *features[] = { #if defined(MBEDTLS_SSL_ALL_ALERT_MESSAGES) "MBEDTLS_SSL_ALL_ALERT_MESSAGES", #endif /* MBEDTLS_SSL_ALL_ALERT_MESSAGES */ +#if defined(MBEDTLS_SSL_RECORD_CHECKING) + "MBEDTLS_SSL_RECORD_CHECKING", +#endif /* MBEDTLS_SSL_RECORD_CHECKING */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) "MBEDTLS_SSL_DTLS_CONNECTION_ID", #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ From bec8885b7deb5337529ac2e3c7fe1b1f4aba2c8e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 18 Jul 2019 08:20:53 +0100 Subject: [PATCH 06/10] State that record checking is DTLS only and doesn't check content type --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d5378df57..9277cd13e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1901,7 +1901,7 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) #if defined(MBEDTLS_SSL_RECORD_CHECKING) /** * \brief Check whether a buffer contains a valid, fresh - * and authentic application data record (DTLS only). + * and authentic record (DTLS only). * * This function does not change the user-visible state * of the SSL context. It's sole purpose is to provide @@ -1928,7 +1928,7 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) * \note This function modifies the input buffer \p buf. If you need * to preserve the original record, you have to maintain a copy. * - * \return \c 0 if the record is valid, fresh (DTLS only) and authentic. + * \return \c 0 if the record is valid, fresh and authentic. * \return MBEDTLS_ERR_SSL_INVALID_MAC if the check completed * successfully but the record was found to be not authentic. * \return MBEDTLS_ERR_SSL_INVALID_RECORD if the check completed From c2b08d12519bdd81ce0ce70d5995eb7f66f0f9fb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 18 Jul 2019 08:21:17 +0100 Subject: [PATCH 07/10] Fix minor issues in documentation of mbedtls_ssl_check_record() --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 9277cd13e..ce6c9d834 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1904,7 +1904,7 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) * and authentic record (DTLS only). * * This function does not change the user-visible state - * of the SSL context. It's sole purpose is to provide + * of the SSL context. Its sole purpose is to provide * an indication of the legitimacy of an incoming record. * * This can be useful e.g. in distributed server environments @@ -1922,7 +1922,7 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) * \note This routine only checks whether the provided buffer begins * with a valid, fresh and authentic record, but does not check * potential data following the initial record. In particular, - * it is possible to pass DTLS datagrams containing records, + * it is possible to pass DTLS datagrams containing records, * in which case only the first record is checked. * * \note This function modifies the input buffer \p buf. If you need From 83b8c3b8eb2c1a34dea5aa1d345faf1aa5f8e438 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 24 Jul 2019 13:59:07 +0100 Subject: [PATCH 08/10] cli/srv ex: Add dbg msg if record checking gives inconsistent result --- programs/ssl/ssl_client2.c | 4 ++-- programs/ssl/ssl_server2.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index cc1816cef..4269420e4 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -557,8 +557,8 @@ static int ssl_check_record( mbedtls_ssl_context const *ssl, ret_repeated = mbedtls_ssl_check_record( ssl, tmp_buf, len ); if( ret != ret_repeated ) { - ret = -1; - goto exit; + mbedtls_printf( "mbedtls_ssl_check_record() returned inconsistent results.\n" ); + return( -1 ); } switch( ret ) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index ddad7102a..c9e2fbb9e 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -689,8 +689,8 @@ static int ssl_check_record( mbedtls_ssl_context const *ssl, ret_repeated = mbedtls_ssl_check_record( ssl, tmp_buf, len ); if( ret != ret_repeated ) { - ret = -1; - goto exit; + mbedtls_printf( "mbedtls_ssl_check_record() returned inconsistent results.\n" ); + return( -1 ); } switch( ret ) From e29dfb2157cdfe3fa1736a98530b2bd601252710 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 24 Jul 2019 14:23:16 +0100 Subject: [PATCH 09/10] Add missing word in documentation of mbedtls_ssl_check_record() --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index ce6c9d834..edb0a4699 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1922,8 +1922,8 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) * \note This routine only checks whether the provided buffer begins * with a valid, fresh and authentic record, but does not check * potential data following the initial record. In particular, - * it is possible to pass DTLS datagrams containing records, - * in which case only the first record is checked. + * it is possible to pass DTLS datagrams containing multiple + * records, in which case only the first record is checked. * * \note This function modifies the input buffer \p buf. If you need * to preserve the original record, you have to maintain a copy. From 32bbe4a66b2631b2261b72d0fc2f6ed409352da7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 24 Jul 2019 16:06:23 +0100 Subject: [PATCH 10/10] Remove unused label in ssl_client2/ssl_server2 --- programs/ssl/ssl_client2.c | 1 - programs/ssl/ssl_server2.c | 1 - 2 files changed, 2 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 4269420e4..f6bdc567d 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -589,7 +589,6 @@ static int ssl_check_record( mbedtls_ssl_context const *ssl, /* Regardless of the outcome, forward the record to the stack. */ } -exit: mbedtls_free( tmp_buf ); return( 0 ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index c9e2fbb9e..2cd00fa39 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -721,7 +721,6 @@ static int ssl_check_record( mbedtls_ssl_context const *ssl, /* Regardless of the outcome, forward the record to the stack. */ } -exit: mbedtls_free( tmp_buf ); return( 0 );