Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of public keys #516

Merged
merged 6 commits into from
Feb 6, 2025
Merged

Improve handling of public keys #516

merged 6 commits into from
Feb 6, 2025

Conversation

simo5
Copy link
Member

@simo5 simo5 commented Feb 5, 2025

Description

This PR improves pkcs11-provider's handling of public keys in three related and interconnected areas by creating a first class association between private and public keys.

In OpenSSL, the EVP_PKEY type can carry both public and private key information in a single structure, and this is reflected sometimes heavily in the APIs and application usge of this type. The most glaring case is key pair generation.

To better handle cases where a private key i sourced but information about the public key is requested we allow a private key object to carry a refcounted pointer to the corresponding public key object.

The three main changes in this PR are:

  • key generation, we stop copying public attributes on the private key object on generation, and instead we associated the public key to the private key we return to the caller.
  • EC public key import is fixed to properly handle this behavior change, in the process the whole EC import/export code has been improved to fix broken corner cases
  • Public Key output, encoder functions to print public key information have been fixed to find the associate public key when public key info is requested for a private key.

A new pkey test has been added to check basic keygen and printing aspects, the existing CMS and ECDH tests fully exercise the EC public key export/import changes, and the edwards tests exercise the ED export changes.

Fixes #480

Checklist

  • Code modified for feature
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • [ ] Documentation updated

Reviewer's checklist:

  • Any issues marked for closing are addressed
  • There is a test suite reasonably covering new functionality or modifications
  • This feature/change has adequate documentation added
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible commit messages
  • Coverity Scan has run if needed (code PR) and no new defects were found

Jakuje
Jakuje previously approved these changes Feb 6, 2025
tests/tpkey.c Outdated
@@ -0,0 +1,135 @@
/* Copyright (C) 2022 Simo Sorce <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

Copy link
Member Author

Choose a reason for hiding this comment

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

heh

@simo5 simo5 added the covscan Triggers Coverity Scanner label Feb 6, 2025
@github-actions github-actions bot removed the covscan Triggers Coverity Scanner label Feb 6, 2025
simo5 added 6 commits February 6, 2025 09:30
In a few case we need to keep track of associated objects in order to be
compatible with the way OpenSSL deals with key pairs. The EVP_PKEY
object can reference both a private and a public key at the same time.

Storing an "associated object" (generally a public key associated to the
private key) allows the code to perform operations wth a public key
where the EVP_PKEY primary object is actually a private key.

The key pair generation code now always stores the public key as an
associated object so that generating a keypair and then immediately
testing it with a pair of sign/verify operations can be accomplished.

Signed-off-by: Simo Sorce <[email protected]>
Signed-off-by: Simo Sorce <[email protected]>
In the ECDH case import involves these steps:

- create new empty key
- set EC domain parameters, by copying them from the server key
- set EC Public key from the peer

The previous code was messing the second step by completely copying the
server public key if one was found on the system.

This is because both on export and import we did not look at the selection
and always exported/imported both parameters and the public key.

As we now retain the public key object we generate with keygen, instead
of discarding it, the export function would export also the public point
and the import would find the server public key cached and import it
fully in the empty shell reservd for the public key.

Later setting the peer public key would then fail as the object was
considered a complete key already.

Signed-off-by: Simo Sorce <[email protected]>
@simo5
Copy link
Member Author

simo5 commented Feb 6, 2025

Thanks for the review @Jakuje

@simo5 simo5 merged commit 5b4ef96 into latchset:main Feb 6, 2025
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
covscan-ok Coverity scan passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EVP_PKEY_verify doesn't work with generated EVP_PKEY
2 participants