Skip to content
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

Bump TUF root version #1312

Merged
merged 3 commits into from
Aug 8, 2023
Merged

Conversation

haydentherapper
Copy link
Contributor

Also update the embedded targets.

Summary

Release Note

Documentation

Also update the embedded targets.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

@asraa, I wanted to confirm, these target file names should not be prefixed with their hash, correct? I looked through the code and believe that's accurate, but wanted to double check.

@haydentherapper
Copy link
Contributor Author

fyi @jku, since you had posted an issue about updating tuf metadata

@asraa
Copy link
Contributor

asraa commented Aug 7, 2023

@asraa, I wanted to confirm, these target file names should not be prefixed with their hash, correct? I looked through the code and believe that's accurate, but wanted to double check.

I'm pretty sure these are used to prepopulate the cache, and when you download to a local store TUF clients strip the hash prefix. I would test it out locally too.

Copy link
Contributor

@znewman01 znewman01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love instructions for doing this change (and for verifying it) to be documented somewhere. Also maybe how frequently we should be doing it and maybe even issues that get automatically filed.

None of that's blocking for now!

@haydentherapper
Copy link
Contributor Author

sigstore/root-signing#817 for tracking automation (across all client codebases too)

To verify, in Cosign, I bumped go-tuf to 0.6.0 and replaced s/s with my local repo. Verified without the replace directive cosign initialize fails, with the replace directive cosign initialize succeeds.

I've also removed set_ecdsa from s/s because we no longer need to support the deprecated verifier.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

haydentherapper commented Aug 7, 2023

Removing set_ecdsa is technically a breaking change, if a client hasn't requested a new root since the generation of v5, since the TUF client will have to load the v2/v3/v4 root keys to verify subsequent roots. That was 10 months ago though. I would say that this change is fine to do, as 10 months is after our deprecation period.

Edit: Other thing to note is the change that @kommendorkapten added recently to go-tuf to fix the noncompliant ecdsa identifier was not added to the deprecated ecdsa key handler. I'd rather not continue to support the deprecated path if we don't have to.

Copy link
Contributor

@znewman01 znewman01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@haydentherapper
Copy link
Contributor Author

Fixing tests now, this is from removing set_ecdsa...

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper haydentherapper merged commit 8a7b9db into sigstore:main Aug 8, 2023
9 checks passed
@haydentherapper haydentherapper deleted the update-root branch August 8, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants