@matheusbsbs / matheus-bagliano / commits / 6bc0cb5176

http-backend: spool ref negotiation requests to buffer

When http-backend spawns "upload-pack" to do ref negotiation, it streams the http request body to upload-pack, who then streams the http response back to the client as it reads. In theory, git can go full-duplex; the client can consume our response while it is still sending the request. In practice, however, HTTP is a half-duplex protocol. Even if our client is ready to read and write simultaneously, we may have other HTTP infrastructure in the way, including the webserver that spawns our CGI, or any intermediate proxies. In at least one documented case[1], this leads to deadlock when trying a fetch over http. What happens is basically: 1. Apache proxies the request to the CGI, http-backend. 2. http-backend gzip-inflates the data and sends the result to upload-pack. 3. upload-pack acts on the data and generates output over the pipe back to Apache. Apache isn't reading because it's busy writing (step 1). This works fine most of the time, because the upload-pack output ends up in a system pipe buffer, and Apache reads it as soon as it finishes writing. But if both the request and the response exceed the system pipe buffer size, then we deadlock (Apache blocks writing to http-backend, http-backend blocks writing to upload-pack, and upload-pack blocks writing to Apache). We need to break the deadlock by spooling either the input or the output. In this case, it's ideal to spool the input, because Apache does not start reading either stdout _or_ stderr until we have consumed all of the input. So until we do so, we cannot even get an error message out to the client. The solution is fairly straight-forward: we read the request body into an in-memory buffer in http-backend, freeing up Apache, and then feed the data ourselves to upload-pack. But there are a few important things to note: 1. We limit the in-memory buffer to prevent an obvious denial-of-service attack. This is a new hard limit on requests, but it's unlikely to come into play. The default value is 10MB, which covers even the ridiculous 100,000-ref negotation in the included test (that actually caps out just over 5MB). But it's configurable on the off chance that you don't mind spending some extra memory to make even ridiculous requests work. 2. We must take care only to buffer when we have to. For pushes, the incoming packfile may be of arbitrary size, and we should connect the input directly to receive-pack. There's no deadlock problem here, though, because we do not produce any output until the whole packfile has been read. For upload-pack's initial ref advertisement, we similarly do not need to buffer. Even though we may generate a lot of output, there is no request body at all (i.e., it is a GET, not a POST). [1] http://article.gmane.org/gmane.comp.version-control.git/269020 Test-adapted-from: Dennis Kaarsemaker <dennis@kaarsemaker.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 20, 2015 at 03:37 UTC 6bc0cb5176a5e42ca4a74e3558e8f0790ed09bb1

3 files changed +105 -11

Documentation/git-http-backend.txt

@@ -255,6 +255,15 @@ The GIT_HTTP_EXPORT_ALL environmental variable may be passed to

255 'git-http-backend' to bypass the check for the "git-daemon-export-ok"

256 file in each repository before allowing export of that repository.

257

258 +The `GIT_HTTP_MAX_REQUEST_BUFFER` environment variable (or the

259 +`http.maxRequestBuffer` config variable) may be set to change the

260 +largest ref negotiation request that git will handle during a fetch; any

261 +fetch requiring a larger buffer will not succeed. This value should not

262 +normally need to be changed, but may be helpful if you are fetching from

263 +a repository with an extremely large number of refs. The value can be

264 +specified with a unit (e.g., `100M` for 100 megabytes). The default is

265 +10 megabytes.

266 +

267 The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and

268 GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}',

269 ensuring that any reflogs created by 'git-receive-pack' contain some

http-backend.c

+85 -11

@@ -13,18 +13,20 @@ static const char content_type[] = "Content-Type";

13 static const char content_length[] = "Content-Length";

14 static const char last_modified[] = "Last-Modified";

15 static int getanyfile = 1;

16 +static unsigned long max_request_buffer = 10 * 1024 * 1024;

18 static struct string_list *query_params;

20 struct rpc_service {

21 const char *name;

22 const char *config_name;

23 + unsigned buffer_input : 1;

24 signed enabled : 2;

25 };

27 static struct rpc_service rpc_service[] = {

26 - { "upload-pack", "uploadpack", 1 },

27 - { "receive-pack", "receivepack", -1 },

28 + { "upload-pack", "uploadpack", 1, 1 },

29 + { "receive-pack", "receivepack", 0, -1 },

30 };

32 static struct string_list *get_parameters(void)

@@ -225,6 +227,7 @@ static void http_config(void)

227 struct strbuf var = STRBUF_INIT;

228

229 git_config_get_bool("http.getanyfile", &getanyfile);

230 + git_config_get_ulong("http.maxrequestbuffer", &max_request_buffer);

231

232 for (i = 0; i < ARRAY_SIZE(rpc_service); i++) {

233 struct rpc_service *svc = &rpc_service[i];

@@ -266,9 +269,52 @@ static struct rpc_service *select_service(const char *name)

269 return svc;

270 }

271

269 -static void inflate_request(const char *prog_name, int out)

272 +/*

273 + * This is basically strbuf_read(), except that if we

274 + * hit max_request_buffer we die (we'd rather reject a

275 + * maliciously large request than chew up infinite memory).

276 + */

277 +static ssize_t read_request(int fd, unsigned char **out)

