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

Create a predicate to convey information about source control system security settings. #47

Open
TomHennen opened this issue Jul 20, 2021 · 14 comments
Assignees

Comments

@TomHennen
Copy link
Contributor

It would be nice to have a predicate that could indicate the security posture of a given commit from a source control system.

Specifically it would be nice to be able to convey the requirements of the SLSA Source Control requirements.

@TomHennen TomHennen self-assigned this Jul 20, 2021
@msuozzo
Copy link

msuozzo commented Jul 21, 2021

Per our discussion today, one idea which could leverage existing tooling would be to publish scorecards (or some derivative representation) somehow encoded in a predicate. Whether at a given commit (can analysis be done per-branch?) or just a point-in-time, this could encapsulate a number of the SLSA Source requirements in a single structure.

Doing this analysis coincident with a build would be tricky and taking a previous point-in-time audit would carry risks: We would need to mitigate the attack of {change repo settings, make a commit, restore settings}.

Relevant properties:

  • Code-Review (code review required >= L4)
  • Commit-Signing (verified history required >= L3)
  • Branch-Protection (to ensure code review)
  • Pull-Requests (to ensure code review)

Other SLSA properties:

  • Version control is implicit in the analysis given that it currently only supports github (tracking issue)
  • Retention History can't currently be addressed by any of the scorecard properties
  • Superuser requirements (i.e. those that can bypass the above checks) could be added to scorecard but aren't currently included.

@TomHennen
Copy link
Contributor Author

This is an interesting idea and could have other benefits.

Thanks for outlining the concerns w.r.t. the current Scorecards architecture, we'd definitely need to address those if we pursued this route.

Regarding Retention History, this is an interesting one that, unlike many of the others, could be resolved by the consuming org and doesn't necessarily need to be addressed by the original source control system... I think? @MarkLodato any thoughts there?

While Scorecards is currently specific to GitHub I don't see a reason it couldn't apply to other source repos. So it seems like it could be sufficiently generic for SLSA's purposes.

I'd love to get @kimsterv's thoughts.

@MarkLodato
Copy link
Contributor

Regarding Retention History, this is an interesting one that, unlike many of the others, could be resolved by the consuming org and doesn't necessarily need to be addressed by the original source control system... I think? @MarkLodato any thoughts there?

Adding it to the scorecard would be a good idea. (Does GitHub have such a knob though?)

I also agree that a system could alternatively satisfy the SLSA requirement by having the build system retain the sources instead of the source system.

Superuser requirements (i.e. those that can bypass the above checks) could be added to scorecard but aren't currently included.

To clarify, there two types of privileged access:

  • "data owner", which is project-specific
  • "system admin", which is platform-wide

We definitely need a scorecard for the "data owner" case, which for GitHub branch protections is the include administrators option.

For the "system admin" case, I don't know if we really need a scorecard since it can be implicit in the source or builder platform. For example, it is up to the consumer to believe whether GitHub's internal access controls are sufficient to meet SLSA 4.

@TomHennen
Copy link
Contributor Author

Oh, I'd like to add that I think there are some questions with respect to scorecards:

  1. Could Scorecards issue the required attestations instead of requiring it of the Source control system.
  2. Should the issued attestation include all scorecard data (going beyond what SLSA needs to check the SLSA requirements).

For 1 the problem I see is how builders know where to get the attestation. Internally we've been thinking that the builder could fetch the attestations from the source control system when it fetching the source. Then they can just be bundled with the other attestations the builder produces. Then they'd be naturally available to any enforcement point that needs to make decisions on the data If Scorecards were producing the attestation we'd need to figure something else out. Do you have any thoughts there?

For 2 the problem is that we've sort of been hoping to keep the content of attestations simple and only put in data that is actually necessary for a given purpose. Scorecards has a lot of data that isn't necessary for SLSA (but also lots that is!). I'd like to avoid enforcement points having to support lots of different types of source attestations (I have a similar goal for the provenance). Are we suggesting everyone would have to use Scorecards to provide this information (seems like this won't work universally)? Or that it's actually fine for there to be multiple ways to convey these properties (it would be hard for enforcement points to support them all, which might cause trouble when checking an artifact that uses multiple sources which have different methods of providing this information).

So, I guess I'm currently (weakly) feeling that 1 would be totally fine as long as we can figure out how to get the builder to collect this information, but I'm a bit concerned about 2.

@TomHennen
Copy link
Contributor Author

Talked with @msuozzo about this briefly.

It seems it would be reasonable to have two attestations

One predicate type for observed source security properties of some source control system. This could be issued by Scorecards for some GitHub repo for example. Another predicate type for attestations issued by the source control system itself. These will each answer questions like "how do I know the current settings aren't just temporary" in different ways. The observer-issued type could say "I've seen these settings for the last 6 months" while the self-issued type could, for example, say "Users cannot disable these settings".

On a bookkeeping note I wonder if further discussion (or at least the proposed predicate types themselves) should be moved to the SLSA repo in-line with #54.

@MarkLodato
Copy link
Contributor

