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

Timestamp somewhere? #46

Closed
dlorenc opened this issue Jul 16, 2021 · 11 comments
Closed

Timestamp somewhere? #46

dlorenc opened this issue Jul 16, 2021 · 11 comments

Comments

@dlorenc
Copy link

dlorenc commented Jul 16, 2021

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

@TomHennen
Copy link
Contributor

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

Timestamps SHOULD use RFC 3339 syntax with timezone "Z" and SHOULD clarify the meaning of the timestamp. For example, a field named timestamp is too ambiguous; a better name would be builtAt or allowedAt or scannedAt.

That leans towards having more specific timestamp fields in the predicate layer and is why the Provenance predicate defines buildStartedOn and buildFinishedOn.

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?

@joshuagl
Copy link
Contributor

joshuagl commented Oct 7, 2021

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?

@dn-scribe
Copy link

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.
The challenge is to identify useful, common, mandatory context fields, and to enable user-defined fields.

@mattmoor
Copy link

mattmoor commented Jan 4, 2022

For context-sensitive timestamp properties, I completely agree that they belong inside of the predicate itself.

However, I think there is significant value in having a standard mechanism for attaching standard timestamp metadata such as exp (and possibly nbf) to statements. As a vehicle for authenticated claims, not having a clear way to specify its window of validity seems like a hole. These aren't context-sensitive properties and could be applied to virtually any statement (possibly as a mitigation against leaked keys).

Would folks be opposed to adding exp (and maybe nbf) to the statement? Probably not the abbreviated OIDC names, but I don't particularly care about what we call them as long as they are clear and capture equivalent semantics.

@dn-scribe
Copy link

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.

@MarkLodato
Copy link
Contributor

However, I think there is significant value in having a standard mechanism for attaching standard timestamp metadata such as exp (and possibly nbf) to statements. As a vehicle for authenticated claims, not having a clear way to specify its window of validity seems like a hole. These aren't context-sensitive properties and could be applied to virtually any statement (possibly as a mitigation against leaked keys).

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:

  • Provenance: subject X was built by Y at time T
  • Code review: subject X was reviewed by Y at time T
  • Test result: subject X passed test Y at time T
  • Vulnerability scan: subject X was scanned by Y at time T with result {Z}
  • Authorization: subject X is allowed in environment Y from time T1 to T2

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.

it makes sense to also add a "created at" option.

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 field to make it more clear that this is metadata about the attestation as opposed to a property of the subject? For example:

{
  "metadata": {
    "createdAt": "2021-01-01...",
  },
  "subject": {...},
  "predicateType": "...",
  "predicate": {...}
}

@mattmoor
Copy link

mattmoor commented Feb 3, 2022

@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 createdAt (however it is stored) being less contentious, and it affords policy engines to make their own judgements around the validity lifespans of different predicate types.

metadata SGTM.

@MarkLodato
Copy link
Contributor

I could make a case for expiring every single one of your examples

Please do! 😀 @mattmoor that would be a huge help.

@mattmoor
Copy link

mattmoor commented Feb 4, 2022

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. .github/CODEOWNERS), I could certainly see time-bounding approvals to ensure that a change has been reviewed by someone with all the context relevant to a repo when it merges, which may have drifted since the original review. Bounding approvals as "good" for ~1 month would probably only adversely affect a small niche of folks, but presents a safeguard.

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 createdAt combined with policy that asserts a maximum lifespan (before re-attestation is needed), but I personally see value in having a way to "time bomb" signed things to allow the producer to require fresh attestation after a certain point in time.

@MarkLodato
Copy link
Contributor

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.

@TomHennen
Copy link
Contributor

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.

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

No branches or pull requests

6 participants