git-cvsserver: implement script based pserver authentication

PATCH In review Áshin László 2 participants 4 messages Jun 15, 2016 1 message not in the archive Documentation

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): 1058578005711783867@unknownmsgid

Áshin László

author

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

PATCH 1/4 git-cvsserver: implement script based pserver authentication

Documentation/git-cvsserver.txt | 46 +++++++++++++++++++++++++++++++++++--- git-cvsserver.perl | 28 +++++++++++++++++++++++ t/t9400-git-cvsserver-server.sh | 45 ++++++++++++++++++++++++++++++++++++++

 3 files changed, 115 insertions(+), 4 deletions(-)

Documentation/git-cvsserver.txt +0 -0

index 7004dd2..b3c3122 100644
@@ -100,10 +100,48 @@ looks like

------

Only anonymous access is provided by pserve by default. To commit you -will have to create pserver accounts, simply add a gitcvs.authdb -setting in the config file of the repositories you want the cvsserver -to allow writes to, for example:
+will have to specify an authentication option in the config file. +Currently there are two options are available for authentication through +pserver in git-cvsserver: one through an authenticator script and an other +through a textual authentication database. If both are specified in the config +file then the script based solution will be used.
+
+ a. To use the authentication script based method, simply add a
+ gitcvs.authscript setting in the config file of the repositories you want
+ the cvsserver to allow writes to, for example:
++

+--
+------

+
+ [gitcvs]
+ authscript = /usr/local/bin/cvsserver-auth.sh
+
+------
+The file specified here must be executable by the user the git-cvsserver runs +under the name of. This script has to read exactly two lines from its standard +input as long as git-cvsserver passes the username and the password on separate +lines. After the script did its task by checking the username and password +given it has to return zero if the authentication was successful and return +other value if it was not.
+
+Here is an example for an authentication script which checks the users against +active directory:
+------
+#!/bin/sh
+# /usr/local/bin/cvsserver-auth.sh

+read username
+read password
+
+wbinfo -a "${username}%${password}"

+------
+--

+
+ b. To use the authentication database based method, simply add a
+ gitcvs.authdb setting in the config file of the repositories you want the
+ cvsserver to allow writes to, for example:
++
+--
------

[gitcvs]
@@ -125,7 +163,7 @@ Alternatively you can produce the password with perl's crypt() operator:
-----
perl -e 'my ($user, $pass) = @ARGV; printf "%s:%s\n", $user, crypt($user, $pass)' $USER password
-----
-
+--
Then provide your password via the pserver method, for example:
------
cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>

git-cvsserver.perl +0 -0

index e9f3037..9664e86 100755
@@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {

}

# Fall through to LOVE
+ } elsif (exists $cfg->{gitcvs}->{authscript}) {
+ my $authscript = $cfg->{gitcvs}->{authscript};
+
+ unless (-x $authscript) {
+ print "E The authentication script specified in ";
+ print "[gitcvs.authscript] cannot be executed\n";
+ print "I HATE YOU\n";
+ exit 1;
+ }
+
+ open my $script_fd, '|-', "'$authscript'"
+ or die "Couldn't open authentication script '$authscript': $!"; +
+ if (length($password) > 0) {
+ $password = descramble($password);
+ }
+
+ print $script_fd "$user\n";
+ print $script_fd "$password\n";
+ close $script_fd;
+
+ unless ($? == 0) {
+ print "E External script authentication failed.\n";
+ print "I HATE YOU\n";
+ exit 1;
+ }
+
+ # Fall through to LOVE
} else {
# Trying to authenticate a user
if (not exists $cfg->{gitcvs}->{authdb}) {

t/t9400-git-cvsserver-server.sh +0 -0

index 8639506..0743e9a 100755
@@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \

# PSERVER AUTHENTICATION
#------------------------

+cat >"$SERVERDIR/authscript.sh" <<EOF
+#!/bin/sh
+read username
+read password
+
+test "x\$username" = xcvsuser -a "x\$password" = xcvspassword +EOF
+
+chmod a+x "$SERVERDIR/authscript.sh"
+
cat >request-anonymous <<EOF
BEGIN AUTH REQUEST
$SERVERDIR
@@ -134,6 +144,41 @@ test_expect_success 'pserver authentication failure (login/non-anonymous user)'
fi &&
sed -ne \$p log | grep "^I HATE YOU\$"'

+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authdb || exit 1 +GIT_DIR="$SERVERDIR" git config gitcvs.authscript "$SERVERDIR/authscript.sh" || exit 1
+
+test_expect_success 'pserver authentication (authscript)' \
+ 'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&
+ sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript, non-anonymous user)' \
+ 'if cat request-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&
+ sed -ne \$p log | grep "^I HATE YOU\$"'
+
+test_expect_success 'pserver authentication success (authscript, non-anonymous user with password)' \
+ 'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&
+ sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication (authscript, login)' \
+ 'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&
+ sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript, login/non-anonymous user)' \
+ 'if cat login-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&
+ sed -ne \$p log | grep "^I HATE YOU\$"'
+
+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authscript || exit 1 +GIT_DIR="$SERVERDIR" git config gitcvs.authdb "$SERVERDIR/auth.db" || exit 1

 # misuse pserver authentication for testing of req_Root

