[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Wed Aug 22 14:19:14 UTC 2018
The branch master has been updated
via 2fe3e2b68272e803a6e35259a49919d57205418b (commit)
via 5627f9f21764af7eac2af2fb8ec867cd65ca8949 (commit)
via 3e7cb13dff37795f022a1bedc5951130099a0fc6 (commit)
via b5b993b2295be98e23fa8bb570b2c38c5bf8aaf3 (commit)
from bc420ebea2c5ad813779ac3395f1c5a1083d49c5 (commit)
- Log -----------------------------------------------------------------
commit 2fe3e2b68272e803a6e35259a49919d57205418b
Author: Matt Caswell <matt at openssl.org>
Date: Wed Aug 22 14:10:48 2018 +0100
Fix BoringSSL external test failures
We recently turned on the TLSv1.3 downgrade sentinels by default.
Unfortunately we are using a very old version of the BoringSSL test
runner which uses an old draft implementation of TLSv1.3 that also
uses the downgrade sentinels by default. The two implementations do
not play well together and were causing spurious test failures. Until
such time as we update the BoringSSL test runner we disable the failing
tests:
SendFallbackSCSV
In this test the client is OpenSSL and the server is the boring test runner.
The client and server fail to negotiate TLSv1.3 because the test runner is
using an old draft TLSv1.3 version. The server does however add the
TLSv1.3->TLSv1.2 downgrade sentinel in the ServerHello random. Since we
recently turned on checking of the downgrade sentinels on the client side
this causes the connection to fail.
VersionNegotiationExtension-TLS11
In this test the test runner is the client and OpenSSL is the server. The
test modifies the supported_versions extension sent by the client to only
include TLSv1.1 (and some other spurious versions), even though the client
does actually support TLSv1.2. The server successfully selects TLSv1.1, but
adds the TLSv1.3->TLSv1.1 downgrade sentinel. This behaviour was recently
switched on by default. The test runner then checks the downgrade sentinel
and aborts the connection because it knows that it really supports TLSv1.2.
VersionNegotiationExtension-TLS1
VersionNegotiationExtension-SSL3
The same as VersionNegotiationExtension-TLS11 but for TLSv1 and SSLv3.
ConflictingVersionNegotiation
In this test the client is the test runner, and OpenSSL is the server. The
client offers TLSv1.2 in ClientHello.version, but also adds a
supported_versions extension that only offers TLSv1.1. The
supported_versions extension takes precedence and the server (correctly)
selects TLSv1.1. However it also adds the TLSv1.3->TLSv1.1 downgrade
sentinel. On the client side it knows it actually offered TLSv1.2 and so the
downgrade sentinel check fails.
[extended tests]
Reviewed-by: Viktor Dukhovni <viktor at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
commit 5627f9f21764af7eac2af2fb8ec867cd65ca8949
Author: Matt Caswell <matt at openssl.org>
Date: Mon Aug 20 18:05:28 2018 +0100
Don't detect a downgrade where the server has a protocol version hole
Reviewed-by: Viktor Dukhovni <viktor at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
commit 3e7cb13dff37795f022a1bedc5951130099a0fc6
Author: Matt Caswell <matt at openssl.org>
Date: Mon Aug 20 17:44:58 2018 +0100
Test that a client protocol "hole" doesn't get detected as a downgrade
Reviewed-by: Viktor Dukhovni <viktor at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
commit b5b993b2295be98e23fa8bb570b2c38c5bf8aaf3
Author: Matt Caswell <matt at openssl.org>
Date: Mon Aug 20 15:12:39 2018 +0100
Use the same min-max version range on the client consistently
We need to ensure that the min-max version range we use when constructing
the ClientHello is the same range we use when we validate the version
selected by the ServerHello. Otherwise this may appear as a fallback or
downgrade.
Fixes #6964
Reviewed-by: Viktor Dukhovni <viktor at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
-----------------------------------------------------------------------
Summary of changes:
ssl/ssl_locl.h | 2 +-
ssl/statem/extensions.c | 2 +-
ssl/statem/extensions_clnt.c | 2 +-
ssl/statem/statem_lib.c | 147 ++++++++++++++++++++--------------
ssl/t1_lib.c | 2 +-
test/ossl_shim/ossl_config.json | 7 +-
test/recipes/70-test_tls13downgrade.t | 19 ++++-
7 files changed, 113 insertions(+), 68 deletions(-)
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 362ae1c..e8819e7 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2384,7 +2384,7 @@ __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello,
__owur int ssl_choose_client_version(SSL *s, int version,
RAW_EXTENSION *extensions);
__owur int ssl_get_min_max_version(const SSL *s, int *min_version,
- int *max_version);
+ int *max_version, int *real_max);
__owur long tls1_default_timeout(void);
__owur int dtls1_do_write(SSL *s, int type);
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 12c712e..307e6b9 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -810,7 +810,7 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
}
if ((context & SSL_EXT_CLIENT_HELLO) != 0) {
- reason = ssl_get_min_max_version(s, &min_version, &max_version);
+ reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);
if (reason != 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_EXTENSIONS,
reason);
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 2d5b60a..4b5e6fe 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -507,7 +507,7 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt,
{
int currv, min_version, max_version, reason;
- reason = ssl_get_min_max_version(s, &min_version, &max_version);
+ reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);
if (reason != 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR,
SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 38121b7..3961c14 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -105,7 +105,7 @@ int tls_setup_handshake(SSL *s)
* enabled. For clients we do this check during construction of the
* ClientHello.
*/
- if (ssl_get_min_max_version(s, &ver_min, &ver_max) != 0) {
+ if (ssl_get_min_max_version(s, &ver_min, &ver_max, NULL) != 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_SETUP_HANDSHAKE,
ERR_R_INTERNAL_ERROR);
return 0;
@@ -1665,9 +1665,16 @@ static void check_for_downgrade(SSL *s, int vers, DOWNGRADE *dgrd)
if (vers == TLS1_2_VERSION
&& ssl_version_supported(s, TLS1_3_VERSION, NULL)) {
*dgrd = DOWNGRADE_TO_1_2;
- } else if (!SSL_IS_DTLS(s) && vers < TLS1_2_VERSION
- && (ssl_version_supported(s, TLS1_2_VERSION, NULL)
- || ssl_version_supported(s, TLS1_3_VERSION, NULL))) {
+ } else if (!SSL_IS_DTLS(s)
+ && vers < TLS1_2_VERSION
+ /*
+ * We need to ensure that a server that disables TLSv1.2
+ * (creating a hole between TLSv1.3 and TLSv1.1) can still
+ * complete handshakes with clients that support TLSv1.2 and
+ * below. Therefore we do not enable the sentinel if TLSv1.3 is
+ * enabled and TLSv1.2 is not.
+ */
+ && ssl_version_supported(s, TLS1_2_VERSION, NULL)) {
*dgrd = DOWNGRADE_TO_1_1;
} else {
*dgrd = DOWNGRADE_NONE;
@@ -1835,8 +1842,7 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
{
const version_info *vent;
const version_info *table;
- int highver = 0;
- int origv;
+ int ret, ver_min, ver_max, real_max, origv;
origv = s->version;
s->version = version;
@@ -1883,65 +1889,62 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
break;
}
- for (vent = table; vent->version != 0; ++vent) {
- const SSL_METHOD *method;
- int err;
-
- if (vent->cmeth == NULL)
- continue;
-
- if (highver != 0 && s->version != vent->version)
- continue;
-
- if (highver == 0 && (s->mode & SSL_MODE_SEND_FALLBACK_SCSV) != 0)
- highver = vent->version;
+ ret = ssl_get_min_max_version(s, &ver_min, &ver_max, &real_max);
+ if (ret != 0) {
+ s->version = origv;
+ SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
+ SSL_F_SSL_CHOOSE_CLIENT_VERSION, ret);
+ return 0;
+ }
+ if (SSL_IS_DTLS(s) ? DTLS_VERSION_LT(s->version, ver_min)
+ : s->version < ver_min) {
+ s->version = origv;
+ SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
+ SSL_F_SSL_CHOOSE_CLIENT_VERSION, SSL_R_UNSUPPORTED_PROTOCOL);
+ return 0;
+ } else if (SSL_IS_DTLS(s) ? DTLS_VERSION_GT(s->version, ver_max)
+ : s->version > ver_max) {
+ s->version = origv;
+ SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
+ SSL_F_SSL_CHOOSE_CLIENT_VERSION, SSL_R_UNSUPPORTED_PROTOCOL);
+ return 0;
+ }
- method = vent->cmeth();
- err = ssl_method_error(s, method);
- if (err != 0) {
- if (s->version == vent->version) {
- s->version = origv;
- SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
- SSL_F_SSL_CHOOSE_CLIENT_VERSION, err);
- return 0;
- }
+ if ((s->mode & SSL_MODE_SEND_FALLBACK_SCSV) == 0)
+ real_max = ver_max;
- continue;
+ /* Check for downgrades */
+ if (s->version == TLS1_2_VERSION && real_max > s->version) {
+ if (memcmp(tls12downgrade,
+ s->s3->server_random + SSL3_RANDOM_SIZE
+ - sizeof(tls12downgrade),
+ sizeof(tls12downgrade)) == 0) {
+ s->version = origv;
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+ SSL_F_SSL_CHOOSE_CLIENT_VERSION,
+ SSL_R_INAPPROPRIATE_FALLBACK);
+ return 0;
+ }
+ } else if (!SSL_IS_DTLS(s)
+ && s->version < TLS1_2_VERSION
+ && real_max > s->version) {
+ if (memcmp(tls11downgrade,
+ s->s3->server_random + SSL3_RANDOM_SIZE
+ - sizeof(tls11downgrade),
+ sizeof(tls11downgrade)) == 0) {
+ s->version = origv;
+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+ SSL_F_SSL_CHOOSE_CLIENT_VERSION,
+ SSL_R_INAPPROPRIATE_FALLBACK);
+ return 0;
}
- if (highver == 0)
- highver = vent->version;
+ }
- if (s->version != vent->version)
+ for (vent = table; vent->version != 0; ++vent) {
+ if (vent->cmeth == NULL || s->version != vent->version)
continue;
- /* Check for downgrades */
- if (s->version == TLS1_2_VERSION && highver > s->version) {
- if (memcmp(tls12downgrade,
- s->s3->server_random + SSL3_RANDOM_SIZE
- - sizeof(tls12downgrade),
- sizeof(tls12downgrade)) == 0) {
- s->version = origv;
- SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
- SSL_F_SSL_CHOOSE_CLIENT_VERSION,
- SSL_R_INAPPROPRIATE_FALLBACK);
- return 0;
- }
- } else if (!SSL_IS_DTLS(s)
- && s->version < TLS1_2_VERSION
- && highver > s->version) {
- if (memcmp(tls11downgrade,
- s->s3->server_random + SSL3_RANDOM_SIZE
- - sizeof(tls11downgrade),
- sizeof(tls11downgrade)) == 0) {
- s->version = origv;
- SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
- SSL_F_SSL_CHOOSE_CLIENT_VERSION,
- SSL_R_INAPPROPRIATE_FALLBACK);
- return 0;
- }
- }
-
- s->method = method;
+ s->method = vent->cmeth();
return 1;
}
@@ -1956,6 +1959,9 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
* @s: The SSL connection
* @min_version: The minimum supported version
* @max_version: The maximum supported version
+ * @real_max: The highest version below the lowest compile time version hole
+ * where that hole lies above at least one run-time enabled
+ * protocol.
*
* Work out what version we should be using for the initial ClientHello if the
* version is initially (D)TLS_ANY_VERSION. We apply any explicit SSL_OP_NO_xxx
@@ -1970,9 +1976,10 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
* Returns 0 on success or an SSL error reason number on failure. On failure
* min_version and max_version will also be set to 0.
*/
-int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version)
+int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version,
+ int *real_max)
{
- int version;
+ int version, tmp_real_max;
int hole;
const SSL_METHOD *single = NULL;
const SSL_METHOD *method;
@@ -1989,6 +1996,12 @@ int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version)
* ssl_method_error(s, s->method)
*/
*min_version = *max_version = s->version;
+ /*
+ * Providing a real_max only makes sense where we're using a version
+ * flexible method.
+ */
+ if (!ossl_assert(real_max == NULL))
+ return ERR_R_INTERNAL_ERROR;
return 0;
case TLS_ANY_VERSION:
table = tls_version_table;
@@ -2021,6 +2034,9 @@ int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version)
*/
*min_version = version = 0;
hole = 1;
+ if (real_max != NULL)
+ *real_max = 0;
+ tmp_real_max = 0;
for (vent = table; vent->version != 0; ++vent) {
/*
* A table entry with a NULL client method is still a hole in the
@@ -2028,15 +2044,22 @@ int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version)
*/
if (vent->cmeth == NULL) {
hole = 1;
+ tmp_real_max = 0;
continue;
}
method = vent->cmeth();
+
+ if (hole == 1 && tmp_real_max == 0)
+ tmp_real_max = vent->version;
+
if (ssl_method_error(s, method) != 0) {
hole = 1;
} else if (!hole) {
single = NULL;
*min_version = method->version;
} else {
+ if (real_max != NULL && tmp_real_max != 0)
+ *real_max = tmp_real_max;
version = (single = method)->version;
*min_version = version;
hole = 0;
@@ -2071,7 +2094,7 @@ int ssl_set_client_hello_version(SSL *s)
if (!SSL_IS_FIRST_HANDSHAKE(s))
return 0;
- ret = ssl_get_min_max_version(s, &ver_min, &ver_max);
+ ret = ssl_get_min_max_version(s, &ver_min, &ver_max, NULL);
if (ret != 0)
return ret;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index df27ba6..ca05a3a 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1103,7 +1103,7 @@ int ssl_set_client_disabled(SSL *s)
s->s3->tmp.mask_k = 0;
ssl_set_sig_mask(&s->s3->tmp.mask_a, s, SSL_SECOP_SIGALG_MASK);
if (ssl_get_min_max_version(s, &s->s3->tmp.min_ver,
- &s->s3->tmp.max_ver) != 0)
+ &s->s3->tmp.max_ver, NULL) != 0)
return 0;
#ifndef OPENSSL_NO_PSK
/* with PSK there must be client callback set */
diff --git a/test/ossl_shim/ossl_config.json b/test/ossl_shim/ossl_config.json
index 0643d71..1e57499 100644
--- a/test/ossl_shim/ossl_config.json
+++ b/test/ossl_shim/ossl_config.json
@@ -248,7 +248,12 @@
"DTLS-Retransmit-Server-12":"Test failure - reason unknown",
"DTLS-Retransmit-Fudge":"Test failure - reason unknown",
"DTLS-Retransmit-Fragmented":"Test failure - reason unknown",
- "TrailingMessageData-ClientHello-DTLS":"Test failure - reason unknown"
+ "TrailingMessageData-ClientHello-DTLS":"Test failure - reason unknown",
+ "SendFallbackSCSV":"Current runner version uses old draft TLSv1.3",
+ "VersionNegotiationExtension-TLS11":"Current runner version uses old draft TLSv1.3",
+ "VersionNegotiationExtension-TLS1":"Current runner version uses old draft TLSv1.3",
+ "VersionNegotiationExtension-SSL3":"Current runner version uses old draft TLSv1.3",
+ "ConflictingVersionNegotiation":"Current runner version uses old draft TLSv1.3"
},
"ErrorMap" : {
":UNEXPECTED_MESSAGE:":"unexpected message",
diff --git a/test/recipes/70-test_tls13downgrade.t b/test/recipes/70-test_tls13downgrade.t
index f7c8812..bdb360a 100644
--- a/test/recipes/70-test_tls13downgrade.t
+++ b/test/recipes/70-test_tls13downgrade.t
@@ -45,7 +45,7 @@ use constant {
$proxy->filter(\&downgrade_filter);
my $testtype = DOWNGRADE_TO_TLS_1_2;
$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 4;
+plan tests => 6;
ok(TLSProxy::Message->fail(), "Downgrade TLSv1.3 to TLSv1.2");
#Test 2: Downgrade from TLSv1.3 to TLSv1.1
@@ -73,6 +73,23 @@ ok(TLSProxy::Message->fail()
&& $alert->description() == TLSProxy::Message::AL_DESC_ILLEGAL_PARAMETER,
"Fallback from TLSv1.3");
+SKIP: {
+ skip "TLSv1.1 disabled", 2 if disabled("tls1_1");
+ #Test 5: A client side protocol "hole" should not be detected as a downgrade
+ $proxy->clear();
+ $proxy->filter(undef);
+ $proxy->clientflags("-no_tls1_2");
+ $proxy->start();
+ ok(TLSProxy::Message->success(), "TLSv1.2 client-side protocol hole");
+
+ #Test 6: A server side protocol "hole" should not be detected as a downgrade
+ $proxy->clear();
+ $proxy->filter(undef);
+ $proxy->serverflags("-no_tls1_2");
+ $proxy->start();
+ ok(TLSProxy::Message->success(), "TLSv1.2 server-side protocol hole");
+}
+
sub downgrade_filter
{
my $proxy = shift;
More information about the openssl-commits
mailing list