Seems reasonable to me, including moving to the SLSA repo since it is mostly SLSA-specific.

@mlieberman85
Copy link

mlieberman85 commented Aug 4, 2021

Could you pull down an assertion of the config based on the commit hash?

e.g. when pulling down the code you could associate the configuration at the time of the commit, or it's merge into the branch you're pulling down?

@TomHennen
Copy link
Contributor Author

Yes, that's exactly what I'm thinking! Though it may only apply to commit hashes that have been merged to certain branches.

@mlieberman85
Copy link

Once upon a time we did something similar with Gerrit where we could associate Gerrit change-ids with the configuration by which we were managing merges. This decoupled it from the commit itself since you could modify the commit and still have same change id. I wonder if other source code control systems like github have unique identifiers for stuff like PRs you could do that same sort of correlation with.

@mlieberman85
Copy link

Has anyone looked at this sort of approach? https://github.com/crev-dev/cargo-crev + https://github.com/crev-dev/crev. Someone from OpenSSF pointed me in this general direction. Haven't used or checked out the tools but the approach seems reasonable.

@TomHennen
Copy link
Contributor Author

Took a quick look at crev, it does look interesting. So far we've left it up to the source control systems as to how they want to handle requiring reviews. It looks like the intention of crev might be to review the entire package, not just changes? "Crev is an actual code review system as opposed to typically practiced code-change review system."

I could imagine people providing reviews over a package as a way of implementing the 2 party review requirement for something where the original project didn't have per-change review enabled. I'm not quite sure how it would scale if packages have frequent releases. Would certainly be worth considering if people are interested.

@TomHennen TomHennen added the slsa-provenance Issues about the SLSA provenance predicate that moved to its own repo label Aug 17, 2021
@TomHennen
Copy link
Contributor Author

Added the slsa-provenance label because this is pretty related to SLSA (it's about the SLSA source requirements) so we might want to move it to the SLSA repo...

@TomHennen
Copy link
Contributor Author

Here's a proposal for attestations that the VCS can issue to attest to the security posture of the source in a given repo.

Join slsa-discussion to get access to the doc without needed to request access!

@laurentsimon
Copy link

laurentsimon commented Jan 28, 2022

Hey (scorecard maintainers here)

I like the idea of having scorecard generate attestations - surprise :-) !

We have this on our roadmap. We think we'll have something for Q2

Few high-level comments:

  1. Our plan is to leverage the scorecard action installed on the repos who want to sign up for SLSA compliance
  2. We are planning to expose scorecard results with more granularity than scores, which should help binary consumers apply additional policies. For example: branch protection has various settings and we plan to expose this
  3. Repo settings would be great to capture, but are harder in general because they are not tied to a commit history (no config-as-code is used). Branch setting changes can be monitored though: the scorecard action is configured to run on branch_protection_rule triggers: This should cover the immediate SLSA requirements over 2LGTM.
  4. We have a way to make the attestations non-falsifiable, by leveraging OIDC's integration w/ sigstore/Fulcio.
  5. We will generate attestations for each repo change (commit to main branch, branch protection changes)
  6. 2LGTM does not always prevents sock puppet attacks: an admin of the repo can add a temporary maintainer account to the repo for LGTM and bypass 2LGTM rules. We plan to expose who LGTM pull requests and who merged it in scorecard results, so as to allow consumers to decide if the users are the expected maintainers. This sort-of means we may need to keep this"list of expected maintainers" somewhere outside the repo's control, maybe on a slsa/something repo not controlled by the admin of the binary-releasing repo. A change to this metadata should require 2+ LGTMs from the repo maintainers/admins to be changed, for example.

Eventually, there may be a need to have a 3P repo containing not just the expected maintainer list, but also how to verify builds. There will be different builders: some will use GCB, some will use sigstore, etc. If we want a single tool to be able to verify builds, we need a central point to declare how consumers are expected to verify them (maintainers, public key, root CA, etc): Note that this is similar to the way users express TUF delegations today, IIUC.

Note that the same puppet attacks that apply to the original binary-releasing repo may apply to the central 3P repo containing verification information. This is not necessarily a show stopper, though:

  1. We can add more stringent security measures that may be too cumbersome for all repos to adopt, e.g. public ceremony (akin to root CAs'), signed commits, GH apps like allstar that watch repos and require LGTMs for any config changes, etc.
  2. So long as the admins of of the 3P repo are not admins of binary-releasing repos, the 3P rogue admin can only tamper with the verification information which would make verification fail (DoS/availability attack), but not trick a binary consumer into accepting a build as valid. In other words, a rogue 3P admin can not simultaneously pull off a sock puppet attack on the 3P repo and a binary-releasing repo since they should not have admin rights to the latter.

Sorry for the long post. I'll take a look at the proposal.

And sorry if I proposed things that are dead obvious and you've already covered in other threads.

Maybe a good time to catch up on these topics.

cc @asraa @lumjjb

@MarkLodato MarkLodato removed the slsa-provenance Issues about the SLSA provenance predicate that moved to its own repo label Apr 6, 2022
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

5 participants