PR: 2115

Submitted by: Robin Seggelmann <seggelmann@fh-muenster.de>
Approved by: steve@openssl.org

Add Renegotiation extension to DTLS, fix DTLS ClientHello processing bug.

7 files changed, 258 insertions, 3 deletions

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index c1b0720bbf..7ca3405efb 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -764,6 +764,24 @@ int dtls1_send_finished(SSL *s, int a, int b, const char *sender, int slen)
 		p+=i;
 		l=i;
 
+	/* Copy the finished so we can use it for
+	 * renegotiation checks
+	 */
+	if(s->type == SSL_ST_CONNECT)
+		{
+		OPENSSL_assert(i <= EVP_MAX_MD_SIZE);
+		memcpy(s->s3->previous_client_finished, 
+		       s->s3->tmp.finish_md, i);
+		s->s3->previous_client_finished_len=i;
+		}
+	else
+		{
+		OPENSSL_assert(i <= EVP_MAX_MD_SIZE);
+		memcpy(s->s3->previous_server_finished, 
+		       s->s3->tmp.finish_md, i);
+		s->s3->previous_server_finished_len=i;
+		}
+
 #ifdef OPENSSL_SYS_WIN16
 		/* MSVC 1.5 does not clear the top bytes of the word unless
 		 * I do this.
diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index 0274745d3c..c4f820070f 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -635,7 +635,15 @@ int dtls1_client_hello(SSL *s)
 			*(p++)=comp->id;
 			}
 		*(p++)=0; /* Add the NULL method */
-		
+
+#ifndef OPENSSL_NO_TLSEXT
+		if ((p = ssl_add_clienthello_dtlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH)) == NULL)
+			{
+			SSLerr(SSL_F_SSL3_CLIENT_HELLO,ERR_R_INTERNAL_ERROR);
+			goto err;
+			}
+#endif		
+
 		l=(p-d);
 		d=buf;
 
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index eeffce3ccc..b1bc3f5850 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -382,3 +382,194 @@ int dtls1_listen(SSL *s, struct sockaddr *client)
 	(void) BIO_dgram_get_peer(SSL_get_rbio(s), client);
 	return 1;
 	}
+
+#ifndef OPENSSL_NO_TLSEXT
+unsigned char *ssl_add_clienthello_dtlsext(SSL *s, unsigned char *p, unsigned char *limit)
+	{
+	int extdatalen = 0;
+	unsigned char *ret = p;
+	int el;
+
+	ret+=2;
+	
+	if (ret>=limit) return NULL; /* this really never occurs, but ... */
+
+	/* Renegotiate extension */
+	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 - p - 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;
+
+	if ((extdatalen = ret-p-2)== 0) 
+		return p;
+
+	s2n(extdatalen,p);
+
+	return ret;
+	}
+
+int ssl_parse_clienthello_dtlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al)
+	{
+	unsigned short type;
+	unsigned short size;
+	unsigned short len;
+	unsigned char *data = *p;
+	int renegotiate_seen = 0;
+
+	if (data >= (d+n-2))
+		{
+		if (s->new_session
+			&& !(s->ctx->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION))
+			{
+			/* We should always see one extension: the renegotiate extension */
+	 		SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_TLSEXT, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+			*al = SSL_AD_ILLEGAL_PARAMETER; /* is this the right alert? */
+			return 0;
+			}
+		return 1;
+		}
+	n2s(data,len);
+
+	if (data > (d+n-len)) 
+		return 1;
+
+	while (data <= (d+n-4))
+		{
+		n2s(data,type);
+		n2s(data,size);
+		
+		if (data+size > (d+n))
+	   		return 1;
+		
+		if (type == TLSEXT_TYPE_renegotiate)
+			{
+			if(!ssl_parse_clienthello_renegotiate_ext(s, data, size, al))
+				return 0;
+			renegotiate_seen = 1;
+			}
+		
+		data+=size;
+		}
+
+	if (s->new_session && !renegotiate_seen
+		&& !(s->ctx->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION))
+		{
+		*al = SSL_AD_ILLEGAL_PARAMETER; /* is this the right alert? */
+	 	SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_TLSEXT, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+		return 0;
+		}
+
+	*p = data;
+	return 1;
+	}
+
+unsigned char *ssl_add_serverhello_dtlsext(SSL *s, unsigned char *p, unsigned char *limit)
+	{
+	int extdatalen = 0;
+	unsigned char *ret = p;
+	
+	ret+=2;
+	
+	if (ret>=limit) return NULL; /* this really never occurs, but ... */
+	
+	if(s->s3->send_connection_binding)
+		{
+		int el;
+
+		if(!ssl_add_serverhello_renegotiate_ext(s, 0, &el, 0))
+			{
+			SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+			return NULL;
+			}
+
+		if((limit - p - 4 - el) < 0) return NULL;
+          
+		s2n(TLSEXT_TYPE_renegotiate,ret);
+		s2n(el,ret);
+
+		if(!ssl_add_serverhello_renegotiate_ext(s, ret, &el, el))
+			{
+			SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+			return NULL;
+			}
+
+		ret += el;
+		}
+
+	if ((extdatalen = ret-p-2)== 0) 
+		return p;
+
+	s2n(extdatalen,p);
+
+	return ret;
+	}
+
+int ssl_parse_serverhello_dtlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al)
+	{
+	unsigned short type;
+	unsigned short size;
+	unsigned short len;
+	unsigned char *data = *p;
+	int renegotiate_seen = 0;
+	
+	if (data >= (d+n-2))
+		{
+		if (s->new_session
+			&& !(s->ctx->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION))
+			{
+			/* We should always see one extension: the renegotiate extension */
+	 		SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_TLSEXT, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+			*al = SSL_AD_ILLEGAL_PARAMETER; /* is this the right alert? */
+			return 0;
+			}
+		return 1;
+		}
+	n2s(data,len);
+	
+	if (data > (d+n-len)) 
+		return 1;
+	
+	while (data <= (d+n-4))
+		{
+		n2s(data,type);
+		n2s(data,size);
+		
+		if (data+size > (d+n))
+	   		return 1;
+		
+		if (type == TLSEXT_TYPE_renegotiate)
+			{
+			if(!ssl_parse_serverhello_renegotiate_ext(s, data, size, al))
+				return 0;
+			renegotiate_seen = 1;
+			}
+		
+		data+=size;
+		}
+
+	if (s->new_session && !renegotiate_seen
+		&& !(s->ctx->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION))
+		{
+		*al = SSL_AD_ILLEGAL_PARAMETER; /* is this the right alert? */
+	 	SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_TLSEXT, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+		return 0;
+		}
+
+	*p = data;
+	return 1;
+	}
+#endif
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index d79fb1985e..cbef062130 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -749,6 +749,8 @@ int dtls1_send_server_hello(SSL *s)
 		p+=sl;
 
 		/* put the cipher */
