"git shell" won't work, need "git-shell"

PATCH In review Tommi Virtanen 11 participants 28 messages Jun 15, 2016 Kbuild

lore

Share & tools

lore

Tommi Virtanen

author

2016-06-15 22:45 UTC #0 lore raw

PATCH 1/1 "git shell" won't work, need "git-shell"

Quoted 1 line

From 8e7935231e8a91d470b3a4a2310803031ef49fc4 Mon Sep 17 00:00:00 2001

From: Tommi Virtanen <[email protected]>
Date: Sun, 24 Aug 2008 23:20:33 +0300
Subject: [PATCH] Install git-shell in bindir, again.

/etc/passwd shell field must be something execable, you can't enter "/usr/bin/git shell" there. git-shell must be present as a separate executable, or it is useless.

Signed-off-by Tommi Virtanen <[email protected]>

---

Hi. Recent changes moved away from "git-foo" to "git foo", except for some commands that needed backwards compatibility. However, git-shell as a separate binary was removed. I hope you will reinstante git-shell as a publicly visible binary in bin. Here's why:

The shell field in /etc/passwd is *exec*ed, not interpreted via sh -c or some such. For example, source of Debian's shadow, containing /bin/login:

libmisc/shell.c:80: execle (file, arg, (char *) 0, envp);

I also tested this for real, and having a

test:x:1001:1001:,,,:/home/test:/usr/bin/git-shell

line works, and

test:x:1001:1001:,,,:/home/test:/usr/bin/git shell

just makes ssh loop asking for a password, logging

"User test not allowed because shell /usr/bin/git shell does not exist"

So, as far as I understand, as it currently is, "git shell" is utterly useless for what it was meant to do. Restoring "git-shell" will fix it.

 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Makefile +1 -1

index 53ab4b5..24d5809 100644
@@ -1351,7 +1351,7 @@ install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_TCLTK

Signature · 4 lines

1.6.0.2.g2ebc0.dirty

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

Junio C Hamano

2016-06-15 22:45 UTC #1 lore raw

Quoted 7 lines · Tommi Virtanen

test:x:1001:1001:,,,:/home/test:/usr/bin/git-shell
line works, and
test:x:1001:1001:,,,:/home/test:/usr/bin/git shell
just makes ssh loop asking for a password, logging

Of course it would. Does using /usr/libexec/git-core/git-shell work?

Tommi Virtanen

author

2016-06-15 22:45 UTC #2 lore raw

Quoted 1 line · Junio C Hamano

Of course it would. Does using /usr/libexec/git-core/git-shell work?

It would, but do you really want people using that?

Signature · 1 line

:(){ :|:&};:

Junio C Hamano

2016-06-15 22:45 UTC #3 lore raw

Quoted 4 lines · Tommi Virtanen

Quoted 1 line · Junio C Hamano
Of course it would. Does using /usr/libexec/git-core/git-shell work?
It would, but do you really want people using that?

I do not have particular preference either way. What people wanted was to have smaller number of git-foo on $PATH, and especially as "git-shell" is not something people would be typing from their command line, so I dunno.

Tommi Virtanen

author

2016-06-15 22:45 UTC #4 lore raw

Quoted 3 lines · Junio C Hamano

I do not have particular preference either way. What people wanted was to have smaller number of git-foo on $PATH, and especially as "git-shell" is not something people would be typing from their command line, so I dunno.

That's true, but I kinda think libexec is something only used *internally*, and you can't claim /etc/passwd to be internal to git..

At the minimum, git-shell(1) should explain that one needs to use the libexec path.

Signature · 1 line

:(){ :|:&};:

Johannes Schindelin

2016-06-15 22:45 UTC #5 lore raw

Hi,

Quoted 10 lines · Tommi Virtanen

