[PATCH] Allow git-shell to be used as a ssh forced-command

[PATCH] Allow git-shell to be used as a ssh forced-command

[Mike Hommey]

When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
variable to what would otherwise be passed to $SHELL -c. When this
variable is set, we use it instead of the contents of argv.

Signed-off-by: Mike Hommey <mh@glandium.org>
---

I was unsure whether I needed to give more information about
forced-commands in the commit message itself, anyways, just in case
you don't know what it is:
http://oreilly.com/catalog/sshtdg/chapter/ch08.html#22858

I'm not sure if it's worth adding a check for SSH2_ORIGINAL_COMMAND.
Are people using the commercial SSH2 ?

 shell.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/shell.c b/shell.c
index e339369..14ff266 100644
--- a/shell.c
+++ b/shell.c
@@ -62,20 +62,25 @@ int main(int argc, char **argv)
 		die("opening /dev/null failed (%s)", strerror(errno));
 	close (devnull_fd);
 
-	/*
-	 * Special hack to pretend to be a CVS server
-	 */
-	if (argc == 2 && !strcmp(argv[1], "cvs server"))
-		argv--;
+	/* Use original command if we were run from a ssh forced-command */
+	prog = getenv("SSH_ORIGINAL_COMMAND");
+	if (!prog) {
+		/*
+		 * Special hack to pretend to be a CVS server
+		 */
+		if (argc == 2 && !strcmp(argv[1], "cvs server"))
+			argv--;
 
-	/*
-	 * We do not accept anything but "-c" followed by "cmd arg",
-	 * where "cmd" is a very limited subset of git commands.
-	 */
-	else if (argc != 3 || strcmp(argv[1], "-c"))
-		die("What do you think I am? A shell?");
+		/*
+		 * We do not accept anything but "-c" followed by "cmd arg",
+		 * where "cmd" is a very limited subset of git commands.
+		 */
+		else if (argc != 3 || strcmp(argv[1], "-c"))
+			die("What do you think I am? A shell?");
+
+		prog = argv[2];
+	}
 
-	prog = argv[2];
 	if (!strncmp(prog, "git", 3) && isspace(prog[3]))
 		/* Accept "git foo" as if the caller said "git-foo". */
 		prog[3] = '-';
-- 
1.6.3.rc0.1.g8bd72.dirty

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Dmitry Potapov]

On Thu, Apr 16, 2009 at 11:10:56PM +0200, Mike Hommey wrote:
> When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> variable to what would otherwise be passed to $SHELL -c. When this
> variable is set, we use it instead of the contents of argv.

It would be nice to provide some justification where it can be used.
IOW, why do you want to have the force command where essentially
you execute the original command as it were no force-command?

Dmitry

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Mike Hommey]

On Fri, Apr 17, 2009 at 08:24:35PM +0400, Dmitry Potapov wrote:
> On Thu, Apr 16, 2009 at 11:10:56PM +0200, Mike Hommey wrote:
> > When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> > variable to what would otherwise be passed to $SHELL -c. When this
> > variable is set, we use it instead of the contents of argv.
> 
> It would be nice to provide some justification where it can be used.
> IOW, why do you want to have the force command where essentially
> you execute the original command as it were no force-command?

You're not executing any command, but only what git-shell allows.
This allows git-shell to be set as a forced-command for a specific
ssh key, for example.

Mike

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Dmitry Potapov]

On Fri, Apr 17, 2009 at 09:49:19PM +0200, Mike Hommey wrote:
> On Fri, Apr 17, 2009 at 08:24:35PM +0400, Dmitry Potapov wrote:
> > On Thu, Apr 16, 2009 at 11:10:56PM +0200, Mike Hommey wrote:
> > > When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> > > variable to what would otherwise be passed to $SHELL -c. When this
> > > variable is set, we use it instead of the contents of argv.
> >_
> > It would be nice to provide some justification where it can be used.
> > IOW, why do you want to have the force command where essentially
> > you execute the original command as it were no force-command?
>_
> You're not executing any command, but only what git-shell allows.
> This allows git-shell to be set as a forced-command for a specific
> ssh key, for example.

