Problem with git-http-backend.exe as iis cgi

Problem with git-http-backend.exe as iis cgi

[Florian Manschwetus]

Moin,
I tried to setup git-http-backend with iis, as iis provides proper impersonation for cgi under windows, which leads to have the filesystem access performed with the logon user, therefore the webserver doesn't need generic access to the files.
I stumbled across a problem, ending up with post requests hanging forever.
After some investigation I managed to get it work by wrapping the http-backend into a bash script, giving a lot of control about the environmental things, I was unable to solve within IIS configuration.
The workaround, I use currently, is to use "/bin/head -c ${CONTENT_LENGTH} | ./git-http-backend.exe", which directly shows the issue.
Git http-backend should check if CONTENT_LENGTH is set to something reasonable (e.g. >0) and should in this case read only CONTENT_LENGTH bytes from stdin, instead of reading till EOF what I suspect it is doing currently.

Mit freundlichen Grüßen / With kind regards
Florian Manschwetus

CS Software Concepts and Solutions GmbH
Geschäftsführer / Managing director: Dr. Werner Alexi 
Amtsgericht Wiesbaden HRB 10004 (Commercial registry)
Schiersteiner Straße 31
D-65187 Wiesbaden
Germany

Re: Problem with git-http-backend.exe as iis cgi

[Konstantin Khomoutov]

On Thu, 10 Mar 2016 07:28:50 +0000
Florian Manschwetus <manschwetus@cs-software-gmbh.de> wrote:

> I tried to setup git-http-backend with iis, as iis provides proper
> impersonation for cgi under windows, which leads to have the
> filesystem access performed with the logon user, therefore the
> webserver doesn't need generic access to the files. I stumbled across
> a problem, ending up with post requests hanging forever. After some
> investigation I managed to get it work by wrapping the http-backend
> into a bash script, giving a lot of control about the environmental
> things, I was unable to solve within IIS configuration. The
> workaround, I use currently, is to use "/bin/head -c
> ${CONTENT_LENGTH} | ./git-http-backend.exe", which directly shows the
> issue. Git http-backend should check if CONTENT_LENGTH is set to
> something reasonable (e.g. >0) and should in this case read only
> CONTENT_LENGTH bytes from stdin, instead of reading till EOF what I
> suspect it is doing currently.

The rfc [1] states in its section 4.2:

| A request-body is supplied with the request if the CONTENT_LENGTH is
| not NULL.  The server MUST make at least that many bytes available
| for the script to read.  The server MAY signal an end-of-file
| condition after CONTENT_LENGTH bytes have been read or it MAY supply
| extension data.  Therefore, the script MUST NOT attempt to read more
| than CONTENT_LENGTH bytes, even if more data is available.  However,
| it is not obliged to read any of the data.

So yes, if Git currently reads until EOF, it's an error.
The correct way would be:

1) Check to see if the CONTENT_LENGTH variable is available in the
   environment.  If no, read nothing.

2) Otherwise read as many bytes it specifies, and no more.

1. https://www.ietf.org/rfc/rfc3875

AW: Problem with git-http-backend.exe as iis cgi

[Florian Manschwetus]

