Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122884
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-333-gbfea15422e-fm-20240327.001-gbfea1542
Precedence: bulk
MIME-Version: 1.0
Message-ID: <b077eff7-d25b-4491-ba97-cc4cfa9cf9bf@app.fastmail.com>
In-Reply-To: 
 <CAPyj-LC9Ygr0vNB3RyTKmmYmq1VbNgzDS_J3bq6NaoXD1YaMhQ@mail.gmail.com>
References: <3e988b3b-65b8-13d3-16cf-1296bfdd7ed2@php.net>
 <CAPyj-LC9Ygr0vNB3RyTKmmYmq1VbNgzDS_J3bq6NaoXD1YaMhQ@mail.gmail.com>
Date: Tue, 02 Apr 2024 17:44:16 +0000
To: "php internals" <internals@lists.php.net>
Subject: Re: [PHP-DEV] Requiring GPG Commit Signing
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: larry@garfieldtech.com ("Larry Garfield")

On Tue, Apr 2, 2024, at 5:27 PM, Ilija Tovilo wrote:
> Hi Derick
>
> On Tue, Apr 2, 2024 at 4:15=E2=80=AFPM Derick Rethans <derick@php.net>=
 wrote:
>>
>> 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:=20
> 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 verify the signature locally using the public key available at http=
s://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

Coming from corporate projects at the moment, I always hard-block pushin=
g straight to the master branch.  Everything goes through a PR, and has =
to be approved by someone other than the author, guaranteeing 4 eyes for=
 every line of code.  And that's for internal backend services.

It's always struck me as mind-boggling that a project the size of PHP do=
esn't do that.  Yes, it's a little more overhead, but with the larger te=
am we now have (thanks to the Foundation) I believe the human-security c=
hecks it gives us are well worth it.  (And just from a technical standpo=
int, even the best developer goofs up and needs their code reviewed by s=
omeone.)

I have no particular input on the code signing front, other than please =
have clear documentation to follow for someone setting it up for the fir=
st time as GPG has always been a UX nightmare. :-)

--Larry Garfield