-
Notifications
You must be signed in to change notification settings - Fork 72
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
Timestamp somewhere? #46
Comments
I think the argument against putting it in a generic field at the statement layer is that it would be difficult to convey what the timestamp means for all predicates. This is discussed a little bit here where it says
That leans towards having more specific timestamp fields in the predicate layer and is why the Provenance predicate defines Are there use cases where someone might want to make a blanket statement against multiple different predicate types based on the value of the timestamp field? What would be the disadvantages of only having specific timestamp fields in the predicate? |
I am favouring the specific timestamp fields in the predicate layer because I'm not sure what use-cases a generic timestamp in the statement layer would satisfy and type specific fields in the predicate layer provide more context for policy evaluation. Should we provide additional details/guidance in the predicate conventions section of the spec? |
Following the rational of separating statements from predicates - one could imagine a policy of "Must have a fresh attestation", or "order of attestations to sub-processes should be 1-2-3" . I think there is a more important subject behind the timestamp issue - which is the context of the attestation (the timestamp is only one component of such a context). In SLSA v.2 environment object can suffice, but I would argue that any attestation requires some context, and thus it should be a part of the statement. Examples to context data: pipeline-run, job-ID, GitHub repo, machine-id. |
For context-sensitive However, I think there is significant value in having a standard mechanism for attaching standard timestamp metadata such as Would folks be opposed to adding |
So, if I understand, you suggest to add a group of timestamps - exp, nbf, so it makes sense to also add a "created at" option. |
I'm not convinced "expires at" and "not before" make sense for most predicates. I think it only applies to a single class of predicates, namely those that I would call "authorization". Consider the main classes predicates being discussed today:
Only the last one has a concept of a validity window; the rest do not expire. For example, consider provenance: if an artifact was built on 2022-01-01, that fact remains true for all time. There might be a policy on the consumer's side that only accepts builds from within the last N days, but that configuration is not part of the attestation. Furthermore, each of of the times "T" above has a different meaning (built at, time of the code review, time of the test (or of the commit?), time of the scan) which is different than the time of the attestation. Thus, my recommendation is to have context-specific timestamps in the predicate. I'm also not sure how a timestamp in the Statement layer can mitigate against leaked keys. Doesn't such protection need to go in the envelope layer (DSSE)? And furthermore, isn't this best achieved through a key rotation/revocation policy? A timestamp field doesn't do any good against key compromise unless the timestamp is authenticated independently.
I agree with this one so long as we (1) establish a clear use case and (2) name the field in way that it is unlikely to be misused. One such use case is remediation of bugs in the attestation generator. For example, if a build system had a bad release that produced incorrect provenance for some period of time, the timestamp could be useful for finding that. (One might also consider adding a version number, but I suspect that most systems are complex enough that a single version number cannot encapsulate all dependencies.) As for field naming, would it make sense to wrap in a {
"metadata": {
"createdAt": "2021-01-01...",
},
"subject": {...},
"predicateType": "...",
"predicate": {...}
} |
@MarkLodato I could make a case for expiring every single one of your examples, and I also wasn't suggesting the fields should necessarily be required, just documented as options for expressing that metadata in standard ways. I could see
|
Please do! 😀 @mattmoor that would be a huge help. |
I'll skip the last one, since you covered it. For vulnerability scans, while I agree with your framing around timestamp being the true signal of validity, I'd compare this to a negative Covid PCR test; it's immediately invalid, but practically the result is accepted for some period of time, e.g. 72 hours. For code reviews, I'm sure we've all seen PRs (or CLs) that take months to merge. In some cases that's due to a lack of approval, but if something been partially approved (e.g. For test results, the easiest answer is that at some level, tests stop being hermetic, and it would be good to know things still work. Probers are sort of a degenerate case of this, but are often a poor substitute to a full spectrum e2e test suite run periodically to exercise assorted code paths. For provenance, I see what Solarwinds did as a spatially distributed way of confirming reproducibility, but frankly I think a temporally distributed confirmation of reproducibility is more valuable. With ephemeral build executors, it's comparable to spatial distribution since the execution environment is necessarily different, but it has the added benefit that any unpinned dependencies that have drifted cause a re-attestation build to fail confirmation, and the deployed build is now staring down the barrel of an expiring attestation. I see parallels here to "build horizon" at Google. A lot of these could just as easily be framed in terms of |
Thanks, @mattmoor. That is helpful. First, regardless of whether we allow "expiration" timestamps, we should have clear guidance that implementations SHOULD NOT rely on external wall clocks, and instead include a "now" timestamp as part of the policy. The problem with a "time bomb" is that it is a large reliability risk, causing a working system to suddenly break with no external state change and no way to roll back. At Google, we generally avoid use timestamps (including build horizon) for this very reason. See Building Secure and Reliable Systems, Chapter 9, page 190, "Limit Your Dependencies on External Notions of Time". This risk could be addressed by including the cutoff timestamp (or effectively "now" + "max age") as part of the policy, which is rolled out carefully and can be rolled back. As for a generic "expiration" timestamp, I continue to feel that most of those cases are better served via policy (vulnerability scan, test result, and provenance), or in rare cases with a predicate-specific expiration (code review). In fact, the code review case highlights why predicate-specific is better: the approval's expiration doesn't mean that the attestation is entirely invalid (i.e. that the code is no longer good) but rather that it must be merged within that window. If it is indeed merged within the window, the attestation should still remain valid. |
I think we've generally agreed that it's fine to have timestamps but that they need to be specific. (e.g. signed_at, etc...). We don't like the idea of expirations in the statement because they cause things to blow up (as Mark suggests). Individual predicates can always add whatever fields they like. So, given the lack of traffic on this bug we'll close it. Feel free to reopen if you disagree. |
We've been discussing support for vulnerability scans as a type of "attestation" over here: sigstore/cosign#442
and it's clear that these will need some form of timestamp to work correctly. A vulnerability scan is timely, and should only be considered valid for a specific period of time after it is generated. This also helps align with the principle of "monotonicity", where the absence of an attestation should never move a decision from DISALLOW to ALLOW.
This could be done with a timestamp inside a custom scan predicate, but it might also be useful to place this at the statement layer. I'm not convinced either way yet.
cc @joshuagl @SantiagoTorres
The text was updated successfully, but these errors were encountered: