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

Require config as code at levels L3+ #141

Merged
merged 20 commits into from
Sep 2, 2021

Conversation

TomHennen
Copy link
Contributor

@TomHennen TomHennen commented Aug 27, 2021

  • Require build instructions to be tracked at all levels.
    • Lets people know how a build was produced.
    • Should support all build methods without requiring architectural changes.
    • If we adopt this document as level motivation (Clarify the goals / use-cases for each level #111) then we may only need to require build instructions at level 2+ or 3+. We can easily relax this requirement later, while making the requirement more strict would be more difficult.
  • Require config-as-code at L3+.
    • Allows people to review the details of how builds are performed.
    • Can require changes to how builds are performed to be reviewed by additional parties.
    • Tracks changes to how builds are produced over time.
    • Provides an easier method of writing policy to make sure an artifact followed the build steps you expect (just point at the build config in source control)
    • May require architectural changes in some builders or changes to how users interface with their build service (e.g. Tekton, LUCI, Cloud Build)
  • Combine "identifies source" with "identifies entry point".
    • The 'identifies source' requirement was already specific to the top-level build script which was the 'entry point'.
      So there doesn't seem to be a reason to have these as separate requirements.
    • 'Identifies source' might have had an overly broad name since it was only talking about where the top-level build script was, but could have been interpreted to mean "identifies all source" (which it did not!).
  • Only require "identifies entry point" at L3+.
    • As discussed with 'config-as-code' above, not all build architectures support singular entry-points that are defined in a script.
    • Requiring 'identifies build instructions' at all levels should cover cases where folks aren't using config-as-code.
  • Small updates to provenance examples to make linking easier.

This PR does not take a position on how/if requiring all sources or dependencies be recorded in the provenance. That should
be handled in a different PR addressing a different issue.

Note #115 suggested the language "require that the provenance identify the independent inputs", but I found "independent inputs" to be fairly broad. The rest of the text of #115 indicated that this was really about tracking what the build did, so "build instructions" seems to cover the spirit of #115.

fixes #115

@TomHennen TomHennen force-pushed the config-as-code-reqs branch from b4ddf69 to 0d02b9f Compare August 27, 2021 19:27
@TomHennen TomHennen requested a review from msuozzo August 27, 2021 19:28
@TomHennen
Copy link
Contributor Author

Matt could you give this a quick check before I request 'official' review?

<td>Config as code
<td>

The build service configuration is defined in source control and drives the build automatically.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"drives the build automatically"

I'm a little hesitant about requiring there to be automation triggering the build. I think it'd be fine to just limit build configuration to "use this workflow" regardless of how/when it's triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't intend for 'build automatically' to mean "triggered automatically", just that the this config is the set of instructions the builder followed automatically to do the build. E.g. no person needed to feed it the individual steps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an alternative phrasing that avoids the implication of automation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The build service configuration is defined in source control and drives the build automatically." -->

"The build service configuration is defined in source control and is executed by the build service."

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe "and is used as the build entrypoint"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not that hot on using 'entrypoint' here because later we distinguish between 'entrypoint' (a singular field to reference) and 'steps' (details about how the build progressed) as being two different things. Yet here both of them count as the build instructions.

I think I'll go with "The build service configuration is defined in source control and is executed by the build service." and we can see what others think.

docs/requirements.md Outdated Show resolved Hide resolved
docs/requirements.md Outdated Show resolved Hide resolved
docs/requirements.md Outdated Show resolved Hide resolved
docs/requirements.md Show resolved Hide resolved
@TomHennen TomHennen force-pushed the config-as-code-reqs branch 3 times, most recently from 4ab49cf to e48fd4d Compare September 1, 2021 15:29
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Also see if there's a 'comment' type that doesn't look bad.
(JSON doesn't support comments but we want to explain some
things inline in these examples).

Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
docs/requirements.md Outdated Show resolved Hide resolved
docs/requirements.md Outdated Show resolved Hide resolved
docs/requirements.md Outdated Show resolved Hide resolved
docs/requirements.md Show resolved Hide resolved
Signed-off-by: Tom Hennen <tomhennen@google.com>
This will let us link to them with anchors.

Also change "Manually Run Commands" to "Explicitly Run Commands"
as some builders may take this list from an RPC that wasn't
really defined 'manually'.

Signed-off-by: Tom Hennen <tomhennen@google.com>
Also rpc -> RPC in provenance example.

Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
Signed-off-by: Tom Hennen <tomhennen@google.com>
@TomHennen TomHennen marked this pull request as ready for review September 1, 2021 19:34
Copy link
Contributor

@msuozzo msuozzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@inferno-chromium
Copy link
Contributor

@trishankatdatadog @joshuagl @brunodom @david-a-wheeler @mlieberman85 @zakgreant - thoughts on this. we would definitely benefit from 1-2 additional eyes on this requirement review.

Copy link
Member

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a couple of minor suggestions, but am happy to approve as is. This clarifies a common source of confusion for folks reading SLSA.

docs/requirements.md Outdated Show resolved Hide resolved
docs/requirements.md Outdated Show resolved Hide resolved
Signed-off-by: Tom Hennen <tomhennen@google.com>
<td>Config as code
<td>

The build service configuration is defined in source control and is executed by the build service.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand what this means for something like GCF where the build definition is entirely specified by the service itself (GCF), but is not part of the user's source control at all.

(I realize there are source provenance questions for GCF, but set those aside for the moment.)

Copy link
Contributor Author

@TomHennen TomHennen Sep 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCF just builds/runs whatever is in a specific repo right?

My suggestion:

  1. Have a recipe.type specific to GCF 'http://google.com/cloud/gcf_recipe/v1' (or whatever)
  2. Set recipe.entrypoint to 'gcf_implicit' (or whatever)
  3. Put the repo that GCF actually built in materials (should be doing this anyways)
  4. Set recipe.definedInMaterial to the index of the repo in 3.

Thoughts?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCF can build a repo, or an arbitrary zip file.

I'm not familiar with the recipe.* things - can you point me to a reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://slsa.dev/provenance/v0.1#recipe.type

If GCF is building from a zip file it wouldn't qualify at SLSA Level 3 as SLSA Level 2 requires the use of source control https://github.com/slsa-framework/slsa/blob/main/docs/requirements.md#version-controlled

but

The way I'd represent building a zip file in the provenance for GCF would probably be very similar:

  1. Have a recipe.type specific to GCF 'http://google.com/cloud/gcf_recipe/v1' (or whatever)
  2. Set recipe.entrypoint to 'gcf_implicit' (or whatever)
  3. Put the path of the zip file and it's hash that GCF actually built in materials (should be doing this anyways)
  4. Set recipe.definedInMaterial to the index of the zip file in 3.

So the provenance would basically look the same, but the artifacts built from zip files shouldn't qualify for anything higher than SLSA L1.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so the recipe.* parts are within the provenance.

My question is more specifically about the phrase "build service configuration is defined in source control" - in GCF, the build configuration is not defined in source control - even in the case of the source being in version control.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I think since GCF is basically a fixed function in that regard (you can think of the build configuration as being as simple as 'gcf ') that it would be completely fine to take the approach I outlined in #141 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the way to think about it is "With GCF the configuration is defined in the source repo being built." So all you have to do is point to that repo and say "GCF built this".

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

Successfully merging this pull request may close these issues.

SLSA 1 & 2 should not require config-as-code
5 participants