Quoted 3 lines · Junio C Hamano
I do not have particular preference either way. What people wanted was to have smaller number of git-foo on $PATH, and especially as "git-shell" is not something people would be typing from their command line, so I dunno.
That's true, but I kinda think libexec is something only used *internally*, and you can't claim /etc/passwd to be internal to git..
At the minimum, git-shell(1) should explain that one needs to use the libexec path.

Or maybe the real issue can be fixed? Namely that your configuration does not work? That would involve you actually finding out what's happening, though.

Ciao,
Dscho

Paolo Bonzini

2016-06-15 22:45 UTC #6 lore raw

Quoted 7 lines

Quoted 3 lines
At the minimum, git-shell(1) should explain that one needs to use the libexec path.
Or maybe the real issue can be fixed? Namely that your configuration does not work? That would involve you actually finding out what's happening, though.

He said so:

Quoted 14 lines

The shell field in /etc/passwd is *exec*ed, not interpreted via sh -c or some such. For example, source of Debian's shadow, containing /bin/login:
libmisc/shell.c:80: execle (file, arg, (char *) 0, envp);
I also tested this for real, and having a
test:x:1001:1001:,,,:/home/test:/usr/bin/git-shell
line works, and
test:x:1001:1001:,,,:/home/test:/usr/bin/git shell

Quoted 7 lines

just makes ssh loop asking for a password, logging
"User test not allowed because shell /usr/bin/git shell does not exist"
So, as far as I understand, as it currently is, "git shell" is utterly useless for what it was meant to do. Restoring "git-shell" will fix it.

Rephrasing your question, do you (Dscho) actually know *anyone* who has a working setup with "/usr/bin/git shell" in /etc/passwd?

Paolo

Johannes Schindelin

2016-06-15 22:45 UTC #7 lore raw

Hi,

Quoted 9 lines · Paolo Bonzini

Quoted 1 line
That would involve you actually finding out what's happening, though.
He said so:
Quoted 5 lines
test:x:1001:1001:,,,:/home/test:/usr/bin/git shell
just makes ssh loop asking for a password, logging
"User test not allowed because shell /usr/bin/git shell does not exist"

Okay, so this means that you cannot pass arguments to the login shell. Makes me wonder... I had the impression that bash was called with --login.

Ciao,
Dscho

Mikael Magnusson

2016-06-15 22:45 UTC #8 lore raw

2008/8/25 Johannes Schindelin <[email protected]>:

Quoted 8 lines

Hi,
Quoted 4 lines · Paolo Bonzini
Quoted 1 line
That would involve you actually finding out what's happening, though.
He said so:

Quoted 8 lines

Quoted 5 lines
Quoted 5 lines
test:x:1001:1001:,,,:/home/test:/usr/bin/git shell
just makes ssh loop asking for a password, logging
"User test not allowed because shell /usr/bin/git shell does not exist"
Okay, so this means that you cannot pass arguments to the login shell. Makes me wonder... I had the impression that bash was called with --login.

When you login, a '-' is prepended in argv[0], ie, bash checks if it's called "-bash". This is documented in man bash, but I couldn't find it in man login or man agetty, not sure where else it might be written down.

Signature · 1 line

Mikael Magnusson

Johannes Schindelin

2016-06-15 22:45 UTC #9 lore raw

Hi,

Quoted 16 lines · Mikael Magnusson

2008/8/25 Johannes Schindelin <[email protected]>:
Quoted 14 lines · Paolo Bonzini
Quoted 12 lines
Quoted 3 lines
That would involve you actually finding out what's happening, though.
He said so:
Quoted 6 lines
test:x:1001:1001:,,,:/home/test:/usr/bin/git shell
just makes ssh loop asking for a password, logging
"User test not allowed because shell /usr/bin/git shell does not exist"

Quoted 8 lines

Quoted 3 lines
Okay, so this means that you cannot pass arguments to the login shell. Makes me wonder... I had the impression that bash was called with --login.
When you login, a '-' is prepended in argv[0], ie, bash checks if it's called "-bash". This is documented in man bash, but I couldn't find it in man login or man agetty, not sure where else it might be written down.

