fix CVE-2024-5535 CVE-2024-9143
(cherry picked from commit 0a73cc0e3535e66ea0319df512f4eda77905b4bd)
This commit is contained in:
parent
bc3a68adbb
commit
5521fb0ca6
1786
backport-CVE-2024-5535-Add-a-test-for-ALPN-and-NPN.patch
Normal file
1786
backport-CVE-2024-5535-Add-a-test-for-ALPN-and-NPN.patch
Normal file
File diff suppressed because it is too large
Load Diff
@ -0,0 +1,168 @@
|
||||
From d7afe8e89ced1f4d5f1e5aab474dd9c069115b6e Mon Sep 17 00:00:00 2001
|
||||
From: xuhuiyue <xuhuiyue@huawei.com>
|
||||
Date: Fri, 28 Jun 2024 17:31:29 +0800
|
||||
Subject: [PATCH 2/2] Fix SSL_select_next_proto and add ALPN validation in the
|
||||
client
|
||||
|
||||
Fix CVE-2024-5535.
|
||||
|
||||
Signed-off-by: xuhuiyue <xuhuiyue@huawei.com>
|
||||
---
|
||||
ssl/ssl_lib.c | 63 +++++++++++++++++++++++-------------
|
||||
ssl/statem/extensions_clnt.c | 27 +++++++++++++++-
|
||||
ssl/statem/extensions_srvr.c | 3 +-
|
||||
3 files changed, 68 insertions(+), 25 deletions(-)
|
||||
|
||||
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
|
||||
index 00410a7385..cb2dca4247 100644
|
||||
--- a/ssl/ssl_lib.c
|
||||
+++ b/ssl/ssl_lib.c
|
||||
@@ -2767,37 +2767,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
|
||||
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
|
||||
index c641ae7351..4ad75c8e2d 100644
|
||||
--- a/ssl/statem/extensions_clnt.c
|
||||
+++ b/ssl/statem/extensions_clnt.c
|
||||
@@ -1602,7 +1602,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_F_TLS_PARSE_STOC_NPN,
|
||||
SSL_R_BAD_EXTENSION);
|
||||
return 0;
|
||||
@@ -1633,6 +1634,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) {
|
||||
@@ -1653,6 +1656,28 @@ int tls_parse_stoc_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
|
||||
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, SSL_F_TLS_PARSE_STOC_ALPN, 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_F_TLS_PARSE_STOC_ALPN, 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) {
|
||||
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
|
||||
index 775d9a7444..a08027fd6d 100644
|
||||
--- a/ssl/statem/extensions_srvr.c
|
||||
+++ b/ssl/statem/extensions_srvr.c
|
||||
@@ -1562,9 +1562,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.33.0
|
||||
|
||||
@ -0,0 +1,201 @@
|
||||
From 72ae83ad214d2eef262461365a1975707f862712 Mon Sep 17 00:00:00 2001
|
||||
From: Viktor Dukhovni <viktor@openssl.org>
|
||||
Date: Thu, 19 Sep 2024 01:02:40 +1000
|
||||
Subject: [PATCH] Harden BN_GF2m_poly2arr against misuse.
|
||||
|
||||
The BN_GF2m_poly2arr() function converts characteristic-2 field
|
||||
(GF_{2^m}) Galois polynomials from a representation as a BIGNUM bitmask,
|
||||
to a compact array with just the exponents of the non-zero terms.
|
||||
|
||||
These polynomials are then used in BN_GF2m_mod_arr() to perform modular
|
||||
reduction. A precondition of calling BN_GF2m_mod_arr() is that the
|
||||
polynomial must have a non-zero constant term (i.e. the array has `0` as
|
||||
its final element).
|
||||
|
||||
Internally, callers of BN_GF2m_poly2arr() did not verify that
|
||||
precondition, and binary EC curve parameters with an invalid polynomial
|
||||
could lead to out of bounds memory reads and writes in BN_GF2m_mod_arr().
|
||||
|
||||
The precondition is always true for polynomials that arise from the
|
||||
standard form of EC parameters for characteristic-two fields (X9.62).
|
||||
See the "Finite Field Identification" section of:
|
||||
|
||||
https://www.itu.int/ITU-T/formal-language/itu-t/x/x894/2018-cor1/ANSI-X9-62.html
|
||||
|
||||
The OpenSSL GF(2^m) code supports only the trinomial and pentanomial
|
||||
basis X9.62 forms.
|
||||
|
||||
This commit updates BN_GF2m_poly2arr() to return `0` (failure) when
|
||||
the constant term is zero (i.e. the input bitmask BIGNUM is not odd).
|
||||
|
||||
Additionally, the return value is made unambiguous when there is not
|
||||
enough space to also pad the array with a final `-1` sentinel value.
|
||||
The return value is now always the number of elements (including the
|
||||
final `-1`) that would be filled when the output array is sufficiently
|
||||
large. Previously the same count was returned both when the array has
|
||||
just enough room for the final `-1` and when it had only enough space
|
||||
for non-sentinel values.
|
||||
|
||||
Finally, BN_GF2m_poly2arr() is updated to reject polynomials whose
|
||||
degree exceeds `OPENSSL_ECC_MAX_FIELD_BITS`, this guards against
|
||||
CPU exhausition attacks via excessively large inputs.
|
||||
|
||||
The above issues do not arise in processing X.509 certificates. These
|
||||
generally have EC keys from "named curves", and RFC5840 (Section 2.1.1)
|
||||
disallows explicit EC parameters. The TLS code in OpenSSL enforces this
|
||||
constraint only after the certificate is decoded, but, even if explicit
|
||||
parameters are specified, they are in X9.62 form, which cannot represent
|
||||
problem values as noted above.
|
||||
|
||||
Initially reported as oss-fuzz issue 71623.
|
||||
|
||||
A closely related issue was earlier reported in
|
||||
<https://github.com/openssl/openssl/issues/19826>.
|
||||
|
||||
Severity: Low, CVE-2024-9143
|
||||
|
||||
Reviewed-by: Matt Caswell <matt@openssl.org>
|
||||
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
|
||||
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
|
||||
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
||||
(Merged from https://github.com/openssl/openssl/pull/25639)
|
||||
|
||||
(cherry picked from commit 8e008cb8b23ec7dc75c45a66eeed09c815b11cd2)
|
||||
---
|
||||
crypto/bn/bn_gf2m.c | 28 +++++++++++++++-------
|
||||
test/ec_internal_test.c | 51 +++++++++++++++++++++++++++++++++++++++++
|
||||
2 files changed, 71 insertions(+), 8 deletions(-)
|
||||
|
||||
diff --git a/crypto/bn/bn_gf2m.c b/crypto/bn/bn_gf2m.c
|
||||
index c811ae82d6..bcc66613cc 100644
|
||||
--- a/crypto/bn/bn_gf2m.c
|
||||
+++ b/crypto/bn/bn_gf2m.c
|
||||
@@ -15,6 +15,7 @@
|
||||
#include "bn_local.h"
|
||||
|
||||
#ifndef OPENSSL_NO_EC2M
|
||||
+# include <openssl/ec.h>
|
||||
|
||||
/*
|
||||
* Maximum number of iterations before BN_GF2m_mod_solve_quad_arr should
|
||||
@@ -1140,16 +1141,26 @@ int BN_GF2m_mod_solve_quad(BIGNUM *r, const BIGNUM *a, const BIGNUM *p,
|
||||
/*
|
||||
* Convert the bit-string representation of a polynomial ( \sum_{i=0}^n a_i *
|
||||
* x^i) into an array of integers corresponding to the bits with non-zero
|
||||
- * coefficient. Array is terminated with -1. Up to max elements of the array
|
||||
- * will be filled. Return value is total number of array elements that would
|
||||
- * be filled if array was large enough.
|
||||
+ * coefficient. The array is intended to be suitable for use with
|
||||
+ * `BN_GF2m_mod_arr()`, and so the constant term of the polynomial must not be
|
||||
+ * zero. This translates to a requirement that the input BIGNUM `a` is odd.
|
||||
+ *
|
||||
+ * Given sufficient room, the array is terminated with -1. Up to max elements
|
||||
+ * of the array will be filled.
|
||||
+ *
|
||||
+ * The return value is total number of array elements that would be filled if
|
||||
+ * array was large enough, including the terminating `-1`. It is `0` when `a`
|
||||
+ * is not odd or the constant term is zero contrary to requirement.
|
||||
+ *
|
||||
+ * The return value is also `0` when the leading exponent exceeds
|
||||
+ * `OPENSSL_ECC_MAX_FIELD_BITS`, this guards against CPU exhaustion attacks,
|
||||
*/
|
||||
int BN_GF2m_poly2arr(const BIGNUM *a, int p[], int max)
|
||||
{
|
||||
int i, j, k = 0;
|
||||
BN_ULONG mask;
|
||||
|
||||
- if (BN_is_zero(a))
|
||||
+ if (!BN_is_odd(a))
|
||||
return 0;
|
||||
|
||||
for (i = a->top - 1; i >= 0; i--) {
|
||||
@@ -1167,12 +1178,13 @@ int BN_GF2m_poly2arr(const BIGNUM *a, int p[], int max)
|
||||
}
|
||||
}
|
||||
|
||||
- if (k < max) {
|
||||
+ if (k > 0 && p[0] > OPENSSL_ECC_MAX_FIELD_BITS)
|
||||
+ return 0;
|
||||
+
|
||||
+ if (k < max)
|
||||
p[k] = -1;
|
||||
- k++;
|
||||
- }
|
||||
|
||||
- return k;
|
||||
+ return k + 1;
|
||||
}
|
||||
|
||||
/*
|
||||
diff --git a/test/ec_internal_test.c b/test/ec_internal_test.c
|
||||
index 8c2cd05631..02cfd4e9d8 100644
|
||||
--- a/test/ec_internal_test.c
|
||||
+++ b/test/ec_internal_test.c
|
||||
@@ -155,6 +155,56 @@ static int field_tests_ecp_mont(void)
|
||||
}
|
||||
|
||||
#ifndef OPENSSL_NO_EC2M
|
||||
+/* Test that decoding of invalid GF2m field parameters fails. */
|
||||
+static int ec2m_field_sanity(void)
|
||||
+{
|
||||
+ int ret = 0;
|
||||
+ BN_CTX *ctx = BN_CTX_new();
|
||||
+ BIGNUM *p, *a, *b;
|
||||
+ EC_GROUP *group1 = NULL, *group2 = NULL, *group3 = NULL;
|
||||
+
|
||||
+ TEST_info("Testing GF2m hardening\n");
|
||||
+
|
||||
+ BN_CTX_start(ctx);
|
||||
+ p = BN_CTX_get(ctx);
|
||||
+ a = BN_CTX_get(ctx);
|
||||
+ if (!TEST_ptr(b = BN_CTX_get(ctx))
|
||||
+ || !TEST_true(BN_one(a))
|
||||
+ || !TEST_true(BN_one(b)))
|
||||
+ goto out;
|
||||
+
|
||||
+ /* Even pentanomial value should be rejected */
|
||||
+ if (!TEST_true(BN_set_word(p, 0xf2)))
|
||||
+ goto out;
|
||||
+ if (!TEST_ptr_null(group1 = EC_GROUP_new_curve_GF2m(p, a, b, ctx)))
|
||||
+ TEST_error("Zero constant term accepted in GF2m polynomial");
|
||||
+
|
||||
+ /* Odd hexanomial should also be rejected */
|
||||
+ if (!TEST_true(BN_set_word(p, 0xf3)))
|
||||
+ goto out;
|
||||
+ if (!TEST_ptr_null(group2 = EC_GROUP_new_curve_GF2m(p, a, b, ctx)))
|
||||
+ TEST_error("Hexanomial accepted as GF2m polynomial");
|
||||
+
|
||||
+ /* Excessive polynomial degree should also be rejected */
|
||||
+ if (!TEST_true(BN_set_word(p, 0x71))
|
||||
+ || !TEST_true(BN_set_bit(p, OPENSSL_ECC_MAX_FIELD_BITS + 1)))
|
||||
+ goto out;
|
||||
+ if (!TEST_ptr_null(group3 = EC_GROUP_new_curve_GF2m(p, a, b, ctx)))
|
||||
+ TEST_error("GF2m polynomial degree > %d accepted",
|
||||
+ OPENSSL_ECC_MAX_FIELD_BITS);
|
||||
+
|
||||
+ ret = group1 == NULL && group2 == NULL && group3 == NULL;
|
||||
+
|
||||
+ out:
|
||||
+ EC_GROUP_free(group1);
|
||||
+ EC_GROUP_free(group2);
|
||||
+ EC_GROUP_free(group3);
|
||||
+ BN_CTX_end(ctx);
|
||||
+ BN_CTX_free(ctx);
|
||||
+
|
||||
+ return ret;
|
||||
+}
|
||||
+
|
||||
/* test EC_GF2m_simple_method directly */
|
||||
static int field_tests_ec2_simple(void)
|
||||
{
|
||||
@@ -443,6 +493,7 @@ int setup_tests(void)
|
||||
ADD_TEST(field_tests_ecp_simple);
|
||||
ADD_TEST(field_tests_ecp_mont);
|
||||
#ifndef OPENSSL_NO_EC2M
|
||||
+ ADD_TEST(ec2m_field_sanity);
|
||||
ADD_TEST(field_tests_ec2_simple);
|
||||
#endif
|
||||
ADD_ALL_TESTS(field_tests_default, crv_len);
|
||||
--
|
||||
2.43.0.windows.1
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
%define soversion 1.1
|
||||
Name: compat-openssl11
|
||||
Version: 1.1.1m
|
||||
Release: 11
|
||||
Release: 12
|
||||
Epoch: 1
|
||||
Summary: Cryptography and SSL/TLS Toolkit
|
||||
License: OpenSSL and SSLeay
|
||||
@ -138,6 +138,9 @@ Patch127: backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-.patch
|
||||
Patch128: backport-CVE-2024-4741-test-Fix-possible-use-after-free.patch
|
||||
Patch129: skip-some-test-cases.patch
|
||||
Patch130: backport-Update-further-expiring-certificates-that-affect-tes.patch
|
||||
Patch131: backport-CVE-2024-5535-Fix-SSL_select_next_proto-and-add-ALPN.patch
|
||||
Patch132: backport-CVE-2024-5535-Add-a-test-for-ALPN-and-NPN.patch
|
||||
Patch133: backport-CVE-2024-9143-Harden-BN_GF2m_poly2arr-against-misuse.patch
|
||||
|
||||
BuildRequires: gcc perl make lksctp-tools-devel coreutils util-linux zlib-devel
|
||||
|
||||
@ -264,6 +267,9 @@ make test || :
|
||||
%ldconfig_scriptlets libs
|
||||
|
||||
%changelog
|
||||
* Thu Dec 12 2024 jinlun <jinlun@huawei.com> - 1:1.1.1m-12
|
||||
- fix CVE-2024-5535 CVE-2024-9143
|
||||
|
||||
* Fri Jun 7 2024 zhujianwei <zhujianwei7@huawei.com> - 1:1.1.1m-11
|
||||
- fix CVE-2024-4741
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user