-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add backwards-compatible SCHEMA replacement #665
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These helpers cover all data validation cases, where we currently use securesystemslib's schema checker, and are intended as a backwards-compatible replacement. This is part of a series of patches to prepare for removal of legacy securesystemslib interfaces. Signed-off-by: Lukas Puehringer <[email protected]>
Reasons: consistency and re-use. Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
lukpueh
force-pushed
the
refactor-schema
branch
from
December 4, 2023 13:51
d31a2e6
to
e7e9db9
Compare
adityasaky
approved these changes
Dec 4, 2023
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.
LGTM!
Speaking of nice side-effect and flexibility. Here's me "registering" sigstore in user code and generating an in-toto link signed with my email address, with 0 changes in the library code. 😎 from securesystemslib.signer import (
KEY_FOR_TYPE_AND_SCHEME,
SIGNER_FOR_URI_SCHEME,
Signer,
SigstoreKey,
SigstoreSigner,
)
from in_toto.runlib import in_toto_run
# All what's needed for in-toto to speak sigstore
KEY_FOR_TYPE_AND_SCHEME[("sigstore-oidc", "Fulcio")] = SigstoreKey
# This is only needed to use generic `Signer.from_priv_key_uri` interface.
# We could also load SigstoreSigner directly.
SIGNER_FOR_URI_SCHEME[SigstoreSigner.SCHEME] = SigstoreSigner
uri, key = SigstoreSigner.import_via_auth()
signer = Signer.from_priv_key_uri(uri, key)
link = in_toto_run("hello-sigstore", [], [], ["echo", "hello", "sigstore"], signer=signer)
# Hack to convert a `Key` to the in-toto -flavoured dict representation
key_dict = key.to_dict()
key_dict["keyid"] = key.keyid
link.verify_signature(key_dict) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds helper functions to cover all data validation cases in a backwards-compatible manner, where we previously used securesystemslib SCHEMA.
This is part of a series of patches to prepare for removal of legacy securesystemslib interfaces. (see secure-systems-lab/securesystemslib#183 for specific issues with SCHEMA)
Details
Most of the checks are simple type validations, now without the hidden redundancy of SCHEMA. (e.g.
NAME_SCHEMA
,PATH_SCHEMA
, orANY_STRING_SCHEMA
all just checked if the input was a string, now there is only_check_str
)A few checks validate more complex data structures, most notably public key dictionaries. While SCHEMA whitelisted allowed fields and their values, we now just try to deserialise using securesystemslib's Signer API, and re-raise a deserialisation error as format error.
This is more Pythonic (if we can parse it as key, it is a key) and scales nicely, because the Signer API allows to register new public key schemes in user code.
Alternative / future work
This PR is a hack with the goal to remove schema in securesystemslibs without breaking the API in in-toto. It also has the nice side-effect of making validation more explicit and flexible.
A more sustainable solution can be found in the python-tuf Metadata API, where
However, this would be a more invasive and surely breaking change. So, even though this PR adds a lot of code, I think we should go with it for now, and revise the class model later.
Reviewing commit by commit should make the review straight-forward.