signer: move spx keygen, sign and verify API #568
Conversation
This was previously part of the `securesystemslib.keys` interface. To prepare for deprecation of this interface, and use of the new signer API as replacement, spx-related functionality and test are moved. This is a backwards incompatible change and a slight feature degradation: - spx is now no longer available via the keys module, and thus no longer available SSlibSigner/SSlibKey (i.e. the legacy API bridge). Given that spx was added recently, support via the legacy API does not seem necessary. The advantage is more modular code and less case-handling, also wrt different optional dependencies. - Currently, SpxSigner does not implement from_priv_key_uri, which was previously available via SSlibSigner, and supported loading signers from file or envvar. Support may be added back later. - The default keyid is computed differently than before (see 6c29cae) See usage example in docstring. Signed-off-by: Lukas Puehringer <[email protected]>
|
cc @rugo |
| """Generate new SPHINCS+ key pair and return as SpxSigner. | ||
|
|
||
| NOTE: The Signer API is still experimental and key generation in | ||
| particular (see #466). |
There was a problem hiding this comment.
@jku: I'm still not sure what's most useful here. This method is a bit of a compromise between a generate_raw, which only returns private and public bytes and requires additional tooling to be usable by the signer; and the full set of generate_file, import_file, generate_envvar, import_envvar methods plus corresponding import_from_priv_key_uri as you suggest in #466 (comment), which seemed like a lot of work and unclear to deduplicate from SSlibSigner.
Any opinions?
There was a problem hiding this comment.
Okay, I changed my mind here. SpxSigner.new_ neither makes a lot of sense for the intended usage pattern of the Signer API, nor is it very useful to create tools on top (see details in c0671c4 commit message).
* Removes previously added SpxSigner.new_ key pair generation method. This method may be convenient to quickly test spx signing, but does not fit well into the signer API usage pattern, where key generation; public key and signer URI loading; and signer loading (from URI) plus signing are separate workflows. * Adds function to generate and return raw key pair bytes. * This function is "syntactic sugar" (quote @jku) over the PySPX library. * It is added here to provide consistent error handling, if the optional library is not available. * Otherwise, it needs additional tooling to fit well into the signer API, e.g. an interface that stores the created private bytes to a place from where they can be loaded using Signer.from_priv_key_uri. * Adds helper factory method to create an SpxKey instance from raw bytes. * Adopts changes in usage example and tests.
An alternative design had RSAKey, ECDSAKey and ED25510Key classes, but that mostly added redundant boilerplate code. To the user it shouldn't matter, so let's do what makes sense from maintainer perspective. TODO: - includes on mv-spx (secure-systems-lab#568) - maybe polish _load_args() - using *args makes this hard to debug - schemes should not be hardcoded, but defined as sslib.signer-wide constants - args per scheme don't need to be hardcoded, but could be computed based on the scheme string Signed-off-by: Lukas Puehringer <[email protected]>
Signature verification for "securesystemslib keys" was previously implemented in 'rsa_keys', 'ecdsa_keys' and 'ed25519_keys' modules, which were called from `SSlibKey.verify_signature` via the legacy interface function `keys.verify_signature()`. This commit moves the entire implementation to SSlibKey, drastically decreasing LOC count and dropping 'nacl' optional dependency for ed25519 keys, in favour of 'pyca/cryptography', which we already use for all other sslib keys. An alternative design had RSAKey, ECDSAKey and ED25510Key classes, but that mostly added redundant boilerplate code. To the user it shouldn't matter, so let's do what makes sense from maintainer perspective. TODO: - includes on mv-spx (secure-systems-lab#568) - maybe polish _load_args() - using *args makes this hard to debug - schemes should not be hardcoded, but defined as sslib.signer-wide constants - args per scheme don't need to be hardcoded, but could be computed based on the scheme string Signed-off-by: Lukas Puehringer <[email protected]>
There was a problem hiding this comment.
I think this looks good and is probably the right way to go about it.
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?
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 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? |
|
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
It's entirely possible that being less rigid about the private key URI meaning would be less work... |
|
Let's continue this conversation in #466 |
This was previously part of the
securesystemslib.keysinterface. To prepare for deprecation of this interface, and use of the new signer API as replacement, spx-related functionality and tests are moved.This is a backwards incompatible change and a slight feature degradation:
spx is now no longer available via the keys module, and thus no longer available via SSlibSigner/SSlibKey (i.e. the legacy API bridge). Given that spx was added recently, support via the legacy API does not seem necessary. The advantage is more modular code and less case-handling, also wrt different optional dependencies.
Currently, SpxSigner does not implement from_priv_key_uri, which was previously available via SSlibSigner, and supported loading signers from file or envvar. Support may be added back later.
The default keyid is computed differently than before (see 6c29cae)
See usage example in docstring.