Re: [RFC][PATCH] Allow transfer of any valid sha1

[ebiederm@xmission.com (Eric W. Biederman)]

Junio C Hamano <junkio@cox.net> writes:

> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Can we fix the check in upload-pack.c something like my
>> patch below does?  Are there any security implications for
>> doing that?
>
>> Could we just make the final check before dying if (!o) ?
>
> The primary implication is about correctness, so I am reluctant
> to break it without a careful alternative check in place.
>
> The issue is that having a single object in the repository does
> not guarantee that you have everything reachable from it, and we
> need that guarantee.  Reachability from the refs is what
> guarantees that.

I don't see why having something reachable from a ref guarantees
that everything is reachable.  Given the recent patch that added
a check to make certain a ref actually existed I believe there
is some evidence that trees may become corrupted, and have the
problems you describe.

> We are careful to update the ref at the very end of the transfer
> (fetch/clone or push); so if an object is reachable from a ref,
> then all the objects reachable from that object are available in
> the repository.

In the normal case I agree.

> Imagine http commit walker started fetching tip of upstream into
> your repository and you interrupted the transfer.  Objects near
> the tip of the upstream history are available after such an
> interrupted transfer.  But a bit older history (but still later
> than what we had before we started the transfer) are not.
>
> We do not update the ref with the downloaded tip object, so that
> we would not break the guarantee.  This guarantee is needed for
> feeding clients from the repository later.  If you tell your
> clients, after such an interrupted transfer, that you are
> willing to serve the objects near the (new) tip, the clients may
> rightfully request objects that are reachable from these
> objects, some of them you do _not_ have!

I clearly would not advertise it.  My problem is that I have
evidence that someone pulled a given sha1 at some point from 
some branch on a given repository.  But I don't have that branch.

Actually trees mirrored with rsync have similar problems all of
the time when the catch a tree in the middle of an update.

> So this "on demand SHA1" stuff needs to be solved by checking if
> the given object is reachable from our refs in upload-pack,
> instead of the current check to see if the given object is
> pointed by our refs.  When upload-pack can prove that the object
> is reachable from one of the refs, it is OK to use it; otherwise
> you should not.

I have a problem with that approach.  Suppose the branch I have
evidence something came from is like your pu branch.   If I want
a copy of your pu branch at some point in the past, but you have
rebased it since that sha1 was published then there will clearly not
be a path from any current head to that branch.  But if I still have a
copy of the sha1 I should actually be able to recover the old copy of
the pu branch from your tree.

> Now, proving that a given SHA1 is the name of an object that
> exists in the repository is cheap (has_sha1_file()), but proving
> that the object is reachable from some of our refs can become
> quite expensive.  That gives this issue a security implication
> as well -- you can easily DoS the git-daemon that way, for
> example.

Exactly, which is why I aimed for the cheap test.

There is a reasonable argument that can be made that the branches
represent the policy that you are willing to serve.  If you have a
tree and share a common object store with a much lager tree, like
David Woodhouse has set up, I can see such a policy being desirable.

That is an argument I have a much harder time shooting down.
At the same time if it is just a policy question the policy it should
be modifiable with an appropriate configuration directive, or
command line option.

Eric