shadow-maint/shadow

Sign commits with PGP

alejandro-colomar opened this issue · 6 comments

alejandro-colomar commented

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 commented

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.

  • 👍1
alejandro-colomar commented

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 commented

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 commented

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 commented

@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 commented

@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 <alx@alejandro-colomar.es>" [ultimate]
gpg:                 aka "Alejandro Colomar <alx@kernel.org>" [ultimate]
gpg:                 aka "Alejandro Colomar Andres <alx.manpages@gmail.com>" [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 <alx@alejandro-colomar.es>" [ultimate]
gpg:                 aka "Alejandro Colomar <alx@kernel.org>" [ultimate]
gpg:                 aka "Alejandro Colomar Andres <alx.manpages@gmail.com>" [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 <alx@alejandro-colomar.es>" [ultimate]
gpg:                 aka "Alejandro Colomar <alx@kernel.org>" [ultimate]
gpg:                 aka "Alejandro Colomar Andres <alx.manpages@gmail.com>" [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).