Thanks!

So does this mean that we could check in git.c if there is a leading "-" in argv[0]? If so, then the builtin git-shell should be called by default.

At least I do not expect many instances of git being called with argv[0] starting with a "-"... :-)

Ciao,
Dscho

Johannes Schindelin

2016-06-15 22:45 UTC #10 lore raw

Hi,

Quoted 27 lines · Johannes Schindelin

Quoted 16 lines · Mikael Magnusson
2008/8/25 Johannes Schindelin <[email protected]>:
Quoted 14 lines · Paolo Bonzini
Quoted 12 lines
Quoted 3 lines
That would involve you actually finding out what's happening, though.
He said so:
Quoted 6 lines
test:x:1001:1001:,,,:/home/test:/usr/bin/git shell
just makes ssh loop asking for a password, logging
"User test not allowed because shell /usr/bin/git shell does not exist"
Quoted 8 lines
Quoted 3 lines
Okay, so this means that you cannot pass arguments to the login shell. Makes me wonder... I had the impression that bash was called with --login.
When you login, a '-' is prepended in argv[0], ie, bash checks if it's called "-bash". This is documented in man bash, but I couldn't find it in man login or man agetty, not sure where else it might be written down.

Quoted 8 lines

Thanks!
So does this mean that we could check in git.c if there is a leading "-" in argv[0]? If so, then the builtin git-shell should be called by default.
At least I do not expect many instances of git being called with argv[0] starting with a "-"... :-)

Oh, well. I just tested again, and slapped my head when it did not work, remembering that we do _not_ call Git as a login shell. Instead, we call ssh with the "-c" option, which just passes it to the shell. Consequently, argv[0] does not get a "-" prepended.

I seem to remember that Hannes had some code to support "-c" as an indicator that Git should execute git-shell, but I just might have dreamt that, too.

Ciao,
Dscho

Johannes Schindelin

2016-06-15 22:45 UTC #11 lore raw

PATCH 1/2 git wrapper: Make while loop more reader-friendly

It is not a good practice to prefer performance over readability in something as performance uncritical as finding the trailing slash of argv[0].

So avoid head-scratching by making the loop user-readable, and not hyper-performance-optimized.

Signed-off-by Johannes Schindelin <[email protected]>

---

The original version threatened to cause eye-cancer with this developer, so that I had to run to the physician real quick. That was quite hard, as no ophtalmologist seemed to be in office at 10:30pm. But I found a real good witch, and she diagnozed me as healthy.

 git.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

git.c +2 -3

index a647741..2fd74c4 100644
@@ -427,9 +427,8 @@ int main(int argc, const char **argv)
 	 * name, and the dirname as the default exec_path
 	 * if we don't have anything better.
 	 */
