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

Sign commits with PGP #1047

Open
alejandro-colomar opened this issue Jul 9, 2024 · 6 comments
Open

Sign commits with PGP #1047

alejandro-colomar opened this issue Jul 9, 2024 · 6 comments
Assignees

Comments

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Jul 9, 2024

See also: #1042

I propose signing all commits with PGP.

See git-commit(1):

     -S[<keyid>], --gpg-sign[=<keyid>], --no-gpg-sign
         GPG-sign commits. The keyid argument is optional and defaults
         to the committer identity; if specified, it must be stuck to
         the option without a space.  --no-gpg-sign is useful to
         countermand both commit.gpgSign configuration variable, and
         earlier --gpg-sign.

Currently, the stable branches have all commits signed, since I push to them manually. However, since we apply PRs by rebasing them with the GH web UI, any signatures on the commits are discarded.

Having signatures on the commits would require that a malicious actor that wants to insert a commit in the master branch needs to have access to the PGP signing key of the impersonated author (or the bad commit would be easy to spot).

Cc: @ikerexxe , @hallyn , @thesamesam , @floppym

@hallyn
Copy link
Member

hallyn commented Jul 10, 2024

I have reservations, but am open to the idea. If we do this, we should not allow the github key that it uses for squash-and-merge. Only developer keys.

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Jul 10, 2024

I have reservations, but am open to the idea.

Let's express the pros and cons of each alternative:

  • Branch protection:

    • A malicious actor needs to compromise the GH credentials of one of us, to be able to push to master.
    • Once those credentials are stolen, it can impersonate any of us with the same credentials. GH logs will show whose credentials were used for a given commit (as we learned in this support ticket), but we need to identify the malicious commits. As we learned in the fallout of the XZ backdoor, it's not clear who did what, since the git metadata for author and committer are trivial to fake (and sometimes faked in normal maintenance operations), so it's not clear that we'd be able to identify the entire set of commits that were malicious.
  • GPG-signed commits:

    • A malicious user must compromise the GPG signing subkey of one of us to be able to create a commit.
    • A malicious user must compromise the SSH key of one of us to be able to push a commit.
    • Once those keys are stolen, only the owner of the GPG key will be impersonated, so the set of possible malicious commits is smaller. That is, once we detect something suspicious, we'd only need to investigate commits with the suspicious key.
    • A compromise of those keys is probably fatal, and means that the malicious actor probably has access to all other credentials of the owner. In my case, for example, my pass(1) password-store (where my GH credentials are stored) is encrypted with my GPG key, and if anyone has my GPG key, it probably has access to my computer and password-store. Only the master GPG key is safe from that, for revoking everything else.
    • We may not want to access those keys from certain computers, for keeping the security of those keys higher than the rest of credentials. For example, in a job-provided laptop, we wouldn't trust our GPG keyring. Which reduces the ability to contribute to this project.

I don't know if GH allows to have branch protection enabled per-contributor. If that was the case, we could have it enabled for you, @hallyn (and @ikerexxe if he wants it too) , since you often contribute from your job laptop. And I could have no branch protection, since I always carry my personal laptop, and would be able to use the key.

If we do this, we should not allow the github key that it uses for squash-and-merge. Only developer keys.

For sure; the github key would be basically saying that it was done via the github API, which adds no safety compared to the current status quo (which is a stronger guarantee that it was done via the GH API, and thus with valid GH credentials).

@ikerexxe
Copy link
Collaborator

Is this ticket still valid? If so, what are the consequences for the contributors, will their commits also have to be signed, or only the maintainers?

@alejandro-colomar
Copy link
Collaborator Author

Is this ticket still valid?

There's no pressure, as we haven't been owned. :-)

But I would still like to be able to sign my own commits, if it doesn't cause significant problems to you and @hallyn. So, it depends on that.

If so, what are the consequences for the contributors, will their commits also have to be signed, or only the maintainers?

The signature of a commit is made by the committer, not by the author. Since contributors don't have committer rights, they don't need to sign anything.


If branch protection can be enabled per user, I'd like to not have branch protection for myself, if you both agree.

If branch protection must be enabled or disabled for the entire project, let's keep it, since Serge doesn't seem convinced.

@hallyn
Copy link
Member