Would it better to set git-shell as the login shell for this account?
IMHO, that provides better security than using a forced-command, and
that is how git-shell is intended to use. So, I am not sure what are
benefits of using a forced-command when it just executes the original
command using git-shell.

Besides, you made SSH_ORIGINAL_COMMAND to take precedent over explicitly
specified parameters given to git-shell. Maybe it should be the other
way around?

Dmitry

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Mike Hommey]

On Sat, Apr 18, 2009 at 01:00:35AM +0400, Dmitry Potapov wrote:
> On Fri, Apr 17, 2009 at 09:49:19PM +0200, Mike Hommey wrote:
> > On Fri, Apr 17, 2009 at 08:24:35PM +0400, Dmitry Potapov wrote:
> > > On Thu, Apr 16, 2009 at 11:10:56PM +0200, Mike Hommey wrote:
> > > > When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> > > > variable to what would otherwise be passed to $SHELL -c. When this
> > > > variable is set, we use it instead of the contents of argv.
> > >_
> > > It would be nice to provide some justification where it can be used.
> > > IOW, why do you want to have the force command where essentially
> > > you execute the original command as it were no force-command?
> >_
> > You're not executing any command, but only what git-shell allows.
> > This allows git-shell to be set as a forced-command for a specific
> > ssh key, for example.
> 
> Would it better to set git-shell as the login shell for this account?
> IMHO, that provides better security than using a forced-command, and
> that is how git-shell is intended to use. So, I am not sure what are
> benefits of using a forced-command when it just executes the original
> command using git-shell.

You may want to provide other kind of accesses for the same account.
Also, an unpriviledged user would be able to, without root access, setup
write access to his private git repositories via forced-commands for
given ssh keys.

> Besides, you made SSH_ORIGINAL_COMMAND to take precedent over explicitly
> specified parameters given to git-shell. Maybe it should be the other
> way around?

If SSH_ORIGINAL_COMMAND is set, which means forced-commands are in use,
I don't see why other parameters should be more important.

Mike

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Shawn O. Pearce]

Mike Hommey <mh@glandium.org> wrote:
> On Sat, Apr 18, 2009 at 01:00:35AM +0400, Dmitry Potapov wrote:
> > Besides, you made SSH_ORIGINAL_COMMAND to take precedent over explicitly
> > specified parameters given to git-shell. Maybe it should be the other
> > way around?
> 
> If SSH_ORIGINAL_COMMAND is set, which means forced-commands are in use,
> I don't see why other parameters should be more important.

Gitosis runs by forced-command, and invokes git-shell to get to
the real requested git command.  I'm not sure if it unsets the
SSH_ORIGINAL_COMMAND environment variable before launching the
git-shell...

But, in git, command line arguments override environment variables,
which in turn override config files, which override any defaults
that may be guessed from the host.  So Dimtry is asking that you
follow that same convention.

-- 
Shawn.

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Mike Hommey]

On Fri, Apr 17, 2009 at 03:41:04PM -0700, Shawn O. Pearce wrote:
> Mike Hommey <mh@glandium.org> wrote:
> > On Sat, Apr 18, 2009 at 01:00:35AM +0400, Dmitry Potapov wrote:
> > > Besides, you made SSH_ORIGINAL_COMMAND to take precedent over explicitly
> > > specified parameters given to git-shell. Maybe it should be the other
> > > way around?
> > 
> > If SSH_ORIGINAL_COMMAND is set, which means forced-commands are in use,
> > I don't see why other parameters should be more important.
> 
> Gitosis runs by forced-command, and invokes git-shell to get to
> the real requested git command.  I'm not sure if it unsets the
> SSH_ORIGINAL_COMMAND environment variable before launching the
> git-shell...
> 
> But, in git, command line arguments override environment variables,
> which in turn override config files, which override any defaults
> that may be guessed from the host.  So Dimtry is asking that you
> follow that same convention.

