openssl
/
openssl
mirror of https://github.com/openssl/openssl
1
0
Fork 0
Code Issues Releases Wiki Activity

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)
This commit is contained in:
Tim Perry 2024-04-16 15:40:21 +02:00 committed by Matt Caswell
parent 6ee369cd6e
commit 972ee925b1
10 changed files with 153 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
CHANGES.md
View File

@ -41,6 +41,12 @@ OpenSSL 3.4
* Tomas Mraz*
* Use an empty renegotiate extension in TLS client hellos instead of
the empty renegotiation SCSV, for all connections with a minimum TLS
version > 1.0.
*Tim Perry*
OpenSSL 3.3
-----------

33
ssl/statem/extensions_clnt.c
View File

@ -16,10 +16,37 @@ EXT_RETURN tls_construct_ctos_renegotiate(SSL_CONNECTION *s, WPACKET *pkt,
unsigned int context, X509 *x,
size_t chainidx)
{
/* Add RI if renegotiating */
if (!s->renegotiate)
return EXT_RETURN_NOT_SENT;
if (!s->renegotiate) {
/* If not renegotiating, send an empty RI extension to indicate support */
#if DTLS_MAX_VERSION_INTERNAL != DTLS1_2_VERSION
# error Internal DTLS version error
#endif
if (!SSL_CONNECTION_IS_DTLS(s)
&& (s->min_proto_version >= TLS1_3_VERSION
|| (ssl_security(s, SSL_SECOP_VERSION, 0, TLS1_VERSION, NULL)
&& s->min_proto_version <= TLS1_VERSION))) {
/*
* For TLS <= 1.0 SCSV is used instead, and for TLS 1.3 this
* extension isn't used at all.
*/
return EXT_RETURN_NOT_SENT;
}
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_renegotiate)
|| !WPACKET_start_sub_packet_u16(pkt)
|| !WPACKET_put_bytes_u8(pkt, 0)
|| !WPACKET_close(pkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return EXT_RETURN_FAIL;
}
return EXT_RETURN_SENT;
}
/* Add a complete RI extension if renegotiating */
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_renegotiate)
|| !WPACKET_start_sub_packet_u16(pkt)
|| !WPACKET_sub_memcpy_u8(pkt, s->s3.previous_client_finished,

5
ssl/statem/statem_clnt.c
View File

@ -4064,8 +4064,9 @@ int ssl_cipher_list_to_bytes(SSL_CONNECTION *s, STACK_OF(SSL_CIPHER) *sk,
int i;
size_t totlen = 0, len, maxlen, maxverok = 0;
int empty_reneg_info_scsv = !s->renegotiate
&& (SSL_CONNECTION_IS_DTLS(s)
|| s->min_proto_version < TLS1_3_VERSION);
&& !SSL_CONNECTION_IS_DTLS(s)
&& ssl_security(s, SSL_SECOP_VERSION, 0, TLS1_VERSION, NULL)
&& s->min_proto_version <= TLS1_VERSION;
SSL *ssl = SSL_CONNECTION_GET_SSL(s);
/* Set disabled masks for this session */

101
test/recipes/70-test_renegotiation.t
View File

@ -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,19 +43,35 @@ $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);
$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;

1
test/recipes/70-test_sslextension.t
View File

@ -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();

2
test/recipes/70-test_sslmessages.t
View File

@ -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],

3
test/recipes/70-test_tls13certcomp.t
View File

@ -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,

6
test/recipes/70-test_tls13kexmodes.t
View File

@ -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,

6
test/recipes/70-test_tls13messages.t
View File

@ -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,

6
test/sslapitest.c
View File

@ -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