[openssl-commits] [openssl] master update

kaduk at mit.edu kaduk at mit.edu
Wed Jan 3 15:53:59 UTC 2018


The branch master has been updated
       via  767938fae99777c84818bdebae239934b8558a74 (commit)
       via  7bc2bddb14246f78da5d314e034359d44e55ce69 (commit)
      from  818b625d6c906ef40bfaf4403c278db8ba7bfa09 (commit)


- Log -----------------------------------------------------------------
commit 767938fae99777c84818bdebae239934b8558a74
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Wed Oct 4 12:09:16 2017 -0500

    Test that supported_groups is permitted in ServerHello
    
    Add a regression test for the functionality enabled in the
    previous commit.
    
    [extended tests]
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/4463)

commit 7bc2bddb14246f78da5d314e034359d44e55ce69
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Wed Oct 4 11:02:23 2017 -0500

    Permit the "supported_groups" extension in ServerHellos
    
    Although this is forbidden by all three(!) relevant specifications,
    there seem to be multiple server implementations in the wild that
    send it.  Since we didn't check for unexpected extensions in any
    given message type until TLS 1.3 support was added, our previous
    behavior was to silently accept these extensions and pass them over
    to the custom extension callback (if any).  In order to avoid
    regression of functionality, relax the check for "extension in
    unexpected context" for this specific case, but leave the protocol
    enforcment mechanism unchanged for other extensions and in other
    extension contexts.
    
    Leave a detailed comment to indicate what is going on.
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/4463)

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

Summary of changes:
 ssl/statem/extensions.c             | 28 +++++++++++++++++++++++++++-
 test/recipes/70-test_sslextension.t | 21 ++++++++++++++++-----
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 505337a..f0f5a02 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -160,8 +160,34 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         final_ec_pt_formats
     },
     {
+        /*
+         * "supported_groups" is spread across several specifications.
+         * It was originally specified as "elliptic_curves" in RFC 4492,
+         * and broadened to include named FFDH groups by RFC 7919.
+         * Both RFCs 4492 and 7919 do not include a provision for the server
+         * to indicate to the client the complete list of groups supported
+         * by the server, with the server instead just indicating the
+         * selected group for this connection in the ServerKeyExchange
+         * message.  TLS 1.3 adds a scheme for the server to indicate
+         * to the client its list of supported groups in the
+         * EncryptedExtensions message, but none of the relevant
+         * specifications permit sending supported_groups in the ServerHello.
+         * Nonetheless (possibly due to the close proximity to the
+         * "ec_point_formats" extension, which is allowed in the ServerHello),
+         * there are several servers that send this extension in the
+         * ServerHello anyway.  Up to and including the 1.1.0 release,
+         * we did not check for the presence of nonpermitted extensions,
+         * so to avoid a regression, we must permit this extension in the
+         * TLS 1.2 ServerHello as well.
+         *
+         * Note that there is no tls_parse_stoc_supported_groups function,
+         * so we do not perform any additional parsing, validation, or
+         * processing on the server's group list -- this is just a minimal
+         * change to preserve compatibility with these misbehaving servers.
+         */
         TLSEXT_TYPE_supported_groups,
-        SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
+        SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+        | SSL_EXT_TLS1_2_SERVER_HELLO,
         NULL, tls_parse_ctos_supported_groups, NULL,
         tls_construct_stoc_supported_groups,
         tls_construct_ctos_supported_groups, NULL
diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t
index 0622e4d..5382d2e 100644
--- a/test/recipes/70-test_sslextension.t
+++ b/test/recipes/70-test_sslextension.t
@@ -29,7 +29,8 @@ plan skip_all => "$test_name needs TLS enabled"
 use constant {
     UNSOLICITED_SERVER_NAME => 0,
     UNSOLICITED_SERVER_NAME_TLS13 => 1,
-    UNSOLICITED_SCT => 2
+    UNSOLICITED_SCT => 2,
+    NONCOMPLIANT_SUPPORTED_GROUPS => 3
 };
 
 my $testtype;
@@ -44,7 +45,7 @@ my $proxy = TLSProxy::Proxy->new(
 
 # Test 1: Sending a zero length extension block should pass
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 6;
+plan tests => 7;
 ok(TLSProxy::Message->success, "Zero extension length test");
 
 sub extension_filter
@@ -154,13 +155,15 @@ sub inject_unsolicited_extension
         $type = TLSProxy::Message::EXT_SERVER_NAME;
     } elsif ($testtype == UNSOLICITED_SCT) {
         $type = TLSProxy::Message::EXT_SCT;
+    } elsif ($testtype == NONCOMPLIANT_SUPPORTED_GROUPS) {
+        $type = TLSProxy::Message::EXT_SUPPORTED_GROUPS;
     }
     $message->set_extension($type, $ext);
     $message->repack();
 }
 
 SKIP: {
-    skip "TLS <= 1.2 disabled", 1 if alldisabled(("tls1", "tls1_1", "tls1_2"));
+    skip "TLS <= 1.2 disabled", 2 if alldisabled(("tls1", "tls1_1", "tls1_2"));
     #Test 4: Inject an unsolicited extension (<= TLSv1.2)
     $proxy->clear();
     $proxy->filter(\&inject_unsolicited_extension);
@@ -168,12 +171,20 @@ SKIP: {
     $proxy->clientflags("-no_tls1_3 -noservername");
     $proxy->start();
     ok(TLSProxy::Message->fail(), "Unsolicited server name extension");
+
+    #Test 5: Inject a noncompliant supported_groups extension (<= TLSv1.2)
+    $proxy->clear();
+    $proxy->filter(\&inject_unsolicited_extension);
+    $testtype = NONCOMPLIANT_SUPPORTED_GROUPS;
+    $proxy->clientflags("-no_tls1_3");
+    $proxy->start();
+    ok(TLSProxy::Message->success(), "Noncompliant supported_groups extension");
 }
 
 SKIP: {
     skip "TLS <= 1.2 or CT disabled", 1
         if alldisabled(("tls1", "tls1_1", "tls1_2")) || disabled("ct");
-    #Test 5: Same as above for the SCT extension which has special handling
+    #Test 6: Same as above for the SCT extension which has special handling
     $proxy->clear();
     $testtype = UNSOLICITED_SCT;
     $proxy->clientflags("-no_tls1_3");
@@ -183,7 +194,7 @@ SKIP: {
 
 SKIP: {
     skip "TLS 1.3 disabled", 1 if disabled("tls1_3");
-    #Test 6: Inject an unsolicited extension (TLSv1.3)
+    #Test 7: Inject an unsolicited extension (TLSv1.3)
     $proxy->clear();
     $proxy->filter(\&inject_unsolicited_extension);
     $testtype = UNSOLICITED_SERVER_NAME_TLS13;


More information about the openssl-commits mailing list