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 1d271c1bfaf72bed80c3b96a8ecbc974f5c3fd6b..3b9c7567b55b7d4e7776ac15b66dc1cd8d01300a 100644 (file)
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -765,6 +765,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 d876c5974cec4533883ee3bd68a583d0e625c665..d71c7f2c969be008b8587523536ae9a062e12ebb 100644 (file)
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -632,7 +632,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 63bfbacc8216fbd1b5c65d9e8ba7c5962cc1a99e..cee9d4e10e92691c8ac713bd66eafc63f91869f0 100644 (file)
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -404,3 +404,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 9098e2eb5ad700fca46fbba4e5e8378d253bad54..6b0a9c4a050143a412caebb7e1b4a41dc166c6d8 100644 (file)
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -730,6 +730,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;
 
@@ -743,6 +745,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 9929d0c92c8be292c64f2d7f22e6087f06c762e8..d738afbff442dd4da244e7af9b800d4ad696686f 100644 (file)
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -861,7 +861,7 @@ int ssl3_get_server_hello(SSL *s)
 #endif
 #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))
                        {
@@ -875,6 +875,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
 
 

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index c698513a093951056937f27e8251150378c17523..a685fd5f0b250ee026f5b2eadac2b89bd627abf4 100644 (file)
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -957,7 +957,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))
                        {
@@ -970,6 +970,17 @@ int ssl3_get_client_hello(SSL *s)
                        SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT);
                        goto err;
                }
+
+       /* 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
         * options, we will now look for them.  We have i-1 compression

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index ffb6085f3d18c3ee3ed5a1a1f3f32f423100273e..73ba499be2141b47822303305248595d006d9325 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -976,6 +976,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