Hi,
What do y'all think about requiring GPG signed commits for the php-src
repository?
I had a look, and this is also something we can enforce through GitHub
as well (by using branch protections).
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: Xdebug: Support
mastodon: @derickr@phpc.social @xdebug@phpc.social
Am 02.04.2024 um 16:15 schrieb Derick Rethans:
What do y'all think about requiring GPG signed commits for the php-src
repository?
+1
On Tue, Apr 2, 2024 at 3:17 PM Derick Rethans <derick@php.net> wrote:
Hi,
What do y’all think about requiring GPG signed commits for the php-src
repository?
+1, most of the devs already do that. I CC’d few of the regular devs that don’t sign commits (taken from the latest history) so they are aware of this.
Cheers
Jakub
On Tue, Apr 2, 2024 at 3:36 PM Jakub Zelenka <bukka@php.net> wrote:
On Tue, Apr 2, 2024 at 3:17 PM Derick Rethans <derick@php.net> wrote:
Hi,
What do y’all think about requiring GPG signed commits for the php-src
repository?+1, most of the devs already do that. I CC’d few of the regular devs that don’t sign commits (taken from the latest history) so they are aware of this.
I meant regular committers, of course.
No problem with this, I apply this since couple of days.
Cheers.
On Tue, 2 Apr 2024 at 15:37, Jakub Zelenka <bukka@php.net> wrote:
On Tue, Apr 2, 2024 at 3:36 PM Jakub Zelenka <bukka@php.net> wrote:
On Tue, Apr 2, 2024 at 3:17 PM Derick Rethans <derick@php.net> wrote:
Hi,
What do y’all think about requiring GPG signed commits for the php-src
repository?+1, most of the devs already do that. I CC’d few of the regular devs that don’t sign commits (taken from the latest history) so they are aware of this.
I meant regular committers, of course.
On Apr 2, 2024, at 11:15 AM, Derick Rethans <derick@php.net> wrote:
What do y'all think about requiring GPG signed commits for the php-src
repository?I had a look, and this is also something we can enforce through GitHub
as well (by using branch protections).
Would this affect only direct pushes to master, or would it be required
for pull requests too? I'd be worried the average drive-by contributor
wouldn't have GPG signing set up.
On Tue, 2 Apr 2024, Calvin Buckley wrote:
On Apr 2, 2024, at 11:15 AM, Derick Rethans <derick@php.net> wrote:
>
> What do y'all think about requiring GPG signed commits for the php-src
> repository?
>
> I had a look, and this is also something we can enforce through GitHub
> as well (by using branch protections).Would this affect only direct pushes to master, or would it be required
for pull requests too? I'd be worried the average drive-by contributor
wouldn't have GPG signing set up.
As Ayesh said, you can also use SSH for this now:
I think it would apply to people merging the commits. But, I am not 100%
sure (until we try, I suppose).
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: Xdebug: Support
mastodon: @derickr@phpc.social @xdebug@phpc.social
Hi,
What do y'all think about requiring GPG signed commits for the php-src
repository?I had a look, and this is also something we can enforce through GitHub
as well (by using branch protections).cheers,
Derick--
https://derickrethans.nl | https://xdebug.org | https://dram.ioAuthor of Xdebug. Like it? Consider supporting me: Xdebug: Support
mastodon: @derickr@phpc.social @xdebug@phpc.social
+1 from me as well, and quite good timing with all the xz fiasco just last week.
Git can also sign with SSH keys now, so this is now merely a config update
On 02/04/2024 15:55, Calvin Buckley wrote:
On Apr 2, 2024, at 11:15 AM, Derick Rethans<derick@php.net> wrote:
What do y'all think about requiring GPG signed commits for the php-src
repository?I had a look, and this is also something we can enforce through GitHub
as well (by using branch protections).Would this affect only direct pushes to master, or would it be required
for pull requests too? I'd be worried the average drive-by contributor
wouldn't have GPG signing set up.
FWIW, I'm a drive-by contributor and I have GPG signing <Changed DateTime(Immutable)::setDate parameters to all be optional by Bilge · Pull Request #13845 · php/php-src · GitHub; set up.
On Tue, Apr 2, 2024 at 4:16 PM Derick Rethans <derick@php.net> wrote:
What do y’all think about requiring GPG signed commits for the php-src
repository?
+1
On Tue, 2 Apr 2024, at 15:15, Derick Rethans wrote:
Hi,
What do y'all think about requiring GPG signed commits for the php-src
repository?
I actually thought this was already required since the github move (and the events that led to it) 3 years ago.
It was certainly discussed: Changes to Git commit workflow - Externals and a user guide was created on the PHP wiki: PHP: vcs:commit-signing
Feedback for the idea was generally positive, but maybe nobody got around to actually doing it.
--
Rowan Tommins
[IMSoP]
On 02/04/2024 16:15, Derick Rethans wrote:
Hi,
What do y'all think about requiring GPG signed commits for the php-src
repository?I had a look, and this is also something we can enforce through GitHub
as well (by using branch protections).cheers,
Derick
I'm in favor of this.
Hey List, Hey Derick
Am 02.04.24 um 16:15 schrieb Derick Rethans:
Hi,
What do y'all think about requiring GPG signed commits for the php-src
repository?
In general I think it is a good idea to do GPG signed commits. But in terms of security the idea is to be able to authenticate a user. But the only thing we truly and reliably can do is connect a github account to a commit. Whether that commit author is actually Jane Doe or Karl Napp is still not necessarily proven.
So if we want to make sure that something like XY doesn't happen, we have to add some additional restrictions to those GPG keys.
If it is just to have signed commits: I am absolutely in favour.
Cheers
Andreas
--
,
(o o)
+---------------------------------------------------------ooO-(_)-Ooo-+
| Andreas Heigl |
| mailto:andreas@heigl.org N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org |
+---------------------------------------------------------------------+
| https://hei.gl/appointmentwithandreas |
+---------------------------------------------------------------------+
| GPG-Key: https://hei.gl/keyandreasheiglorg |
+---------------------------------------------------------------------+
On 02/04/2024 18:27, Ilija Tovilo wrote:
If your GitHub account is compromised,
[...] 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 key point (pun not intended) is that git doesn't record who pushed a commit - pushing is just data synchronization, not part of the history. What it records is who "authored" the commit, and by default that's just plain text; so if somebody compromises an SSH key or access token authorised to your GitHub account, they can push commits "authored by" Derick, or Nikita, or Bill Gates, and there is no way to tell them apart from the real thing.
In fact, you don't need to compromise anybody's key: you could socially engineer a situation where you have push access to the repository, or break the security in some other way. As I understand it, this is exactly what happened 3 years ago: someone gained direct write access to the git.php.net server, and added commits "authored by" Nikita and others to the history in the repository.
If all commits are signed, a compromised key or account can only be used to sign commits with that specific identity: your GitHub account can't be used to sign commits as Derick or Nikita, only as you. The impact is limited to one identity, not the integrity of the entire repository.
Regards,
--
Rowan Tommins
[IMSoP]
Hi Derick
On Tue, Apr 2, 2024 at 4:15 PM 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: About commit signature verification - GitHub Docs
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 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
On Tue, Apr 2, 2024, at 5:27 PM, Ilija Tovilo wrote:
Hi Derick
On Tue, Apr 2, 2024 at 4:15 PM 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:
About commit signature verification - GitHub DocsGitHub 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 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
Coming from corporate projects at the moment, I always hard-block pushing 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 doesn't do that. Yes, it's a little more overhead, but with the larger team we now have (thanks to the Foundation) I believe the human-security checks it gives us are well worth it. (And just from a technical standpoint, even the best developer goofs up and needs their code reviewed by someone.)
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 first time as GPG has always been a UX nightmare. 
--Larry Garfield
On Tue, Apr 2, 2024 at 8:45 PM Rowan Tommins [IMSoP] <imsop.php@rwec.co.uk> wrote:
On 02/04/2024 20:02, Ilija Tovilo wrote:
But, does it matter? I'm not sure we look at some commits closer than others, based on its author. It's true that it might be easier to identify malicious commits if they all come from the same user, but it wouldn't prevent them.It’s like the difference between stealing someone’s credit card, and cloning the card of everyone who comes into the shop: in the first case, someone needs to check their credit card statements carefully; in the second, you’ll have a hard job even working out who to contact.
Similarly, if you discover a compromised key or signing account, you can look for uses of that key or account, which might be a tiny number from a non-core contributor; if you discover a compromised account pushing unsigned commits, you have to audit every commit in the repository.
I agree it’s not a complete solution, but no security measure is; it’s always about reducing the attack surface or limiting the damage.
Nice comparison. Fully agree with that. I would add that potentially even more important point than auditability is possibility to revoke access of the compromised account as otherwise you can’t easily identify such account and prevent further issues.
Regards
Jakub
Hi Rowan
On Tue, Apr 2, 2024 at 8:48 PM Rowan Tommins [IMSoP]
<imsop.php@rwec.co.uk> wrote:
In fact, you don't need to compromise anybody's key: you could socially engineer a situation where you have push access to the repository, or break the security in some other way. As I understand it, this is exactly what happened 3 years ago: someone gained direct write access to the git.php.net server, and added commits "authored by" Nikita and others to the history in the repository.
Right, but I would like to believe that attaining push access _without
gaining access to a maintainers account_ should be substantially
harder on GitHub than our self-hosted git server.
If all commits are signed, a compromised key or account can only be used to sign commits with that specific identity: your GitHub account can't be used to sign commits as Derick or Nikita, only as you. The impact is limited to one identity, not the integrity of the entire repository.
But, does it matter? I'm not sure we look at some commits closer than
others, based on its author. It's true that it might be easier to
identify malicious commits if they all come from the same user, but it
wouldn't prevent them.
To be clear: I'm not against commit signing, I've been doing it for
years. I'm just unsure if it's a sufficient solution (apart from
releases, which are a whole different can of worms).
Ilija
On 02/04/2024 20:02, Ilija Tovilo wrote:
But, does it matter? I'm not sure we look at some commits closer than
others, based on its author. It's true that it might be easier to
identify malicious commits if they all come from the same user, but it
wouldn't prevent them.
It's like the difference between stealing someone's credit card, and cloning the card of everyone who comes into the shop: in the first case, someone needs to check their credit card statements carefully; in the second, you'll have a hard job even working out who to contact.
Similarly, if you discover a compromised key or signing account, you can look for uses of that key or account, which might be a tiny number from a non-core contributor; if you discover a compromised account pushing unsigned commits, you have to audit every commit in the repository.
I agree it's not a complete solution, but no security measure is; it's always about reducing the attack surface or limiting the damage.
Regards,
--
Rowan Tommins
[IMSoP]
On Tue, Apr 2, 2024 at 9:43 PM Rowan Tommins [IMSoP]
<imsop.php@rwec.co.uk> wrote:
Similarly, if you discover a compromised key or signing account, you can look for uses of that key or account, which might be a tiny number from a non-core contributor; if you discover a compromised account pushing unsigned commits, you have to audit every commit in the repository.
Right, that and what Jakub mentioned are fair arguments.
I agree it's not a complete solution, but no security measure is; it's always about reducing the attack surface or limiting the damage.
Right. That was the original intention of my e-mail: To point out that
we might also want to consider other mitigations. Not that we
shouldn't do commit signing.
Ilija