--
1.7.2.rc1.dirty

Ævar Arnfjörð Bjarmason

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

Quoted 12 lines · Áshin László

Documentation/git-cvsserver.txt | 46 +++++++++++++++++++++++++++++++++++--- git-cvsserver.perl | 28 +++++++++++++++++++++++ t/t9400-git-cvsserver-server.sh | 45 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 4 deletions(-)
Documentation/git-cvsserver.txt +0 -0
index 7004dd2..b3c3122 100644
@@ -100,10 +100,48 @@ looks like
------

Quoted 1 line

Only anonymous access is provided by pserve by default. To commit you

I think it's always called "pserver" with an -r.

Documentation/git-cvsserver.txt

Quoted 8 lines

Documentation/git-cvsserver.txt +5 -3

-will have to create pserver accounts, simply add a gitcvs.authdb
-setting in the config file of the repositories you want the cvsserver
-to allow writes to, for example:
+will have to specify an authentication option in the config file.
+Currently there are two options are available for authentication through
+pserver in git-cvsserver: one through an authenticator script and an other
+through a textual authentication database. If both are specified in the config
+file then the script based solution will be used.

If both are specified shouldn't we throw an error?

Documentation/git-cvsserver.txt

Quoted 13 lines

+ a. To use the authentication script based method, simply add a
+ gitcvs.authscript setting in the config file of the repositories you want
+ the cvsserver to allow writes to, for example:
++
+--
+------
Documentation/git-cvsserver.txt +7 -0
+
+   [gitcvs]
+    authscript = /usr/local/bin/cvsserver-auth.sh
+
+------
+The file specified here must be executable by the user the git-cvsserver runs
+under the name of. This script has to read exactly two lines from its standard

"the name of" is redundant here.

Documentation/git-cvsserver.txt

Quoted 4 lines

Documentation/git-cvsserver.txt +4 -0

+input as long as git-cvsserver passes the username and the password on separate
+lines. After the script did its task by checking the username and password
+given it has to return zero if the authentication was successful and return
+other value if it was not.

Better as: "The script will receive two lines on standard input, the first is the username and the second is the password. It should return 0 if the user was successfully authenticated, and a non-zero value if not".

Documentation/git-cvsserver.txt

Quoted 11 lines

Documentation/git-cvsserver.txt +10 -0

+
+Here is an example for an authentication script which checks the users against
+active directory:
+------
+#!/bin/sh
+# /usr/local/bin/cvsserver-auth.sh
 
+read username
+read password
+
+wbinfo -a "${username}%${password}"

Do all POSIX shells implicitly exit with the return value of the last statement they evaluate. I don't know.

Quoted 27 lines

