Use empty renegotiate extension instead of SCSV for TLS > 1.0

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24161)

7 files changed, 114 insertions, 11 deletions

diff --git a/test/recipes/70-test_renegotiation.t b/test/recipes/70-test_renegotiation.t
index 37fbfd5854..445d447dc9 100644
--- a/test/recipes/70-test_renegotiation.t
+++ b/test/recipes/70-test_renegotiation.t
@@ -7,6 +7,7 @@
 # https://www.openssl.org/source/license.html
 
 use strict;
+use List::Util 'first';
 use OpenSSL::Test qw/:DEFAULT cmdstr srctop_file bldtop_dir/;
 use OpenSSL::Test::Utils;
 use TLSProxy::Proxy;
@@ -26,7 +27,7 @@ plan skip_all => "$test_name needs the sock feature enabled"
 plan skip_all => "$test_name needs TLS <= 1.2 enabled"
     if alldisabled(("ssl3", "tls1", "tls1_1", "tls1_2"));
 
-plan tests => 5;
+plan tests => 9;
 
 my $proxy = TLSProxy::Proxy->new(
     undef,
@@ -42,9 +43,10 @@ $proxy->reneg(1);
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 ok(TLSProxy::Message->success(), "Basic renegotiation");
 
-#Test 2: Client does not send the Reneg SCSV. Reneg should fail
+#Test 2: Seclevel 0 client does not send the Reneg SCSV. Reneg should fail
 $proxy->clear();
-$proxy->filter(\&reneg_filter);
+$proxy->filter(\&reneg_scsv_filter);
+$proxy->cipherc("DEFAULT:\@SECLEVEL=0");
 $proxy->clientflags("-no_tls1_3");
 $proxy->serverflags("-client_renegotiation");
 $proxy->reneg(1);
@@ -52,9 +54,24 @@ $proxy->start();
 ok(TLSProxy::Message->fail(), "No client SCSV");
 
 SKIP: {
+    skip "TLSv1.2 disabled", 1
+        if disabled("tls1_2");
+
+    #Test 3: TLS 1.2 client does not send the Reneg extension. Reneg should fail
+
+    $proxy->clear();
+    $proxy->filter(\&reneg_ext_filter);
+    $proxy->clientflags("-no_tls1_3");
+    $proxy->serverflags("-client_renegotiation");
+    $proxy->reneg(1);
+    $proxy->start();
+    ok(TLSProxy::Message->fail(), "No client extension");
+}
+
+SKIP: {
     skip "TLSv1.2 or TLSv1.1 disabled", 1
         if disabled("tls1_2") || disabled("tls1_1");
-    #Test 3: Check that the ClientHello version remains the same in the reneg
+    #Test 4: Check that the ClientHello version remains the same in the reneg
     #        handshake
     $proxy->clear();
     $proxy->filter(undef);
@@ -84,7 +101,7 @@ SKIP: {
     skip "TLSv1.2 disabled", 1
         if disabled("tls1_2");
 
-    #Test 4: Test for CVE-2021-3449. client_sig_algs instead of sig_algs in
+    #Test 5: Test for CVE-2021-3449. client_sig_algs instead of sig_algs in
     #        resumption ClientHello
     $proxy->clear();
     $proxy->filter(\&sigalgs_filter);
@@ -98,7 +115,7 @@ SKIP: {
 SKIP: {
     skip "TLSv1.2 and TLSv1.1 disabled", 1
         if disabled("tls1_2") && disabled("tls1_1");
-    #Test 5: Client fails to do renegotiation
+    #Test 6: Client fails to do renegotiation
     $proxy->clear();
     $proxy->filter(undef);
     $proxy->serverflags("-no_tls1_3");
@@ -109,7 +126,60 @@ SKIP: {
         "Check client renegotiation failed");
 }
 
-sub reneg_filter
+SKIP: {
+    skip "TLSv1 disabled", 1
+        if disabled("tls1");
+
+    #Test 7: Check that SECLEVEL 0 sends SCSV not RI extension
+    $proxy->clear();
+    $proxy->filter(undef);
+    $proxy->cipherc("DEFAULT:\@SECLEVEL=0");
+    $proxy->start();
+
+    my $clientHello = first { $_->mt == TLSProxy::Message::MT_CLIENT_HELLO } @{$proxy->message_list};
+    my $has_scsv = 255 ~~ @{$clientHello->ciphersuites};
+    my $has_ri_extension = exists $clientHello->extension_data()->{TLSProxy::Message::EXT_RENEGOTIATE};
+
+    ok($has_scsv && !$has_ri_extension, "SECLEVEL=0 should use SCSV not RI extension by default");
+}
+
+SKIP: {
+    skip "TLSv1.2 disabled", 1
+        if disabled("tls1_2");
+
+    #Test 8: Check that SECLEVEL0 + TLS 1.2 sends RI extension not SCSV
+    $proxy->clear();
+    $proxy->filter(undef);
+    $proxy->cipherc("DEFAULT:\@SECLEVEL=0");
+    $proxy->clientflags("-tls1_2");
+    $proxy->start();
+
+    my $clientHello = first { $_->mt == TLSProxy::Message::MT_CLIENT_HELLO } @{$proxy->message_list};
+    my $has_scsv = 255 ~~ @{$clientHello->ciphersuites};
+    my $has_ri_extension = exists $clientHello->extension_data()->{TLSProxy::Message::EXT_RENEGOTIATE};
+
+    ok(!$has_scsv && $has_ri_extension, "TLS1.2 should use RI extension despite SECLEVEL=0");
+}
+
+
+SKIP: {
+    skip "TLSv1.3 disabled", 1
+        if disabled("tls1_3");
+
+    #Test 9: Check that TLS 1.3 sends neither RI extension nor SCSV
+    $proxy->clear();
+    $proxy->filter(undef);
+    $proxy->clientflags("-tls1_3");
+    $proxy->start();
+
+    my $clientHello = first { $_->mt == TLSProxy::Message::MT_CLIENT_HELLO } @{$proxy->message_list};
+    my $has_scsv = 255 ~~ @{$clientHello->ciphersuites};
+    my $has_ri_extension = exists $clientHello->extension_data()->{TLSProxy::Message::EXT_RENEGOTIATE};
+
+    ok(!$has_scsv && !$has_ri_extension, "TLS1.3 should not use RI extension or SCSV");
+}
+
+sub reneg_scsv_filter
 {
     my $proxy = shift;
 
@@ -129,6 +199,23 @@ sub reneg_filter
     }
 }
 
+sub reneg_ext_filter
+{
+    my $proxy = shift;
+
+    # We're only interested in the initial ClientHello message
+    if ($proxy->flight != 0) {
+        return;
+    }
+
+    foreach my $message (@{$proxy->message_list}) {
+        if ($message->mt == TLSProxy::Message::MT_CLIENT_HELLO) {
+            $message->delete_extension(TLSProxy::Message::EXT_RENEGOTIATE);
+            $message->repack();
+        }
+    }
+}
+
 sub sigalgs_filter
 {
     my $proxy = shift;
diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t
index 37fba871e9..5218d5ff94 100644
--- a/test/recipes/70-test_sslextension.t
+++ b/test/recipes/70-test_sslextension.t
@@ -206,6 +206,7 @@ SKIP: {
     #Test 3: Sending a zero length extension block should pass
     $proxy->clear();
     $proxy->filter(\&extension_filter);
+    $proxy->cipherc("DEFAULT:\@SECLEVEL=0");
     $proxy->ciphers("AES128-SHA:\@SECLEVEL=0");
     $proxy->clientflags("-no_tls1_3");
     $proxy->start();
diff --git a/test/recipes/70-test_sslmessages.t b/test/recipes/70-test_sslmessages.t
index 0afb700679..cad0147ab5 100644
--- a/test/recipes/70-test_sslmessages.t
+++ b/test/recipes/70-test_sslmessages.t
@@ -128,7 +128,7 @@ my $proxy = TLSProxy::Proxy->new(
         checkhandshake::DEFAULT_EXTENSIONS],
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_RENEGOTIATE,
         TLSProxy::Message::CLIENT,
-        checkhandshake::RENEGOTIATE_CLI_EXTENSION],
+        checkhandshake::DEFAULT_EXTENSIONS],
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_NPN,
         TLSProxy::Message::CLIENT,
         checkhandshake::NPN_CLI_EXTENSION],
diff --git a/test/recipes/70-test_tls13certcomp.t b/test/recipes/70-test_tls13certcomp.t
index bc960c8b37..e2d65bd87c 100644
--- a/test/recipes/70-test_tls13certcomp.t
+++ b/test/recipes/70-test_tls13certcomp.t
@@ -109,6 +109,9 @@ plan skip_all => "$test_name needs compression and algorithms enabled"
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_COMPRESS_CERTIFICATE,
         TLSProxy::Message::CLIENT,
         checkhandshake::CERT_COMP_CLI_EXTENSION],
+    [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_RENEGOTIATE,
+        TLSProxy::Message::CLIENT,
+        checkhandshake::DEFAULT_EXTENSIONS],
 
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
         TLSProxy::Message::SERVER,
diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t
index 1f45edc7b7..738f2dcf7c 100644
--- a/test/recipes/70-test_tls13kexmodes.t
+++ b/test/recipes/70-test_tls13kexmodes.t
@@ -102,6 +102,9 @@ plan skip_all => "$test_name needs EC enabled"
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK,
         TLSProxy::Message::CLIENT,
         checkhandshake::PSK_CLI_EXTENSION],
