git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Trust issues with hooks and config files
@ 2014-03-06 21:47 Julian Brost
  2014-03-07 21:04 ` Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Julian Brost @ 2014-03-06 21:47 UTC (permalink / raw)
  To: git

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

I've noticed some behavior of git that might lead to some security
issues if the user is not aware of this.

Assume we have an evil user on a system, let's call him eve. He
prepares a repository where he allows other user to push changes to.
If he now adds a post-receive hook, git will happly execute it as
whatever user pushes to this repository:

  root@argon /tmp/git-eve # ls -l /tmp/git-eve/hooks/post-receive
  -rwxr-xr-x 1 eve users [...] /tmp/git-eve/hooks/post-receive
  root@argon /tmp/git-root # cat /tmp/git-eve/hooks/post-receive
  #!/bin/sh
  id
  root@argon /tmp/git-root # git push /tmp/git-eve master
  Counting objects: 3, done.
  Writing objects: 100% (3/3), 185 bytes | 0 bytes/s, done.
  Total 3 (delta 0), reused 0 (delta 0)
  remote: uid=0(root) gid=0(root) groups=0(root),[...]
  To /tmp/git-eve
   * [new branch]      master -> master

Something similiar might happen if eve adds some alias to the config
file in the repository and grants any other user read access to the
repository. These aliases will be executed when some other user is
running any git command in this repository. Even though git does not
allow defining aliases for existing commands, you might mistype
something, so adding an alias for "lg" instead of "log" might succeed:

  root@argon /tmp/git-eve # ls -l /tmp/git-eve/config
  -rw-r--r-- 1 eve users [...] /tmp/git-eve/config
  root@argon /tmp/git-eve # cat config
  [core]
  	repositoryformatversion = 0
  	filemode = true
  	bare = true
  [alias]
  	lg = !id
  root@argon /tmp/git-eve # git lg
  uid=0(root) gid=0(root) groups=0(root),[...]

This gets even worse if you know something about the aliases your
victim uses, so for example you can override an alias 'l = log'
defined in the user's config with something malicious in the
repository config file.

I'd suggest taking a similar approach as Mercurial [1], i.e. ignoring
configuration files and hooks owned by another user unless the owner
is explicitly trusted.

Regards,
Julian

[1] http://mercurial.selenic.com/wiki/Trust
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCAAGBQJTGOz5AAoJECLcYT6QIdBtJLEP/1VPMyRws5IYOVXJDcLukxkh
87RuL6ZCXE9v66VgEmTYYtJx1Umy18YXCx+ufAuJL2xzo/QH/QhWl/npa+U3ac7D
2A/3rXt1PvdzoQeT3514t5ntO9WyquHE2N8Ix+xdxwFo/T+Ve+nDV8/hra9he1Nb
zdldBccyHBDQdEudBLs6tDoJU9fvQ4TAGCGw7CXHCDV4hhyXHt8Nyf9nNOWxXgYh
5QcDs0sj1MCFm5AdN1SOU7FobiwS//Q8QdKdr9O6L18IoUPnSw2a1S2hGJmwQ/IL
Y1nQMdFuSx+4n6KKgUBtlo4WTi38u98FG4N0MXqZOSX7SKDVEOYfF+1W31Trhtuw
KMtojlwBYXsq0CWrW1OQ4Oed91lDGBtLLzF8MSCN1NoG4+Eb/V+RueLzulC5lWU/
IpDr3d14vFBEydHzYY+35P57Rf2Fl5HkXLQzQ0UmROeAmhUVCnduRj4dn35nb47Z
G/73UdgX1FMB4lOD8kD9KX0Sov3XLz4n5u706h+lElapd24wBXlaysWVpsmImuW0
xPLSpX0Dfrtj0sOCvqM0oX40z3bCJ1ibqZOmPGwF0P66DJOOq9sqDYfHlgnvt/qU
pCqsy0+FyCUuGP17UliEWcFAfdzXrUhxkRneQXC8ieX8YSoP4OtjzIPHrsc54s/2
7VR0wCTxaHvd05T8WruK
=kc4p
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Trust issues with hooks and config files
  2014-03-06 21:47 Trust issues with hooks and config files Julian Brost
@ 2014-03-07 21:04 ` Jeff King
  2014-03-09 17:27   ` Julian Brost
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2014-03-07 21:04 UTC (permalink / raw)
  To: Julian Brost; +Cc: git

On Thu, Mar 06, 2014 at 10:47:43PM +0100, Julian Brost wrote:

> I've noticed some behavior of git that might lead to some security
> issues if the user is not aware of this.
> 
> Assume we have an evil user on a system, let's call him eve. He
> prepares a repository where he allows other user to push changes to.
> If he now adds a post-receive hook, git will happly execute it as
> whatever user pushes to this repository:

Yes, this is a well-known issue. The only safe operation on a repository
for which somebody else controls hooks and config is to fetch from it
(upload-pack on the remote repository does not respect any dangerous
config or hooks).

> Something similiar might happen if eve adds some alias to the config
> file in the repository and grants any other user read access to the
> repository. These aliases will be executed when some other user is
> running any git command in this repository. Even though git does not
> allow defining aliases for existing commands, you might mistype
> something, so adding an alias for "lg" instead of "log" might succeed:

Much easier is to define an external diff or textconv tool; then the
victim does not even have to typo.

> I'd suggest taking a similar approach as Mercurial [1], i.e. ignoring
> configuration files and hooks owned by another user unless the owner
> is explicitly trusted

It has been discussed, but nobody has produced patches. I think that
nobody is really interested in doing so because:

  1. It introduces complications into previously-working setups (e.g., a
     daemon running as "nobody" serving repos owned by a "git" user
     needs to mark "git" as trusted).

  2. In most cases, cross-server boundaries provide sufficient
     insulation (e.g., I might not push to an evil person's repo, but rather
     to a shared repo whose hooks and config are controlled by root on
     the remote server).

If you want to work on it, I think it's an interesting area. But any
development would need to think about the transition plan for existing
sites that will be broken.

At the very least, the current trust model could stand to be documented
much better (I do not think the rule of "fetching is safe, everything
else is not" is mentioned anywhere explicitly).

-Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Trust issues with hooks and config files
  2014-03-07 21:04 ` Jeff King
