Skip to content

Move away from deprecated OpenSSL function EVP_PKEY_get0_EC_KEY. - #1384

Merged
emelsimsek merged 2 commits into
masterfrom
emelsimsek/Move-away-from-deprecated-functions
Sep 18, 2025
Merged

emelsimsek merged 2 commits into
masterfrom
emelsimsek/Move-away-from-deprecated-functions

Conversation

@emelsimsek

@emelsimsek emelsimsek commented Sep 1, 2025 •

Copy link
Copy Markdown
Contributor

EVP_PKEY_get0_EC_KEY is deprecated in OpenSSL 3.0 along with other functions.

It is recommended to be replaced by EVP_PKEY_get_params.

This PR makes the replacement for openssl version >=3+ while ensuring openssl 1+ stays the same.

WhileEVP_PKEY_get0_EC_KEY did not seem to cause an issue. We discovered that if a FIPS provider does not export the key in a way expected by EVP_PKEY_get0_EC_KEY, it returns NULL. This causes the current code to crash due to the absent NULL check:

const EC_KEY *ecdh = EVP_PKEY_get0_EC_KEY(pk);
const EC_GROUP *eg = EC_KEY_get0_group(ecdh);

@emelsimsek
emelsimsek marked this pull request as draft September 1, 2025 11:19
@emelsimsek
emelsimsek requested a review from petere September 1, 2025 13:37
@emelsimsek
emelsimsek marked this pull request as ready for review September 1, 2025 13:38
@emelsimsek emelsimsek changed the title Move away from deprecated OpenSSL functions. Move away from deprecated OpenSSL function EVP_PKEY_get0_EC_KEY. Sep 1, 2025
@emelsimsek

Copy link
Copy Markdown
Contributor Author

@petere we may have other deprecated functions within the codebase. Let me know if we want to handle all at once.

Comment thread lib/usual/tls/tls_compat.c Outdated
@petere

petere commented Sep 1, 2025

Copy link
Copy Markdown
Member

In OpenSSL, deprecated doesn't mean that the function will be removed. It only means that you should use the new ones if you no longer need to support the old versions. So I don't these changes are necessary.

@emelsimsek

Copy link
Copy Markdown
Contributor Author

In OpenSSL, deprecated doesn't mean that the function will be removed. It only means that you should use the new ones if you no longer need to support the old versions. So I don't these changes are necessary.

Yes, the function still exists in OpenSSL 3.0+ but its underlying implementation seems to have changed when a crypto provider is used.

Here is the recommendation:
openssl/openssl#28129.

@petere

petere commented Sep 3, 2025

Copy link
Copy Markdown
Member

Can you illustrate what the impact is for practical purposes? Does EVP_PKEY_get0_EC_KEY() no longer work with certain OpenSSL versions? How is this function used in PgBouncer?

@emelsimsek

emelsimsek commented Sep 9, 2025 •

Copy link
Copy Markdown
Contributor Author

Can you illustrate what the impact is for practical purposes? Does EVP_PKEY_get0_EC_KEY() no longer work with certain OpenSSL versions? How is this function used in PgBouncer?

PgBouncer uses this function for logging server's session/temporary key's properties.

So in a topology where

PgBouncer --->use TLS to connect to ---> PostgreSQL

PgBouncer logs the server's key's properties such as:

2025-09-09 10:57:33.701 +03 [290416] LOG S-0x5d9cddde6f28: mytestdb/test1@127.0.0.1:9700 SSL established: TLSv1.3/TLS_AES_256_GCM_SHA384/ECDH=prime256v1

PgBouncer crashes when EVP_PKEY_get0_EC_KEY() returns NULL. (We can choose to handle NULL here by not logging key properties and call it a fix as well)

It looks like this has become a problem with OpenSSL 3+ when FIPS mode is configured with a custom FIPS provider.

E.g. The following configuration causes PgBouncer to crash

openssl list -providers
Providers:
  default
    name: OpenSSL Default Provider
    version: 3.3.3
    status: active
  symcryptprovider
    name: symcryptprovider
    version: 1.8.1
    status: active

Note that SymCrypt is the core cryptographic function library by Microsoft.

However, if we configure OpenSSL with the built-in FIPS module, PgBouncer does not crash.

openssl list --providers
Providers:
  base
    name: OpenSSL Base Provider
    version: 3.1.2
    status: active
  fips
    name: OpenSSL FIPS Provider
    version: 3.1.2
    status: active

It is my understanding that crypto providers do not have to export the key anymore (with OpenSSL 3+ anyways). So this issue may not be the fault of or limited to SymCrypt provider.

@petere petere mentioned this pull request Sep 17, 2025

@petere petere left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have studied up on the problem and I agree with this solution. (I don't claim to understand every detail.)

Let's just make the code a bit prettier.

Comment thread lib/usual/tls/tls_compat.c Outdated
Comment thread lib/usual/tls/tls_compat.c
Comment thread lib/usual/tls/tls_compat.c
Comment thread lib/usual/tls/tls_compat.c
Comment thread lib/usual/tls/tls_compat.c Outdated
Comment thread lib/usual/tls/tls_compat.h
Comment thread lib/usual/tls/tls_compat.h
Comment thread lib/usual/tls/tls_compat.h
Comment thread lib/usual/tls/tls_util.c Outdated
@petere

petere commented Sep 17, 2025

Copy link
Copy Markdown
Member

Also squash the commits and write a nice commit message for posterity.

@emelsimsek
emelsimsek force-pushed the emelsimsek/Move-away-from-deprecated-functions branch from 8b776fd to 43ca8dd Compare September 18, 2025 17:01
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.

3 participants