Git development
 help / color / mirror / Atom feed
* The git protocol and DoS
From: H. Peter Anvin @ 2005-10-19 20:00 UTC (permalink / raw)
  To: Git Mailing List

I've been concerned for a while that the git protocol may be inherently 
vulnerable to a "SYNful DoS" attack (spraying raw TCP SYN packets with 
enough data to start substantial server activity.)  Although SYN cookies 
protect against this to some degree, it makes me wonder if something 
should be added to the protocol itself.

One way to do this would be to start the transaction by having the 
server transmit a cookie to the client, and to require the client to 
send a SHA1 of the (cookie + request) together with the request.  This 
would be done with a fairly short timeout.

It would, however, require a protocol change; I would like to hear what 
people think about this at this stac=ge.

	-hpa

^ permalink raw reply

* Re: [PATCH] git-clone: don't unpack objects
From: Junio C Hamano @ 2005-10-19 20:09 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: Git Mailing List
In-Reply-To: <20051019154341.2aed6998.tihirvon@gmail.com>

Timo Hirvonen <tihirvon@gmail.com> writes:

> Pass --keep flag to git-clone-pack.

Hmph.  Unconditionally?

I do not personally mind this change; I even have this in my
$HOME/share/git-core/templates/config file:

        [clone]
                keeppack = 1

Are there cases where you would do 'git clone' over git native
transport and want the resulting pack expanded?

One use pattern I could think of that you may not want to have
the keeppack behaviour is when you keep multiple, related
foreign repositories.  For example, I could do this:

    cd /tarpit
    git-clone git://kernel.org/pub/scm/.../torvalds/linux-2.6.git/ 2.6

    mkdir -p $HOME/share/git-core/templates/objects/info
    cat >$HOME/share/git-core/templates/objects/info/alternates <<\EOF
    /tarpit/2.6/.git/objects
    EOF
    # disable clone.keeppack in the templates configuration as well.
    ed $HOME/share/git-core/templates/config

    cd /tarpit
    git-clone git://kernel.org/pub/scm/.../jgarzik/libata-dev.git/ ata

The new 'ata' repository created above is set up to borrow from
the cloned '2.6' repository, even before git-clone-pack is run
(thanks to the alternates in the templates).  git-clone-pack
still needs to download and unpack 100k objects, but most of
them are already available through alternates and does not hit
the disk.  It ends up leaving about 1k unpacked object files
that are unique in ata repository.  But we cannot do this if we
say --keep in git-clone unconditionally.

BTW, probably a better way to do the above example would be
(this time without funny templates trick):

    cd /tarpit
    git-clone git://kernel.org/pub/scm/.../torvalds/linux-2.6.git/ 2.6
    git-clone -l -s -n 2.6 ata

    cd ata
    mkdir .git/refs/2.6
    mv .git/refs/heads .git/refs/tags .git/refs/2.6/.
    git-fetch-pack git://kernel.org/pub/scm/.../jgarzik/libata-dev.git/ |
    while read sha1 path
    do
	case "$path" in HEAD) continue ;; esac
	mkdir -p `dirname ".git/$path"`
    	echo "$sha1" > ".git/$path"
    done
    rm -fr .git/refs/2.6

This one asks the other side to send only 6.5k objects instead
of the full cloning-and-discarding, so it achieves the same
result with a lot less burden on the network.

So in that sense, the first example that showed using --keep
does a suboptimal thing does not qualify as a counterargument to
your change, because it already is doing something suboptimal.

What do people on the list think?

^ permalink raw reply

* Re: [PATCH] git-clone: don't unpack objects
From: Johannes Schindelin @ 2005-10-19 20:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Timo Hirvonen, Git Mailing List
In-Reply-To: <7v4q7d5kwo.fsf@assigned-by-dhcp.cox.net>

Hi,

On Wed, 19 Oct 2005, Junio C Hamano wrote:

> What do people on the list think?

For projects with a long history, it might make sense to do it 
unconditionally, but for young projects, the packs would be very small and 
inefficient.

However, I cannot think of other use cases where "--keep" would not make 
sense, and since projects tend to get a long history, maybe it would be 
good to introduce a "--nokeep" to git-clone.

JM2C,
Dscho