+------
+--
+
+ b. To use the authentication database based method, simply add a
+ gitcvs.authdb setting in the config file of the repositories you want the
+ cvsserver to allow writes to, for example:
++
+--
------
[gitcvs]
Documentation/git-cvsserver.txt +0 -0
@@ -125,7 +163,7 @@ Alternatively you can produce the password with
perl's crypt() operator:
-----
perl -e 'my ($user, $pass) = @ARGV; printf "%s:%s\n", $user, crypt($user, $pass)' $USER password
-----
-
+--
Then provide your password via the pserver method, for example:
------
cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>
git-cvsserver.perl +0 -0
index e9f3037..9664e86 100755

git-cvsserver.perl if ($state->{method} eq 'pserver') { · line 205

Quoted 9 lines

git-cvsserver.perl +6 -0

@@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {
         }
 
         # Fall through to LOVE
+    } elsif (exists $cfg->{gitcvs}->{authscript}) {
+        my $authscript = $cfg->{gitcvs}->{authscript};
+
+        unless (-x $authscript) {
+            print "E The authentication script specified in ";
+            print "[gitcvs.authscript] cannot be executed\n";

I *think* you have to prefix anything that's not "I (HATE|LOVE) YOU" with "E " if it's an error. I.e. this should probably be:

    print "E The authentication script specified in";
    print "E [gitcvs.authscript] cannot be executed\n";

But that's just my hazy memory. Maybe CVS clients will report the error anyway.

Quoted 30 lines

git-cvsserver.perl +22 -0

+            print "I HATE YOU\n";
+            exit 1;
+        }
+
+        open my $script_fd, '|-', "'$authscript'"
+            or die "Couldn't open authentication script '$authscript': $!";
+
+        if (length($password) > 0) {
+            $password = descramble($password);
+        }
+
+        print $script_fd "$user\n";
+        print $script_fd "$password\n";
+        close $script_fd;
+
+        unless ($? == 0) {
+            print "E External script authentication failed.\n";
+            print "I HATE YOU\n";
+            exit 1;
+        }
+
+        # Fall through to LOVE
     } else {
         # Trying to authenticate a user
         if (not exists $cfg->{gitcvs}->{authdb}) {

t/t9400-git-cvsserver-server.sh +0 -0

index 8639506..0743e9a 100755

t/t9400-git-cvsserver-server.sh test_expect_success 'basic checkout' \ · line 72

Quoted 9 lines

t/t9400-git-cvsserver-server.sh +6 -0

@@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \
  # PSERVER AUTHENTICATION
  #------------------------
 
+cat >"$SERVERDIR/authscript.sh" <<EOF
+#!/bin/sh
+read username
+read password
+
+test "x\$username" = xcvsuser -a "x\$password" = xcvspassword

This is offtopic. But I've never figured out why you need to do

    test "x$foo" = "xbar"

As opposed to:

    test "$foo" = "bar"

In shellscript. Are there really some shells that get equality tests where one term is "" wrong?

Quoted 21 lines

t/t9400-git-cvsserver-server.sh +4 -0
+EOF
+
+chmod a+x "$SERVERDIR/authscript.sh"
+
cat >request-anonymous <<EOF
BEGIN AUTH REQUEST
$SERVERDIR
t/t9400-git-cvsserver-server.sh +0 -0
@@ -134,6 +144,41 @@ test_expect_success 'pserver authentication
failure (login/non-anonymous user)'
fi &&
sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +2 -0
+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authdb || exit 1
+GIT_DIR="$SERVERDIR" git config gitcvs.authscript
"$SERVERDIR/authscript.sh" || exit 1
t/t9400-git-cvsserver-server.sh +6 -0
+
+test_expect_success 'pserver authentication (authscript)' \
+  'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&
+   sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript,

Quoted 7 lines

non-anonymous user)' \
+ 'if cat request-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&

This should probably be (untested):

    test_must_fail cat request-git git-cvsserver pserver >log 2>&1 &&

Quoted 24 lines

+ sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +2 -0
+
+test_expect_success 'pserver authentication success (authscript,
non-anonymous user with password)' \
+ 'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&
+ sed -ne \$p log | grep "^I LOVE YOU\$"'
t/t9400-git-cvsserver-server.sh +6 -0
+
+test_expect_success 'pserver authentication (authscript, login)' \
+  'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&
+   sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript,
login/non-anonymous user)' \
+ 'if cat login-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&
+ sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +3 -0
+
+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authscript || exit 1
+GIT_DIR="$SERVERDIR" git config gitcvs.authdb "$SERVERDIR/auth.db" || exit 1

