Require signed commits for all Kubernetes orgs and repositories

[dims]

CVE-2024-3094 highlights the need to make sure we know the folks who are making changes to our codebase better. One of the aspects (among many others!) is to ensure we require commits that get merged to be proven. CLA helps somewhat in this regard, but we need to do better.

Now that GitHub supports SSH key based git signing in addition to GPG keys:
https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification

If we are able to require SSH or GPG based keys to sign commits, then folks browsing GitHub UI or inspect our git tree can be confident of the source of the commits (somewhat!). So can we please add one more layer to the multi-layer security for our github orgs by requiring this?

thanks,
Dims

[dims]

/sig steering
/sig contributor-experience

[k8s-ci-robot]

@dims: The label(s) sig/steering cannot be applied, because the repository doesn't have them.

In response to this:

/sig steering
/sig contributor-experience

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[dims]

/committee steering

[thockin]

I agree with this. I started using SSH signing a while back - it's super easy and does not, IMO, represent any hardship to contributors.

I suppose we need:

• a database of known contributors' keys
• a bot that:
  • verifies that l commits from ostensibly known contributors have known signatures
  • flags commits from known contributors with unknown signatures (big scary)
  • flags any commits without signatures or with unknown signatures for closer review
• good docs on how to pull and verify the signature DB and configure git to use it

We still need to deal with commits from the world at large, signed or unsigned. I guess we just raise the bar on reviews?

There must be prior art here, but it's far from my expertise.

[jeefy]

If we are able to require SSH or GPG based keys to sign commits

AFAIK this is configurable at the branch level (OTOH I don't know if you can set a policy higher-up)

There must be prior art here, but it's far from my expertise.

I'll look into it this week and see if this is something we can (somewhat easily) suggest-or-adopt broadly

I guess we just raise the bar on reviews?

My meager two cents, this is probably the biggest preventative measure. Also means more cognitive load on maintainers :\

[aojea]

big +1

[BenTheElder]

I suppose we need:

a database of known contributors' keys
a bot that:
verifies that l commits from ostensibly known contributors have known signatures
flags commits from known contributors with unknown signatures (big scary)
flags any commits without signatures or with unknown signatures for closer review
good docs on how to pull and verify the signature DB and configure git to use it

This sounds incredibly unwieldy, and as you noted we need to allow contributions from those that are not existing "known" contributors. Are we going to require everyone who participates to submit a key to a special kubernetes key databsae?

Also worth noting: collaborative PR development / rebasing others's commits becomes ~impossible AFAIK.

One of the aspects (among many others!) is to ensure we require commits that get merged to be proven.

What does this "prove" vs PR review + merge controls?

If we're merging unreviewed code we have a big problem. We have controls in place to prevent this.

All commits should be traced to PRs currently, which then have author accounts. You can click through to the PR from the merge commit metadata.

If we are able to require SSH or GPG based keys to sign commits, then folks browsing GitHub UI or inspect our git tree can be confident of the source of the commits (somewhat!).

They still can't be, because anyone can push commits to a fork and PR it and have it show up "in the repo" thanks to github.
On the other hand if they're inspecting the log from one of our upstream branches, we already can say that these commits came from approved PRs.

-1

[aojea]

I thought this was advocating for this https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits , github is the database https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account

[BenTheElder]

github is the database https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account

In which case, I can just add a key to your account when I compromise the account, doesn't really gain us much if anything.

[aojea]

In which case, I can just add a key to your account when I compromise the account, doesn't really gain us much if anything.

don't we require 2FA for github accounts?
I don't think the goal is to find the ultimate and definitive solution, but at least reduce the chances of getting compromised with the minimum disruption
If leveraging existing github capabilities can give us a net improvement then it sounds reasonable to accept it, we are talking from random to at least a github authenticated account with 2FA that seems a net improvement to me

[thockin]

I'm about as far from a security expert as can be, but I don't really buy that signing is of no value. Defense in depth, right?

This sounds incredibly unwieldy, and as you noted we need to allow contributions from those that are not existing "known" contributors. Are we going to require everyone who participates to submit a key to a special kubernetes key databsae?

This is something to discuss. A good first step might be that anyone who has /approve access uses signed commits and that we cross-verify. Sort of akin to the idea of key-signing parties.

E.g. I make a commit adding my pubkey(s), and you can verify with me directly (IRL, Zoom, whatever). Henceforth, the project can trust that any commit signed by one of those keys was me (or that my privkey was compromised). Perfect? Is it "better enough" to be worth the effort? This is where I am ill equipped to judge.

Also worth noting: collaborative PR development / rebasing others's commits becomes ~impossible AFAIK.

If I absorb your commit and then rebase, the result would signed by me but still be attributed to you, right? Is that actively bad or just weird?

One of the aspects (among many others!) is to ensure we require commits that get merged to be proven.

What does this "prove" vs PR review + merge controls?

It proves that some commit which is merged into the codebase with my name is actually from me (or was rebased by someone who we trust). Heaven forbid someone gets push access to master, we could at least identify this. Would a non-PR push also trigger other alarms? I am not sure, truthfully.

All commits should be traced to PRs currently, which then have author accounts. You can click through to the PR from the merge commit metadata.

Do we actively verify this? I see that "Kubernetes Prow Robot" signs merge commits with key "B5690EEEBB952194" but I don't know why I should trust that key specifically and not any other? I can trivially fake a commit that looks like a merge-commit from Prow, with a different key. I also note that "Kubernetes Release Robot" does NOT sign commits.

If we are able to require SSH or GPG based keys to sign commits, then folks browsing GitHub UI or inspect our git tree can be confident of the source of the commits (somewhat!).

They still can't be, because anyone can push commits to a fork and PR it and have it show up "in the repo" thanks to github.

I'm not following - such a commit would not be signed by me.