[-- Attachment #1: Type: text/plain, Size: 2514 bytes --]

Hi,
I put together a first patch for the issue.

Mit freundlichen Grüßen / With kind regards
Florian Manschwetus

E-Mail: manschwetus@cs-software-gmbh.de
Tel.: +49-(0)611-8908534
 
CS Software Concepts and Solutions GmbH
Geschäftsführer / Managing director: Dr. Werner Alexi 
Amtsgericht Wiesbaden HRB 10004 (Commercial registry)
Schiersteiner Straße 31
D-65187 Wiesbaden
Germany
Tel.: 0611/8908555


-----Ursprüngliche Nachricht-----
Von: Konstantin Khomoutov [mailto:kostix+git@007spb.ru] 
Gesendet: Donnerstag, 10. März 2016 13:55
An: Florian Manschwetus
Cc: git@vger.kernel.org
Betreff: Re: Problem with git-http-backend.exe as iis cgi

On Thu, 10 Mar 2016 07:28:50 +0000
Florian Manschwetus <manschwetus@cs-software-gmbh.de> wrote:

> I tried to setup git-http-backend with iis, as iis provides proper 
> impersonation for cgi under windows, which leads to have the 
> filesystem access performed with the logon user, therefore the 
> webserver doesn't need generic access to the files. I stumbled across 
> a problem, ending up with post requests hanging forever. After some 
> investigation I managed to get it work by wrapping the http-backend 
> into a bash script, giving a lot of control about the environmental 
> things, I was unable to solve within IIS configuration. The 
> workaround, I use currently, is to use "/bin/head -c ${CONTENT_LENGTH} 
> | ./git-http-backend.exe", which directly shows the issue. Git 
> http-backend should check if CONTENT_LENGTH is set to something 
> reasonable (e.g. >0) and should in this case read only CONTENT_LENGTH 
> bytes from stdin, instead of reading till EOF what I suspect it is 
> doing currently.

The rfc [1] states in its section 4.2:

| A request-body is supplied with the request if the CONTENT_LENGTH is 
| not NULL.  The server MUST make at least that many bytes available for 
| the script to read.  The server MAY signal an end-of-file condition 
| after CONTENT_LENGTH bytes have been read or it MAY supply extension 
| data.  Therefore, the script MUST NOT attempt to read more than 
| CONTENT_LENGTH bytes, even if more data is available.  However, it is 
| not obliged to read any of the data.

So yes, if Git currently reads until EOF, it's an error.
The correct way would be:

1) Check to see if the CONTENT_LENGTH variable is available in the
   environment.  If no, read nothing.

2) Otherwise read as many bytes it specifies, and no more.

1. https://www.ietf.org/rfc/rfc3875

