- Sending of handshake_failures during renegotiation added
- Handle two legacy modes differently: SSL_LEGACY_BREAK_HANDSHAKE and SSL_LEGACY_NO_RENEGOTIATION
diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
index 3d6e42c..dc2619a 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -122,8 +122,9 @@
#define SSL_RENEGOTIATION_ENABLED 0
#define SSL_RENEGOTIATION_DISABLED 1
-#define SSL_NO_LEGACY_RENEGOTIATION 0
-#define SSL_ALLOW_LEGACY_RENEGOTIATION 1
+#define SSL_LEGACY_NO_RENEGOTIATION 0
+#define SSL_LEGACY_ALLOW_RENEGOTIATION 1
+#define SSL_LEGACY_BREAK_HANDSHAKE 2
#define SSL_MAX_CONTENT_LEN 16384
@@ -758,10 +759,25 @@
/**
* \brief Prevent or allow legacy renegotiation.
- * (Default: SSL_NO_LEGACY_RENEGOTIATION)
- * Allowing legacy renegotiation makes the connection
- * vulnerable to specific man in the middle attacks.
- * (See RFC 5746)
+ * (Default: SSL_LEGACY_NO_RENEGOTIATION)
+ *
+ * SSL_LEGACY_NO_RENEGOTIATION allows connections to
+ * be established even if the peer does not support
+ * secure renegotiation, but does not allow renegotiation
+ * to take place if not secure.
+ * (Interoperable and secure option)
+ *
+ * SSL_LEGACY_ALLOW_RENEGOTIATION allows renegotiations
+ * with non-upgraded peers. Allowing legacy renegotiation
+ * makes the connection vulnerable to specific man in the
+ * middle attacks. (See RFC 5746)
+ * (Most interoperable and least secure option)
+ *
+ * SSL_LEGACY_BREAK_HANDSHAKE breaks off connections
+ * if peer does not support secure renegotiation. Results
+ * in interoperability issues with non-upgraded peers
+ * that do not support renegotiation altogether.
+ * (Most secure option, interoperability issues)
*
* \param ssl SSL context
* \param allow_legacy Prevent or allow (SSL_NO_LEGACY_RENEGOTIATION or
@@ -914,6 +930,8 @@
int ssl_handshake_server( ssl_context *ssl );
void ssl_handshake_wrapup( ssl_context *ssl );
+int ssl_send_fatal_handshake_failure( ssl_context *ssl );
+
int ssl_derive_keys( ssl_context *ssl );
int ssl_read_record( ssl_context *ssl );
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 9b82110..3d5008b 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -328,12 +328,17 @@
unsigned char *buf,
size_t len )
{
+ int ret;
+
if( ssl->renegotiation == SSL_INITIAL_HANDSHAKE )
{
if( len != 1 || buf[0] != 0x0 )
{
SSL_DEBUG_MSG( 1, ( "non-zero length renegotiated connection field" ) );
- /* TODO: Send handshake failure alert */
+
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+
return( POLARSSL_ERR_SSL_BAD_HS_SERVER_HELLO );
}
@@ -348,7 +353,10 @@
ssl->peer_verify_data, ssl->verify_data_len ) != 0 )
{
SSL_DEBUG_MSG( 1, ( "non-matching renegotiated connection field" ) );
- /* TODO: Send handshake failure alert */
+
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+
return( POLARSSL_ERR_SSL_BAD_HS_SERVER_HELLO );
}
}
@@ -366,6 +374,7 @@
size_t ext_len = 0;
unsigned char *buf, *ext;
int renegotiation_info_seen = 0;
+ int handshake_failure = 0;
SSL_DEBUG_MSG( 2, ( "=> parse server hello" ) );
@@ -563,18 +572,39 @@
/*
* Renegotiation security checks
*/
- if( ssl->renegotiation == SSL_RENEGOTIATION &&
- ssl->secure_renegotiation == SSL_SECURE_RENEGOTIATION &&
- renegotiation_info_seen == 0 )
+ if( ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
+ ssl->allow_legacy_renegotiation == SSL_LEGACY_BREAK_HANDSHAKE )
{
- SSL_DEBUG_MSG( 1, ( "renegotiation_info extension missing" ) );
- return( POLARSSL_ERR_SSL_BAD_HS_SERVER_HELLO );
+ SSL_DEBUG_MSG( 1, ( "legacy renegotiation, breaking off handshake" ) );
+ handshake_failure = 1;
+ }
+ else if( ssl->renegotiation == SSL_RENEGOTIATION &&
+ ssl->secure_renegotiation == SSL_SECURE_RENEGOTIATION &&
+ renegotiation_info_seen == 0 )
+ {
+ SSL_DEBUG_MSG( 1, ( "renegotiation_info extension missing (secure)" ) );
+ handshake_failure = 1;
}
-
- if( !ssl->allow_legacy_renegotiation &&
- ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION )
+ else if( ssl->renegotiation == SSL_RENEGOTIATION &&
+ ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
+ ssl->allow_legacy_renegotiation == SSL_LEGACY_NO_RENEGOTIATION )
{
SSL_DEBUG_MSG( 1, ( "legacy renegotiation not allowed" ) );
+ handshake_failure = 1;
+ }
+ else if( ssl->renegotiation == SSL_RENEGOTIATION &&
+ ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
+ renegotiation_info_seen == 1 )
+ {
+ SSL_DEBUG_MSG( 1, ( "renegotiation_info extension present (legacy)" ) );
+ handshake_failure = 1;
+ }
+
+ if( handshake_failure == 1 )
+ {
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+
return( POLARSSL_ERR_SSL_BAD_HS_SERVER_HELLO );
}
diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 742b912..209e5bd 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -42,12 +42,17 @@
unsigned char *buf,
size_t len )
{
+ int ret;
+
if( ssl->renegotiation == SSL_INITIAL_HANDSHAKE )
{
if( len != 1 || buf[0] != 0x0 )
{
SSL_DEBUG_MSG( 1, ( "non-zero length renegotiated connection field" ) );
- /* TODO: Send handshake failure alert */
+
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+
return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO );
}
@@ -60,7 +65,10 @@
memcmp( buf + 1, ssl->peer_verify_data, ssl->verify_data_len ) != 0 )
{
SSL_DEBUG_MSG( 1, ( "non-matching renegotiated connection field" ) );
- /* TODO: Send handshake failure alert */
+
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+
return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO );
}
}
@@ -77,7 +85,8 @@
unsigned int comp_len;
unsigned int ext_len = 0;
unsigned char *buf, *p, *ext;
- int renegotiation_info_seen;
+ int renegotiation_info_seen = 0;
+ int handshake_failure = 0;
SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) );
@@ -277,6 +286,10 @@
if( ssl->renegotiation == SSL_RENEGOTIATION )
{
SSL_DEBUG_MSG( 1, ( "received RENEGOTIATION SCSV during renegotiation" ) );
+
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+
return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO );
}
ssl->secure_renegotiation = SSL_SECURE_RENEGOTIATION;
@@ -306,7 +319,6 @@
ssl_optimize_checksum( ssl, ssl->session_negotiate->ciphersuite );
ext = buf + 44 + sess_len + ciph_len + comp_len;
- renegotiation_info_seen = 0;
while( ext_len )
{
@@ -349,26 +361,39 @@
/*
* Renegotiation security checks
*/
- if( ssl->renegotiation == SSL_RENEGOTIATION &&
- ssl->secure_renegotiation == SSL_SECURE_RENEGOTIATION &&
- renegotiation_info_seen == 0 )
+ if( ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
+ ssl->allow_legacy_renegotiation == SSL_LEGACY_BREAK_HANDSHAKE )
+ {
+ SSL_DEBUG_MSG( 1, ( "legacy renegotiation, breaking off handshake" ) );
+ handshake_failure = 1;
+ }
+ else if( ssl->renegotiation == SSL_RENEGOTIATION &&
+ ssl->secure_renegotiation == SSL_SECURE_RENEGOTIATION &&
+ renegotiation_info_seen == 0 )
{
SSL_DEBUG_MSG( 1, ( "renegotiation_info extension missing (secure)" ) );
- return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO );
+ handshake_failure = 1;
}
-
- if( ssl->renegotiation == SSL_RENEGOTIATION &&
- ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
- renegotiation_info_seen == 1 )
- {
- SSL_DEBUG_MSG( 1, ( "renegotiation_info extension present (legacy)" ) );
- return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO );
- }
-
- if( !ssl->allow_legacy_renegotiation &&
- ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION )
+ else if( ssl->renegotiation == SSL_RENEGOTIATION &&
+ ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
+ ssl->allow_legacy_renegotiation == SSL_LEGACY_NO_RENEGOTIATION )
{
SSL_DEBUG_MSG( 1, ( "legacy renegotiation not allowed" ) );
+ handshake_failure = 1;
+ }
+ else if( ssl->renegotiation == SSL_RENEGOTIATION &&
+ ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
+ renegotiation_info_seen == 1 )
+ {
+ SSL_DEBUG_MSG( 1, ( "renegotiation_info extension present (legacy)" ) );
+ handshake_failure = 1;
+ }
+
+ if( handshake_failure == 1 )
+ {
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+
return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_HELLO );
}
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index d47ec9f..7e638cd 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -2020,6 +2020,20 @@
return( 0 );
}
+int ssl_send_fatal_handshake_failure( ssl_context *ssl )
+{
+ int ret;
+
+ if( ( ret = ssl_send_alert_message( ssl,
+ SSL_ALERT_LEVEL_FATAL,
+ SSL_ALERT_MSG_HANDSHAKE_FAILURE ) ) != 0 )
+ {
+ return( ret );
+ }
+
+ return( 0 );
+}
+
int ssl_send_alert_message( ssl_context *ssl,
unsigned char level,
unsigned char message )
@@ -3513,15 +3527,28 @@
return( POLARSSL_ERR_SSL_UNEXPECTED_MESSAGE );
}
- if( ssl->disable_renegotiation == SSL_RENEGOTIATION_DISABLED )
+ if( ssl->disable_renegotiation == SSL_RENEGOTIATION_DISABLED ||
+ ( ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION &&
+ ssl->allow_legacy_renegotiation == SSL_LEGACY_NO_RENEGOTIATION ) )
{
SSL_DEBUG_MSG( 3, ( "ignoring renegotiation, sending alert" ) );
- if( ( ret = ssl_send_alert_message( ssl,
- SSL_ALERT_LEVEL_WARNING,
- SSL_ALERT_MSG_NO_RENEGOTIATION ) ) != 0 )
+ if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
{
- return( ret );
+ /*
+ * SSLv3 does not have a "no_renegotiation" alert
+ */
+ if( ( ret = ssl_send_fatal_handshake_failure( ssl ) ) != 0 )
+ return( ret );
+ }
+ else
+ {
+ if( ( ret = ssl_send_alert_message( ssl,
+ SSL_ALERT_LEVEL_WARNING,
+ SSL_ALERT_MSG_NO_RENEGOTIATION ) ) != 0 )
+ {
+ return( ret );
+ }
}
}
else
diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c
index df5274a..0c962f2 100644
--- a/programs/ssl/ssl_client2.c
+++ b/programs/ssl/ssl_client2.c
@@ -51,7 +51,7 @@
#define DFL_KEY_FILE ""
#define DFL_FORCE_CIPHER 0
#define DFL_RENEGOTIATION SSL_RENEGOTIATION_ENABLED
-#define DFL_ALLOW_LEGACY SSL_NO_LEGACY_RENEGOTIATION
+#define DFL_ALLOW_LEGACY SSL_LEGACY_NO_RENEGOTIATION
#define GET_REQUEST "GET %s HTTP/1.0\r\n\r\n"