Don't fail on TSA signatures we can't verify #45
Merged
Conversation
This file contains 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
For #43 As long as there are enough TSA signatures that do verify to meet the threshold specified by the verification policy. Signed-off-by: Zach Steindler <steiza@github.com>
haydentherapper
previously approved these changes
Dec 14, 2023
There was a problem hiding this comment.
Looks good!
We have the same fail-fast vs fail-after-threshold for tlogs - https://github.com/sigstore/sigstore-go/blob/main/pkg/verify/tlog.go#L85-L161
I can make this update, or would you like to?
pkg/verify/tsa_test.go
Outdated
| assert.Error(t, err) // only 1 good should not meet threshold of 2 | ||
| } | ||
|
|
||
| type oneGoodOneBadTimestampEntity struct { |
There was a problem hiding this comment.
tiny nit would be calling this "goodAndUntrustedTimestampEntity" rather than "bad"
haydentherapper
added a commit
to haydentherapper/sigstore-go
that referenced
this pull request
Dec 15, 2023
Like sigstore#45, as long as the threshold of expected timestamps is met, then verification should succeed. Otherwise, entries without trust root material should be skipped. One benefit of having the log key ID be used to look up the correct trust root material is that we can still error out if the signature is invalid. Ref: sigstore#43 Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
|
Rekor is done in #47 (wanted to code some today :) ) |
Signed-off-by: Zach Steindler <steiza@github.com>
haydentherapper
approved these changes
Dec 15, 2023
phillmv
approved these changes
Dec 15, 2023
haydentherapper
added a commit
to haydentherapper/sigstore-go
that referenced
this pull request
Dec 15, 2023
Like sigstore#45, as long as the threshold of expected timestamps is met, then verification should succeed. Otherwise, entries without trust root material should be skipped. One benefit of having the log key ID be used to look up the correct trust root material is that we can still error out if the signature is invalid. Ref: sigstore#43 Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
haydentherapper
added a commit
that referenced
this pull request
Jan 3, 2024
Like #45, as long as the threshold of expected timestamps is met, then verification should succeed. Otherwise, entries without trust root material should be skipped. One benefit of having the log key ID be used to look up the correct trust root material is that we can still error out if the signature is invalid. Ref: #43 Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
haydentherapper
added a commit
to haydentherapper/sigstore-go
that referenced
this pull request
Jan 3, 2024
Like sigstore#45, as long as the threshold of expected timestamps is met, then verification should succeed. Otherwise, entries without trust root material should be skipped. One benefit of having the log key ID be used to look up the correct trust root material is that we can still error out if the signature is invalid. Ref: sigstore#43 Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
haydentherapper
added a commit
that referenced
this pull request
Jan 3, 2024
* Don't fail on Rekor entry verification for untrusted entries Like #45, as long as the threshold of expected timestamps is met, then verification should succeed. Otherwise, entries without trust root material should be skipped. One benefit of having the log key ID be used to look up the correct trust root material is that we can still error out if the signature is invalid. Ref: #43 Signed-off-by: Hayden Blauzvern <hblauzvern@google.com> * Address comments Signed-off-by: Hayden Blauzvern <hblauzvern@google.com> * Add option to unify signed and log timestamps This adds the WithObserverTimestamp option to verify that a threshold is met using both RFC3161 signed timestamps and log integrated timestamps. This updates the verification API for the log to only verify log inclusion proofs or SETs, so that log and timestamp verification are not conflated. This also fixes a bug where the integrated time is used with an inclusion proof. This is not safe to do, since the integrated time is not authenticated metadata without a SignedEntryTimestamp signature. Signed-off-by: Hayden Blauzvern <hblauzvern@google.com> * Fix conformance test Signed-off-by: Hayden Blauzvern <hblauzvern@google.com> * Add WithIntegratedTimestamps Signed-off-by: Hayden Blauzvern <hblauzvern@google.com> * Complete TODOs, add tests Signed-off-by: Hayden Blauzvern <hblauzvern@google.com> --------- Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
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.
Summary
See #43
As long as there are enough TSA signatures (that we can verify) to meet the threshold specified by the verification policy, we say that the bundle is valid.
Release Note
NONE
Documentation
N/A