threads / discuss / 2191

Server side programs

Subject: Server side programs

## tl;dr

30 messages between Oct 22, 2005 and Oct 27, 2005.

replies: 29people: 7as markdown or json

Andreas Ericsson· Oct 22, 2005, 22:22 UTC · lore

Are git-receive-pack and git-upload-pack the only two binaries that get called directly over a SSH-tunnel?

The git tutorial explicitly mentions this for git-receive-pack. Several of the other docs (git-fetch-pack, git-peek-remote and git-clone-pack) mentions git-upload-pack. No other program from the git suite is mentioned as an immediate target for ssh connections.

The reason I'm asking is that I'm adding support for userrelative paths (git pull ssh://host:~user/somedir) and removing the possibilities to use a compromised but limited account for finding out what other useraccounts are available.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Junio C Hamano· Oct 23, 2005, 00:30 UTC · re: Andreas Ericsson · lore

Re: Server side programs

Andreas Ericsson <ae@op5.se> writes:
> Are git-receive-pack and git-upload-pack the only two binaries that get 
> called directly over a SSH-tunnel?

There are git-ssh-fetch and git-ssh-upload which call each other (and the older name pairs git-ssh-push and git-sh-pull do so as well). However, you do not have to use these commit walkers over ssh; fetch-pack/upload-pack pair work quite well.

I think your question is "what is the absolute minimum set of binaries you need to allow", so I think the two listed are enough. If you want to let your users coming over SSH *create* a new repository on your machine, you would need a bit more, though (namely, shell access to run mkdir and git-init-db).

> The reason I'm asking is that I'm adding support for userrelative paths 
> (git pull ssh://host:~user/somedir) and removing the possibilities to 
> use a compromised but limited account for finding out what other 
> useraccounts are available.

Sorry, it is not clear to me what you are adding. I do this regularly:

	$ git push kernel.org:git/
	$ git fetch kernel.org:git/

to push into and fetch from $HOME/git/ repository on the other end.

Also I can do this already (assuming the other end hangs all users under the same directory, presumably /home/$user):

	$ git fetch kernel.org:../torvalds/git/
Are you in addition trying to let me do this:
	$ git fetch kernel.org:~torvalds/git/

which would work even when ~torvalds == /home/torvalds and ~junio == /home2/junio, without having to tell people where the user home directories are?

At one point, Linus posted an outline of "restricted login shell for use with git over ssh". I think you could start from there, perhaps extend it so that it checks the binaries *and* pathnames the user can specify (e.g. only under your own $HOME is allowed, and no /../ in them, or something silly like that).

Andreas Ericsson· Oct 23, 2005, 09:41 UTC · re: Junio C Hamano · lore

Re: User-relative paths (was: Server side programs)

Junio C Hamano wrote:
Show 18 quoted lines
> Andreas Ericsson <ae@op5.se> writes:
> 
> 
>>Are git-receive-pack and git-upload-pack the only two binaries that get 
>>called directly over a SSH-tunnel?
> 
> 
> There are git-ssh-fetch and git-ssh-upload which call each other
> (and the older name pairs git-ssh-push and git-sh-pull do so as
> well).  However, you do not have to use these commit walkers
> over ssh; fetch-pack/upload-pack pair work quite well.
> 
> I think your question is "what is the absolute minimum set of
> binaries you need to allow", so I think the two listed are
> enough.  If you want to let your users coming over SSH *create*
> a new repository on your machine, you would need a bit more,
> though (namely, shell access to run mkdir and git-init-db).
> 
That's good to know.
Show 16 quoted lines
> 
>>The reason I'm asking is that I'm adding support for userrelative paths 
>>(git pull ssh://host:~user/somedir) and removing the possibilities to 
>>use a compromised but limited account for finding out what other 
>>useraccounts are available.
> 
> 
> Sorry, it is not clear to me what you are adding.  I do this
> regularly:
> 
> 	$ git push kernel.org:git/
> 	$ git fetch kernel.org:git/
> 
> to push into and fetch from $HOME/git/ repository on the other
> end.
> 

Looking at the old code I see that that should work. I'll re-work the patch to take this into account. The help-text was a bit fuzzy however, and if you tried

	$ git fetch ssh://kernel.org:git

it wouldn't work. Here's how I decided it was somehow broken, or at least non-intuitive:

	$ git clone ssh://git.op5.se:git foo
	defaulting to local storage area
	ssh: ssh: Name or service not known
	fatal: unexpected EOF
Show 13 quoted lines
> Also I can do this already (assuming the other end hangs all
> users under the same directory, presumably /home/$user):
> 
> 	$ git fetch kernel.org:../torvalds/git/
> 
> Are you in addition trying to let me do this:
> 
> 	$ git fetch kernel.org:~torvalds/git/
> 
> which would work even when ~torvalds == /home/torvalds and
> ~junio == /home2/junio, without having to tell people where
> the user home directories are?
> 

Yes. Our servers' home-directories have varying paths and it'd be nice to be able to pull other team-members code without having to remember how that particular server is configured.

Show 6 quoted lines
> At one point, Linus posted an outline of "restricted login shell
> for use with git over ssh".  I think you could start from there,
> perhaps extend it so that it checks the binaries *and* pathnames
> the user can specify (e.g. only under your own $HOME is allowed,
> and no /../ in them, or something silly like that).
> 

I found this in the archives: http://article.gmane.org/gmane.comp.version-control.git/5784/match=restricted+login

Is that what you're referring to?

There's no reason why git-upload-pack and git-receive-pack couldn't be used safely with a perfectly ordinary shell provided we're careful about giving the same error message for when a directory is missing and when it isn't a git archive.

Anyways, the attached patch does this. I've tested all the various syntaxes and they work as expected. rsync, http and local files take the same syntax as before. I haven't added support for user-relative paths to the git-daemon (can't see the point, really) although that can be done easily enough.

This patch works without conflicts alongside Johannes Schindelin's recent contribution.

Let me know if you want things done differently.
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231


diff --git a/Makefile b/Makefile
index 903c57c..87188ea 100644
--- a/Makefile
+++ b/Makefile
@@ -115,15 +115,18 @@ PROGRAMS = \
 	git-local-fetch$X git-ls-files$X git-ls-tree$X git-merge-base$X \
 	git-merge-index$X git-mktag$X git-pack-objects$X git-patch-id$X \
 	git-peek-remote$X git-prune-packed$X git-read-tree$X \
-	git-receive-pack$X git-rev-list$X git-rev-parse$X \
+	git-rev-list$X git-rev-parse$X \
 	git-send-pack$X git-show-branch$X \
 	git-show-index$X git-ssh-fetch$X \
 	git-ssh-upload$X git-tar-tree$X git-unpack-file$X \
 	git-unpack-objects$X git-update-index$X git-update-server-info$X \
-	git-upload-pack$X git-verify-pack$X git-write-tree$X \
+	git-verify-pack$X git-write-tree$X \
 	git-update-ref$X git-symbolic-ref$X git-check-ref-format$X \
 	$(SIMPLE_PROGRAMS)
 
+# server side programs called (possibly) over an ssh-tunnel
+SERVERSIDE_PROGRAMS = git-receive-pack$X git-upload-pack$X
+
 # Backward compatibility -- to be removed after 1.0
 PROGRAMS += git-ssh-pull$X git-ssh-push$X
 
@@ -315,7 +318,7 @@ SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
 ### Build rules
 
-all: $(PROGRAMS) $(SCRIPTS)
+all: $(PROGRAMS) $(SCRIPTS) $(SERVERSIDE_PROGRAMS)
 
 all:
 	$(MAKE) -C templates
@@ -359,6 +362,9 @@ git-cherry-pick: git-revert
 %.o: %.S
 	$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
+$(SERVERSIDE_PROGRAMS) : git-%$X : %.o srvside-ssh.o $(LIB_FILE)
+	$(CC) $(ALL_CFLAGS) -o $@ $(filter %o,$^) $(LIBS)
+
 git-%$X: %.o $(LIB_FILE)
 	$(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
 
@@ -383,6 +389,7 @@ init-db.o: init-db.c
 
 $(LIB_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H)
+$(patsubst git-%$X,%.o,$(SERVERSIDE_PROGRAMS)): $(LIB_H) srvside-ssh.o
 $(DIFF_OBJS): diffcore.h
 
 $(LIB_FILE): $(LIB_OBJS)
@@ -410,9 +417,10 @@ check:
 
 ### Installation rules
 
-install: $(PROGRAMS) $(SCRIPTS)
+install: $(PROGRAMS) $(SCRIPTS) $(SERVERSIDE_PROGRAMS)
 	$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(bindir))
-	$(INSTALL) $(PROGRAMS) $(SCRIPTS) $(call shellquote,$(DESTDIR)$(bindir))
+	$(INSTALL) $(PROGRAMS) $(SERVERSIDE_PROGRAMS) $(SCRIPTS) \
+		$(call shellquote,$(DESTDIR)$(bindir))
 	sh ./cmd-rename.sh $(call shellquote,$(DESTDIR)$(bindir))
 	$(MAKE) -C templates install
 	$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR))
diff --git a/connect.c b/connect.c
index b171c5d..0d78b3e 100644
--- a/connect.c
+++ b/connect.c
@@ -436,33 +436,44 @@ static int git_tcp_connect(int fd[2], co
 int git_connect(int fd[2], char *url, const char *prog)
 {
 	char command[1024];
-	char *host, *path;
-	char *colon;
+	char *host, *path, *ptr = NULL;
 	int pipefd[2][2];
 	pid_t pid;
 	enum protocol protocol;
 
+	protocol = PROTO_LOCAL;
 	host = NULL;
 	path = url;
-	colon = strchr(url, ':');
-	protocol = PROTO_LOCAL;
-	if (colon) {
-		*colon = 0;
-		host = url;
-		path = colon+1;
-		protocol = PROTO_SSH;
-		if (!memcmp(path, "//", 2)) {
-			char *slash = strchr(path + 2, '/');
-			if (slash) {
-				int nr = slash - path - 2;
-				memmove(path, path+2, nr);
-				path[nr] = 0;
-				protocol = get_protocol(url);
-				host = path;
-				path = slash;
-			}
+	host = strstr(url, "://");
+	if (host) {
+		*host = '\0';
+		host += 3;
+		protocol = get_protocol(url);
+	}
+	else host = url;
+
+	ptr = strchr(host, ':');
+	path = strchr(host, '/');
+
+	/* leading colon marks relative path for ssh.
+	 * Check for host == url and default to PROTO_SSH to allow
+	 *   $ git fetch kernel.org:git
+	 */
+	if(ptr && (!path || ptr < path)) {
+		if(host == url)
+			protocol = PROTO_SSH;
+
+		if(protocol == PROTO_SSH) {
+			*ptr = '\0';
+			path = ptr + 1;
 		}
 	}
+	else if(path != url) {
+		/* null-terminate host and copy path if it isn't relative */
+		ptr = strdup(path);
+		*path = '\0';
+		path = ptr;
+	}
 
 	if (protocol == PROTO_GIT)
 		return git_tcp_connect(fd, prog, host, path);
diff --git a/receive-pack.c b/receive-pack.c
index 8f157bc..9a040ff 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -8,6 +8,8 @@ static const char receive_pack_usage[] =
 
 static const char unpacker[] = "git-unpack-objects";
 
+extern void srvside_chdir(const char *path, int strict);
+
 static int show_ref(const char *path, const unsigned char *sha1)
 {
 	packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
@@ -265,18 +267,9 @@ int main(int argc, char **argv)
 	if (!dir)
 		usage(receive_pack_usage);
 
-	/* chdir to the directory. If that fails, try appending ".git" */
-	if (chdir(dir) < 0) {
-		if (chdir(mkpath("%s.git", dir)) < 0)
-			die("unable to cd to %s", dir);
-	}
-
-	/* If we have a ".git" directory, chdir to it */
-	chdir(".git");
-	putenv("GIT_DIR=.");
+	/* Find the right directory */
+	srvside_chdir(dir, 0);
 
-	if (access("objects", X_OK) < 0 || access("refs/heads", X_OK) < 0)
-		die("%s doesn't appear to be a git directory", dir);
 	write_head_info();
 
 	/* EOF */
diff --git a/srvside-ssh.c b/srvside-ssh.c
new file mode 100644
index 0000000..0ed5d30
--- /dev/null
+++ b/srvside-ssh.c
@@ -0,0 +1,63 @@
+#include "cache.h"
+#include <unistd.h>
+#include <pwd.h>
+
+extern const char *__progname;
+
+
+/*
+ * Provide support for user-relative paths, but carefully.
+ *
+ * If someone has compromised an account with access limited to a few
+ * commands (git-receive-pack, git-upload-pack) we don't want to let
+ * them find out about other users through git.  We prevent that by the
+ * simple expedient of maintaining the 'path' variable as is and always
+ * supplying the same (not entirely) helpful error message.
+ */
+#define DIR_OOPS \
+	die("%s: '%s': unable to chdir or not a git-archive", __progname, path)
+void srvside_chdir(char *path, int strict)
+{
+	char *dir = path;
+	struct passwd *pw;
+
+	if(chdir(path) < 0 && *path == '~') {
+		char *slash;
+		char *user = (char *)path + 1;
+
+		if((slash = strchr(dir, '/')))
+			*slash = '\0';
+
+		if(!(pw = getpwnam(user))) {
+			if(slash)
+				*slash = '/';
+			DIR_OOPS;
+		}
+		if(chdir(pw->pw_dir) < 0) {
+			DIR_OOPS;
+		}
+
+		/* We're in someones homedir so re-insert the slash (for the
+		 * error message) and set dir just beyond it. If there was no
+		 * spoon we supply the local one */
+		if(slash) {
+			*slash = '/';
+			dir = slash + 1;
+		}
+		else
+			dir = "./";
+	}
+
+	/* chdir to the directory. If that fails, try appending ".git" */
+	if (chdir(dir) < 0) {
+		if (strict || chdir(mkpath("%s.git", dir)) < 0)
+			DIR_OOPS;
+	}
+	if (!strict)
+		chdir(".git");
+
+	if (access("objects", X_OK) || access("refs", X_OK))
+		DIR_OOPS;
+
+	putenv("GIT_DIR=.");
+}
diff --git a/upload-pack.c b/upload-pack.c
index accdba6..356c9b1 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -5,6 +5,7 @@
 #include "object.h"
 
 static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>";
+extern void srvside_chdir(const char *path, int strict);
 
 #define MAX_HAS 256
 #define MAX_NEEDS 256
@@ -202,7 +203,6 @@ static int upload_pack(void)
 
 int main(int argc, char **argv)
 {
-	const char *dir;
 	int i;
 	int strict = 0;
 
@@ -227,20 +227,10 @@ int main(int argc, char **argv)
 	
 	if (i != argc-1)
 		usage(upload_pack_usage);
-	dir = argv[i];
 
-	/* chdir to the directory. If that fails, try appending ".git" */
-	if (chdir(dir) < 0) {
-		if (strict || chdir(mkpath("%s.git", dir)) < 0)
-			die("git-upload-pack unable to chdir to %s", dir);
-	}
-	if (!strict)
-		chdir(".git");
-
-	if (access("objects", X_OK) || access("refs", X_OK))
-		die("git-upload-pack: %s doesn't seem to be a git archive", dir);
+	/* Find the right directory */
+	srvside_chdir(argv[i], strict);
 
-	putenv("GIT_DIR=.");
 	upload_pack();
 	return 0;
 }
Petr Baudis· Oct 23, 2005, 18:37 UTC · re: Andreas Ericsson · lore

Re: User-relative paths (was: Server side programs)

Dear diary, on Sun, Oct 23, 2005 at 11:41:52AM CEST, I got a letter where Andreas Ericsson <ae@op5.se> told me that...

Show 5 quoted lines
> Anyways, the attached patch does this. I've tested all the various 
> syntaxes and they work as expected. rsync, http and local files take the 
> same syntax as before. I haven't added support for user-relative paths 
> to the git-daemon (can't see the point, really) although that can be 
> done easily enough.

It would be useful to add a [PATCH] tag to subject when you submit a patch, so that we notice it better. ;-)

You don't update the documentation even though there seem to be some syntactic changes. You should at least update

	Documentation/pull-fetch-param.txt

Also before Junio asks you, in the followup patches, you might want to sign off the patch if you want it integrated.

Show 14 quoted lines
> diff --git a/Makefile b/Makefile
> index 903c57c..87188ea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -359,6 +362,9 @@ git-cherry-pick: git-revert
>  %.o: %.S
>  	$(CC) -o $*.o -c $(ALL_CFLAGS) $<
>  
> +$(SERVERSIDE_PROGRAMS) : git-%$X : %.o srvside-ssh.o $(LIB_FILE)
> +	$(CC) $(ALL_CFLAGS) -o $@ $(filter %o,$^) $(LIBS)
> +
>  git-%$X: %.o $(LIB_FILE)
>  	$(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
>  

Why are you adding own compilation command, and why is it inconsistent with the git-%$X's one?

Show 18 quoted lines
> diff --git a/connect.c b/connect.c
> index b171c5d..0d78b3e 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -436,33 +436,44 @@ static int git_tcp_connect(int fd[2], co
> +	/* leading colon marks relative path for ssh.
> +	 * Check for host == url and default to PROTO_SSH to allow
> +	 *   $ git fetch kernel.org:git
> +	 */
> +	if(ptr && (!path || ptr < path)) {
> +		if(host == url)
> +			protocol = PROTO_SSH;
> +
> +		if(protocol == PROTO_SSH) {
> +			*ptr = '\0';
> +			path = ptr + 1;
>  		}
>  	}
If I understand this right,
	ssh://foo.bar:baz/quux

will make foo.bar the host and baz/quux the path. Please, do NOT do this! It is supposed to be a URL, dammit! And you know, URLs have defined _syntax_, and that's important at least every time the URL gets out of GIT's context. Or stop it calling URL altogether, to prevent any confusion. But in URLs, the space between : and / is a port definition. See also RFC3986 (aka STD066) and RFC2718.

Thanks.
Show 25 quoted lines
> diff --git a/receive-pack.c b/receive-pack.c
> index 8f157bc..9a040ff 100644
> --- a/receive-pack.c
> +++ b/receive-pack.c
> @@ -265,18 +267,9 @@ int main(int argc, char **argv)
>  	if (!dir)
>  		usage(receive_pack_usage);
>  
> -	/* chdir to the directory. If that fails, try appending ".git" */
> -	if (chdir(dir) < 0) {
> -		if (chdir(mkpath("%s.git", dir)) < 0)
> -			die("unable to cd to %s", dir);
> -	}
> -
> -	/* If we have a ".git" directory, chdir to it */
> -	chdir(".git");
> -	putenv("GIT_DIR=.");
> +	/* Find the right directory */
> +	srvside_chdir(dir, 0);
>  
> -	if (access("objects", X_OK) < 0 || access("refs/heads", X_OK) < 0)
> -		die("%s doesn't appear to be a git directory", dir);
>  	write_head_info();
>  
>  	/* EOF */
No srvside_chdir() declaration?
Show 11 quoted lines
> diff --git a/srvside-ssh.c b/srvside-ssh.c
> new file mode 100644
> index 0000000..0ed5d30
> --- /dev/null
> +++ b/srvside-ssh.c
> @@ -0,0 +1,63 @@
> +#include "cache.h"
> +#include <unistd.h>
> +#include <pwd.h>
> +
> +extern const char *__progname;

How portable is this? It appears that no standard really defines this, and Google faintly hints at least some Cygwin-related problems...

Show 12 quoted lines
> diff --git a/upload-pack.c b/upload-pack.c
> index accdba6..356c9b1 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -5,6 +5,7 @@
>  #include "object.h"
>  
>  static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>";
> +extern void srvside_chdir(const char *path, int strict);
>  
>  #define MAX_HAS 256
>  #define MAX_NEEDS 256
What about a .h file?
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Junio C Hamano· Oct 23, 2005, 19:50 UTC · re: Petr Baudis · lore

Re: User-relative paths

Petr Baudis <pasky@suse.cz> writes:
Show 17 quoted lines
>> diff --git a/Makefile b/Makefile
>> index 903c57c..87188ea 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -359,6 +362,9 @@ git-cherry-pick: git-revert
>>  %.o: %.S
>>  	$(CC) -o $*.o -c $(ALL_CFLAGS) $<
>>  
>> +$(SERVERSIDE_PROGRAMS) : git-%$X : %.o srvside-ssh.o $(LIB_FILE)
>> +	$(CC) $(ALL_CFLAGS) -o $@ $(filter %o,$^) $(LIBS)
>> +
>>  git-%$X: %.o $(LIB_FILE)
>>  	$(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
>>  
>
> Why are you adding own compilation command, and why is it inconsistent
> with the git-%$X's one?

Although I'd prefer the simplicity of putting srvside-ssh.o in LIB_OBJS, this is arguably defensible; it avoids relinking of everything else merely because srvside-ssh.c is changed.

Petr Baudis· Oct 23, 2005, 22:25 UTC · re: Junio C Hamano · lore

Re: User-relative paths

Dear diary, on Sun, Oct 23, 2005 at 09:50:25PM CEST, I got a letter where Junio C Hamano <junkio@cox.net> told me that...

Show 23 quoted lines
> Petr Baudis <pasky@suse.cz> writes:
> 
> >> diff --git a/Makefile b/Makefile
> >> index 903c57c..87188ea 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -359,6 +362,9 @@ git-cherry-pick: git-revert
> >>  %.o: %.S
> >>  	$(CC) -o $*.o -c $(ALL_CFLAGS) $<
> >>  
> >> +$(SERVERSIDE_PROGRAMS) : git-%$X : %.o srvside-ssh.o $(LIB_FILE)
> >> +	$(CC) $(ALL_CFLAGS) -o $@ $(filter %o,$^) $(LIBS)
> >> +
> >>  git-%$X: %.o $(LIB_FILE)
> >>  	$(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
> >>  
> >
> > Why are you adding own compilation command, and why is it inconsistent
> > with the git-%$X's one?
> 
> Although I'd prefer the simplicity of putting srvside-ssh.o in
> LIB_OBJS, this is arguably defensible; it avoids relinking of
> everything else merely because srvside-ssh.c is changed.

I'm talking only about the compilation command, not about the dependency line.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Daniel Barkalow· Oct 24, 2005, 06:28 UTC · re: Junio C Hamano · lore

Re: User-relative paths

On Sun, 23 Oct 2005, Junio C Hamano wrote:
Show 23 quoted lines
> Petr Baudis <pasky@suse.cz> writes:
> 
> >> diff --git a/Makefile b/Makefile
> >> index 903c57c..87188ea 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -359,6 +362,9 @@ git-cherry-pick: git-revert
> >>  %.o: %.S
> >>  	$(CC) -o $*.o -c $(ALL_CFLAGS) $<
> >>  
> >> +$(SERVERSIDE_PROGRAMS) : git-%$X : %.o srvside-ssh.o $(LIB_FILE)
> >> +	$(CC) $(ALL_CFLAGS) -o $@ $(filter %o,$^) $(LIBS)
> >> +
> >>  git-%$X: %.o $(LIB_FILE)
> >>  	$(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
> >>  
> >
> > Why are you adding own compilation command, and why is it inconsistent
> > with the git-%$X's one?
> 
> Although I'd prefer the simplicity of putting srvside-ssh.o in
> LIB_OBJS, this is arguably defensible; it avoids relinking of
> everything else merely because srvside-ssh.c is changed.
The line:
$(SERVERSIDE_PROGRAMS): srvside-ssh.o
would suffice for that.
	-Daniel
*This .sig left intentionally blank*
Andreas Ericsson· Oct 25, 2005, 07:47 UTC · re: Petr Baudis · lore

Re: User-relative paths

Petr Baudis wrote:
Show 13 quoted lines
> Dear diary, on Sun, Oct 23, 2005 at 11:41:52AM CEST, I got a letter
> where Andreas Ericsson <ae@op5.se> told me that...
> 
>>Anyways, the attached patch does this. I've tested all the various 
>>syntaxes and they work as expected. rsync, http and local files take the 
>>same syntax as before. I haven't added support for user-relative paths 
>>to the git-daemon (can't see the point, really) although that can be 
>>done easily enough.
> 
> 
> It would be useful to add a [PATCH] tag to subject when you submit a
> patch, so that we notice it better. ;-)
> 

Will do in the future. I thought it was auto-imported for buildtest if it was.

Show 5 quoted lines
> You don't update the documentation even though there seem to be some
> syntactic changes. You should at least update
> 
> 	Documentation/pull-fetch-param.txt
> 

True. I'll need to re-work the patch a bit to take Junio's RFC on paths into account. I'll do this then.

Show 23 quoted lines
> Also before Junio asks you, in the followup patches, you might want to
> sign off the patch if you want it integrated.
> 
> 
>>diff --git a/Makefile b/Makefile
>>index 903c57c..87188ea 100644
>>--- a/Makefile
>>+++ b/Makefile
>>@@ -359,6 +362,9 @@ git-cherry-pick: git-revert
>> %.o: %.S
>> 	$(CC) -o $*.o -c $(ALL_CFLAGS) $<
>> 
>>+$(SERVERSIDE_PROGRAMS) : git-%$X : %.o srvside-ssh.o $(LIB_FILE)
>>+	$(CC) $(ALL_CFLAGS) -o $@ $(filter %o,$^) $(LIBS)
>>+
>> git-%$X: %.o $(LIB_FILE)
>> 	$(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)
>> 
> 
> 
> Why are you adding own compilation command, and why is it inconsistent
> with the git-%$X's one?
> 

Mainly because I'm really no good at Makefiles and just noticed that this seems to do what I want. My own projects rarely stretch over 15 files and it's usually just one or two binaries, so I haven't gotten round to learning the finer points of make.

Show 34 quoted lines
> 
>>diff --git a/connect.c b/connect.c
>>index b171c5d..0d78b3e 100644
>>--- a/connect.c
>>+++ b/connect.c
>>@@ -436,33 +436,44 @@ static int git_tcp_connect(int fd[2], co
>>+	/* leading colon marks relative path for ssh.
>>+	 * Check for host == url and default to PROTO_SSH to allow
>>+	 *   $ git fetch kernel.org:git
>>+	 */
>>+	if(ptr && (!path || ptr < path)) {
>>+		if(host == url)
>>+			protocol = PROTO_SSH;
>>+
>>+		if(protocol == PROTO_SSH) {
>>+			*ptr = '\0';
>>+			path = ptr + 1;
>> 		}
>> 	}
> 
> 
> If I understand this right,
> 
> 	ssh://foo.bar:baz/quux
> 
> will make foo.bar the host and baz/quux the path. Please, do NOT do
> this! It is supposed to be a URL, dammit! And you know, URLs have
> defined _syntax_, and that's important at least every time the URL gets
> out of GIT's context. Or stop it calling URL altogether, to prevent any
> confusion. But in URLs, the space between : and / is a port definition.
> See also RFC3986 (aka STD066) and RFC2718.
> 
> Thanks.
> 
Right you are. I was thinking scp like syntax rather than url.
Show 47 quoted lines
> 
>>diff --git a/receive-pack.c b/receive-pack.c
>>index 8f157bc..9a040ff 100644
>>--- a/receive-pack.c
>>+++ b/receive-pack.c
>>@@ -265,18 +267,9 @@ int main(int argc, char **argv)
>> 	if (!dir)
>> 		usage(receive_pack_usage);
>> 
>>-	/* chdir to the directory. If that fails, try appending ".git" */
>>-	if (chdir(dir) < 0) {
>>-		if (chdir(mkpath("%s.git", dir)) < 0)
>>-			die("unable to cd to %s", dir);
>>-	}
>>-
>>-	/* If we have a ".git" directory, chdir to it */
>>-	chdir(".git");
>>-	putenv("GIT_DIR=.");
>>+	/* Find the right directory */
>>+	srvside_chdir(dir, 0);
>> 
>>-	if (access("objects", X_OK) < 0 || access("refs/heads", X_OK) < 0)
>>-		die("%s doesn't appear to be a git directory", dir);
>> 	write_head_info();
>> 
>> 	/* EOF */
> 
> 
> No srvside_chdir() declaration?
> 
> 
>>diff --git a/srvside-ssh.c b/srvside-ssh.c
>>new file mode 100644
>>index 0000000..0ed5d30
>>--- /dev/null
>>+++ b/srvside-ssh.c
>>@@ -0,0 +1,63 @@
>>+#include "cache.h"
>>+#include <unistd.h>
>>+#include <pwd.h>
>>+
>>+extern const char *__progname;
> 
> 
> How portable is this? It appears that no standard really defines this,
> and Google faintly hints at least some Cygwin-related problems...
> 

Somewhat, but not very. It was more of a quick hack since the old code had hardcoded program names. If this gets supported in git-daemon as well it should say "git-daemon" in the error message, so I think either pass it as a parameter or invent some git_progname variable and use some small init-code for all programs.

Show 17 quoted lines
> 
>>diff --git a/upload-pack.c b/upload-pack.c
>>index accdba6..356c9b1 100644
>>--- a/upload-pack.c
>>+++ b/upload-pack.c
>>@@ -5,6 +5,7 @@
>> #include "object.h"
>> 
>> static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>";
>>+extern void srvside_chdir(const char *path, int strict);
>> 
>> #define MAX_HAS 256
>> #define MAX_NEEDS 256
> 
> 
> What about a .h file?
> 

Prototype patch, sort of, and since it's only one function I thought it'd be better to keep it as unobtrusive as possible.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Junio C Hamano· Oct 23, 2005, 19:56 UTC · re: Andreas Ericsson · lore

Re: User-relative paths

Andreas Ericsson <ae@op5.se> writes:
Show 14 quoted lines
> Junio C Hamano wrote:
>> Andreas Ericsson <ae@op5.se> writes:
>> ...
>> At one point, Linus posted an outline of "restricted login shell
>> for use with git over ssh".  I think you could start from there,
>> perhaps extend it so that it checks the binaries *and* pathnames
>> the user can specify (e.g. only under your own $HOME is allowed,
>> and no /../ in them, or something silly like that).
>>
>
> I found this in the archives:
> http://article.gmane.org/gmane.comp.version-control.git/5784/match=restricted+login
>
> Is that what you're referring to?
No, it is this one:
    http://marc.theaimsgroup.com/?l=git&m=112681457828137&w=2
But it is orthogonal to what you are doing in this patch.
> Let me know if you want things done differently.

I think srvside_chdir() should not do the userdir expansion under --strict (otherwise you would need a matching change in daemon.c as well, but I would rather not).

The --strict flag in upload-pack is to make sure git-daemon can see what is being accessed and make its policy decision even before it calls upload-pack. In a pathological case, somebody can create a directory "/~foo/bar/.git", where the "/~foo" directory is different from "/home/foo", and have git-daemon check that the former is OK and call your upload-pack. Your upload-pack uses srvside_chdir() and exposes /home/foo/bar/.git; this circumvents git-daemon's policy decision, doesn't it?

I also agree with everything Pasky already said.
 * In a URL, a colon after hostname means "port number
   follows".  So it was a good intention to make these
   consistent:
        git fetch ssh://kernel.org:git
        git fetch kernel.org:git
   it should not be done.  IOW, if I wanted to use the former
   form (which I do not think I'd use myself), I should say either one
   of:
        git fetch ssh://kernel.org:~/git
        git fetch ssh://kernel.org:~junio/git
   Oh, I just noticed you do not handle the former, because you
   did not have to, but now you need to.
 * Use of "extern const char *__progname" is questionable.  I
   could be easily talked into:
    - have "extern const char *git_program_name" in cache.h or
      somewhere;
    - convert programs (gradually) to set that at the beginning
      of main();
    - update die() and error() to use that variable when
      reporting (both callers and implementation) -- this is
      optional.
Linus Torvalds· Oct 23, 2005, 21:30 UTC · re: Junio C Hamano · lore

Re: User-relative paths

On Sun, 23 Oct 2005, Junio C Hamano wrote:
Show 6 quoted lines
> 
> No, it is this one:
> 
>     http://marc.theaimsgroup.com/?l=git&m=112681457828137&w=2
> 
> But it is orthogonal to what you are doing in this patch.
Well, not necessarily.

It's quite arguable that sanity testing might be per-user and could be done by the shell. I'm not at all sure that srvside_chdir() should do any extra testing: if you have real ssh access, the user has the right to do anything he damn well pleases.

So it's quite possible that you should do testing in the thing that receives the request, ie in a restricted shell (or, as we already do, in git-daemon).

I tried to find my original unquote example and stupid shell, but couldn't. So I wrote something untested as usual.

It's incomplete and almost certainly buggy and generally broken, but here's somethign that you _could_ install as "git-shell", and then put that as somebodys shell in /etc/passwd, and it's a start. A very rough start.

Somebody else gets to test it out ;)
		Linus

--- 2906a25bbd1dedbd6bab9ed984a503340229b020

diff --git a/Makefile b/Makefile
index 7eacf61..34bbdb6 100644
--- a/Makefile
+++ b/Makefile
@@ -102,7 +102,7 @@ SCRIPT_PYTHON = \
 # The ones that do not have to link with lcrypto nor lz.
 SIMPLE_PROGRAMS = \
 	git-get-tar-commit-id$X git-mailinfo$X git-mailsplit$X \
-	git-stripspace$X git-var$X git-daemon$X
+	git-stripspace$X git-var$X git-daemon$X git-shell$X
 
 # ... and all the rest
 PROGRAMS = \
diff --git a/shell.c b/shell.c
new file mode 100644
index 0000000..676d398
--- /dev/null
+++ b/shell.c
@@ -0,0 +1,89 @@
+#include "cache.h"
+
+static char *dequote(char *arg)
+{
+	char *dst = arg;
+	char *src = arg;
+	char c;
+
+	if (*src != '\'')
+		return NULL;
+	for (;;) {
+		c = *++src;
+		if (!c)
+			return NULL;
+		if (c != '\'') {
+			*dst++ = c;
+			continue;
+		}
+		switch (*++src) {
+		case '\0':
+			*dst = 0;
+			return arg;
+		case '\\':
+			if (*++src == '\'' &&
+			    *++src == '\'') {
+				*dst = '\'';
+				continue;
+			}
+		/* Fallthrough */
+		default:
+			return NULL;
+		}
+	}
+}
+
+static int do_receive_pack(char *arg)
+{
+	char cwd[1000];
+	char *my_argv[4];
+
+	arg = dequote(arg);
+	if (!arg)	
+		die("bad argument");
+
+	my_argv[0] = "git-receive-pack";
+	my_argv[1] = arg;
+	my_argv[2] = NULL;
+	return execvp("git-receive-pack", my_argv);
+}
+
+static struct commands {
+	const char *name;
+	int (*exec)(char *arg);
+} cmd_list[] = {
+	{ "git-receive-pack", do_receive_pack },
+	{ NULL },
+};
+
+int main(int argc, char **argv)
+{
+	char *prog;
+	struct commands *cmd;
+
+	/* We want to see "-c cmd args", and nothing else */
+	if (argc != 3 || strcmp(argv[1], "-c"))
+		die("What do you think I am? A shell?");
+	prog = argv[2];
+	argv += 2;
+	argc -= 2;
+	for (cmd = cmd_list ; cmd->name ; cmd++) {
+		int len = strlen(cmd->name);
+		char *arg;
+		if (strncmp(cmd->name, prog, len))
+			continue;
+		arg = NULL;
+		switch (prog[len]) {
+		case '\0':
+			arg = NULL;
+			break;
+		case ' ':
+			arg = prog + len + 1;
+			break;
+		default:
+			continue;
+		}
+		exit(cmd->exec(arg));
+	}
+	die("unrecognized command '%s'", prog);
+}
Junio C Hamano· Oct 23, 2005, 22:57 UTC · re: Linus Torvalds · lore

Re: User-relative paths

Linus Torvalds <torvalds@osdl.org> writes:
Show 8 quoted lines
>> But it is orthogonal to what you are doing in this patch.
>
> Well, not necessarily.
>
> It's quite arguable that sanity testing might be per-user and could be 
> done by the shell. I'm not at all sure that srvside_chdir() should do any 
> extra testing: if you have real ssh access, the user has the right to do 
> anything he damn well pleases.

The point of the patch, unless I am mistaken, is to add ~user/ expansion to the pathname grokking, so that a remote user does not have to know exactly where on the server each user's home directory is.

I agree 100% with you that srvside_chdir() is not the place to do policy checking. In order to avoid the aliasing problem (which motivated HPA to add --strict option), the receiving end, be it git-daemon driving upload-pack or git-shell driving receive-pack or upload-pack, can do ~user/ expansion first, then run their policy checking on the canonicalized path before spawning the lower level programs using the already canonicalized path.

To also support the case where upload-pack and receive-pack are started directly from the ssh connection, these programs need to apply ~user/ expansion to the incoming path themselves by default. In order to avoid double expansion, git-daemon and git-shell should pass --no-user-expansion flag to the lower level programs when it starts them if we do this.

A common library that takes the path supplied from the other end and does ~user/ expansion would be useful for the above; we can lift that logic from Andreas' srvside_chdir().

Junio C Hamano· Oct 23, 2005, 23:02 UTC · re: Linus Torvalds · lore

Re: User-relative paths

Linus Torvalds <torvalds@osdl.org> writes:
Show 23 quoted lines
> It's incomplete and almost certainly buggy and generally broken, but 
> here's somethign that you _could_ install as "git-shell", and then put 
> that as somebodys shell in /etc/passwd, and it's a start. A very rough 
> start.
>
> Somebody else gets to test it out ;)
>
> +		if (c != '\'') {
> +			*dst++ = c;
> +			continue;
> +		}
> +		switch (*++src) {
> +		case '\0':
> +			*dst = 0;
> +			return arg;
> +		case '\\':
> +			if (*++src == '\'' &&
> +			    *++src == '\'') {
> +				*dst = '\'';
> +				continue;
> +			}
> +		/* Fallthrough */
> +		default:

I think this misses HPA's addition to minimally suppport csh braindamage (bang bang).

H. Peter Anvin· Oct 24, 2005, 01:08 UTC · re: Junio C Hamano · lore

Re: User-relative paths

Junio C Hamano wrote:
Show 30 quoted lines
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> 
>>It's incomplete and almost certainly buggy and generally broken, but 
>>here's somethign that you _could_ install as "git-shell", and then put 
>>that as somebodys shell in /etc/passwd, and it's a start. A very rough 
>>start.
>>
>>Somebody else gets to test it out ;)
>>
>>+		if (c != '\'') {
>>+			*dst++ = c;
>>+			continue;
>>+		}
>>+		switch (*++src) {
>>+		case '\0':
>>+			*dst = 0;
>>+			return arg;
>>+		case '\\':
>>+			if (*++src == '\'' &&
>>+			    *++src == '\'') {
>>+				*dst = '\'';
>>+				continue;
>>+			}
>>+		/* Fallthrough */
>>+		default:
> 
> I think this misses HPA's addition to minimally suppport csh
> braindamage (bang bang).
> 
If this is meant to dequote shell-quoted paths, it really should be modal.
	-hpa
Linus Torvalds· Oct 24, 2005, 01:37 UTC · re: H. Peter Anvin · lore

Re: User-relative paths

On Sun, 23 Oct 2005, H. Peter Anvin wrote:
> 
> If this is meant to dequote shell-quoted paths, it really should be modal.

It _only_ accepts quoted strings, so it "is" modal. It has one mode: string. And it's a bitch about enforcing it, too (it just dies if it wasn't one).

		Linus
H. Peter Anvin· Oct 24, 2005, 01:44 UTC · re: Linus Torvalds · lore

Re: User-relative paths

Linus Torvalds wrote:
Show 9 quoted lines
> 
> On Sun, 23 Oct 2005, H. Peter Anvin wrote:
> 
>>If this is meant to dequote shell-quoted paths, it really should be modal.
> 
> It _only_ accepts quoted strings, so it "is" modal. It has one mode: 
> string. And it's a bitch about enforcing it, too (it just dies if it 
> wasn't one).
> 

That wasn't what I meant. '...' is a modal escape in the shell. Thus, something like this which actually mimics the state machine, at least for the potential characters we care about.

#define EMIT(x) { ( ++len < n ) && *dst++ = (x) )
int unquote(char *dst, size_t n, const char *src)
{
	enum state st = { st_zero, st_quote, st_escape };
	int len = 0;
	char c;
	
	while ( (c = *src++) ) {
		switch ( st ) {
		case st_zero:
			if ( c == '\'' )
				st = st_quote;
			else if ( c == '\\' )
				st = st_escape;
			else
				EMIT(c);
			break;
			
		case st_quote:
			if ( c == '\'' )
				st = st_zero;
			else
				EMIT(c);
			break;
			
		case st_escape:
			EMIT(c);
			st = st_zero;
			break;
		}
	}
	
	if ( n )
		*dst = 0;
	
	return (st == st_zero) ? len : -1;
}
Junio C Hamano· Oct 24, 2005, 01:56 UTC · re: H. Peter Anvin · lore

Re: User-relative paths

"H. Peter Anvin" <hpa@zytor.com> writes:
> If this is meant to dequote shell-quoted paths, it really should be modal.

You are right, but this is not meant to dequote arbitrary shell quoted paths. It specifically is for unwrapping what sq_quote() produced, and refuses to proceed if you feed some shell-valid but not sq_quote() produced string. The reason is because it is part of the git-shell "login shell" -- the input validation is enforced there.

And the reason it lives next to quote.c::sq_quote() is that you would hopefully notice sq_dequote() needs matching changes when you ever update sq_quote.

Junio C Hamano· Oct 24, 2005, 00:21 UTC · re: Linus Torvalds · lore

[PATCH] Add git-shell.

This adds a very git specific restricted shell, that can be added to /etc/shells and set to the pw_shell in the /etc/passwd file, to give users ability to push into repositories over ssh without giving them full interactive shell acount.

[jc: I updated Linus' patch to match what the current sq_quote()
 does.]
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
   This one has the '!' dequoting I mentioned earlier, and
   actually has been tested twice.
   The system administrator should set things up so that the
   system default PATH environment variable lets users run the
   supported commands.
   We currently rely on the user to have a full shell access for
   repository administrative actions (e.g. git-init-db to create
   a repository, git-repack, and hooks management).  Probably
   some of them may need to become accessible from git-shell,
   but I do not know which ones offhand:
   - Creating a repository.  Probably wherever the user has
     write privilege, or maybe only under the home directory --
     the policy would be up to the system administrator, with
     usual filesystem quota applied.
   - I think setting up hooks should be forbidden -- the user
     can execute arbitrary commands if we allowed it.  Instead,
     the administrator can set things up for people, and would
     probably make creative use of hooks/update to implement
     access control (e.g. forbidding non-fast-forward pushes).
   - Similarly, packing can be left to hooks/post-update or cron
     job, either of which is under administrator control.
 Makefile |    2 +-
 quote.c  |   41 ++++++++++++++++++++++++++++++++++++++++-
 quote.h  |    6 ++++++
 shell.c  |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 shell.c

applies-to: c043f0993afe5c057409ef748ce313c89c20c7dc f176536e2cd5949fa134d8b24dc07dd1f0744b6f

diff --git a/Makefile b/Makefile
index 5bdf3cc..5b0306d 100644
--- a/Makefile
+++ b/Makefile
@@ -116,7 +116,7 @@ PROGRAMS = \
 	git-merge-index$X git-mktag$X git-pack-objects$X git-patch-id$X \
 	git-peek-remote$X git-prune-packed$X git-read-tree$X \
 	git-receive-pack$X git-rev-list$X git-rev-parse$X \
-	git-send-pack$X git-show-branch$X \
+	git-send-pack$X git-show-branch$X git-shell$X \
 	git-show-index$X git-ssh-fetch$X \
 	git-ssh-upload$X git-tar-tree$X git-unpack-file$X \
 	git-unpack-objects$X git-update-index$X git-update-server-info$X \
diff --git a/quote.c b/quote.c
index 009e694..e662a7d 100644
--- a/quote.c
+++ b/quote.c
@@ -15,6 +15,11 @@
 #undef EMIT
 #define EMIT(x) ( (++len < n) && (*bp++ = (x)) )
 
+static inline int need_bs_quote(char c)
+{
+	return (c == '\'' || c == '!');
+}
+
 size_t sq_quote_buf(char *dst, size_t n, const char *src)
 {
 	char c;
@@ -23,7 +28,7 @@ size_t sq_quote_buf(char *dst, size_t n,
 
 	EMIT('\'');
 	while ((c = *src++)) {
-		if (c == '\'' || c == '!') {
+		if (need_bs_quote(c)) {
 			EMIT('\'');
 			EMIT('\\');
 			EMIT(c);
@@ -52,6 +57,40 @@ char *sq_quote(const char *src)
 	return buf;
 }
 
+char *sq_dequote(char *arg)
+{
+	char *dst = arg;
+	char *src = arg;
+	char c;
+
+	if (*src != '\'')
+		return NULL;
+	for (;;) {
+		c = *++src;
+		if (!c)
+			return NULL;
+		if (c != '\'') {
+			*dst++ = c;
+			continue;
+		}
+		/* We stepped out of sq */
+		switch (*++src) {
+		case '\0':
+			*dst = 0;
+			return arg;
+		case '\\':
+			c = *++src;
+			if (need_bs_quote(c) && *++src == '\'') {
+				*dst++ = c;
+				continue;
+			}
+		/* Fallthrough */
+		default:
+			return NULL;
+		}
+	}
+}
+
 /*
  * C-style name quoting.
  *
diff --git a/quote.h b/quote.h
index 2fdde3b..2486e6e 100644
--- a/quote.h
+++ b/quote.h
@@ -31,6 +31,12 @@
 extern char *sq_quote(const char *src);
 extern size_t sq_quote_buf(char *dst, size_t n, const char *src);
 
+/* This unwraps what sq_quote() produces in place, but returns
+ * NULL if the input does not look like what sq_quote would have
+ * produced.
+ */
+extern char *sq_dequote(char *);
+
 extern int quote_c_style(const char *name, char *outbuf, FILE *outfp,
 			 int nodq);
 extern char *unquote_c_style(const char *quoted, const char **endp);
diff --git a/shell.c b/shell.c
new file mode 100644
index 0000000..2c4789e
--- /dev/null
+++ b/shell.c
@@ -0,0 +1,59 @@
+#include "cache.h"
+#include "quote.h"
+
+static int do_generic_cmd(const char *me, char *arg)
+{
+	const char *my_argv[4];
+
+	arg = sq_dequote(arg);
+	if (!arg)
+		die("bad argument");
+
+	my_argv[0] = me;
+	my_argv[1] = arg;
+	my_argv[2] = NULL;
+
+	return execvp(me, (char**) my_argv);
+}
+
+static struct commands {
+	const char *name;
+	int (*exec)(const char *me, char *arg);
+} cmd_list[] = {
+	{ "git-receive-pack", do_generic_cmd },
+	{ "git-upload-pack", do_generic_cmd },
+	{ NULL },
+};
+
+int main(int argc, char **argv)
+{
+	char *prog;
+	struct commands *cmd;
+
+	/* We want to see "-c cmd args", and nothing else */
+	if (argc != 3 || strcmp(argv[1], "-c"))
+		die("What do you think I am? A shell?");
+
+	prog = argv[2];
+	argv += 2;
+	argc -= 2;
+	for (cmd = cmd_list ; cmd->name ; cmd++) {
+		int len = strlen(cmd->name);
+		char *arg;
+		if (strncmp(cmd->name, prog, len))
+			continue;
+		arg = NULL;
+		switch (prog[len]) {
+		case '\0':
+			arg = NULL;
+			break;
+		case ' ':
+			arg = prog + len + 1;
+			break;
+		default:
+			continue;
+		}
+		exit(cmd->exec(cmd->name, arg));
+	}
+	die("unrecognized command '%s'", prog);
+}
---
0.99.8.GIT
Linus Torvalds· Oct 24, 2005, 00:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add git-shell.

On Sun, 23 Oct 2005, Junio C Hamano wrote:
Show 5 quoted lines
>
> This adds a very git specific restricted shell, that can be
> added to /etc/shells and set to the pw_shell in the /etc/passwd
> file, to give users ability to push into repositories over ssh
> without giving them full interactive shell acount.

Did you actually test that it works as somebody's login-shell and can be used for pushing?

I think it should add "pull" functionality too, so that you can have restricted reading (hey, git may be open source, but not everything that is maintained in it necessarily will be..)

			Linus
Linus Torvalds· Oct 24, 2005, 00:55 UTC · re: Linus Torvalds · lore

Re: [PATCH] Add git-shell.

On Sun, 23 Oct 2005, Linus Torvalds wrote:
Show 5 quoted lines
> 
> Did you actually test that it works as somebody's login-shell and can be 
> used for pushing?
> 
> I think it should add "pull" functionality too

Gaah, I only read your description, didn't look closer at the patch. The description just said "push", but you added the pull side too, and apparently even tested it. Goodie.

Never mind me.
			Linus
Junio C Hamano· Oct 24, 2005, 01:36 UTC · re: Linus Torvalds · lore

Re: [PATCH] Add git-shell.

Linus Torvalds <torvalds@osdl.org> writes:
Show 9 quoted lines
> On Sun, 23 Oct 2005, Junio C Hamano wrote:
>>
>> This adds a very git specific restricted shell, that can be
>> added to /etc/shells and set to the pw_shell in the /etc/passwd
>> file, to give users ability to push into repositories over ssh
>> without giving them full interactive shell acount.
>
> Did you actually test that it works as somebody's login-shell and can be 
> used for pushing?

I made a temporary user on my notebook, set it as his login shell, and run peek-remote against it (which means the git-shell spawned upload-pack on the other end). No, I did not try pushing when I sent the patch

But I just did, and it seems to work, so this should hit the master branch soon ;-).

Junio C Hamano· Oct 24, 2005, 02:08 UTC · re: Junio C Hamano · lore

Re: User-relative paths

Junio C Hamano <junkio@cox.net> writes:
Show 16 quoted lines
>  * In a URL, a colon after hostname means "port number
>    follows".  So it was a good intention to make these
>    consistent:
>
>         git fetch ssh://kernel.org:git
>         git fetch kernel.org:git
>
>    it should not be done.  IOW, if I wanted to use the former
>    form (which I do not think I'd use myself), I should say either one
>    of:
>
>         git fetch ssh://kernel.org:~/git
>         git fetch ssh://kernel.org:~junio/git
>
>    Oh, I just noticed you do not handle the former, because you
>    did not have to, but now you need to.

The "should be this way" examples should have been something like this:

        git fetch ssh://kernel.org/~/git
        git fetch ssh://kernel.org/~junio/git
Otherwise what I said does not make *any* sense.
          	 Junio "Oh, I am an idiot" Hamano
Andreas Ericsson· Oct 25, 2005, 09:11 UTC · re: Junio C Hamano · lore

[PATCH] git_progname (was: Re: User-relative paths)

Junio C Hamano wrote:
Show 34 quoted lines
> Andreas Ericsson <ae@op5.se> writes:
> 
> 
>>Junio C Hamano wrote:
>>
>>>Andreas Ericsson <ae@op5.se> writes:
>>>...
>>>At one point, Linus posted an outline of "restricted login shell
>>>for use with git over ssh".  I think you could start from there,
>>>perhaps extend it so that it checks the binaries *and* pathnames
>>>the user can specify (e.g. only under your own $HOME is allowed,
>>>and no /../ in them, or something silly like that).
>>>
>>
>>I found this in the archives:
>>http://article.gmane.org/gmane.comp.version-control.git/5784/match=restricted+login
>>
>>Is that what you're referring to?
> 
> 
> No, it is this one:
> 
>     http://marc.theaimsgroup.com/?l=git&m=112681457828137&w=2
> 
> But it is orthogonal to what you are doing in this patch.
> 
> 
>>Let me know if you want things done differently.
> 
> 
> I think srvside_chdir() should not do the userdir expansion
> under --strict (otherwise you would need a matching change in
> daemon.c as well, but I would rather not).
> 
True. I'll rework it.
Show 7 quoted lines
> The --strict flag in upload-pack is to make sure git-daemon can
> see what is being accessed and make its policy decision even
> before it calls upload-pack.  In a pathological case, somebody
> can create a directory "/~foo/bar/.git", where the "/~foo"
> directory is different from "/home/foo", and have git-daemon
> check that the former is OK and call your upload-pack.  Your
> upload-pack uses srvside_chdir() and exposes /home/foo/bar/.git;

It shouldn't, because srvside_chdir() will only user-expand paths that start with a tilde.

Show 30 quoted lines
> this circumvents git-daemon's policy decision, doesn't it?
> 
> I also agree with everything Pasky already said.
> 
>  * In a URL, a colon after hostname means "port number
>    follows".  So it was a good intention to make these
>    consistent:
> 
>         git fetch ssh://kernel.org:git
>         git fetch kernel.org:git
> 
>    it should not be done.  IOW, if I wanted to use the former
>    form (which I do not think I'd use myself), I should say either one
>    of:
> 
>         git fetch ssh://kernel.org:~/git
>         git fetch ssh://kernel.org:~junio/git
> 
>    Oh, I just noticed you do not handle the former, because you
>    did not have to, but now you need to.
> 
>  * Use of "extern const char *__progname" is questionable.  I
>    could be easily talked into:
> 
>     - have "extern const char *git_program_name" in cache.h or
>       somewhere;
> 
>     - convert programs (gradually) to set that at the beginning
>       of main();
> 

See the attached patch, which adds git_progname as a global variable to daemon.c with a minimum of fuzz. The one-liner below will add it to the rest of the programs. GNU sed >= 4.0.9 required.

grep -l "int main" *.c | xargs -- sed -i '/^#include/i#include "main.h"'
Show 5 quoted lines
>     - update die() and error() to use that variable when
>       reporting (both callers and implementation) -- this is
>       optional.
> 
> 
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231


diff --git a/Makefile b/Makefile
index 5b0306d..f8e4511 100644
--- a/Makefile
+++ b/Makefile
@@ -147,7 +147,7 @@ LIB_FILE=libgit.a
 LIB_H = \
 	blob.h cache.h commit.h count-delta.h csum-file.h delta.h \
 	diff.h epoch.h object.h pack.h pkt-line.h quote.h refs.h \
-	run-command.h strbuf.h tag.h tree.h
+	run-command.h strbuf.h tag.h tree.h main.h
 
 DIFF_OBJS = \
 	diff.o diffcore-break.o diffcore-order.o diffcore-pathspec.o \
diff --git a/cache.h b/cache.h
index d776016..db5d667 100644
--- a/cache.h
+++ b/cache.h
@@ -45,6 +45,13 @@
 #endif
 #endif
 
+#if defined(__GLIBC__)
+extern const char *__progname;
+#define git_progname __progname
+#else
+extern const char *git_progname;
+#endif
+
 /*
  * Intensive research over the course of many years has shown that
  * port 9418 is totally unused by anything else. Or
diff --git a/daemon.c b/daemon.c
index 0c6182f..c197ee5 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1,3 +1,4 @@
+#include "main.h"
 #include "cache.h"
 #include "pkt-line.h"
 #include <signal.h>
diff --git a/main.h b/main.h
new file mode 100644
index 0000000..472f134
--- /dev/null
+++ b/main.h
@@ -0,0 +1,22 @@
+/* unistd.h must be available and the glibc version includes features.h
+ * from it which #defines __GLIBC__ and friends */
+#include <unistd.h>
+#ifndef __GLIBC__
+const char *git_progname;
+static int git_main(int, char **);
+
+int main(int argc, char **argv)
+{
+	char *p;
+	git_progname = p = *argv;
+
+	/* don't use any library functions. We won't have the headers */
+	while(*p)
+		if(*p++ == '/')
+			git_progname = p;
+
+	return git_main(argc, argv);
+}
+
+#define main(argc, argv) git_main(argc, argv)
+#endif /* __GLIBC__ */
Petr Baudis· Oct 25, 2005, 09:31 UTC · re: Andreas Ericsson · lore

Re: [PATCH] git_progname (was: Re: User-relative paths)

Could you please also trim the mails you are replying to a bit?

Dear diary, on Tue, Oct 25, 2005 at 11:11:54AM CEST, I got a letter where Andreas Ericsson <ae@op5.se> told me that...

Show 5 quoted lines
> See the attached patch, which adds git_progname as a global variable to 
> daemon.c with a minimum of fuzz. The one-liner below will add it to the 
> rest of the programs. GNU sed >= 4.0.9 required.
> 
> grep -l "int main" *.c | xargs -- sed -i '/^#include/i#include "main.h"'

Urgh. Now this is ugly. What about making it a bit more intrusive while quite more saner?

Show 18 quoted lines
> diff --git a/cache.h b/cache.h
> index d776016..db5d667 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -45,6 +45,13 @@
>  #endif
>  #endif
>  
> +#if defined(__GLIBC__)
> +extern const char *__progname;
> +#define git_progname __progname
> +#else
> +extern const char *git_progname;
> +#endif
> +
>  /*
>   * Intensive research over the course of many years has shown that
>   * port 9418 is totally unused by anything else. Or

Also, when you already solve this for non-__GLIBC__ systems, I doubt that there is any win in keeping the __GLIBC__-specific hack, except that most developers won't see any bugs in the generic solution since they are using glibc.

Show 28 quoted lines
> diff --git a/main.h b/main.h
> new file mode 100644
> index 0000000..472f134
> --- /dev/null
> +++ b/main.h
> @@ -0,0 +1,22 @@
> +/* unistd.h must be available and the glibc version includes features.h
> + * from it which #defines __GLIBC__ and friends */
> +#include <unistd.h>
> +#ifndef __GLIBC__
> +const char *git_progname;
> +static int git_main(int, char **);
> +
> +int main(int argc, char **argv)
> +{
> +	char *p;
> +	git_progname = p = *argv;
> +
> +	/* don't use any library functions. We won't have the headers */
> +	while(*p)
> +		if(*p++ == '/')
> +			git_progname = p;
> +
> +	return git_main(argc, argv);
> +}
> +
> +#define main(argc, argv) git_main(argc, argv)
> +#endif /* __GLIBC__ */

Someone said that converting main()s to git_main()s would help the libification effort, but I suspect that you actually want to set the progname to whatever you call when you call its git_main().

We can make it go into the init section, but that won't be too portable either.

So I'd say just add setup_progname("foo") at the start of your main().
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Andreas Ericsson· Oct 25, 2005, 11:12 UTC · re: Petr Baudis · lore

Re: [PATCH] git_progname

Petr Baudis wrote:
> Could you please also trim the mails you are replying to a bit?
> 
Aye.
Show 9 quoted lines
> Dear diary, on Tue, Oct 25, 2005 at 11:11:54AM CEST, I got a letter
> where Andreas Ericsson <ae@op5.se> told me that...
> 
>>grep -l "int main" *.c | xargs -- sed -i '/^#include/i#include "main.h"'
> 
> 
> Urgh. Now this is ugly. What about making it a bit more intrusive while
> quite more saner?
> 

I'm not sure what you're referring to. The one-liner is a one-liner. It's sort of supposed to be ugly.

Including main.h is a fairly sane option for common initialization code.
Show 6 quoted lines
> 
> Also, when you already solve this for non-__GLIBC__ systems, I doubt
> that there is any win in keeping the __GLIBC__-specific hack, except
> that most developers won't see any bugs in the generic solution since
> they are using glibc.
> 
True. I'll rework it if someone thinks it's worth it.
> 
> Someone said that converting main()s to git_main()s would help the
> libification effort,

I had some thoughts along those lines as well, especially for reading configuration files.

> but I suspect that you actually want to set the
> progname to whatever you call when you call its git_main().
> 

This I don't understand. Do you mean "set the progname to whatever you call" as in "set the progname to whatever the program author calls the program" or as in "set the progname to whatever it's called as from the command-line"?

Show 5 quoted lines
> We can make it go into the init section, but that won't be too portable
> either.
> 
> So I'd say just add setup_progname("foo") at the start of your main().
> 
Set the progname to whatever the author calls it then.

If so, I'd put const char *git_progname = GIT_PROGNAME; anywhere in the main() file and expanding the %.o: rule to include -DGIT_PROGNAME=\"git-$*\"

This would bark if there's a program that doesn't have it that's linked against something that uses it, which the setup_progname() approach wouldn't do.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Andreas Ericsson· Oct 25, 2005, 12:53 UTC · re: Andreas Ericsson · lore

Re: [PATCH] git_progname

Andreas Ericsson wrote:
Show 12 quoted lines
> Petr Baudis wrote:
> 
>>> grep -l "int main" *.c | xargs -- sed -i '/^#include/i#include "main.h"'
>>
>>
>> Urgh. Now this is ugly. What about making it a bit more intrusive while
>> quite more saner?
>>
> 
> I'm not sure what you're referring to. The one-liner is a one-liner. 
> It's sort of supposed to be ugly.
> 
Oh. I saw what you meant now. Lots of main.h included. :)
Silly me. Sorry about that.
I guess some manual editing could be done.
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Petr Baudis· Oct 25, 2005, 13:32 UTC · re: Andreas Ericsson · lore

Re: [PATCH] git_progname

Dear diary, on Tue, Oct 25, 2005 at 02:53:14PM CEST, I got a letter where Andreas Ericsson <ae@op5.se> told me that...

Show 9 quoted lines
> Andreas Ericsson wrote:
> >Petr Baudis wrote:
> >>Urgh. Now this is ugly. What about making it a bit more intrusive while
> >>quite more saner?
> >
> >I'm not sure what you're referring to. The one-liner is a one-liner. 
> >It's sort of supposed to be ugly.
> 
> Oh. I saw what you meant now. Lots of main.h included. :)

No, I didn't mean the oneliner at all, actually - just the notion that you stealthily hijack main(). We'll see what Junio thinks about it. ;)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Junio C Hamano· Oct 26, 2005, 06:07 UTC · re: Petr Baudis · lore

Re: [PATCH] git_progname

Petr Baudis <pasky@suse.cz> writes:
> No, I didn't mean the oneliner at all, actually - just the notion that
> you stealthily hijack main(). We'll see what Junio thinks about it. ;)
May I just say "yuck"?
I think what you suggested makes the most sense.
>> So I'd say just add setup_progname("foo") at the start of your main().
Matthias Urlichs· Oct 27, 2005, 08:34 UTC · re: Petr Baudis · lore

Re: [PATCH] git_progname (was: Re: User-relative paths)

Hi, Petr Baudis wrote:
> Someone said that converting main()s to git_main()s would help the
> libification effort,

Wasn't me, and frankly I doubt it. "old-style" programs just call the compatibility interface instead, and "new-style" ones call the libraries' initialization *after* they've parsed any environment vars or flags that are necessary.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
While the difficulties and dangers of problems tend to increase at a
geometric rate, the knowledge and manpower qualified to deal with these
problems tend to increase at an arithmetic rate.
					-- Yehezkel Dror
Linus Torvalds· Oct 23, 2005, 00:42 UTC · re: Andreas Ericsson · lore

Re: Server side programs

On Sun, 23 Oct 2005, Andreas Ericsson wrote:
>
> Are git-receive-pack and git-upload-pack the only two binaries that get called
> directly over a SSH-tunnel?
With the normal pack thing, yes.

They will exec other programs (mainly git-rev-list and git-[un]pack-objects), and a client can ask for other programs (git-send-pack takes an "--exec=" argument, for example), but those two should be sufficient if you have a server-side special "restricted shell" that you want to run instead of a real one.

One more issue: you can't create a new archive or delete an old one (or do administration like repacking, fsck etc) with those interfaces, so if you want these limited users to be able to do that, then you'd need to add a few administration commands too.

		Linus

← back to recent threads