Added an option for a more generic observer time, and aligned with v0.2 #179
Conversation
protos/sigstore_verification.proto
Outdated
| // Verify SET timestamps indicates that the timestamp from | ||
| // the SET should be used when verifying the X.509 | ||
| // certifiacte chain | ||
| bool verify_set_timestamp = 4; |
There was a problem hiding this comment.
To confirm the expected behavior:
- If false, then do not allow an SET to be used as a timestamp or count towards the threshold (the default)
- If true, then allow an SET to count towards the threshold and use the timestamp
ObserverTimestampOptionstakes precedent over this. IfObserverTimestampOptions.threshold > 0andverify_set = false, then SETs can be used for timestamps, but not count towards the tlog threshold
There was a problem hiding this comment.
My last bullet point is wrong. I think verify_set must be true in order to count towards the ObserverTimestampOptions. If verify_set=true is set and only the TSA threshold is set up, then no threshold would be met (working as intended).
(tldr below is just me trying this out in a client)
As I've been prototyping this in sigstore-go, the only question I have is if it's sufficient to only set WithTransparencyLog. This means that the threshold for number of logs and timestamps must match, which I would prefer each should be explicitly configured.
I think the correct client approach would be to require when is used WithTransparencyLog(threshold int, verifySET bool), also one of the following is required: WithTSA(threshold) or WithObserverTimestamp(threshold) or WithInsecureNoTimestamp. If verifySET==true, then the timestamps returned from verifying the log can count towards the WithObserverTimestamp threshold. If WithTransparencyLog(1, true) and WithTSA(1), then SETs could be used to satisfy the log threshold, but not the timestamp threshold.
There was a problem hiding this comment.
OK, sorry I keep going back and forth, but I thought of one case this doesn't handle. How would you specify requiring X log proofs, 0 promises, and Y timestamps that are either SETs or TSA timestamps? This was the point of my last bullet point, some way to specify SETs shouldn't count towards.
So either:
- we need SETs to be their own struct to support this, or:
- a comment that
verify_setonly dictates whether or not SETs count towards the log threshold. We will always still try to verify SETs and extract timestamps, whose policy is dictated by ObserverTimestampOptions.
I prefer the second, I think it creates a cleaner API.
Sorry for rambling on about this, hopefully this makes sense! Late Friday isn't the time to figure this out 😄
Edit: Summarized in sigstore/sigstore-go#44 (comment)
There was a problem hiding this comment.
As v0.2 has the SET as optional, and the proof as required. My thinking in this discussion was only around how the SET counts toward the timestamp threshold, not against the tlog threshold, and so:
a comment that verify_set only dictates whether or not SETs count towards the log threshold. We will always still try to verify SETs and extract timestamps, whose policy is dictated by ObserverTimestampOptions.
would be sort of the opposite: a comment that verify_set only dictates that it count towards the timestamp (if present)
This would mean that if the tlog does only contain a proof, this entry does not count up towards the timestamp threshold. I.e a tlog entry without SET but verified with verify_set=true would not fail, just not increase the counter towards timestamp threshold.
How would you specify requiring X log proofs, 0 promises, and Y timestamps that are either SETs or TSA timestamps
Like this:
WithTLog(x, true) + WithObserverTimestamp(y)
My reading:
WithTLog(x, true): We require x inclsuion proofs (required by v0.2 bundle), verifySET=true we trust the timestamp from the SET if included.
WithObserverTimestamp(y): we require at least y timestamps (as verifySET=true, the SET counts if present, if not then y TSA timestamps is required).
There was a problem hiding this comment.
Good point about v0.2 requiring inclusion proof. Continuing discussion on sigstore-go.
|
Updated to match this PR: sigstore/sigstore-go#51 |
in v0.2 inclusion proofs are preferred over SETs, and so it's now optional to extract the timestamp from the SET. Signed-off-by: Fredrik Skogman <[email protected]>
Signed-off-by: Fredrik Skogman <[email protected]>
Co-authored-by: William Woodruff <[email protected]> Signed-off-by: Fredrik Skogman <[email protected]>
Signed-off-by: Fredrik Skogman <[email protected]>
Co-authored-by: Hayden B <[email protected]> Signed-off-by: Fredrik Skogman <[email protected]>
default is to only require one observer of any kind. added missing options to the ArtifactVerificationOptions Signed-off-by: Fredrik Skogman <[email protected]>
kommendorkapten
force-pushed
the
observer-timestamp
branch
from
67b2b52 to
f6af7d3
Compare
|
As part of adding the |
Summary
Based from the discussion sigstore/sigstore-go#44
Currently there is not possible to define a policy that either a SET or a signed timestamp should be used. This PR addresses that via the
ObserverTimestampOptions. Also as v0.2 does prefer inclusion proofs over SET, this PR allows for option on using the SET or not, and when using the SET, the timestamp can be used during X.509 certificate verification.Release Note
TBD
Documentation
N/A