-
Notifications
You must be signed in to change notification settings - Fork 1.3k
provenance slsa v1 #6005
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
provenance slsa v1 #6005
Conversation
67c0095
to
5e1fdbf
Compare
// TODO: handle builder components versions | ||
// Version: map[string]string{ | ||
// "buildkit": version.Version, | ||
// }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't set the builder components versions. I can't find any real use cases in the wild atm. Maybe setting BuildKit version would be fine but as BuildKit has backward compatibility guarantee, it should not affect reproducibility.
solver/llbsolver/solver.go
Outdated
// TODO: use provenance slsa v1 for build history? | ||
pc, err := NewProvenanceCreator(ctx2, provenancetypes.ProvenanceSLSA02, cap, res, attrs, j, usage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For backward compatiblity with tools using v0.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tonistiigi As discussed offline we would want an opt-in to enable SLSA v1 for provenance in build history. I guess this would not be through the new version
attribute but a builtin-var like BUILDKIT_HISTORY_PROVENANCE_SLSA
that takes the version as value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added opt-in for provenance slsa v1 for build record history
BuildKitMetadata BuildKitMetadata `json:"https://mobyproject.org/buildkit@v1#metadata,omitempty"` | ||
Hermetic bool `json:"https://mobyproject.org/buildkit@v1#hermetic,omitempty"` | ||
// since v1 completeness and reproducible are marked implicit from | ||
// builder.id, but we still keep them for better accuracy and compatibility | ||
Completeness BuildKitComplete `json:"https://mobyproject.org/buildkit@v1#completeness,omitempty"` | ||
Reproducible bool `json:"https://mobyproject.org/buildkit@v1#reproducible,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think we care much about it but found in the spec that extension fields should have a specific format: https://slsa.dev/spec/v1.1/provenance#extension-fields
Extension fields SHOULD use names of the form
<vendor>_<fieldname>
, e.g.examplebuilder_isCodeReviewed
. This practice avoids field name collisions by namespacing each vendor. Non-extension field names never contain an underscore.
Updated docs and examples with pushed images. |
| `filename` | String | `provenance.json` | Set filename for provenance attestation when exported with `local` or `tar` exporter | | ||
| `reproducible` | `true`,`false` | `false` | Explicitly marked as reproducible. See [reproducible](#reproducible) | | ||
| `inline-only` | `true`,`false` | `false` | Only embed provenance into exporters that support inline content. See [inline-only](#inline-only) | | ||
| `version` | String | `v0.2` | SLSA provenance version to use (`v0.2` or `v1`) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to still stick to v0.2 by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For backwards compatibility with existing tools. We need to give the tools some time to upgrade their compatibility and test, then we can switch the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"_type": "https://in-toto.io/Statement/v0.1",
"predicateType": ["https://slsa.dev/provenance/v1"](https://slsa.dev/provenance/v1),
Spec seems to use "_type": "https://in-toto.io/Statement/v1",
although I don't see constant in library.
I don't think digestMapping
and llbDefinition
should be in externalParameters
. Afaics this is things that resembles build request from user. I think they could be in internalParameters
iiuc or completely separate extension field.
Because the builder.id is more important now, I wonder if we should fill in some generic one (linking to our docs). Maybe even different ones depending on if build is complete, hermetic. I think we also need to add a way to set version, not sure if there is ever any need to builderDependencies.
Extension fields SHOULD use names of the form _, e.g. examplebuilder_isCodeReviewed. This practice avoids field name collisions by namespacing each vendor. Non-extension field names never contain an underscore.
So maybe buildkit_completeness
, buildkit_layers
would be enough?
byproducts
This can be follow-up but should also think how we can create links to logs etc (eg. in Github actions) here.
"environment": {
"platform": "linux/amd64"
}
I'm not sure if we need nesting in here. Just nativePlatform/systemPlatform
etc. unless we think we will have more properties for environment. "environment" was field in v0.2 afaics. Similar for configSource
but don't have better naming ideas there.
Materials bool `json:"materials"` | ||
} | ||
|
||
func ConvertSLSA02ToSLSA1(p02 *ProvenancePredicateSLSA02) *ProvenancePredicateSLSA1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can create a tracking issue to revert this and convert from 1 to 02, as eventually we want to remove 02.
|
||
buildDef := ProvenanceBuildDefinitionSLSA1{ | ||
ProvenanceBuildDefinition: slsa1.ProvenanceBuildDefinition{ | ||
BuildType: p02.BuildType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be actual URL iiuc. We can point it to docs file of github I think as it probably isn't very easy for us to update mobyproject.org
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to https://github.com/moby/buildkit/blob/master/docs/attestations/slsa-definitions.md. I looked at others buildType in the wild such as https://cloud.google.com/build/gcb-buildtypes/google-worker/v1. Maybe a dedicated page for v1 would be better.
|
||
type ProvenanceExternalParametersSLSA1 struct { | ||
ConfigSource ProvenanceConfigSourceSLSA1 `json:"configSource,omitempty"` | ||
Parameters Parameters `json:"parameters,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this make more sense as "request"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes request looks more aligned, will change that.
Yes I didn't set the in-toto statement header to v1 here. Was thinking it could be a follow-up as I don't think the underlying provenance v1 predicate requires a specific in-toto version but I agree that we should consider updating to v1 (#4269). cc @cdupuis
Oh indeed,
Yes this is what I was thinking but that's not a requirement from the spec. Happy to change it if we want to.
I kept environment on purpose so it matches
I looked at the Google's buildType: https://cloud.google.com/build/gcb-buildtypes/google-worker/v1. They name it
Yes for GHA logs it could be this kind of URI |
If the old format is based on v0.2 requirements that don't exist anymore then I think we should change it. Does it mean mixing snake and camelcase though?
But that doesn't exist anymore after v0.2. Could you push new images after updates. |
Yes old format of field names for extension fields are URIs.
Yes looks like it: https://slsa.dev/spec/v1.1/provenance#extension-fields
True, I just wanted to keep Completeness struct as there is no equivalent in v1. I'm revisiting that to be aligned with v1. |
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
moby/buildkit#6005 Signed-off-by: Brandon Mitchell <[email protected]>
fixes #3684
Adds support for SLSA provenance v1. This is currently opt-in with a new
version
attribute:If
version
is not set it defaults tov0.2
.Examples
cc @mlieberman85 @lehors @cdupuis