git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Santiago Torres <santiago@nyu.edu>
Cc: Git <git@vger.kernel.org>
Subject: Re: [RFC] Malicously tampering git metadata?
Date: Fri, 18 Dec 2015 18:10:32 -0500	[thread overview]
Message-ID: <20151218231032.GA16904@thunk.org> (raw)
In-Reply-To: <20151216032639.GA1901@LykOS>

On Tue, Dec 15, 2015 at 10:26:39PM -0500, Santiago Torres wrote:
> 4) The developer pushes to upstream. All the traffic can be re-routed
> back to the original repository. The target branch now contains a
> vulnerable piece of code.

I assume you are assuming here that the "push to upstream" doesn't
involve some kind of human verification?  If someone tried pushing
something like this to Linus, he would be checking the git diff stats
and git commit structure for things that might look like "developer
negligence".  He's been known to complain to subsystem developers in
his own inimitable when the git commit structure looks suspicious, so
I'm pretty sure he would notice this.

But normally that developnment process we don't talk about "pushing to
upstream" as much as "requesting a pull".  So it would be useful when
you describe the attack to explicit describe the development workflow
that is vulnerable to your attack.

For example, in my use case, I'm authorative over changes in fs/ext4.
So when I pull from Linus's repo, I examine (using "gitk fs/ext4") all
commits coming from upstream that modify fs/ext4.  So if someone tries
introducing a change in fs/ext4 coming from "upstream", I will notice.
Then when I request a pull request from Linus, the git pull request
describes what commits are new in my tree that are not in his, and
shows the diffstats from upstream.  When Linus verifies my pull, there
are multiple opportunities where he will notice funny business:

a) He would notice that my origin commit is something that is not in
his upstream tree.

b) His diffstat is different from my diffstat (since thanks to the
man-in-the middle, the conception of what commits are new in the git
pull request will be different from his).

c) His diffstat will show that files outside of fs/ext4 have been
modified, which is a red flag that will merit more close examination.
(And if the attacker had tried to introduce a change in fs/ext4, I
would have noticed when I pulled from the man-in-the-middle git
repo.)

Now, if there is zero checking when the user pushes to upstream, then
yes, there are all sorts of potential problems.  But that's one of the
reasons why it's generally considered a good thing for Linux
developers to use as the origin commit for their work official
releases (which can be demarked using GPG-signed git tags).

So for example, the changes for ext4 that were sent to Linus for v4.4
was based off of v4.3-rc2:

git tag  --verify v4.3-rc2
object 1f93e4a96c9109378204c147b3eec0d0e8100fde
type commit
tag v4.3-rc2
tagger Linus Torvalds <torvalds@linux-foundation.org> 1442784761 -0700

Linux 4.3-rc2
gpg: Signature made Sun 20 Sep 2015 05:32:41 PM EDT using RSA key ID 00411886
gpg: Good signature from "Linus Torvalds <torvalds@linux-foundation.org>" [full]


And the changes which I sent to Linus were also signed by a tag, and
better yet, someone can indepedently verify this after the fact:

% git show --oneline --show-signature f41683a204ea61568f0fd0804d47c19561f2ee39
f41683a merged tag 'ext4_for_linus_stable'
gpg: Signature made Sun 06 Dec 2015 10:35:27 PM EST using RSA key ID 950D81A3
gpg: Good signature from "Theodore Ts'o <tytso@mit.edu>" [ultimate]
gpg:                 aka "Theodore Ts'o <tytso@debian.org>" [ultimate]
gpg:                 aka "Theodore Ts'o <tytso@google.com>" [ultimate]

They can also verify that the chain of commits that I sent to Linus
were rooted in Linus's signed v4.3-rc2 tag, so this kind of
after-the-fact auditing means that if there *were* funny business, it
could be caught even if Linus slipped up in his checking.


Now, the crazy behavior where github users randomly and promiscuously
do pushes and pull without doing any kind of verification may very
well be dangerous.  But so is someone who saves a 80 patch series from
their inbox, and without reading or verifying all of the patches
applies them blindly to their tree using "git am" --- or if they were
using cvs or svn, bulk applied the patches without doing any
verification....

Cheers,

					- Ted

  parent reply	other threads:[~2015-12-18 23:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-16  3:26 [RFC] Malicously tampering git metadata? Santiago Torres
2015-12-16  7:20 ` Stefan Beller
2015-12-18  1:06   ` Santiago Torres
2015-12-18  3:55     ` Jeff King
2015-12-18  4:02 ` Jeff King
2015-12-18 23:10 ` Theodore Ts'o [this message]
2015-12-19 17:30   ` Santiago Torres
2015-12-20  1:28     ` Theodore Ts'o
2016-01-12 18:21       ` Santiago Torres
2016-01-12 18:39         ` Stefan Beller
2016-01-14 17:16           ` Santiago Torres
2016-01-14 17:21             ` Stefan Beller
2016-01-22 18:00               ` Santiago Torres
2016-01-22 18:51                 ` Stefan Beller

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

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

  git send-email \
    --in-reply-to=20151218231032.GA16904@thunk.org \
    --to=tytso@mit.edu \
    --cc=git@vger.kernel.org \
    --cc=santiago@nyu.edu \
    /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.
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 NNTP newsgroup(s).