Pedro Monreal Gonzalez
aa970d108e
- Security fix: [bsc#1227138, CVE-2024-5535] * SSL_select_next_proto buffer overread * Add openssl-CVE-2024-5535.patch - Build with enabled sm2 and sm4 support [bsc#1222899] OBS-URL: https://build.opensuse.org/request/show/1188975 OBS-URL: https://build.opensuse.org/package/show/security:tls/openssl-3?expand=0&rev=106
327 lines
13 KiB
Diff
327 lines
13 KiB
Diff
From 4ada436a1946cbb24db5ab4ca082b69c1bc10f37 Mon Sep 17 00:00:00 2001
|
|
From: Matt Caswell <matt@openssl.org>
|
|
Date: Fri, 31 May 2024 11:14:33 +0100
|
|
Subject: [PATCH] Fix SSL_select_next_proto
|
|
|
|
Ensure that the provided client list is non-NULL and starts with a valid
|
|
entry. When called from the ALPN callback the client list should already
|
|
have been validated by OpenSSL so this should not cause a problem. When
|
|
called from the NPN callback the client list is locally configured and
|
|
will not have already been validated. Therefore SSL_select_next_proto
|
|
should not assume that it is correctly formatted.
|
|
|
|
We implement stricter checking of the client protocol list. We also do the
|
|
same for the server list while we are about it.
|
|
|
|
CVE-2024-5535
|
|
|
|
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
|
(Merged from https://github.com/openssl/openssl/pull/24718)
|
|
---
|
|
ssl/ssl_lib.c | 63 ++++++++++++++++++++++++++++++++-------------------
|
|
1 file changed, 40 insertions(+), 23 deletions(-)
|
|
|
|
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
|
|
index 5493d9b9c7..f218dcf1db 100644
|
|
--- a/ssl/ssl_lib.c
|
|
+++ b/ssl/ssl_lib.c
|
|
@@ -2953,37 +2953,54 @@ int SSL_select_next_proto(unsigned char **out, unsigned char *outlen,
|
|
unsigned int server_len,
|
|
const unsigned char *client, unsigned int client_len)
|
|
{
|
|
- unsigned int i, j;
|
|
- const unsigned char *result;
|
|
- int status = OPENSSL_NPN_UNSUPPORTED;
|
|
+ PACKET cpkt, csubpkt, spkt, ssubpkt;
|
|
+
|
|
+ if (!PACKET_buf_init(&cpkt, client, client_len)
|
|
+ || !PACKET_get_length_prefixed_1(&cpkt, &csubpkt)
|
|
+ || PACKET_remaining(&csubpkt) == 0) {
|
|
+ *out = NULL;
|
|
+ *outlen = 0;
|
|
+ return OPENSSL_NPN_NO_OVERLAP;
|
|
+ }
|
|
+
|
|
+ /*
|
|
+ * Set the default opportunistic protocol. Will be overwritten if we find
|
|
+ * a match.
|
|
+ */
|
|
+ *out = (unsigned char *)PACKET_data(&csubpkt);
|
|
+ *outlen = (unsigned char)PACKET_remaining(&csubpkt);
|
|
|
|
/*
|
|
* For each protocol in server preference order, see if we support it.
|
|
*/
|
|
- for (i = 0; i < server_len;) {
|
|
- for (j = 0; j < client_len;) {
|
|
- if (server[i] == client[j] &&
|
|
- memcmp(&server[i + 1], &client[j + 1], server[i]) == 0) {
|
|
- /* We found a match */
|
|
- result = &server[i];
|
|
- status = OPENSSL_NPN_NEGOTIATED;
|
|
- goto found;
|
|
+ if (PACKET_buf_init(&spkt, server, server_len)) {
|
|
+ while (PACKET_get_length_prefixed_1(&spkt, &ssubpkt)) {
|
|
+ if (PACKET_remaining(&ssubpkt) == 0)
|
|
+ continue; /* Invalid - ignore it */
|
|
+ if (PACKET_buf_init(&cpkt, client, client_len)) {
|
|
+ while (PACKET_get_length_prefixed_1(&cpkt, &csubpkt)) {
|
|
+ if (PACKET_equal(&csubpkt, PACKET_data(&ssubpkt),
|
|
+ PACKET_remaining(&ssubpkt))) {
|
|
+ /* We found a match */
|
|
+ *out = (unsigned char *)PACKET_data(&ssubpkt);
|
|
+ *outlen = (unsigned char)PACKET_remaining(&ssubpkt);
|
|
+ return OPENSSL_NPN_NEGOTIATED;
|
|
+ }
|
|
+ }
|
|
+ /* Ignore spurious trailing bytes in the client list */
|
|
+ } else {
|
|
+ /* This should never happen */
|
|
+ return OPENSSL_NPN_NO_OVERLAP;
|
|
}
|
|
- j += client[j];
|
|
- j++;
|
|
}
|
|
- i += server[i];
|
|
- i++;
|
|
+ /* Ignore spurious trailing bytes in the server list */
|
|
}
|
|
|
|
- /* There's no overlap between our protocols and the server's list. */
|
|
- result = client;
|
|
- status = OPENSSL_NPN_NO_OVERLAP;
|
|
-
|
|
- found:
|
|
- *out = (unsigned char *)result + 1;
|
|
- *outlen = result[0];
|
|
- return status;
|
|
+ /*
|
|
+ * There's no overlap between our protocols and the server's list. We use
|
|
+ * the default opportunistic protocol selected earlier
|
|
+ */
|
|
+ return OPENSSL_NPN_NO_OVERLAP;
|
|
}
|
|
|
|
#ifndef OPENSSL_NO_NEXTPROTONEG
|
|
--
|
|
2.45.2
|
|
|
|
From 4279c89a726025c758db3dafb263b17e52211304 Mon Sep 17 00:00:00 2001
|
|
From: Matt Caswell <matt@openssl.org>
|
|
Date: Fri, 31 May 2024 11:18:27 +0100
|
|
Subject: [PATCH] More correctly handle a selected_len of 0 when
|
|
processing NPN
|
|
|
|
In the case where the NPN callback returns with SSL_TLEXT_ERR_OK, but
|
|
the selected_len is 0 we should fail. Previously this would fail with an
|
|
internal_error alert because calling OPENSSL_malloc(selected_len) will
|
|
return NULL when selected_len is 0. We make this error detection more
|
|
explicit and return a handshake failure alert.
|
|
|
|
Follow on from CVE-2024-5535
|
|
|
|
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
|
(Merged from https://github.com/openssl/openssl/pull/24718)
|
|
---
|
|
ssl/statem/extensions_clnt.c | 3 ++-
|
|
1 file changed, 2 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
|
|
index 842be0722b..a07dc62e9a 100644
|
|
--- a/ssl/statem/extensions_clnt.c
|
|
+++ b/ssl/statem/extensions_clnt.c
|
|
@@ -1536,7 +1536,8 @@ int tls_parse_stoc_npn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
|
|
PACKET_data(pkt),
|
|
PACKET_remaining(pkt),
|
|
s->ctx->ext.npn_select_cb_arg) !=
|
|
- SSL_TLSEXT_ERR_OK) {
|
|
+ SSL_TLSEXT_ERR_OK
|
|
+ || selected_len == 0) {
|
|
SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_R_BAD_EXTENSION);
|
|
return 0;
|
|
}
|
|
--
|
|
2.45.2
|
|
|
|
From 889ed19ba25abebd2690997acd6d4791cbe5c493 Mon Sep 17 00:00:00 2001
|
|
From: Matt Caswell <matt@openssl.org>
|
|
Date: Fri, 31 May 2024 11:46:38 +0100
|
|
Subject: [PATCH] Clarify the SSL_select_next_proto() documentation
|
|
|
|
We clarify the input preconditions and the expected behaviour in the event
|
|
of no overlap.
|
|
|
|
Follow on from CVE-2024-5535
|
|
|
|
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
|
(Merged from https://github.com/openssl/openssl/pull/24718)
|
|
---
|
|
doc/man3/SSL_CTX_set_alpn_select_cb.pod | 26 +++++++++++++++++--------
|
|
1 file changed, 18 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/doc/man3/SSL_CTX_set_alpn_select_cb.pod b/doc/man3/SSL_CTX_set_alpn_select_cb.pod
|
|
index 102e657851..a29557dd91 100644
|
|
--- a/doc/man3/SSL_CTX_set_alpn_select_cb.pod
|
|
+++ b/doc/man3/SSL_CTX_set_alpn_select_cb.pod
|
|
@@ -52,7 +52,8 @@ SSL_select_next_proto, SSL_get0_alpn_selected, SSL_get0_next_proto_negotiated
|
|
SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() are used by the client to
|
|
set the list of protocols available to be negotiated. The B<protos> must be in
|
|
protocol-list format, described below. The length of B<protos> is specified in
|
|
-B<protos_len>.
|
|
+B<protos_len>. Setting B<protos_len> to 0 clears any existing list of ALPN
|
|
+protocols and no ALPN extension will be sent to the server.
|
|
|
|
SSL_CTX_set_alpn_select_cb() sets the application callback B<cb> used by a
|
|
server to select which protocol to use for the incoming connection. When B<cb>
|
|
@@ -73,9 +74,16 @@ B<server_len> and B<client>, B<client_len> must be in the protocol-list format
|
|
described below. The first item in the B<server>, B<server_len> list that
|
|
matches an item in the B<client>, B<client_len> list is selected, and returned
|
|
in B<out>, B<outlen>. The B<out> value will point into either B<server> or
|
|
-B<client>, so it should be copied immediately. If no match is found, the first
|
|
-item in B<client>, B<client_len> is returned in B<out>, B<outlen>. This
|
|
-function can also be used in the NPN callback.
|
|
+B<client>, so it should be copied immediately. The client list must include at
|
|
+least one valid (nonempty) protocol entry in the list.
|
|
+
|
|
+The SSL_select_next_proto() helper function can be useful from either the ALPN
|
|
+callback or the NPN callback (described below). If no match is found, the first
|
|
+item in B<client>, B<client_len> is returned in B<out>, B<outlen> and
|
|
+B<OPENSSL_NPN_NO_OVERLAP> is returned. This can be useful when implementating
|
|
+the NPN callback. In the ALPN case, the value returned in B<out> and B<outlen>
|
|
+must be ignored if B<OPENSSL_NPN_NO_OVERLAP> has been returned from
|
|
+SSL_select_next_proto().
|
|
|
|
SSL_CTX_set_next_proto_select_cb() sets a callback B<cb> that is called when a
|
|
client needs to select a protocol from the server's provided list, and a
|
|
@@ -85,9 +93,10 @@ must be set to point to the selected protocol (which may be within B<in>).
|
|
The length of the protocol name must be written into B<outlen>. The
|
|
server's advertised protocols are provided in B<in> and B<inlen>. The
|
|
callback can assume that B<in> is syntactically valid. The client must
|
|
-select a protocol. It is fatal to the connection if this callback returns
|
|
-a value other than B<SSL_TLSEXT_ERR_OK>. The B<arg> parameter is the pointer
|
|
-set via SSL_CTX_set_next_proto_select_cb().
|
|
+select a protocol (although it may be an empty, zero length protocol). It is
|
|
+fatal to the connection if this callback returns a value other than
|
|
+B<SSL_TLSEXT_ERR_OK> or if the zero length protocol is selected. The B<arg>
|
|
+parameter is the pointer set via SSL_CTX_set_next_proto_select_cb().
|
|
|
|
SSL_CTX_set_next_protos_advertised_cb() sets a callback B<cb> that is called
|
|
when a TLS server needs a list of supported protocols for Next Protocol
|
|
@@ -149,7 +158,8 @@ A match was found and is returned in B<out>, B<outlen>.
|
|
=item OPENSSL_NPN_NO_OVERLAP
|
|
|
|
No match was found. The first item in B<client>, B<client_len> is returned in
|
|
-B<out>, B<outlen>.
|
|
+B<out>, B<outlen> (or B<NULL> and 0 in the case where the first entry in
|
|
+B<client> is invalid).
|
|
|
|
=back
|
|
|
|
--
|
|
2.45.2
|
|
|
|
From 087501b4f572825e27ca8cc2c5874fcf6fd47cf7 Mon Sep 17 00:00:00 2001
|
|
From: Matt Caswell <matt@openssl.org>
|
|
Date: Fri, 21 Jun 2024 10:41:55 +0100
|
|
Subject: [PATCH] Correct return values for
|
|
tls_construct_stoc_next_proto_neg
|
|
|
|
Return EXT_RETURN_NOT_SENT in the event that we don't send the extension,
|
|
rather than EXT_RETURN_SENT. This actually makes no difference at all to
|
|
the current control flow since this return value is ignored in this case
|
|
anyway. But lets make it correct anyway.
|
|
|
|
Follow on from CVE-2024-5535
|
|
|
|
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
|
(Merged from https://github.com/openssl/openssl/pull/24718)
|
|
---
|
|
ssl/statem/extensions_srvr.c | 3 ++-
|
|
1 file changed, 2 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
|
|
index 4ea085e1a1..2da880450f 100644
|
|
--- a/ssl/statem/extensions_srvr.c
|
|
+++ b/ssl/statem/extensions_srvr.c
|
|
@@ -1476,9 +1476,10 @@ EXT_RETURN tls_construct_stoc_next_proto_neg(SSL *s, WPACKET *pkt,
|
|
return EXT_RETURN_FAIL;
|
|
}
|
|
s->s3.npn_seen = 1;
|
|
+ return EXT_RETURN_SENT;
|
|
}
|
|
|
|
- return EXT_RETURN_SENT;
|
|
+ return EXT_RETURN_NOT_SENT;
|
|
}
|
|
#endif
|
|
|
|
--
|
|
2.45.2
|
|
|
|
From 017e54183b95617825fb9316d618c154a34c634e Mon Sep 17 00:00:00 2001
|
|
From: Matt Caswell <matt@openssl.org>
|
|
Date: Fri, 21 Jun 2024 11:51:54 +0100
|
|
Subject: [PATCH] Add ALPN validation in the client
|
|
|
|
The ALPN protocol selected by the server must be one that we originally
|
|
advertised. We should verify that it is.
|
|
|
|
Follow on from CVE-2024-5535
|
|
|
|
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
|
(Merged from https://github.com/openssl/openssl/pull/24718)
|
|
---
|
|
ssl/statem/extensions_clnt.c | 24 ++++++++++++++++++++++++
|
|
1 file changed, 24 insertions(+)
|
|
|
|
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
|
|
index a07dc62e9a..b21ccf9273 100644
|
|
--- a/ssl/statem/extensions_clnt.c
|
|
+++ b/ssl/statem/extensions_clnt.c
|
|
@@ -1566,6 +1566,8 @@ int tls_parse_stoc_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
|
|
size_t chainidx)
|
|
{
|
|
size_t len;
|
|
+ PACKET confpkt, protpkt;
|
|
+ int valid = 0;
|
|
|
|
/* We must have requested it. */
|
|
if (!s->s3.alpn_sent) {
|
|
@@ -1584,6 +1586,28 @@ int tls_parse_stoc_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
|
|
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
|
|
return 0;
|
|
}
|
|
+
|
|
+ /* It must be a protocol that we sent */
|
|
+ if (!PACKET_buf_init(&confpkt, s->ext.alpn, s->ext.alpn_len)) {
|
|
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
|
|
+ return 0;
|
|
+ }
|
|
+ while (PACKET_get_length_prefixed_1(&confpkt, &protpkt)) {
|
|
+ if (PACKET_remaining(&protpkt) != len)
|
|
+ continue;
|
|
+ if (memcmp(PACKET_data(pkt), PACKET_data(&protpkt), len) == 0) {
|
|
+ /* Valid protocol found */
|
|
+ valid = 1;
|
|
+ break;
|
|
+ }
|
|
+ }
|
|
+
|
|
+ if (!valid) {
|
|
+ /* The protocol sent from the server does not match one we advertised */
|
|
+ SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
|
|
+ return 0;
|
|
+ }
|
|
+
|
|
OPENSSL_free(s->s3.alpn_selected);
|
|
s->s3.alpn_selected = OPENSSL_malloc(len);
|
|
if (s->s3.alpn_selected == NULL) {
|
|
--
|
|
2.45.2
|
|
|