Server side version negotiation rewrite

This commit changes the way that we do server side protocol version negotiation. Previously we had a whole set of code that had an "up front" state machine dedicated to the negotiating the protocol version. This adds significant complexity to the state machine. Historically the justification for doing this was the support of SSLv2 which works quite differently to SSLv3+. However, we have now removed support for SSLv2 so there is little reason to maintain this complexity. The one slight difficulty is that, although we no longer support SSLv2, we do still support an SSLv3+ ClientHello in an SSLv2 backward compatible ClientHello format. This is generally only used by legacy clients. This commit adds support within the SSLv3 code for these legacy format ClientHellos. Server side version negotiation now works in much the same was as DTLS, i.e. we introduce the concept of TLS_ANY_VERSION. If s->version is set to that then when a ClientHello is received it will work out the most appropriate version to respond with. Also, SSLv23_method and SSLv23_server_method have been replaced with TLS_method and TLS_server_method respectively. The old SSLv23* names still exist as macros pointing at the new name, although they are deprecated. Subsequent commits will look at client side version negotiation, as well of removal of the old s23* code. Reviewed-by: Kurt Roeckx <[email protected]>
bnedu · May 16, 2015 · 32ec415 · 32ec415
1 parent 756eff7
commit 32ec415
Show file tree

Hide file tree

Showing 26 changed files with 645 additions and 322 deletions.
diff --git a/apps/ciphers.c b/apps/ciphers.c
@@ -94,7 +94,7 @@ int ciphers_main(int argc, char **argv)
     SSL_CTX *ctx = NULL;
     SSL *ssl = NULL;
     STACK_OF(SSL_CIPHER) *sk = NULL;
-    const SSL_METHOD *meth = SSLv23_server_method();
+    const SSL_METHOD *meth = TLS_server_method();
     int ret = 1, i, verbose = 0, Verbose = 0, use_supported = 0;
 #ifndef OPENSSL_NO_SSL_TRACE
     int stdname = 0;

diff --git a/apps/s_server.c b/apps/s_server.c
@@ -987,7 +987,7 @@ int s_server_main(int argc, char *argv[])
     ENGINE *e = NULL;
     EVP_PKEY *s_key = NULL, *s_dkey = NULL;
     SSL_CONF_CTX *cctx = NULL;
-    const SSL_METHOD *meth = SSLv23_server_method();
+    const SSL_METHOD *meth = TLS_server_method();
     SSL_EXCERT *exc = NULL;
     STACK_OF(OPENSSL_STRING) *ssl_args = NULL;
     STACK_OF(X509) *s_chain = NULL, *s_dchain = NULL;

diff --git a/crypto/threads/mttest.c b/crypto/threads/mttest.c
@@ -194,7 +194,7 @@ int main(int argc, char *argv[])
     SSL_CTX *c_ctx = NULL;
     char *scert = TEST_SERVER_CERT;
     char *ccert = TEST_CLIENT_CERT;
-    SSL_METHOD *ssl_method = SSLv23_method();
+    SSL_METHOD *ssl_method = TLS_method();
 
     RAND_seed(rnd_seed, sizeof rnd_seed);
 

diff --git a/demos/bio/saccept.c b/demos/bio/saccept.c
@@ -45,7 +45,7 @@ int main(int argc, char *argv[])
     /* Add ciphers and message digests */
     OpenSSL_add_ssl_algorithms();
 
-    ctx = SSL_CTX_new(SSLv23_server_method());
+    ctx = SSL_CTX_new(TLS_server_method());
     if (!SSL_CTX_use_certificate_file(ctx, CERT_FILE, SSL_FILETYPE_PEM))
         goto err;
     if (!SSL_CTX_use_PrivateKey_file(ctx, CERT_FILE, SSL_FILETYPE_PEM))

diff --git a/demos/bio/server-arg.c b/demos/bio/server-arg.c
@@ -29,7 +29,7 @@ int main(int argc, char *argv[])
     /* Add ciphers and message digests */
     OpenSSL_add_ssl_algorithms();
 
-    ctx = SSL_CTX_new(SSLv23_server_method());
+    ctx = SSL_CTX_new(TLS_server_method());
 
     cctx = SSL_CONF_CTX_new();
     SSL_CONF_CTX_set_flags(cctx, SSL_CONF_FLAG_SERVER);

diff --git a/demos/bio/server-conf.c b/demos/bio/server-conf.c
@@ -49,7 +49,7 @@ int main(int argc, char *argv[])
         goto err;
     }
 
-    ctx = SSL_CTX_new(SSLv23_server_method());
+    ctx = SSL_CTX_new(TLS_server_method());
     cctx = SSL_CONF_CTX_new();
     SSL_CONF_CTX_set_flags(cctx, SSL_CONF_FLAG_SERVER);
     SSL_CONF_CTX_set_flags(cctx, SSL_CONF_FLAG_CERTIFICATE);

diff --git a/demos/easy_tls/easy-tls.c b/demos/easy_tls/easy-tls.c
@@ -668,7 +668,7 @@ SSL_CTX *tls_create_ctx(struct tls_create_ctx_args a, void *apparg)
 
     ret =
         SSL_CTX_new((a.client_p ? SSLv23_client_method :
-                     SSLv23_server_method) ());
+                     TLS_server_method) ());
 
     if (ret == NULL)
         goto err;

diff --git a/demos/ssl/serv.cpp b/demos/ssl/serv.cpp
@@ -55,7 +55,7 @@ void main ()
 
   SSL_load_error_strings();
   SSLeay_add_ssl_algorithms();