+    [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_RENEGOTIATE,
+        TLSProxy::Message::CLIENT,
+        checkhandshake::DEFAULT_EXTENSIONS],
 
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
         TLSProxy::Message::SERVER,
@@ -152,6 +155,9 @@ plan skip_all => "$test_name needs EC enabled"
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK,
         TLSProxy::Message::CLIENT,
         checkhandshake::PSK_CLI_EXTENSION],
+    [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_RENEGOTIATE,
+        TLSProxy::Message::CLIENT,
+        checkhandshake::DEFAULT_EXTENSIONS],
 
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
         TLSProxy::Message::SERVER,
diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t
index f579cd3c9f..f8b5bf9663 100644
--- a/test/recipes/70-test_tls13messages.t
+++ b/test/recipes/70-test_tls13messages.t
@@ -105,6 +105,9 @@ plan skip_all => "$test_name needs EC enabled"
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_POST_HANDSHAKE_AUTH,
         TLSProxy::Message::CLIENT,
         checkhandshake::POST_HANDSHAKE_AUTH_CLI_EXTENSION],
+    [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_RENEGOTIATE,
+        TLSProxy::Message::CLIENT,
+        checkhandshake::DEFAULT_EXTENSIONS],
 
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
         TLSProxy::Message::SERVER,
