-
Notifications
You must be signed in to change notification settings - Fork 56
Use trust config everywhere #1363
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
Draft
jku
wants to merge
21
commits into
sigstore:main
Choose a base branch
from
jku:use-trust-config-everywhere
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+1,030
−887
Conversation
This file contains hidden or 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
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Verifier only needs a Rekor client for the detached materials hack in _cli... and it should not be using SigningConfig to get it. * This is an API change for Verifier: I believe the proposed API should be stable now * RekorClient definitely needs more work: I'm just punting the can down the road here. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
* Let's forget that v0.1 ever existed (it was not really used): We could try to support both but since 0.1 does not really work, I won't bother * Support signing config v0.2 in a minimal way (see note on selectors below) Things that could be improved: * Rekor client is still a bit of a hack: that area likely needs a redesign * The "service selectors" in SigningConfig are not all yet supported: Only the ANY selector works (this is the one staging will use soon) * The CLI does not yet use the OIDC provider specified in SigningConfig (this should be a small refactor) Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
This is not super useful at this point as the TUF repositories do not have the required signing config yet so we can't simplify the code yet: The goal is still for trustconfig to be the only source configuration like the OIDC URL. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
We have previously done this for TrustedRoot but doing this for the whole TrustConfig makes sense. The only complication is that production instance does not have the SigningConfig component yet so we need to provide a fallback for that. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
This change makes almost all code paths now use TrustConfig to choose the sigstore instance (urls, keys, validity periods, etc). * OIDC url now comes from signingconfig too * Some production()/staging() methods remain because they're used by tests or special cases like "fix-bundle" * Likewise some hard coded urls are left in the code since they are used by some special case Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Probably makes sense to handle this in ClientTrustConfig only: less code that way. The tests will start passing once staging TUF contains signingconfig (and we have updated our test copies of staging TUF) Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Commit is large just because the test and embedded assets for staging are updated. * Update the embedded data in sigstore/_store * Also update the test assets in test/assets * refactor the embedded asset lookup: use the URL to build the asset dir. This means less code duplication and easier to make this work with non-Public Good Instance TUF repos * Make the tuf module work with non-PGI instances: if the local TUF metadata is initialized out of band, tuf module just works with it. If a root.json is provided in _store, it is still always used to initialize the client Of special note is "signing_config.v0.2.json" for production: This does not actually exist yet in the TUF repository but I've added one in sigstore/_store and use it as a workaround in ClientTrustConfig.from_tuf() -- this way the code can otherwise remain identical for both staging and prod. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Default now comes from the trust config. The option is also not especially in conflict with staging. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
9685668
to
4376d4f
Compare
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
They will be if there are any rekor 2 instances, but currently TSA is not strictly needed. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
6f7b8c7
to
262043f
Compare
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.
Use TrustConfig throughout the project
This looks complete to me now. I'm keeping it a draft while
Apologies for the seemingly large PR. It's mostly just the assets getting updated: this asset update is not strictly necessary but it is done to get test coverage for signingconfig handling (since staging now has a signingconfig 0.2).
Fixes #1347, fixes #1347
TrustConfig.signing_config
to get the OIDC urlfrom_tuf()
method for TrustConfig and not Trustedroot