[-- Attachment #2: http-backend-content-length.patch --]
[-- Type: application/octet-stream, Size: 1755 bytes --]

--- http-backend.c.orig	2016-03-29 07:29:32.694722500 +0200
+++ http-backend.c	2016-03-29 07:58:50.801482700 +0200
@@ -277,16 +277,32 @@
  */
 static ssize_t read_request(int fd, unsigned char **out)
 {
-	size_t len = 0, alloc = 8192;
-	unsigned char *buf = xmalloc(alloc);
+	unsigned char *buf = null;
+	size_t len = 0;
+	/* get request size */
+	size_t req_len = git_env_ulong("CONTENT_LENGTH",
+					   0);
+
+	/* check request size */
+	if (max_request_buffer < req_len) {
+		die("request was larger than our maximum size (%lu);"
+			    " try setting GIT_HTTP_MAX_REQUEST_BUFFER",
+			    max_request_buffer);
+	}
+
+	if (req_len <= 0) {
+		*out = null;
+		return 0;
+	}
+
+	/* allocate buffer */
+	buf = xmalloc(req_len)
 
-	if (max_request_buffer < alloc)
-		max_request_buffer = alloc;
 
 	while (1) {
 		ssize_t cnt;
 
-		cnt = read_in_full(fd, buf + len, alloc - len);
+		cnt = read_in_full(fd, buf + len, req_len - len);
 		if (cnt < 0) {
 			free(buf);
 			return -1;
@@ -294,21 +310,18 @@
 
 		/* partial read from read_in_full means we hit EOF */
 		len += cnt;
-		if (len < alloc) {
+		if (len < req_len) {
+			/* TODO request incomplete?? */
+			/* maybe just remove this block and condition along with the loop, */
+			/* if read_in_full is prooven reliable */
 			*out = buf;
 			return len;
+		} else {
+			/* request complete */
+			*out = buf;
+			return len;
+			
 		}
-
-		/* otherwise, grow and try again (if we can) */
-		if (alloc == max_request_buffer)
-			die("request was larger than our maximum size (%lu);"
-			    " try setting GIT_HTTP_MAX_REQUEST_BUFFER",
-			    max_request_buffer);
-
-		alloc = alloc_nr(alloc);
-		if (alloc > max_request_buffer)
-			alloc = max_request_buffer;
-		REALLOC_ARRAY(buf, alloc);
 	}
 }
 

Re: Problem with git-http-backend.exe as iis cgi

[Chris Packham]

Hi Florian

On Tue, Mar 29, 2016 at 7:01 PM, Florian Manschwetus
<manschwetus@cs-software-gmbh.de> wrote:
> Hi,
> I put together a first patch for the issue.
>
> Mit freundlichen Grüßen / With kind regards
> Florian Manschwetus
>
> E-Mail: manschwetus@cs-software-gmbh.de
> Tel.: +49-(0)611-8908534
>
> CS Software Concepts and Solutions GmbH
> Geschäftsführer / Managing director: Dr. Werner Alexi
> Amtsgericht Wiesbaden HRB 10004 (Commercial registry)
> Schiersteiner Straße 31
> D-65187 Wiesbaden
> Germany
> Tel.: 0611/8908555
>
>
> -----Ursprüngliche Nachricht-----
> Von: Konstantin Khomoutov [mailto:kostix+git@007spb.ru]
> Gesendet: Donnerstag, 10. März 2016 13:55
> An: Florian Manschwetus
> Cc: git@vger.kernel.org
> Betreff: Re: Problem with git-http-backend.exe as iis cgi
>
> On Thu, 10 Mar 2016 07:28:50 +0000
> Florian Manschwetus <manschwetus@cs-software-gmbh.de> wrote:
>
>> I tried to setup git-http-backend with iis, as iis provides proper
>> impersonation for cgi under windows, which leads to have the
>> filesystem access performed with the logon user, therefore the
>> webserver doesn't need generic access to the files. I stumbled across
>> a problem, ending up with post requests hanging forever. After some
>> investigation I managed to get it work by wrapping the http-backend
>> into a bash script, giving a lot of control about the environmental
>> things, I was unable to solve within IIS configuration. The
>> workaround, I use currently, is to use "/bin/head -c ${CONTENT_LENGTH}
>> | ./git-http-backend.exe", which directly shows the issue. Git
>> http-backend should check if CONTENT_LENGTH is set to something
>> reasonable (e.g. >0) and should in this case read only CONTENT_LENGTH
>> bytes from stdin, instead of reading till EOF what I suspect it is
>> doing currently.
>
> The rfc [1] states in its section 4.2:
>
> | A request-body is supplied with the request if the CONTENT_LENGTH is
> | not NULL.  The server MUST make at least that many bytes available for
> | the script to read.  The server MAY signal an end-of-file condition
> | after CONTENT_LENGTH bytes have been read or it MAY supply extension
> | data.  Therefore, the script MUST NOT attempt to read more than
> | CONTENT_LENGTH bytes, even if more data is available.  However, it is
> | not obliged to read any of the data.
>
> So yes, if Git currently reads until EOF, it's an error.
> The correct way would be:
>
> 1) Check to see if the CONTENT_LENGTH variable is available in the
>    environment.  If no, read nothing.
>
> 2) Otherwise read as many bytes it specifies, and no more.
>
> 1. https://www.ietf.org/rfc/rfc3875

Your patch description seems well thought out but if you want someone
to notice it you should have a read of
https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches

Re: Problem with git-http-backend.exe as iis cgi

[Junio C Hamano]

Konstantin Khomoutov <kostix+git@007spb.ru> writes:

> On Thu, 10 Mar 2016 07:28:50 +0000
> Florian Manschwetus <manschwetus@cs-software-gmbh.de> wrote:
>
>> I tried to setup git-http-backend with iis, as iis provides proper
>> impersonation for cgi under windows, which leads to have the
>> filesystem access performed with the logon user, therefore the
>> webserver doesn't need generic access to the files. I stumbled across
>> a problem, ending up with post requests hanging forever. After some
>> investigation I managed to get it work by wrapping the http-backend
>> into a bash script, giving a lot of control about the environmental
>> things, I was unable to solve within IIS configuration. The
>> workaround, I use currently, is to use "/bin/head -c
>> ${CONTENT_LENGTH} | ./git-http-backend.exe", which directly shows the
>> issue. Git http-backend should check if CONTENT_LENGTH is set to
>> something reasonable (e.g. >0) and should in this case read only
>> CONTENT_LENGTH bytes from stdin, instead of reading till EOF what I
>> suspect it is doing currently.
> ...
> So yes, if Git currently reads until EOF, it's an error.

This sounded vaguely familiar.  Isn't this responding to a stale
thread?

http://thread.gmane.org/gmane.comp.version-control.git/290114
proposed a patch along the line, and corrections to the patch was
suggested in the review, but it was not followed through, it seems.

Re: Problem with git-http-backend.exe as iis cgi

[Johannes Schindelin]

Hi Junio,

On Tue, 21 Jun 2016, Junio C Hamano wrote:

> Konstantin Khomoutov <kostix+git@007spb.ru> writes:

[purportedly on 10 Mar 13:55 2016, see $gmane/297739]

> Isn't this responding to a stale thread?

I was puzzled, too. I think the mail somehow got re-sent (and another one
by Konst, too).

Ciao,
Dscho