From 9ad69b994ae7c73ba06d9f75efd2625102de814c Mon Sep 17 00:00:00 2001 From: Ken Zalewski Date: Mon, 21 Oct 2024 16:24:47 -0400 Subject: [PATCH] Patch to openssl-1.1.1zb. This version addresses one vulnerability: CVE-2024-9143 --- CHANGES | 134 +++++++++++++++++++++++++++++++++++++ NEWS | 18 +++++ README | 2 +- crypto/bn/bn_gf2m.c | 28 +++++--- include/openssl/opensslv.h | 4 +- test/ec_internal_test.c | 51 ++++++++++++++ 6 files changed, 226 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index c440948..7d82f7a 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,140 @@ https://github.com/openssl/openssl/commits/ and pick the appropriate release branch. + Changes between 1.1.1za and 1.1.1zb [16 Oct 2024] + + *) 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. + + (CVE-2024-9143) + [Viktor Dukhovni] + + + Changes between 1.1.1y and 1.1.1za [26 Jun 2024] + + *) 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) + [Matt Caswell] + + + Changes between 1.1.1x and 1.1.1y [27 May 2024] + + *) Only free the read buffers if we're not using them + + If we're part way through processing a record, or the application has + not released all the records then we should not free our buffer because + they are still needed. + + (CVE-2024-4741) + [Matt Caswell] + [Watson Ladd] + + *) Fix unconstrained session cache growth in TLSv1.3 + + In TLSv1.3 we create a new session object for each ticket that we send. + We do this by duplicating the original session. If SSL_OP_NO_TICKET is in + use then the new session will be added to the session cache. However, if + early data is not in use (and therefore anti-replay protection is being + used), then multiple threads could be resuming from the same session + simultaneously. If this happens and a problem occurs on one of the threads, + then the original session object could be marked as not_resumable. When we + duplicate the session object this not_resumable status gets copied into the + new session object. The new session object is then added to the session + cache even though it is not_resumable. + + Subsequently, another bug means that the session_id_length is set to 0 for + sessions that are marked as not_resumable - even though that session is + still in the cache. Once this happens the session can never be removed from + the cache. When that object gets to be the session cache tail object the + cache never shrinks again and grows indefinitely. + + (CVE-2024-2511) + [Matt Caswell] + + + Changes between 1.1.1w and 1.1.1x [25 Jan 2024] + + *) Add NULL checks where ContentInfo data can be NULL + + PKCS12 structures contain PKCS7 ContentInfo fields. These fields are + optional and can be NULL even if the "type" is a valid value. OpenSSL + was not properly accounting for this and a NULL dereference can occur + causing a crash. + + (CVE-2024-0727) + [Matt Caswell] + + *) Make DH_check_pub_key() and DH_generate_key() safer yet + + We already check for an excessively large P in DH_generate_key(), but not in + DH_check_pub_key(), and none of them check for an excessively large Q. + + This change adds all the missing excessive size checks of P and Q. + + It's to be noted that behaviours surrounding excessively sized P and Q + differ. DH_check() raises an error on the excessively sized P, but only + sets a flag for the excessively sized Q. This behaviour is mimicked in + DH_check_pub_key(). + + (CVE-2024-5678) + [Richard Levitte] + [Hugo Landau] + + Changes between 1.1.1v and 1.1.1w [11 Sep 2023] *) Fix POLY1305 MAC implementation corrupting XMM registers on Windows. diff --git a/NEWS b/NEWS index 1b849cd..7810ece 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,24 @@ This file gives a brief overview of the major changes between each OpenSSL release. For more details please read the CHANGES file. + Major changes between OpenSSL 1.1.1za and OpenSSL 1.1.1zb [16 Oct 2024] + + o Harden BN_GF2m_poly2arr against misuse + + Major changes between OpenSSL 1.1.1y and OpenSSL 1.1.1za [26 Jun 2024] + + o Fix SSL_select_next_proto + + Major changes between OpenSSL 1.1.1x and OpenSSL 1.1.1y [27 May 2024] + + o Only free the read buffers if we're not using them + o Fix unconstrained session cache growth in TLSv1.3 + + Major changes between OpenSSL 1.1.1w and OpenSSL 1.1.1x [25 Jan 2024] + + o Add NULL checks where ContentInfo data can be NULL + o Make DH_check_pub_key() and DH_generate_key() safer yet + Major changes between OpenSSL 1.1.1v and OpenSSL 1.1.1w [11 Sep 2023] o Fix POLY1305 MAC implementation corrupting XMM registers on Windows diff --git a/README b/README index e924e15..6612eb0 100644 --- a/README +++ b/README @@ -1,5 +1,5 @@ - OpenSSL 1.1.1w 11 Sep 2023 + OpenSSL 1.1.1zb 16 Oct 2024 Copyright (c) 1998-2023 The OpenSSL Project Copyright (c) 1995-1998 Eric A. Young, Tim J. Hudson diff --git a/crypto/bn/bn_gf2m.c b/crypto/bn/bn_gf2m.c index a2ea867..6709471 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 /* * Maximum number of iterations before BN_GF2m_mod_solve_quad_arr should @@ -1109,16 +1110,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--) { @@ -1136,12 +1147,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/include/openssl/opensslv.h b/include/openssl/opensslv.h index a1a5d07..ddf42b6 100644 --- a/include/openssl/opensslv.h +++ b/include/openssl/opensslv.h @@ -39,8 +39,8 @@ extern "C" { * (Prior to 0.9.5a beta1, a different scheme was used: MMNNFFRBB for * major minor fix final patch/beta) */ -# define OPENSSL_VERSION_NUMBER 0x101011afL -# define OPENSSL_VERSION_TEXT "OpenSSL 1.1.1za 26 Jun 2024" +# define OPENSSL_VERSION_NUMBER 0x101011bfL +# define OPENSSL_VERSION_TEXT "OpenSSL 1.1.1zb 16 Oct 2024" /*- * The macros below are to be used for shared library (.so, .dll, ...) diff --git a/test/ec_internal_test.c b/test/ec_internal_test.c index 390f41f..1590a18 100644 --- a/test/ec_internal_test.c +++ b/test/ec_internal_test.c @@ -150,6 +150,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) { @@ -367,6 +417,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);