git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Lundkvist Per <per.lundkvist@saabgroup.com>
To: Elijah Newren <newren@gmail.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [EXTERNAL] Re: Soundness of signature verification excluding unsigned empty merges
Date: Thu, 23 Mar 2023 09:55:46 +0000	[thread overview]
Message-ID: <f50de8a07d0448ae8929c5e0c8fba77c@saabgroup.com> (raw)
In-Reply-To: <CABPp-BFeB9av00=HUt3-Q=e_croHHOKfFu0Oc=axkTQEddG9nQ@mail.gmail.com>

Elijah Newren <newren@gmail.com> wrote:

...
> 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.

>
> 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.

>   * 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.

      reply	other threads:[~2023-03-23  9:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-21 10:32 Soundness of signature verification excluding unsigned empty merges Lundkvist Per
2023-03-21 16:43 ` Junio C Hamano
2023-03-22 11:50   ` [EXTERNAL] " Lundkvist Per
2023-03-22  1:41 ` Elijah Newren
2023-03-22 12:14   ` [EXTERNAL] " Lundkvist Per
2023-03-23  1:49     ` Elijah Newren
2023-03-23  9:55       ` Lundkvist Per [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f50de8a07d0448ae8929c5e0c8fba77c@saabgroup.com \
    --to=per.lundkvist@saabgroup.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).