-
Notifications
You must be signed in to change notification settings - Fork 275
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
Move (most of) Key to Securesystemslib #2165
Conversation
Unsure what to do with the documentation: I don't want to reproduce the docs in python-tuf... but I don't know if we can import the Key docs from securesystemslib? |
I think that's a good idea. It's what we for in-toto's key utilities: |
oh nice: we should be doing that for Signer and Signature as well -- basically for the "modern" securesystemslib api. it's interesting how that autodoc magic seems to work without ever importing securesystemslib... |
this is now out of date since I modified the securesystemslib PR -- it's still roughly correct, but I am not sure if a python-tuf Key is even needed anymore, maybe not. Will update next week |
Pull Request Test Coverage Report for Build 3939714721Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
tox.ini
Outdated
@@ -31,7 +31,7 @@ install_command = python3 -m pip install {opts} {packages} | |||
# Must to be invoked explicitly with, e.g. `tox -e with-sslib-master` | |||
[testenv:with-sslib-master] | |||
commands_pre = | |||
python3 -m pip install git+https://github.com/secure-systems-lab/securesystemslib.git@master#egg=securesystemslib[crypto,pynacl] | |||
python3 -m pip install --force-reinstall git+https://github.com/jku/securesystemslib.git@privkeyuri#egg=securesystemslib[crypto,pynacl] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is testing leftover -- but it does lead to one passing test 🍾
Just tried out signing with post quantum keys (sphincs+) using the new signer API. Works like a charm and out of the box! 🎉 cc @rugo see snippet# Install `tuf` from this branch and `securesystemslib` from secure-systems-lab/securesystemslib#456
python3 -m pip install git+https://github.com/jku/python-tuf.git@no-key#egg=tuf
python3 -m pip install --force-reinstall git+https://github.com/jku/securesystemslib.git@privkeyuri#egg=securesystemslib[crypto,pynacl,PySPX] POC: Verify that root metadata is signed by at least one sphincs+ key as required import os
from securesystemslib.keys import generate_sphincs_key
from securesystemslib.signer import Signer, SSlibKey, SSlibSigner
from tuf.api.metadata import Metadata, Root
# Generate pq keys
pq_key = generate_sphincs_key()
pq_pubkey = SSlibKey.from_securesystemslib_key(pq_key)
pq_signer = SSlibSigner(pq_key)
# # Generic solution: Use `Signer.from_priv_key_uri` instead of `SSlibSigner`,
# # this also works:
#
# os.environ["PQ_KEY"] = pq_key["keyval"]["private"]
# pq_signer = Signer.from_priv_key_uri("envvar:PQ_KEY", pq_pubkey)
# Generate root role and configure signature requirement
root = Root()
root.add_key(pq_pubkey, Root.type)
# Wrap root in metadata envelope and add signature
root_metadata = Metadata(root)
root_metadata.sign(pq_signer)
# Assert `verify_delegate` indeed needs at least one sig from above key
assert root_metadata.signed.roles["root"].threshold == 1
assert root_metadata.signed.roles["root"].keyids == [pq_pubkey.keyid]
root_metadata.verify_delegate(Root.type, root_metadata) |
Current state:
|
Ok, PR is ready for review I think -- not for merge though, we do need a securesystemslib release before that. Updated description |
Rebased |
If the sslib release version matches, pip does not install the version from git because the same version is already installed. Force the install. Signed-off-by: Jussi Kukkonen <[email protected]>
tuf.exceptions should IMO be seen as the "default exception source". Signed-off-by: Jussi Kukkonen <[email protected]>
Key has been moved to Securesystemslib: use it from there. This still fails tests as Key API has changed a bit: issues are fixed in followup commits. Signed-off-by: Jussi Kukkonen <[email protected]>
New Securesystemslib Keys can now be instantiated in two ways: * deserialize via Key.from_dict() as before * generate new keys via implementation specific methods Fix all cases where we call Key() or Key.from_securesystemslib_key() and use SSlibKey methods instead. Fix related tests. Signed-off-by: Jussi Kukkonen <[email protected]>
Key.verify_signature() API has changed: * argument is bytes, not metadata * raised error now comes from securesystemslib Signed-off-by: Jussi Kukkonen <[email protected]>
verify_delegate() unfortunately needs an almost complete rewrite as the Key.verify_signature() API change affects it quite a bit. Refactoring the role and key lookup into a separate method makes the code readable again. Signed-off-by: Jussi Kukkonen <[email protected]>
Signed-off-by: Jussi Kukkonen <[email protected]>
Simplify the lookup of delegated keys and roles by moving it to Targets and Root: this follows the examples set by add_key() and remove_key(). Most of the methods are trivial but they make sense because this way the calling code does not have to care if the object is a Targets or a Root: the same methods work on both. The new methods are public since they are useful to applications as well. Signed-off-by: Jussi Kukkonen <[email protected]>
This method is now in SSlibKey Signed-off-by: Jussi Kukkonen <[email protected]>
Rebased and then added
|
We want to document some securesystemslib classes (Key gets documented with this change already as it's part of the metadata API). Signed-off-by: Jussi Kukkonen <[email protected]>
TODO: Package should depend on securesystemslib >= 0.26 now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Package should depend on securesystemslib >= 0.26 now
I think this is the only thing that's missing.
Otherwise LGTM. 👍
We need signer.Key which was added in 0.26. Signed-off-by: Jussi Kukkonen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This updates python-tuf sources for latest securesystemslib signer api changes. Most of the changes are in tests (since we test Key quite extensively) but verify_delegate() is also significantly rewritten. The commits are mostly logical -- the one exception is that the original refactoring of role/key lookup in Metadata was not great: it is fixed in a later commit by moving the functionality from Metadata to Targets and Root where it makes more sense.
Changelist:
Key.from_securesystemslib_key()
is nowSSlibKey.from_securesystemslib_key()
Key.verify_signature()
arguments and exceptions change (it used to take Metadata as arg)Metadata.verify_delegate()
has to be rewritten because ofKey.verify_signature()
changesget_delegated_role()
andget_key()
: these simplify the role and key lookup inMetadata.verify_delegate()
WRT API change: