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

CryptoSigner: support init from PrivateKeyTypes #675

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Nov 15, 2023

closes #664 (see #664 (comment))

Previously CryptoSigner instances could only be created via
from_priv_key_uri factory method, because:

  • CryptoSigner was abstract
  • Subclass implmentations were protected

This change makes the CryptoSigner constructor usable directly, so that
signers can be created directly from pyca/cryptography private key
objects.

If a public key is not passed along, it is created from the private key,
with default keyid and signing scheme for key type.

Details

  • Internally, this now (1) uses composition instead of inheritance, and (2)
    eliminates one level of abstraction:

    (1) CryptoSigner is no longer an abstract base class for protected
    rsa, ecdsa and ed25519 "signer" subclasses. It only made creation more
    difficult, without advantage. Instead CryptoSigner holds references to
    the "signers"

    (2) There is no more abstraction over individual pyca/cryptography rsa,
    ecdsa and ed25519 private key types. cryptography provides a common
    "PrivateKeyTypes" interface with sign method, which we can use directly.
    The different additional sign args for the different key types are
    managed in new internal dataclasses.

  • Otherwise code is mostly moved around:

    • _get_rsa_padding and _get_hash_algorithm helpers are moved unchanged to module scope (these might move out altogether with signer: deduplicate signing scheme dissection  #594)

    • from_securesystemslib_key can now pretty much directly use the constructor, thus no longer requires the from_pem helper

Previously CryptoSigner instances could only be created via
`from_priv_key_uri` factory method, because:
- CryptoSigner was abstract
- Subclass implmentations were protected

This change makes the CryptoSigner constructor usable directly, so that
signers can be created directly from pyca/cryptography private key
objects.

If a public key is not passed along, it is created from the private key,
with default keyid and signing scheme for key type.

*Details*

Internally, this now (1) uses composition instead of inheritance, and (2)
eliminates one level of abstraction:

(1) CryptoSigner is no longer an abstract base class for protected
rsa, ecdsa and ed25519 "signer" subclasses. It only made creation more
difficult, without advantage. Instead CryptoSigner holds references to
the "signers"

(2) There is no more abstraction over individual pyca/cryptography rsa,
ecdsa and ed25519 private key types. cryptography provides a common
"PrivateKeyTypes" interface with sign method, which we can use directly.
The different additional sign args for the different key types are
managed in new internal data classes.

Signed-off-by: Lukas Puehringer <[email protected]>
lukpueh added a commit to lukpueh/in-toto that referenced this pull request Nov 20, 2023
lukpueh added a commit to lukpueh/in-toto that referenced this pull request Nov 20, 2023
lukpueh added a commit to lukpueh/in-toto that referenced this pull request Nov 20, 2023
lukpueh added a commit to lukpueh/in-toto that referenced this pull request Nov 20, 2023
lukpueh added a commit to lukpueh/in-toto that referenced this pull request Nov 21, 2023
Copy link
Collaborator

@jku jku left a comment

Choose a reason for hiding this comment

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

Looks good to me, I find this quite readable now.

securesystemslib/signer/_crypto_signer.py Show resolved Hide resolved


class CryptoSigner(Signer):
"""PYCA/cryptography Signer implementations."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

a bit of detail on how to get an instance of CryptoSigner could be useful. I guess something like:

  • if you have a privkeyuri for a private key stored in a file, use the generic Signer.from_priv_key_uri()
  • if you just want to create a completely new private key, use CryptoSigner.generate_*() (or CryptoSigner() for even more control over key details)
  • if you want to use an existing cryptography private key, use CryptoSigner()

@lukpueh lukpueh merged commit acb84c5 into secure-systems-lab:main Nov 22, 2023
17 checks passed
lukpueh added a commit to lukpueh/in-toto that referenced this pull request Nov 24, 2023
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.

How should I load signers for immediate signing, e.g. in CLI?
2 participants