[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Thu May 4 15:06:24 UTC 2017


The branch master has been updated
       via  7a4e6a1e506d9bc88987b0ab6c028d8a057ca668 (commit)
       via  16afd71c1dbafad398fc0c40e90b06acccca29de (commit)
       via  bade29da33155afc87ed5806c996efea7684666c (commit)
       via  bb78552ee14c1caa867dfe782f6d1222c776e439 (commit)
      from  689f112d9806fa4a0c2f8c108226639455bc770d (commit)


- Log -----------------------------------------------------------------
commit 7a4e6a1e506d9bc88987b0ab6c028d8a057ca668
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 4 15:17:53 2017 +0100

    Updates to serverinfo fix based on review feedback
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3382)

commit 16afd71c1dbafad398fc0c40e90b06acccca29de
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 4 11:28:08 2017 +0100

    Add a test for loading serverinfo data from memory
    
    The previous commit fixed a bug which occurs when serverinfo is loaded
    from memory (not from a file). This adds a test for loading serverinfo
    from memory.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3382)

commit bade29da33155afc87ed5806c996efea7684666c
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 4 10:21:39 2017 +0100

    Fix SSL_CTX_use_serverinfo_ex() et al to properly handle V1 data
    
    SSL_CTX_use_serverinfo_ex() et al were always processing data as if it was
    V2 format, even if it was V1. This bug was masked because, although we had
    a test which loaded V1 serverinfo data from a file, the function
    SSL_CTX_use_serverinfo_file() transparently converts V1 data to V2 before
    calling SSL_CTX_use_serverinfo_ex().
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3382)

commit bb78552ee14c1caa867dfe782f6d1222c776e439
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 4 10:28:00 2017 +0100

    Revert "Fix clang compile time error"
    
    This reverts commit 1608d658af4163d2096cb469705d4ba96067877b.
    
    This is the wrong fix for this issue. The next commit provides a better
    fix.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3382)

-----------------------------------------------------------------------

Summary of changes:
 ssl/ssl_rsa.c     | 37 +++++++++++++++++++---------
 test/sslapitest.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 12 deletions(-)

diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index c3f2716..6f1c380 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -8,7 +8,6 @@
  */
 
 #include <stdio.h>
-#include <assert.h>
 #include "ssl_locl.h"
 #include "packet_locl.h"
 #include <openssl/bio.h>
@@ -19,6 +18,12 @@
 
 static int ssl_set_cert(CERT *c, X509 *x509);
 static int ssl_set_pkey(CERT *c, EVP_PKEY *pkey);
