Revise checkpoint key ID comment, deprecate log ID #629
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
jku
approved these changes
May 5, 2025
protos/sigstore_trustroot.proto
Outdated
Comment on lines
74
to
75
| // extract the key name (i.e. log origin, base_url) and 4-byte key ID, | ||
| // and compare the key name and key ID against each log instance. |
There was a problem hiding this comment.
maybe avoid saying that key name is compared since it seems only the keyid is compared:
Suggested change
| // extract the key name (i.e. log origin, base_url) and 4-byte key ID, | |
| // and compare the key name and key ID against each log instance. | |
| // use the key name (i.e. log origin, base_url) and public key to calculate | |
| // key ID which can then be compared against the TrustedRoot log instances |
There was a problem hiding this comment.
Thanks, incorporated!
There was a problem hiding this comment.
Actually sorry, I need to revert this partially. The client wouldn't know the log public key yet. The client only has what the checkpoint provides, which is the origin and key ID in the checkpoint signature line. The client can verify that the key ID is properly computed if it wants to after it retrieves the public key from the TrustedRoot, but I don't think that's strictly necessary.
Our use of log ID assumes that each deployment has a unique key. Additionally, its calculation was based on RFC6962 for CT. For Rekor v2, we will now follow the C2SP spec to compute key IDs. For ECDSA keys, the log_id value will match. Ed25519 and RSA won't however. Rather than continue to set both of these values, I'd like to deprecate log_id and replace it with checkpoint_key_id for future log deployments. Also update the base_url comment to state that it should match the log checkpoint origin, which will be true for Rekor v2. Rekor v1 has its own computation for an origin that isn't a valid URL. Signed-off-by: Hayden B <[email protected]>
kommendorkapten
approved these changes
May 8, 2025
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.
Our use of log ID assumes that each deployment has a unique key. Additionally, its calculation was based on RFC6962 for CT.
For Rekor v2, we will now follow the C2SP spec to compute key IDs. For ECDSA keys, the log_id value will match. Ed25519 and RSA won't however. Rather than continue to set both of these values, I'd like to deprecate log_id and replace it with checkpoint_key_id for future log deployments.
Also update the base_url comment to state that it should match the log checkpoint origin, which will be true for Rekor v2. Rekor v1 has its own computation for an origin that isn't a valid URL.
Summary
Release Note
Documentation