From 13f6d57b1ef964f2b9cbd8f68783884caef0e5cb Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Tue, 8 Dec 2009 13:14:03 +0000 Subject: [PATCH] Add support for magic cipher suite value (MCSV). Make secure renegotiation work in SSLv3: initial handshake has no extensions but includes MCSV, if server indicates RI support then renegotiation handshakes include RI. NB: current MCSV value is bogus for testing only, will be updated when we have an official value. Change mismatch alerts to handshake_failure as required by spec. Also have some debugging fprintfs so we can clearly see what is going on if OPENSSL_RI_DEBUG is set. --- CHANGES | 5 ++--- ssl/s3_clnt.c | 2 +- ssl/s3_srvr.c | 2 +- ssl/ssl3.h | 3 +++ ssl/ssl_lib.c | 31 +++++++++++++++++++++++++++++++ ssl/t1_lib.c | 10 +++++----- ssl/t1_reneg.c | 24 +++++++++++++++++++----- 7 files changed, 62 insertions(+), 15 deletions(-) diff --git a/CHANGES b/CHANGES index 60bf09a70a..58f695dd85 100644 --- a/CHANGES +++ b/CHANGES @@ -867,15 +867,14 @@ the updated NID creation version. This should correctly handle UTF8. [Steve Henson] - *) Implement - https://svn.resiprocate.org/rep/ietf-drafts/ekr/draft-rescorla-tls-renegotiate.txt. Re-enable + *) Implement draft-ietf-tls-renegotiation. Re-enable renegotiation but require the extension as needed. Unfortunately, SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION turns out to be a bad idea. It has been replaced by SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION which can be set with SSL_CTX_set_options(). This is really not recommended unless you know what you are doing. - [Eric Rescorla and Ben Laurie] + [Eric Rescorla , Ben Laurie, Steve Henson] *) Fixes to stateless session resumption handling. Use initial_ctx when issuing and attempting to decrypt tickets in case it has changed during diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 44f09b8463..3d40ba41fa 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -912,7 +912,7 @@ int ssl3_get_server_hello(SSL *s) #ifndef OPENSSL_NO_TLSEXT /* TLS extensions*/ - if (s->version > SSL3_VERSION) + if (s->version >= SSL3_VERSION) { if (!ssl_parse_serverhello_tlsext(s,&p,d,n, &al)) { diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 77d7d878e3..5c74f1750b 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) { if (!ssl_parse_clienthello_tlsext(s,&p,d,n, &al)) { diff --git a/ssl/ssl3.h b/ssl/ssl3.h index 5f0eee6dbb..d929569aef 100644 --- a/ssl/ssl3.h +++ b/ssl/ssl3.h @@ -128,6 +128,9 @@ extern "C" { #endif +/* Magic Cipher Suite Value. NB: bogus value used for testing */ +#define SSL3_CK_MCSV 0x03000FEC + #define SSL3_CK_RSA_NULL_MD5 0x03000001 #define SSL3_CK_RSA_NULL_SHA 0x03000002 #define SSL3_CK_RSA_RC4_40_MD5 0x03000003 diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 04b6bfcb1c..3583c4d0aa 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1357,6 +1357,22 @@ int ssl_cipher_list_to_bytes(SSL *s,STACK_OF(SSL_CIPHER) *sk,unsigned char *p, j = put_cb ? put_cb(c,p) : ssl_put_cipher_by_char(s,c,p); p+=j; } + /* If p == q, no ciphers and caller indicates an error, otherwise + * add MCSV + */ + if (p != q) + { + static SSL_CIPHER msvc = + { + 0, NULL, SSL3_CK_MCSV, 0, 0, 0, 0, 0, 0, 0, 0, 0 + }; + j = put_cb ? put_cb(&msvc,p) : ssl_put_cipher_by_char(s,&msvc,p); + p+=j; +#ifdef OPENSSL_RI_DEBUG + fprintf(stderr, "MCSV sent by client\n"); +#endif + } + return(p-q); } @@ -1367,6 +1383,8 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,unsigned char *p,int num, STACK_OF(SSL_CIPHER) *sk; int i,n; + s->s3->send_connection_binding = 0; + n=ssl_put_cipher_by_char(s,NULL,NULL); if ((num%n) != 0) { @@ -1383,6 +1401,19 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,unsigned char *p,int num, for (i=0; i> 8) & 0xff)) && + (p[n-1] == (SSL3_CK_MCSV & 0xff))) + { + s->s3->send_connection_binding = 1; + p += n; +#ifdef OPENSSL_RI_DEBUG + fprintf(stderr, "MCSV received by server\n"); +#endif + continue; + } + c=ssl_get_cipher_by_char(s,p); p+=n; if (c != NULL) diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index ce53e50aeb..7f576365e1 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -275,8 +275,9 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned cha int extdatalen=0; unsigned char *ret = p; - /* don't add extensions for SSLv3 */ - if (s->client_version == SSL3_VERSION) + /* don't add extensions for SSLv3 unless doing secure renegotiation */ + if (s->client_version == SSL3_VERSION + && !s->s3->send_connection_binding) return p; ret+=2; @@ -504,8 +505,8 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned cha int extdatalen=0; unsigned char *ret = p; - /* don't add extensions for SSLv3 */ - if (s->version == SSL3_VERSION) + /* don't add extensions for SSLv3, unless doing secure renegotiation */ + if (s->version == SSL3_VERSION && !s->s3->send_connection_binding) return p; ret+=2; @@ -633,7 +634,6 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in s->servername_done = 0; s->tlsext_status_type = -1; - s->s3->send_connection_binding = 0; if (data >= (d+n-2)) { diff --git a/ssl/t1_reneg.c b/ssl/t1_reneg.c index 5222094f28..07fd5cb570 100644 --- a/ssl/t1_reneg.c +++ b/ssl/t1_reneg.c @@ -130,10 +130,14 @@ int ssl_add_clienthello_renegotiate_ext(SSL *s, unsigned char *p, int *len, memcpy(p, s->s3->previous_client_finished, s->s3->previous_client_finished_len); +#ifdef OPENSSL_RI_DEBUG + fprintf(stderr, "RI extension sent by client\n"); +#endif } *len=s->s3->previous_client_finished_len + 1; - + + return 1; } @@ -166,7 +170,7 @@ int ssl_parse_clienthello_renegotiate_ext(SSL *s, unsigned char *d, int len, if(ilen != s->s3->previous_client_finished_len) { SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,SSL_R_RENEGOTIATION_MISMATCH); - *al=SSL_AD_ILLEGAL_PARAMETER; + *al=SSL_AD_HANDSHAKE_FAILURE; return 0; } @@ -174,9 +178,12 @@ int ssl_parse_clienthello_renegotiate_ext(SSL *s, unsigned char *d, int len, s->s3->previous_client_finished_len)) { SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,SSL_R_RENEGOTIATION_MISMATCH); - *al=SSL_AD_ILLEGAL_PARAMETER; + *al=SSL_AD_HANDSHAKE_FAILURE; return 0; } +#ifdef OPENSSL_RI_DEBUG + fprintf(stderr, "RI extension received by server\n"); +#endif s->s3->send_connection_binding=1; @@ -206,6 +213,9 @@ int ssl_add_serverhello_renegotiate_ext(SSL *s, unsigned char *p, int *len, memcpy(p, s->s3->previous_server_finished, s->s3->previous_server_finished_len); +#ifdef OPENSSL_RI_DEBUG + fprintf(stderr, "RI extension sent by server\n"); +#endif } *len=s->s3->previous_client_finished_len @@ -249,7 +259,7 @@ int ssl_parse_serverhello_renegotiate_ext(SSL *s, unsigned char *d, int len, if(ilen != expected_len) { SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_RENEGOTIATE_EXT,SSL_R_RENEGOTIATION_MISMATCH); - *al=SSL_AD_ILLEGAL_PARAMETER; + *al=SSL_AD_HANDSHAKE_FAILURE; return 0; } @@ -257,7 +267,7 @@ int ssl_parse_serverhello_renegotiate_ext(SSL *s, unsigned char *d, int len, s->s3->previous_client_finished_len)) { SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_RENEGOTIATE_EXT,SSL_R_RENEGOTIATION_MISMATCH); - *al=SSL_AD_ILLEGAL_PARAMETER; + *al=SSL_AD_HANDSHAKE_FAILURE; return 0; } d += s->s3->previous_client_finished_len; @@ -269,6 +279,10 @@ int ssl_parse_serverhello_renegotiate_ext(SSL *s, unsigned char *d, int len, *al=SSL_AD_ILLEGAL_PARAMETER; return 0; } +#ifdef OPENSSL_RI_DEBUG + fprintf(stderr, "RI extension received by client\n"); +#endif + s->s3->send_connection_binding=1; return 1; } -- 2.39.2