@ 2014-03-09 17:27   ` Julian Brost
  2014-03-10 15:18     ` Junio C Hamano
  2014-03-16 13:45     ` Sitaram Chamarty
  0 siblings, 2 replies; 5+ messages in thread
From: Julian Brost @ 2014-03-09 17:27 UTC (permalink / raw)
  To: Jeff King; +Cc: git

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 07.03.2014 22:04, Jeff King wrote:
> Yes, this is a well-known issue. The only safe operation on a
> repository for which somebody else controls hooks and config is to
> fetch from it (upload-pack on the remote repository does not
> respect any dangerous config or hooks).

I'm a little bit surprised that you and some other people I asked see
this as such a low-priority problem as this makes social engineering
attacks on multi-user systems, like they are common at universities,
really easy (this is also how I noticed the problem).

> It has been discussed, but nobody has produced patches. I think
> that nobody is really interested in doing so because:
> 
> 1. It introduces complications into previously-working setups
> (e.g., a daemon running as "nobody" serving repos owned by a "git"
> user needs to mark "git" as trusted).
> 
> 2. In most cases, cross-server boundaries provide sufficient 
> insulation (e.g., I might not push to an evil person's repo, but
> rather to a shared repo whose hooks and config are controlled by
> root on the remote server).
> 
> If you want to work on it, I think it's an interesting area. But
> any development would need to think about the transition plan for
> existing sites that will be broken.

I can understand the problem with backward compatibility but in my
opinion the default behavior should definitely be to ignore untrusted
config files and hooks as it would otherwise only protect users that
are already aware of the issue anyways and manually enable this option.

Are there any plans for some major release in the future that would
allow introducing backward incompatible changes?

I would definitely spend some time working on a patch but so far I
have no idea of git's internals and never looked at the source before.

> At the very least, the current trust model could stand to be
> documented much better (I do not think the rule of "fetching is
> safe, everything else is not" is mentioned anywhere explicitly).

Good point but not enough in my opinion as I haven't read every git
manpage before running it for the first time.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCAAGBQJTHKRfAAoJECLcYT6QIdBtGdkQALD24YhS2aHBzi/c6a/1+eOC
xERUshDJvU/3picpTqqwFlqHB6X53vo7uXcCmJU8oaMsGLeLrTyrX77YOzrhlkuK
kERkXGPgyoW1G4enATSURDbjW6kp7SG0lRvGTPNySqDt9FiZYTDP7CGsQRDDl8cL
al50BoNFsx9K/kiPgbDJsenDi/MAclAYlHbHJnEB6aBo06G89zwC2tECFtXewnAD
EbCKPI4tFrUZW/rWxHAEDVs+cI038nMzSNMi7I+HAMG48s+iMfFF69pkdOQjhIsc
3irisLQcKPVNRjSK/dGEKbqkAy9wziNza69tl0EgQn6ewju5NZ4xbAkWQG9LEWfZ
Ux1safkumQsAKiYfn87t5YDXZ3vDYKbChKQv/UlicobVRm0YbhitY2AQAzu+wx1V
mXmG6D91IxfR4B0+AA+3/E8huSD4JJ2laIUwIoYV+y4+ZAlxnT4cNdYiYAH4oEme
wZ9R0wsxVfUm+uFdSBqsgEpv7Bp9PRcREuVXXz0GQH0wQ8zdwOeeNvA3v6ZtodRZ
0q7WOVJpd/3j4fQoWUXFAOZxDIZVorM0dQQvbhXiwOE9UY5Gwpq22lGKHM1t+8EO
lOUCQBXjscfPgyLifdSbnaf11RGgVLERQlpz6hpEGht1J3AqF6tTZPjto+iVV97v
nObYVtMXsAtrKbka2FF+
=ps1M
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Trust issues with hooks and config files
  2014-03-09 17:27   ` Julian Brost
