Submitted by: Peter Sylvester <peter.sylvester@edelweb.fr>

Reviewed by: steve
Improved localisation of TLS extension handling and code tidy.

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
 

ssl/s3_srvr.c

@@ -916,7 +916,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;
@@ -1196,7 +1196,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;
 			}
 	}
@@ -1251,7 +1250,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;
 			}
@@ -1267,7 +1265,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;
 			}
@@ -1316,7 +1313,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;
 		}

ssl/ssl.h

@@ -2292,6 +2292,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		 319
+#define SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT		 320
 #define SSL_F_SSL_SESSION_NEW				 189
 #define SSL_F_SSL_SESSION_PRINT_FP			 190
 #define SSL_F_SSL_SESSION_SET1_ID_CONTEXT		 312

ssl/ssl_err.c

@@ -247,6 +247,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"},

ssl/ssl_locl.h

@@ -1121,11 +1121,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);

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 int 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.