Except SSH_ORIGINAL_COMMAND is not a traditional environment variable
like GIT_USER_EMAIL or such. SSH_ORIGINAL_COMMAND is the only way to
know from a ssh forced-command what the remote wanted to call
originally. Setting a forced-command with options after git-shell, which
would have to be "-c something" or "cvs server", is pointless, because
you don't need it and should put that "something" as forced-command directly.

However, the patch in its current form will definitely break gitosis if
it doesn't unset SSH_ORIGINAL_COMMAND.

Mike

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Matthieu Moy]

Mike Hommey <mh@glandium.org> writes:

> However, the patch in its current form will definitely break gitosis if
> it doesn't unset SSH_ORIGINAL_COMMAND.

... and any home-made script without knowledge of this feature. If I
wanted to add some restrictions to git-shell, it would seem natural to
me to write a script like

#! /bin/sh

if [ ??? ]; then
	git-shell $whatever
else
	echo "Sorry, forbidden"
	exit 1
fi

(I never did this with Git because I never had to manage any kind of
permission control with it, but I have a script like that for SVN that
adds some argument to the SVN command)

If the command ignores its arguments, and use some other environment
variable instead, then the security hole is not far.

-- 
Matthieu

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> variable to what would otherwise be passed to $SHELL -c. When this
> variable is set, we use it instead of the contents of argv.

How does this interact with a typical gitosis installation?  I think it
extracts bits from the ORIGINAL_COMMAND and formulates a sanitized command
line and then passes it to git-shell but I do not think it removes the
variable from the environment.

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Tommi Virtanen]

On Fri, Apr 17, 2009 at 04:40:54PM -0700, Junio C Hamano wrote:
> How does this interact with a typical gitosis installation?  I think it
> extracts bits from the ORIGINAL_COMMAND and formulates a sanitized command
> line and then passes it to git-shell but I do not think it removes the
> variable from the environment.

Yup, not sanitizing. I could make that change easily, if you want, but
there's plenty of installations out there already.

I'd say add a git-shell flag that enables any magic behavior, don't
just magically work differently based on that environment variable.
Sometimes that is set for reasons that are not related to what you're
actually running in the end; e.g. "chroot /foo exec-whatever-i-gave-you".

-- 
:(){ :|:&};:

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Junio C Hamano]

Tommi Virtanen <tv@eagain.net> writes:

> On Fri, Apr 17, 2009 at 04:40:54PM -0700, Junio C Hamano wrote:
>> How does this interact with a typical gitosis installation?  I think it
>> extracts bits from the ORIGINAL_COMMAND and formulates a sanitized command
>> line and then passes it to git-shell but I do not think it removes the
>> variable from the environment.
>
> Yup, not sanitizing. I could make that change easily, if you want, but
> there's plenty of installations out there already.
>
> I'd say add a git-shell flag that enables any magic behavior, don't
> just magically work differently based on that environment variable.

Good.  I see Mike too already realized that the patch is unacceptable in
the current form.

Re: [PATCH] Allow git-shell to be used as a ssh forced-command

[Mike Hommey]

On Fri, Apr 17, 2009 at 04:48:59PM -0700, Tommi Virtanen wrote:
> On Fri, Apr 17, 2009 at 04:40:54PM -0700, Junio C Hamano wrote:
> > How does this interact with a typical gitosis installation?  I think it
> > extracts bits from the ORIGINAL_COMMAND and formulates a sanitized command
> > line and then passes it to git-shell but I do not think it removes the
> > variable from the environment.
> 
> Yup, not sanitizing. I could make that change easily, if you want, but
> there's plenty of installations out there already.
> 
> I'd say add a git-shell flag that enables any magic behavior, don't
> just magically work differently based on that environment variable.
> Sometimes that is set for reasons that are not related to what you're
> actually running in the end; e.g. "chroot /foo exec-whatever-i-gave-you".

