Re: [EXTERNAL] Re: Soundness of signature verification excluding unsigned empty merges

From: Lundkvist Per <per.lundkvist@saabgroup.com>
Date: Mar 23, 2023, 09:55 UTC
Message-ID: <f50de8a07d0448ae8929c5e0c8fba77c@saabgroup.com>
In-Reply-To: <CABPp-BFeB9av00=HUt3-Q=e_croHHOKfFu0Oc=axkTQEddG9nQ@mail.gmail.com>

Elijah Newren <newren@gmail.com> wrote:

...

Show 15 quoted lines

> So long as you use --remerge-diff rather than --cc, you should be able
> to rely on "empty output means the merge would be clean -- with the
> current merge algorithm and config options -- and match the results
> recorded in the merge commit".  I designed it to behave exactly that
> way, so if anyone ever discovers a case where --remerge-diff gives
> empty output despite that not being true, then that would be a bug we
> would need to investigate and fix.  And even if someone did make a
> merge commit using some other algorithm where it conflicted, and they
> tweaked it to fix the conflicts, show --remerge-diff would only come
> back empty if the tweaks made by that individual with the other merge
> algorithm happen to match what you'd get with a clean merge with Git's
> current merge algorithm and config options.  So, you'd be pretty safe
> from false positives.  You might have to wade through a some false
> negatives when merge algorithms or config options change, but as long
> as you're prepared for that, it shouldn't be a big deal.

OK, sounds good. We will definitely use --remerge-diff.

Show 8 quoted lines

>
> So, if you did that, the major vectors left for an attacker to fool
> the "was this code from one of our employees, or from a merge from our
> CI system" that I can think of would be:
>   * trawl through your code review system to look for historical
> signed commits that passed CI with review comments like "We shouldn't
> do this; it'd open a security hole.", because they could then merge
> those bad changes.

Yes, this is the major issue.

Show 6 quoted lines

>   * find systems that sign commits automatically (in a big enough
> organization there always seem to be exceptions to human-signed stuff.
> Maybe a system that autofixes obvious code issues and pushes up an
> amended-and-signed commit, or something similar, and perhaps the
> autofixing doesn't check the input was signed, or the nature of the
> autofixing can be controlled somehow.)

I think we would not allow this at all.

>   * figure out how to access the system that records which gpg keys
> are associated with employees, and add extra attacker-controlled
> gpg-keys to existing employees.

...

We would require keys to be properly verified with the owner before being signed. Commits signed with unknown keys would be trivially detectable. Since OpenPGP supports a CA-like hierarchical trust model with delegation, we can actually simplify this procedure a bit. An attacker would have to get access to a CA private key, sign its own fake key(s) and upload this to our central system. If they are able to do that then we're in big trouble.

There is always the issue of establishing initial trust. They might try to inject their own CA keys for example. This requires not blindly trusting a key without first verifying its validity and following other manual procedures.