^ permalink raw reply

* Re: Pushing a single tag (ref + object)?
From: Linus Torvalds @ 2005-10-19 20:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvezt8mvv.fsf@assigned-by-dhcp.cox.net>



On Wed, 19 Oct 2005, Junio C Hamano wrote:
> 
> Your /bin/sh scripting is rotting ;-) That is not a variable.

Ahh. No. My /bin/sh scripting just has never been that good to begin with, 
since there's little reason to do so inside the kernel ;)

I literally didn't understand what "set" does. Thx. man-pages to the 
rescue.

		Linus

^ permalink raw reply

* Re: [PATCH] git-clone: don't unpack objects
From: Linus Torvalds @ 2005-10-19 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Timo Hirvonen, Git Mailing List
In-Reply-To: <7v4q7d5kwo.fsf@assigned-by-dhcp.cox.net>



On Wed, 19 Oct 2005, Junio C Hamano wrote:
> 
> Are there cases where you would do 'git clone' over git native
> transport and want the resulting pack expanded?

No, I think the unconditional --keep is fine for clone.

What to do about incremental pulls is not as clear, but I think the clone 
case is pretty much obvious these days.

The main reason for unpacking was that packs used to be "strange". These 
days, packs are arguably the _common_ thing.

Now, having tons and tons of small packs is probably worse than having 
tons and tons of individual files, but if people get used to doing 
occasional full repacks, that doesn't matter.

		Linus

^ permalink raw reply

* Re: The git protocol and DoS
From: Junio C Hamano @ 2005-10-19 20:50 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: git
In-Reply-To: <4356A5C5.5080905@zytor.com>

"H. Peter Anvin" <hpa@zytor.com> writes:

> It would, however, require a protocol change; I would like to hear what 
> people think about this at this stac=ge.

Well, it is full two days since a majorly visible git protocol
enabled server has been announced, and you probably know what
kind of hits you are getting (and please let us know if you have
numbers, I am curious).  If we do a protocol change, earlier the
better.  You already said that the kernel.org git is
experimental.  Does anybody run git daemons and rely on the
current protocol?

I suspect it would not make *any* sense to have a backward
compatible server that optionally allows this cookie exchange --
attackers can just say "I am an older client".  OTOH, it
probably makes sense to have an option on the client side to
skip the cookie exchange stage.  I do not think autodetecting
new/old server on the client side in connect.c is possible.

^ permalink raw reply

* Re: The git protocol and DoS
From: H. Peter Anvin @ 2005-10-19 20:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmzl544f3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
>>It would, however, require a protocol change; I would like to hear what 
>>people think about this at this stac=ge.
> 
> Well, it is full two days since a majorly visible git protocol
> enabled server has been announced, and you probably know what
> kind of hits you are getting (and please let us know if you have
> numbers, I am curious).

About 350 hits so far, total.  Utter peanuts.

> If we do a protocol change, earlier the
> better.  You already said that the kernel.org git is
> experimental.  Does anybody run git daemons and rely on the
> current protocol? 
 >
> I suspect it would not make *any* sense to have a backward
> compatible server that optionally allows this cookie exchange --
> attackers can just say "I am an older client".  OTOH, it
> probably makes sense to have an option on the client side to
> skip the cookie exchange stage.  I do not think autodetecting
> new/old server on the client side in connect.c is possible.
> 

You mean an option on the *server* to skip the cookie exchange?  If so, 
how would you expect the client to handle it?

	-hpa

^ permalink raw reply

* Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)
From: Martin Langhoff @ 2005-10-19 20:56 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <Pine.LNX.4.64.0510190724000.3369@g5.osdl.org>

On 10/20/05, Linus Torvalds <torvalds@osdl.org> wrote:
> Interesting.
>
> As you say, I'm obviously testing on ppc all the time, and if
> git-fetch-pack has problems on ppc, I haven't seen them. And I fetch a
> lot.
>
> It might be just the repo you're using. I can't test, since I obviously
> cannot ssh into the repo you pointed at (I tried to see if you had a
> git-daemon running, but no such luck).

Ok -- I seem to be wrong in blaming PPC, so it's back to cg-fetch, and
I think I have a patch that does the right thing.

