From dc370e4969f8e0948f356e189ef0e382a0a3a446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jan 2015 10:24:59 +0000 Subject: [PATCH 1/7] Improve script portability --- tests/ssl-opt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 23f19ea27..6813754bc 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -372,7 +372,7 @@ fi # Pick a "unique" port in the range 10000-19999. PORT="0000$$" -PORT="1$(echo $PORT | tail -c 5)" +PORT="1$( printf $PORT | tail -c 4 )" # fix commands to use this port P_SRV="$P_SRV server_port=$PORT" From 6a0017b7c01f4a8c562486f6cb662286120f6c2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jan 2015 10:33:29 +0000 Subject: [PATCH 2/7] Rename variable for clarity --- programs/ssl/ssl_server2.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 242ca2ac3..70fffc533 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -631,7 +631,7 @@ void term_handler( int sig ) int main( int argc, char *argv[] ) { - int ret = 0, len, written, frags, exchanges; + int ret = 0, len, written, frags, exchanges_left; int version_suites[4][2]; unsigned char buf[IO_BUF_LEN]; #if defined(POLARSSL_KEY_EXCHANGE__SOME__PSK_ENABLED) @@ -1636,7 +1636,7 @@ reset: } #endif /* POLARSSL_X509_CRT_PARSE_C */ - exchanges = opt.exchanges; + exchanges_left = opt.exchanges; data_exchange: /* * 6. Read the HTTP Request @@ -1741,7 +1741,7 @@ data_exchange: * (only if we're going to exhange more data afterwards) */ #if defined(POLARSSL_SSL_RENEGOTIATION) - if( opt.renegotiate && exchanges > 1 ) + if( opt.renegotiate && exchanges_left > 1 ) { printf( " . Requestion renegotiation..." ); fflush( stdout ); @@ -1794,7 +1794,7 @@ data_exchange: /* * 7b. Continue doing data exchanges? */ - if( --exchanges > 0 ) + if( --exchanges_left > 0 ) goto data_exchange; /* From 34377b1e1c03faa59b6d6d165d1b6c7350f1402a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jan 2015 10:46:46 +0000 Subject: [PATCH 3/7] Fix send_close_notify usage. --- programs/ssl/ssl_client2.c | 6 ++++-- programs/ssl/ssl_server2.c | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 48b95af5b..657202944 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1299,8 +1299,10 @@ send_request: close_notify: printf( " . Closing the connection..." ); - /* Don't check for errors, the connection might already be closed */ - ssl_close_notify( &ssl ); + /* No error checking, the connection might be closed already */ + do ret = ssl_close_notify( &ssl ); + while( ret == POLARSSL_ERR_NET_WANT_WRITE ); + ret = 0; printf( " done\n" ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 70fffc533..5b4a6bdac 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1803,8 +1803,10 @@ data_exchange: close_notify: printf( " . Closing the connection..." ); - /* Don't check for errors, the connection might already be closed */ - ssl_close_notify( &ssl ); + /* No error checking, the connection might be closed already */ + do ret = ssl_close_notify( &ssl ); + while( ret == POLARSSL_ERR_NET_WANT_WRITE ); + ret = 0; printf( " done\n" ); goto reset; From 5d9cde25da079f82bd8f1cedbf900504c6b7a067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jan 2015 10:49:41 +0000 Subject: [PATCH 4/7] Move renego SCSV after actual ciphersuites --- library/ssl_cli.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 77ae8b4da..95699b4a8 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -633,18 +633,6 @@ static int ssl_write_client_hello( ssl_context *ssl ) // Skip writing ciphersuite length for now p += 2; - /* - * Add TLS_EMPTY_RENEGOTIATION_INFO_SCSV - */ -#if defined(POLARSSL_SSL_RENEGOTIATION) - if( ssl->renegotiation == SSL_INITIAL_HANDSHAKE ) -#endif - { - *p++ = (unsigned char)( SSL_EMPTY_RENEGOTIATION_INFO >> 8 ); - *p++ = (unsigned char)( SSL_EMPTY_RENEGOTIATION_INFO ); - n++; - } - for( i = 0; ciphersuites[i] != 0; i++ ) { ciphersuite_info = ssl_ciphersuite_from_id( ciphersuites[i] ); @@ -668,6 +656,18 @@ static int ssl_write_client_hello( ssl_context *ssl ) *p++ = (unsigned char)( ciphersuites[i] ); } + /* + * Add TLS_EMPTY_RENEGOTIATION_INFO_SCSV + */ +#if defined(POLARSSL_SSL_RENEGOTIATION) + if( ssl->renegotiation == SSL_INITIAL_HANDSHAKE ) +#endif + { + *p++ = (unsigned char)( SSL_EMPTY_RENEGOTIATION_INFO >> 8 ); + *p++ = (unsigned char)( SSL_EMPTY_RENEGOTIATION_INFO ); + n++; + } + /* Some versions of OpenSSL don't handle it correctly if not at end */ #if defined(POLARSSL_SSL_FALLBACK_SCSV) if( ssl->fallback == SSL_IS_FALLBACK ) From 59c6f2ef21b7c9fabd1c7e4eb4101530a78b97b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jan 2015 11:06:40 +0000 Subject: [PATCH 5/7] Avoid nested if's without braces. Creates a potential for confusing code if we later want to add an else clause. --- library/ssl_cli.c | 16 +++++++++------- library/ssl_srv.c | 16 ++++++++++------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 95699b4a8..c3729eeae 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -605,16 +605,18 @@ static int ssl_write_client_hello( ssl_context *ssl ) */ #if defined(POLARSSL_SSL_RENEGOTIATION) if( ssl->renegotiation == SSL_INITIAL_HANDSHAKE ) -#endif - if( ssl->session_negotiate->ticket != NULL && - ssl->session_negotiate->ticket_len != 0 ) { - ret = ssl->f_rng( ssl->p_rng, ssl->session_negotiate->id, 32 ); +#endif + if( ssl->session_negotiate->ticket != NULL && + ssl->session_negotiate->ticket_len != 0 ) + { + ret = ssl->f_rng( ssl->p_rng, ssl->session_negotiate->id, 32 ); - if( ret != 0 ) - return( ret ); + if( ret != 0 ) + return( ret ); - ssl->session_negotiate->length = n = 32; + ssl->session_negotiate->length = n = 32; + } } #endif /* POLARSSL_SSL_SESSION_TICKETS */ diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 0b947ba08..138d1f981 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1248,10 +1248,12 @@ static int ssl_parse_client_hello( ssl_context *ssl ) #if defined(POLARSSL_SSL_RENEGOTIATION) if( ssl->renegotiation == SSL_INITIAL_HANDSHAKE ) #endif - if( ( ret = ssl_fetch_input( ssl, 5 ) ) != 0 ) { - SSL_DEBUG_RET( 1, "ssl_fetch_input", ret ); - return( ret ); + if( ( ret = ssl_fetch_input( ssl, 5 ) ) != 0 ) + { + SSL_DEBUG_RET( 1, "ssl_fetch_input", ret ); + return( ret ); + } } buf = ssl->in_hdr; @@ -1301,10 +1303,12 @@ static int ssl_parse_client_hello( ssl_context *ssl ) #if defined(POLARSSL_SSL_RENEGOTIATION) if( ssl->renegotiation == SSL_INITIAL_HANDSHAKE ) #endif - if( ( ret = ssl_fetch_input( ssl, 5 + n ) ) != 0 ) { - SSL_DEBUG_RET( 1, "ssl_fetch_input", ret ); - return( ret ); + if( ( ret = ssl_fetch_input( ssl, 5 + n ) ) != 0 ) + { + SSL_DEBUG_RET( 1, "ssl_fetch_input", ret ); + return( ret ); + } } buf = ssl->in_msg; From a9a991633de1ec7dc72c5ec731f4f8a764b3b105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jan 2015 13:19:20 +0000 Subject: [PATCH 6/7] generate_errors.pl now errors on duplicate codes Duplication could easily happen during merges, now it can't go unnoticed. --- scripts/generate_errors.pl | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/generate_errors.pl b/scripts/generate_errors.pl index b9a8e9c0f..b25e99af8 100755 --- a/scripts/generate_errors.pl +++ b/scripts/generate_errors.pl @@ -56,13 +56,22 @@ my $hl_code_check = ""; my $headers = ""; +my %error_codes_seen; + while (my $line = ) { next if ($line =~ /compat-1.2.h/); my ($error_name, $error_code) = $line =~ /(POLARSSL_ERR_\w+)\s+\-(0x\w+)/; my ($description) = $line =~ /\/\*\*< (.*?)\.? \*\//; + + die "Duplicated error code: $error_code ($error_name)\n" + if( $error_codes_seen{$error_code}++ ); + $description =~ s/\\/\\\\/g; - $description = "DESCRIPTION MISSING" if ($description eq ""); + if ($description eq "") { + $description = "DESCRIPTION MISSING"; + warn "Missing description for $error_name\n"; + } my ($module_name) = $error_name =~ /^POLARSSL_ERR_([^_]+)/; From 11c919208de4bab3cbf6f940dc3dadb1c834ede7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jan 2015 13:21:29 +0000 Subject: [PATCH 7/7] Fix error code description. --- include/polarssl/ssl.h | 2 +- library/error.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 4de2d8c3d..5a96e05ca 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -120,7 +120,7 @@ #define POLARSSL_ERR_SSL_NO_CIPHER_CHOSEN -0x7380 /**< The server has no ciphersuites in common with the client. */ #define POLARSSL_ERR_SSL_NO_RNG -0x7400 /**< No RNG was provided to the SSL module. */ #define POLARSSL_ERR_SSL_NO_CLIENT_CERTIFICATE -0x7480 /**< No client certification received from the client, but required by the authentication mode. */ -#define POLARSSL_ERR_SSL_CERTIFICATE_TOO_LARGE -0x7500 /**< Our own certificate(s) is/are too large to send in an SSL message.*/ +#define POLARSSL_ERR_SSL_CERTIFICATE_TOO_LARGE -0x7500 /**< Our own certificate(s) is/are too large to send in an SSL message. */ #define POLARSSL_ERR_SSL_CERTIFICATE_REQUIRED -0x7580 /**< The own certificate is not set, but needed by the server. */ #define POLARSSL_ERR_SSL_PRIVATE_KEY_REQUIRED -0x7600 /**< The own private key or pre-shared key is not set, but needed. */ #define POLARSSL_ERR_SSL_CA_CHAIN_REQUIRED -0x7680 /**< No CA Chain is set, but required to operate. */ diff --git a/library/error.c b/library/error.c index 00956360b..1a57ee9da 100644 --- a/library/error.c +++ b/library/error.c @@ -386,7 +386,7 @@ void polarssl_strerror( int ret, char *buf, size_t buflen ) if( use_ret == -(POLARSSL_ERR_SSL_NO_CLIENT_CERTIFICATE) ) snprintf( buf, buflen, "SSL - No client certification received from the client, but required by the authentication mode" ); if( use_ret == -(POLARSSL_ERR_SSL_CERTIFICATE_TOO_LARGE) ) - snprintf( buf, buflen, "SSL - DESCRIPTION MISSING" ); + snprintf( buf, buflen, "SSL - Our own certificate(s) is/are too large to send in an SSL message" ); if( use_ret == -(POLARSSL_ERR_SSL_CERTIFICATE_REQUIRED) ) snprintf( buf, buflen, "SSL - The own certificate is not set, but needed by the server" ); if( use_ret == -(POLARSSL_ERR_SSL_PRIVATE_KEY_REQUIRED) )