hallyn commented Jul 16, 2024

@alejandro-colomar well certainly nothing should stop you from signing your own commits, right? That shouldn't break anything for anyone.

I'd like to say that we should encourage contributors to sign their commits. However, if someone manages to take over someone's github account so as to be able to post a PR from there, then they can just as well upload new gpg keys, so in a sense I guess it doesn't do much for us.

@alejandro-colomar
Copy link
Collaborator Author

@alejandro-colomar well certainly nothing should stop you from signing your own commits, right? That shouldn't break anything for anyone.

I do sign them all. But since I cannot manually push, I need to push via github webUI. And then we have a rebase rule on GH, which obliterates my signatures. :-)

If I could fast-forward my commits, they'd be signed.

In fact, look at the latest PR you merged from me:
https://github.com/shadow-maint/shadow/pull/1038/commits

All the commits in the PR are signed; yet the commits on master aren't signed.

So, we need a way to git merge --ff-only. Whether that's via the command line, or GH, I don't care too much.

I'd like to say that we should encourage contributors to sign their commits.

Agree.

However, if someone manages to take over someone's github account so as to be able to post a PR from there, then they can just as well upload new gpg keys, so in a sense I guess it doesn't do much for us.

Someone with GH credentials can upload new gpg keys, which would lie to GH. However, it wouldn't lie to your local keyring (unless you download that key blindly).

Here's for example what I see in the 4.15.x branch:

$ git log --oneline --show-signature -3
70a572d4 (HEAD -> 4.15.x, stable/4.15.x, shadow/4.15.x) gpg: Signature made Mon Jul 15 0>
gpg:                using RSA key EA3A87F0A4EBA030E45DF2409E8C1AFBBEFFDB32
gpg: Good signature from "Alejandro Colomar <[email protected]>" [ultimate]
gpg:                 aka "Alejandro Colomar <[email protected]>" [ultimate]
gpg:                 aka "Alejandro Colomar Andres <[email protected]>" [ultimate]
lib/find_new_[gu]id.c: include stdint.h for UINT16_MAX/UINT32_MAX
69b9883f gpg: Signature made Mon Jul 15 00:57:25 2024 CEST
gpg:                using RSA key EA3A87F0A4EBA030E45DF2409E8C1AFBBEFFDB32
gpg: Good signature from "Alejandro Colomar <[email protected]>" [ultimate]
gpg:                 aka "Alejandro Colomar <[email protected]>" [ultimate]
gpg:                 aka "Alejandro Colomar Andres <[email protected]>" [ultimate]
lib/port.c: getportent(): Make sure the aren't too many fields in the CSV
b0b04dd1 gpg: Signature made Mon Jul 15 00:55:59 2024 CEST
gpg:                using RSA key EA3A87F0A4EBA030E45DF2409E8C1AFBBEFFDB32
gpg: Good signature from "Alejandro Colomar <[email protected]>" [ultimate]
gpg:                 aka "Alejandro Colomar <[email protected]>" [ultimate]
gpg:                 aka "Alejandro Colomar Andres <[email protected]>" [ultimate]

(That's too noisy; I have some compact view:)

$ git lg -3
* 70a572d47dd4 G - 2024-07-07 13:31:41 +0200 (9 days ago) (HEAD -> 4.15.x, stable/4.15.x>
|   lib/find_new_[gu]id.c: include stdint.h for UINT16_MAX/UINT32_MAX - Chris Hofstaedtl>
* 69b9883feeda G - 2024-07-02 14:51:04 +0200 (2 weeks ago)
|   lib/port.c: getportent(): Make sure the aren't too many fields in the CSV - Alejandr>
* b0b04dd109bf G - 2024-07-02 14:43:26 +0200 (2 weeks ago)
|   lib/port.c: getportent(): Make sure there are at least 2 ':' in the line - Alejandro>

The G to the right of the commit sha is the abbreviation for Good signature.

To impersonate me properly, one would have to make you believe that new key is my new key. Please don't trust anyone claiming to say a new key is mine. If I ever get a new PGP key, be assured that it will be certified (not signed) with my current (master) key; and most likely, I'll also publish a revocation of the current master together with it. I have many backups of that master, so I don't think I'll ever lose the key (hopefully).

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

3 participants