To recap: repo is slightly broken because cg-fetch has fetched tag
objects, but hasn't followed them through to the commit objects they
refer to.

Internally cg-fetch is actually using git-ssh-fetch (I misreported it
using git-fetch-pack), which is working correctly. However, cg-fetch
attempts to optimize the fetch process, by not calling git-ssh-fetch
if it has the tagobj that the ref points to. How those tag objects get
there without commits in the first place I don't know. So I've removed
the optimization and life is much better.

There is a second bug during the tag fetch. Some of the references
(created by git-cvsimport) are multiline, and break cg-fetch. It's
probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
-n1 instead of cat. I'll deal with git-cvsimport later.

On this broken-ish repo, git-fetch-pack segfaults as I've reported.
Running it under strace shows that it dies walking the local repo,
trying to find a missing commit. Unlikely that this is PPC only ;-)

cheers,



martin

^ permalink raw reply

* Re: [PATCH] git-clone: don't unpack objects
From: Junio C Hamano @ 2005-10-19 20:58 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds
In-Reply-To: <Pine.LNX.4.64.0510191327560.3369@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> No, I think the unconditional --keep is fine for clone.
>
> What to do about incremental pulls is not as clear, but I think the clone 
> case is pretty much obvious these days.
>
> The main reason for unpacking was that packs used to be "strange". These 
> days, packs are arguably the _common_ thing.

OK, that makes sense. 

I'd move this 'unconditional keep' to clone-pack.  The --keep
flag becomes a no-op.

^ permalink raw reply

* Re: The git protocol and DoS
From: Junio C Hamano @ 2005-10-19 21:06 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: git
In-Reply-To: <4356B2C7.601@zytor.com>

"H. Peter Anvin" <hpa@zytor.com> writes:

> You mean an option on the *server* to skip the cookie exchange?  If so, 
> how would you expect the client to handle it?

No, what I was thinking was to tell the client "you will be
talking to an old server, so do not try to read that cookie and
get stuck".

^ permalink raw reply

* Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)
From: Linus Torvalds @ 2005-10-19 21:09 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <46a038f90510191356w56b78413p6b9fe5b67fc9ee74@mail.gmail.com>



On Thu, 20 Oct 2005, Martin Langhoff wrote:
> 
> On this broken-ish repo, git-fetch-pack segfaults as I've reported.
> Running it under strace shows that it dies walking the local repo,
> trying to find a missing commit. Unlikely that this is PPC only ;-)

Ok, that's not horribly surprising, but it's wrong.

Since I'm a lazy bastard, and couldn't necessarily re-create the right 
kind of broken repository even if I weren't, could you just enable 
core-dumps, and run gdb on the thing and report what "where" says is the 
call-chain.

It's almost certainly something that looks up an object and dereferences 
it without checking whether the object lookup succeeded or not. But if you 
can say where the SIGSEGV happens, it will be easier to figure out where.

Of course, what I really _should_ do is improve "sparse" to the point 
where I can mark functions as returning "unsafe" pointers, and then have 
sparse statically find all the dangerous users. I should do that anyway, 
since it would be useful for the kernel too, but it's actually quite 
nontrivial.

		Linus "being lazy is not a state of
			mind - it's a way of life" Torvalds

^ permalink raw reply

* Re: git-send-pack segfaulting on DebianPPC
From: Junio C Hamano @ 2005-10-19 21:15 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Linus Torvalds, git, Petr Baudis
In-Reply-To: <46a038f90510191356w56b78413p6b9fe5b67fc9ee74@mail.gmail.com>

Martin Langhoff <martin.langhoff@gmail.com> writes:

> There is a second bug during the tag fetch. Some of the references
> (created by git-cvsimport) are multiline, and break cg-fetch. It's
> probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
> -n1 instead of cat. I'll deal with git-cvsimport later.

When you update git-cvsimport, please make the tags it generates
git-check-ref-format happy.  The rules are:

 - No pathname component starting with a dot '.' (cg-fetch uses
   refs/heads/.$name-fetching to store the SHA1 it has not fully
   completed; for_each_ref() deliberately ignores path component
   that start with a dot to ignore them, probably to work this
   around).

 - No double-dots anywhere (ref1..ref2 notation is used on the
   command line and ref with embedded double-dots introduces
   ambiguities in parsing).

 - No byte with value lower than \040 (SP) nor \177 (DEL).

 - No tilde '~', caret '^' or colon ':' (the first two are used
   in extended SHA1 expressions; colon is used as src:dst
   notation in push and pull).

 - Not ending with a slash '/'.

