Design key import/generation for the signer API

[jku]

With #456 the signer API comes a lot closer to being a "fully featured API". The missing part is key generation and import (and maybe storing private keys)

Related issues: #446, #408, #280, #451 (and probably more)

Current status (after #456):

• Using signers and keys is possible without knowing the signer or key implementation, and without using the "old" API
• except to create keys this sort of code is typically used:

  key = keys.generate_ed25519_key() # key is a dictionary. keys is a module we would like to start hiding from users
  pubkey = SSlibKey.from_securesystemslib_key(key) # pubkey is an implementation of Key
  signer = Signer.from_priv_key_uri("file:path/to/file?encrypted=false", pubkey) # signer is an implementation of Signer
  
  

• the key dictionary is only used as an intermediate step

Proposal

• Signer implementations should provide implementation specific methods to create new keys or import them from other systems
• these methods may have whatever arguments they need
• Signer should expose the public key so callers can access it
• typical usage:

  # call implementation specific API (in this case SSlibsigner to generate a ed25519 key)
  signer = SSlibSigner.new_ed25519(...) 
  pubkey = signer.public_key
  

• this hides the keys module from users, and makes SSlibKey.from_securesystemslib_key unneeded
• AFAICT this should work for KMS, HSM, GPG, ...
• The implementation specific method could live elsewhere as well, but it seemed like it would make most sense in signer.

---

The part I have not 100% figured out is private key storage: how and when do you store the private key content in a way that you can then use the from_priv_key_uri() to load it again. This is somewhat different depending on the key type and in many cases does not even make sense (you can't really store environment variables)... possibly it should happen during the constructor, so SSlibSigner.new_ed25519() could actually take a priv_key_uri argument and and the generated key would immediately get stored ?

[jku]

As background for why the public_key should be a Signer property, and why I think import/generate-methods should be Signer implementation specific constructors:

In real-world scenarios, if you generate/import a Key, you always want to "connect" that to a specific signer, or at least the private key uri: you never want to just generate a Key because then how would you know how to sign for that Key? Somehow you need to be able to connect a signer private key uri to a keyid: we can leave that to the user of course but if both are properties of the Signer, that gets solved neatly

As an example if you import a public key from a Yubikey, you then always want to store configuration like
{"abcd": "hsm:yubikeylabelhere"} where abcd is the keyid of the public key, because otherwise the application would never know how to sign for abcd (or even that it should sign for abcd).

Constructing a Signer is not the only solution but I think it makes sense. Something like this

# import signer and key from a yubikey
signer = Signer.import("hsm:")
# store the whole public key in metadata, and private key access details
# (private key uri, keyid, maybe other things) in application configuration
repo.set_targets_key(signer.public_key)
configuration.save_signer(signer)

[jku]

So I guess the signer implementation specific APIs look roughly like this -- probably no signer is going to implement all three types of methods (and some signers might implement multiple versions of some methods):

Possible implementation specific methods:

• import(...): get everything, including public key, from "keyvault": for GCP and HSM at least. arguments should be data that is enough to construct a private key uri. Examples:
  • GCPSigner.import(gcp_key_id: str) -> GCPSigner
  • HSMSigner.import() -> HSMSigner (optional args like token_label may get added later)
• generate(...): construct new keys: this applies to e.g. SSlibSigner but not GCP or HSM. This likely needs not just the key parameters, but enough info to know how to generate a private key uri. Possible examples
  • SSlibSigner.generate_rsa(uri: str, bits: int, scheme: str) -> SSlibSSigner
  • SSlibSigner.generate_ecdsa(uri: str, bits: int) -> SSlibSSigner
• write() or secret() for storing/accessing key content: for those signers that actually want to write the private key material somewhere -- either a file or just the terminal output: currently this is just SSlibSigner with file: and envvar:
  • SSlibSigner.write() -- if uri scheme is "file:", then this writes the file
  • SSlibSigner.secret() -> str -- returns the private key content (useful for "envvar:" scheme)

Common properties I think Signer interface should require:

• priv_key_uri: str -- needed by application to when it wants to store the private key access details
• public_key: Key -- for public key import, for storing private key access details, but also for signing in some cases

[jku]

The major question I have is WRT import(...) functionality: the proposal in previous comment makes it implementation specific functionality just like generate(...). There is a difference though: in some (many?) cases import(priv_key_uri: str) would be feasible -- meaning if we make it the callers job to create the URI, we could have a common API in Signer, not just implementation specific ones. Unfortunately that likely wouldn't cover all cases.

A minor question is on SSlibSigner: the generate examples show how it gets a bit cumbersome: we have multiple signing schemes and multiple storage models in the same class so the constructor needs to handle a lot of cases...

[jku]

Making note of a comment from Lukas:

• if import() returns a Signer, it would need a SecretsHandler
• import() could just return a Tuple[Key, str] where the string is a private key uri: this could be used to either persist these or to construct a Signer

[jku]

In the spirit of that finding:

• maybe generate() should not return a Signer either: it would just create the file (or return the private key material in envvar case) and return the same tuple: public key and private key uri. This would mean the output methods ("write" or "secret" would not be needed) which sounds good

[jku]

maybe generate() should not return a Signer either

I've tried this in a limited manner for SSlibKey in repository-editor-for-tuf and I think it works: it won't be pretty if you support all the different schemes and keytypes but that can't be avoided. For me the only case I'm not sure about is import() for SSlibKey -- does it even make sense, is it important to hash out now, and what would it look like?

Maybe something like this:

SSlibSigner.import_envvar(sslib_key_dict: Dict) -> Tuple[str, str, Key]
# returns
# * the environment variable string to use: "SSLIB_KEY_abdcf1234=dcba09876"
# * private key uri: "envvar:SSLIB_KEY_abdcf1234"
# * the public key

SSlibSigner.import_file(sslib_key_dict: Dict, priv_key_path: str, passphrase: Optional[str]) -> Tuple[str, str, Key]
# Returns
# * secret (possibly encrypted)
# * private key uri: "file:path/to/secret"
# * the public key

Note that neither of them actually stores the secret anywhere, just returns it: this is because we cannot know if the secret is actually meant to be used on this machine or not: writing to disk does not make sense if it's not

[lukpueh]

New PoC for import_ idea:

securesystemslib/securesystemslib/signer/_gpg_signer.py

Lines 274 to 294 in c95e3c7

	def import_(cls, keyid: str, homedir=None) -> Tuple[str, Key]:
	"""Load key and signer details from GnuPG keyring
	Args:
	keyid: GnuPG local user signing key id.
	homedir: GnuPG home directory path. If not passed, the default homedir is
	used.
	Returns:
	Tuple of private key uri and the public key.
	"""
	uri = f"{cls.SCHEME}:{homedir or ''}{'?key=' + keyid}"
	public_key = (
	GPGKey._from_legacy_dict( # pylint: disable=protected-access
	gpg.export_pubkey(keyid, homedir)
	)
	)
	return (uri, public_key)

Returning signer URI and public key does feel ergonomic.

[lukpueh]

For me the only case I'm not sure about is import() for SSlibKey -- does it even make sense, is it important to hash out now, and what would it look like?

Also not sure about this. Import functions that take an sslib_key_dict don't seem that useful. Where does it come from, and what does it or its location at import time say about the location of the private key at signing time?

For an SSlibSigner the location of public key and location of private key, as well as location at import time and location at signing time are more independent from each other, compared to HSM, GPG, GCP signers.

[jku]

I'm convinced this is close to a good solution. "Signing system initialization" really does mean 3 separate things:

1. setting up the actual secret storage
2. defining "signing config" for later use: which signing mechanism to use, how to find secret storage?
3. publishing the public key

We don't always have to (and can't) handle all three on our code but all three need to happen before signing is useful. This is why the import process either has 3 return values or gets something as given (in HSM and and KMS cases the secrets storage has been setup in an external process, and we get public key from there).

I guess the above begs the question: would import make more sense for SSlibSigner if we don't try to cram key creation in there -- then the API would look more like the other signers. Essentially, should there be a SSlibPrivateKey (that would handle the key generation and would be the SSlibSigner import argument)? This way no signer would try to "generate keys", they are always imported?

For an SSlibSigner the location of public key and location of private key, as well as location at import time and location at signing time are more independent from each other, compared to HSM, GPG, GCP signers.

I think thinking of signing time and import time as independent is very useful for others too: in the GCP case for example, I don't have signer permissions on my personal account (which I authenticate with to add a GCP key to metadata). So the situation is 100% the same: code knows where "secret storage" is going to be later but can't actually expect it is available right now.

[jku]

I'll reiterate that I think import/generate are internal details of the specific signing system, and that SSlibSigner is not even that interesting now that we have HSM and KMS... but to ensure that SSlibSigner can be implemented in a somewhat reasonable way, here's a proposal for that:

class SSlibSigner():
  @classmethod
  def generate_file(cls, scheme: str, signing_time_priv_key_path: str, encrypted: bool) -> Tuple[str, Key, bytes]
  @classmethod
  def import_file(cls, key_dict: Dict, signing_time_priv_key_path: str, encrypted: bool) -> Tuple[str, Key, bytes]
  # Returns:
  # * signer uri containing the signing time path and encrypted status
  # * public key
  # * the private key bytes that should be written to signing time location

  @classmethod
  def generate_envvar(cls, scheme: str) -> Tuple[str, Key, str]
  @classmethod
  def import_envvar(cls, key_dict: Dict) -> Tuple[str, Key, str]
  # Returns:
  # * signer uri containing the variable name
  # * public key
  # * the environment variable declaration ("PRIVATE_KEY_abcdef=0a1b2c3d") 

Some comments:

• maybe we should have had EnvVarSigner and FileSigner to begin with, instead of a SSlibSigner -- it would be clearer
• methods for SSlibSigner (or EnvVarSigner and FileSigner) likely should have more optional args (like keysize) but I left them out for clarity for now
• there's likely no need to make public_key or private_key_uri required Signer properties (since they are available as import() return values)
• In general, there's no need for write() or secret() -- import/generate return values contain the same data, and storing the data is an application issue
• I would also say even generate() is just syntactic sugar: private key creation could be handled somewhere else and keys just imported -- but if generate() makes SSlibSigner nicer, I have no objections. I would suggest we add only import for now, and use the existing key creation methods.

[lukpueh]

Following is just related thinking out loud:

It's a little worrying that we don't have a good story for how to handle e.g. keys in files in a general way but that's not this PRs fault... maybe Signer.from_priv_key_uri() actually could select the signer implementation also based on the public key -- it seem like in some cases that would be the smart thing. So in case of "file:" URIs, it would do another lookup based on the keytype to find out if it should use SPXSigner or something else?

Originally posted by @jku in #568 (review)

[lukpueh]

It's a little worrying that we don't have a good story for how to handle e.g. keys in files in a general way but that's not this PRs fault... maybe Signer.from_priv_key_uri() actually could select the signer implementation also based on the public key -- it seem like in some cases that would be the smart thing. So in case of "file:" URIs, it would do another lookup based on the keytype to find out if it should use SPXSigner or something else?

I've also thought about this, but probably in a different way, although I am not sure I understand your proposal.

In my mind, the goal is to make the SSlibSigner.from_priv_key_uri implementation re-usable, for e.g. SpxSigner. A simple idea would be to move that method to something like a _FileAndEnvvarTransportMixin, which inherits to SSlibSigner and SpxSigner. In the dispatch table we would then register for transport and key type, e.g. SpxSigner for file+spx and envvar+spx; and SSlibSigner for file+sslib and envvar+sslib or simply file and envvar.

This design is not very flexible, that is a signer won't be able to use a custom and a re-usable transport at the same time. But that could surely be solved. The big question is, is it worth the effort / do we foresee a lot of demand for re-usable transports?

Originally posted by @lukpueh in #568 (comment)

[lukpueh]

Yeah I think we're talking about roughly the same thing. The core issues are A) dispatch and B) signer construction: I was trying to say

• adding keytypes to the URI feels wrong: URI is supposed to answer the question "how do I access the private key corresponding to this public key". Putting the public keytype in the uri just feels plain incorrect -- maybe this is me being too tied to the original idea but that is where I'm coming from: the URI was originally meant to define the private key storage mechanism
• the keytype (of the public key) has to be taken into account in the signer dispatch though
• The construction of a new signers/keys (and the potential storage of private key material that requires specific API) of course has to take keytype somehow into account as well
• a couple of options I can see:
  • Signer.from_priv_key() handles the "public key based dispatch" too. The shared implementations of e.g. a file-based-private-key could be in a Mixin... The thing to note here is that those Mixins are not fully internal: they form a part of the public API (API to e.g. store the private key in a file, or to get the envvar content)
  • signers like SpxSigner don't implement a from_priv_key(): new mid-level classes are made that handle "IO", e.g. FileSigner. Then FileSigner is the one that is dispatched to from Signer, FileSigner has an internal dispatch list of keytype-to-actual-signer, FileSigner has custom API to store the private key bytes in an encrypted file, and knows how to read and provide those bytes to the actual implementation like SpxSigner when it constructs one.

It's entirely possible that being less rigid about the private key URI meaning would be less work...

Originally posted by @jku in #568 (comment)