Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command

2013-02-11 Thread Junio C Hamano
Jonathan Nieder  writes:

> Junio C Hamano wrote:
>> Jonathan Nieder  writes:
>
>>> +To disable interactive logins, displaying a greeting instead:
>>> ++
>>> +
>>> +$ chsh -s /usr/bin/git-shell
>>> +$ mkdir $HOME/git-shell-commands
>>> +$ cat >$HOME/git-shell-commands/help <<\EOF
>>> +#!/bin/sh
>>> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
>>
>> Where in the sshd to git-shell exec chain is $USER variable set for
>> the user?  Just being curious if this is the simplest but one of the
>> more robust ways to get the user's name.
>
> That's a good question.  environment= in an authorized_keys file is
> obsolete, so USER generally represents the actual logged in user.
>
> That means the main way to base behavior on private key (letting one
> system user represent multiple people) is a gitolite-style command=
> wrapper that checks SSH_ORIGINAL_COMMAND.  In that setup, there is no
> reason to forward simple no-args "are you there?" requests to the
> git-shell, so we can forget about it here.
>
> So by the time we get to git-shell, most likely either
>
>  A) this is a generic system user, with a username like "git", and the
> above example would insult the client with "Hi git!" or "Hi
> project-x-git!"
>
> or
>
>  B) each person has a separate account on the system, perhaps to help
> the admin to set filesystem permissions based on users and groups,
> and the above would address the user by her normal name.

What return value getuid(2) would give us was not something I was
worried about.  Use of git-shell would be pointless if that does not
work to offer isolation between users.

I was wondering who would set the $USER variable based on the uid
assigned to the process during the remote login process and it is a
behaviour we can rely on across platforms.  It appears that when
coming over ssh, it is the ssh daemon that sets USER (and LOGNAME,
HOME, etc.) before running the login shell (session.c::do_child()
that is called from do_exec_pty() or do_exec_no_pty() in openssh).


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command

2013-02-10 Thread Jonathan Nieder
Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> +To disable interactive logins, displaying a greeting instead:
>> ++
>> +
>> +$ chsh -s /usr/bin/git-shell
>> +$ mkdir $HOME/git-shell-commands
>> +$ cat >$HOME/git-shell-commands/help <<\EOF
>> +#!/bin/sh
>> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
>
> Where in the sshd to git-shell exec chain is $USER variable set for
> the user?  Just being curious if this is the simplest but one of the
> more robust ways to get the user's name.

That's a good question.  environment= in an authorized_keys file is
obsolete, so USER generally represents the actual logged in user.

That means the main way to base behavior on private key (letting one
system user represent multiple people) is a gitolite-style command=
wrapper that checks SSH_ORIGINAL_COMMAND.  In that setup, there is no
reason to forward simple no-args "are you there?" requests to the
git-shell, so we can forget about it here.

So by the time we get to git-shell, most likely either

 A) this is a generic system user, with a username like "git", and the
above example would insult the client with "Hi git!" or "Hi
project-x-git!"

or

 B) each person has a separate account on the system, perhaps to help
the admin to set filesystem permissions based on users and groups,
and the above would address the user by her normal name.

Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command

2013-02-10 Thread Junio C Hamano
Jonathan Nieder  writes:

> diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
> index 4fe93203..60051e63 100644
> --- a/Documentation/git-shell.txt
> +++ b/Documentation/git-shell.txt
> @@ -59,6 +59,26 @@ users to list repositories they have access to, create, 
> delete, or
>  rename repositories, or change repository descriptions and
>  permissions.
>  
> +If the `help` command exists and exits with nonzero status, the
> +interactive shell is aborted.
> +
> +EXAMPLE
> +---
> +
> +To disable interactive logins, displaying a greeting instead:
> ++
> +
> +$ chsh -s /usr/bin/git-shell
> +$ mkdir $HOME/git-shell-commands
> +$ cat >$HOME/git-shell-commands/help <<\EOF
> +#!/bin/sh
> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"

Where in the sshd to git-shell exec chain is $USER variable set for
the user?  Just being curious if this is the simplest but one of the
more robust ways to get the user's name.

