From fe9ce2b7d663ee791ba6aebd8c236ba89076e824 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Tue, 24 Apr 2012 12:15:17 +0000 Subject: [PATCH] Submitted by: Peter Sylvester Reviewed by: steve Improved localisation of TLS extension handling and code tidy. --- ssl/s3_clnt.c | 19 ++++--------------- ssl/s3_srvr.c | 6 +----- ssl/ssl.h | 1 + ssl/ssl_err.c | 1 + ssl/ssl_locl.h | 4 +--- ssl/t1_lib.c | 27 ++++++++++++++++++++++++--- 6 files changed, 32 insertions(+), 26 deletions(-) diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index c00ace1598..8ac7f8a371 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -822,7 +822,7 @@ int ssl3_get_server_hello(SSL *s) STACK_OF(SSL_CIPHER) *sk; const SSL_CIPHER *c; unsigned char *p,*d; - int i,al,ok; + int i,al=SSL_AD_INTERNAL_ERROR,ok; unsigned int j; long n; #ifndef OPENSSL_NO_COMP @@ -928,7 +928,6 @@ int ssl3_get_server_hello(SSL *s) { if (!ssl_get_new_session(s,0)) { - al=SSL_AD_INTERNAL_ERROR; goto f_err; } } @@ -1002,7 +1001,6 @@ int ssl3_get_server_hello(SSL *s) */ if (s->session->compress_meth != 0) { - al=SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,SSL_R_INCONSISTENT_COMPRESSION); goto f_err; } @@ -1039,19 +1037,10 @@ int ssl3_get_server_hello(SSL *s) #ifndef OPENSSL_NO_TLSEXT /* TLS extensions*/ - if (s->version >= SSL3_VERSION) + if (!ssl_parse_serverhello_tlsext(s,&p,d,n)) { - if (!ssl_parse_serverhello_tlsext(s,&p,d,n, &al)) - { - /* 'al' set by ssl_parse_serverhello_tlsext */ - SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,SSL_R_PARSE_TLSEXT); - goto f_err; - } - if (ssl_check_serverhello_tlsext(s) <= 0) - { - SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,SSL_R_SERVERHELLO_TLSEXT); - goto err; - } + SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,SSL_R_PARSE_TLSEXT); + goto err; } #endif diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 357ae9503f..2132514efd 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -914,7 +914,7 @@ int ssl3_check_client_hello(SSL *s) int ssl3_get_client_hello(SSL *s) { - int i,j,ok,al,ret= -1; + int i,j,ok,al=SSL_AD_INTERNAL_ERROR,ret= -1; unsigned int cookie_len; long n; unsigned long id; @@ -1194,7 +1194,6 @@ int ssl3_get_client_hello(SSL *s) l2n(Time,pos); if (RAND_pseudo_bytes(pos,SSL3_RANDOM_SIZE-4) <= 0) { - al=SSL_AD_INTERNAL_ERROR; goto f_err; } } @@ -1249,7 +1248,6 @@ int ssl3_get_client_hello(SSL *s) /* Can't disable compression */ if (s->options & SSL_OP_NO_COMPRESSION) { - al=SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_INCONSISTENT_COMPRESSION); goto f_err; } @@ -1265,7 +1263,6 @@ int ssl3_get_client_hello(SSL *s) } if (s->s3->tmp.new_compression == NULL) { - al=SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_INVALID_COMPRESSION_ALGORITHM); goto f_err; } @@ -1314,7 +1311,6 @@ int ssl3_get_client_hello(SSL *s) */ if (s->session->compress_meth != 0) { - al=SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_INCONSISTENT_COMPRESSION); goto f_err; } diff --git a/ssl/ssl.h b/ssl/ssl.h index ac3fbdde08..2964111a53 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -2277,6 +2277,7 @@ void ERR_load_SSL_strings(void); #define SSL_F_SSL_RSA_PRIVATE_DECRYPT 187 #define SSL_F_SSL_RSA_PUBLIC_ENCRYPT 188 #define SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT 320 +#define SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT 321 #define SSL_F_SSL_SESSION_NEW 189 #define SSL_F_SSL_SESSION_PRINT_FP 190 #define SSL_F_SSL_SESSION_SET1_ID_CONTEXT 312 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 6781ccf664..a5367cdb45 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -248,6 +248,7 @@ static ERR_STRING_DATA SSL_str_functs[]= {ERR_FUNC(SSL_F_SSL_RSA_PRIVATE_DECRYPT), "SSL_RSA_PRIVATE_DECRYPT"}, {ERR_FUNC(SSL_F_SSL_RSA_PUBLIC_ENCRYPT), "SSL_RSA_PUBLIC_ENCRYPT"}, {ERR_FUNC(SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT), "SSL_SCAN_CLIENTHELLO_TLSEXT"}, +{ERR_FUNC(SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT), "SSL_SCAN_SERVERHELLO_TLSEXT"}, {ERR_FUNC(SSL_F_SSL_SESSION_NEW), "SSL_SESSION_new"}, {ERR_FUNC(SSL_F_SSL_SESSION_PRINT_FP), "SSL_SESSION_print_fp"}, {ERR_FUNC(SSL_F_SSL_SESSION_SET1_ID_CONTEXT), "SSL_SESSION_set1_id_context"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 98f1d97548..f9eb23d128 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1123,11 +1123,9 @@ int tls1_shared_list(SSL *s, unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit); unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit); int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n); -int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n, int *al); +int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n); int ssl_prepare_clienthello_tlsext(SSL *s); int ssl_prepare_serverhello_tlsext(SSL *s); -int ssl_check_clienthello_tlsext(SSL *s); -int ssl_check_serverhello_tlsext(SSL *s); #ifndef OPENSSL_NO_HEARTBEATS int tls1_heartbeat(SSL *s); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 4d46dec771..9ac0d9326a 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -123,6 +123,8 @@ const char tls1_version_str[]="TLSv1" OPENSSL_VERSION_PTEXT; static int tls_decrypt_ticket(SSL *s, const unsigned char *tick, int ticklen, const unsigned char *sess_id, int sesslen, SSL_SESSION **psess); +static int ssl_check_clienthello_tlsext(SSL *s); +int ssl_check_serverhello_tlsext(SSL *s); #endif SSL3_ENC_METHOD TLSv1_enc_data={ @@ -1706,7 +1708,7 @@ static char ssl_next_proto_validate(unsigned char *d, unsigned len) } #endif -int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al) +static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al) { unsigned short length; unsigned short type; @@ -1960,7 +1962,7 @@ int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in && !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) { *al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_TLSEXT, + SSLerr(SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED); return 0; } @@ -2040,7 +2042,7 @@ int ssl_prepare_serverhello_tlsext(SSL *s) return 1; } -int ssl_check_clienthello_tlsext(SSL *s) +static int ssl_check_clienthello_tlsext(SSL *s) { int ret=SSL_TLSEXT_ERR_NOACK; int al = SSL_AD_UNRECOGNIZED_NAME; @@ -2277,6 +2279,25 @@ int ssl_check_serverhello_tlsext(SSL *s) } } +int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n) + { + int al = -1; + if (s->version < SSL3_VERSION) + return 1; + if (ssl_scan_serverhello_tlsext(s, p, d, n, &al) <= 0) + { + ssl3_send_alert(s,SSL3_AL_FATAL,al); + return 0; + } + + if (ssl_check_serverhello_tlsext(s) <= 0) + { + SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_TLSEXT,SSL_R_SERVERHELLO_TLSEXT); + return 0; + } + return 1; +} + /* Since the server cache lookup is done early on in the processing of the * ClientHello, and other operations depend on the result, we need to handle * any TLS session ticket extension at the same time.