@ 2014-03-10 15:18     ` Junio C Hamano
  2014-03-16 13:45     ` Sitaram Chamarty
  1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-03-10 15:18 UTC (permalink / raw)
  To: Julian Brost; +Cc: Jeff King, git

Julian Brost <julian@0x4a42.net> writes:

> On 07.03.2014 22:04, Jeff King wrote:
>> 
>> If you want to work on it, I think it's an interesting area. But
>> any development would need to think about the transition plan for
>> existing sites that will be broken.
>
> I can understand the problem with backward compatibility but in my
> opinion the default behavior should definitely be to ignore untrusted
> config files and hooks as it would otherwise only protect users that
> are already aware of the issue anyways and manually enable this option.
>
> Are there any plans for some major release in the future that would
> allow introducing backward incompatible changes?

Git 2.0 has been in the planning for quite some months, and I am
inclined to merge these topics prepared for that release to 'master'
during this cycle.  Anything new like this one is way too late for
it, but that does not mean we can never do 3.0 in the future.

Perhaps going this way might be possible:

 * Introduce a configuration that is read only from $HOME/.gitconfig
   (or its xdg equivalent) to enable or disable the "unsafe" behaviour.

   By default (i.e. when the above configuration is not set), allow
   "unsafe" read; when instructed by the above configuration to
   forbid "unsafe" read, ignore configuration files that are not
   owned by the owner of the process.  People can toggle the
   "unsafe" read to experiment with the above (~gitdaemon/.gitconfig
   can perhaps be used to restrict the daemon access)

   Keep it that way for a few releases.

 * After a few releases, start warning people who do not have the
   "unsafe" option in their $HOME/.gitconfig about a future default
   change, to force them to explicitly set it.

   Keep it that way for a few releases.

 * Flip the default, perhaps still keeping the warning on the
   flipped default to help people who have not been following along.

   Keep it that way for a few releases.

 * Then finally remove the warning.

A release cycle usually last 10-12 weeks on average.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Trust issues with hooks and config files
  2014-03-09 17:27   ` Julian Brost
  2014-03-10 15:18     ` Junio C Hamano
@ 2014-03-16 13:45     ` Sitaram Chamarty
  1 sibling, 0 replies; 5+ messages in thread
From: Sitaram Chamarty @ 2014-03-16 13:45 UTC (permalink / raw)
  To: Julian Brost, Jeff King; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 3176 bytes --]

On 03/09/2014 10:57 PM, Julian Brost wrote:
> On 07.03.2014 22:04, Jeff King wrote:
>> Yes, this is a well-known issue. The only safe operation on a
>> repository for which somebody else controls hooks and config is to
>> fetch from it (upload-pack on the remote repository does not
>> respect any dangerous config or hooks).
>
> I'm a little bit surprised that you and some other people I asked see
> this as such a low-priority problem as this makes social engineering
> attacks on multi-user systems, like they are common at universities,
> really easy (this is also how I noticed the problem).

This, and a lot more control related issues, are solved by tools like
gitolite (which I maintain) and Gerrit (from Google), and also many GUI
based access control tools like gitlab, gitorious, etc.

In these schemes the user does not have a "unix" account on the server,
and any hooks that run will run as the "hosting user".  Access is either
via ssh pub keys (most commonly) or http auth.

It is my belief that most multi-user systems have installed one of these
systems, and therefore the situation you speak of does not arise.  They
probably didn't install them to solve *this* problem, but to keep some
semblance of control over who can access what repo, but as a side-effect
they solve this problem also.

sitaram

>> It has been discussed, but nobody has produced patches. I think
>> that nobody is really interested in doing so because:
>
>> 1. It introduces complications into previously-working setups
>> (e.g., a daemon running as "nobody" serving repos owned by a "git"
>> user needs to mark "git" as trusted).
>
>> 2. In most cases, cross-server boundaries provide sufficient
>> insulation (e.g., I might not push to an evil person's repo, but
>> rather to a shared repo whose hooks and config are controlled by
>> root on the remote server).
>
>> If you want to work on it, I think it's an interesting area. But
>> any development would need to think about the transition plan for
>> existing sites that will be broken.
>
> I can understand the problem with backward compatibility but in my
> opinion the default behavior should definitely be to ignore untrusted
> config files and hooks as it would otherwise only protect users that
> are already aware of the issue anyways and manually enable this option.
>
> Are there any plans for some major release in the future that would
> allow introducing backward incompatible changes?
>
> I would definitely spend some time working on a patch but so far I
> have no idea of git's internals and never looked at the source before.
>
>> At the very least, the current trust model could stand to be
>> documented much better (I do not think the rule of "fetching is
>> safe, everything else is not" is mentioned anywhere explicitly).
>
> Good point but not enough in my opinion as I haven't read every git
> manpage before running it for the first time.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-03-16 13:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-06 21:47 Trust issues with hooks and config files Julian Brost
2014-03-07 21:04 ` Jeff King
2014-03-09 17:27   ` Julian Brost
2014-03-10 15:18     ` Junio C Hamano
2014-03-16 13:45     ` Sitaram Chamarty

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