+		if (s->s3->tmp.new_cipher == NULL)
+			return -1;
 		i=ssl3_put_cipher_by_char(s->s3->tmp.new_cipher,p);
 		p+=i;
 
@@ -762,6 +764,14 @@ int dtls1_send_server_hello(SSL *s)
 			*(p++)=s->s3->tmp.new_compression->id;
 #endif
 
+#ifndef OPENSSL_NO_TLSEXT
+		if ((p = ssl_add_serverhello_dtlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH)) == NULL)
+			{
+			SSLerr(SSL_F_SSL3_SEND_SERVER_HELLO,ERR_R_INTERNAL_ERROR);
+			return -1;
+			}
+#endif
+
 		/* do the header */
 		l=(p-d);
 		d=buf;
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 35583ef30e..dfd8bf24d6 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -915,7 +915,7 @@ int ssl3_get_server_hello(SSL *s)
 
 #ifndef OPENSSL_NO_TLSEXT
 	/* TLS extensions*/
-	if (s->version > SSL3_VERSION)
+	if (s->version > SSL3_VERSION && s->version != DTLS1_VERSION && s->version != DTLS1_BAD_VER)
 		{
 		if (!ssl_parse_serverhello_tlsext(s,&p,d,n, &al))
 			{
@@ -929,6 +929,17 @@ int ssl3_get_server_hello(SSL *s)
 				goto err;
 			}
 		}
+
+	/* DTLS extensions */
+	if (s->version == DTLS1_VERSION || s->version == DTLS1_BAD_VER)
+	{
+		if (!ssl_parse_serverhello_dtlsext(s,&p,d,n, &al))
+		{
+			/* 'al' set by ssl_parse_serverhello_dtlsext */
+			SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_PARSE_TLSEXT);
+			goto f_err;
+		}
+	}
 #endif
 
 	if (p != (d+n))
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 77d7d878e3..ef760098b0 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1015,7 +1015,7 @@ int ssl3_get_client_hello(SSL *s)
 
 #ifndef OPENSSL_NO_TLSEXT
 	/* TLS extensions*/
-	if (s->version > SSL3_VERSION)
+	if (s->version > SSL3_VERSION && s->version != DTLS1_VERSION && s->version != DTLS1_BAD_VER)
 		{
 		if (!ssl_parse_clienthello_tlsext(s,&p,d,n, &al))
 			{
@@ -1081,6 +1081,17 @@ int ssl3_get_client_hello(SSL *s)
 			s->cipher_list_by_id = sk_SSL_CIPHER_dup(s->session->ciphers);
 			}
 		}
+
+	/* DTLS extensions */
+	if (s->version == DTLS1_VERSION || s->version == DTLS1_BAD_VER)
+		{
+		if (!ssl_parse_clienthello_dtlsext(s,&p,d,n, &al))
+			{
+				/* 'al' set by ssl_parse_clienthello_dtlsext */
+				SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_PARSE_TLSEXT);
+				goto f_err;
+			}
+		}
 #endif
 
 	/* Worst case, we will use the NULL compression, but if we have other
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 8f37008c1d..c7c93ac61d 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1053,6 +1053,12 @@ 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);
+
+unsigned char *ssl_add_clienthello_dtlsext(SSL *s, unsigned char *p, unsigned char *limit);
+unsigned char *ssl_add_serverhello_dtlsext(SSL *s, unsigned char *p, unsigned char *limit);
+int ssl_parse_clienthello_dtlsext(SSL *s, unsigned char **data, unsigned char *d, int n, int *al);
+int ssl_parse_serverhello_dtlsext(SSL *s, unsigned char **data, unsigned char *d, int n, int *al);
+
 #ifdef OPENSSL_NO_SHA256
 #define tlsext_tick_md	EVP_sha1
 #else