Quoted 1 line

 # misuse pserver authentication for testing of req_Root

Otherwise looking good.

Áshin László

author

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

Hi,

Quoted 13 lines · Ævar Arnfjörð Bjarmason

Quoted 12 lines · Áshin László
Documentation/git-cvsserver.txt | 46 +++++++++++++++++++++++++++++++++++--- git-cvsserver.perl | 28 +++++++++++++++++++++++ t/t9400-git-cvsserver-server.sh | 45 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 4 deletions(-)
Documentation/git-cvsserver.txt +0 -0
index 7004dd2..b3c3122 100644
@@ -100,10 +100,48 @@ looks like
------

Quoted 3 lines

Quoted 1 line
Only anonymous access is provided by pserve by default. To commit you
I think it's always called "pserver" with an -r.

Yes but it is not part of my changeset.

Quoted 10 lines

Documentation/git-cvsserver.txt

Quoted 8 lines

Documentation/git-cvsserver.txt +5 -3

-will have to create pserver accounts, simply add a gitcvs.authdb
-setting in the config file of the repositories you want the cvsserver
-to allow writes to, for example:
+will have to specify an authentication option in the config file.
+Currently there are two options are available for authentication through
+pserver in git-cvsserver: one through an authenticator script and an other
+through a textual authentication database. If both are specified in the config
+file then the script based solution will be used.

If both are specified shouldn't we throw an error?

Actually a check can be added to the code. OK, I will do that.

Quoted 16 lines

Documentation/git-cvsserver.txt
Quoted 13 lines
+ a. To use the authentication script based method, simply add a
+ gitcvs.authscript setting in the config file of the repositories you want
+ the cvsserver to allow writes to, for example:
++
+--
+------
Documentation/git-cvsserver.txt +7 -0
+
+   [gitcvs]
+    authscript = /usr/local/bin/cvsserver-auth.sh
+
+------
+The file specified here must be executable by the user the git-cvsserver runs
+under the name of. This script has to read exactly two lines from its standard
"the name of" is redundant here.

Quoted 9 lines

Documentation/git-cvsserver.txt
Quoted 4 lines
Documentation/git-cvsserver.txt +4 -0
+input as long as git-cvsserver passes the username and the password on separate
+lines. After the script did its task by checking the username and password
+given it has to return zero if the authentication was successful and return
+other value if it was not.
Better as: "The script will receive two lines on standard input, the first is the username and the second is the password. It should return 0 if the user was successfully authenticated, and a non-zero value if not".

OK, a little bit I overcomplicated it :)

Quoted 7 lines

Documentation/git-cvsserver.txt +6 -0

+
+Here is an example for an authentication script which checks the users against
+active directory:
+------
+#!/bin/sh
+# /usr/local/bin/cvsserver-auth.sh

Quoted 7 lines

Documentation/git-cvsserver.txt
Quoted 4 lines
Documentation/git-cvsserver.txt +4 -0
+read username
+read password
+
+wbinfo -a "${username}%${password}"
Do all POSIX shells implicitly exit with the return value of the last statement they evaluate. I don't know.

"The exit status of a sequential list *shall* be the exit status of the last command in the list."
( After registration,
http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html#tag_02_08
)
Generally, all kind of list constructions use this principle according to this page.

Terminology of "shall":
"For an implementation that conforms to IEEE Std 1003.1-2001, describes a feature or behavior that is mandatory. *An application can rely on the existence of the feature or behavior*.

For an application or user, describes a behavior that is mandatory."

Quoted 40 lines