> On this broken-ish repo, git-fetch-pack segfaults as I've reported.
> Running it under strace shows that it dies walking the local repo,
> trying to find a missing commit. Unlikely that this is PPC only ;-)

^ permalink raw reply

* Re: Errors from http-fetch
From: Nick Hengeveld @ 2005-10-19 21:26 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0510191536350.25300@iabervon.org>

On Wed, Oct 19, 2005 at 03:46:10PM -0400, Daniel Barkalow wrote:

> I've now started getting errors from http-fetch; my guess for the output 
> is that it has problems if an object is not available loose, and is only 
> available in a pack file which is already being downloaded for a different 
> object, possibly only if the first request to be made for the loose 
> object is not the first one to fail.

Are you getting a curl return code of 52/CURLE_GOT_NOTHING?  Any chance
you have access to the associated server logs?

In the situation you describe, fetch() won't be called for the second
object until fetch() for the first object is finished - at which point
the pack will have been fully downloaded for the first object (or
failed, causing fetch() for the first object to fail as well.)

-- 
For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.

^ permalink raw reply

* Re: The git protocol and DoS
From: Linus Torvalds @ 2005-10-19 21:31 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Junio C Hamano, git
In-Reply-To: <4356B2C7.601@zytor.com>



On Wed, 19 Oct 2005, H. Peter Anvin wrote:
> 
> You mean an option on the *server* to skip the cookie exchange?  If so, how
> would you expect the client to handle it?

Hey guys, I actually planned for the protocol to be extensible.

