Add support for magic cipher suite value (MCSV). Make secure renegotiation
authorDr. Stephen Henson <steve@openssl.org>
Tue, 8 Dec 2009 13:14:03 +0000 (13:14 +0000)
committerDr. Stephen Henson <steve@openssl.org>
Tue, 8 Dec 2009 13:14:03 +0000 (13:14 +0000)
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
ssl/s3_clnt.c
ssl/s3_srvr.c
ssl/ssl3.h
ssl/ssl_lib.c
ssl/t1_lib.c
ssl/t1_reneg.c

diff --git a/CHANGES b/CHANGES
index 60bf09a70a769db96a6f8c2bbd5dd473980a71e9..58f695dd857d65a0380c7d7bc86337acebc8a5e9 100644 (file)
--- a/CHANGES
+++ b/CHANGES
      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 <ekr@networkresonance.com> and Ben Laurie]
+     [Eric Rescorla <ekr@networkresonance.com>, 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
index 44f09b84639261302caf99a4aab7afed1d40a2ea..3d40ba41fac58179b27ba1a6ec8966b795c0279b 100644 (file)
@@ -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))
                        {
index 77d7d878e381cf48dce1af3752ecbc3231d897f1..5c74f1750bdb6cb2ee9aa77a854b12b582497531 100644 (file)
@@ -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))
                        {
index 5f0eee6dbba6c25778cc130f77c5e92f4f53806c..d929569aef8bd0861e34277e88abddd472f2f52c 100644 (file)
 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
index 04b6bfcb1c0606fa7e42a29327e65e4122789eb6..3583c4d0aa1c9fb3a4b60ca114da11534c776682 100644 (file)
@@ -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<num; i+=n)
                {
+               /* Check for MCSV */
+               if ((n != 3 || !p[0]) &&
+                       (p[n-2] == ((SSL3_CK_MCSV >> 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)
index ce53e50aeb58d2cef39bffc025b5735717cbb06d..7f576365e124c518da6c1d8cb08c9e0614b0bd34 100644 (file)
@@ -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))
                {
index 5222094f284e6a426087a0a0069f1cfbdffd490d..07fd5cb570dd7e63f6548991041e94657b4b58a2 100644 (file)
@@ -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;
     }