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.

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index c1b0720bbf37dca483b42fd717861d9d146389fb..7ca3405efbb0fa393cb6357326eb9e31ee7d5607 100644 (file)
--- 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 0274745d3ca644a0db9532d58f3ae04a461ddd1e..c4f820070f1725b8244a54f89b70853c2fe511ac 100644 (file)
--- 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 2786b61c29618a08b1bbf232e54250b74e50bf8d..99935563d1c9f70832a1bcdd0f215cf3ffffd628 100644 (file)
--- 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 d79fb1985ef687a41e267aa5198a79417cd0e092..cbef062130e69e4d50994d057f7e622db132007a 100644 (file)
--- 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 35583ef30effe52caad7c82eb453897310f7a361..dfd8bf24d64a21e9340ff30550910a50f4039bf6 100644 (file)
--- 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 77d7d878e381cf48dce1af3752ecbc3231d897f1..ef760098b04ec9611546a9760129c35668841390 100644 (file)
--- 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 8f37008c1df38501bcb628d4a51665757982ed7e..c7c93ac61df3c266993c84f8ae3d1b0e2b080cb1 100644 (file)
--- 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