+
+#define  SYNTHV1CONTEXT     (SSL_EXT_TLS1_2_AND_BELOW_ONLY \
+                             | SSL_EXT_CLIENT_HELLO \
+                             | SSL_EXT_TLS1_2_SERVER_HELLO \
+                             | SSL_EXT_IGNORE_ON_RESUMPTION)
+
 int SSL_use_certificate(SSL *ssl, X509 *x)
 {
     int rv;
@@ -814,7 +819,7 @@ static int serverinfo_process_buffer(unsigned int version,
         unsigned int ext_type = 0;
         PACKET data;
 
-        if (!PACKET_get_net_4(&pkt, &context)
+        if ((version == SSL_SERVERINFOV2 && !PACKET_get_net_4(&pkt, &context))
                 || !PACKET_get_net_2(&pkt, &ext_type)
                 || !PACKET_get_length_prefixed_2(&pkt, &data))
             return 0;
@@ -822,7 +827,18 @@ static int serverinfo_process_buffer(unsigned int version,
         if (ctx == NULL)
             continue;
 
-        if (version == SSL_SERVERINFOV1) {
+        /*
+         * The old style custom extensions API could be set separately for
+         * server/client, i.e. you could set one custom extension for a client,
+         * and *for the same extension in the same SSL_CTX* you could set a
+         * custom extension for the server as well. It seems quite weird to be
+         * setting a custom extension for both client and server in a single
+         * SSL_CTX - but theoretically possible. This isn't possible in the
+         * new API. Therefore, if we have V1 serverinfo we use the old API. We
+         * also use the old API even if we have V2 serverinfo but the context
+         * looks like an old style <= TLSv1.2 extension.
+         */
+        if (version == SSL_SERVERINFOV1 || context == SYNTHV1CONTEXT) {
             if (!SSL_CTX_add_server_custom_ext(ctx, ext_type,
                                                serverinfo_srv_add_cb,
                                                NULL, NULL,
@@ -904,7 +920,6 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
     int ret = 0;
     BIO *bin = NULL;
     size_t num_extensions = 0, contextoff = 0;
-    unsigned int version = 0;
 
     if (ctx == NULL || file == NULL) {
         SSLerr(SSL_F_SSL_CTX_USE_SERVERINFO_FILE, ERR_R_PASSED_NULL_PARAMETER);
@@ -922,6 +937,8 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
     }
 
     for (num_extensions = 0;; num_extensions++) {
+        unsigned int version;
+
         if (PEM_read_bio(bin, &name, &header, &extension, &extension_length)
             == 0) {
             /*
@@ -988,15 +1005,13 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
         }
         serverinfo = tmp;
         if (contextoff > 0) {
-            unsigned int synthcontext = SSL_EXT_CLIENT_HELLO
-                                        | SSL_EXT_TLS1_2_SERVER_HELLO;
             unsigned char *sinfo = serverinfo + serverinfo_length;
 
             /* We know this only uses the last 2 bytes */
             sinfo[0] = 0;
             sinfo[1] = 0;
-            sinfo[2] = (synthcontext >> 8) & 0xff;
-            sinfo[3] = synthcontext & 0xff;
+            sinfo[2] = (SYNTHV1CONTEXT >> 8) & 0xff;
+            sinfo[3] = SYNTHV1CONTEXT & 0xff;
         }
         memcpy(serverinfo + serverinfo_length + contextoff,
                extension, extension_length);
@@ -1010,10 +1025,8 @@ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file)
         extension = NULL;
     }
 
-    assert(version != 0);
-    if (version != 0)
-        ret = SSL_CTX_use_serverinfo_ex(ctx, version, serverinfo,
-                                        serverinfo_length);
+    ret = SSL_CTX_use_serverinfo_ex(ctx, SSL_SERVERINFOV2, serverinfo,
+                                    serverinfo_length);
  end:
     /* SSL_CTX_use_serverinfo makes a local copy of the serverinfo. */
     OPENSSL_free(name);
diff --git a/test/sslapitest.c b/test/sslapitest.c
index c43adba..bfa3a30 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -53,6 +53,21 @@ struct sslapitest_log_counts {
     unsigned int server_application_secret_count;
 };
 
+
+static unsigned char serverinfov1[] = {
+    0xff, 0xff, /* Dummy extension type */
+    0x00, 0x01, /* Extension length is 1 byte */
+    0xff        /* Dummy extension data */
+};
+
+static unsigned char serverinfov2[] = {
+    0x00, 0x00, 0x00,
+    (unsigned char)(SSL_EXT_CLIENT_HELLO & 0xff), /* Dummy context - 4 bytes */
+    0xff, 0xff, /* Dummy extension type */
+    0x00, 0x01, /* Extension length is 1 byte */
+    0xff        /* Dummy extension data */
+};
+
 static void client_keylog_callback(const SSL *ssl, const char *line)
 {
     int line_length = strlen(line);
@@ -2043,6 +2058,63 @@ end:
     return testresult;
 }
 
+/*
+ * Test loading of serverinfo data in various formats. test_sslmessages actually
+ * tests to make sure the extensions appear in the handshake
+ */
+static int test_serverinfo(int tst)
+{
+    unsigned int version;
+    unsigned char *sibuf;
+    size_t sibuflen;
+    int ret, expected, testresult = 0;
+    SSL_CTX *ctx;
+
+    ctx = SSL_CTX_new(TLS_method());
+    if (!TEST_ptr(ctx))
+        goto end;
+
+    if ((tst & 0x01) == 0x01)
+        version = SSL_SERVERINFOV2;
+    else
+        version = SSL_SERVERINFOV1;
+
+    if ((tst & 0x02) == 0x02) {
+        sibuf = serverinfov2;
+        sibuflen = sizeof(serverinfov2);
+        expected = (version == SSL_SERVERINFOV2);
+    } else {
+        sibuf = serverinfov1;
+        sibuflen = sizeof(serverinfov1);
+        expected = (version == SSL_SERVERINFOV1);
+    }
+
+    if ((tst & 0x04) == 0x04) {
+        ret = SSL_CTX_use_serverinfo_ex(ctx, version, sibuf, sibuflen);
+    } else {
+        ret = SSL_CTX_use_serverinfo(ctx, sibuf, sibuflen);
+
+        /*
+         * The version variable is irrelevant in this case - it's what is in the
+         * buffer that matters
+         */
+        if ((tst & 0x02) == 0x02)
+            expected = 0;
+        else
+            expected = 1;
+    }
+
+    if (!TEST_true(ret == expected))
+        goto end;
+
+    testresult = 1;
+
+ end:
+    SSL_CTX_free(ctx);
+
+    return testresult;
+}
+
 int test_main(int argc, char *argv[])
 {
     int testresult = 1;
@@ -2093,6 +2165,7 @@ int test_main(int argc, char *argv[])
 #else
     ADD_ALL_TESTS(test_custom_exts, 2);
 #endif
+    ADD_ALL_TESTS(test_serverinfo, 8);
 
     testresult = run_tests(argv[0]);
 


More information about the openssl-commits mailing list