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 <der...@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: > https://docs.github.com/en/authentication/managing-commit-signature-verification/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 >> 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
