diff --git a/CHANGES.md b/CHANGES.md index 7241aced6101be918d541b623a0ec1520677c7f4..11f4d6a8d89ee3bcecf6f74e75971ee37d973b94 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -24,7 +24,19 @@ OpenSSL 3.1 ### Changes between 3.1.4 and 3.1.5 [xx XXX xxxx] - * none yet + * Fix excessive time spent in DH check / generation with large Q parameter + value. + + Applications that use the functions DH_generate_key() to generate an + X9.42 DH key may experience long delays. Likewise, applications that use + DH_check_pub_key(), DH_check_pub_key_ex() or EVP_PKEY_public_check() + to check an X9.42 DH key or X9.42 DH parameters may experience long delays. + Where the key or parameters that are being checked have been obtained from + an untrusted source this may lead to a Denial of Service. + + ([CVE-2023-5678]) + + *Richard Levitte* ### Changes between 3.1.3 and 3.1.4 [24 Oct 2023] @@ -19868,6 +19880,7 @@ ndif +[CVE-2023-5678]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-5678 [CVE-2023-5363]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-5363 [CVE-2023-4807]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-4807 [CVE-2023-3817]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-3817 diff --git a/NEWS.md b/NEWS.md index 7093f673d25b0f41cd9e51ab312f0449497d3bb6..a953810c92afd77104d5fa2e778c4d0e9a3805ea 100644 --- a/NEWS.md +++ b/NEWS.md @@ -21,7 +21,8 @@ OpenSSL 3.1 ### Major changes between OpenSSL 3.1.4 and OpenSSL 3.1.5 [under development] - * none + * Fix excessive time spent in DH check / generation with large Q parameter + value ([CVE-2023-5678]) ### Major changes between OpenSSL 3.1.3 and OpenSSL 3.1.4 [24 Oct 2023] @@ -1478,6 +1479,7 @@ OpenSSL 0.9.x +[CVE-2023-5678]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-5678 [CVE-2023-5363]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-5363 [CVE-2023-4807]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-4807 [CVE-2023-3817]: https://www.openssl.org/news/vulnerabilities.html#CVE-2023-3817 diff --git a/apps/list.c b/apps/list.c index 514abacfc835fd884bfa67eb991b90d4ed920488..b9439c9e549636a8fe404182f585d21619f95b1d 100644 --- a/apps/list.c +++ b/apps/list.c @@ -1209,9 +1209,11 @@ static int provider_cmp(const OSSL_PROVIDER * const *a, static int collect_providers(OSSL_PROVIDER *provider, void *stack) { STACK_OF(OSSL_PROVIDER) *provider_stack = stack; - - sk_OSSL_PROVIDER_push(provider_stack, provider); - return 1; + /* + * If OK - result is the index of inserted data + * Error - result is -1 or 0 + */ + return sk_OSSL_PROVIDER_push(provider_stack, provider) > 0 ? 1 : 0; } static void list_provider_info(void) @@ -1226,8 +1228,13 @@ static void list_provider_info(void) BIO_printf(bio_err, "ERROR: Memory allocation\n"); return; } + + if (OSSL_PROVIDER_do_all(NULL, &collect_providers, providers) != 1) { + BIO_printf(bio_err, "ERROR: Memory allocation\n"); + return; + } + BIO_printf(bio_out, "Providers:\n"); - OSSL_PROVIDER_do_all(NULL, &collect_providers, providers); sk_OSSL_PROVIDER_sort(providers); for (i = 0; i < sk_OSSL_PROVIDER_num(providers); i++) { const OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(providers, i); diff --git a/apps/rehash.c b/apps/rehash.c index 5c6d5340becfeb6038cd0a5ff6d372de909fc431..9696aa9f4ef494db06f42a6a770d1aa9d13d9b6c 100644 --- a/apps/rehash.c +++ b/apps/rehash.c @@ -355,9 +355,9 @@ static int do_dir(const char *dirname, enum Hash h) OPENSSL_DIR_CTX *d = NULL; struct stat st; unsigned char idmask[MAX_COLLISIONS / 8]; - int n, numfiles, nextid, buflen, errs = 0; + int n, numfiles, nextid, dirlen, buflen, errs = 0; size_t i; - const char *pathsep; + const char *pathsep = ""; const char *filename; char *buf, *copy = NULL; STACK_OF(OPENSSL_STRING) *files = NULL; @@ -366,9 +366,12 @@ static int do_dir(const char *dirname, enum Hash h) BIO_printf(bio_err, "Skipping %s, can't write\n", dirname); return 1; } - buflen = strlen(dirname); - pathsep = (buflen && !ends_with_dirsep(dirname)) ? "/": ""; - buflen += NAME_MAX + 1 + 1; + dirlen = strlen(dirname); + if (dirlen != 0 && !ends_with_dirsep(dirname)) { + pathsep = "/"; + dirlen++; + } + buflen = dirlen + NAME_MAX + 1; buf = app_malloc(buflen, "filename buffer"); if (verbose) @@ -427,12 +430,12 @@ static int do_dir(const char *dirname, enum Hash h) while (bit_isset(idmask, nextid)) nextid++; - BIO_snprintf(buf, buflen, "%s%s%n%08x.%s%d", - dirname, pathsep, &n, bp->hash, + BIO_snprintf(buf, buflen, "%s%s%08x.%s%d", + dirname, pathsep, bp->hash, suffixes[bp->type], nextid); if (verbose) BIO_printf(bio_out, "link %s -> %s\n", - ep->filename, &buf[n]); + ep->filename, &buf[dirlen]); if (unlink(buf) < 0 && errno != ENOENT) { BIO_printf(bio_err, "%s: Can't unlink %s, %s\n", @@ -449,12 +452,12 @@ static int do_dir(const char *dirname, enum Hash h) bit_set(idmask, nextid); } else if (remove_links) { /* Link to be deleted */ - BIO_snprintf(buf, buflen, "%s%s%n%08x.%s%d", - dirname, pathsep, &n, bp->hash, + BIO_snprintf(buf, buflen, "%s%s%08x.%s%d", + dirname, pathsep, bp->hash, suffixes[bp->type], ep->old_id); if (verbose) BIO_printf(bio_out, "unlink %s\n", - &buf[n]); + &buf[dirlen]); if (unlink(buf) < 0 && errno != ENOENT) { BIO_printf(bio_err, "%s: Can't unlink %s, %s\n", diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c index 7ba2beae7fd6b9be1f19c86a002524d3fe66f403..e20eb62081c5ee4895c330465f89c54a4da036d2 100644 --- a/crypto/dh/dh_check.c +++ b/crypto/dh/dh_check.c @@ -249,6 +249,18 @@ int DH_check_pub_key_ex(const DH *dh, const BIGNUM *pub_key) */ int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *ret) { + /* Don't do any checks at all with an excessively large modulus */ + if (BN_num_bits(dh->params.p) > OPENSSL_DH_CHECK_MAX_MODULUS_BITS) { + ERR_raise(ERR_LIB_DH, DH_R_MODULUS_TOO_LARGE); + *ret = DH_MODULUS_TOO_LARGE | DH_CHECK_PUBKEY_INVALID; + return 0; + } + + if (dh->params.q != NULL && BN_ucmp(dh->params.p, dh->params.q) < 0) { + *ret |= DH_CHECK_INVALID_Q_VALUE | DH_CHECK_PUBKEY_INVALID; + return 1; + } + return ossl_ffc_validate_public_key(&dh->params, pub_key, ret); } diff --git a/crypto/dh/dh_err.c b/crypto/dh/dh_err.c index 4152397426cc939ca7214fc4db808835a7dcdb1c..f76ac0dd1463f14fc13e6cd8f48a008be60a5f8f 100644 --- a/crypto/dh/dh_err.c +++ b/crypto/dh/dh_err.c @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2023 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -54,6 +54,7 @@ static const ERR_STRING_DATA DH_str_reasons[] = { {ERR_PACK(ERR_LIB_DH, 0, DH_R_PARAMETER_ENCODING_ERROR), "parameter encoding error"}, {ERR_PACK(ERR_LIB_DH, 0, DH_R_PEER_KEY_ERROR), "peer key error"}, + {ERR_PACK(ERR_LIB_DH, 0, DH_R_Q_TOO_LARGE), "q too large"}, {ERR_PACK(ERR_LIB_DH, 0, DH_R_SHARED_INFO_ERROR), "shared info error"}, {ERR_PACK(ERR_LIB_DH, 0, DH_R_UNABLE_TO_CHECK_GENERATOR), "unable to check generator"}, diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c index d84ea99241b9e87e576503c2513063b3d2072f60..afc49f5cdc87d7ed71a419144ad9cd117e9234da 100644 --- a/crypto/dh/dh_key.c +++ b/crypto/dh/dh_key.c @@ -49,6 +49,12 @@ int ossl_dh_compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh) goto err; } + if (dh->params.q != NULL + && BN_num_bits(dh->params.q) > OPENSSL_DH_MAX_MODULUS_BITS) { + ERR_raise(ERR_LIB_DH, DH_R_Q_TOO_LARGE); + goto err; + } + if (BN_num_bits(dh->params.p) < DH_MIN_MODULUS_BITS) { ERR_raise(ERR_LIB_DH, DH_R_MODULUS_TOO_SMALL); return 0; @@ -267,6 +273,12 @@ static int generate_key(DH *dh) return 0; } + if (dh->params.q != NULL + && BN_num_bits(dh->params.q) > OPENSSL_DH_MAX_MODULUS_BITS) { + ERR_raise(ERR_LIB_DH, DH_R_Q_TOO_LARGE); + return 0; + } + if (BN_num_bits(dh->params.p) < DH_MIN_MODULUS_BITS) { ERR_raise(ERR_LIB_DH, DH_R_MODULUS_TOO_SMALL); return 0; diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index a1e6bbb617fcb22cc351bc711457899e4f64b586..69e4f61aa1801f69131898000c739ce7881af80c 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -513,6 +513,7 @@ DH_R_NO_PARAMETERS_SET:107:no parameters set DH_R_NO_PRIVATE_VALUE:100:no private value DH_R_PARAMETER_ENCODING_ERROR:105:parameter encoding error DH_R_PEER_KEY_ERROR:111:peer key error +DH_R_Q_TOO_LARGE:130:q too large DH_R_SHARED_INFO_ERROR:113:shared info error DH_R_UNABLE_TO_CHECK_GENERATOR:121:unable to check generator DSA_R_BAD_FFC_PARAMETERS:114:bad ffc parameters diff --git a/doc/man7/EVP_KDF-SS.pod b/doc/man7/EVP_KDF-SS.pod index 7f158e421698ee7f03f7c2053ab0899cf613e735..c8d19691a797b86fd11ecb9e89351e2606eeb28f 100644 --- a/doc/man7/EVP_KDF-SS.pod +++ b/doc/man7/EVP_KDF-SS.pod @@ -53,7 +53,7 @@ This parameter is ignored for KMAC. These parameters work as described in L. -=item "key" (B) +=item "key" (B) This parameter set the shared secret that is used for key derivation. @@ -116,7 +116,7 @@ fixedinfo value "label" and salt "salt": SN_hmac, strlen(SN_hmac)); *p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, SN_sha256, strlen(SN_sha256)); - *p++ = OSSL_PARAM_construct_octet_string(EVP_KDF_CTRL_SET_KEY, + *p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SECRET, "secret", (size_t)6); *p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, "label", (size_t)5); @@ -143,7 +143,7 @@ fixedinfo value "label", salt of "salt" and KMAC outlen of 20: *p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_MAC, SN_kmac128, strlen(SN_kmac128)); - *p++ = OSSL_PARAM_construct_octet_string(EVP_KDF_CTRL_SET_KEY, + *p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SECRET, "secret", (size_t)6); *p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, "label", (size_t)5); diff --git a/doc/man7/EVP_MAC-KMAC.pod b/doc/man7/EVP_MAC-KMAC.pod index 0214c1e254076a521bebbb9eb85940e9cd3ecb3e..bdadb0ff6047727d034085f5faa3d7dbf8209f0d 100644 --- a/doc/man7/EVP_MAC-KMAC.pod +++ b/doc/man7/EVP_MAC-KMAC.pod @@ -51,7 +51,7 @@ It is an optional value with a length of at most 512 bytes, and is empty by defa =item "size" (B) Sets the MAC size. -By default, it is 16 for C and 32 for C. +By default, it is 32 for C and 64 for C. =item "block-size" (B) diff --git a/include/crypto/dherr.h b/include/crypto/dherr.h index bb24d131eb8873257fc1105904529b88108b6056..519327f795742a8f19038e9e274b33c7f99a6061 100644 --- a/include/crypto/dherr.h +++ b/include/crypto/dherr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 2020-2021 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2020-2023 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy diff --git a/include/internal/ffc.h b/include/internal/ffc.h index c4f090875f33c64fefe13be30a2e24f8541cb010..e96f08d68e938036e669717df24c13d4f2515c80 100644 --- a/include/internal/ffc.h +++ b/include/internal/ffc.h @@ -58,8 +58,11 @@ # define FFC_CHECK_INVALID_Q_VALUE 0x00020 # define FFC_CHECK_INVALID_J_VALUE 0x00040 -# define FFC_CHECK_BAD_LN_PAIR 0x00080 -# define FFC_CHECK_INVALID_SEED_SIZE 0x00100 +/* + * 0x80, 0x100 reserved by include/openssl/dh.h with check bits that are not + * relevant for FFC. + */ + # define FFC_CHECK_MISSING_SEED_OR_COUNTER 0x00200 # define FFC_CHECK_INVALID_G 0x00400 # define FFC_CHECK_INVALID_PQ 0x00800 @@ -68,6 +71,8 @@ # define FFC_CHECK_Q_MISMATCH 0x04000 # define FFC_CHECK_G_MISMATCH 0x08000 # define FFC_CHECK_COUNTER_MISMATCH 0x10000 +# define FFC_CHECK_BAD_LN_PAIR 0x20000 +# define FFC_CHECK_INVALID_SEED_SIZE 0x40000 /* Validation Return codes */ # define FFC_ERROR_PUBKEY_TOO_SMALL 0x01 diff --git a/include/openssl/dh.h b/include/openssl/dh.h index 8bc17448a0817e992fd69f8017a108741f212264..f1c0ed06b375a9a71e44da93a9919bb9baf71534 100644 --- a/include/openssl/dh.h +++ b/include/openssl/dh.h @@ -144,7 +144,7 @@ DECLARE_ASN1_ITEM(DHparams) # define DH_GENERATOR_3 3 # define DH_GENERATOR_5 5 -/* DH_check error codes */ +/* DH_check error codes, some of them shared with DH_check_pub_key */ /* * NB: These values must align with the equivalently named macros in * internal/ffc.h. @@ -154,10 +154,10 @@ DECLARE_ASN1_ITEM(DHparams) # define DH_UNABLE_TO_CHECK_GENERATOR 0x04 # define DH_NOT_SUITABLE_GENERATOR 0x08 # define DH_CHECK_Q_NOT_PRIME 0x10 -# define DH_CHECK_INVALID_Q_VALUE 0x20 +# define DH_CHECK_INVALID_Q_VALUE 0x20 /* +DH_check_pub_key */ # define DH_CHECK_INVALID_J_VALUE 0x40 # define DH_MODULUS_TOO_SMALL 0x80 -# define DH_MODULUS_TOO_LARGE 0x100 +# define DH_MODULUS_TOO_LARGE 0x100 /* +DH_check_pub_key */ /* DH_check_pub_key error codes */ # define DH_CHECK_PUBKEY_TOO_SMALL 0x01 diff --git a/include/openssl/dherr.h b/include/openssl/dherr.h index 5d2a762a96f8cdb16840a27f2ce572e4a4e2afd6..074a70145f9f5b42d5af6c87375a161440d5f5ba 100644 --- a/include/openssl/dherr.h +++ b/include/openssl/dherr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2023 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -50,6 +50,7 @@ # define DH_R_NO_PRIVATE_VALUE 100 # define DH_R_PARAMETER_ENCODING_ERROR 105 # define DH_R_PEER_KEY_ERROR 111 +# define DH_R_Q_TOO_LARGE 130 # define DH_R_SHARED_INFO_ERROR 113 # define DH_R_UNABLE_TO_CHECK_GENERATOR 121 diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 78d4f040565d1d54f83abbd4fc73504f614846b7..bcfe57b46f083ba80d7c417803f7a1a1870f9164 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3365,6 +3365,10 @@ void ssl3_free(SSL *s) OPENSSL_free(s->s3.alpn_selected); OPENSSL_free(s->s3.alpn_proposed); +#ifndef OPENSSL_NO_PSK + OPENSSL_free(s->s3.tmp.psk); +#endif + #ifndef OPENSSL_NO_SRP ssl_srp_ctx_free_intern(s); #endif diff --git a/ssl/statem/extensions_cust.c b/ssl/statem/extensions_cust.c index 401a4c5c76b104269106140ec2fc833021c14807..73b82fc7de2d3361560fd93af7aaeaa1ef851bbd 100644 --- a/ssl/statem/extensions_cust.c +++ b/ssl/statem/extensions_cust.c @@ -220,6 +220,8 @@ int custom_ext_add(SSL *s, int context, WPACKET *pkt, X509 *x, size_t chainidx, || !WPACKET_start_sub_packet_u16(pkt) || (outlen > 0 && !WPACKET_memcpy(pkt, out, outlen)) || !WPACKET_close(pkt)) { + if (meth->free_cb != NULL) + meth->free_cb(s, meth->ext_type, context, out, meth->add_arg); SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -228,6 +230,9 @@ int custom_ext_add(SSL *s, int context, WPACKET *pkt, X509 *x, size_t chainidx, * We can't send duplicates: code logic should prevent this. */ if (!ossl_assert((meth->ext_flags & SSL_EXT_FLAG_SENT) == 0)) { + if (meth->free_cb != NULL) + meth->free_cb(s, meth->ext_type, context, out, + meth->add_arg); SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; }