From 5e3a051a95ef81d2e60839b311684af06b3be323 Mon Sep 17 00:00:00 2001 From: Sitaram Chamarty Date: Sun, 9 Oct 2011 19:19:35 +0530 Subject: [PATCH] "deny" rules for entire repo - strictly speaking, this should be phrased: "deny" rules for the first level access check - requires a gitolite option to be set, like so: config gitolite-options.deny-repo = 1 --- contrib/gerrit.mkd | 6 -- doc/3-faq-tips-etc.mkd | 5 +- doc/gitolite.conf.mkd | 74 ++++++++++++-- doc/progit-article.mkd | 12 +++ src/gitolite.pm | 36 +++++-- t/t10-all-yall | 31 ++++++ t/t11-deny-repo | 227 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 367 insertions(+), 24 deletions(-) create mode 100644 t/t11-deny-repo diff --git a/contrib/gerrit.mkd b/contrib/gerrit.mkd index d68c007..3d786d6 100644 --- a/contrib/gerrit.mkd +++ b/contrib/gerrit.mkd @@ -72,12 +72,6 @@ review stuff :) matter; they're probably equivalent except perhaps in some far-fetched scenarios. - * One big difference is that gitolite does not process "deny" rules ("-1 no - Access" in gerrit terms) for *read* access -- we only support those for - write access. Gerrit uses this to "hide a handful of projects on an - otherwise public server"; in gitolite you'd better avoid giving `R = @all` - in the first place :) - * Update 2010-10-24: as per [this][gitlog1] Gerrit now has *read* access control at the branch level -- they can afford to do that because they have a full jgit stack to play with. Even then it was not easy -- they diff --git a/doc/3-faq-tips-etc.mkd b/doc/3-faq-tips-etc.mkd index 0325ca5..4f69563 100644 --- a/doc/3-faq-tips-etc.mkd +++ b/doc/3-faq-tips-etc.mkd @@ -168,8 +168,9 @@ Note that at this point no git program has entered the picture, and we have no way of knowing what **ref** (branch, tag, etc) he is trying to update, even if it is a "write" operation. -For a "read" operation to pass this check, the username (or `@all`) must have -read permission (i.e., R, RW, or RW+) on at least one branch of the repo. +For a "read" operation to pass this check, the username (or `@all` users) must +have read permission (i.e., R, RW, RW+, etc.) on at least one branch of the +repo (or `@all` repos). For a "write" operation, there is an additional restriction: lines specifying only `R` (read access) don't count. *The user must have write access to diff --git a/doc/gitolite.conf.mkd b/doc/gitolite.conf.mkd index e1b3d61..4245d2e 100644 --- a/doc/gitolite.conf.mkd +++ b/doc/gitolite.conf.mkd @@ -13,7 +13,9 @@ In this document: * advanced access control * creating and deleting branches * "deny" rules - * ***IMPORTANT NOTES ABOUT "DENY" RULES***: + * warnings and required reading + * "deny" rules for refs in a repo + * "deny" rules for the entire repo * summary: permissions * virtual "ref"-types * other tips @@ -198,13 +200,14 @@ looks like "refs/heads/foo", while a tag ref looks like "refs/tags/bar") * a **refex** is a *perl regex* that matches a ref. When you try to push a commit to a branch or a tag, that "ref" is matched against the refex part - of each rule + of each rule. * if the refex does not start with `refs/`, gitolite assumes a prefix of `refs/heads/`. This is useful because *branch* matching is the most common case, as you can see this applies to lines 1 through 5 here. - * if no refex appears, the rule applies to all refs in that repo + * if no refex appears, the rule applies to all refs in that repo (as if you + had specified `refs/.*` as the refex). * refexes are prefix-matched (they are internally anchored with `^` before being used). This means only the beginning of the actual ref needs to @@ -330,6 +333,30 @@ Some usage hints: #### "deny" rules + + +##### warnings and required reading + +Gitolite performs access checks at 2 levels. The first check is performed for +both read *and* write operations, while the second one happens only for write +operations. + +**Required reading**: [this section][two_l] of the documentation. + +[two_l]: http://sitaramc.github.com/gitolite/doc/3-faq-tips-etc.html#_two_levels_of_access_rights_checking + +**Warning**: When using deny rules, the order of your rules matters, where +earlier it did not. If you're just starting to add a deny rule to an existing +ruleset, it's a good idea to review the entire ruleset once, to make sure +you're doing it right. + + + +##### "deny" rules for refs in a repo + +You can use "deny" rules for the second check, to prevent people pushing +branches or tags that they should not be allowed to. + Take a look at the following snippet, which *seems* to say that "bruce" can write versioned tags (anything containing `refs/tags/v[0-9]`), but the other staffers can't: @@ -376,16 +403,43 @@ And here's how it works: before the third one, and it has a `-` as the permission, so the push fails - + -##### ***IMPORTANT NOTES ABOUT "DENY" RULES***: +##### "deny" rules for the entire repo - * deny rules do NOT affect read access. They only apply to write access. +The previous section described deny rules for the second check, which is a +fairly common need. However, gitolite does not process deny rules for the +first check -- it's usually simple enough to make sure your config file does +not allow the particular user/repo combindation at all. - * when using deny rules, the order of your rules starts to matter, where - earlier it did not. If you're just starting to add a deny rule to an - existing ruleset, it's a good idea to review the entire ruleset once, to - make sure you're doing it right. +But there's one case where this becomes cumbersome: when you want to create +exceptions to uses of `@all`. + +For example, if you want gitweb to show `@all` repos except the special +'gitolite-admin' repo, you must manually (and laboriously) maintain a list of +all your repos (except gitolite-admin) and use that in place of `@all`. Oh +joy... + +But now you can do this: + + repo gitolite-admin + - = gitweb daemon + [... other access rules ...] + config gitolite-options.deny-repo = 1 + + repo @all + R = gitweb daemon + +Here are some notes on how/why this works: + + * The order is important -- the deny rule must come first. + + * The 'config' line is what tells git to behave differently, i.e., apply + deny rules for the first check also. + + * Since there is no "ref" to match against the refexes in the rules, + gitolite just ignores the refexes, and simply looks at the permission (R, + RW, "-", etc) and the user list. diff --git a/doc/progit-article.mkd b/doc/progit-article.mkd index 338143d..99f0eed 100644 --- a/doc/progit-article.mkd +++ b/doc/progit-article.mkd @@ -118,6 +118,18 @@ Let us say, in the situation above, we want engineers to be able to rewind any b Again, you simply follow the rules top down until you hit a match for your access mode, or a deny. Non-rewind push to master or integ is allowed by the first rule. A rewind push to those refs does not match the first rule, drops down to the second, and is therefore denied. Any push (rewind or non-rewind) to refs other than master or integ won't match the first two rules anyway, and the third rule allows it. +You can also use deny rules to hide specific repos from people (or gitweb, or git-daemon, etc.), when you have otherwise allowed them access to *all* repos. For example, a server containing open source repos may nevertheless wish to hide the special 'gitolite-admin' repo from gitweb, even though all the other repos can be made visible: + + repo gitolite-admin + - = gitweb daemon + [... other access rules ...] + config gitolite-options.deny-repo = 1 + + repo @all + R = gitweb daemon + +See the documentation for more on this. + ### Restricting pushes by files changed ### diff --git a/src/gitolite.pm b/src/gitolite.pm index afabbda..c4e13f5 100644 --- a/src/gitolite.pm +++ b/src/gitolite.pm @@ -227,7 +227,8 @@ sub check_ref { for my $ar (@allowed_refs) { my $refex = $ar->[1]; # refex? sure -- a regex to match a ref against :) - next unless $ref =~ /^$refex/; + next unless $ref =~ /^$refex/ or $ref eq 'joker'; + # joker matches any refex; it will only ever be sent internally return "DENIED by $refex" if $ar->[2] eq '-' and $dry_run; die "$perm $ref $ENV{GL_USER} DENIED by $refex\n" if $ar->[2] eq '-'; @@ -613,7 +614,7 @@ sub report_basic my $perm .= ( $repos{$r}{C}{'@all'} ? ' @C' : ( $repos{$r}{C}{$user} ? ' C' : ' ' ) ); $perm .= perm_code( $repos{$r}{R}{'@all'} || $repos{'@all'}{R}{'@all'}, $repos{'@all'}{R}{$user}, $repos{$r}{R}{$user}, 'R'); $perm .= perm_code( $repos{$r}{W}{'@all'} || $repos{'@all'}{W}{'@all'}, $repos{'@all'}{W}{$user}, $repos{$r}{W}{$user}, 'W'); - print "$perm\t$r\r\n" if $perm =~ /\S/; + print "$perm\t$r\r\n" if $perm =~ /\S/ and not check_deny_repo($r); } print "only $BIG_INFO_CAP out of $count candidate repos examined\r\nplease use a partial reponame or regex pattern to limit output\r\n" if $GL_BIG_CONFIG and $count > $BIG_INFO_CAP; print "$GL_SITE_INFO\n" if $GL_SITE_INFO; @@ -815,6 +816,7 @@ sub add_repo_conf } $perm .= perm_code( $repos{$repo}{R}{'@all'} || $repos{'@all'}{R}{'@all'}, $repos{'@all'}{R}{$ENV{GL_USER}}, $repos{$repo}{R}{$ENV{GL_USER}}, 'R' ); $perm .= perm_code( $repos{$repo}{W}{'@all'} || $repos{'@all'}{W}{'@all'}, $repos{'@all'}{W}{$ENV{GL_USER}}, $repos{$repo}{W}{$ENV{GL_USER}}, 'W' ); + $perm =~ s/./ /g if check_deny_repo($repo); # set up for caching %repos $last_repo = $repo; @@ -855,6 +857,19 @@ sub check_repo_write_enabled { } } +sub check_deny_repo { + my $repo = shift; + + return 0 unless check_config_key($repo, "gitolite-options.deny-repo"); + # there are no 'gitolite-options.deny-repo' keys + + # the 'joker' ref matches any refex. Think of it like a ".*" in reverse. + # A pattern of ".*" matches any string. Similarly a string called 'joker' + # matches any pattern :-) See check_ref() for implementation. + return 1 if ( check_access($repo, 'joker', 'R', 1) ) =~ /DENIED by/; + return 0; +} + sub check_config_key { my($repo, $key) = @_; my @ret = (); @@ -960,10 +975,19 @@ sub check_access my ($repo, $ref, $aa, $dry_run) = @_; # aa = attempted access - my ($perm, $creator, $wild) = repo_rights($repo); - $perm =~ s/ /_/g; - $creator =~ s/^\(|\)$//g; - return ($perm, $creator) unless $ref; + my ($perm, $creator, $wild); + unless ($ref) { + ($perm, $creator, $wild) = repo_rights($repo); + $perm =~ s/ /_/g; + $creator =~ s/^\(|\)$//g; + return ($perm, $creator); + } + + ($perm, $creator, $wild) = repo_rights($repo) unless $ref eq 'joker'; + # calling it when ref eq joker is infinitely recursive! check_access + # will only be called with ref eq joker only when repo_rights has + # already been called and %repos populated already. (See comments + # elsewhere for what 'joker' is and why it is called that). # until I do some major refactoring (which will bloat the update hook a # bit, sadly), this code duplicates stuff in the current update hook. diff --git a/t/t10-all-yall b/t/t10-all-yall index 74289b6..805278a 100644 --- a/t/t10-all-yall +++ b/t/t10-all-yall @@ -1,4 +1,24 @@ # vim: syn=sh: +has_export_ok() { + runremote ls -al $TEST_BASE_FULL/$1.git/git-daemon-export-ok + expect "gitolite-test gitolite-test .* $TEST_BASE_FULL/$1.git/git-daemon-export-ok" +} + +does_not_have_export_ok() { + runremote ls -al $TEST_BASE_FULL/$1.git/git-daemon-export-ok + expect "ls: cannot access $TEST_BASE_FULL/$1.git/git-daemon-export-ok: No such file or directory" +} + +is_in_projects_list() { + runremote cat projects.list + expect "^$1.git$" +} + +is_not_in_projects_list() { + runremote cat projects.list + notexpect "^$1.git$" +} + for bc in 0 1 do for ais in 0 1 @@ -56,5 +76,16 @@ do runlocal git ls-remote u6:dev/wild1 expect refs/heads/wild1 + name "gitweb and daemon" + + for REPO in foo bar dev/wild1 + do + [ "$ais" = "0" ] && does_not_have_export_ok $REPO + [ "$ais" = "0" ] && is_not_in_projects_list $REPO + + [ "$ais" = "1" ] && has_export_ok $REPO + [ "$ais" = "1" ] && is_in_projects_list $REPO + done + done done diff --git a/t/t11-deny-repo b/t/t11-deny-repo new file mode 100644 index 0000000..673a80d --- /dev/null +++ b/t/t11-deny-repo @@ -0,0 +1,227 @@ +# vim: syn=sh: +# can_read cannot_read has_export_ok is_in_projects_list +# can_push cannot_push does_not_have_export_ok is_not_in_projects_list + +can_read() { + # args: user, repo + runlocal git ls-remote $1:$2 + expect refs/heads + notexpect DENIED +} + +can_push() { + cd ~/td + rm -rf clone + runlocal git clone $1:$2 clone + expect Cloning into + notexpect DENIED + notexpect fatal + cd clone + mdc + runlocal git push origin HEAD:${3:-master} + expect_push_ok "HEAD -> ${3:-master}" +} + +cannot_read() { + # args: user, repo + runlocal git ls-remote $1:$2 + notexpect refs/heads + expect DENIED +} + +cannot_push() { + cd ~/td + rm -rf clone + runlocal git clone $1:$2 clone + expect Cloning into + notexpect DENIED + notexpect fatal + cd clone + mdc + runlocal git push origin HEAD:${3:-master} + expect DENIED +} + +has_export_ok() { + runremote ls -al $TEST_BASE_FULL/$1.git/git-daemon-export-ok + expect "gitolite-test gitolite-test .* $TEST_BASE_FULL/$1.git/git-daemon-export-ok" +} + +does_not_have_export_ok() { + runremote ls -al $TEST_BASE_FULL/$1.git/git-daemon-export-ok + expect "ls: cannot access $TEST_BASE_FULL/$1.git/git-daemon-export-ok: No such file or directory" +} + +is_in_projects_list() { + runremote cat projects.list + expect "^$1.git$" +} + +is_not_in_projects_list() { + runremote cat projects.list + notexpect "^$1.git$" +} + +for bc in 0 1 +do + for ais in 0 1 + do + cd $TESTDIR + $TESTDIR/rollback || die "rollback failed" + editrc GL_WILDREPOS 1 + editrc GL_BIG_CONFIG $bc + echo "\$GL_ALL_INCLUDES_SPECIAL = $ais;" | addrc + + name "set 1" + REPO=one + echo " + repo $REPO + RW+ = u1 + R = u2 + - = u2 u3 + R = @all + + " | ugc + + can_push u1 $REPO + + can_read u2 $REPO + cannot_push u2 $REPO + + can_read u3 $REPO + cannot_push u3 $REPO + + can_read u6 $REPO + cannot_push u6 $REPO + + [ "$ais" = "0" ] && does_not_have_export_ok $REPO + [ "$ais" = "0" ] && is_not_in_projects_list $REPO + + [ "$ais" = "1" ] && has_export_ok $REPO + [ "$ais" = "1" ] && is_in_projects_list $REPO + + name "set 1a -- add the deny-repo flag" + echo " + config gitolite-options.deny-repo = 1 + " | ugc + + can_push u1 $REPO + + can_read u2 $REPO + cannot_push u2 $REPO + + cannot_read u3 $REPO + + can_read u6 $REPO + cannot_push u6 $REPO + + [ "$ais" = "0" ] && does_not_have_export_ok $REPO + [ "$ais" = "0" ] && is_not_in_projects_list $REPO + + [ "$ais" = "1" ] && has_export_ok $REPO + [ "$ais" = "1" ] && is_in_projects_list $REPO + + name "set 2 -- add gitweb and daemon" + REPO=two + echo " + repo $REPO + RW+ = u1 + R = u2 + - = u2 u3 gitweb daemon + R = @all + + " | ugc + + [ "$ais" = "0" ] && does_not_have_export_ok $REPO + [ "$ais" = "0" ] && is_not_in_projects_list $REPO + + [ "$ais" = "1" ] && has_export_ok $REPO + [ "$ais" = "1" ] && is_in_projects_list $REPO + + name "set 2a -- add the deny-repo flag" + echo " + config gitolite-options.deny-repo = 1 + " | ugc + + does_not_have_export_ok $REPO + is_not_in_projects_list $REPO + + name "set 3 -- allow gitweb to all but admin repo" + REPO=three + echo " + repo gitolite-admin + - = gitweb daemon + config gitolite-options.deny-repo = 1 + + repo $REPO + RW+ = u3 + R = gitweb daemon + + " | ugc + + has_export_ok $REPO + is_in_projects_list $REPO + does_not_have_export_ok gitolite-admin + is_not_in_projects_list gitolite-admin + + name "set 4 -- allow gitweb to all but admin repo" + REPO=four + echo " + repo $REPO + RW+ = u4 + - = gitweb daemon + + repo @all + R = @all + + " | ugc + + [ "$ais" = "0" ] && { + does_not_have_export_ok $REPO + is_not_in_projects_list $REPO + does_not_have_export_ok gitolite-admin + is_not_in_projects_list gitolite-admin + } + + [ "$ais" = "1" ] && { + has_export_ok $REPO + is_in_projects_list $REPO + does_not_have_export_ok gitolite-admin + is_not_in_projects_list gitolite-admin + } + + name "set 5 -- go wild" + echo " + repo foo/..* + C = u1 + RW+ = CREATOR + - = gitweb daemon + R = @all + + repo bar/..* + C = u2 + RW+ = CREATOR + - = gitweb daemon + R = @all + config gitolite-options.deny-repo = 1 + " | ugc -r + + can_push u1 foo/one + can_push u2 bar/two + + [ "$ais" = "0" ] && { + does_not_have_export_ok foo/one + is_not_in_projects_list foo/one + does_not_have_export_ok bar/two + is_not_in_projects_list bar/two + } + + [ "$ais" = "1" ] && { + has_export_ok foo/one + is_in_projects_list foo/one + does_not_have_export_ok bar/two + is_not_in_projects_list bar/two + } + + done +done