Skip to content

Add PrivateKey object for signing, only - #111

Closed
mandelsoft wants to merge 2 commits into
eclipse-keypont:masterfrom
mandelsoft:master
Closed

Add PrivateKey object for signing, only#111
mandelsoft wants to merge 2 commits into
eclipse-keypont:masterfrom
mandelsoft:master

Conversation

@mandelsoft

@mandelsoft mandelsoft commented Nov 30, 2024

Copy link
Copy Markdown

Proposed Changes

Fully implementing rsa.Signer requires a private and public key. But if only signing is required, the private key would be
sufficient. Currently, the public key must be present and both must have the same Id. If one of those constraints is not satisfied signing with the private key is not possible.

Types of Changes

New Feature: introduce interface PrivateKey with sole signing method and provide appropriate context methods to find private keys.

Verification

Create keys without Id and use new FindPrivateKey method flavors.

Testing

Tests are added.

Linked Issues

User-Facing Change

Support finding private keys without matching public key usable for sole signing operation.

Further Comments

@IceManGreen

Copy link
Copy Markdown
Contributor

Thank you @mandelsoft for your proposition, this is indeed a better approach for private keys in KMS.
Also, this pull request might be the solution to #112.

@solcates solcates 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.

Looks good and safe to me. Thanks for the contribution.

@IceManGreen

Copy link
Copy Markdown
Contributor

However, the code is still in addendum.go. The implementation should be in keys.go though.

@mandelsoft can you fix this ?

Nicolas-Peiffer added a commit that referenced this pull request Aug 4, 2026
The four FindPrivateKey* methods have had no direct test coverage since
they landed in #115 — the only exercise was one FindRSAPrivateKey subtest
in rsa_test.go, which touches neither attribute matching nor argument
validation.

Port the tests proposed in PR #111, adapted to this
branch's cryptoki binding:

  - TestFindKeysRequiresIdOrLabel gains assertions that FindPrivateKey
    and FindPrivateKeys reject a nil id *and* nil label.
  - TestFindingPrivateKeysWithAttributes covers matching on CkaLabel
    (single and shared) and filtering on CkaKeyType.

Also add TestFindingPrivateKeyNotFound, which is not from #111. It pins
the current not-found behaviour, where the singular lookups return an
error while the plural ones return an empty slice. Note that this
contradicts the doc comments on FindPrivateKey and
FindPrivateKeyWithAttributes, which still promise "or nil if it cannot
be found"; the test documents what the code does today rather than
asserting it is correct.

The tests are unchanged in intent from mandelsoft's original, which
proposed them alongside a PrivateKey implementation that #115 has since
superseded.

#111

Co-Authored-By: Uwe Krueger <553075+mandelsoft@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Nicolas-Peiffer

Copy link
Copy Markdown
Contributor

Thanks for this, @mandelsoft — and apologies it sat open so long.


In 2026 the big project for crypto11 is to add the support of PKCS#11 v3.2 and the support of the ML-KEM PQC algorithm:

The other big project for 2026 is giving + moving the crypto11 project under the Eclipse Foundation.

So this is an opportunity to look back at PR and issues, like yours.


Closing #111 as superseded by #115: crypto11@v1.6.x now carries this feature. #115 ("RSA support for asymmetric decryption", commit c3b42eb) needed private-key retrieval for its decryption path and landed the same design you proposed here — the PrivateKey interface (Sign + KeyType() uint) and all four lookups:

It also adds RSAPrivateKey for Sign + Decrypt on a private half alone.

Two differences from your version, in the interest of full disclosure:

  1. Not-found: yours returns (nil, nil); master returns an error. If you were relying on the nil-nil signal, say so — arguably master has this wrong, since its doc comment still promises "or nil if it cannot be found".
  2. Tests: your TestFindingPrivateKeysWithAttributes and the FindPrivateKey(nil, nil) argument-validation assertions have no equivalent on master. Those are still worth having — happy to cherry-pick them with attribution, or send them as a small PR if you'd prefer to keep the credit.

So we added your tests on 62bbca8 and we mention you as contributor. This targets crypto11@v2

The implementation is redundant now, but you framed the problem first and the result matches your proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants