avar@xxxxxxxx (Ævar Arnfjörð Bjarmason) writes: > + unless ($user eq 'anonymous') { > + # Trying to authenticate a user > + if (not exists $cfg->{gitcvs}->{users}) { > + print "E the repo config file needs a [gitcvs.users] section with user/password key-value pairs\n"; > + print "I HATE YOU\n"; > + exit 1; > + } elsif (exists $cfg->{gitcvs}->{users} and not exists $cfg->{gitcvs}->{users}->{$user}) { > + print "E the repo config file has a [gitcvs.users] section but the user $user is not defined in it\n"; > + print "I HATE YOU\n"; > + exit 1; > + } else { > + my $descrambled_password = descramble($password); > + my $cleartext_password = $cfg->{gitcvs}->{users}->{$user}; > + if ($descrambled_password ne $cleartext_password) { > + print "E The password supplied for user $user was incorrect\n"; > + print "I HATE YOU\n"; > + exit 1; > + } I do not know what the real pserver does but by sending these E lines in the latter two different forms back to the client you are leaking sensitive information, which is probably not what you want (the first one is Ok, though. It would help the server administrator to notice misconfiguration, and until it is corrected nobody would be able to log in anyway). Admittedly, the pserver password scrambling is not a real security, but if we were paranoid, we would probably be even adding random delay in "no user found" case and "password does not match" case, so that the client cannot even tell from the response latency if a username exists at the server. > @@ -1176,12 +1196,6 @@ sub req_ci > > $log->info("req_ci : " . ( defined($data) ? $data : "[NULL]" )); > > - if ( $state->{method} eq 'pserver') > - { > - print "error 1 pserver access cannot commit\n"; > - exit; > - } > - Is this correct? You are still allowing anonymous pserver access, so shouldn't you check if this was an anonymous access or authenticated one and refuse access like before for anonymous people? > + my ($str) = @_; > + > + # This should never happen, the same format has been used since > + # CVS was spawned > + $str =~ s/^(.)//; > + die "invalid password format $1" unless $1 eq 'A'; I do not quite understand what "spawn" means in this sentence. - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html