From 5a3d8eebb7667b32af0ccc3f12f314df6809d32d Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Mon, 3 Nov 2014 17:47:11 +0000 Subject: [PATCH] Only handle RI extension for SSLv3 Don't send or parse any extensions other than RI (which is needed to handle secure renegotation) for SSLv3. Reviewed-by: Matt Caswell --- ssl/t1_lib.c | 141 +++++++++++++++++++++++++-------------------------- 1 file changed, 70 insertions(+), 71 deletions(-) diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index db45c60534..149e7d6e11 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1147,15 +1147,38 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c } #endif - /* don't add extensions for SSLv3 unless doing secure renegotiation */ - if (s->client_version == SSL3_VERSION - && !s->s3->send_connection_binding) - return orig; - ret+=2; if (ret>=limit) return NULL; /* this really never occurs, but ... */ + /* Add RI if renegotiating */ + if (s->renegotiate) + { + int el; + + if(!ssl_add_clienthello_renegotiate_ext(s, 0, &el, 0)) + { + SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); + return NULL; + } + + if((limit - ret - 4 - el) < 0) return NULL; + + s2n(TLSEXT_TYPE_renegotiate,ret); + s2n(el,ret); + + if(!ssl_add_clienthello_renegotiate_ext(s, ret, &el, el)) + { + SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); + return NULL; + } + + ret += el; + } + /* Only add RI for SSLv3 */ + if (s->client_version == SSL3_VERSION) + goto done; + if (s->tlsext_hostname != NULL) { /* Add TLS extension servername to the Client Hello message */ @@ -1188,31 +1211,6 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c ret+=size_str; } - /* Add RI if renegotiating */ - if (s->renegotiate) - { - int el; - - if(!ssl_add_clienthello_renegotiate_ext(s, 0, &el, 0)) - { - SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); - return NULL; - } - - if((limit - ret - 4 - el) < 0) return NULL; - - s2n(TLSEXT_TYPE_renegotiate,ret); - s2n(el,ret); - - if(!ssl_add_clienthello_renegotiate_ext(s, ret, &el, el)) - { - SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); - return NULL; - } - - ret += el; - } - #ifndef OPENSSL_NO_SRP /* Add SRP username if there is one */ if (s->srp_ctx.login != NULL) @@ -1484,11 +1482,8 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c if (!custom_ext_add(s, 0, &ret, limit, al)) return NULL; #ifdef TLSEXT_TYPE_encrypt_then_mac - if (s->version != SSL3_VERSION) - { - s2n(TLSEXT_TYPE_encrypt_then_mac,ret); - s2n(0,ret); - } + s2n(TLSEXT_TYPE_encrypt_then_mac,ret); + s2n(0,ret); #endif /* Add padding to workaround bugs in F5 terminators. @@ -1521,6 +1516,8 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c } } + done: + if ((extdatalen = ret-orig-2)== 0) return orig; @@ -1542,21 +1539,10 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned c int using_ecc = (alg_k & (SSL_kECDHE|SSL_kECDHr|SSL_kECDHe)) || (alg_a & SSL_aECDSA); using_ecc = using_ecc && (s->session->tlsext_ecpointformatlist != NULL); #endif - /* don't add extensions for SSLv3, unless doing secure renegotiation */ - if (s->version == SSL3_VERSION && !s->s3->send_connection_binding) - return orig; ret+=2; if (ret>=limit) return NULL; /* this really never occurs, but ... */ - if (!s->hit && s->servername_done == 1 && s->session->tlsext_hostname != NULL) - { - if ((long)(limit - ret - 4) < 0) return NULL; - - s2n(TLSEXT_TYPE_server_name,ret); - s2n(0,ret); - } - if(s->s3->send_connection_binding) { int el; @@ -1581,6 +1567,18 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned c ret += el; } + /* Only add RI for SSLv3 */ + if (s->version == SSL3_VERSION) + goto done; + + if (!s->hit && s->servername_done == 1 && s->session->tlsext_hostname != NULL) + { + if ((long)(limit - ret - 4) < 0) return NULL; + + s2n(TLSEXT_TYPE_server_name,ret); + s2n(0,ret); + } + #ifndef OPENSSL_NO_EC if (using_ecc) { @@ -1721,12 +1719,11 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned c #ifdef TLSEXT_TYPE_encrypt_then_mac if (s->s3->flags & TLS1_FLAGS_ENCRYPT_THEN_MAC) { - /* Don't use encrypt_then_mac if AEAD, RC4 or SSL 3.0: + /* Don't use encrypt_then_mac if AEAD or RC4 * might want to disable for other cases too. */ if (s->s3->tmp.new_cipher->algorithm_mac == SSL_AEAD - || s->s3->tmp.new_cipher->algorithm_enc == SSL_RC4 - || s->version == SSL3_VERSION) + || s->s3->tmp.new_cipher->algorithm_enc == SSL_RC4) s->s3->flags &= ~TLS1_FLAGS_ENCRYPT_THEN_MAC; else { @@ -1751,6 +1748,8 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned c ret += len; } + done: + if ((extdatalen = ret-orig-2)== 0) return orig; @@ -1973,6 +1972,14 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char if (s->tlsext_debug_cb) s->tlsext_debug_cb(s, 0, type, data, size, s->tlsext_debug_arg); + if (type == TLSEXT_TYPE_renegotiate) + { + if(!ssl_parse_clienthello_renegotiate_ext(s, data, size, al)) + return 0; + renegotiate_seen = 1; + } + else if (s->version == SSL3_VERSION) + {} /* The servername extension is treated as follows: - Only the hostname type is supported with a maximum length of 255. @@ -1996,7 +2003,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char */ - if (type == TLSEXT_TYPE_server_name) + else if (type == TLSEXT_TYPE_server_name) { unsigned char *sdata; int servname_type; @@ -2217,12 +2224,6 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char return 0; } } - else if (type == TLSEXT_TYPE_renegotiate) - { - if(!ssl_parse_clienthello_renegotiate_ext(s, data, size, al)) - return 0; - renegotiate_seen = 1; - } else if (type == TLSEXT_TYPE_signature_algorithms) { int dsize; @@ -2418,10 +2419,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char } #ifdef TLSEXT_TYPE_encrypt_then_mac else if (type == TLSEXT_TYPE_encrypt_then_mac) - { - if (s->version != SSL3_VERSION) - s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC; - } + s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC; #endif /* If this ClientHello extension was unhandled and this is * a nonresumed connection, check whether the extension is a @@ -2544,7 +2542,16 @@ static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char s->tlsext_debug_cb(s, 1, type, data, size, s->tlsext_debug_arg); - if (type == TLSEXT_TYPE_server_name) + + if (type == TLSEXT_TYPE_renegotiate) + { + if(!ssl_parse_serverhello_renegotiate_ext(s, data, size, al)) + return 0; + renegotiate_seen = 1; + } + else if (s->version == SSL3_VERSION) + {} + else if (type == TLSEXT_TYPE_server_name) { if (s->tlsext_hostname == NULL || size > 0) { @@ -2726,13 +2733,6 @@ static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char memcpy(s->s3->alpn_selected, data + 3, len); s->s3->alpn_selected_len = len; } - - else if (type == TLSEXT_TYPE_renegotiate) - { - if(!ssl_parse_serverhello_renegotiate_ext(s, data, size, al)) - return 0; - renegotiate_seen = 1; - } #ifndef OPENSSL_NO_HEARTBEATS else if (type == TLSEXT_TYPE_heartbeat) { @@ -2759,10 +2759,9 @@ static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char #ifdef TLSEXT_TYPE_encrypt_then_mac else if (type == TLSEXT_TYPE_encrypt_then_mac) { - /* Ignore if inappropriate ciphersuite or SSL 3.0 */ + /* Ignore if inappropriate ciphersuite */ if (s->s3->tmp.new_cipher->algorithm_mac != SSL_AEAD - && s->s3->tmp.new_cipher->algorithm_enc != SSL_RC4 - && s->version != SSL3_VERSION) + && s->s3->tmp.new_cipher->algorithm_enc != SSL_RC4) s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC; } #endif -- 2.25.1