Quoted 27 lines
+------
+--
+
+ b. To use the authentication database based method, simply add a
+ gitcvs.authdb setting in the config file of the repositories you want the
+ cvsserver to allow writes to, for example:
++
+--
------
[gitcvs]
Documentation/git-cvsserver.txt +0 -0
@@ -125,7 +163,7 @@ Alternatively you can produce the password with
perl's crypt() operator:
-----
perl -e 'my ($user, $pass) = @ARGV; printf "%s:%s\n", $user, crypt($user, $pass)' $USER password
-----
-
+--
Then provide your password via the pserver method, for example:
------
cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>
git-cvsserver.perl +0 -0
index e9f3037..9664e86 100755
git-cvsserver.perl if ($state->{method} eq 'pserver') { · line 205
Quoted 9 lines
git-cvsserver.perl +6 -0
@@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {
         }
 
         # Fall through to LOVE
+    } elsif (exists $cfg->{gitcvs}->{authscript}) {
+        my $authscript = $cfg->{gitcvs}->{authscript};
+
+        unless (-x $authscript) {
+            print "E The authentication script specified in ";
+            print "[gitcvs.authscript] cannot be executed\n";
I *think* you have to prefix anything that's not "I (HATE|LOVE) YOU" with "E " if it's an error. I.e. this should probably be:

Quoted 5 lines

print "E The authentication script specified in"; print "E [gitcvs.authscript] cannot be executed\n";
But that's just my hazy memory. Maybe CVS clients will report the error anyway.

Please note that there is no "\n" after the first line. This is only a line break in the source code.
Should I have used the following?

print "E The authentication script specified in" . "E [gitcvs.authscript] cannot be executed\n";

Quoted 42 lines

Quoted 30 lines

git-cvsserver.perl +22 -0

+            print "I HATE YOU\n";
+            exit 1;
+        }
+
+        open my $script_fd, '|-', "'$authscript'"
+            or die "Couldn't open authentication script '$authscript': $!";
+
+        if (length($password) > 0) {
+            $password = descramble($password);
+        }
+
+        print $script_fd "$user\n";
+        print $script_fd "$password\n";
+        close $script_fd;
+
+        unless ($? == 0) {
+            print "E External script authentication failed.\n";
+            print "I HATE YOU\n";
+            exit 1;
+        }
+
+        # Fall through to LOVE
     } else {
         # Trying to authenticate a user
         if (not exists $cfg->{gitcvs}->{authdb}) {

t/t9400-git-cvsserver-server.sh +0 -0

index 8639506..0743e9a 100755

t/t9400-git-cvsserver-server.sh test_expect_success 'basic checkout' \ · line 72

Quoted 9 lines

t/t9400-git-cvsserver-server.sh +6 -0

@@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \
  # PSERVER AUTHENTICATION
  #------------------------
 
+cat >"$SERVERDIR/authscript.sh" <<EOF
+#!/bin/sh
+read username
+read password
+
+test "x\$username" = xcvsuser -a "x\$password" = xcvspassword

This is offtopic. But I've never figured out why you need to do

Quoted 8 lines

test "x$foo" = "xbar"
As opposed to:
test "$foo" = "bar"
In shellscript. Are there really some shells that get equality tests where one term is "" wrong?

Maybe this is too much. Maybe x was used only where " couldn't be:

test x$foo = xbar

So, if $foo is empty we have to make sure that test gets something to both side of the equation mark. But if quotation marks can be used it is possibly unneeded:

    test "$foo" = "bar"

My solution is an overkill in this case. But which of the above ones is preferred then?
I think I can answer my own question. If $foo contains spaces only the quotation mark based version will do what we want. I will remove the x characters.

Quoted 31 lines

Quoted 21 lines
t/t9400-git-cvsserver-server.sh +4 -0
+EOF
+
+chmod a+x "$SERVERDIR/authscript.sh"
+
cat >request-anonymous <<EOF
BEGIN AUTH REQUEST
$SERVERDIR
t/t9400-git-cvsserver-server.sh +0 -0
@@ -134,6 +144,41 @@ test_expect_success 'pserver authentication
failure (login/non-anonymous user)'
fi &&
sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +2 -0
+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authdb || exit 1
+GIT_DIR="$SERVERDIR" git config gitcvs.authscript
"$SERVERDIR/authscript.sh" || exit 1
t/t9400-git-cvsserver-server.sh +6 -0
+
+test_expect_success 'pserver authentication (authscript)' \
+  'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&
+   sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript,
Quoted 7 lines
non-anonymous user)' \
+ 'if cat request-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&
This should probably be (untested):

Quoted 1 line

test_must_fail cat request-git git-cvsserver pserver >log 2>&1 &&

I copied this code from above so this was not my decision. These test cases are all can be find elsewhere in the code, the only difference is in their names and the fact that the script based authentication method is active while they are running.

Quoted 24 lines

Quoted 24 lines
+ sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +2 -0
+
+test_expect_success 'pserver authentication success (authscript,
non-anonymous user with password)' \
+ 'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&
+ sed -ne \$p log | grep "^I LOVE YOU\$"'
t/t9400-git-cvsserver-server.sh +6 -0
+
+test_expect_success 'pserver authentication (authscript, login)' \
+  'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&
+   sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript,
login/non-anonymous user)' \
+ 'if cat login-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&
+ sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +3 -0
+
+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authscript || exit 1
+GIT_DIR="$SERVERDIR" git config gitcvs.authdb "$SERVERDIR/auth.db" || exit 1

