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
