Skip to content

Conversation

@a3f
Copy link
Contributor

@a3f a3f commented Jul 8, 2025

We already check that the RSA *key is not NULL in jose_openssl_jwk_from_RSA(), but fail to do so for EC_KEY *key in jose_openssl_jwk_from_EC_KEY().

But EVP_PKEY_get0_EC_KEY() can return NULL too, e.g., if the EVP_PKEY comes from an OpenSSL provider that is not creating a keymgmt instance for a public key and the default provider is not loaded1.

Instead of crashing inside OpenSSL when we pass a NULL pointer to EC_KEY_get0_private_key(), detect this case and return gracefully.

@sarroutbi
Copy link
Collaborator

Thanks for your PR @a3f. Could you please rebase master branch to check if CI issue in your PR is fixed?

We already check that the RSA *key is not NULL in
jose_openssl_jwk_from_RSA(), but fail to do so for EC_KEY *key in
jose_openssl_jwk_from_EC_KEY().

But EVP_PKEY_get0_EC_KEY() can return NULL too, e.g., if
the EVP_PKEY comes from an OpenSSL provider that is not creating a
keymgmt instance for a public key and the default provider is not
loaded[1].

Instead of crashing inside OpenSSL when we pass a NULL pointer to
EC_KEY_get0_private_key(), detect this case and return gracefully.

[1]: openssl/openssl#25679

Signed-off-by: Ahmad Fatoum <[email protected]>
@a3f
Copy link
Contributor Author

a3f commented Jul 9, 2025

I have rebased, CI run needs maintainer approval.

Copy link
Collaborator

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

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

LGTM. @sergio-correia, PTAL

@sarroutbi sarroutbi requested a review from sergio-correia July 9, 2025 10:56
@sarroutbi sarroutbi merged commit 5aaaaf6 into latchset:master Jul 9, 2025
43 of 44 checks passed
@a3f a3f deleted the jose_openssl_jwk_from_EC_KEY-NULL-check branch July 9, 2025 12:27
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