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

signer: Improved the "file-based signer" #617

Closed
lukpueh opened this issue Aug 11, 2023 · 5 comments
Closed

signer: Improved the "file-based signer" #617

lukpueh opened this issue Aug 11, 2023 · 5 comments
Assignees
Labels
contribfest Issues for KubeCon EU contribfest

Comments

@lukpueh
Copy link
Member

lukpueh commented Aug 11, 2023

Originally posted by @jku in #604 (comment)

NOTE: Adopt in docs/CRYPTO_SIGNER.md (remove signer registration)

@jku
Copy link
Collaborator

jku commented Mar 14, 2024

I no longer remember if CryptoSigner is the best place, but we should have a nice file-based signer (a signer that reads private keys from a file) in securesystemslib:

  • securesystemslib examples really need a working file signer
  • RSTUF has a file signer that works for them: we should move it somehow to securesystemslib
  • the main "feature" in RSTUF is a environment variable that defines the "base directory" where the private keys are searched for: this matches e.g. the various KMS keys pretty well
  • we should make it clear we don't recommend storing private keys on disk: this is mostly for examples and testing

TODO:

  • Add links to RSTUF implementation here
  • decide on correct place -- is CryptoSigner good or should we add a new signer?

@jku jku changed the title signer: make CryptoSigner the default file uri signer signer: Improved the "file-based signer" Mar 14, 2024
@jku jku added the contribfest Issues for KubeCon EU contribfest label Mar 14, 2024
@lukpueh
Copy link
Member Author

lukpueh commented Mar 14, 2024

An existing FileNameSigner with usage context can be found in RSTUF.

The implementation can be copy-pasted to securesystemslib with little change:

  • the environment variable name ONLINE_KEY_DIR is rather RSTUF-specific, and should be more generic in securesystemslib
  • as Jussi mentions above, a bit of discussion is left as to how FileNameSigner relates to CryptoSigner

@lukpueh
Copy link
Member Author

lukpueh commented Mar 14, 2024

how FileNameSigner relates to CryptoSigner

Currently, FileNameSigner subclasses CryptoSigner to make use of its sign, but add its own from_priv_key_uri. This makes sense for CryptoSigner, which is mainly characterised by its sign implementation, but could implement several different ways of accessing the private key material, e.g. from file path (CryptoSigner, SSlibSigner), file name (FileNameSigner), environment variable (SSlibSigner), etc.

The signer API, however, is not designed to separate the two tasks and all other signers, so far, are clearly characterised by both their sign and from_priv_key_uri methods together.

For the sake of a simple consistent interface, and given that file-based signing should be an edge case, I suggest we keep CryptoSigner only and update its from_priv_key_uri to behave like FileNameSigner. This still allows applications to subclass CryptoSigner for its sign method, and implement their own from_priv_key_uri.

@jku jku self-assigned this Mar 22, 2024
@jku
Copy link
Collaborator

jku commented Mar 22, 2024

this might not be ready today but I'll handle it

@lukpueh
Copy link
Member Author

lukpueh commented Apr 19, 2024

Addressed in #759

@lukpueh lukpueh closed this as completed Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribfest Issues for KubeCon EU contribfest
Projects
None yet
Development

No branches or pull requests

2 participants