diff --git a/README.mkd b/README.mkd index 7f3956a..12596a4 100644 --- a/README.mkd +++ b/README.mkd @@ -86,10 +86,7 @@ detail [here][gsdiff]. ### security Due to the environment in which this was created and the need it fills, I -consider this a "security" program, albeit a very modest one. The code is -very small and easily reviewable -- the 2 programs that actually control -access when a user logs in total about 220 lines of code (about 90 lines -according to "sloccount"). +consider this a "security" program, albeit a very modest one. For the first person to find a security hole in it, defined as allowing a normal user (not the gitolite admin) to read a repo, or write/rewind a ref, @@ -99,10 +96,10 @@ or in Unix, perl, shell, etc.)... well I can't afford 1000 USD rewards like djb, so you'll have to settle for 1000 INR (Indian Rupees) as a "token" prize :-) -Update 2010-01-31: this security promise does not apply if you enable any of -the external command helpers (like rsync). It's probably quite secure, but I -just haven't thought about it enough to be able to make such promises, like I -can for the rest of "master". +However, there are a few optional features (which must be explicitly enabled +in the RC file) where I just haven't had the time to reason about security +thoroughly enough. Please read the comments in `conf/example.gitolite.rc` for +details, looking for the word "security". ---- diff --git a/conf/example.gitolite.rc b/conf/example.gitolite.rc index bcb281f..ddf5baa 100644 --- a/conf/example.gitolite.rc +++ b/conf/example.gitolite.rc @@ -108,6 +108,26 @@ $GIT_PATH=""; # -------------------------------------- + + +# ---------------------------------------------------------------------- +# SECURITY SENSITIVE SETTINGS +# +# Settings below this point may have security implications. That +# usually means that I have not thought hard enough about all the +# possible ways to crack security if these settings are enabled. + +# Please see details on each setting for specifics, if any. +# ---------------------------------------------------------------------- + + + +# -------------------------------------- +# EXTERNAL COMMAND HELPER -- HTPASSWD + +# security note: runs an external command (htpasswd) with specific arguments, +# including a user-chosen "password". + # if you want to enable the "htpasswd" command, give this the absolute path to # whatever file apache (etc) expect to find the passwords in. @@ -118,7 +138,10 @@ $HTPASSWD_FILE = ""; # -------------------------------------- # EXTERNAL COMMAND HELPER -- RSYNC -# + +# security note: runs an external command (rsync) with specific arguments, all +# presumably filled in correctly by the client-side rsync. + # base path of all the files that are accessible via rsync. Must be an # absolute path. Leave it undefined or set to the empty string to disable the # rsync helper. @@ -126,6 +149,21 @@ $RSYNC_BASE = ""; # $RSYNC_BASE = "/home/git/up-down"; # $RSYNC_BASE = "/tmp/up-down"; +# -------------------------------------- +# ALLOW REPO CONFIG TO USE WILDCARDS + +# security note: this used to in a separate "wildrepos" branch. You can +# create repositories based on wild cards, give "ownership" to the specific +# user who created it, allow him/her to hand out R and RW permissions to other +# users to collaborate, etc. This is powerful stuff, and I've made it as +# secure as I can, but it hasn't had the kind of rigorous line-by-line +# analysis that the old "master" branch had. + +# This has now been rolled into master, with all the functionality gated by +# this variable. Set this to 1 if you want to enable the wildrepos features. +# Please see doc/4-wildcard-repositories.mkd for details. +# $GL_WILDREPOS = 0; + # -------------------------------------- # per perl rules, this should be the last line in such a file: 1; diff --git a/doc/3-faq-tips-etc.mkd b/doc/3-faq-tips-etc.mkd index b2a1ba7..2cf7381 100644 --- a/doc/3-faq-tips-etc.mkd +++ b/doc/3-faq-tips-etc.mkd @@ -149,7 +149,7 @@ plain "git archive", because the Makefile adds a file called git clone git://sitaramc.indefero.net/sitaramc/gitolite.git cd gitolite make master.tar - # or maybe "make wildrepos.tar" or "make pu.tar" + # or maybe "make pu.tar" @@ -626,8 +626,7 @@ per-repo "gitconfig" settings. Please see `doc/2-admin.mkd` and #### repos named with wildcards -**This feature only exists in the "wildrepos" branch!** Please see -`doc/4-wildcard-repositories.mkd` for all the details. +Please see `doc/4-wildcard-repositories.mkd` for all the details. #### access control for external commands diff --git a/doc/4-wildcard-repositories.mkd b/doc/4-wildcard-repositories.mkd index 2324a02..aba4f66 100644 --- a/doc/4-wildcard-repositories.mkd +++ b/doc/4-wildcard-repositories.mkd @@ -2,15 +2,14 @@ ***IMPORTANT NOTE***: -This branch contains features that are likely to be much more brittle than the -"master" branch. Creating repositories based on wild cards, giving -"ownership" to the specific user who created it, allowing him/her to hand out -R and RW permissions to other users to collaborate, all these are possible. -And any of these could have a bug in it. +This feature may be somewhat "brittle" in terms of security. Creating +repositories based on wild cards, giving "ownership" to the specific user who +created it, allowing him/her to hand out R and RW permissions to other users +to collaborate, all these are possible. And any of these could have a bug in +it. I haven't found any yet, but that doesn't mean there aren't any. -"Brittle" also means some features in "master" may not work here. For -example, you cannot specify gitconfig values for a wildcard repo; it only -works for actual repos. +Also, there are some limitations. For example, you cannot specify gitconfig +values for a wildcard repo; it only works for actual repos. There may be other such missing features. Sometimes it's just not possible to make it work. Or it may be cumbersome enough that unless there are *no* @@ -25,7 +24,8 @@ In this document: * wildcard repos without creater name in them * side-note: line-anchored regexes * contrast with refexes - * handing out rights to wildcard-matached repos + * handing out rights to wildcard-matched repos + * setting a gitweb description for a wildcard-matched repo * reporting * other issues and discussion @@ -121,7 +121,7 @@ actually push such a branch! You can anchor it if you really care, by using `master$` instead of `master`, but anchoring is *not* the default for refexes.] -### Handing out rights to wildcard-matached repos +### Handing out rights to wildcard-matched repos In the examples above, we saw two special "user" names: READERS and WRITERS. The permissions they have are controlled by the config file, but ***who is @@ -166,7 +166,12 @@ The following points are important: * whoever you specify as "R" will match the special user READERS. "RW" will match WRITERS. -### Reporting +### setting a gitweb description for a wildcard-matched repo + +Similar to the getperm/setperm commands, there are the getdesc/setdesc +commands, thanks to Teemu. + +### reporting Remember the cool stuff you see when you just do `ssh git@server` (grep for "myrights" in `doc/3-faq-tips-etc.mkd` if you forgot, or go [here][mr]). @@ -177,7 +182,11 @@ This still works, except the format is a little more compressed to accommodate a new column (at the start) for "C" permissions, which indicate that you are allowed to *create* repos matching that pattern. -### Other issues and discussion +In addition, there is also the "expand" command, which takes any regex pattern +and returns you a list of all wildcard-created repos that you have access to +which fit that pattern. + +### other issues and discussion * *what if the repo name being pushed matches more than one pattern*? diff --git a/src/gitolite.pm b/src/gitolite.pm index 106624a..7f85315 100644 --- a/src/gitolite.pm +++ b/src/gitolite.pm @@ -33,7 +33,8 @@ our $USERNAME_PATT=qr(^\@?[0-9a-zA-Z][0-9a-zA-Z._\@+-]*$); # very simple patter # same as REPONAME, plus some common regex metas our $REPOPATT_PATT=qr(^\@?[0-9a-zA-Z][\\^.$|()[\]*+?{}0-9a-zA-Z._\@/-]*$); -our $REPO_UMASK; +# these come from the RC file +our ($REPO_UMASK, $GL_WILDREPOS); our %repos; # ---------------------------------------------------------------------------- @@ -134,6 +135,8 @@ sub new_repo my ($repo, $hooks_dir, $creater) = @_; umask($REPO_UMASK); + die "wildrepos disabled, can't set creater $creater on new repo $repo\n" + if $creater and not $GL_WILDREPOS; system("mkdir", "-p", "$repo.git") and die "$ABRT mkdir $repo.git failed: $!\n"; # erm, note that's "and die" not "or die" as is normal in perl @@ -226,6 +229,7 @@ sub parse_acl # please update that also if the interface or the env vars change my ($GL_CONF_COMPILED, $repo, $c, $r, $w) = @_; + $c = $r = $w = "NOBODY" unless $GL_WILDREPOS; # void $r if same as $w (otherwise "readers" overrides "writers"; this is # the same problem that needed a sort sub for the Dumper in the compile @@ -251,6 +255,8 @@ sub parse_acl 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; # didn't find $repo in %repos, so it must be a wildcard-match case my @matched = grep { $repo =~ /^$_$/ } sort keys %repos; diff --git a/src/gl-auth-command b/src/gl-auth-command index 3628c19..f7f415a 100755 --- a/src/gl-auth-command +++ b/src/gl-auth-command @@ -24,7 +24,7 @@ use warnings; # ---------------------------------------------------------------------------- # these are set by the "rc" file -our ($GL_LOGT, $GL_CONF_COMPILED, $REPO_BASE, $GIT_PATH, $REPO_UMASK, $GL_ADMINDIR, $RSYNC_BASE, $HTPASSWD_FILE); +our ($GL_LOGT, $GL_CONF_COMPILED, $REPO_BASE, $GIT_PATH, $REPO_UMASK, $GL_ADMINDIR, $RSYNC_BASE, $HTPASSWD_FILE, $GL_WILDREPOS); # and these are set by gitolite.pm our ($R_COMMANDS, $W_COMMANDS, $REPONAME_PATT, $REPOPATT_PATT); our %repos; @@ -110,6 +110,7 @@ my $CUSTOM_COMMANDS=qr/^\s*(expand|(get|set)(perms|desc))\s/; # commands is sort of a dead end for normal (git) processing if ($ENV{SSH_ORIGINAL_COMMAND} =~ $CUSTOM_COMMANDS) { + die "wildrepos disabled, sorry\n" unless $GL_WILDREPOS; my $cmd = $ENV{SSH_ORIGINAL_COMMAND}; my ($verb, $repo) = ($cmd =~ /^\s*(\S+)\s+\/?(.*?)(?:.git)?$/); if ($repo =~ $REPONAME_PATT and $verb =~ /getperms|setperms/) { @@ -171,8 +172,8 @@ if ( -d "$repo_base_abs/$repo.git" ) { } else { &parse_acl($GL_CONF_COMPILED, $repo, $user, $user, $user); - # auto-vivify new repo if you have C access - if ( $repos{$repo}{C}{$user} || $repos{$repo}{C}{'@all'} ) { + # auto-vivify new repo if you have C access (and wildrepos is on) + if ( $GL_WILDREPOS and $repos{$repo}{C}{$user} || $repos{$repo}{C}{'@all'} ) { wrap_chdir("$repo_base_abs"); new_repo($repo, "$GL_ADMINDIR/src/hooks", $user); wrap_chdir($ENV{HOME}); diff --git a/src/gl-compile-conf b/src/gl-compile-conf index 5c19ebb..e173a56 100755 --- a/src/gl-compile-conf +++ b/src/gl-compile-conf @@ -61,7 +61,7 @@ $Data::Dumper::Sortkeys = sub { return [ reverse sort keys %{$_[0]} ]; }; open STDOUT, ">", "/dev/null" if (@ARGV and shift eq '-q'); # these are set by the "rc" file -our ($GL_ADMINDIR, $GL_CONF, $GL_KEYDIR, $GL_CONF_COMPILED, $REPO_BASE, $REPO_UMASK, $PROJECTS_LIST, $GIT_PATH, $SHELL_USERS); +our ($GL_ADMINDIR, $GL_CONF, $GL_KEYDIR, $GL_CONF_COMPILED, $REPO_BASE, $REPO_UMASK, $PROJECTS_LIST, $GIT_PATH, $SHELL_USERS, $GL_WILDREPOS); # and these are set by gitolite.pm our ($REPONAME_PATT, $REPOPATT_PATT, $USERNAME_PATT, $AUTH_COMMAND, $AUTH_OPTIONS, $ABRT, $WARN); @@ -133,7 +133,8 @@ sub expand_list for my $item (@list) { - die "$ABRT bad user or repo name $item\n" unless $item =~ $REPOPATT_PATT or $item =~ $USERNAME_PATT; + die "$ABRT bad user or repo name $item\n" + unless ($GL_WILDREPOS ? $item =~ $REPOPATT_PATT : $item =~ $REPONAME_PATT) or $item =~ $USERNAME_PATT; if ($item =~ /^@/) # nested group { die "$ABRT undefined group $item\n" unless $groups{$item}; @@ -206,6 +207,7 @@ sub parse_conf_file my $perms = $1; my @refs; @refs = split(' ', $2) if $2; my @users = split ' ', $3; + die "wildrepos disabled, cant use 'C' in config\n" if $perms eq 'C' and not $GL_WILDREPOS; # if no ref is given, this PERM applies to all refs @refs = qw(refs/.*) unless @refs;