278 +{

279 + size_t len = 0, alloc = 8192;

280 + unsigned char *buf = xmalloc(alloc);

281 +

282 + if (max_request_buffer < alloc)

283 + max_request_buffer = alloc;

284 +

285 + while (1) {

286 + ssize_t cnt;

287 +

288 + cnt = read_in_full(fd, buf + len, alloc - len);

289 + if (cnt < 0) {

290 + free(buf);

291 + return -1;

292 + }

293 +

294 + /* partial read from read_in_full means we hit EOF */

295 + len += cnt;

296 + if (len < alloc) {

297 + *out = buf;

298 + return len;

299 + }

300 +

301 + /* otherwise, grow and try again (if we can) */

302 + if (alloc == max_request_buffer)

303 + die("request was larger than our maximum size (%lu);"

304 + " try setting GIT_HTTP_MAX_REQUEST_BUFFER",

305 + max_request_buffer);

306 +

307 + alloc = alloc_nr(alloc);

308 + if (alloc > max_request_buffer)

309 + alloc = max_request_buffer;

310 + REALLOC_ARRAY(buf, alloc);

311 + }

312 +}

313 +

314 +static void inflate_request(const char *prog_name, int out, int buffer_input)

315 {

316 git_zstream stream;

317 + unsigned char *full_request = NULL;

318 unsigned char in_buf[8192];

319 unsigned char out_buf[8192];

320 unsigned long cnt = 0;

@@ -277,11 +323,21 @@ static void inflate_request(const char *prog_name, int out)

323 git_inflate_init_gzip_only(&stream);

324

325 while (1) {

280 - ssize_t n = xread(0, in_buf, sizeof(in_buf));

326 + ssize_t n;

327 +

328 + if (buffer_input) {

329 + if (full_request)

330 + n = 0; /* nothing left to read */

331 + else

332 + n = read_request(0, &full_request);

333 + stream.next_in = full_request;

334 + } else {

335 + n = xread(0, in_buf, sizeof(in_buf));

336 + stream.next_in = in_buf;

337 + }

338 +

339 if (n <= 0)

340 die("request ended in the middle of the gzip stream");

283 -

284 - stream.next_in = in_buf;

341 stream.avail_in = n;

342

343 while (0 < stream.avail_in) {

@@ -307,9 +363,22 @@ static void inflate_request(const char *prog_name, int out)

363 done:

364 git_inflate_end(&stream);

365 close(out);

366 + free(full_request);

367 +}

368 +

369 +static void copy_request(const char *prog_name, int out)

370 +{

371 + unsigned char *buf;

372 + ssize_t n = read_request(0, &buf);

373 + if (n < 0)

374 + die_errno("error reading request body");

375 + if (write_in_full(out, buf, n) != n)

376 + die("%s aborted reading request", prog_name);

377 + close(out);

378 + free(buf);

379 }

380

312 -static void run_service(const char **argv)

381 +static void run_service(const char **argv, int buffer_input)

382 {

383 const char *encoding = getenv("HTTP_CONTENT_ENCODING");

384 const char *user = getenv("REMOTE_USER");

@@ -334,7 +403,7 @@ static void run_service(const char **argv)

403 "GIT_COMMITTER_EMAIL=%s@http.%s", user, host);

404

405 cld.argv = argv;

337 - if (gzipped_request)

406 + if (buffer_input || gzipped_request)

407 cld.in = -1;

408 cld.git_cmd = 1;

409 if (start_command(&cld))

@@ -342,7 +411,9 @@ static void run_service(const char **argv)

411

412 close(1);

413 if (gzipped_request)

345 - inflate_request(argv[0], cld.in);

414 + inflate_request(argv[0], cld.in, buffer_input);

415 + else if (buffer_input)

416 + copy_request(argv[0], cld.in);

417 else

418 close(0);

419

@@ -392,7 +463,7 @@ static void get_info_refs(char *arg)

463 packet_flush(1);

464

465 argv[0] = svc->name;

395 - run_service(argv);

466 + run_service(argv, 0);

467

468 } else {

469 select_getanyfile();

@@ -496,7 +567,7 @@ static void service_rpc(char *service_name)

567 end_headers();

568

569 argv[0] = svc->name;

499 - run_service(argv);

570 + run_service(argv, svc->buffer_input);

571 strbuf_release(&buf);

572 }

573

@@ -623,6 +694,9 @@ int main(int argc, char **argv)

694 not_found("Repository not exported: '%s'", dir);

695

696 http_config();

697 + max_request_buffer = git_env_ulong("GIT_HTTP_MAX_REQUEST_BUFFER",

698 + max_request_buffer);

699 +

700 cmd->imp(cmd_arg);

701 return 0;

702 }

t/t5551-http-fetch-smart.sh

+11

@@ -253,5 +253,16 @@ test_expect_success EXPENSIVE 'clone the 50,000 tag repo to check OS command lin

253 )

254 '

255

256 +test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '

257 + git -C too-many-refs fetch -q --tags &&

258 + (

259 + cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&

260 + create_tags 50001 100000

261 + ) &&

262 + git -C too-many-refs fetch -q --tags &&

263 + git -C too-many-refs for-each-ref refs/tags >tags &&

264 + test_line_count = 100000 tags

265 +'

266 +

267 stop_httpd

268 test_done