{"thread":{"id":"8484","subject":"[PATCH] cvsserver: Add some useful commandline options","startedAt":"2007-06-07T14:57:00Z","lastAt":"2007-06-14T15:36:55Z","messageCount":3,"participants":["Frank Lichtenheld"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"44248","messageId":"1181228221959-git-send-email-frank@lichtenheld.de","threadId":"8484","inReplyTo":null,"subject":"[PATCH] cvsserver: Make req_Root more critical of its input data","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-07T14:57:00Z","receivedAt":"2007-06-07T14:57:00Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"The path submitted with the Root request has to be absolute\n(cvs does it this way and it may save us some sanity checks\nlater)\n\nIf multiple roots are specified (e.g. because we use\npserver authentication which will already include the\nroot), ensure that they say all the same.\n\nProbably neither is a security risk, and neither should ever\nbe triggered by a sane client, but when validating\ninput data, it's better to be save than sorry.\n\nSigned-off-by: Frank Lichtenheld <frank@lichtenheld.de>\n---\n git-cvsserver.perl              |   11 +++++++++++\n t/t9400-git-cvsserver-server.sh |   34 ++++++++++++++++++++++++++++++++++\n 2 files changed, 45 insertions(+), 0 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 2b4825a..d41b29f 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -167,6 +167,17 @@ sub req_Root\n     my ( $cmd, $data ) = @_;\n     $log->debug(\"req_Root : $data\");\n \n+    unless ($data =~ m#^/#) {\n+\tprint \"error 1 Root must be an absolute pathname\\n\";\n+\treturn 0;\n+    }\n+\n+    if ($state->{CVSROOT}\n+\t&& ($state->{CVSROOT} ne $data)) {\n+\tprint \"error 1 Conflicting roots specified\\n\";\n+\treturn 0;\n+    }\n+\n     $state->{CVSROOT} = $data;\n \n     $ENV{GIT_DIR} = $state->{CVSROOT} . \"/\";\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex e9ef315..41dcf64 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -110,6 +110,40 @@ test_expect_success 'pserver authentication failure (login/non-anonymous user)'\n    tail -n1 log | grep -q \"^I HATE YOU$\"'\n \n \n+# misuse pserver authentication for testing of req_Root\n+\n+cat >request-relative  <<EOF\n+BEGIN AUTH REQUEST\n+gitcvs.git\n+anonymous\n+\n+END AUTH REQUEST\n+EOF\n+\n+cat >request-conflict  <<EOF\n+BEGIN AUTH REQUEST\n+$SERVERDIR\n+anonymous\n+\n+END AUTH REQUEST\n+Root $WORKDIR\n+EOF\n+\n+test_expect_success 'req_Root failure (relative pathname)' \\\n+  'if cat request-relative | git-cvsserver pserver >log 2>&1\n+   then\n+       echo unexpected success\n+       false\n+   else\n+       true\n+   fi &&\n+   tail log | grep -q \"^error 1 Root must be an absolute pathname$\"'\n+\n+test_expect_success 'req_Root failure (conflicting roots)' \\\n+  'cat request-conflict | git-cvsserver pserver >log 2>&1 &&\n+   tail log | grep -q \"^error 1 Conflicting roots specified$\"'\n+\n+\n #--------------\n # CONFIG TESTS\n #--------------\n-- \n1.5.2.1\n"},{"id":"44247","messageId":"11812282222271-git-send-email-frank@lichtenheld.de","threadId":"8484","inReplyTo":"1181228221959-git-send-email-frank@lichtenheld.de","subject":"[PATCH] cvsserver: Add some useful commandline options","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-07T14:57:01Z","receivedAt":"2007-06-07T14:57:01Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"Make git-cvsserver understand some options inspired by\ngit-daemon, namely --base-path, --export-all, --strict-paths.\n\nAlso allow the caller to specify a whitelist of allowed\ndirectories, again similar to git-daemon.\n\nWhile already adding option parsing also support the common\n--help and --version options.\n\nRationale:\n\nWhile the gitcvs.enabled configuration option already\noffers means to limit git-cvsserver access to a repository,\nthere are some use cases where other methods of access\ncontrol prove to be more useful.\n\nE.g. if setting up a pserver for a collection of public\nrepositories one might want limit the exported repositories\nto exactly the directory this collection is located whithout\nhaving to worry about other repositories that might lie around\nwith the configuration variable set (never trust your users ;)\n\nSigned-off-by: Frank Lichtenheld <frank@lichtenheld.de>\n---\n Documentation/git-cvsserver.txt |   42 ++++++++++++++++++++\n git-cvsserver.perl              |   79 ++++++++++++++++++++++++++++++++++++---\n t/t9400-git-cvsserver-server.sh |   28 ++++++++++++++\n 3 files changed, 143 insertions(+), 6 deletions(-)\n\n --interpolated-path can't be supported because the information isn't\n available in CVS protocoll.\n\n --user-path could be supported, I just hadn't motivation/time enough to\n do that.\n\n The documentation part of the patch could probably use some improvement\n still.\n\ndiff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\nindex e5005f0..6d1e311 100644\n--- a/Documentation/git-cvsserver.txt\n+++ b/Documentation/git-cvsserver.txt\n@@ -7,10 +7,52 @@ git-cvsserver - A CVS server emulator for git\n \n SYNOPSIS\n --------\n+\n+SSH:\n+\n [verse]\n export CVS_SERVER=git-cvsserver\n 'cvs' -d :ext:user@server/path/repo.git co <HEAD_name>\n \n+pserver (/etc/inetd.conf):\n+\n+[verse]\n+cvspserver stream tcp nowait nobody /usr/bin/git-cvsserver git-cvsserver pserver\n+\n+Usage:\n+\n+[verse]\n+'git-cvsserver' [options] [pserver|server] [<directory> ...]\n+\n+OPTIONS\n+-------\n+\n+All these options obviously only make sense if enforced by the server side.\n+They have been implemented to resemble the gitlink:git-daemon[1] options as\n+closely as possible.\n+\n+--base-path <path>::\n+Prepend 'path' to requested CVSROOT\n+\n+--strict-paths::\n+Don't allow recursing into subdirectories\n+\n+--export-all::\n+Don't check for `gitcvs.enabled` in config\n+\n+--version, -V::\n+Print version information and exit\n+\n+--help, -h, -H::\n+Print usage information and exit\n+\n+<directory>::\n+You can specify a list of allowed directories. If no directories\n+are given, all are allowed. This is an additional restriction, gitcvs\n+access still needs to be enabled by the `gitcvs.enabled` config option\n+unless '--export-all' was given, too.\n+\n+\n DESCRIPTION\n -----------\n \ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex d41b29f..9fbd9db 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -22,6 +22,9 @@ use bytes;\n use Fcntl;\n use File::Temp qw/tempdir tempfile/;\n use File::Basename;\n+use Getopt::Long qw(:config require_order no_ignore_case);\n+\n+my $VERSION = '@@GIT_VERSION@@';\n \n my $log = GITCVS::log->new();\n my $cfg;\n@@ -85,15 +88,52 @@ my $methods = {\n my $state = { prependdir => '' };\n $log->info(\"--------------- STARTING -----------------\");\n \n+my $usage =\n+    \"Usage: git-cvsserver [options] [pserver|server] [<directory> ...]\\n\".\n+    \"    --base-path <path>  : Prepend to requested CVSROOT\\n\".\n+    \"    --strict-paths      : Don't allow recursing into subdirectories\\n\".\n+    \"    --export-all        : Don't check for gitcvs.enabled in config\\n\".\n+    \"    --version, -V       : Print version information and exit\\n\".\n+    \"    --help, -h, -H      : Print usage information and exit\\n\".\n+    \"\\n\".\n+    \"<directory> ... is a list of allowed directories. If no directories\\n\".\n+    \"are given, all are allowed. This is an additional restriction, gitcvs\\n\".\n+    \"access still needs to be enabled by the gitcvs.enabled config option.\\n\";\n+\n+my @opts = ( 'help|h|H', 'version|V',\n+\t     'base-path=s', 'strict-paths', 'export-all' );\n+GetOptions( $state, @opts )\n+    or die $usage;\n+\n+if ($state->{version}) {\n+    print \"git-cvsserver version $VERSION\\n\";\n+    exit;\n+}\n+if ($state->{help}) {\n+    print $usage;\n+    exit;\n+}\n+\n my $TEMP_DIR = tempdir( CLEANUP => 1 );\n $log->debug(\"Temporary directory is '$TEMP_DIR'\");\n \n+$state->{method} = 'ext';\n+if (@ARGV) {\n+    if ($ARGV[0] eq 'pserver') {\n+\t$state->{method} = 'pserver';\n+\tshift @ARGV;\n+    } elsif ($ARGV[0] eq 'server') {\n+\tshift @ARGV;\n+    }\n+}\n+\n+# everything else is a directory\n+$state->{allowed_roots} = [ @ARGV ];\n+\n # if we are called with a pserver argument,\n # deal with the authentication cat before entering the\n # main loop\n-$state->{method} = 'ext';\n-if (@ARGV && $ARGV[0] eq 'pserver') {\n-    $state->{method} = 'pserver';\n+if ($state->{method} eq 'pserver') {\n     my $line = <STDIN>; chomp $line;\n     unless( $line =~ /^BEGIN (AUTH|VERIFICATION) REQUEST$/) {\n        die \"E Do not understand $line - expecting BEGIN AUTH REQUEST\\n\";\n@@ -178,13 +218,40 @@ sub req_Root\n \treturn 0;\n     }\n \n-    $state->{CVSROOT} = $data;\n+    $state->{CVSROOT} = $state->{'base-path'} || '';\n+    $state->{CVSROOT} =~ s#/+$##;\n+    $state->{CVSROOT} .= $data;\n \n     $ENV{GIT_DIR} = $state->{CVSROOT} . \"/\";\n+\n+    if (@{$state->{allowed_roots}}) {\n+\tmy $allowed = 0;\n+\tforeach my $dir (@{$state->{allowed_roots}}) {\n+\t    next unless $dir =~ m#^/#;\n+\t    $dir =~ s#/+$##;\n+\t    if ($state->{'strict-paths'}) {\n+\t\tif ($ENV{GIT_DIR} =~ m#^\\Q$dir\\E/?$#) {\n+\t\t    $allowed = 1;\n+\t\t    last;\n+\t\t}\n+\t    } elsif ($ENV{GIT_DIR} =~ m#^\\Q$dir\\E(/?$|/)#) {\n+\t\t$allowed = 1;\n+\t\tlast;\n+\t    }\n+\t}\n+\n+\tunless ($allowed) {\n+\t    print \"E $ENV{GIT_DIR} does not seem to be a valid GIT repository\\n\";\n+\t    print \"E \\n\";\n+\t    print \"error 1 $ENV{GIT_DIR} is not a valid repository\\n\";\n+\t    return 0;\n+\t}\n+    }\n+\n     unless (-d $ENV{GIT_DIR} && -e $ENV{GIT_DIR}.'HEAD') {\n        print \"E $ENV{GIT_DIR} does not seem to be a valid GIT repository\\n\";\n-        print \"E \\n\";\n-        print \"error 1 $ENV{GIT_DIR} is not a valid repository\\n\";\n+       print \"E \\n\";\n+       print \"error 1 $ENV{GIT_DIR} is not a valid repository\\n\";\n        return 0;\n     }\n \ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 41dcf64..392f890 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -143,6 +143,34 @@ test_expect_success 'req_Root failure (conflicting roots)' \\\n   'cat request-conflict | git-cvsserver pserver >log 2>&1 &&\n    tail log | grep -q \"^error 1 Conflicting roots specified$\"'\n \n+test_expect_success 'req_Root (strict paths)' \\\n+  'cat request-anonymous | git-cvsserver --strict-paths pserver $SERVERDIR >log 2>&1 &&\n+   tail -n1 log | grep -q \"^I LOVE YOU$\"'\n+\n+test_expect_failure 'req_Root failure (strict-paths)' \\\n+  'cat request-anonymous | git-cvsserver --strict-paths pserver $WORKDIR >log 2>&1'\n+\n+test_expect_success 'req_Root (w/o strict-paths)' \\\n+  'cat request-anonymous | git-cvsserver pserver $WORKDIR/ >log 2>&1 &&\n+   tail -n1 log | grep -q \"^I LOVE YOU$\"'\n+\n+test_expect_failure 'req_Root failure (w/o strict-paths)' \\\n+  'cat request-anonymous | git-cvsserver pserver $WORKDIR/gitcvs >log 2>&1'\n+\n+cat >request-base  <<EOF\n+BEGIN AUTH REQUEST\n+/gitcvs.git\n+anonymous\n+\n+END AUTH REQUEST\n+EOF\n+\n+test_expect_success 'req_Root (base-path)' \\\n+  'cat request-base | git-cvsserver --strict-paths --base-path $WORKDIR/ pserver $SERVERDIR >log 2>&1 &&\n+   tail -n1 log | grep -q \"^I LOVE YOU$\"'\n+\n+test_expect_failure 'req_Root failure (base-path)' \\\n+  'cat request-anonymous | git-cvsserver --strict-paths --base-path $WORKDIR pserver $SERVERDIR >log 2>&1'\n \n #--------------\n # CONFIG TESTS\n-- \n1.5.2.1\n"},{"id":"45081","messageId":"20070614153655.GA457@planck.djpig.de","threadId":"8484","inReplyTo":"11812282222271-git-send-email-frank@lichtenheld.de","subject":"Re: [PATCH] cvsserver: Add some useful commandline options","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2007-06-14T15:36:55Z","receivedAt":"2007-06-14T15:36:55Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Thu, Jun 07, 2007 at 04:57:01PM +0200, Frank Lichtenheld wrote:\n> Make git-cvsserver understand some options inspired by\n> git-daemon, namely --base-path, --export-all, --strict-paths.\n\nNote that this patch doesn't actually implement --export-all ...\nWhich I would have caught if I implemented a test for it, which I also\ndidn't do...\n\nWill send a fix somewhen in the next days.\n\nGruesse,\n-- \nFrank Lichtenheld <frank@lichtenheld.de>\nhttp://www.djpig.de/\n"}]}