SECURITY NOTE: if you deleted or renamed a pubkey file after 5fd9328
went in (April 12th), please:
- upgrade asap, then
- go to your latest gitolite-admin clone and "git push -f"
Otherwise this is not urgent.
5fd9328 (and its minor successor 813a2a9) were about preventing the
gitolite admin from sneaking in files to src/ and hooks/ into
$GL_ADMINDIR. It seemed easy enough to do this by converting the
path-less checkout to a with-paths checkout, but this has caused a worse
problem -- deleting a keydir/foo.pub now no longer has an effect; the
file still hangs around in the work tree.
Ouch! (and thanks to teukka for noticing)
We now do this check as a separate step, so the checkout can revert to
being path-less.
Always passing "-p 22" to ssh (or "-P 22" to scp) if no custom port is given on
the command line causes trouble when not using a host name but an SSH session
name (as defined in .ssh/config) which defines a non-standard port, because the
port given on the command line overrides that port.
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Having to specify "D" separately from RW or RW+ was cumbersome, and
although I don't actually use this feature, I can see the point.
One way to think of this is:
- RW and RW+ were the only existing branch level rights
- it doesnt make sense to have D rights without W (hence RW) rights
- so we simply suffix a D to these if required.
Thus you can have RW, RW+, RWD, RW+D.
I hope the (hopefully few) of you who have started to use this feature
will convert your configs when you next upgrade to "pu".
I now regret pushing the previous syntax to master too quickly -- lots
of people use master only, and on the next promotion of pu the syntax
will change. To reduce this exposure, this change will be promoted to
master very soon.
This is a pretty big hole, really. Only the fact that Eli called it an
"accidental feature" helped catch it :)
Notes on the code:
An explicit list of paths -- maybe just "conf", "keydir", and "local" --
would have been easier, but this isn't too bad, I think.
people with shell access should be allowed to bypass the update hook, to
allow them to clone locally and push. You can now do this by setting an
env var that the ssh "front door" will never set, like so:
GL_BYPASS_UPDATE_HOOK=1 git push
Note that this will NOT work for the gitolite-admin repo, because the
post-update hook on that one requires a bit more. If you really want to
do that, try:
GL_ADMINDIR=~/.gitolite GL_BINDIR=~/.gitolite/src GL_BYPASS_UPDATE_HOOK=1 git push
(assuming default values in ~/.gitolite.rc)
Previous implementations of "give shell access to some gitolite users"
feature were crap. There was no easy/elegant way to ensure that someone
who had repo admin access would not manage to get himself shell access.
Giving someone shell access requires that you should have shell access
in the first place, so the simplest way is to enable it from the server
side only.
So now that we decided to do that, we may as well prepare for other,
future, commands by starting a server-side utility program with
sub-commands (the only current one being "shell-add")
normally, RW+ means permission to rewind or delete.
Now, if you use "D" permission anywhere in a repo config, that means
"delete" and RW+ then means only "rewind", no delete.
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!
a configuration like this:
repo CREATER/.*
C = CREATER
RW+ = WRITERS
was buggy; CREATER was implicitly part of WRITERS so he got RW
permissions implicitly, so the push went through
what this means is that until now, everyone who used easy-install
(without needing to set $GIT_PATH in the rc file) had a client-side PATH
that was perfectly valid on the server side also!
- no need to put it at the end of the config file now, yeaaay!
- @all for @all is meaningless and not supported. People asking will
be told to get a life or use git-daemon.
- NAME/ limits for @all repos is ignored for efficiency reasons.
Data dumper was failing (returning an empty string!) on an input config
file of about 350 lines or so (output 2400 lines or so).
Removing the sort sub fixed the problem.
To recap why that sub was put in (see deleted lines in this commit for
details), what we really want is that $creater must appear *last* in the
resulting dump.
So we trick it. "man ascii" tells you that ~ is the highest valued
ASCII character (yes, I know, not utf-8 safe etc... I'll deal with that
if and when needed or punt!). So we just put that in front of $creater
and remove it later...
You *don't* want to do this for $readers and $writers -- then they will
once again sort *after* $creater, which would be a bad thing. Also,
it's probably better this way, because now the order of the hash keys
will be: $readers, $writers, any actual users listed, and then $creater.
This means the effective access rights will be:
1. if you are the creater you get CREATER's rights
2. else if your userid is listed *explicitly* in the config, you get
those rights
3. else if you've been setperm'd as a writer, you get WRITERS rights
4. else if you've been setperm'd as a reader, you get READERS rights
This is different from what used to happen till now; READERS and WRITERS
used to trump explicitly given rights. I'd been meaning to fix that
somehow, but never got around to it, until this DDD (damn Data Dumper!)
forced my hand :)
There are some disadvantages to the old-style personal branch scheme.
It only allows one specific pattern (of refname) to be used, forces that
pattern to be applicable to *all* repos in the entire config, and
requires editing the rc file (on the server) to be edited to achieve
this.
In other words, it's a very blunt instrument...
The new style depends on using lines like this within a specific repo
config:
RW+ personal/USER/ = @userlist
The important thing is that the "branch" name should contain `/USER/`
(including the slashes). Access is still determined by the right hand
side of course.
This gives you the following advantages:
- allow it only for repos that need it
- allow different patterns to be used for different repos
- allow *multiple* patterns; just add more than one such line
- allow the pattern to have suffixes (eg: foo/USER/bar)
The new style personal branches work by interpreting the special
sequence /USER/ (including the slashes) in a refname. Docs should be in
the next commit...
the changes to cp/scp are because without "-p" they dont carry perms
across to existing files. So if you forgot to chmod +x your custom
hook and ran easy install, then after that you have to go to the server
side to fix the perms...
