Git fork

remote-curl: error on incomplete packet

Currently, remote-curl acts as a proxy and blindly forwards packets
between an HTTP server and fetch-pack. In the case of a stateless RPC
connection where the connection is terminated with a partially written
packet, remote-curl will blindly send the partially written packet
before waiting on more input from fetch-pack. Meanwhile, fetch-pack will
read the partial packet and continue reading, expecting more input. This
results in a deadlock between the two processes.

For a stateless connection, inspect packets before sending them and
error out if a packet line packet is incomplete.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Denton Liu and committed by
Junio C Hamano
74b082ad 101736a1

+106 -3
+56 -3
remote-curl.c
··· 679 679 } 680 680 #endif 681 681 682 + struct check_pktline_state { 683 + char len_buf[4]; 684 + int len_filled; 685 + int remaining; 686 + }; 687 + 688 + static void check_pktline(struct check_pktline_state *state, const char *ptr, size_t size) 689 + { 690 + while (size) { 691 + if (!state->remaining) { 692 + int digits_remaining = 4 - state->len_filled; 693 + if (digits_remaining > size) 694 + digits_remaining = size; 695 + memcpy(&state->len_buf[state->len_filled], ptr, digits_remaining); 696 + state->len_filled += digits_remaining; 697 + ptr += digits_remaining; 698 + size -= digits_remaining; 699 + 700 + if (state->len_filled == 4) { 701 + state->remaining = packet_length(state->len_buf); 702 + if (state->remaining < 0) { 703 + die(_("remote-curl: bad line length character: %.4s"), state->len_buf); 704 + } else if (state->remaining < 4) { 705 + state->remaining = 0; 706 + } else { 707 + state->remaining -= 4; 708 + } 709 + state->len_filled = 0; 710 + } 711 + } 712 + 713 + if (state->remaining) { 714 + int remaining = state->remaining; 715 + if (remaining > size) 716 + remaining = size; 717 + ptr += remaining; 718 + size -= remaining; 719 + state->remaining -= remaining; 720 + } 721 + } 722 + } 723 + 682 724 struct rpc_in_data { 683 725 struct rpc_state *rpc; 684 726 struct active_request_slot *slot; 727 + int check_pktline; 728 + struct check_pktline_state pktline_state; 685 729 }; 686 730 687 731 /* ··· 702 746 return size; 703 747 if (size) 704 748 data->rpc->any_written = 1; 749 + if (data->check_pktline) 750 + check_pktline(&data->pktline_state, ptr, size); 705 751 write_or_die(data->rpc->in, ptr, size); 706 752 return size; 707 753 } ··· 778 824 * If flush_received is true, do not attempt to read any more; just use what's 779 825 * in rpc->buf. 780 826 */ 781 - static int post_rpc(struct rpc_state *rpc, int flush_received) 827 + static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_received) 782 828 { 783 829 struct active_request_slot *slot; 784 830 struct curl_slist *headers = http_copy_default_headers(); ··· 920 966 curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in); 921 967 rpc_in_data.rpc = rpc; 922 968 rpc_in_data.slot = slot; 969 + rpc_in_data.check_pktline = stateless_connect; 970 + memset(&rpc_in_data.pktline_state, 0, sizeof(rpc_in_data.pktline_state)); 923 971 curl_easy_setopt(slot->curl, CURLOPT_FILE, &rpc_in_data); 924 972 curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0); 925 973 ··· 936 984 if (!rpc->any_written) 937 985 err = -1; 938 986 987 + if (rpc_in_data.pktline_state.len_filled) 988 + err = error(_("%d bytes of length header were received"), rpc_in_data.pktline_state.len_filled); 989 + if (rpc_in_data.pktline_state.remaining) 990 + err = error(_("%d bytes of body are still expected"), rpc_in_data.pktline_state.remaining); 991 + 939 992 curl_slist_free_all(headers); 940 993 free(gzip_body); 941 994 return err; ··· 985 1038 break; 986 1039 rpc->pos = 0; 987 1040 rpc->len = n; 988 - err |= post_rpc(rpc, 0); 1041 + err |= post_rpc(rpc, 0, 0); 989 1042 } 990 1043 991 1044 close(client.in); ··· 1342 1395 BUG("The entire rpc->buf should be larger than LARGE_PACKET_MAX"); 1343 1396 if (status == PACKET_READ_EOF) 1344 1397 break; 1345 - if (post_rpc(&rpc, status == PACKET_READ_FLUSH)) 1398 + if (post_rpc(&rpc, 1, status == PACKET_READ_FLUSH)) 1346 1399 /* We would have an err here */ 1347 1400 break; 1348 1401 /* Reset the buffer for next request */
+2
t/lib-httpd.sh
··· 129 129 prepare_httpd() { 130 130 mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH" 131 131 cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH" 132 + install_script incomplete-length-upload-pack-v2-http.sh 133 + install_script incomplete-body-upload-pack-v2-http.sh 132 134 install_script broken-smart-http.sh 133 135 install_script error-smart-http.sh 134 136 install_script error.sh
+8
t/lib-httpd/apache.conf
··· 117 117 SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} 118 118 SetEnv GIT_HTTP_EXPORT_ALL 119 119 </LocationMatch> 120 + ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/ 121 + ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/ 120 122 ScriptAliasMatch /error_git_upload_pack/(.*)/git-upload-pack error.sh/ 121 123 ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1 122 124 ScriptAlias /broken_smart/ broken-smart-http.sh/ ··· 126 128 <Directory ${GIT_EXEC_PATH}> 127 129 Options FollowSymlinks 128 130 </Directory> 131 + <Files incomplete-length-upload-pack-v2-http.sh> 132 + Options ExecCGI 133 + </Files> 134 + <Files incomplete-body-upload-pack-v2-http.sh> 135 + Options ExecCGI 136 + </Files> 129 137 <Files broken-smart-http.sh> 130 138 Options ExecCGI 131 139 </Files>
+3
t/lib-httpd/incomplete-body-upload-pack-v2-http.sh
··· 1 + printf "Content-Type: text/%s\n" "application/x-git-upload-pack-result" 2 + echo 3 + printf "%s%s" "0079" "45"
+3
t/lib-httpd/incomplete-length-upload-pack-v2-http.sh
··· 1 + printf "Content-Type: text/%s\n" "application/x-git-upload-pack-result" 2 + echo 3 + printf "%s" "00"
+34
t/t5702-protocol-v2.sh
··· 586 586 ! grep "Send header: Transfer-Encoding: chunked" log 587 587 ' 588 588 589 + test_expect_success 'clone repository with http:// using protocol v2 with incomplete pktline length' ' 590 + test_when_finished "rm -f log" && 591 + 592 + git init "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_length" && 593 + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_length" file && 594 + 595 + test_must_fail env GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" git -c protocol.version=2 \ 596 + clone "$HTTPD_URL/smart/incomplete_length" incomplete_length_child 2>err && 597 + 598 + # Client requested to use protocol v2 599 + grep "Git-Protocol: version=2" log && 600 + # Server responded using protocol v2 601 + grep "git< version 2" log && 602 + # Client reported appropriate failure 603 + test_i18ngrep "bytes of length header were received" err 604 + ' 605 + 606 + test_expect_success 'clone repository with http:// using protocol v2 with incomplete pktline body' ' 607 + test_when_finished "rm -f log" && 608 + 609 + git init "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_body" && 610 + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/incomplete_body" file && 611 + 612 + test_must_fail env GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" git -c protocol.version=2 \ 613 + clone "$HTTPD_URL/smart/incomplete_body" incomplete_body_child 2>err && 614 + 615 + # Client requested to use protocol v2 616 + grep "Git-Protocol: version=2" log && 617 + # Server responded using protocol v2 618 + grep "git< version 2" log && 619 + # Client reported appropriate failure 620 + test_i18ngrep "bytes of body are still expected" err 621 + ' 622 + 589 623 test_expect_success 'clone big repository with http:// using protocol v2' ' 590 624 test_when_finished "rm -f log" && 591 625