-  meth = SSLv23_server_method();
+  meth = TLS_server_method();
   ctx = SSL_CTX_new (meth);
   if (!ctx) {
     ERR_print_errors_fp(stderr);

diff --git a/demos/state_machine/state_machine.c b/demos/state_machine/state_machine.c
@@ -119,7 +119,7 @@ SSLStateMachine *SSLStateMachine_new(const char *szCertificateFile,
 
     die_unless(pMachine);
 
-    pMachine->pCtx = SSL_CTX_new(SSLv23_server_method());
+    pMachine->pCtx = SSL_CTX_new(TLS_server_method());
     die_unless(pMachine->pCtx);
 
     n = SSL_CTX_use_certificate_file(pMachine->pCtx, szCertificateFile,

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
@@ -1562,13 +1562,18 @@ __owur const SSL_METHOD *SSLv3_server_method(void); /* SSLv3 */
 __owur const SSL_METHOD *SSLv3_client_method(void); /* SSLv3 */
 # endif
 
-__owur const SSL_METHOD *SSLv23_method(void); /* Negotiate highest available SSL/TLS
-                                        * version */
-__owur const SSL_METHOD *SSLv23_server_method(void); /* Negotiate highest available
-                                               * SSL/TLS version */
+#ifdef OPENSSL_USE_DEPRECATED
+#define SSLv23_method           TLS_method
+#define SSLv23_server_method    TLS_server_method
+#endif
+/* This next one will be deprecated in a subsequent commit */
 __owur const SSL_METHOD *SSLv23_client_method(void); /* Negotiate highest available
                                                * SSL/TLS version */
 
+/* Negotiate highest available SSL/TLS version */
+__owur const SSL_METHOD *TLS_method(void);
+__owur const SSL_METHOD *TLS_server_method(void);
+
 __owur const SSL_METHOD *TLSv1_method(void); /* TLSv1.0 */
 __owur const SSL_METHOD *TLSv1_server_method(void); /* TLSv1.0 */
 __owur const SSL_METHOD *TLSv1_client_method(void); /* TLSv1.0 */

diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
@@ -167,6 +167,9 @@ extern "C" {
 # define TLS1_2_VERSION                  0x0303
 # define TLS_MAX_VERSION                 TLS1_2_VERSION
 
+/* Special value for method supporting multiple versions */
+# define TLS_ANY_VERSION                 0x10000
+
 # define TLS1_VERSION_MAJOR              0x03
 # define TLS1_VERSION_MINOR              0x01
 

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
@@ -1109,6 +1109,21 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
      * then it was unexpected (Hello Request or Client Hello).
      */
 
+    /*
+     * Lets just double check that we've not got an SSLv2 record
+     */
+    if (rr->rec_version == SSL2_VERSION) {
+        /*
+         * Should never happen. ssl3_get_record() should only give us an SSLv2
+         * record back if this is the first packet and we are looking for an
+         * initial ClientHello. Therefore |type| should always be equal to
+         * |rr->type|. If not then something has gone horribly wrong
+         */
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_SSL3_READ_BYTES, ERR_R_INTERNAL_ERROR);
+        goto f_err;
+    }
+
     /*
      * In case of record types for which we have 'fragment' storage, fill
      * that so that we can process the data at a fixed place.
@@ -1464,4 +1479,12 @@ void ssl3_record_sequence_update(unsigned char *seq)
     }
 }
 
+int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl)
+{
+    return SSL3_RECORD_is_sslv2_record(&rl->rrec);
+}
 
+int RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl)
+{
+    return SSL3_RECORD_get_length(&rl->rrec);
+}
diff --git a/ssl/record/record.h b/ssl/record/record.h
@@ -132,6 +132,10 @@ typedef struct ssl3_buffer_st {
 #define SEQ_NUM_SIZE                            8
 
 typedef struct ssl3_record_st {
+    /* Record layer version */
+    /* r */
+    int rec_version;
+
     /* type of record */
     /* r */
     int type;
@@ -298,6 +302,8 @@ typedef struct record_layer_st {
  *                                                                           *
  *****************************************************************************/
 
+#define MIN_SSL2_RECORD_LEN     9
+
 #define RECORD_LAYER_set_read_ahead(rl, ra)     ((rl)->read_ahead = (ra))
 #define RECORD_LAYER_get_read_ahead(rl)         ((rl)->read_ahead)
 #define RECORD_LAYER_get_packet(rl)             ((rl)->packet)
@@ -319,6 +325,8 @@ void RECORD_LAYER_dup(RECORD_LAYER *dst, RECORD_LAYER *src);
 void RECORD_LAYER_reset_read_sequence(RECORD_LAYER *rl);
 void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl);
 int RECORD_LAYER_setup_comp_buffer(RECORD_LAYER *rl);
+int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl);
+int RECORD_LAYER_get_rrec_length(RECORD_LAYER *rl);
 __owur int ssl3_pending(const SSL *s);
 __owur int ssl23_read_bytes(SSL *s, int n);
 __owur int ssl23_write_bytes(SSL *s);

diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
@@ -186,6 +186,8 @@ int ssl3_release_write_buffer(SSL *s);
 #define SSL3_RECORD_set_off(r, o)               ((r)->off = (o))
 #define SSL3_RECORD_add_off(r, o)               ((r)->off += (o))
 #define SSL3_RECORD_get_epoch(r)                ((r)->epoch)
+#define SSL3_RECORD_is_sslv2_record(r) \
+            ((r)->rec_version == SSL2_VERSION)
 
 void SSL3_RECORD_clear(SSL3_RECORD *r);
 void SSL3_RECORD_release(SSL3_RECORD *r);

diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
@@ -166,6 +166,7 @@ void SSL3_RECORD_set_seq_num(SSL3_RECORD *r, const unsigned char *seq_num)
  */
 #define MAX_EMPTY_RECORDS 32
 
+#define SSL2_RT_HEADER_LENGTH   2
 /*-
  * Call this to get a new input record.
  * It will return <= 0 if more data is needed, normally due to an error
@@ -216,71 +217,121 @@ int ssl3_get_record(SSL *s)
         RECORD_LAYER_set_rstate(&s->rlayer, SSL_ST_READ_BODY);
 
         p = RECORD_LAYER_get_packet(&s->rlayer);
-        if (s->msg_callback)
-            s->msg_callback(0, 0, SSL3_RT_HEADER, p, 5, s,
-                            s->msg_callback_arg);
 
-        /* Pull apart the header into the SSL3_RECORD */
-        rr->type = *(p++);
-        ssl_major = *(p++);
-        ssl_minor = *(p++);
-        version = (ssl_major << 8) | ssl_minor;
-        n2s(p, rr->length);
+        /*
+         * Check whether this is a regular record or an SSLv2 style record. The
+         * latter is only used in an initial ClientHello for old clients.
+         */
+        if (s->first_packet && s->server && !s->read_hash && !s->enc_read_ctx
+                && (p[0] & 0x80) && (p[2] == SSL2_MT_CLIENT_HELLO)) {
+            /* SSLv2 style record */
+            if (s->msg_callback)
+                s->msg_callback(0, SSL2_VERSION, 0,  p + 2,
+                                RECORD_LAYER_get_packet_length(&s->rlayer) - 2,
+                                s, s->msg_callback_arg);
+
+            rr->type = SSL3_RT_HANDSHAKE;
+            rr->rec_version = SSL2_VERSION;
+
+            rr->length = ((p[0] & 0x7f) << 8) | p[1];
+
+            if (rr->length > SSL3_BUFFER_get_len(&s->rlayer.rbuf)
+                                    - SSL2_RT_HEADER_LENGTH) {
+                al = SSL_AD_RECORD_OVERFLOW;
+                SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_PACKET_LENGTH_TOO_LONG);
+                goto f_err;
+            }
 
-        /* Lets check version */
-        if (!s->first_packet) {
-            if (version != s->version) {
-                SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_WRONG_VERSION_NUMBER);
-                if ((s->version & 0xFF00) == (version & 0xFF00)
-                    && !s->enc_write_ctx && !s->write_hash)
-                    /*
-                     * Send back error using their minor version number :-)
-                     */
-                    s->version = (unsigned short)version;
-                al = SSL_AD_PROTOCOL_VERSION;
+            if (rr->length < MIN_SSL2_RECORD_LEN) {
+                al = SSL_AD_HANDSHAKE_FAILURE;
+                SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_LENGTH_TOO_SHORT);
                 goto f_err;
             }
-        }
+        } else {
+            /* SSLv3+ style record */
+            if (s->msg_callback)
+                s->msg_callback(0, 0, SSL3_RT_HEADER, p, 5, s,
+                                s->msg_callback_arg);
+
+            /* Pull apart the header into the SSL3_RECORD */
+            rr->type = *(p++);
+            ssl_major = *(p++);
+            ssl_minor = *(p++);
+            version = (ssl_major << 8) | ssl_minor;
+            rr->rec_version = version;
+            n2s(p, rr->length);
+
+            /* Lets check version */
+            if (!s->first_packet) {
+                if (version != s->version) {
+                    SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_WRONG_VERSION_NUMBER);
+                    if ((s->version & 0xFF00) == (version & 0xFF00)
+                        && !s->enc_write_ctx && !s->write_hash)
+                        /*
+                         * Send back error using their minor version number :-)
+                         */
+                        s->version = (unsigned short)version;
+                    al = SSL_AD_PROTOCOL_VERSION;
+                    goto f_err;
+                }
+            }
 