Quoted 4 lines

Quoted 1 line
 # misuse pserver authentication for testing of req_Root
Otherwise looking good.

I am not familiar the way that a patch goes in this project. Where can I find the repository my patches will be commited? When will this kind of changes go to mainline?

Thanks,
László ÁSHIN

Ævar Arnfjörð Bjarmason

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

Quoted 16 lines · Áshin László

Hi,
Quoted 13 lines · Ævar Arnfjörð Bjarmason
Quoted 12 lines · Áshin László
Documentation/git-cvsserver.txt | 46 +++++++++++++++++++++++++++++++++++--- git-cvsserver.perl | 28 +++++++++++++++++++++++ t/t9400-git-cvsserver-server.sh | 45 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 4 deletions(-)
Documentation/git-cvsserver.txt +0 -0
index 7004dd2..b3c3122 100644
@@ -100,10 +100,48 @@ looks like
------

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
Only anonymous access is provided by pserve by default. To commit you
I think it's always called "pserver" with an -r.
Yes but it is not part of my changeset.

Sorry, misread the diff.

Quoted 11 lines

Quoted 10 lines

Documentation/git-cvsserver.txt

Quoted 8 lines

Documentation/git-cvsserver.txt +5 -3

-will have to create pserver accounts, simply add a gitcvs.authdb
-setting in the config file of the repositories you want the cvsserver
-to allow writes to, for example:
+will have to specify an authentication option in the config file.
+Currently there are two options are available for authentication through
+pserver in git-cvsserver: one through an authenticator script and an other
+through a textual authentication database. If both are specified in the config
+file then the script based solution will be used.

If both are specified shouldn't we throw an error?

Quoted 1 line

Actually a check can be added to the code. OK, I will do that.

Nice.

Quoted 52 lines

Quoted 16 lines
Documentation/git-cvsserver.txt
Quoted 13 lines
+ a. To use the authentication script based method, simply add a
+ gitcvs.authscript setting in the config file of the repositories you want
+ the cvsserver to allow writes to, for example:
++
+--
+------
Documentation/git-cvsserver.txt +7 -0
+
+   [gitcvs]
+    authscript = /usr/local/bin/cvsserver-auth.sh
+
+------
+The file specified here must be executable by the user the git-cvsserver runs
+under the name of. This script has to read exactly two lines from its standard
"the name of" is redundant here.
Quoted 9 lines
Documentation/git-cvsserver.txt
Quoted 4 lines
Documentation/git-cvsserver.txt +4 -0
+input as long as git-cvsserver passes the username and the password on separate
+lines. After the script did its task by checking the username and password
+given it has to return zero if the authentication was successful and return
+other value if it was not.
Better as: "The script will receive two lines on standard input, the first is the username and the second is the password. It should return 0 if the user was successfully authenticated, and a non-zero value if not".
OK, a little bit I overcomplicated it :)
Quoted 7 lines
Quoted 7 lines
Documentation/git-cvsserver.txt +6 -0
+
+Here is an example for an authentication script which checks the users against
+active directory:
+------
+#!/bin/sh
+# /usr/local/bin/cvsserver-auth.sh
Quoted 7 lines
Documentation/git-cvsserver.txt
Quoted 4 lines
Documentation/git-cvsserver.txt +4 -0
+read username
+read password
+
+wbinfo -a "${username}%${password}"
Do all POSIX shells implicitly exit with the return value of the last statement they evaluate. I don't know.
"The exit status of a sequential list *shall* be the exit status of the last command in the list."
( After registration,
http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html#tag_02_08
)
Generally, all kind of list constructions use this principle according to this page.