@@ -158,6 +161,9 @@ plan skip_all => "$test_name needs EC enabled"
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_POST_HANDSHAKE_AUTH,
         TLSProxy::Message::CLIENT,
         checkhandshake::POST_HANDSHAKE_AUTH_CLI_EXTENSION],
+    [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_RENEGOTIATE,
+        TLSProxy::Message::CLIENT,
+        checkhandshake::DEFAULT_EXTENSIONS],
 
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
         TLSProxy::Message::SERVER,
diff --git a/test/sslapitest.c b/test/sslapitest.c
index 0b2d7b5e6d..ce8f642802 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -713,14 +713,14 @@ static int full_client_hello_callback(SSL *s, int *al, void *arg)
     int *ctr = arg;
     const unsigned char *p;
     int *exts;
-    /* We only configure two ciphers, but the SCSV is added automatically. */
 #ifdef OPENSSL_NO_EC
-    const unsigned char expected_ciphers[] = {0x00, 0x9d, 0x00, 0xff};
+    const unsigned char expected_ciphers[] = {0x00, 0x9d};
 #else
     const unsigned char expected_ciphers[] = {0x00, 0x9d, 0xc0,
-                                              0x2c, 0x00, 0xff};
+                                              0x2c};
 #endif
     const int expected_extensions[] = {
+                                       65281,
 #ifndef OPENSSL_NO_EC
                                        11, 10,
 #endif