I still think forcing the site administrator create a directory for
each and every user only to house a single script that denies the
access is a wrong design, but the code seems to correctly implement
that design.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command

2013-02-10 Thread Ethan Reesor
I feel like the suggestion I posted in response to Junio C Hamano
's complaint on the RFC for this patch provides a
more elegant solution to the problem of administrators wanting to
prevent interactive sessions for users with their login shell set to
git-prompt. The suggestion was as follows:

> How is this for an alternative? Have shell.c look for a
>[shell]
>missing_commands_directory = "Stuff is broke."
> setting. If the setting is missing, then it prints the default message
> (the current message). That way, there's a default setting, there can
> be a system-wide message, there can be a user specific message, and
> those messages can be set via `git-config`.

On Mon, Feb 11, 2013 at 12:58 AM, Jonathan Nieder  wrote:
> If I disable git-shell's interactive mode by removing the
> ~/git-shell-commands directory, then attempts to use 'ssh' with the
> git account interactively produce an error message intended for the
> administrator:
>
> $ ssh git@myserver
> fatal: Interactive git shell is not enabled.
> hint: ~/git-shell-commands should exist and have read and execute 
> access.
> $
>
> That is helpful for the new admin who is wondering "What? Why isn't
> the git-shell I just set up working?", but once the site setup is
> finished, it is better to give the user a friendly hint that she is on
> the right track, like GitHub does:
>
> Hi ! You've successfully authenticated, but
> GitHub does not provide shell access.
>
> An appropriate greeting might even include more complex information,
> like a list of repositories the user has access to.  If the
> git-shell-commands directory exists and contains a "help" script, we
> already run it when the shell is run without any commands, giving the
> server a chance to provide a custom message.  Unfortunately, the
> presence of the git-shell-commands directory means we also enter an
> interactive mode, prompting and accepting commands (of which there may
> be none) from the user, which many servers would not want.  To solve
> this, we abort the interactive shell on a non-zero exit code from the
> "help" script.  This lets the server say whatever it likes, and then
> hang up.
>
> Downside: this will prevent interactive git-shell logins in existing
> setups where the "help" script exits with nonzero status by mistake.
> Hopefully those are rare enough to not cause much trouble in practice.
>
> Reported-by: Ethan Reesor 
> Signed-off-by: Jonathan Nieder 
> Improved-by: Jeff King 
> ---
>  Documentation/git-shell.txt | 20 
>  shell.c | 10 --
>  2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
> index 4fe93203..60051e63 100644
> --- a/Documentation/git-shell.txt
> +++ b/Documentation/git-shell.txt
> @@ -59,6 +59,26 @@ users to list repositories they have access to, create, 
> delete, or
>  rename repositories, or change repository descriptions and
>  permissions.
>
> +If the `help` command exists and exits with nonzero status, the
> +interactive shell is aborted.
> +
> +EXAMPLE
> +---
> +
> +To disable interactive logins, displaying a greeting instead:
> ++
> +
> +$ chsh -s /usr/bin/git-shell
> +$ mkdir $HOME/git-shell-commands
> +$ cat >$HOME/git-shell-commands/help <<\EOF
> +#!/bin/sh
> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
> +printf '%s\n' "provide interactive shell access."
> +exit 128
> +EOF
> +$ chmod +x $HOME/git-shell-commands/help
> +
> +
>  SEE ALSO
>  
>  ssh(1),
> diff --git a/shell.c b/shell.c
> index 84b237fe..3abc2b84 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -63,10 +63,16 @@ static void cd_to_homedir(void)
>
>  static void run_shell(void)
>  {
> -   int done = 0;
> +   int done = 0, status;
> static const char *help_argv[] = { HELP_COMMAND, NULL };
> /* Print help if enabled */
> -   run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> +   status = run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> +   if (!status)
> +   ; /* success */
> +   else if (status == -1 && errno == ENOENT)
> +   ; /* help disabled */
> +   else
> +   exit(status);
>
> do {
> struct strbuf line = STRBUF_INIT;
> --
> 1.8.1.3
>



--
Ethan Reesor (Gmail)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html