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

Clarify key hint #148

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Conversation

kommendorkapten
Copy link
Member

@kommendorkapten kommendorkapten commented Oct 4, 2023

Summary

Clarified how the use of (reduntant) key hint.
Key hint is both specified in the bundle's verification material but may also be specified inside the content if it'a DSSE envelope. This discussion started in this PR #145

This PR favours simplicity over efficiency; that is; the key hint should be equal and duplicated to simplify any consumption of the bundle, as the value may appear in multiple places. The key hint (if used) is usually a short string so I argue it's not too bad.

Release Note

Clarified how to populate the key hint in the bundle, when the content is a DSSE envelope with a key hint too.

Documentation

This PR is only documentation.

Key hint is both specified in the bundle's `verification material` but
may also be specified inside the `content` if it'a DSSE envelope.

Signed-off-by: Fredrik Skogman <[email protected]>
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

No objection to the language, although I'll repeat a previous general concern about identifier/value duplication: every time we duplicate a value (and expect a conforming client to check it for consistency), we introduce a possible error state (or arguably worse, an ambiguous state where a validation path exists but some clients may be looser or stricter than others). Maybe that's unavoidable in this case, but I think as a general design principle we should continue to deduplicate state wherever possible.

@haydentherapper
Copy link
Collaborator

The other option is saying that when a DSSE envelope is used, the key hint should not be set, or if set, should be ignored.

@woodruffw
Copy link
Member

IMO the current language is fine -- requiring that the two be exactly equal makes the most sense to me, I'm just griping about having to think about an additional error state :-)

@kommendorkapten
Copy link
Member Author

Agree that this is not ideal, but I think the current proposal (require them to be equal) is the best option given the current way the bundle is structured.

@kommendorkapten kommendorkapten merged commit 6b78019 into sigstore:main Oct 5, 2023
25 checks passed
@kommendorkapten kommendorkapten deleted the clarify-key-hint branch October 5, 2023 13:07
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.

3 participants