What about triggering only when there is no option at all? Doing so
would normally terminate git-shell telling the user it's not a shell.

Mike

[PATCH v2] Allow git-shell to be used as a ssh forced-command

[Mike Hommey]

When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
variable to what would otherwise be passed to $SHELL -c. When this
variable is set and git-shell was given no argument, we use it.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 shell.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/shell.c b/shell.c
index b968be7..86c9273 100644
--- a/shell.c
+++ b/shell.c
@@ -72,11 +72,16 @@ int main(int argc, char **argv)
 	/*
 	 * We do not accept anything but "-c" followed by "cmd arg",
 	 * where "cmd" is a very limited subset of git commands.
+	 * If no argument is given at all, see whether we were run from
+	 * a ssh forced-command and use the original command if so.
 	 */
 	else if (argc != 3 || strcmp(argv[1], "-c"))
-		die("What do you think I am? A shell?");
+		if (argc != 1 || !(prog = getenv("SSH_ORIGINAL_COMMAND")))
+			die("What do you think I am? A shell?");
+
+	if (!prog)
+		prog = argv[2];
 
-	prog = argv[2];
 	if (!strncmp(prog, "git", 3) && isspace(prog[3]))
 		/* Accept "git foo" as if the caller said "git-foo". */
 		prog[3] = '-';
-- 
1.6.3.rc0.54.g1042b

Re: [PATCH v2] Allow git-shell to be used as a ssh forced-command

[Dmitry Potapov]

On Tue, Apr 21, 2009 at 07:43:06AM +0200, Mike Hommey wrote:
> When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> variable to what would otherwise be passed to $SHELL -c. When this
> variable is set and git-shell was given no argument, we use it.
> 
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>  shell.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/shell.c b/shell.c
> index b968be7..86c9273 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -72,11 +72,16 @@ int main(int argc, char **argv)
>  	/*
>  	 * We do not accept anything but "-c" followed by "cmd arg",
>  	 * where "cmd" is a very limited subset of git commands.
> +	 * If no argument is given at all, see whether we were run from
> +	 * a ssh forced-command and use the original command if so.
>  	 */
>  	else if (argc != 3 || strcmp(argv[1], "-c"))
> -		die("What do you think I am? A shell?");
> +		if (argc != 1 || !(prog = getenv("SSH_ORIGINAL_COMMAND")))
> +			die("What do you think I am? A shell?");
> +
> +	if (!prog)

It appears to me that 'prog' may be used unitialized here.


Dmitry

Re: [PATCH v2] Allow git-shell to be used as a ssh forced-command

[Eygene Ryabinkin]

Dmitry, *, good day.

Tue, Apr 21, 2009 at 12:05:59PM +0400, Dmitry Potapov wrote:
> On Tue, Apr 21, 2009 at 07:43:06AM +0200, Mike Hommey wrote:
> > When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> > variable to what would otherwise be passed to $SHELL -c. When this
> > variable is set and git-shell was given no argument, we use it.
> > 
> > Signed-off-by: Mike Hommey <mh@glandium.org>
> > ---
> >  shell.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/shell.c b/shell.c
> > index b968be7..86c9273 100644
> > --- a/shell.c
> > +++ b/shell.c
> > @@ -72,11 +72,16 @@ int main(int argc, char **argv)
> >  	/*
> >  	 * We do not accept anything but "-c" followed by "cmd arg",
> >  	 * where "cmd" is a very limited subset of git commands.
> > +	 * If no argument is given at all, see whether we were run from
> > +	 * a ssh forced-command and use the original command if so.
> >  	 */
> >  	else if (argc != 3 || strcmp(argv[1], "-c"))
> > -		die("What do you think I am? A shell?");
> > +		if (argc != 1 || !(prog = getenv("SSH_ORIGINAL_COMMAND")))
> > +			die("What do you think I am? A shell?");
> > +
> > +	if (!prog)
> 
> It appears to me that 'prog' may be used unitialized here.