Quoted 6 lines

Terminology of "shall":
"For an implementation that conforms to IEEE Std 1003.1-2001, describes a feature or behavior that is mandatory. *An application can rely on the existence of the feature or behavior*.
For an application or user, describes a behavior that is mandatory."

Nice, good to have that cleared up in general :)

Quoted 43 lines

Quoted 43 lines
Quoted 27 lines
+------
+--
+
+ b. To use the authentication database based method, simply add a
+ gitcvs.authdb setting in the config file of the repositories you want the
+ cvsserver to allow writes to, for example:
++
+--
------
[gitcvs]
Documentation/git-cvsserver.txt +0 -0
@@ -125,7 +163,7 @@ Alternatively you can produce the password with
perl's crypt() operator:
-----
perl -e 'my ($user, $pass) = @ARGV; printf "%s:%s\n", $user, crypt($user, $pass)' $USER password
-----
-
+--
Then provide your password via the pserver method, for example:
------
cvs -d:pserver:someuser:somepassword <at> server/path/repo.git co <HEAD_name>
git-cvsserver.perl +0 -0
index e9f3037..9664e86 100755
git-cvsserver.perl if ($state->{method} eq 'pserver') { · line 205
Quoted 9 lines
git-cvsserver.perl +6 -0
@@ -197,6 +197,34 @@ if ($state->{method} eq 'pserver') {
         }
 
         # Fall through to LOVE
+    } elsif (exists $cfg->{gitcvs}->{authscript}) {
+        my $authscript = $cfg->{gitcvs}->{authscript};
+
+        unless (-x $authscript) {
+            print "E The authentication script specified in ";
+            print "[gitcvs.authscript] cannot be executed\n";
I *think* you have to prefix anything that's not "I (HATE|LOVE) YOU" with "E " if it's an error. I.e. this should probably be:
print "E The authentication script specified in"; print "E [gitcvs.authscript] cannot be executed\n";

Quoted 6 lines

Quoted 2 lines
But that's just my hazy memory. Maybe CVS clients will report the error anyway.
Please note that there is no "\n" after the first line. This is only a line break in the source code.
Should I have used the following?

Ah yes, I missed that there wasn't a \n there.

Quoted 2 lines

print "E The authentication script specified in" . "E [gitcvs.authscript] cannot be executed\n";

Probably yeah, at least I managed to misread it :)

Quoted 59 lines

Quoted 42 lines
Quoted 30 lines
git-cvsserver.perl +22 -0
+            print "I HATE YOU\n";
+            exit 1;
+        }
+
+        open my $script_fd, '|-', "'$authscript'"
+            or die "Couldn't open authentication script '$authscript': $!";
+
+        if (length($password) > 0) {
+            $password = descramble($password);
+        }
+
+        print $script_fd "$user\n";
+        print $script_fd "$password\n";
+        close $script_fd;
+
+        unless ($? == 0) {
+            print "E External script authentication failed.\n";
+            print "I HATE YOU\n";
+            exit 1;
+        }
+
+        # Fall through to LOVE
     } else {
         # Trying to authenticate a user
         if (not exists $cfg->{gitcvs}->{authdb}) {
t/t9400-git-cvsserver-server.sh +0 -0
index 8639506..0743e9a 100755
t/t9400-git-cvsserver-server.sh test_expect_success 'basic checkout' \ · line 72
Quoted 9 lines
t/t9400-git-cvsserver-server.sh +6 -0
@@ -64,6 +64,16 @@ test_expect_success 'basic checkout' \
  # PSERVER AUTHENTICATION
  #------------------------
 
+cat >"$SERVERDIR/authscript.sh" <<EOF
+#!/bin/sh
+read username
+read password
+
+test "x\$username" = xcvsuser -a "x\$password" = xcvspassword
This is offtopic. But I've never figured out why you need to do
Quoted 8 lines
test "x$foo" = "xbar"
As opposed to:
test "$foo" = "bar"
In shellscript. Are there really some shells that get equality tests where one term is "" wrong?
Maybe this is too much. Maybe x was used only where " couldn't be:
test x$foo = xbar
So, if $foo is empty we have to make sure that test gets something to both side of the equation mark. But if quotation marks can be used it is possibly unneeded:

Quoted 7 lines

test "$foo" = "bar"
My solution is an overkill in this case. But which of the above ones is preferred then?
I think I can answer my own question. If $foo contains spaces only the quotation mark based version will do what we want. I will remove the x characters.

Ok.

Quoted 31 lines

Quoted 31 lines
Quoted 21 lines
t/t9400-git-cvsserver-server.sh +4 -0
+EOF
+
+chmod a+x "$SERVERDIR/authscript.sh"
+
cat >request-anonymous <<EOF
BEGIN AUTH REQUEST
$SERVERDIR
t/t9400-git-cvsserver-server.sh +0 -0
@@ -134,6 +144,41 @@ test_expect_success 'pserver authentication
failure (login/non-anonymous user)'
fi &&
sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +2 -0
+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authdb || exit 1
+GIT_DIR="$SERVERDIR" git config gitcvs.authscript
"$SERVERDIR/authscript.sh" || exit 1
t/t9400-git-cvsserver-server.sh +6 -0
+
+test_expect_success 'pserver authentication (authscript)' \
+  'cat request-anonymous | git-cvsserver pserver >log 2>&1 &&
+   sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript,
Quoted 7 lines
non-anonymous user)' \
+ 'if cat request-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&
This should probably be (untested):

Quoted 6 lines

Quoted 1 line
test_must_fail cat request-git git-cvsserver pserver >log 2>&1 &&
I copied this code from above so this was not my decision. These test cases are all can be find elsewhere in the code, the only difference is in their names and the fact that the script based authentication method is active while they are running.

Yeah, a lot of the test code is ancient and unoptimal. Just leave it like that I guess.

Quoted 24 lines

Quoted 24 lines
Quoted 24 lines
+ sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +2 -0
+
+test_expect_success 'pserver authentication success (authscript,
non-anonymous user with password)' \
+ 'cat login-git-ok | git-cvsserver pserver >log 2>&1 &&
+ sed -ne \$p log | grep "^I LOVE YOU\$"'
t/t9400-git-cvsserver-server.sh +6 -0
+
+test_expect_success 'pserver authentication (authscript, login)' \
+  'cat login-anonymous | git-cvsserver pserver >log 2>&1 &&
+   sed -ne \$p log | grep "^I LOVE YOU\$"'
+
+test_expect_success 'pserver authentication failure (authscript,
login/non-anonymous user)' \
+ 'if cat login-git | git-cvsserver pserver >log 2>&1
+ then
+ false
+ else
+ true
+ fi &&
+ sed -ne \$p log | grep "^I HATE YOU\$"'
t/t9400-git-cvsserver-server.sh +3 -0
+
+GIT_DIR="$SERVERDIR" git config --unset gitcvs.authscript || exit 1
+GIT_DIR="$SERVERDIR" git config gitcvs.authdb "$SERVERDIR/auth.db" || exit 1

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
 # misuse pserver authentication for testing of req_Root
Otherwise looking good.
I am not familiar the way that a patch goes in this project. Where can I find the repository my patches will be commited? When will this kind of changes go to mainline?

Patches are:

* Discussed on the mailing list (you are here)

* Submitted for inclusion (see Documentation/SubmittingPatches). You didn't CC Junio so it isn't there yet.

* Maybe show up in a "What's cooking" post if they're more complex and warrant further discussion.

* Get merged into git://git.kernel.org/pub/scm/git/git.git, either pu, next, master, maint or some combo of that.