From 72b63abaf2407e0f566c1872d82706c7aac64c96 Mon Sep 17 00:00:00 2001
From: Sitaram Chamarty <sitaram@atc.tcs.com>
Date: Sun, 28 Mar 2010 23:44:37 +0530
Subject: [PATCH] auth, gitolite.pm: do not leak info about repo existence

All this is about a user trying to look if a repo exists or not, when he
does not have any access to that repo.  Ideally, "repo does not exist"
should be indistinguishable from "you dont have perms to that repo".

(1) if $GL_WILDREPOS is not set, you either get a permissions error, or
    a "$repo not found in compiled config" death.  Fixed.

(2) if $GL_WILDREPOS is set, you either get either a permissions error,
    or a "$repo has no matches" death.  Fixed.

(3) The following combination leaks info about repo existence:

      - actual repo doesn't exist
      - spying user don't have C perms
      - repo patt doesn't contain CREATER
      - RW+ = CREATER is specified (as is normal)

    In such case, the "convenience copy" of the ACL that parse_acl
    makes, coupled with substituting CREATER for the invoking user means
    $repos{$actual_repo} has RW+ for the spying user.  This means the
    access denied doesn't happen, and control passes to git, which
    promptly expresses it unhappiness and angst over being given a repo
    that 'does not appear to be a git repository'

    This doesn't happen if all those conditions are not met:

      - if repo exists, CREATER is set to the real creater, so RW+ =
        CREATER does not gain spying user anything
      - if spying user has C perms it just gets created, because he has
        rights.  This is also info leak but we can't prevent it; tighten
        the config (maybe by including CREATER in repo pattern) if this
        is not wanted
      - if repo patt contains CREATER it will never match someone else's
        repo anyway!
---
 src/gitolite.pm     | 18 ++++++++++++------
 src/gl-auth-command |  4 ++++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/gitolite.pm b/src/gitolite.pm
index e398b09..9850af8 100644
--- a/src/gitolite.pm
+++ b/src/gitolite.pm
@@ -253,15 +253,21 @@ sub parse_acl
     # want the config dumped as is, really
     return unless $repo;
 
-    return $ENV{GL_REPOPATT} = "" if $repos{$repo};
-    # didn't find it, but wild is off?  too bad, die!!! muahahaha
-    die "$repo not found in compiled config\n" unless $GL_WILDREPOS;
+    # return with "no wildcard match" status if you found the actual repo in
+    # the config or if wild is unset
+    return $ENV{GL_REPOPATT} = "" if $repos{$repo} or not $GL_WILDREPOS;
 
-    # didn't find $repo in %repos, so it must be a wildcard-match case
+    # didn't find actual repo in %repos, and wild is set, so find the repo
+    # pattern that matches the actual repo
     my @matched = grep { $repo =~ /^$_$/ } sort keys %repos;
-    die "$repo has no matches\n" unless @matched;
+
+    # didn't find a match?  avoid leaking info to user about repo existence;
+    # as before, pretend "no wildcard match" status
+    return $ENV{GL_REPOPATT} = "" unless @matched;
+
     die "$repo has multiple matches\n@matched\n" if @matched > 1;
-    # found exactly one pattern that matched, copy its ACL
+
+    # found exactly one pattern that matched, copy its ACL for convenience
     $repos{$repo} = $repos{$matched[0]};
     # and return the pattern
     return $ENV{GL_REPOPATT} = $matched[0];
diff --git a/src/gl-auth-command b/src/gl-auth-command
index 0d618e9..583cd4e 100755
--- a/src/gl-auth-command
+++ b/src/gl-auth-command
@@ -180,6 +180,10 @@ if ( -d "$repo_base_abs/$repo.git" ) {
         wrap_chdir("$repo_base_abs");
         new_repo($repo, "$GL_ADMINDIR/hooks/common", $user);
         wrap_chdir($ENV{HOME});
+    } else {
+        # repo didn't exist, and you didn't have perms to create it.  Delete
+        # the "convenience" copy of the ACL that parse_acl makes for us
+        delete $repos{$repo};
     }
 }