You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
109 lines
4.0 KiB
109 lines
4.0 KiB
From 99fb785a5f85315b95288921a321a935ea29a51e Mon Sep 17 00:00:00 2001 |
|
From: Matt Caswell <matt@openssl.org> |
|
Date: Fri, 31 May 2024 11:14:33 +0100 |
|
Subject: [PATCH 01/10] 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/24717) |
|
--- |
|
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 016135fe18..cf52b317cf 100644 |
|
--- a/ssl/ssl_lib.c |
|
+++ b/ssl/ssl_lib.c |
|
@@ -3518,37 +3518,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.46.0 |
|
|
|
|