On Tue, May 17, 2011 at 05:08, Sitaram Chamarty <sitaramc@xxxxxxxxx> wrote: > On Tue, May 17, 2011 at 7:19 AM, Shawn Pearce <spearce@xxxxxxxxxxx> wrote: >> On Mon, May 16, 2011 at 18:32, Sitaram Chamarty <sitaramc@xxxxxxxxx> wrote: > >>> PS: Gitolite does have unreleased code to do this but it's a hack with >>> several limitations. Gitolite makes a temp "clone -l", deletes all >>> refs from it that the user has no access to, then redirects the >>> git-upload-pack to that repo instead ;-) >> >> Cute hack. Doesn't prevent the evil client from making an indirect >> reference to something you shouldn't have. :-) > > You mean he constructs a commit that references a SHA he should not be > having, pushes that to the branch he is allowed to read/write, then > pulls it down again to now really get that commit? Yes. Or, he has a SHA-1 he suspects is a tree or blob and lists that in a tree he pushes to a branch he can write to. Now he can fetch that branch back, and obtain that object whose SHA-1 he has but whose contents he does not have. There is another attack that is incredibly improbable, but that JGit tries to protect against here as well. An evil user could try to push an object that uses the REF_DELTA format and specifies a SHA-1 base that he wants to see at least some of the content of. The delta copy instructions copy some of the base, and then insert content the attacker knows. In theory the attacker cannot predict the SHA-1 of the resulting object and thus cannot reference it in a tree or commit in order to make a link and fetch it back. However if there is a weakness in SHA-1 that has not been discovered yet an attacker may be able to craft the text he knows and supplied as delta insert commands in such a way that the text of the object he is trying to copy has little to no impact on the resulting SHA-1. Now he can predict the SHA-1 this delta creates, and if he can make a link to it, he can fetch it back and acquire at least part of the remote object. Its paranoid to check the REF_DELTA bases for visibility before applying the delta, but we do it in JGit because its better to be slightly paranoid than to assume this theoretical attack is too improbable to succeed. (And it is given what we know about SHA-1 today.) > Yeah, I started writing a hook that looks at `rev-list > oldsha..newsha`, and for each commit run `git branch --contains SHA` > and make sure it either (a) is totally new to the repo, ie no ref > contains this commit or (b) at least one of the refs that contains > this commit is allowed for this user. Yea, that isn't sufficient because of the tree/blob link issue. (See above.) > I haven't had time to do that though. Also, if there has been a > rewind/force-push and the attacker knows the now unreachable SHA, this > would not catch it (it'd look like a totally new commit). That's a > hard one. Yes. This the branch --contains test is insufficient because you need to verify the "new" object actually was transmitted by the client in this exchange, and wasn't just already present on disk. This is hard because in C Git unpack-objects will not write the object if the object already exists, and then there is no list of objects the client sent. Again this is another area where JGit is paranoid. It keeps track of every object actually sent by the user. The only "new" objects permitted are those that were sent by the user, any other "new" objects are attempts to access something the client shouldn't have access to, or is a broken pack file created by a broken client (i.e. it did not send all objects it should have sent). > Having two repos is still the best plan ;-) Yes, but tell that to Gerrit Code Review users. They really use the branch ACL features. :-) -- Shawn. -- 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