Skip to content

Navigation Menu

Sign in
Appearance settings
Sign up
Appearance settings

Fix LibreSSL DH opaque struct access and X448 curve parsing - #5461

#5461
Open
blackandwhitebg wants to merge 3 commits into
pocoproject:mainpocoproject/poco:mainfrom
blackandwhitebg:mainblackandwhitebg/poco:mainCopy head branch name to clipboard
Open

Fix LibreSSL DH opaque struct access and X448 curve parsing#5461
blackandwhitebg wants to merge 3 commits into
pocoproject:mainpocoproject/poco:mainfrom
blackandwhitebg:mainblackandwhitebg/poco:mainCopy head branch name to clipboard

Conversation

@blackandwhitebg

Copy link
Copy Markdown

Summary

This PR resolves two LibreSSL compatibility issues in Poco::Net::Context:

  1. Compilation Fix: Replaces direct dh->p and dh->g struct member access with accessor functions (DH_set0_pqg, DH_set_length) for modern LibreSSL releases (2.7.0+), resolving opaque struct compilation errors (C2027).
  2. Runtime Fix: Excludes X448 from the default ECDH curve list during initECDH() when compiling against LibreSSL. LibreSSL does not support X448 in its curve parser, which previously caused SSL_CTX_set1_curves_list() to fail and throw an SSLContextException.

Testing Environment

  • POCO Branch: main
  • LibreSSL Version: 4.3.2

@matejk

matejk commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

@blackandwhitebg please rebase PR on latest main.

@matejk matejk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for chasing this down -- I reproduced both problems against LibreSSL 4.3.2 on Ubuntu 26.04 and macOS, and both are real:

  • main fails to compile with 8 incomplete type 'DH' errors, because #if !defined(LIBRESSL_VERSION_NUMBER) sends every LibreSSL version down the dh->p path even though DH has been opaque since LibreSSL 2.7.0.
  • SSL_CTX_set1_curves_list() returns 0 for any list containing X448 on LibreSSL, so every Context throws. Confirmed with a direct probe.

The DH_free() calls on the NotImplementedException paths and checking the DH_set0_pqg() return value are good catches -- main leaks p and g today because it ignores that return value entirely.

Two structural requests before this goes in.

1. A LibreSSL version macro instead of raw hex. Crypto.h already has POCO_OPENSSL_VERSION_PREREQ and asks callers not to compare OPENSSL_VERSION_NUMBER directly; the same should apply to LibreSSL. LIBRESSL_VERSION_NUMBER uses the same 0xMNNFFPPS encoding, so it is the same shape. Please add to Crypto/include/Poco/Crypto/Crypto.h, next to the existing macro:

#ifndef POCO_LIBRESSL_VERSION_PREREQ
	/// Check for LibreSSL >= major.minor.fix using LIBRESSL_VERSION_NUMBER,
	/// which uses the same 0xMNNFFPPS encoding as OPENSSL_VERSION_NUMBER.
	///
	/// Evaluates to 0 when not building against LibreSSL, so it needs no
	/// #if defined(LIBRESSL_VERSION_NUMBER) guard around it.
	#if defined(LIBRESSL_VERSION_NUMBER)
		#define POCO_LIBRESSL_VERSION_PREREQ(maj, min, fix) \
			(LIBRESSL_VERSION_NUMBER >= (((maj) << 28) | ((min) << 20) | ((fix) << 12)))
	#else
		#define POCO_LIBRESSL_VERSION_PREREQ(maj, min, fix) 0
	#endif
#endif

2. Drop pre-2.7.0 LibreSSL rather than keeping a legacy branch. LibreSSL 2.7.0 is from 2018, and Poco already requires OpenSSL >= 1.1.1. Setting a minimum the same way, right after the OpenSSL one:

#if defined(LIBRESSL_VERSION_NUMBER) && !POCO_LIBRESSL_VERSION_PREREQ(2, 7, 0)
#error "LibreSSL version too old. At least LibreSSL 2.7.0 is required."
#endif

This is needed as a separate check because LibreSSL hardcodes OPENSSL_VERSION_NUMBER to 0x20000000L for every release, so the existing OpenSSL 1.1.1 gate passes on all LibreSSL versions and can never say anything about which one is in use. Without the LibreSSL minimum, a 2.6 build sails past that gate and then fails deep in Context.cpp with the incomplete type 'DH' errors this PR is fixing.

It also lets the whole #if/#else split in initDH() go away -- the DH_set0_pqg() path becomes unconditional, the last direct struct access leaves the file, and anyone on ancient LibreSSL gets one clear message instead of eight compile errors.

Also, the branch still partly reverts #5460: the merge restored the FIPS block in initDH(), but initECDH() still has its FIPS branch removed, and the merge moved the OPENSSL_NO_DH guard in a way that changes behaviour. Details inline.