True.  The proper patch would be
-----
From 898e5ae3891d294ee5cb28a430093aced7cb26bd Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh@glandium.org>
Date: Tue, 21 Apr 2009 07:43:06 +0200
Subject: [PATCH] Allow git-shell to be used as a ssh forced-command

When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
variable to what would otherwise be passed to $SHELL -c. When this
variable is set and git-shell was given no argument, we use it.

Signed-off-by: Mike Hommey <mh@glandium.org>
Patch-problem-spotted-by: Dmitry Potapov <dpotapov@gmail.com>
Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 shell.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/shell.c b/shell.c
index b968be7..93aeded 100644
--- a/shell.c
+++ b/shell.c
@@ -47,7 +47,7 @@ static struct commands {
 
 int main(int argc, char **argv)
 {
-	char *prog;
+	char *prog = NULL;
 	struct commands *cmd;
 	int devnull_fd;
 
@@ -72,11 +72,16 @@ int main(int argc, char **argv)
 	/*
 	 * We do not accept anything but "-c" followed by "cmd arg",
 	 * where "cmd" is a very limited subset of git commands.
+	 * If no argument is given at all, see whether we were run from
+	 * a ssh forced-command and use the original command if so.
 	 */
 	else if (argc != 3 || strcmp(argv[1], "-c"))
-		die("What do you think I am? A shell?");
+		if (argc != 1 || !(prog = getenv("SSH_ORIGINAL_COMMAND")))
+			die("What do you think I am? A shell?");
+
+	if (!prog)
+		prog = argv[2];
 
-	prog = argv[2];
 	if (!strncmp(prog, "git", 3) && isspace(prog[3]))
 		/* Accept "git foo" as if the caller said "git-foo". */
 		prog[3] = '-';
-- 
1.6.1.3
-----
-- 
Eygene

Re: [PATCH v2] Allow git-shell to be used as a ssh forced-command

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> variable to what would otherwise be passed to $SHELL -c. When this
> variable is set and git-shell was given no argument, we use it.

I might be being extremely slow, but I had an impression that I saw only
the downsides discussed in the earlier thread and did not see what use
case this is supposed to help.

Is it a valid question to ask "what's wrong with gitosis"?

Re: [PATCH v2] Allow git-shell to be used as a ssh forced-command

[Mike Hommey]

On Tue, Apr 21, 2009 at 01:13:17AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > When using a forced-command, OpenSSH sets the SSH_ORIGINAL_COMMAND
> > variable to what would otherwise be passed to $SHELL -c. When this
> > variable is set and git-shell was given no argument, we use it.
> 
> I might be being extremely slow, but I had an impression that I saw only
> the downsides discussed in the earlier thread and did not see what use
> case this is supposed to help.
> 
> Is it a valid question to ask "what's wrong with gitosis"?

Why use gitosis where all you need is git-shell?

Mike

Re: [PATCH v2] Allow git-shell to be used as a ssh forced-command

[Dmitry Potapov]

On Tue, Apr 21, 2009 at 01:13:17AM -0700, Junio C Hamano wrote:
> 
> Is it a valid question to ask "what's wrong with gitosis"?

Git user needs /bin/sh as the login shell, and some sysadmins aren't
happy about adding any user with a real shell. OTOH, it seems Mike
is not concern about having a real shell for git user...

BTW, no so long ago, I posted a smiliar patch, which not only allowed
to use git-shell as a force-command (but in a more explicit way), it
also made possible to assotiate some name with every ssh key, so this
name could be used later in git hooks.

http://marc.info/?l=git&m=121483868801547&w=2

Dmitry