From 499684404ccf27b261cd72770c068fee4ea6f369 Mon Sep 17 00:00:00 2001
From: "Dr. Stephen Henson" <steve@openssl.org>
Date: Tue, 1 Dec 2009 17:42:15 +0000
Subject: [PATCH] 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.
---
ssl/d1_both.c | 18 +++++
ssl/d1_clnt.c | 10 ++-
ssl/d1_lib.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++++
ssl/d1_srvr.c | 10 +++
ssl/s3_clnt.c | 13 +++-
ssl/s3_srvr.c | 13 +++-
ssl/ssl_locl.h | 6 ++
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 2786b61c29..99935563d1 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
--
2.25.1