Note that #5464 needs to land first: it fixes a mis-nested #else in isFIPSEnabled() that leaves the function with no return on the LibreSSL path. With the above applied on top of #5464, LibreSSL 4.3.2 and OpenSSL 3 both build clean under -Werror=return-type and Crypto, JWT and 65/66 NetSSL tests pass. The one failure is TCPServerTest::testReuseSessionTLS13, a LibreSSL TLS 1.3 session resumption difference unrelated to this PR.

I have also opened #5465, a macOS LibreSSL CI job, so this cannot regress again. It will be merged after this one.

A rebase on main would make this easier to read than the merge commit, but that is up to you.

{
#ifndef OPENSSL_NO_DH

#if POCO_OPENSSL_VERSION_PREREQ(3, 0, 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The merge moved #ifndef OPENSSL_NO_DH below this block, so on an OPENSSL_NO_DH build the FIPS path now runs and returns early, and the throw SSLContextException("Implementation does not support DH") further down is never reached -- a non-empty dhParamsFile is silently accepted and ignored. The guard needs to stay outermost.

Suggested change
#if POCO_OPENSSL_VERSION_PREREQ(3, 0, 0)
#ifndef OPENSSL_NO_DH
#if POCO_OPENSSL_VERSION_PREREQ(3, 0, 0)

Comment on lines +634 to +635
#ifndef OPENSSL_NO_DH

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paired with the above -- remove these two lines. Line 635 is also trailing whitespace (git diff --check flags it).

Suggested change
#ifndef OPENSSL_NO_DH

}

if (1 != EVP_PKEY_fromdata(pKeyCtx, &pKey, EVP_PKEY_KEY_PARAMETERS, params))
if (1 != EVP_PKEY_fromdata(pKeyCtx, &pKey, EVP_PKEY_KEYPAIR, params))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reverts part of #5460 and is unrelated to LibreSSL -- the whole POCO_OPENSSL_VERSION_PREREQ(3, 0, 0) block is unreachable there, since LibreSSL pins OPENSSL_VERSION_NUMBER to 0x20000000L. Please keep EVP_PKEY_KEY_PARAMETERS; only parameters are being imported here.

Suggested change
if (1 != EVP_PKEY_fromdata(pKeyCtx, &pKey, EVP_PKEY_KEYPAIR, params))
if (1 != EVP_PKEY_fromdata(pKeyCtx, &pKey, EVP_PKEY_KEY_PARAMETERS, params))

}

#if !defined(LIBRESSL_VERSION_NUMBER)
#if !defined(LIBRESSL_VERSION_NUMBER) || LIBRESSL_VERSION_NUMBER >= 0x2070000fL

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the POCO_LIBRESSL_VERSION_PREREQ macro and the 2.7.0 minimum from the summary come in: with the minimum enforced in Crypto.h, this #if and the #else // Legacy LibreSSL (< 2.7.0) branch below it can both go, leaving one unconditional path down to the #endif on line 943.

While rewriting the block, the comment on lines 904-905 duplicates itself ("Verify allocations AND verification that") and the second line restates the code. One line covers the non-obvious part:

// DH_set0_pqg() only takes ownership of p and g when it succeeds.

Comment on lines +965 to +970
#if defined(LIBRESSL_VERSION_NUMBER)
// LibreSSL does not support X448 in its curve parser
const std::string groups(curve.empty() ? "X25519:P-256:P-384:P-521" : curve);
#else
const std::string groups(curve.empty() ? "X448:X25519:P-521:P-384:P-256" : curve);
#endif

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still drops the FIPS branch added in #5460, so FIPS builds go back to advertising X25519/X448. The LibreSSL guard only needs to change the non-FIPS default.

Also, the order is inverted relative to the OpenSSL branch. I checked that LibreSSL accepts the list in either order, so keep it strongest-first for consistency.

This one is a vendor check rather than a version check, so plain defined(LIBRESSL_VERSION_NUMBER) is right here -- no need for the new macro.

Suggested change
#if defined(LIBRESSL_VERSION_NUMBER)
// LibreSSL does not support X448 in its curve parser
const std::string groups(curve.empty() ? "X25519:P-256:P-384:P-521" : curve);
#else
const std::string groups(curve.empty() ? "X448:X25519:P-521:P-384:P-256" : curve);
#endif
#if defined(LIBRESSL_VERSION_NUMBER)
// LibreSSL's curve list parser does not know X448.
const char* defaultGroups = "X25519:P-521:P-384:P-256";
#else
const char* defaultGroups = "X448:X25519:P-521:P-384:P-256";
#endif
const std::string groups(curve.empty()
? (SSLManager::isFIPSEnabled()
? "P-521:P-384:P-256" // FIPS 140-2 + 140-3 safe
: defaultGroups)
: curve);

Comment thread NetSSL_OpenSSL/src/Context.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.