From a9964dbcd518bcbf33a8fcac512e99d8e563ca8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 3 Jul 2014 19:29:16 +0200 Subject: [PATCH 1/3] Add ssl_set_renegotiation_enforced() --- include/polarssl/ssl.h | 32 ++++++++++++++++++++++++++++++++ library/ssl_tls.c | 24 +++++++++++++++++++++--- tests/ssl-opt.sh | 4 ++-- 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 91e398144..b90b232a5 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -221,6 +221,9 @@ #define SSL_RENEGOTIATION_DISABLED 0 #define SSL_RENEGOTIATION_ENABLED 1 +#define SSL_RENEGOTIATION_NOT_ENFORCED -1 +#define SSL_RENEGO_MAX_RECORDS_DEFAULT 16 + #define SSL_LEGACY_NO_RENEGOTIATION 0 #define SSL_LEGACY_ALLOW_RENEGOTIATION 1 #define SSL_LEGACY_BREAK_HANDSHAKE 2 @@ -620,6 +623,7 @@ struct _ssl_context */ int state; /*!< SSL handshake: current state */ int renegotiation; /*!< Initial or renegotiation */ + int renego_records_seen; /*!< Records since renego request */ int major_ver; /*!< equal to SSL_MAJOR_VERSION_3 */ int minor_ver; /*!< either 0 (SSL3) or 1 (TLS1.0) */ @@ -744,6 +748,7 @@ struct _ssl_context int verify_result; /*!< verification result */ int disable_renegotiation; /*!< enable/disable renegotiation */ int allow_legacy_renegotiation; /*!< allow legacy renegotiation */ + int renego_max_records; /*!< grace period for renegotiation */ const int *ciphersuite_list[4]; /*!< allowed ciphersuites / version */ #if defined(POLARSSL_SSL_SET_CURVES) const ecp_group_id *curve_list; /*!< allowed curves */ @@ -1421,6 +1426,33 @@ void ssl_set_renegotiation( ssl_context *ssl, int renegotiation ); */ void ssl_legacy_renegotiation( ssl_context *ssl, int allow_legacy ); +/** + * \brief Enforce server-requested renegotiation. + * (Default: enforced, max_records = 16) + * (No effect on client.) + * + * When a server requests a renegotiation, the client can + * comply or ignore the request. This function allows the + * server to decide if it should enforce its renegotiation + * requests by closing the connection if the client doesn't + * initiate a renegotiation. + * + * However, records could already be in transit from the + * client to the server when the request is emitted. In order + * to increase reliability, the server can accept a number of + * records containing application data before the ClientHello + * that was requested. + * + * The optimal value is highly dependent on the specific usage + * scenario. + * + * \param ssl SSL context + * \param max_records Use SSL_RENEGOTIATION_NOT_ENFORCED if you don't want to + * enforce renegotiation, or a non-negative value to enforce + * it but allow for a grace period of max_records records. + */ +void ssl_set_renegotiation_enforced( ssl_context *ssl, int max_records ); + /** * \brief Return the number of data bytes available to read * diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a1428dccf..7c8f3064b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3055,7 +3055,10 @@ void ssl_handshake_wrapup( ssl_context *ssl ) ssl->handshake = NULL; if( ssl->renegotiation == SSL_RENEGOTIATION ) + { ssl->renegotiation = SSL_RENEGOTIATION_DONE; + ssl->renego_records_seen = 0; + } /* * Switch in our now active transform context @@ -3345,6 +3348,8 @@ int ssl_init( ssl_context *ssl ) ssl_set_ciphersuites( ssl, ssl_list_ciphersuites() ); + ssl->renego_max_records = SSL_RENEGO_MAX_RECORDS_DEFAULT; + #if defined(POLARSSL_DHM_C) if( ( ret = mpi_read_string( &ssl->dhm_P, 16, POLARSSL_DHM_RFC5114_MODP_1024_P) ) != 0 || @@ -3435,6 +3440,8 @@ int ssl_session_reset( ssl_context *ssl ) ssl->transform_in = NULL; ssl->transform_out = NULL; + ssl->renego_records_seen = 0; + memset( ssl->out_ctr, 0, SSL_BUFFER_LEN ); memset( ssl->in_ctr, 0, SSL_BUFFER_LEN ); @@ -3952,6 +3959,11 @@ void ssl_legacy_renegotiation( ssl_context *ssl, int allow_legacy ) ssl->allow_legacy_renegotiation = allow_legacy; } +void ssl_set_renegotiation_enforced( ssl_context *ssl, int max_records ) +{ + ssl->renego_max_records = max_records; +} + #if defined(POLARSSL_SSL_SESSION_TICKETS) int ssl_set_session_tickets( ssl_context *ssl, int use_tickets ) { @@ -4296,9 +4308,15 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) } else if( ssl->renegotiation == SSL_RENEGOTIATION_PENDING ) { - SSL_DEBUG_MSG( 1, ( "renegotiation requested, " - "but not honored by client" ) ); - return( POLARSSL_ERR_SSL_UNEXPECTED_MESSAGE ); + ssl->renego_records_seen++; + + if( ssl->renego_max_records >= 0 && + ssl->renego_records_seen > ssl->renego_max_records ) + { + SSL_DEBUG_MSG( 1, ( "renegotiation requested, " + "but not honored by client" ) ); + return( POLARSSL_ERR_SSL_UNEXPECTED_MESSAGE ); + } } else if( ssl->in_msgtype != SSL_MSG_APPLICATION_DATA ) { diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 102a5b50c..6c2a92d5c 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -630,8 +630,8 @@ run_test "Renegotiation #5 (server-initiated, client-rejected)" \ -C "=> renegotiate" \ -S "=> renegotiate" \ -s "write hello request" \ - -s "SSL - An unexpected message was received from our peer" \ - -s "failed" + -S "SSL - An unexpected message was received from our peer" \ + -S "failed" # Tests for auth_mode From fae355e8ee3b220e1e754ad3de1724623a21ed6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 4 Jul 2014 14:32:27 +0200 Subject: [PATCH 2/3] Add tests for ssl_set_renegotiation_enforced() --- programs/ssl/ssl_server2.c | 9 +++++ tests/ssl-opt.sh | 70 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index d5f01bc0b..9cb742b3f 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -99,6 +99,7 @@ int main( int argc, char *argv[] ) #define DFL_RENEGOTIATION SSL_RENEGOTIATION_DISABLED #define DFL_ALLOW_LEGACY SSL_LEGACY_NO_RENEGOTIATION #define DFL_RENEGOTIATE 0 +#define DFL_RENEGO_DELAY -2 #define DFL_MIN_VERSION -1 #define DFL_MAX_VERSION -1 #define DFL_AUTH_MODE SSL_VERIFY_OPTIONAL @@ -159,6 +160,7 @@ struct options int renegotiation; /* enable / disable renegotiation */ int allow_legacy; /* allow legacy renegotiation */ int renegotiate; /* attempt renegotiation? */ + int renego_delay; /* delay before enforcing renegotiation */ int min_version; /* minimum protocol version accepted */ int max_version; /* maximum protocol version accepted */ int auth_mode; /* verify mode for connection */ @@ -676,6 +678,7 @@ int main( int argc, char *argv[] ) opt.renegotiation = DFL_RENEGOTIATION; opt.allow_legacy = DFL_ALLOW_LEGACY; opt.renegotiate = DFL_RENEGOTIATE; + opt.renego_delay = DFL_RENEGO_DELAY; opt.min_version = DFL_MIN_VERSION; opt.max_version = DFL_MAX_VERSION; opt.auth_mode = DFL_AUTH_MODE; @@ -765,6 +768,10 @@ int main( int argc, char *argv[] ) if( opt.renegotiate < 0 || opt.renegotiate > 1 ) goto usage; } + else if( strcmp( p, "renego_delay" ) == 0 ) + { + opt.renego_delay = atoi( q ); + } else if( strcmp( p, "min_version" ) == 0 ) { if( strcmp( q, "ssl3" ) == 0 ) @@ -1264,6 +1271,8 @@ int main( int argc, char *argv[] ) ssl_set_renegotiation( &ssl, opt.renegotiation ); ssl_legacy_renegotiation( &ssl, opt.allow_legacy ); + if( opt.renego_delay != DFL_RENEGO_DELAY ) + ssl_set_renegotiation_enforced( &ssl, opt.renego_delay ); #if defined(POLARSSL_X509_CRT_PARSE_C) if( strcmp( opt.ca_path, "none" ) != 0 && diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 6c2a92d5c..d4a41439d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -616,9 +616,11 @@ run_test "Renegotiation #4 (client-initiated, server-rejected)" \ -c "found renegotiation extension" \ -c "=> renegotiate" \ -S "=> renegotiate" \ - -S "write hello request" + -S "write hello request" \ + -c "SSL - An unexpected message was received from our peer" \ + -c "failed" -run_test "Renegotiation #5 (server-initiated, client-rejected)" \ +run_test "Renegotiation #5 (server-initiated, client-rejected, default)" \ "$P_SRV debug_level=4 renegotiation=1 renegotiate=1" \ "$P_CLI debug_level=4 renegotiation=0" \ 0 \ @@ -633,6 +635,70 @@ run_test "Renegotiation #5 (server-initiated, client-rejected)" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +run_test "Renegotiation #6 (server-initiated, client-rejected, not enforced)" \ + "$P_SRV debug_level=4 renegotiation=1 renegotiate=1 \ + renego_delay=-1" \ + "$P_CLI debug_level=4 renegotiation=0" \ + 0 \ + -C "client hello, adding renegotiation extension" \ + -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ + -S "found renegotiation extension" \ + -s "server hello, secure renegotiation extension" \ + -c "found renegotiation extension" \ + -C "=> renegotiate" \ + -S "=> renegotiate" \ + -s "write hello request" \ + -S "SSL - An unexpected message was received from our peer" \ + -S "failed" + +run_test "Renegotiation #7 (server-initiated, client-rejected, delay 1)" \ + "$P_SRV debug_level=4 renegotiation=1 renegotiate=1 \ + renego_delay=1" \ + "$P_CLI debug_level=4 renegotiation=0" \ + 0 \ + -C "client hello, adding renegotiation extension" \ + -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ + -S "found renegotiation extension" \ + -s "server hello, secure renegotiation extension" \ + -c "found renegotiation extension" \ + -C "=> renegotiate" \ + -S "=> renegotiate" \ + -s "write hello request" \ + -S "SSL - An unexpected message was received from our peer" \ + -S "failed" + +run_test "Renegotiation #8 (server-initiated, client-rejected, delay 0)" \ + "$P_SRV debug_level=4 renegotiation=1 renegotiate=1 \ + renego_delay=0" \ + "$P_CLI debug_level=4 renegotiation=0" \ + 0 \ + -C "client hello, adding renegotiation extension" \ + -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ + -S "found renegotiation extension" \ + -s "server hello, secure renegotiation extension" \ + -c "found renegotiation extension" \ + -C "=> renegotiate" \ + -S "=> renegotiate" \ + -s "write hello request" \ + -s "SSL - An unexpected message was received from our peer" \ + -s "failed" + +run_test "Renegotiation #9 (server-initiated, client-accepted, delay 0)" \ + "$P_SRV debug_level=4 renegotiation=1 renegotiate=1 \ + renego_delay=0" \ + "$P_CLI debug_level=4 renegotiation=1" \ + 0 \ + -c "client hello, adding renegotiation extension" \ + -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ + -s "found renegotiation extension" \ + -s "server hello, secure renegotiation extension" \ + -c "found renegotiation extension" \ + -c "=> renegotiate" \ + -s "=> renegotiate" \ + -s "write hello request" \ + -S "SSL - An unexpected message was received from our peer" \ + -S "failed" + # Tests for auth_mode run_test "Authentication #1 (server badcert, client required)" \ From 23647b4df51e23ef976375cac2de1664c0d369e7 Mon Sep 17 00:00:00 2001 From: Paul Bakker Date: Fri, 4 Jul 2014 15:00:12 +0200 Subject: [PATCH 3/3] Update ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index a0a8a18f9..3f5cf88d5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,6 +20,8 @@ Features * Optimize for RAM usage in example config.h for NSA Suite B profile. * Add POLARSSL_REMOVE_ARC4_CIPHERSUITES to allow removing RC4 ciphersuites from the default list (inactive by default). + * Add server-side enforcement of sent renegotiation requests + (ssl_set_renegotiation_enforced()) Changes * Add LINK_WITH_PTHREAD option in CMake for explicit linking that is