-	do
-		--slash;
-	while (cmd <= slash && !is_dir_sep(*slash));
+	while (cmd <= slash && !is_dir_sep(*slash))
+		slash--;
 	if (slash < cmd) {
 		cmd = lookup_program_in_path(cmd);
 		for (slash = (char *)cmd + strlen(cmd) - 1;

Signature · 1 line

1.6.0.211.ga840e.dirty

Johannes Schindelin

2016-06-15 22:45 UTC #12 lore raw

PATCH 2/2 git wrapper: execute git-shell when argv[1] is '-c'

When a program is called via ssh's '-c' option, the login shell is called on the remote computer, with the given arguments. In the case that Git was specified as login shell in /etc/passwd, Git used to complain that it does not know the '-c' option and die.

This commit assumes that '-c' indicates that Git was specified as a login shell, and hands off to git-shell.

Noticed by Tommi Virtanen.

Signed-off-by Johannes Schindelin <[email protected]>

---

This should be pretty uncontroversial, as it turns a former error into something useful.

However, I would not know where in the documentation (in addition to git-shell.txt, I guess), this change should be described.

 git.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

git.c +10 -1

index 2fd74c4..b16736a 100644
@@ -419,14 +419,23 @@ static void execv_dashed_external(const char **argv)
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
-	char *slash = (char *)cmd + strlen(cmd);
+	char *slash;
 	int done_alias = 0;
 
 	/*
+	 * When Git is called with "-c", it is either an error, or Git
+	 * was specified as a login shell in /etc/passwd.  Assuming the
+	 * latter, we continue with git-shell.
+	 */
+	if (argc > 2 && !strcmp(argv[1], "-c"))
+		cmd = "git-shell";
+
+	/*
 	 * Take the basename of argv[0] as the command
 	 * name, and the dirname as the default exec_path
 	 * if we don't have anything better.
 	 */
+	slash = (char *)cmd + strlen(cmd);
 	while (cmd <= slash && !is_dir_sep(*slash))
 		slash--;
 	if (slash < cmd) {

Signature · 1 line

1.6.0.211.ga840e.dirty

Tommi Virtanen

author

2016-06-15 22:45 UTC #13 lore raw

Re: [PATCH 2/2] git wrapper: execute git-shell when argv[1] is '-c'

Quoted 9 lines · Johannes Schindelin

When a program is called via ssh's '-c' option, the login shell is called on the remote computer, with the given arguments. In the case that Git was specified as login shell in /etc/passwd, Git used to complain that it does not know the '-c' option and die.
This commit assumes that '-c' indicates that Git was specified as a login shell, and hands off to git-shell.
Noticed by Tommi Virtanen.

My imagination is insufficient in coming up with an uglier kludge, and I sincerely hope my name isn't associated with this in any way.

Signature · 1 line

:(){ :|:&};:

Johannes Schindelin

2016-06-15 22:45 UTC #14 lore raw

Re: [PATCH 2/2] git wrapper: execute git-shell when argv[1] is '-c'

Hi,

Quoted 6 lines · Tommi Virtanen

Quoted 5 lines · Johannes Schindelin
When a program is called via ssh's '-c' option, the login shell is called on the remote computer, with the given arguments. In the case that Git was specified as login shell in /etc/passwd, Git used to complain that it does not know the '-c' option and die.

Quoted 7 lines

Quoted 4 lines
This commit assumes that '-c' indicates that Git was specified as a login shell, and hands off to git-shell.
Noticed by Tommi Virtanen.
My imagination is insufficient in coming up with an uglier kludge, and I sincerely hope my name isn't associated with this in any way.

I have no problems deleting your name from the commit message. None at all. Even if you actually did notice the issue.

I do disagree with you that it is a kludge though. I think it makes complete sense to add this to Documentation/git.txt in addition to other documentation that is lacking from my patch, though:

-- snip --
-c <command>::
execute <command> in git-shell.
-- snap --

Ciao,
Dscho

Tommi Virtanen

author

2016-06-15 22:45 UTC #15 lore raw

Re: [PATCH 2/2] git wrapper: execute git-shell when argv[1] is '-c'

Quoted 8 lines · Johannes Schindelin

I do disagree with you that it is a kludge though. I think it makes complete sense to add this to Documentation/git.txt in addition to other documentation that is lacking from my patch, though:
-- snip --
-c <command>::
execute <command> in git-shell.
-- snap --

All I'm going to say is that that's not the way you build trustable software. You take a minimal interface and restrict untrusted users to that, you don't add a feature to the widest possible interface..

Signature · 1 line

:(){ :|:&};:

Johannes Schindelin

2016-06-15 22:45 UTC #16 lore raw

Re: [PATCH 2/2] git wrapper: execute git-shell when argv[1] is '-c'

Hi,

Quoted 5 lines · Tommi Virtanen

Quoted 4 lines · Johannes Schindelin
I do disagree with you that it is a kludge though. I think it makes complete sense to add this to Documentation/git.txt in addition to other documentation that is lacking from my patch, though:

Quoted 8 lines

Quoted 4 lines
-- snip --
-c <command>::
execute <command> in git-shell.
-- snap --
All I'm going to say is that that's not the way you build trustable software. You take a minimal interface and restrict untrusted users to that, you don't add a feature to the widest possible interface..

I do not get your point.

But then, I think I start not to care anymore, as I think your reasoning is bogus. "widest possible interface", "trustable software", etc. Sounds pretty buzzy-wordy to me.

Yeah, you would need to audit it. Maybe you would even have to check for "*argv[0] == '-'" and set argv[0] to "git-shell" if so. But buzz-wording just makes me go away and not listen anymore.

Whatever,
Dscho

Matthieu Moy

2016-06-15 22:45 UTC #17 lore raw

Re: [PATCH 2/2] git wrapper: execute git-shell when argv[1] is '-c'

Quoted 7 lines · Johannes Schindelin

Quoted 3 lines · Tommi Virtanen
All I'm going to say is that that's not the way you build trustable software. You take a minimal interface and restrict untrusted users to that, you don't add a feature to the widest possible interface..
I do not get your point.

With your patch, AAUI, one would put /usr/bin/git as a shell in passwd. It's pretty clear that someone calling the shell with -c will get the restriction of git-shell, but I'd hardly guarantee that the shell in /etc/passwd will never be called without -c. At least, if the user tries to login, he will execute git without argument (which fortunately isn't serious, he'll just get the help message for git, which is unhelpful but not dangerous). My knowledge in Unix isn't sufficient to be sure there's no way at all to call git in a dangerous way here. With git-shell, it's much simpler to understand: either you give -c, and you get the restricted command set, or you don't, and you get nothing.

Signature · 1 line

Matthieu

Junio C Hamano

2016-06-15 22:45 UTC #18 lore raw

Re: [PATCH 2/2] git wrapper: execute git-shell when argv[1] is '-c'

Quoted 4 lines · Matthieu Moy

Quoted 2 lines · Johannes Schindelin
On Tue, 26 Aug 2008, Tommi Virtanen wrote:

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
All I'm going to say is that that's not the way you build trustable software. You take a minimal interface and restrict untrusted users to that, you don't add a feature to the widest possible interface..
I do not get your point.
With your patch, AAUI, one would put /usr/bin/git as a shell in passwd....

Well, it was sheer stupidity of mine.

Let's stop this and apply this patch instead. The patch text is obvious so I won't quote.

-- * --
From: Junio C Hamano <[email protected]>
Date: Mon, 25 Aug 2008 22:39:17 -0700
Subject: [PATCH] Revert "Build-in "git-shell""

This reverts commit daa0cc9a92c9c2c714aa5f7da6d0ff65b93e0698. It was a stupid idea to do this; when run as a log-in shell, it is spawned with argv[0] set to "-git-shell", so the usual name-based dispatch would not work to begin with.

Signed-off-by Junio C Hamano <[email protected]>

 Makefile        |    2 +-
 builtin-shell.c |   90 -------------------------------------------------------
 builtin.h       |    1 -
 git.c           |    1 -
 shell.c         |   89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 90 insertions(+), 93 deletions(-)

Signature · 1 line

1.6.0.1.113.g0a79b

Mike Ralphson

2016-06-15 22:45 UTC #19 lore raw

Re: [PATCH 1/2] git wrapper: Make while loop more reader-friendly

In reply to Johannes Schindelin

2008/8/25 Johannes Schindelin <[email protected]>:

Quoted 31 lines

It is not a good practice to prefer performance over readability in something as performance uncritical as finding the trailing slash of argv[0].

So avoid head-scratching by making the loop user-readable, and not hyper-performance-optimized.

Signed-off-by Johannes Schindelin <[email protected]>

---

 git.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

git.c +2 -4

index a647741..2fd74c4 100644
@@ -427,9 +427,8 @@ int main(int argc, const char **argv)
        * name, and the dirname as the default exec_path
        * if we don't have anything better.
        */
-       do
-               --slash;
-       while (cmd <= slash && !is_dir_sep(*slash));
+       while (cmd <= slash && !is_dir_sep(*slash))
+               slash--;
       if (slash < cmd) {
               cmd = lookup_program_in_path(cmd);
               for (slash = (char *)cmd + strlen(cmd) - 1;
--

Quoted 1 line

1.6.0.211.ga840e.dirty

Isn't it more likely that this form was chosen to indicate that the loop body was expected to execute at least once, rather than zero or more times?

Mike

Petr Baudis

2016-06-15 22:45 UTC #20 lore raw

In reply to Junio C Hamano

Quoted 10 lines · Junio C Hamano

Quoted 4 lines · Tommi Virtanen
Quoted 1 line · Junio C Hamano
Of course it would. Does using /usr/libexec/git-core/git-shell work?
It would, but do you really want people using that?
I do not have particular preference either way. What people wanted was to have smaller number of git-foo on $PATH, and especially as "git-shell" is not something people would be typing from their command line, so I dunno.

Can we agree that direct calls of libexec stuff should never be part of the "official" interface (i.e. not workarounds for deprecated usage)? Considering that calling the git-shell executable directly is the _only_ sensible way of using this interface, it should follow that it has to be in /usr/bin, no matter if users type this command or not.

(I'm actually a little confused that you bring up the "typing from their command line" aspect at all, since that never seemed to be relevant criterium. People type the commit command all the time, yet we do not install git-commit. Typing the three git-* commands we do install - git-receive-pack, git-upload-pack and git-upload-archive - should be on the other hand pretty rare occasion. About gitk, well, 'git k' would just look silly, I guess. ;-)

Signature · 3 lines

				Petr "Pasky" Baudis
The next generation of interesting software will be done
on the Macintosh, not the IBM PC.  -- Bill Gates

Dmitry Potapov

2016-06-15 22:45 UTC #21 lore raw

Quoted 3 lines · Petr Baudis

Can we agree that direct calls of libexec stuff should never be part of the "official" interface (i.e. not workarounds for deprecated usage)?

Agreed. It looks somewhat strange to type the libexec path in /etc/passwd.

Quoted 3 lines

Considering that calling the git-shell executable directly is the _only_ sensible way of using this interface, it should follow that it has to be in /usr/bin, no matter if users type this command or not.

Perhaps, /usr/sbin would be a better place, as it is intended only for system administration binaries.

Dmitry

Tommi Virtanen

author

2016-06-15 22:45 UTC #22 lore raw

Quoted 2 lines · Dmitry Potapov

Perhaps, /usr/sbin would be a better place, as it is intended only for system administration binaries.

I'd argue that git-shell isn't *exclusively* for root, which is the criteria for sbin. It's pretty easy to imagine a user setting up their own ~/.ssh/authorized_keys with a special passphraseless key that can only do git operations.

Signature · 1 line

:(){ :|:&};:

Dmitry Potapov

2016-06-15 22:45 UTC #23 lore raw

Quoted 6 lines · Tommi Virtanen

Quoted 2 lines · Dmitry Potapov
Perhaps, /usr/sbin would be a better place, as it is intended only for system administration binaries.
I'd argue that git-shell isn't *exclusively* for root, which is the criteria for sbin.

I don't think that your criteria is correct. mysqld is also not exclusively for root as you can run it on a non-privilege port, yet, it is placed in /usr/sbin. Placing in /usr/sbin does not mean that users do not have access to it, but that those binaries are not run by users from their command line.

Dmitry

Johannes Schindelin

2016-06-15 22:45 UTC #24 lore raw

In reply to Dmitry Potapov

Hi,

Quoted 8 lines · Dmitry Potapov

Quoted 4 lines · Petr Baudis
Can we agree that direct calls of libexec stuff should never be part of the "official" interface (i.e. not workarounds for deprecated usage)?
Agreed. It looks somewhat strange to type the libexec path in /etc/passwd.

FWIW I do not agree. IMNHO libexec/ is just a way to organize executable parts of any software package that are usually not called from the command line. And a login shell qualifies for that.

Quoted 6 lines

Quoted 3 lines
Considering that calling the git-shell executable directly is the _only_ sensible way of using this interface, it should follow that it has to be in /usr/bin, no matter if users type this command or not.
Perhaps, /usr/sbin would be a better place, as it is intended only for system administration binaries.

Does it not strike you as odd, then, that "sh" -- by far the most common login shell -- does not live in /usr/sbin/?

Ciao,
Dscho

Dmitry Potapov

2016-06-15 22:45 UTC #25 lore raw

Quoted 3 lines · Johannes Schindelin

Does it not strike you as odd, then, that "sh" -- by far the most common login shell -- does not live in /usr/sbin/?

Not at all. "sh" is *often* run directly from the command line. Without it being in PATH, system(3) and many other things would not work, but no one tuns git-shell as the real shell to do some job.

Dmitry

Mike Hommey

2016-06-15 22:45 UTC #26 lore raw

In reply to Johannes Schindelin

Quoted 21 lines · Johannes Schindelin

Hi,
Quoted 8 lines · Dmitry Potapov
Quoted 4 lines · Petr Baudis
Can we agree that direct calls of libexec stuff should never be part of the "official" interface (i.e. not workarounds for deprecated usage)?
Agreed. It looks somewhat strange to type the libexec path in /etc/passwd.
FWIW I do not agree. IMNHO libexec/ is just a way to organize executable parts of any software package that are usually not called from the command line. And a login shell qualifies for that.
Quoted 4 lines
Quoted 3 lines
Considering that calling the git-shell executable directly is the _only_ sensible way of using this interface, it should follow that it has to be in /usr/bin, no matter if users type this command or not.

Quoted 5 lines

Quoted 2 lines
Perhaps, /usr/sbin would be a better place, as it is intended only for system administration binaries.
Does it not strike you as odd, then, that "sh" -- by far the most common login shell -- does not live in /usr/sbin/?

But nologin is in /usr/sbin.

Mike

Dmitry V. Levin

2016-06-15 22:45 UTC #27 lore raw

In reply to Tommi Virtanen

Hi,

Please apply this compatibility fix (commit v1.6.0.1-90-g27a6ed4) to maint as well.

Quoted 57 lines · Tommi Virtanen

Quoted 1 line
From 8e7935231e8a91d470b3a4a2310803031ef49fc4 Mon Sep 17 00:00:00 2001
From: Tommi Virtanen <[email protected]>
Date: Sun, 24 Aug 2008 23:20:33 +0300
Subject: [PATCH] Install git-shell in bindir, again.
/etc/passwd shell field must be something execable, you can't enter "/usr/bin/git shell" there. git-shell must be present as a separate executable, or it is useless.
Signed-off-by Tommi Virtanen <[email protected]>
---
Hi. Recent changes moved away from "git-foo" to "git foo", except for some commands that needed backwards compatibility. However, git-shell as a separate binary was removed. I hope you will reinstante git-shell as a publicly visible binary in bin. Here's why:
The shell field in /etc/passwd is *exec*ed, not interpreted via sh -c or some such. For example, source of Debian's shadow, containing /bin/login:
libmisc/shell.c:80: execle (file, arg, (char *) 0, envp);
I also tested this for real, and having a
test:x:1001:1001:,,,:/home/test:/usr/bin/git-shell
line works, and
test:x:1001:1001:,,,:/home/test:/usr/bin/git shell
just makes ssh loop asking for a password, logging
"User test not allowed because shell /usr/bin/git shell does not exist"
So, as far as I understand, as it currently is, "git shell" is utterly useless for what it was meant to do. Restoring "git-shell" will fix it.
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Makefile +1 -1
index 53ab4b5..24d5809 100644
@@ -1351,7 +1351,7 @@ install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_TCLTK
Signature · 1 line
1.6.0.2.g2ebc0.dirty

Signature · 1 line

ldv