-        if ((version >> 8) != SSL3_VERSION_MAJOR) {
-            SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_WRONG_VERSION_NUMBER);
-            goto err;
-        }
+            if ((version >> 8) != SSL3_VERSION_MAJOR) {
+                SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_WRONG_VERSION_NUMBER);
+                goto err;
+            }
 
-        if (rr->length >
-                SSL3_BUFFER_get_len(&s->rlayer.rbuf)
-                - SSL3_RT_HEADER_LENGTH) {
-            al = SSL_AD_RECORD_OVERFLOW;
-            SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_PACKET_LENGTH_TOO_LONG);
-            goto f_err;
+            if (rr->length >
+                    SSL3_BUFFER_get_len(&s->rlayer.rbuf)
+                    - SSL3_RT_HEADER_LENGTH) {
+                al = SSL_AD_RECORD_OVERFLOW;
+                SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_PACKET_LENGTH_TOO_LONG);
+                goto f_err;
+            }
         }
 
         /* now s->rlayer.rstate == SSL_ST_READ_BODY */
     }
 
-    /* s->rlayer.rstate == SSL_ST_READ_BODY, get and decode the data */
-
-    if (rr->length >
-        RECORD_LAYER_get_packet_length(&s->rlayer) - SSL3_RT_HEADER_LENGTH) {
-        /* now s->packet_length == SSL3_RT_HEADER_LENGTH */
+    /*
+     * s->rlayer.rstate == SSL_ST_READ_BODY, get and decode the data.
+     * Calculate how much more data we need to read for the rest of the record
+     */
+    if (rr->rec_version == SSL2_VERSION) {
+        i = rr->length + SSL2_RT_HEADER_LENGTH - SSL3_RT_HEADER_LENGTH;
+    } else {
         i = rr->length;
+    }
+    if (i > 0) {
+        /* now s->packet_length == SSL3_RT_HEADER_LENGTH */
+
         n = ssl3_read_n(s, i, i, 1);
         if (n <= 0)
             return (n);         /* error or non-blocking io */
         /*
-         * now n == rr->length, and s->packet_length == SSL3_RT_HEADER_LENGTH
-         * + rr->length
+         * now n == rr->length, and
+         * s->packet_length == SSL3_RT_HEADER_LENGTH + rr->length
+         * or
+         * s->packet_length == SSL2_RT_HEADER_LENGTH + rr->length
+         * (if SSLv2 packet)
          */
+    } else {
+        n = 0;
     }
 
     /* set state for later operations */
     RECORD_LAYER_set_rstate(&s->rlayer, SSL_ST_READ_HEADER);
 
     /*
-     * At this point, s->packet_length == SSL3_RT_HEADER_LNGTH + rr->length,
+     * At this point, s->packet_length == SSL3_RT_HEADER_LENGTH + rr->length,
+     * or s->packet_length == SSL2_RT_HEADER_LENGTH + rr->length
      * and we have that many bytes in s->packet
      */
-    rr->input = &(RECORD_LAYER_get_packet(&s->rlayer)[SSL3_RT_HEADER_LENGTH]);
+    if(rr->rec_version == SSL2_VERSION) {
+        rr->input = &(RECORD_LAYER_get_packet(&s->rlayer)[SSL2_RT_HEADER_LENGTH]);
+    } else {
+        rr->input = &(RECORD_LAYER_get_packet(&s->rlayer)[SSL3_RT_HEADER_LENGTH]);
+    }
 
     /*
      * ok, we can now read from 's->packet' data into 'rr' rr->input points

diff --git a/ssl/s23_meth.c b/ssl/s23_meth.c
@@ -60,6 +60,7 @@
 #include <openssl/objects.h>
 #include "ssl_locl.h"
 
+/*
 static const SSL_METHOD *ssl23_get_method(int ver);
 static const SSL_METHOD *ssl23_get_method(int ver)
 {
@@ -76,7 +77,5 @@ static const SSL_METHOD *ssl23_get_method(int ver)
         return (TLSv1_2_method());
     else
         return (NULL);
-}
+}*/
 
-IMPLEMENT_ssl23_meth_func(SSLv23_method,
-                          ssl23_accept, ssl23_connect, ssl23_get_method)
diff --git a/ssl/s23_srvr.c b/ssl/s23_srvr.c
@@ -134,9 +134,6 @@ static const SSL_METHOD *ssl23_get_server_method(int ver)
         return (NULL);
 }
 
-IMPLEMENT_ssl23_meth_func(SSLv23_server_method,
-                          ssl23_accept,
-                          ssl_undefined_function, ssl23_get_server_method)
 
 int ssl23_accept(SSL *s)
 {