From c27c9cb4f7ab74d772521fd927918f354724c2fc Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Mon, 14 Dec 2009 13:56:04 +0000 Subject: [PATCH 1/1] Allow initial connection (but no renegoriation) to servers which don't support RI. Reorganise RI checking code and handle some missing cases. --- ssl/t1_lib.c | 84 +++++++++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 7f576365e1..bdbb806fa5 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -636,21 +636,11 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in s->tlsext_status_type = -1; 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 */ - *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; - } - return 1; - } + goto ri_check; n2s(data,len); if (data > (d+n-len)) - return 1; + goto ri_check; while (data <= (d+n-4)) { @@ -658,7 +648,7 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in n2s(data,size); if (data+size > (d+n)) - return 1; + goto ri_check; #if 0 fprintf(stderr,"Received extension type %d size %d\n",type,size); #endif @@ -971,17 +961,23 @@ int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in /* session ticket processed earlier */ data+=size; } - - if (s->new_session && !renegotiate_seen - && !(s->ctx->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) - { - 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; - } - *p = data; + + ri_check: + + /* Need RI if renegotiating */ + + if (!renegotiate_seen && s->new_session && + !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) + { + /* FIXME: Spec currently doesn't give alert to use */ + *al = SSL_AD_ILLEGAL_PARAMETER; + SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_TLSEXT, + SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED); + return 0; + } + return 1; } @@ -995,19 +991,7 @@ int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in int renegotiate_seen = 0; if (data >= (d+n-2)) - { - /* Because the client does not see any renegotiation during an - attack, we must enforce this on all server hellos, even the - first */ - if (!(s->ctx->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) - { - /* We should always see one extension: the renegotiate extension */ - *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; - } - return 1; - } + goto ri_check; n2s(data,len); @@ -1017,7 +1001,7 @@ int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in n2s(data,size); if (data+size > (d+n)) - return 1; + goto ri_check; if (s->tlsext_debug_cb) s->tlsext_debug_cb(s, 1, type, data, size, @@ -1141,14 +1125,6 @@ int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in return 0; } - if (!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; - } - if (!s->hit && tlsext_servername == 1) { if (s->tlsext_hostname) @@ -1171,6 +1147,26 @@ int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, in } *p = data; + + ri_check: + + /* Determine if we need to see RI. Strictly speaking if we want to + * avoid an attack we should *always* see RI even on initial server + * hello because the client doesn't see any renegotiation during an + * attack. However this would mean we could not connect to any server + * which doesn't support RI so for the immediate future tolerate RI + * absence on initial connect only. + */ + if (!renegotiate_seen && s->new_session && + !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) + { + /* FIXME: Spec currently doesn't give alert to use */ + *al = SSL_AD_ILLEGAL_PARAMETER; + SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_TLSEXT, + SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED); + return 0; + } + return 1; } -- 2.34.1