From d5b8c464991797946aca0f3c5c42ddcd5cd8f7df Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Tue, 1 Dec 2009 17:41:42 +0000 Subject: [PATCH] PR: 2115 Submitted by: Robin Seggelmann 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 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 -- 2.34.1