From a49dcaedc471e79508b3e5674c538ca90f5c4e2e Mon Sep 17 00:00:00 2001
From: jsing <>
Date: Sun, 31 May 2020 16:36:35 +0000
Subject: Correct downgrade sentinels when a version pinned method is in use.

Previously only the enabled protocol versions were considered, however we
also have to consider the method in use which may be version pinned.

Found the hard way by danj@ with haproxy and force-tlsv12.

ok beck@ inoguchi@ tb@
---
 src/lib/libssl/ssl_clnt.c     |  4 ++--
 src/lib/libssl/ssl_locl.h     |  3 ++-
 src/lib/libssl/ssl_srvr.c     |  6 +++---
 src/lib/libssl/ssl_versions.c | 34 +++++++++++++++++++++++++++++++++-
 4 files changed, 40 insertions(+), 7 deletions(-)

(limited to 'src/lib')

diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index fb29e4f5f6..4d003466c4 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.67 2020/05/19 16:35:20 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.68 2020/05/31 16:36:35 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -873,7 +873,7 @@ ssl3_get_server_hello(SSL *s)
 	    sizeof(s->s3->server_random), NULL))
 		goto err;
 
-	if (!SSL_IS_DTLS(s) && !ssl_enabled_version_range(s, NULL, &max_version))
+	if (!ssl_downgrade_max_version(s, &max_version))
 		goto err;
 	if (!SSL_IS_DTLS(s) && max_version >= TLS1_2_VERSION &&
 	    s->version < max_version) {
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index e7e3e56154..03c2c227ed 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.277 2020/05/29 18:00:10 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.278 2020/05/31 16:36:35 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1095,6 +1095,7 @@ int ssl_version_set_min(const SSL_METHOD *meth, uint16_t ver, uint16_t max_ver,
 int ssl_version_set_max(const SSL_METHOD *meth, uint16_t ver, uint16_t min_ver,
     uint16_t *out_ver);
 uint16_t ssl_max_server_version(SSL *s);
+int ssl_downgrade_max_version(SSL *s, uint16_t *max_ver);
 int ssl_cipher_is_permitted(const SSL_CIPHER *cipher, uint16_t min_ver,
     uint16_t max_ver);
 
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index 6a90ad17eb..fac24f4d00 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.76 2020/05/19 16:35:20 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.77 2020/05/31 16:36:35 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -852,6 +852,8 @@ ssl3_get_client_hello(SSL *s)
 	 * Use version from inside client hello, not from record header.
 	 * (may differ: see RFC 2246, Appendix E, second paragraph)
 	 */
+	if (!ssl_downgrade_max_version(s, &max_version))
+		goto err;
 	if (ssl_max_shared_version(s, client_version, &shared_version) != 1) {
 		SSLerror(s, SSL_R_WRONG_VERSION_NUMBER);
 		if ((s->client_version >> 8) == SSL3_VERSION_MAJOR &&
@@ -1047,8 +1049,6 @@ ssl3_get_client_hello(SSL *s)
 	 */
 	arc4random_buf(s->s3->server_random, SSL3_RANDOM_SIZE);
 
-	if (!SSL_IS_DTLS(s) && !ssl_enabled_version_range(s, NULL, &max_version))
-		goto err;
 	if (!SSL_IS_DTLS(s) && max_version >= TLS1_2_VERSION &&
 	    s->version < max_version) {
 		/*
diff --git a/src/lib/libssl/ssl_versions.c b/src/lib/libssl/ssl_versions.c
index 2b5e94e5b8..03eb41582a 100644
--- a/src/lib/libssl/ssl_versions.c
+++ b/src/lib/libssl/ssl_versions.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_versions.c,v 1.4 2018/11/06 01:40:23 jsing Exp $ */
+/* $OpenBSD: ssl_versions.c,v 1.5 2020/05/31 16:36:35 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  *
@@ -223,3 +223,35 @@ ssl_max_server_version(SSL *s)
 
 	return (max_version);
 }
+
+int
+ssl_downgrade_max_version(SSL *s, uint16_t *max_ver)
+{
+	uint16_t min_version, max_version;
+
+	/*
+	 * The downgrade maximum version is based on the versions that are
+	 * enabled, however we also have to then limit to the versions
+	 * supported by the method. The SSL method will be changed during
+	 * version negotiation and when switching from the new stack to
+	 * the legacy context, as such we want to use the method from the
+	 * context.
+	 */
+
+	if (SSL_IS_DTLS(s)) {
+		*max_ver = DTLS1_VERSION;
+		return 1;
+	}
+
+	if (!ssl_enabled_version_range(s, &min_version, &max_version))
+		return 0;
+
+	if (!ssl_clamp_version_range(&min_version, &max_version,
+	    s->ctx->method->internal->min_version,
+	    s->ctx->method->internal->max_version))
+		return 0;
+
+	*max_ver = max_version;
+
+	return 1;
+}
-- 
cgit v1.2.3-55-g6feb