The client always starts out by sending the "command" first, and if you 
want to add a challenge-response thing, I really think you should make it 
a nice compatible upgrade (and then later on, you can have a server option 
that says "if the client doesn't do the challenge-response version, I 
won't talk to him").

Basically, right now the client sends a

	"git-upload-pack /absolute/pathname/to/repo"

over the protocol, and the whole point of this was that (a) it's 
extensible and (b) the server knows what to expect, and can close the 
socket if it doesn't get a valid packet.

So if you add some extra challenge-response thing, please just do so by 
changing the string. Teach the server to also accept

	"git-upload-pack --challenge /absolute/pathname/to/repo"

for example. Then later, add a "secure server" mode that refuses to do the 
old non-challenge response.

HOWEVER. The server _already_ has some of this logic: if you start it 
outside of inetd, it will start killing its own children when there are 
too many of them, but it will start by sending them a SIGTERM. And the 
git-daemon code is set up so that a SIGTERM will kill any deamon that 
hasn't seen the proper handshake yet.

Once it's seen the proper handshake, the deamon will block SIGTERM. 
Exactly so that if there is a SYN attack, people who use a non-git-aware 
SYN generator will be second-class citizens. So there's not a real 
challenge-response thing, but at least it's set up so that real git 
clients (or something that looks like one) can be recognized, and get 
preferred treatment over people who just open a connection.

Of course, this part doesn't work with the kernel.org setup, since that 
uses inetd, but we could easily add a timeout too, and do the same exact 
thing for SIGALRM (and just do an "alarm(timeout)" at the head of 
"execute()" before we start really trying to read from the socket).

In other words, git-daemon _already_ has support to help fight SYN 
attacks, although it currently only works when stand-alone. It could be 
extended to work with inetd, though.

NOTE! Right now, a git-aware SYN-flooder could send a SYN + 
"git-upload-pack /valid/directory" thing in the proper packed-line format, 
and _then_ just go away. But once you're talking to a git-aware 
SYN-flooder, I don't think a challenge-response makes it any better, since 
a git-aware SYN-flooder would just be written to give the right response.

So unless you actually have _passwords_, and make the response something 
that the other end has to figure out some other way, I don't see what else 
we could do..

			Linus

^ permalink raw reply

* Re: The git protocol and DoS
From: Junio C Hamano @ 2005-10-19 21:54 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: H. Peter Anvin, git
In-Reply-To: <Pine.LNX.4.64.0510191410570.3369@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> But once you're talking to a git-aware 
> SYN-flooder, I don't think a challenge-response makes it any better, since 
> a git-aware SYN-flooder would just be written to give the right response.

I think Peter's point is that the one that can give the right
response needs to read from the server to compute it, and at
that point it is not a "SYN-flooder" anymore.

^ permalink raw reply

* Re: The git protocol and DoS
From: H. Peter Anvin @ 2005-10-19 21:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vek6h43oj.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
>>You mean an option on the *server* to skip the cookie exchange?  If so, 
>>how would you expect the client to handle it?
> 
> No, what I was thinking was to tell the client "you will be
> talking to an old server, so do not try to read that cookie and
> get stuck".
> 

Oh, right.

	-hpa

^ permalink raw reply

* Re: The git protocol and DoS
From: H. Peter Anvin @ 2005-10-19 22:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7voe5l2mvu.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> 
>>But once you're talking to a git-aware 
>>SYN-flooder, I don't think a challenge-response makes it any better, since 
>>a git-aware SYN-flooder would just be written to give the right response.
> 
> I think Peter's point is that the one that can give the right
> response needs to read from the server to compute it, and at
> that point it is not a "SYN-flooder" anymore.
> 

Right.  It has been shown that requiring some effort on the part of the 
client before the server spends work on it can greatly reduce the 
capabilities of a limited-resource client to execute a DoS.

	-hpa

^ permalink raw reply

* [PATCH] Do not send "want" lines for complete objects
From: Johannes Schindelin @ 2005-10-19 22:07 UTC (permalink / raw)
  To: git, junkio

It was all good and well to check if all remote refs are complete (local 
refs or descendants thereof), but we can just as easily use the same 
information to avoid sending "want" lines just for the complete objects in 
the case that not all remote refs are complete (or their names differ).

Also, git-fetch-pack does not have to ask for descendants of remote refs 
which are complete (for now, git-rev-list is told to ignore only the first 
parent). That change also eliminates a code path where a popen()ed handle 
was not pclose()ed.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	With this patch, in a particular setup, git-fetch got 805 objects 
	instead of 8404, i.e. 10 times less! This setup involves 691 
	tags...

	NOTE: I'd rather have git-rev-list circumvented, so that 
	descendants of ack'ed objects are not sent via "have" lines.

	2nd NOTE: If this patch reminds you of my "has_ref()" patch: Yes,
	it is the next generation of that idea.

 fetch-pack.c |   33 +++++++++++++++++++++++++--------
 1 files changed, 25 insertions(+), 8 deletions(-)

applies-to: f76de7f266b0b7feb38c73c7ccb635d6da18a582
f22af070becd6546eeff44895769b8299a6ddb5e
diff --git a/fetch-pack.c b/fetch-pack.c
index 969e72a..9dfd072 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -12,31 +12,49 @@ static const char fetch_pack_usage[] =
 "git-fetch-pack [-q] [-v] [--exec=upload-pack] [host:]directory <refs>...";
 static const char *exec = "git-upload-pack";
 
+#define COMPLETE	(1U << 0)
+
 static int find_common(int fd[2], unsigned char *result_sha1,
 		       struct ref *refs)
 {
 	int fetching;
 	static char line[1000];
-	int count = 0, flushes = 0, retval;
+	static char rev_command[1024];
+	int count = 0, flushes = 0, retval, rev_command_len;
 	FILE *revs;
 
-	revs = popen("git-rev-list $(git-rev-parse --all)", "r");
-	if (!revs)
-		die("unable to run 'git-rev-list'");
-
+	strcpy(rev_command, "git-rev-list $(git-rev-parse --all)");
+	rev_command_len = strlen(rev_command);
 	fetching = 0;
 	for ( ; refs ; refs = refs->next) {
 		unsigned char *remote = refs->old_sha1;
-		unsigned char *local = refs->new_sha1;
 
-		if (!memcmp(remote, local, 20))
+		/*
+		   If that object is complete (i.e. it is a descendant of a
+		   local ref), we don't want it, nor its descendants.
+		*/
+		if (has_sha1_file(remote)
+				&& parse_object(remote)->flags & COMPLETE) {
+			if (rev_command_len + 44 < sizeof(rev_command)) {
+				snprintf(rev_command + rev_command_len, 44,
+					" ^%s^", sha1_to_hex(remote));
+				rev_command_len += 43;
+			}
+
 			continue;
+		}
+
 		packet_write(fd[1], "want %s\n", sha1_to_hex(remote));
 		fetching++;
 	}
 	packet_flush(fd[1]);
 	if (!fetching)
 		return 1;
+
+	revs = popen(rev_command, "r");
+	if (!revs)
+		die("unable to run 'git-rev-list'");
+
 	flushes = 1;
 	retval = -1;
 	while (fgets(line, sizeof(line), revs) != NULL) {
@@ -81,7 +99,6 @@ static int find_common(int fd[2], unsign
 	return retval;
 }
 
-#define COMPLETE	(1U << 0)
 static struct commit_list *complete = NULL;
 
 static int mark_complete(const char *path, const unsigned char *sha1)
---
0.99.8.GIT

^ permalink raw reply related

* Re: Errors from http-fetch
From: Daniel Barkalow @ 2005-10-19 22:10 UTC (permalink / raw)
  To: Nick Hengeveld; +Cc: git
In-Reply-To: <20051019212644.GB6160@reactrix.com>

On Wed, 19 Oct 2005, Nick Hengeveld wrote:

> On Wed, Oct 19, 2005 at 03:46:10PM -0400, Daniel Barkalow wrote:
> 
> > I've now started getting errors from http-fetch; my guess for the output 
> > is that it has problems if an object is not available loose, and is only 
> > available in a pack file which is already being downloaded for a different 
> > object, possibly only if the first request to be made for the loose 
> > object is not the first one to fail.
> 
> Are you getting a curl return code of 52/CURLE_GOT_NOTHING?  Any chance
> you have access to the associated server logs?

I'm getting 404 for the object, which it reports. This was pulling from 
kernel.org.

> In the situation you describe, fetch() won't be called for the second
> object until fetch() for the first object is finished - at which point
> the pack will have been fully downloaded for the first object (or
> failed, causing fetch() for the first object to fail as well.)

My theory is that you have: prefetch(#2), prefetch(#1), (other stuff), #1 
gives an error, start downloading the pack, fetch(#2), #2 gives an error, 
and there are no more packs to try.

That is, it was a later object that prompted getting the pack, because 
either the server decided to send that error message sooner, some packet 
got dropped and retransmitted, the connection got a lower-numbered file 
descriptor and they were ready at the same time, or something like that.

Another thing I noticed was that it was in the middle of downloading the 
pack when it suddenly exitted due to not being able to find the object; it 
would probably be worth having a call to finish up the active transfers 
after giving up on completing the whole thing, since we probably do 
actually want to finish downloading a big pack if we've started.

Consider the situation where you happen to catch the server in the middle 
of mirroring, and you're halfway through a big pack; restarting is nice, 
but it might as well let you finish downloading the pack, so you have less 
to do when you try again later.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: The git protocol and DoS
From: Petr Baudis @ 2005-10-19 22:20 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Git Mailing List
In-Reply-To: <4356A5C5.5080905@zytor.com>

Dear diary, on Wed, Oct 19, 2005 at 10:00:05PM CEST, I got a letter
where "H. Peter Anvin" <hpa@zytor.com> told me that...
> One way to do this would be to start the transaction by having the 
> server transmit a cookie to the client, and to require the client to 
> send a SHA1 of the (cookie + request) together with the request.  This 
> would be done with a fairly short timeout.

  If (well, it sounds like a good idea, so rather "when") you do this,
it would be a good idea to do in a way that makes it easy to later add
support for some kind of authentication (really, not everyone wants to
give away ssh accounts). Let's say it works like:

[client]	git-upload-pack <path>
[server]	challenge somethingnonsensical
[client]	challenge-response <username>:sha1(somethingnonsensical<password>)
[server]	All right, the pack goes like this...

  Suddenly you have support for hopefully secure authentication, and at
the same time you have the cookie implemented in backwards-compatible
fashion (in the sense that new client will be able to talk to old
server) - just assume the username and password empty. This might be
even hardcoded for now, just leave a room for its addition (in an
elegant and compatible way) in the protocol, please.

  Thanks,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply

* Re: Errors from http-fetch
From: Nick Hengeveld @ 2005-10-19 22:37 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0510191755280.25300@iabervon.org>

On Wed, Oct 19, 2005 at 06:10:52PM -0400, Daniel Barkalow wrote:

> I'm getting 404 for the object, which it reports. This was pulling from 
> kernel.org.

Something similar was reported yesterday, and when I investigated I
found that one of the kernel.org servers was returning a 404 for the
object in question and the other wasn't.  Vger bounced my reply because
it contained Content-type: headers...

> That is, it was a later object that prompted getting the pack, because 
> either the server decided to send that error message sooner, some packet 
> got dropped and retransmitted, the connection got a lower-numbered file 
> descriptor and they were ready at the same time, or something like that.

I could see that happen if the pack appeared on the repo after #1 404ed
and subsequently decided there were no packs, and before #2 404ed.  Or,
as was the case I saw yesterday, servers with the same DNS name aren't
in sync.  However, the pack download for #2 should never start in that
case because fetch() for #1 would fail and exit.

> Another thing I noticed was that it was in the middle of downloading the 
> pack when it suddenly exitted due to not being able to find the object; it 
> would probably be worth having a call to finish up the active transfers 
> after giving up on completing the whole thing, since we probably do 
> actually want to finish downloading a big pack if we've started.

I'm not sure how that could happen - once a pack download request
starts, it has to finish before any fetch() calls can fail.  However,
there could certainly be other object requests in process when one
fails, and it would be polite to let them finish.

-- 
For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.

^ permalink raw reply

* Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)
From: Petr Baudis @ 2005-10-19 22:37 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Linus Torvalds, Junio C Hamano, git
In-Reply-To: <46a038f90510191356w56b78413p6b9fe5b67fc9ee74@mail.gmail.com>

Dear diary, on Wed, Oct 19, 2005 at 10:56:35PM CEST, I got a letter
where Martin Langhoff <martin.langhoff@gmail.com> told me that...
> To recap: repo is slightly broken because cg-fetch has fetched tag
> objects, but hasn't followed them through to the commit objects they
> refer to.

That must be leftover of some old fetch, before cg-fetch got fixed wrt.
this issue.

> Internally cg-fetch is actually using git-ssh-fetch (I misreported it
> using git-fetch-pack), which is working correctly. However, cg-fetch
> attempts to optimize the fetch process, by not calling git-ssh-fetch
> if it has the tagobj that the ref points to. How those tag objects get
> there without commits in the first place I don't know. So I've removed
> the optimization and life is much better.

The "optimization" or something alike needs to be there at least for the
user interface, so that we can actually say what tags are we
downloading; besides, there can be a *lot* of tags. But I'm not sure how
moot will this all be after Cogito moves to the remote peeking (Real
Soon Now, promise ;-).

> There is a second bug during the tag fetch. Some of the references
> (created by git-cvsimport) are multiline, and break cg-fetch. It's
> probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
> -n1 instead of cat. I'll deal with git-cvsimport later.

Eek. Did I miss something and are multiline refs meaningful now? If not,
they are pretty bad and I'd imagine other parts of Cogito would be
pretty confused about that. I'd call this a corrupted repository,
tough. Perhaps a check should be added to fsck.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply

* Re: The git protocol and DoS
From: Tony Luck @ 2005-10-19 22:39 UTC (permalink / raw)
  To: Petr Baudis; +Cc: H. Peter Anvin, Git Mailing List
In-Reply-To: <20051019222044.GP30889@pasky.or.cz>

On 10/19/05, Petr Baudis <pasky@suse.cz> wrote:
> [client]        git-upload-pack <path>
> [server]        challenge somethingnonsensical
> [client]        challenge-response <username>:sha1(somethingnonsensical<password>)
> [server]        All right, the pack goes like this...

I think this requires that the server store the cleartext version of
the password so
that it can validate sha1(somethingnonsensical<password>) ... which is generally
thought to be a bad idea.

-Tony

^ permalink raw reply

* Re: [PATCH] Do not send "want" lines for complete objects
From: Junio C Hamano @ 2005-10-19 23:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0510200004290.7689@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Also, git-fetch-pack does not have to ask for descendants of remote refs 
> which are complete (for now, git-rev-list is told to ignore only the first 
> parent).

I should interpret what you said with s/descendant/ancestor/
applied, I think.  I would like to make sure I understand what
you are doing here.

> +		if (has_sha1_file(remote)
> +				&& parse_object(remote)->flags & COMPLETE) {
> +			if (rev_command_len + 44 < sizeof(rev_command)) {
> +				snprintf(rev_command + rev_command_len, 44,
> +					" ^%s^", sha1_to_hex(remote));
> +				rev_command_len += 43;
> +			}

This rev-list command is to generate the list of "have", and we
learned that the other side says she has remote -- we choose not
to tell her that we have ancestors of it, but we do tell her
about the remote head itself.

Let's draw a single strand of pearl case to illustrate.  You
have a commit chain A->B->C->D, and the other end says she has
C.  At this point, telling her that you have C is enough, and by
not telling her about A and B, you would save her from depreting
MAX_HAVE slots.  Of course, if the other end has D as another
head, then the above logic would give "^D^" to rev-list as well,
telling it not to tell her about C, but that is what we want --
because she already knows you have C too when you tell her that
you have D.  I think I like this optimization.

One thing that might help, when we are telling the other end
about what we have, is an output ordering option to get-rev-list
that shows not in chronological order, but in the order of
distance from the tip.  That may give the other end a better
chance to find the latest (in commit order) common commit in
each branch without running out is MAX_HAS buffer.

> 	NOTE: I'd rather have git-rev-list circumvented, so that 
> 	descendants of ack'ed objects are not sent via "have" lines.

I think since we do the traversal ourselves this may start to
make more sense.

^ permalink raw reply

* Re: git-send-pack segfaulting on DebianPPC (was: Re: cg-clone, tag objects and cg-push/git-push don't play nice)
From: Martin Langhoff @ 2005-10-19 23:23 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Linus Torvalds, Junio C Hamano, git, Penny @ Catalyst
In-Reply-To: <20051019223743.GQ30889@pasky.or.cz>

On 10/20/05, Petr Baudis <pasky@suse.cz> wrote:
> Dear diary, on Wed, Oct 19, 2005 at 10:56:35PM CEST, I got a letter
> where Martin Langhoff <martin.langhoff@gmail.com> told me that...
> > To recap: repo is slightly broken because cg-fetch has fetched tag
> > objects, but hasn't followed them through to the commit objects they
> > refer to.
>
> That must be leftover of some old fetch, before cg-fetch got fixed wrt.
> this issue.

Yes, I'm trying to figure out if I can fix it...

> > Internally cg-fetch is actually using git-ssh-fetch (I misreported it
> > using git-fetch-pack), which is working correctly. However, cg-fetch
> > attempts to optimize the fetch process, by not calling git-ssh-fetch
> > if it has the tagobj that the ref points to. How those tag objects get
> > there without commits in the first place I don't know. So I've removed
> > the optimization and life is much better.
>
> The "optimization" or something alike needs to be there at least for the
> user interface, so that we can actually say what tags are we
> downloading; besides, there can be a *lot* of tags. But I'm not sure how
> moot will this all be after Cogito moves to the remote peeking (Real
> Soon Now, promise ;-).
>
> > There is a second bug during the tag fetch. Some of the references
> > (created by git-cvsimport) are multiline, and break cg-fetch. It's
> > probably a bug in git-cvsimport, but I'm fixing cg-fetch to use head
> > -n1 instead of cat. I'll deal with git-cvsimport later.
>
> Eek. Did I miss something and are multiline refs meaningful now? If not,
> they are pretty bad and I'd imagine other parts of Cogito would be
> pretty confused about that. I'd call this a corrupted repository,
> tough. Perhaps a check should be added to fsck.

Actually, all the tagsrefs have only one line, but something is going
weird around tagid=$(cat $tag) as $tag ends up containing many
filenames. So I undid my initial change to "head -n1", and I'm trying
to fix the loop.

cheers,


martin

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox