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

repo: Ensure keyids are correct in signing event #536

Merged
merged 4 commits into from
Feb 4, 2025

Conversation

jku
Copy link
Member

@jku jku commented Feb 3, 2025

Keyids should match the hash of the key content. The signing tool already does this but makes sense to verify in signing-event (since metadata can be modified outside the signing tools).

Upstream python-tuf does not enforce keyids since I/ we think that

  • this provides no additional security and
  • forcing keyid updates is not a good idea and makes repositories a pain to maintain

This is all true but because other clients (awslabs/tough) disagree, this now becomes an interop issue

See sigstore/root-signing#1431

The signing tool already does this but makes sense to verify
in signing-event (since metadata can be modified outside the signing
tools).
@jku jku requested a review from kommendorkapten as a code owner February 3, 2025 16:08
@jku jku marked this pull request as draft February 3, 2025 16:35
jku added 2 commits February 3, 2025 18:43
There is a private embedded in the code for e2e tests to use.
This key has an "incorrect" keyid (hash does not match)
The test key keyid changed: this means all expected results also change.
The only real change is the keyid, but this means the ordering of keys
changes so it looks like a larger change.
@jku jku force-pushed the check-keyids-in-signing-event branch from beb88f2 to f8f9b5a Compare February 3, 2025 16:44
@jku jku marked this pull request as ready for review February 3, 2025 16:47
@jku jku force-pushed the check-keyids-in-signing-event branch from ba255ee to 2dceec4 Compare February 4, 2025 08:43
@jku
Copy link
Member Author

jku commented Feb 4, 2025

I've added a release prep commit (with changelog and version number changes), with the assumption we want to do a release.

Copy link
Member

@kommendorkapten kommendorkapten left a comment

Choose a reason for hiding this comment

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

Looks good!

@jku jku merged commit e17a197 into theupdateframework:main Feb 4, 2025
11 checks passed
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.

2 participants