Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122882
Precedence: bulk
MIME-Version: 1.0
References: <3e988b3b-65b8-13d3-16cf-1296bfdd7ed2@php.net>
In-Reply-To: <3e988b3b-65b8-13d3-16cf-1296bfdd7ed2@php.net>
Date: Tue, 2 Apr 2024 19:27:32 +0200
Message-ID: <CAPyj-LC9Ygr0vNB3RyTKmmYmq1VbNgzDS_J3bq6NaoXD1YaMhQ@mail.gmail.com>
Subject: Re: [PHP-DEV] Requiring GPG Commit Signing
To: PHP Developers Mailing List <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: tovilo.ilija@gmail.com (Ilija Tovilo)

Hi Derick

On Tue, Apr 2, 2024 at 4:15=E2=80=AFPM Derick Rethans <derick@php.net> wrot=
e:
>
> What do y'all think about requiring GPG signed commits for the php-src
> repository?

Let me repost my internal response for visibility.

I'm currently struggling to understand what kind of attack signing
commits prevents.

If your GitHub account is compromised, GitHub allows the attacker to
commit via web interface and will happily sign their commits with a
gpg key auto-generated for your account.

See: https://docs.github.com/en/authentication/managing-commit-signature-ve=
rification/about-commit-signature-verification

> GitHub will automatically use GPG to sign commits you make using the web =
interface. Commits signed by GitHub will have a verified status. You can ve=
rify the signature locally using the public key available at https://github=
.com/web-flow.gpg.

Even if this wasn't the case, the attacker may simply register their
own gpg key in your account, with the commits appearing as verified.

If your ssh key is compromised instead, and you use ssh to sign your
commits, the attacker may sign their malicious commits with that same
key they may use to push.

The only thing this really seems to prevent is pushing commits via a
compromised ssh key, while commits need to be signed with gpg. If
that's the intention, we should require using gpg rather than ssh for
signing (or using a different ssh key, I suppose). Additionally, it
may help for people who push via HTTP+auth token, but that's probably
not advisable in the first place.

Something that may also help is restricting pushes to patch branches
(PHP-x.y.z) to release managers. These branches are not commonly
looked at by the public, and so it may be easier to sneak malicious
commits into them.

In addition, we should keep GitHub privileges narrow, especially
branch protection configuration.

As mentioned by others, this does not prevent the xz issue. But paired
with an auto-deployment solution, it could definitely help. It would
be even better if release managers cannot change CI, and CI
maintainers cannot create releases, as this essentially enforces the
4-eyes principle. The former may be hard to enforce, as CI lives in
the same repository.

Another solution might be to require PRs, and PR verifications. But
this will inevitably create overhead for maintainers.

Ilija