{"thread":{"id":"37381","subject":"[PATCH 00/18] Signed push","startedAt":"2014-08-19T22:06:09Z","lastAt":"2014-09-05T02:41:48Z","messageCount":59,"participants":["Junio C Hamano","Duy Nguyen","Nico Williams","brian m. carlson","Bert Wesarg","David Turner","Shawn Pearce","Kyle J. McKay","Stefan Beller","Jason Pyeron"],"isPatch":true,"patchVersion":1,"patchTotal":18},"messages":[{"id":"247969","messageId":"1408485987-3590-1-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":null,"subject":"[PATCH 00/18] Signed push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:09Z","receivedAt":"2014-08-19T22:06:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While signed tags and commits assert that the objects thusly signed\ncame from you, who signed these objects, there is not a good way to\nassert that you wanted to have a particular object at the tip of a\nparticular branch. My signing v2.0.1 tag only means I want to call\nthe version v2.0.1, and it does not mean I want to push it out to my\n'master' branch---it is likely that I only want it in 'maint', so\nthe signature on the object alone is insufficient.\n\nThe only assurance to you that 'maint' points at what I wanted to\nplace there comes from your trust on the hosting site and my\nauthentication with it, which cannot easily audited later.\n\nThis series introduces a cryptographic assurance for ref updates\ndone by \"git push\" by introducing a mechanism that allows you to\nsign a \"push certificate\" (for the lack of better name) every time\nyou push. Think of it as working on an axis orthogonal to the\ntraditional \"signed tags\".\n\nThe most interesting part starts at 15/18; everything that precedes\nthat patch are preparatory clean-ups.\n\n [PATCH 15/18] the beginning of the signed push\n\n\tThis step presents the basic idea. If you remember the\n\tunderlying \"git push\" protocol exchange, it goes like this:\n\n\t- The server advertises the existing refs and where they\n\t point at, and the capabilities the server supports;\n\n\t- The \"git push\" client sends update commands (one or more\n\t \"old-sha1 new-sha1 refname\") followed by the pack data;\n\n\t- The server unpacks and updates the refname to point at\n\t new-sha1.\n\n\tWe introduce \"push-cert\" capability, and allow the client to\n\tsign the \"update commands\" it will send to the server and\n\tsend this signature using the new \"push-cert\" command.\n\n\tThis certificate is exported to the pre/post-receive hooks\n\tof the server, so that the pre-receive hook can GPG validate\n\t(and possibly reject a bad push); post-receive hook can log\n\tthe certificate.\n\n [PATCH 16/18] receive-pack: GPG-validate push certificates\n\n\tThis step builds a native GPG validation into the server to\n\thelp the pre-receive hook. The signature is verified\n\tagainst the GPG keychain the server uses (it is likely that\n\tyou would want to set and export GNUPGHOME when starting\n\tyour server), and verification result is given to the\n\tpre/post-receive hook.\n\n [PATCH 17/18] send-pack: send feature request on push-cert packet\n [PATCH 18/18] signed push: final protocol update\n\n\tWith the protocol introduced in 15/18, the update commands\n\tand the push certificate record the same information twice;\n\tthe protocol was kept inefficient to make it easier to\n\treview the changes.\n\n\tThese two steps updates the protocol to the final version,\n\twhich does not to send the update commands when a push\n\tcertificate is in use.\n\nIf the server's GPG keychain and pre-receive hook are properly set\nup, a \"git push --signed\" over an unauthenticated and unencrypted\ncommunication channel (aka \"git daemon\") can be made as secure as,\nand even more secure than, the authenticated \"git push ssh://\".\n\nWith the signed push certificate, together with the connectivity\ncheck done when the server accepts the packed data, we are assured\nthat the trusted user vouches for the history leading to the\nproposed tips of refs (aka \"new-sha1\"s), and a man-in-the-middle\nwould not be able to make the server accept an update altered in\ntransit.\n\n\nJunio C Hamano (18):\n receive-pack: do not overallocate command structure\n receive-pack: parse feature request a bit earlier\n receive-pack: do not reuse old_sha1[] to other things\n receive-pack: factor out queueing of command\n send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher\n send-pack: refactor decision to send update per ref\n send-pack: always send capabilities\n send-pack: factor out capability string generation\n send-pack: rename \"new_refs\" to \"need_pack_data\"\n send-pack: refactor inspecting and resetting status and sending commands\n send-pack: clarify that cmds_sent is a boolean\n gpg-interface: move parse_gpg_output() to where it should be\n gpg-interface: move parse_signature() to where it should be\n pack-protocol doc: typofix for PKT-LINE\n the beginning of the signed push\n receive-pack: GPG-validate push certificates\n send-pack: send feature request on push-cert packet\n signed push: final protocol update\n\n Documentation/git-push.txt | 9 +-\n Documentation/git-receive-pack.txt | 30 +++-\n Documentation/technical/pack-protocol.txt | 24 ++-\n Documentation/technical/protocol-capabilities.txt | 12 +-\n builtin/push.c | 1 +\n builtin/receive-pack.c | 161 +++++++++++++++---\n commit.c | 36 -----\n gpg-interface.c | 57 +++++++\n gpg-interface.h | 18 ++-\n send-pack.c | 188 ++++++++++++++++------\n send-pack.h | 1 +\n t/t5534-push-signed.sh | 77 +++++++++\n tag.c | 20 ---\n tag.h | 1 -\n transport.c | 4 +\n transport.h | 5 +\n 16 files changed, 502 insertions(+), 142 deletions(-)\n create mode 100755 t/t5534-push-signed.sh\n\n-- \n2.1.0-301-g54593e2\n"},{"id":"247970","messageId":"1408485987-3590-2-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 01/18] receive-pack: do not overallocate command structure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:10Z","receivedAt":"2014-08-19T22:06:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"An \"update\" command in the protocol exchange consists of 40-hex old\nobject name, SP, 40-hex new object name, SP, and a refname, but the\nfirst instance is further followed by a NUL with feature requests.\n\nThe command structure, which has a flex-array member that stores the\nrefname at the end, was allocated based on the whole length of the\nupdate command, without excluding the trailing feature requests.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/receive-pack.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex f93ac45..1663beb 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -872,10 +872,11 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n \t\t\t\tquiet = 1;\n \t\t}\n-\t\tcmd = xcalloc(1, sizeof(struct command) + len - 80);\n+\t\tcmd = xcalloc(1, sizeof(struct command) + reflen + 1);\n \t\thashcpy(cmd->old_sha1, old_sha1);\n \t\thashcpy(cmd->new_sha1, new_sha1);\n-\t\tmemcpy(cmd->ref_name, line + 82, len - 81);\n+\t\tmemcpy(cmd->ref_name, refname, reflen);\n+\t\tcmd->ref_name[reflen] = '\\0';\n \t\t*p = cmd;\n \t\tp = &cmd->next;\n \t}\n-- \n2.1.0-301-g54593e2\n"},{"id":"247971","messageId":"1408485987-3590-3-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 02/18] receive-pack: parse feature request a bit earlier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:11Z","receivedAt":"2014-08-19T22:06:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ideally, we should have also allowed the first \"shallow\" to carry\nthe feature request trailer, but that is water under the bridge\nnow. This makes the next step to factor out the queuing of commands\neasier to review.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/receive-pack.c | 24 +++++++++++++-----------\n 1 file changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 1663beb..43f35c4 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -840,7 +840,7 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\tunsigned char old_sha1[20], new_sha1[20];\n \t\tstruct command *cmd;\n \t\tchar *refname;\n-\t\tint len, reflen;\n+\t\tint len, reflen, linelen;\n \n \t\tline = packet_read_line(0, &len);\n \t\tif (!line)\n@@ -853,7 +853,18 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (len < 83 ||\n+\t\tlinelen = strlen(line);\n+\t\tif (linelen < len) {\n+\t\t\tconst char *feature_list = line + linelen + 1;\n+\t\t\tif (parse_feature_request(feature_list, \"report-status\"))\n+\t\t\t\treport_status = 1;\n+\t\t\tif (parse_feature_request(feature_list, \"side-band-64k\"))\n+\t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n+\t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n+\t\t\t\tquiet = 1;\n+\t\t}\n+\n+\t\tif (linelen < 83 ||\n \t\t line[40] != ' ' ||\n \t\t line[81] != ' ' ||\n \t\t get_sha1_hex(line, old_sha1) ||\n@@ -863,15 +874,6 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \n \t\trefname = line + 82;\n \t\treflen = strlen(refname);\n-\t\tif (reflen + 82 < len) {\n-\t\t\tconst char *feature_list = refname + reflen + 1;\n-\t\t\tif (parse_feature_request(feature_list, \"report-status\"))\n-\t\t\t\treport_status = 1;\n-\t\t\tif (parse_feature_request(feature_list, \"side-band-64k\"))\n-\t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n-\t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n-\t\t\t\tquiet = 1;\n-\t\t}\n \t\tcmd = xcalloc(1, sizeof(struct command) + reflen + 1);\n \t\thashcpy(cmd->old_sha1, old_sha1);\n \t\thashcpy(cmd->new_sha1, new_sha1);\n-- \n2.1.0-301-g54593e2\n"},{"id":"247972","messageId":"1408485987-3590-4-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 03/18] receive-pack: do not reuse old_sha1[] to other things","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:12Z","receivedAt":"2014-08-19T22:06:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This piece of code reads object names of shallow boundaries,\nnot old_sha1[].\n\nSigned-off-by: Junio C Hamano \n---\n builtin/receive-pack.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 43f35c4..ee855b4 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -847,9 +847,11 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\t\tbreak;\n \n \t\tif (len == 48 && starts_with(line, \"shallow \")) {\n-\t\t\tif (get_sha1_hex(line + 8, old_sha1))\n-\t\t\t\tdie(\"protocol error: expected shallow sha, got '%s'\", line + 8);\n-\t\t\tsha1_array_append(shallow, old_sha1);\n+\t\t\tunsigned char sha1[20];\n+\t\t\tif (get_sha1_hex(line + 8, sha1))\n+\t\t\t\tdie(\"protocol error: expected shallow sha, got '%s'\",\n+\t\t\t\t line + 8);\n+\t\t\tsha1_array_append(shallow, sha1);\n \t\t\tcontinue;\n \t\t}\n \n-- \n2.1.0-301-g54593e2\n"},{"id":"247973","messageId":"1408485987-3590-5-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 04/18] receive-pack: factor out queueing of command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:13Z","receivedAt":"2014-08-19T22:06:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Make a helper function to accept a line of a protocol message and\nqueue an update command out of the code from read_head_info().\n\nSigned-off-by: Junio C Hamano \n---\n builtin/receive-pack.c | 50 +++++++++++++++++++++++++++++---------------------\n 1 file changed, 29 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ee855b4..341bb46 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -831,16 +831,40 @@ static void execute_commands(struct command *commands,\n \t\t \"the reported refs above\");\n }\n \n+static struct command **queue_command(struct command **p,\n+\t\t\t\t const char *line,\n+\t\t\t\t int linelen)\n+{\n+\tunsigned char old_sha1[20], new_sha1[20];\n+\tstruct command *cmd;\n+\tconst char *refname;\n+\tint reflen;\n+\n+\tif (linelen < 83 ||\n+\t line[40] != ' ' ||\n+\t line[81] != ' ' ||\n+\t get_sha1_hex(line, old_sha1) ||\n+\t get_sha1_hex(line + 41, new_sha1))\n+\t\tdie(\"protocol error: expected old/new/ref, got '%s'\", line);\n+\n+\trefname = line + 82;\n+\treflen = linelen - 82;\n+\tcmd = xcalloc(1, sizeof(struct command) + reflen + 1);\n+\thashcpy(cmd->old_sha1, old_sha1);\n+\thashcpy(cmd->new_sha1, new_sha1);\n+\tmemcpy(cmd->ref_name, refname, reflen);\n+\tcmd->ref_name[reflen] = '\\0';\n+\t*p = cmd;\n+\treturn &cmd->next;\n+}\n+\n static struct command *read_head_info(struct sha1_array *shallow)\n {\n \tstruct command *commands = NULL;\n \tstruct command **p = &commands;\n \tfor (;;) {\n \t\tchar *line;\n-\t\tunsigned char old_sha1[20], new_sha1[20];\n-\t\tstruct command *cmd;\n-\t\tchar *refname;\n-\t\tint len, reflen, linelen;\n+\t\tint len, linelen;\n \n \t\tline = packet_read_line(0, &len);\n \t\tif (!line)\n@@ -866,23 +890,7 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\t\t\tquiet = 1;\n \t\t}\n \n-\t\tif (linelen < 83 ||\n-\t\t line[40] != ' ' ||\n-\t\t line[81] != ' ' ||\n-\t\t get_sha1_hex(line, old_sha1) ||\n-\t\t get_sha1_hex(line + 41, new_sha1))\n-\t\t\tdie(\"protocol error: expected old/new/ref, got '%s'\",\n-\t\t\t line);\n-\n-\t\trefname = line + 82;\n-\t\treflen = strlen(refname);\n-\t\tcmd = xcalloc(1, sizeof(struct command) + reflen + 1);\n-\t\thashcpy(cmd->old_sha1, old_sha1);\n-\t\thashcpy(cmd->new_sha1, new_sha1);\n-\t\tmemcpy(cmd->ref_name, refname, reflen);\n-\t\tcmd->ref_name[reflen] = '\\0';\n-\t\t*p = cmd;\n-\t\tp = &cmd->next;\n+\t\tp = queue_command(p, line, linelen);\n \t}\n \treturn commands;\n }\n-- \n2.1.0-301-g54593e2\n"},{"id":"247974","messageId":"1408485987-3590-6-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 05/18] send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:14Z","receivedAt":"2014-08-19T22:06:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"20e8b465 (refactor ref status logic for pushing, 2010-01-08)\nrestructured the code to set status for each ref to be pushed, but\ndid not quite go far enough. We inspect the status set earlier by\nset_refs_status_for_push() and then perform yet another update to\nthe status of a ref with an otherwise OK status to be deleted to\nmark it with REF_STATUS_REJECT_NODELETE when the protocol tells us\nnever to delete.\n\nSplit the latter into a separate loop that comes before we enter the\nper-ref loop. This way we would have one less condition to check in\nthe main loop.\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 6129b0f..7428ae6 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -231,6 +231,15 @@ int send_pack(struct send_pack_args *args,\n \t\treturn 0;\n \t}\n \n+\t/*\n+\t * NEEDSWORK: why is delete-refs so specific to send-pack\n+\t * machinery that set_ref_status_for_push() cannot set this\n+\t * bit for us???\n+\t */\n+\tfor (ref = remote_refs; ref; ref = ref->next)\n+\t\tif (ref->deletion && !allow_deleting_refs)\n+\t\t\tref->status = REF_STATUS_REJECT_NODELETE;\n+\n \tif (!args->dry_run)\n \t\tadvertise_shallow_grafts_buf(&req_buf);\n \n@@ -249,17 +258,13 @@ int send_pack(struct send_pack_args *args,\n \t\tcase REF_STATUS_REJECT_FETCH_FIRST:\n \t\tcase REF_STATUS_REJECT_NEEDS_FORCE:\n \t\tcase REF_STATUS_REJECT_STALE:\n+\t\tcase REF_STATUS_REJECT_NODELETE:\n \t\tcase REF_STATUS_UPTODATE:\n \t\t\tcontinue;\n \t\tdefault:\n \t\t\t; /* do nothing */\n \t\t}\n \n-\t\tif (ref->deletion && !allow_deleting_refs) {\n-\t\t\tref->status = REF_STATUS_REJECT_NODELETE;\n-\t\t\tcontinue;\n-\t\t}\n-\n \t\tif (!ref->deletion)\n \t\t\tnew_refs++;\n \n-- \n2.1.0-301-g54593e2\n"},{"id":"247975","messageId":"1408485987-3590-7-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 06/18] send-pack: refactor decision to send update per ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:15Z","receivedAt":"2014-08-19T22:06:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A new helper function ref_update_to_be_sent() decides for each ref\nif the update is to be sent based on the status previously set by\nset_ref_status_for_push() and also if this is a mirrored push.\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 36 +++++++++++++++++++++---------------\n 1 file changed, 21 insertions(+), 15 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 7428ae6..f3c5ebe 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -190,6 +190,26 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)\n \tfor_each_commit_graft(advertise_shallow_grafts_cb, sb);\n }\n \n+static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)\n+{\n+\tif (!ref->peer_ref && !args->send_mirror)\n+\t\treturn 0;\n+\n+\t/* Check for statuses set by set_ref_status_for_push() */\n+\tswitch (ref->status) {\n+\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n+\tcase REF_STATUS_REJECT_ALREADY_EXISTS:\n+\tcase REF_STATUS_REJECT_FETCH_FIRST:\n+\tcase REF_STATUS_REJECT_NEEDS_FORCE:\n+\tcase REF_STATUS_REJECT_STALE:\n+\tcase REF_STATUS_REJECT_NODELETE:\n+\tcase REF_STATUS_UPTODATE:\n+\t\treturn 0;\n+\tdefault:\n+\t\treturn 1;\n+\t}\n+}\n+\n int send_pack(struct send_pack_args *args,\n \t int fd[], struct child_process *conn,\n \t struct ref *remote_refs,\n@@ -248,23 +268,9 @@ int send_pack(struct send_pack_args *args,\n \t */\n \tnew_refs = 0;\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n-\t\tif (!ref->peer_ref && !args->send_mirror)\n+\t\tif (!ref_update_to_be_sent(ref, args))\n \t\t\tcontinue;\n \n-\t\t/* Check for statuses set by set_ref_status_for_push() */\n-\t\tswitch (ref->status) {\n-\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n-\t\tcase REF_STATUS_REJECT_ALREADY_EXISTS:\n-\t\tcase REF_STATUS_REJECT_FETCH_FIRST:\n-\t\tcase REF_STATUS_REJECT_NEEDS_FORCE:\n-\t\tcase REF_STATUS_REJECT_STALE:\n-\t\tcase REF_STATUS_REJECT_NODELETE:\n-\t\tcase REF_STATUS_UPTODATE:\n-\t\t\tcontinue;\n-\t\tdefault:\n-\t\t\t; /* do nothing */\n-\t\t}\n-\n \t\tif (!ref->deletion)\n \t\t\tnew_refs++;\n \n-- \n2.1.0-301-g54593e2\n"},{"id":"247976","messageId":"1408485987-3590-8-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 07/18] send-pack: always send capabilities","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:16Z","receivedAt":"2014-08-19T22:06:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We tried to avoid sending one extra byte, NUL and nothing behind it\nto signal there is no protocol capabilities being sent, on the first\ncommand packet on the wire, but it just made the code look ugly.\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex f3c5ebe..2fa6c34 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -281,8 +281,7 @@ int send_pack(struct send_pack_args *args,\n \t\t\tchar *new_hex = sha1_to_hex(ref->new_sha1);\n \t\t\tint quiet = quiet_supported && (args->quiet || !args->progress);\n \n-\t\t\tif (!cmds_sent && (status_report || use_sideband ||\n-\t\t\t\t\t quiet || agent_supported)) {\n+\t\t\tif (!cmds_sent)\n \t\t\t\tpacket_buf_write(&req_buf,\n \t\t\t\t\t\t \"%s %s %s%c%s%s%s%s%s\",\n \t\t\t\t\t\t old_hex, new_hex, ref->name, 0,\n@@ -292,7 +291,6 @@ int send_pack(struct send_pack_args *args,\n \t\t\t\t\t\t agent_supported ? \" agent=\" : \"\",\n \t\t\t\t\t\t agent_supported ? git_user_agent_sanitized() : \"\"\n \t\t\t\t\t\t);\n-\t\t\t}\n \t\t\telse\n \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n \t\t\t\t\t\t old_hex, new_hex, ref->name);\n-- \n2.1.0-301-g54593e2\n"},{"id":"247977","messageId":"1408485987-3590-9-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 08/18] send-pack: factor out capability string generation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:17Z","receivedAt":"2014-08-19T22:06:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A run of 'var ? \" var\" : \"\"' fed to a long printf string in a deeply\nnested block was hard to read. Move it outside the loop and format\nit into a strbuf.\n\nAs an added bonus, the trick to add \"agent=\" by using\ntwo conditionals is replaced by a more readable version.\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 21 +++++++++++++--------\n 1 file changed, 13 insertions(+), 8 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 2fa6c34..c1c64ee 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -218,6 +218,7 @@ int send_pack(struct send_pack_args *args,\n \tint in = fd[0];\n \tint out = fd[1];\n \tstruct strbuf req_buf = STRBUF_INIT;\n+\tstruct strbuf cap_buf = STRBUF_INIT;\n \tstruct ref *ref;\n \tint new_refs;\n \tint allow_deleting_refs = 0;\n@@ -251,6 +252,15 @@ int send_pack(struct send_pack_args *args,\n \t\treturn 0;\n \t}\n \n+\tif (status_report)\n+\t\tstrbuf_addstr(&cap_buf, \" report-status\");\n+\tif (use_sideband)\n+\t\tstrbuf_addstr(&cap_buf, \" side-band-64k\");\n+\tif (quiet_supported && (args->quiet || !args->progress))\n+\t\tstrbuf_addstr(&cap_buf, \" quiet\");\n+\tif (agent_supported)\n+\t\tstrbuf_addf(&cap_buf, \" agent=%s\", git_user_agent_sanitized());\n+\n \t/*\n \t * NEEDSWORK: why is delete-refs so specific to send-pack\n \t * machinery that set_ref_status_for_push() cannot set this\n@@ -279,18 +289,12 @@ int send_pack(struct send_pack_args *args,\n \t\t} else {\n \t\t\tchar *old_hex = sha1_to_hex(ref->old_sha1);\n \t\t\tchar *new_hex = sha1_to_hex(ref->new_sha1);\n-\t\t\tint quiet = quiet_supported && (args->quiet || !args->progress);\n \n \t\t\tif (!cmds_sent)\n \t\t\t\tpacket_buf_write(&req_buf,\n-\t\t\t\t\t\t \"%s %s %s%c%s%s%s%s%s\",\n+\t\t\t\t\t\t \"%s %s %s%c%s\",\n \t\t\t\t\t\t old_hex, new_hex, ref->name, 0,\n-\t\t\t\t\t\t status_report ? \" report-status\" : \"\",\n-\t\t\t\t\t\t use_sideband ? \" side-band-64k\" : \"\",\n-\t\t\t\t\t\t quiet ? \" quiet\" : \"\",\n-\t\t\t\t\t\t agent_supported ? \" agent=\" : \"\",\n-\t\t\t\t\t\t agent_supported ? git_user_agent_sanitized() : \"\"\n-\t\t\t\t\t\t);\n+\t\t\t\t\t\t cap_buf.buf);\n \t\t\telse\n \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n \t\t\t\t\t\t old_hex, new_hex, ref->name);\n@@ -311,6 +315,7 @@ int send_pack(struct send_pack_args *args,\n \t\tpacket_flush(out);\n \t}\n \tstrbuf_release(&req_buf);\n+\tstrbuf_release(&cap_buf);\n \n \tif (use_sideband && cmds_sent) {\n \t\tmemset(&demux, 0, sizeof(demux));\n-- \n2.1.0-301-g54593e2\n"},{"id":"247978","messageId":"1408485987-3590-10-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 09/18] send-pack: rename \"new_refs\" to \"need_pack_data\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:18Z","receivedAt":"2014-08-19T22:06:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The variable counts how many non-deleting command is being sent, but\nis only checked with 0-ness to decide if we need to send the pack\ndata.\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex c1c64ee..590eb0a 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -220,7 +220,7 @@ int send_pack(struct send_pack_args *args,\n \tstruct strbuf req_buf = STRBUF_INIT;\n \tstruct strbuf cap_buf = STRBUF_INIT;\n \tstruct ref *ref;\n-\tint new_refs;\n+\tint need_pack_data = 0;\n \tint allow_deleting_refs = 0;\n \tint status_report = 0;\n \tint use_sideband = 0;\n@@ -276,13 +276,12 @@ int send_pack(struct send_pack_args *args,\n \t/*\n \t * Finally, tell the other end!\n \t */\n-\tnew_refs = 0;\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tif (!ref_update_to_be_sent(ref, args))\n \t\t\tcontinue;\n \n \t\tif (!ref->deletion)\n-\t\t\tnew_refs++;\n+\t\t\tneed_pack_data = 1;\n \n \t\tif (args->dry_run) {\n \t\t\tref->status = REF_STATUS_OK;\n@@ -327,7 +326,7 @@ int send_pack(struct send_pack_args *args,\n \t\tin = demux.out;\n \t}\n \n-\tif (new_refs && cmds_sent) {\n+\tif (need_pack_data && cmds_sent) {\n \t\tif (pack_objects(out, remote_refs, extra_have, args) < 0) {\n \t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n \t\t\t\tref->status = REF_STATUS_NONE;\n-- \n2.1.0-301-g54593e2\n"},{"id":"247981","messageId":"1408485987-3590-11-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 10/18] send-pack: refactor inspecting and resetting status and sending commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:19Z","receivedAt":"2014-08-19T22:06:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The main loop over remote_refs list inspects the ref status\nto see if we need to generate pack data (i.e. a delete-only push\ndoes not need to send any additional data), resets it to \"expecting\nthe status report\" state, and formats the actual update commands\nto be sent.\n\nSplit the former two out of the main loop, as it will become\nconditional in later steps.\n\nBesides, we should have code that does real thing here, before the\n\"Finally, tell the other end!\" part ;-)\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 49 ++++++++++++++++++++++++++++++-------------------\n 1 file changed, 30 insertions(+), 19 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 590eb0a..f3262f2 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -274,7 +274,8 @@ int send_pack(struct send_pack_args *args,\n \t\tadvertise_shallow_grafts_buf(&req_buf);\n \n \t/*\n-\t * Finally, tell the other end!\n+\t * Clear the status for each ref and see if we need to send\n+\t * the pack data.\n \t */\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tif (!ref_update_to_be_sent(ref, args))\n@@ -283,25 +284,35 @@ int send_pack(struct send_pack_args *args,\n \t\tif (!ref->deletion)\n \t\t\tneed_pack_data = 1;\n \n-\t\tif (args->dry_run) {\n+\t\tif (args->dry_run || !status_report)\n \t\t\tref->status = REF_STATUS_OK;\n-\t\t} else {\n-\t\t\tchar *old_hex = sha1_to_hex(ref->old_sha1);\n-\t\t\tchar *new_hex = sha1_to_hex(ref->new_sha1);\n-\n-\t\t\tif (!cmds_sent)\n-\t\t\t\tpacket_buf_write(&req_buf,\n-\t\t\t\t\t\t \"%s %s %s%c%s\",\n-\t\t\t\t\t\t old_hex, new_hex, ref->name, 0,\n-\t\t\t\t\t\t cap_buf.buf);\n-\t\t\telse\n-\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n-\t\t\t\t\t\t old_hex, new_hex, ref->name);\n-\t\t\tref->status = status_report ?\n-\t\t\t\tREF_STATUS_EXPECTING_REPORT :\n-\t\t\t\tREF_STATUS_OK;\n-\t\t\tcmds_sent++;\n-\t\t}\n+\t\telse\n+\t\t\tref->status = REF_STATUS_EXPECTING_REPORT;\n+\t}\n+\n+\t/*\n+\t * Finally, tell the other end!\n+\t */\n+\tfor (ref = remote_refs; ref; ref = ref->next) {\n+\t\tchar *old_hex, *new_hex;\n+\n+\t\tif (args->dry_run)\n+\t\t\tcontinue;\n+\n+\t\tif (!ref_update_to_be_sent(ref, args))\n+\t\t\tcontinue;\n+\n+\t\told_hex = sha1_to_hex(ref->old_sha1);\n+\t\tnew_hex = sha1_to_hex(ref->new_sha1);\n+\t\tif (!cmds_sent)\n+\t\t\tpacket_buf_write(&req_buf,\n+\t\t\t\t\t \"%s %s %s%c%s\",\n+\t\t\t\t\t old_hex, new_hex, ref->name, 0,\n+\t\t\t\t\t cap_buf.buf);\n+\t\telse\n+\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n+\t\t\t\t\t old_hex, new_hex, ref->name);\n+\t\tcmds_sent++;\n \t}\n \n \tif (args->stateless_rpc) {\n-- \n2.1.0-301-g54593e2\n"},{"id":"247979","messageId":"1408485987-3590-12-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 11/18] send-pack: clarify that cmds_sent is a boolean","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:20Z","receivedAt":"2014-08-19T22:06:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We use it to make sure that the feature request is sent only once on\nthe very first request packet (ignoring the \"shallow \" line, which\nwas an unfortunate mistake we cannot retroactively fix with existing\nreceive-pack already deployed in the field) and we set it to \"true\"\nwith cmds_sent++, not because we care about the actual number of\nupdates sent but because it is merely an old idiomatic way.\n\nSet it explicitly to one to clarify that the code that uses this\nvariable only cares about its zero-ness.\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex f3262f2..05926d2 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -304,15 +304,16 @@ int send_pack(struct send_pack_args *args,\n \n \t\told_hex = sha1_to_hex(ref->old_sha1);\n \t\tnew_hex = sha1_to_hex(ref->new_sha1);\n-\t\tif (!cmds_sent)\n+\t\tif (!cmds_sent) {\n \t\t\tpacket_buf_write(&req_buf,\n \t\t\t\t\t \"%s %s %s%c%s\",\n \t\t\t\t\t old_hex, new_hex, ref->name, 0,\n \t\t\t\t\t cap_buf.buf);\n-\t\telse\n+\t\t\tcmds_sent = 1;\n+\t\t} else {\n \t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n \t\t\t\t\t old_hex, new_hex, ref->name);\n-\t\tcmds_sent++;\n+\t\t}\n \t}\n \n \tif (args->stateless_rpc) {\n-- \n2.1.0-301-g54593e2\n"},{"id":"247980","messageId":"1408485987-3590-13-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 12/18] gpg-interface: move parse_gpg_output() to where it should be","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:21Z","receivedAt":"2014-08-19T22:06:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Earlier, ffb6d7d5 (Move commit GPG signature verification to\ncommit.c, 2013-03-31) moved this helper that used to be in pretty.c\n(i.e. the output code path) to commit.c for better reusability.\n\nIt was a good first step in the right direction, but still suffers a\nmyopic view that commits will be the only thing we would ever want\nto sign---we would actually want to be able to reuse it even wider.\n\nThe function interprets what GPG said; gpg-interface is obviously a\nbetter place. Move it there.\n\nSigned-off-by: Junio C Hamano \n---\n commit.c | 36 ------------------------------------\n gpg-interface.c | 36 ++++++++++++++++++++++++++++++++++++\n gpg-interface.h | 17 ++++++++++++-----\n 3 files changed, 48 insertions(+), 41 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex ae7f2b1..01cdad2 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1220,42 +1220,6 @@ free_return:\n \tfree(buf);\n }\n \n-static struct {\n-\tchar result;\n-\tconst char *check;\n-} sigcheck_gpg_status[] = {\n-\t{ 'G', \"\\n[GNUPG:] GOODSIG \" },\n-\t{ 'B', \"\\n[GNUPG:] BADSIG \" },\n-\t{ 'U', \"\\n[GNUPG:] TRUST_NEVER\" },\n-\t{ 'U', \"\\n[GNUPG:] TRUST_UNDEFINED\" },\n-};\n-\n-static void parse_gpg_output(struct signature_check *sigc)\n-{\n-\tconst char *buf = sigc->gpg_status;\n-\tint i;\n-\n-\t/* Iterate over all search strings */\n-\tfor (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {\n-\t\tconst char *found, *next;\n-\n-\t\tif (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1, &found)) {\n-\t\t\tfound = strstr(buf, sigcheck_gpg_status[i].check);\n-\t\t\tif (!found)\n-\t\t\t\tcontinue;\n-\t\t\tfound += strlen(sigcheck_gpg_status[i].check);\n-\t\t}\n-\t\tsigc->result = sigcheck_gpg_status[i].result;\n-\t\t/* The trust messages are not followed by key/signer information */\n-\t\tif (sigc->result != 'U') {\n-\t\t\tsigc->key = xmemdupz(found, 16);\n-\t\t\tfound += 17;\n-\t\t\tnext = strchrnul(found, '\\n');\n-\t\t\tsigc->signer = xmemdupz(found, next - found);\n-\t\t}\n-\t}\n-}\n-\n void check_commit_signature(const struct commit* commit, struct signature_check *sigc)\n {\n \tstruct strbuf payload = STRBUF_INIT;\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex ff07012..3c9624c 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -21,6 +21,42 @@ void signature_check_clear(struct signature_check *sigc)\n \tsigc->key = NULL;\n }\n \n+static struct {\n+\tchar result;\n+\tconst char *check;\n+} sigcheck_gpg_status[] = {\n+\t{ 'G', \"\\n[GNUPG:] GOODSIG \" },\n+\t{ 'B', \"\\n[GNUPG:] BADSIG \" },\n+\t{ 'U', \"\\n[GNUPG:] TRUST_NEVER\" },\n+\t{ 'U', \"\\n[GNUPG:] TRUST_UNDEFINED\" },\n+};\n+\n+void parse_gpg_output(struct signature_check *sigc)\n+{\n+\tconst char *buf = sigc->gpg_status;\n+\tint i;\n+\n+\t/* Iterate over all search strings */\n+\tfor (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {\n+\t\tconst char *found, *next;\n+\n+\t\tif (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1, &found)) {\n+\t\t\tfound = strstr(buf, sigcheck_gpg_status[i].check);\n+\t\t\tif (!found)\n+\t\t\t\tcontinue;\n+\t\t\tfound += strlen(sigcheck_gpg_status[i].check);\n+\t\t}\n+\t\tsigc->result = sigcheck_gpg_status[i].result;\n+\t\t/* The trust messages are not followed by key/signer information */\n+\t\tif (sigc->result != 'U') {\n+\t\t\tsigc->key = xmemdupz(found, 16);\n+\t\t\tfound += 17;\n+\t\t\tnext = strchrnul(found, '\\n');\n+\t\t\tsigc->signer = xmemdupz(found, next - found);\n+\t\t}\n+\t}\n+}\n+\n void set_signing_key(const char *key)\n {\n \tfree(configured_signing_key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 37c23da..8d677cc 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -5,16 +5,23 @@ struct signature_check {\n \tchar *payload;\n \tchar *gpg_output;\n \tchar *gpg_status;\n-\tchar result; /* 0 (not checked),\n-\t\t * N (checked but no further result),\n-\t\t * U (untrusted good),\n-\t\t * G (good)\n-\t\t * B (bad) */\n+\n+\t/*\n+\t * possible \"result\":\n+\t * 0 (not checked)\n+\t * N (checked but no further result)\n+\t * U (untrusted good)\n+\t * G (good)\n+\t * B (bad)\n+\t */\n+\tchar result;\n \tchar *signer;\n \tchar *key;\n };\n \n extern void signature_check_clear(struct signature_check *sigc);\n+extern void parse_gpg_output(struct signature_check *);\n+\n extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);\n extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output, struct strbuf *gpg_status);\n extern int git_gpg_config(const char *, const char *, void *);\n-- \n2.1.0-301-g54593e2\n"},{"id":"247984","messageId":"1408485987-3590-14-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 13/18] gpg-interface: move parse_signature() to where it should be","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:22Z","receivedAt":"2014-08-19T22:06:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Our signed-tag objects set the standard format used by Git to store\nGPG-signed payload (i.e. the payload followed by its detached\nsignature), and it made sense to have a helper to find the boundary\nbetween the payload and its signature in tag.c back then.\n\nNewer code added later to parse other kinds of objects that learned\nto use the same format to store GPG-signed payload (e.g. signed\ncommits), however, kept using the helper from the same location.\n\nMove it to gpg-interface; the helper is no longer about signed tag,\nbut it is how our code and data interact with GPG.\n\nSigned-off-by: Junio C Hamano \n---\n gpg-interface.c | 21 +++++++++++++++++++++\n gpg-interface.h | 1 +\n tag.c | 20 --------------------\n tag.h | 1 -\n 4 files changed, 22 insertions(+), 21 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 3c9624c..0dd11ea 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -7,6 +7,9 @@\n static char *configured_signing_key;\n static const char *gpg_program = \"gpg\";\n \n+#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n+#define PGP_MESSAGE \"-----BEGIN PGP MESSAGE-----\"\n+\n void signature_check_clear(struct signature_check *sigc)\n {\n \tfree(sigc->payload);\n@@ -57,6 +60,24 @@ void parse_gpg_output(struct signature_check *sigc)\n \t}\n }\n \n+/*\n+ * Look at GPG signed content (e.g. a signed tag object), whose\n+ * payload is followed by a detached signature on it. Return the\n+ * offset where the embedded detached signature begins, or the end of\n+ * the data when there is no such signature.\n+ */\n+size_t parse_signature(const char *buf, unsigned long size)\n+{\n+\tchar *eol;\n+\tsize_t len = 0;\n+\twhile (len < size && !starts_with(buf + len, PGP_SIGNATURE) &&\n+\t\t\t!starts_with(buf + len, PGP_MESSAGE)) {\n+\t\teol = memchr(buf + len, '\\n', size - len);\n+\t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n+\t}\n+\treturn len;\n+}\n+\n void set_signing_key(const char *key)\n {\n \tfree(configured_signing_key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 8d677cc..93c76c2 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -20,6 +20,7 @@ struct signature_check {\n };\n \n extern void signature_check_clear(struct signature_check *sigc);\n+extern size_t parse_signature(const char *buf, unsigned long size);\n extern void parse_gpg_output(struct signature_check *);\n \n extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);\ndiff --git a/tag.c b/tag.c\nindex 82d841b..5b0ac62 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -4,9 +4,6 @@\n #include \"tree.h\"\n #include \"blob.h\"\n \n-#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n-#define PGP_MESSAGE \"-----BEGIN PGP MESSAGE-----\"\n-\n const char *tag_type = \"tag\";\n \n struct object *deref_tag(struct object *o, const char *warn, int warnlen)\n@@ -143,20 +140,3 @@ int parse_tag(struct tag *item)\n \tfree(data);\n \treturn ret;\n }\n-\n-/*\n- * Look at a signed tag object, and return the offset where\n- * the embedded detached signature begins, or the end of the\n- * data when there is no such signature.\n- */\n-size_t parse_signature(const char *buf, unsigned long size)\n-{\n-\tchar *eol;\n-\tsize_t len = 0;\n-\twhile (len < size && !starts_with(buf + len, PGP_SIGNATURE) &&\n-\t\t\t!starts_with(buf + len, PGP_MESSAGE)) {\n-\t\teol = memchr(buf + len, '\\n', size - len);\n-\t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n-\t}\n-\treturn len;\n-}\ndiff --git a/tag.h b/tag.h\nindex bc8a1e4..f4580ae 100644\n--- a/tag.h\n+++ b/tag.h\n@@ -17,6 +17,5 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si\n extern int parse_tag(struct tag *item);\n extern struct object *deref_tag(struct object *, const char *, int);\n extern struct object *deref_tag_noverify(struct object *);\n-extern size_t parse_signature(const char *buf, unsigned long size);\n \n #endif /* TAG_H */\n-- \n2.1.0-301-g54593e2\n"},{"id":"247985","messageId":"1408485987-3590-15-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 14/18] pack-protocol doc: typofix for PKT-LINE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:23Z","receivedAt":"2014-08-19T22:06:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Everywhere else we use PKT-LINE to denote the pkt-line formatted\ndata, but \"shallow/deepen\" messages are described with PKT_LINE().\n\nFix them.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/technical/pack-protocol.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt\nindex 18dea8d..a845d51 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -212,9 +212,9 @@ out of what the server said it could do with the first 'want' line.\n want-list = first-want\n \t\t *additional-want\n \n- shallow-line = PKT_LINE(\"shallow\" SP obj-id)\n+ shallow-line = PKT-LINE(\"shallow\" SP obj-id)\n \n- depth-request = PKT_LINE(\"deepen\" SP depth)\n+ depth-request = PKT-LINE(\"deepen\" SP depth)\n \n first-want = PKT-LINE(\"want\" SP obj-id SP capability-list LF)\n additional-want = PKT-LINE(\"want\" SP obj-id LF)\n-- \n2.1.0-301-g54593e2\n"},{"id":"247982","messageId":"1408485987-3590-16-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 15/18] the beginning of the signed push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:24Z","receivedAt":"2014-08-19T22:06:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While signed tags and commits assert that the objects thusly signed\ncame from you, who signed these objects, there is not a good way to\nassert that you wanted to have a particular object at the tip of a\nparticular branch. My signing v2.0.1 tag only means I want to call\nthe version v2.0.1, and it does not mean I want to push it out to my\n'master' branch---it is likely that I only want it in 'maint'.\n\nIntroduce a mechanism that allows you to sign a \"push certificate\"\n(for the lack of better name) every time you push, asserting that\nwhat object you are pushing to update which ref that used to point\nat what other object. Think of it as a cryptographic protection for\nref updates, similar to signed tags/commits but working on an\northogonal axis.\n\nThe basic flow based on this mechanism goes like this:\n\n 1. You push out your work with \"git push -s\".\n\n 2. The sending side learns where the remote refs are as usual,\n together with what protocol extension the receiving end\n supports. If the receiving end does not advertise the protocol\n extension \"push-cert\", the sending side falls back to the normal\n push that is not signed.\n\n Otherwise, a text file, that looks like the following, is\n prepared in core:\n\n\tcertificate version 0.1\n\tpusher Junio C Hamano 1315427886 -0700\n\n\t7339ca65... 21580ecb... refs/heads/master\n\t3793ac56... 12850bec... refs/heads/next\n\n Each line shows the old and the new object name at the tip of\n the ref this push tries to update, in the way identical to how\n the underlying \"git push\" protocol exchange tells the ref\n updates to the receiving end (by recording the \"old\" object\n name, the push certificate also protects against replaying). It\n is expected that new command packet types other than the\n old-new-refname kind will be included in push certificate in the\n same way as would appear in the plain vanilla command packets in\n unsigned pushes.\n\n The user then is asked to sign this push certificate using GPG,\n formatted in a way similar to how signed tag objects are signed,\n and the result is sent to the other side (i.e. receive-pack).\n\n In the protocol exchange, this step comes immediately before the\n sender tells what the result of the push should be, which in\n turn comes before it sends the pack data.\n\n 3. When the receiving end sees a push certificate, the certificate\n is written out as a blob. The pre-receive hook can learn about\n the certificate by checking GIT_PUSH_CERT environment variable,\n which, if present, tells the object name of this blob, and make\n the decision to allow or reject this push. Additionally, the\n post-receive hook can also look at the certificate, which may be\n a good place to log all the received certificates for later\n audits.\n\nBecause a push certificate carry the same information as the usual\ncommand packets in the protocol exchange, we can omit the latter\nwhen a push certificate is in use and reduce the protocol overhead.\nThis however is not included in this patch to make it easier to\nreview (in other words, the series at this step should never be\nreleased without the remainder of the series, as it implements an\ninterim protocol that will be incompatible with the final one,\nmerely for reviewing purposes). As such, the documentation update\nfor the protocol is left out of this step.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-push.txt | 9 +++++-\n Documentation/git-receive-pack.txt | 16 ++++++++-\n builtin/push.c | 1 +\n builtin/receive-pack.c | 43 ++++++++++++++++++++++++-\n send-pack.c | 66 ++++++++++++++++++++++++++++++++++++++\n send-pack.h | 1 +\n t/t5534-push-signed.sh | 63 ++++++++++++++++++++++++++++++++++++\n transport.c | 4 +++\n transport.h | 5 +++\n 9 files changed, 205 insertions(+), 3 deletions(-)\n create mode 100755 t/t5534-push-signed.sh\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 21cd455..21b3f29 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -10,7 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=]\n-\t [--repo=] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream]\n+\t [--repo=] [-f | --force] [--prune] [-v | --verbose]\n+\t [-u | --set-upstream] [--signed]\n \t [--force-with-lease[=[:]]]\n \t [--no-verify] [ [...]]\n \n@@ -129,6 +130,12 @@ already exists on the remote side.\n \tfrom the remote but are pointing at commit-ish that are\n \treachable from the refs being pushed.\n \n+--signed::\n+\tGPG-sign the push request to update refs on the receiving\n+\tside, to allow it to be checked by the hooks and/or be\n+\tlogged. See linkgit:git-receive-pack[1] for the details\n+\ton the receiving end.\n+\n --receive-pack=::\n --exec=::\n \tPath to the 'git-receive-pack' program on the remote\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex b1f7dc6..6c458af 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -53,6 +53,11 @@ the update. Refs to be created will have sha1-old equal to 0\\{40},\n while refs to be deleted will have sha1-new equal to 0\\{40}, otherwise\n sha1-old and sha1-new should be valid objects in the repository.\n \n+When accepting a signed push (see linkgit:git-push[1]), the signed\n+push certificate is stored in a blob and an environment variable\n+`GIT_PUSH_CERT` can be consulted for its object name. See the\n+description of `post-receive` hook for an example.\n+\n This hook is called before any refname is updated and before any\n fast-forward checks are performed.\n \n@@ -101,9 +106,13 @@ the update. Refs that were created will have sha1-old equal to\n 0\\{40}, otherwise sha1-old and sha1-new should be valid objects in\n the repository.\n \n+The `GIT_PUSH_CERT` environment variable can be inspected, just as\n+in `pre-receive` hook, after accepting a signed push.\n+\n Using this hook, it is easy to generate mails describing the updates\n to the repository. This example script sends one mail message per\n-ref listing the commits pushed to the repository:\n+ref listing the commits pushed to the repository, and logs the push\n+certificates of signed pushes to a file:\n \n \t#!/bin/sh\n \t# mail out commit update information.\n@@ -119,6 +128,11 @@ ref listing the commits pushed to the repository:\n \t\tfi |\n \t\tmail -s \"Changes to ref $ref\" commit-list@mydomain\n \tdone\n+\t# log signed push certificate, if any\n+\tif test -n \"${GIT_PUSH_CERT-}\"\n+\tthen\n+\t\tgit cat-file blob ${GIT_PUSH_CERT} >>/var/log/push-log\n+\tfi\n \texit 0\n \n The exit code from this hook invocation is ignored, however a\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f50e3d5..ae56f73 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -506,6 +506,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"no-verify\", &flags, N_(\"bypass pre-push hook\"), TRANSPORT_PUSH_NO_HOOK),\n \t\tOPT_BIT(0, \"follow-tags\", &flags, N_(\"push missing but relevant tags\"),\n \t\t\tTRANSPORT_PUSH_FOLLOW_TAGS),\n+\t\tOPT_BIT(0, \"signed\", &flags, N_(\"GPG sign the push\"), TRANSPORT_PUSH_CERT),\n \t\tOPT_END()\n \t};\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 341bb46..f30df8a 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -46,6 +46,8 @@ static void *head_name_to_free;\n static int sent_capabilities;\n static int shallow_update;\n static const char *alt_shallow_file;\n+static struct strbuf push_cert = STRBUF_INIT;\n+static unsigned char push_cert_sha1[20];\n \n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n@@ -142,7 +144,7 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \telse\n \t\tpacket_write(1, \"%s %s%c%s%s agent=%s\\n\",\n \t\t\t sha1_to_hex(sha1), path, 0,\n-\t\t\t \" report-status delete-refs side-band-64k quiet\",\n+\t\t\t \" report-status delete-refs side-band-64k quiet push-cert\",\n \t\t\t prefer_ofs_delta ? \" ofs-delta\" : \"\",\n \t\t\t git_user_agent_sanitized());\n \tsent_capabilities = 1;\n@@ -252,6 +254,22 @@ static int copy_to_sideband(int in, int out, void *arg)\n \treturn 0;\n }\n \n+static void prepare_push_cert_sha1(struct child_process *proc)\n+{\n+\tstatic int already_done;\n+\tstruct argv_array env = ARGV_ARRAY_INIT;\n+\n+\tif (!already_done) {\n+\t\talready_done = 1;\n+\t\tif (write_sha1_file(push_cert.buf, push_cert.len, \"blob\", push_cert_sha1))\n+\t\t\thashclr(push_cert_sha1);\n+\t}\n+\tif (!is_null_sha1(push_cert_sha1)) {\n+\t\targv_array_pushf(&env, \"GIT_PUSH_CERT=%s\", sha1_to_hex(push_cert_sha1));\n+\t\tproc->env = env.argv;\n+\t}\n+}\n+\n typedef int (*feed_fn)(void *, const char **, size_t *);\n static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_state)\n {\n@@ -271,6 +289,8 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta\n \tproc.in = -1;\n \tproc.stdout_to_stderr = 1;\n \n+\tprepare_push_cert_sha1(&proc);\n+\n \tif (use_sideband) {\n \t\tmemset(&muxer, 0, sizeof(muxer));\n \t\tmuxer.proc = copy_to_sideband;\n@@ -890,6 +910,27 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\t\t\tquiet = 1;\n \t\t}\n \n+\t\tif (!strcmp(line, \"push-cert\")) {\n+\t\t\tint true_flush = 0;\n+\t\t\tchar certbuf[1024];\n+\n+\t\t\tfor (;;) {\n+\t\t\t\tlen = packet_read(0, NULL, NULL,\n+\t\t\t\t\t\t certbuf, sizeof(certbuf), 0);\n+\t\t\t\tif (!len) {\n+\t\t\t\t\ttrue_flush = 1;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t\tif (!strcmp(certbuf, \"push-cert-end\\n\"))\n+\t\t\t\t\tbreak; /* end of cert */\n+\t\t\t\tstrbuf_addstr(&push_cert, certbuf);\n+\t\t\t}\n+\n+\t\t\tif (true_flush)\n+\t\t\t\tbreak;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tp = queue_command(p, line, linelen);\n \t}\n \treturn commands;\ndiff --git a/send-pack.c b/send-pack.c\nindex 05926d2..a073891 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -11,6 +11,7 @@\n #include \"transport.h\"\n #include \"version.h\"\n #include \"sha1-array.h\"\n+#include \"gpg-interface.h\"\n \n static int feed_object(const unsigned char *sha1, int fd, int negative)\n {\n@@ -210,6 +211,64 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a\n \t}\n }\n \n+/*\n+ * the beginning of the next line, or the end of buffer.\n+ *\n+ * NEEDSWORK: perhaps move this to git-compat-util.h or somewhere and\n+ * convert many similar uses found by \"git grep -A4 memchr\".\n+ */\n+static const char *next_line(const char *line, size_t len)\n+{\n+\tconst char *nl = memchr(line, '\\n', len);\n+\tif (!nl)\n+\t\treturn line + len; /* incomplete line */\n+\treturn nl + 1;\n+}\n+\n+static void generate_push_cert(struct strbuf *req_buf,\n+\t\t\t const struct ref *remote_refs,\n+\t\t\t struct send_pack_args *args)\n+{\n+\tconst struct ref *ref;\n+\tchar stamp[60];\n+\tchar *signing_key = xstrdup(get_signing_key());\n+\tconst char *cp, *np;\n+\tstruct strbuf cert = STRBUF_INIT;\n+\tint update_seen = 0;\n+\n+\tdatestamp(stamp, sizeof(stamp));\n+\tstrbuf_addf(&cert, \"certificate version 0.1\\n\");\n+\tstrbuf_addf(&cert, \"pusher %s %s\\n\", signing_key, stamp);\n+\tstrbuf_addstr(&cert, \"\\n\");\n+\n+\tfor (ref = remote_refs; ref; ref = ref->next) {\n+\t\tif (!ref_update_to_be_sent(ref, args))\n+\t\t\tcontinue;\n+\t\tupdate_seen = 1;\n+\t\tstrbuf_addf(&cert, \"%s %s %s\\n\",\n+\t\t\t sha1_to_hex(ref->old_sha1),\n+\t\t\t sha1_to_hex(ref->new_sha1),\n+\t\t\t ref->name);\n+\t}\n+\tif (!update_seen)\n+\t\tgoto free_return;\n+\n+\tif (sign_buffer(&cert, &cert, signing_key))\n+\t\tdie(_(\"failed to sign the push certificate\"));\n+\n+\tpacket_buf_write(req_buf, \"push-cert\\n\");\n+\tfor (cp = cert.buf; cp < cert.buf + cert.len; cp = np) {\n+\t\tnp = next_line(cp, cert.buf + cert.len - cp);\n+\t\tpacket_buf_write(req_buf,\n+\t\t\t\t \"%.*s\", (int)(np - cp), cp);\n+\t}\n+\tpacket_buf_write(req_buf, \"push-cert-end\\n\");\n+\n+free_return:\n+\tfree(signing_key);\n+\tstrbuf_release(&cert);\n+}\n+\n int send_pack(struct send_pack_args *args,\n \t int fd[], struct child_process *conn,\n \t struct ref *remote_refs,\n@@ -245,6 +304,10 @@ int send_pack(struct send_pack_args *args,\n \t\tagent_supported = 1;\n \tif (server_supports(\"no-thin\"))\n \t\targs->use_thin_pack = 0;\n+\tif (args->push_cert && !server_supports(\"push-cert\")) {\n+\t\twarning(_(\"the receiving end does not support --signed push\"));\n+\t\targs->push_cert = 0;\n+\t}\n \n \tif (!remote_refs) {\n \t\tfprintf(stderr, \"No refs in common and none specified; doing nothing.\\n\"\n@@ -273,6 +336,9 @@ int send_pack(struct send_pack_args *args,\n \tif (!args->dry_run)\n \t\tadvertise_shallow_grafts_buf(&req_buf);\n \n+\tif (!args->dry_run && args->push_cert)\n+\t\tgenerate_push_cert(&req_buf, remote_refs, args);\n+\n \t/*\n \t * Clear the status for each ref and see if we need to send\n \t * the pack data.\ndiff --git a/send-pack.h b/send-pack.h\nindex 8e84392..3555d8e 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -11,6 +11,7 @@ struct send_pack_args {\n \t\tuse_thin_pack:1,\n \t\tuse_ofs_delta:1,\n \t\tdry_run:1,\n+\t\tpush_cert:1,\n \t\tstateless_rpc:1;\n };\n \ndiff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh\nnew file mode 100755\nindex 0000000..0eb5004\n--- /dev/null\n+++ b/t/t5534-push-signed.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+\n+test_description='signed push'\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-gpg.sh\n+\n+prepare_dst () {\n+\trm -fr dst &&\n+\ttest_create_repo dst &&\n+\n+\tgit push dst master:noop master:ff master:noff\n+}\n+\n+test_expect_success setup '\n+\t# master, ff and noff branches pointing at the same commit\n+\ttest_tick &&\n+\tgit commit --allow-empty -m initial &&\n+\n+\tgit checkout -b noop &&\n+\tgit checkout -b ff &&\n+\tgit checkout -b noff &&\n+\n+\t# noop stays the same, ff advances, noff rewrites\n+\ttest_tick &&\n+\tgit commit --allow-empty --amend -m rewritten &&\n+\tgit checkout ff &&\n+\n+\ttest_tick &&\n+\tgit commit --allow-empty -m second\n+'\n+\n+test_expect_success 'unsigned push does not send push certificate' '\n+\tprepare_dst &&\n+\tmkdir -p dst/.git/hooks &&\n+\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n+\tif test -n \"${GIT_PUSH_CERT-}\"\n+\tthen\n+\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n+\tfi\n+\tEOF\n+\n+\tgit push dst noop ff +noff &&\n+\ttest -f dst/push-cert &&\n+\t! test -s dst/push-cert\n+'\n+\n+test_expect_success GPG 'signed push sends push certificate' '\n+\tprepare_dst &&\n+\tmkdir -p dst/.git/hooks &&\n+\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n+\tif test -n \"${GIT_PUSH_CERT-}\"\n+\tthen\n+\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n+\tfi\n+\tEOF\n+\n+\tgit push --signed dst noop ff +noff &&\n+\tgrep \"$(git rev-parse noop ff) refs/heads/ff\" dst/push-cert &&\n+\tgrep \"$(git rev-parse noop noff) refs/heads/noff\" dst/push-cert\n+'\n+\n+test_done\ndiff --git a/transport.c b/transport.c\nindex 662421b..07fdf86 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -480,6 +480,9 @@ static int set_git_option(struct git_transport_options *opts,\n \t\t\t\tdie(\"transport: invalid depth option '%s'\", value);\n \t\t}\n \t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_PUSH_CERT)) {\n+\t\topts->push_cert = !!value;\n+\t\treturn 0;\n \t}\n \treturn 1;\n }\n@@ -823,6 +826,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \targs.progress = transport->progress;\n \targs.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);\n \targs.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);\n+\targs.push_cert = !!(flags & TRANSPORT_PUSH_CERT);\n \n \tret = send_pack(&args, data->fd, data->conn, remote_refs,\n \t\t\t&data->extra_have);\ndiff --git a/transport.h b/transport.h\nindex 02ea248..3e0091e 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -12,6 +12,7 @@ struct git_transport_options {\n \tunsigned check_self_contained_and_connected : 1;\n \tunsigned self_contained_and_connected : 1;\n \tunsigned update_shallow : 1;\n+\tunsigned push_cert : 1;\n \tint depth;\n \tconst char *uploadpack;\n \tconst char *receivepack;\n@@ -123,6 +124,7 @@ struct transport {\n #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256\n #define TRANSPORT_PUSH_NO_HOOK 512\n #define TRANSPORT_PUSH_FOLLOW_TAGS 1024\n+#define TRANSPORT_PUSH_CERT 2048\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)\n@@ -156,6 +158,9 @@ struct transport *transport_get(struct remote *, const char *);\n /* Accept refs that may update .git/shallow without --depth */\n #define TRANS_OPT_UPDATE_SHALLOW \"updateshallow\"\n \n+/* Send push certificates */\n+#define TRANS_OPT_PUSH_CERT \"pushcert\"\n+\n /**\n * Returns 0 if the option was used, non-zero otherwise. Prints a\n * message to stderr if the option is not used.\n-- \n2.1.0-301-g54593e2\n"},{"id":"247983","messageId":"1408485987-3590-17-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 16/18] receive-pack: GPG-validate push certificates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:25Z","receivedAt":"2014-08-19T22:06:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Reusing the GPG signature check helpers we already have, verify\nthe signature in receive-pack and give the results to the hooks\nvia GIT_PUSH_CERT_{SIGNER,KEY,STATUS} environment variables.\n\nPolicy decisions, such as accepting or rejecting a good signature by\na key that is not fully trusted, is left to the hook and kept\noutside of the core.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-receive-pack.txt | 22 ++++++++++++++++++----\n builtin/receive-pack.c | 29 +++++++++++++++++++++++++++++\n t/t5534-push-signed.sh | 18 ++++++++++++++++--\n 3 files changed, 63 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex 6c458af..a66884c 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -56,7 +56,21 @@ sha1-old and sha1-new should be valid objects in the repository.\n When accepting a signed push (see linkgit:git-push[1]), the signed\n push certificate is stored in a blob and an environment variable\n `GIT_PUSH_CERT` can be consulted for its object name. See the\n-description of `post-receive` hook for an example.\n+description of `post-receive` hook for an example. In addition, the\n+certificate is verified using GPG and the result is exported with\n+the following environment variables:\n+\n+GIT_PUSH_CERT_SIGNER::\n+\tThe name and the e-mail address of the owner of the key that\n+\tsigned the push certificate.\n+\n+GIT_PUSH_CERT_KEY::\n+\tThe GPG key ID of the key that signed the push certificate.\n+\n+GIT_PUSH_CERT_STATUS::\n+\tThe status of GPG verification of the push certificate,\n+\tusing the same mnemonic as used in `%G?` format of `git log`\n+\tfamily of commands (see linkgit:git-log[1]).\n \n This hook is called before any refname is updated and before any\n fast-forward checks are performed.\n@@ -106,13 +120,13 @@ the update. Refs that were created will have sha1-old equal to\n 0\\{40}, otherwise sha1-old and sha1-new should be valid objects in\n the repository.\n \n-The `GIT_PUSH_CERT` environment variable can be inspected, just as\n+The `GIT_PUSH_CERT*` environment variables can be inspected, just as\n in `pre-receive` hook, after accepting a signed push.\n \n Using this hook, it is easy to generate mails describing the updates\n to the repository. This example script sends one mail message per\n ref listing the commits pushed to the repository, and logs the push\n-certificates of signed pushes to a file:\n+certificates of signed pushes with good signatures to a file:\n \n \t#!/bin/sh\n \t# mail out commit update information.\n@@ -129,7 +143,7 @@ certificates of signed pushes to a file:\n \t\tmail -s \"Changes to ref $ref\" commit-list@mydomain\n \tdone\n \t# log signed push certificate, if any\n-\tif test -n \"${GIT_PUSH_CERT-}\"\n+\tif test -n \"${GIT_PUSH_CERT-}\" && test ${GIT_PUSH_CERT_STATUS} = G\n \tthen\n \t\tgit cat-file blob ${GIT_PUSH_CERT} >>/var/log/push-log\n \tfi\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex f30df8a..abdc296 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -15,6 +15,8 @@\n #include \"connected.h\"\n #include \"argv-array.h\"\n #include \"version.h\"\n+#include \"tag.h\"\n+#include \"gpg-interface.h\"\n \n static const char receive_pack_usage[] = \"git receive-pack \";\n \n@@ -48,6 +50,7 @@ static int shallow_update;\n static const char *alt_shallow_file;\n static struct strbuf push_cert = STRBUF_INIT;\n static unsigned char push_cert_sha1[20];\n+static struct signature_check sigcheck;\n \n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n@@ -260,12 +263,38 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \tstruct argv_array env = ARGV_ARRAY_INIT;\n \n \tif (!already_done) {\n+\t\tstruct strbuf gpg_output = STRBUF_INIT;\n+\t\tstruct strbuf gpg_status = STRBUF_INIT;\n+\t\tint bogs /* beginning_of_gpg_sig */;\n+\n \t\talready_done = 1;\n \t\tif (write_sha1_file(push_cert.buf, push_cert.len, \"blob\", push_cert_sha1))\n \t\t\thashclr(push_cert_sha1);\n+\n+\t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n+\t\tsigcheck.result = 'N';\n+\n+\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tif (verify_signed_buffer(push_cert.buf, bogs,\n+\t\t\t\t\t push_cert.buf + bogs, push_cert.len - bogs,\n+\t\t\t\t\t &gpg_output, &gpg_status) < 0) {\n+\t\t\t; /* error running gpg */\n+\t\t} else {\n+\t\t\tsigcheck.payload = push_cert.buf;\n+\t\t\tsigcheck.gpg_output = gpg_output.buf;\n+\t\t\tsigcheck.gpg_status = gpg_status.buf;\n+\t\t\tparse_gpg_output(&sigcheck);\n+\t\t}\n+\n+\t\tstrbuf_release(&gpg_output);\n+\t\tstrbuf_release(&gpg_status);\n \t}\n \tif (!is_null_sha1(push_cert_sha1)) {\n \t\targv_array_pushf(&env, \"GIT_PUSH_CERT=%s\", sha1_to_hex(push_cert_sha1));\n+\t\targv_array_pushf(&env, \"GIT_PUSH_CERT_SIGNER=%s\", sigcheck.signer);\n+\t\targv_array_pushf(&env, \"GIT_PUSH_CERT_KEY=%s\", sigcheck.key);\n+\t\targv_array_pushf(&env, \"GIT_PUSH_CERT_STATUS=%c\", sigcheck.result);\n+\n \t\tproc->env = env.argv;\n \t}\n }\ndiff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh\nindex 0eb5004..659bca0 100755\n--- a/t/t5534-push-signed.sh\n+++ b/t/t5534-push-signed.sh\n@@ -52,12 +52,26 @@ test_expect_success GPG 'signed push sends push certificate' '\n \tif test -n \"${GIT_PUSH_CERT-}\"\n \tthen\n \t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n-\tfi\n+\tfi &&\n+\n+\tcat >../push-cert-status <expect <<-\\EOF &&\n+\tSIGNER=C O Mitter \n+\tKEY=13B6F51ECDDE430D\n+\tSTATUS=G\n \tEOF\n \n \tgit push --signed dst noop ff +noff &&\n \tgrep \"$(git rev-parse noop ff) refs/heads/ff\" dst/push-cert &&\n-\tgrep \"$(git rev-parse noop noff) refs/heads/noff\" dst/push-cert\n+\tgrep \"$(git rev-parse noop noff) refs/heads/noff\" dst/push-cert &&\n+\ttest_cmp expect dst/push-cert-status\n '\n \n test_done\n-- \n2.1.0-301-g54593e2\n"},{"id":"247987","messageId":"1408485987-3590-18-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 17/18] send-pack: send feature request on push-cert packet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:26Z","receivedAt":"2014-08-19T22:06:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We would want to update the interim protocol so that we do not send\nthe usual update commands when the push certificate feature is in\nuse, as the same information is in the certificate. Once that\nhappens, the push-cert packet may become the only protocol command,\nbut then there is no packet to put the feature request behind, like\nwe always did.\n\nAs we have prepared the receiving end that understands the push-cert\nfeature to accept the feature request on the first protocol packet\n(other than \"shallow \", which was an unfortunate historical mistake\nthat has to come before everything else), we can give the feature\nrequest on the push-cert packet instead of the first update protocol\npacket, in preparation for the next step to actually update to the\nfinal protocol.\n\nSigned-off-by: Junio C Hamano \n---\n send-pack.c | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex a073891..2de9826 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -225,9 +225,10 @@ static const char *next_line(const char *line, size_t len)\n \treturn nl + 1;\n }\n \n-static void generate_push_cert(struct strbuf *req_buf,\n-\t\t\t const struct ref *remote_refs,\n-\t\t\t struct send_pack_args *args)\n+static int generate_push_cert(struct strbuf *req_buf,\n+\t\t\t const struct ref *remote_refs,\n+\t\t\t struct send_pack_args *args,\n+\t\t\t const char *cap_string)\n {\n \tconst struct ref *ref;\n \tchar stamp[60];\n@@ -256,7 +257,7 @@ static void generate_push_cert(struct strbuf *req_buf,\n \tif (sign_buffer(&cert, &cert, signing_key))\n \t\tdie(_(\"failed to sign the push certificate\"));\n \n-\tpacket_buf_write(req_buf, \"push-cert\\n\");\n+\tpacket_buf_write(req_buf, \"push-cert%c%s\", 0, cap_string);\n \tfor (cp = cert.buf; cp < cert.buf + cert.len; cp = np) {\n \t\tnp = next_line(cp, cert.buf + cert.len - cp);\n \t\tpacket_buf_write(req_buf,\n@@ -267,6 +268,7 @@ static void generate_push_cert(struct strbuf *req_buf,\n free_return:\n \tfree(signing_key);\n \tstrbuf_release(&cert);\n+\treturn 1;\n }\n \n int send_pack(struct send_pack_args *args,\n@@ -337,7 +339,8 @@ int send_pack(struct send_pack_args *args,\n \t\tadvertise_shallow_grafts_buf(&req_buf);\n \n \tif (!args->dry_run && args->push_cert)\n-\t\tgenerate_push_cert(&req_buf, remote_refs, args);\n+\t\tcmds_sent = generate_push_cert(&req_buf, remote_refs, args,\n+\t\t\t\t\t cap_buf.buf);\n \n \t/*\n \t * Clear the status for each ref and see if we need to send\n-- \n2.1.0-301-g54593e2\n"},{"id":"247986","messageId":"1408485987-3590-19-git-send-email-gitster@pobox.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"[PATCH 18/18] signed push: final protocol update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:06:27Z","receivedAt":"2014-08-19T22:06:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the interim protocol, we used to send the update commands even\nthough we already send a signed copy of the same information when\npush certificate is in use. Update the send-pack/receive-pack pair\nnot to do so.\n\nThe notable thing on the receive-pack side is that it makes sure\nthat there is no command sent over the traditional protocol packet\noutside the push certificate. Otherwise a pusher can claim to be\npushing one set of ref updates in the signed certificate while\nissuing commands to update unrelated refs, and such an update will\nevade later audits.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/technical/pack-protocol.txt | 20 ++++++++++++++++-\n Documentation/technical/protocol-capabilities.txt | 12 +++++++++--\n builtin/receive-pack.c | 26 +++++++++++++++++++++++\n send-pack.c | 2 +-\n 4 files changed, 56 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt\nindex a845d51..f6c3073 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -465,7 +465,7 @@ contain all the objects that the server will need to complete the new\n references.\n \n ----\n- update-request = *shallow command-list [pack-file]\n+ update-request = *shallow ( command-list | push-cert ) [pack-file]\n \n shallow = PKT-LINE(\"shallow\" SP obj-id)\n \n@@ -481,12 +481,30 @@ references.\n old-id = obj-id\n new-id = obj-id\n \n+ push-cert = PKT-LINE(\"push-cert\" NUL capability-list LF)\n+\t\t PKT-LINE(\"certificate version 0.1\" LF)\n+\t\t PKT-LINE(\"pusher\" ident LF)\n+\t\t PKT-LINE(LF)\n+\t\t *PKT-LINE(command LF)\n+\t\t *PKT-LINE(GPG signature lines LF)\n+\t\t PKT-LINE(\"push-cert-end\" LF)\n+\n pack-file = \"PACK\" 28*(OCTET)\n ----\n \n If the receiving end does not support delete-refs, the sending end MUST\n NOT ask for delete command.\n \n+If the receiving end does not support push-cert, the sending end MUST\n+NOT send a push-cert command.\n+\n+When a push-cert command is sent, command-list MUST NOT be sent; the\n+commands recorded in the push certificate is used instead. The GPG\n+signature lines are detached signature for the contents recorded in\n+the push certificate before the signature block begins and is used\n+to certify that the commands were given by the pusher which must be\n+the signer.\n+\n The pack-file MUST NOT be sent if the only command used is 'delete'.\n \n A pack-file MUST be sent if either create or update command is used,\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex e174343..5b398e9 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -18,8 +18,8 @@ was sent. Server MUST NOT ignore capabilities that client requested\n and server advertised. As a consequence of these rules, server MUST\n NOT advertise capabilities it does not understand.\n \n-The 'report-status', 'delete-refs', and 'quiet' capabilities are sent and\n-recognized by the receive-pack (push to server) process.\n+The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities\n+are sent and recognized by the receive-pack (push to server) process.\n \n The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized\n by both upload-pack and receive-pack protocols. The 'agent' capability\n@@ -250,3 +250,11 @@ allow-tip-sha1-in-want\n If the upload-pack server advertises this capability, fetch-pack may\n send \"want\" lines with SHA-1s that exist at the server but are not\n advertised by upload-pack.\n+\n+push-cert\n+---------\n+\n+The receive-pack server that advertises this capability is willing\n+to accept a signed push certificate. A send-pack client MUST NOT\n+send push-cert packet unless the receive-pack server advertises this\n+capability.\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex abdc296..96e4c99 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -907,6 +907,28 @@ static struct command **queue_command(struct command **p,\n \treturn &cmd->next;\n }\n \n+static void queue_commands_from_cert(struct command **p,\n+\t\t\t\t struct strbuf *push_cert)\n+{\n+\tconst char *boc, *eoc;\n+\n+\tif (*p)\n+\t\tdie(\"protocol error: got both push certificate and unsigned commands\");\n+\n+\tboc = strstr(push_cert->buf, \"\\n\\n\");\n+\tif (!boc)\n+\t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n+\telse\n+\t\tboc += 2;\n+\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\n+\twhile (boc < eoc) {\n+\t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\n+\t\tp = queue_command(p, boc, eol ? eol - boc : eoc - eol);\n+\t\tboc = eol ? eol + 1 : eoc;\n+\t}\n+}\n+\n static struct command *read_head_info(struct sha1_array *shallow)\n {\n \tstruct command *commands = NULL;\n@@ -962,6 +984,10 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \n \t\tp = queue_command(p, line, linelen);\n \t}\n+\n+\tif (push_cert.len)\n+\t\tqueue_commands_from_cert(p, &push_cert);\n+\n \treturn commands;\n }\n \ndiff --git a/send-pack.c b/send-pack.c\nindex 2de9826..ca2ae2b 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -365,7 +365,7 @@ int send_pack(struct send_pack_args *args,\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tchar *old_hex, *new_hex;\n \n-\t\tif (args->dry_run)\n+\t\tif (args->dry_run || args->push_cert)\n \t\t\tcontinue;\n \n \t\tif (!ref_update_to_be_sent(ref, args))\n-- \n2.1.0-301-g54593e2\n"},{"id":"247990","messageId":"xmqqbnrgun5o.fsf@gitster.dls.corp.google.com","threadId":"37381","inReplyTo":"1408485987-3590-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 02/18] receive-pack: parse feature request a bit earlier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:31:31Z","receivedAt":"2014-08-19T22:31:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ideally, we should have also allowed the first \"shallow\" to carry\n> the feature request trailer, but that is water under the bridge\n> now. This makes the next step to factor out the queuing of commands\n> easier to review.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n> ...\n> @@ -853,7 +853,18 @@ static struct command *read_head_info(struct sha1_array *shallow)\n> \t\t\tcontinue;\n> \t\t}\n> \n> -\t\tif (len < 83 ||\n> +\t\tlinelen = strlen(line);\n> +\t\tif (linelen < len) {\n> +\t\t\tconst char *feature_list = line + linelen + 1;\n> +\t\t\tif (parse_feature_request(feature_list, \"report-status\"))\n> +\t\t\t\treport_status = 1;\n> +\t\t\tif (parse_feature_request(feature_list, \"side-band-64k\"))\n> +\t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n> +\t\t\tif (parse_feature_request(feature_list, \"quiet\"))\n> +\t\t\t\tquiet = 1;\n> +\t\t}\n> +\n> +\t\tif (linelen < 83 ||\n> \t\t line[40] != ' ' ||\n> \t\t line[81] != ' ' ||\n> \t\t get_sha1_hex(line, old_sha1) ||\n> @@ -863,15 +874,6 @@ static struct command *read_head_info(struct sha1_array *shallow)\n> \n> \t\trefname = line + 82;\n> \t\treflen = strlen(refname);\n\nA later patch updates this to \"reflen = linelen - 82\" while moving\nthis code to a helper function, but it may be better to do that in\nthis patch.\n"},{"id":"247991","messageId":"xmqq4mx8un4d.fsf@gitster.dls.corp.google.com","threadId":"37381","inReplyTo":"1408485987-3590-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 03/18] receive-pack: do not reuse old_sha1[] to other things","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T22:32:18Z","receivedAt":"2014-08-19T22:32:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This piece of code reads object names of shallow boundaries,\n> not old_sha1[].\n>\n> Signed-off-by: Junio C Hamano \n> ---\n\nI should double the subject line and say \"do not reuse ... for other\nthings\".\n"},{"id":"247992","messageId":"CACsJy8AMCR5v1YYfA1AWWWXuYyuvOpsuXUfYzWfxhPYZE9SbcQ@mail.gmail.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 00/18] Signed push","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-19T23:07:02Z","receivedAt":"2014-08-19T23:07:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Aug 20, 2014 at 5:06 AM, Junio C Hamano wrote:\n> While signed tags and commits assert that the objects thusly signed\n> came from you, who signed these objects, there is not a good way to\n> assert that you wanted to have a particular object at the tip of a\n> particular branch. My signing v2.0.1 tag only means I want to call\n> the version v2.0.1, and it does not mean I want to push it out to my\n> 'master' branch---it is likely that I only want it in 'maint', so\n> the signature on the object alone is insufficient.\n>\n> The only assurance to you that 'maint' points at what I wanted to\n> place there comes from your trust on the hosting site and my\n> authentication with it, which cannot easily audited later.\n\nI only had a quick read of a few important patches and may miss\nsomething. But all this audit recording is left to the hook, right? I\nsuppose git-notes could be used to store the push cert. blob, or the\nserver could make a signed tag to record this info in the ref.. or do\nyou intend any other way to record these blobs?\n-- \nDuy\n"},{"id":"247993","messageId":"CAPc5daWgPgxPf2gL1e+-nwj7kGGBPDnGpA1syLVSVcqug+pEZw@mail.gmail.com","threadId":"37381","inReplyTo":"CACsJy8AMCR5v1YYfA1AWWWXuYyuvOpsuXUfYzWfxhPYZE9SbcQ@mail.gmail.com","subject":"Re: [PATCH 00/18] Signed push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-19T23:29:28Z","receivedAt":"2014-08-19T23:29:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I don't think anybody can come up with a good auditing and recording\nscheme without sufficient experience. Because I want to avoid whatever\nrandom scheme we happen to implement first in git-core, even if it is\nway suboptimal, ends up as the de-facto standard, I deliberately stayed\naway from adding one of my own in this initial series, whose purpose is\nto lay a groundwork for server operators can build on. For the same\nreason, I tried to avoid anything that defines and forces a particular policy\n(e.g. does a server that accepts push certificate accept an unsigned\npush to some of its refs? Is it OK if the GPG key is slightly stale? etc.).\n\n\"git notes\" may or may not be a good vehicle to store them. This series\ndeliberately refrains from making that judgement. I'd rather leave these\noperational issues to folks who work on systems like Gitolite and Gerrit.\n\n\nOn Tue, Aug 19, 2014 at 4:07 PM, Duy Nguyen wrote:\n> On Wed, Aug 20, 2014 at 5:06 AM, Junio C Hamano wrote:\n>> While signed tags and commits assert that the objects thusly signed\n>> came from you, who signed these objects, there is not a good way to\n>> assert that you wanted to have a particular object at the tip of a\n>> particular branch. My signing v2.0.1 tag only means I want to call\n>> the version v2.0.1, and it does not mean I want to push it out to my\n>> 'master' branch---it is likely that I only want it in 'maint', so\n>> the signature on the object alone is insufficient.\n>>\n>> The only assurance to you that 'maint' points at what I wanted to\n>> place there comes from your trust on the hosting site and my\n>> authentication with it, which cannot easily audited later.\n>\n> I only had a quick read of a few important patches and may miss\n> something. But all this audit recording is left to the hook, right? I\n> suppose git-notes could be used to store the push cert. blob, or the\n> server could make a signed tag to record this info in the ref.. or do\n> you intend any other way to record these blobs?\n> --\n> Duy\n"},{"id":"247994","messageId":"20140820011935.GC3104@localhost","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 00/18] Signed push","fromName":"Nico Williams","fromEmail":"nico@cryptonector.com","sentAt":"2014-08-20T01:19:36Z","receivedAt":"2014-08-20T01:19:36Z","isPatch":true,"sender":{"key":"nico@cryptonector.com","avatar":null},"body":"On Tue, Aug 19, 2014 at 03:06:09PM -0700, Junio C Hamano wrote:\n> While signed tags and commits assert that the objects thusly signed\n> came from you, who signed these objects, there is not a good way to\n> assert that you wanted to have a particular object at the tip of a\n> particular branch. My signing v2.0.1 tag only means I want to call\n> the version v2.0.1, and it does not mean I want to push it out to my\n> 'master' branch---it is likely that I only want it in 'maint', so\n> the signature on the object alone is insufficient.\n> \n> [...]\n> \n> This series introduces a cryptographic assurance for ref updates\n> done by \"git push\" by introducing a mechanism that allows you to\n> sign a \"push certificate\" (for the lack of better name) every time\n> you push. Think of it as working on an axis orthogonal to the\n> traditional \"signed tags\".\n\nSounds a lot like the \"branch object\" concept I suggest earlier, where\neach push would also push a commit to a branch object describing the\nupdates to the branch, including signing of the updates to the branch\n(hey, it's just a signed commit), groups of commits pushed together / to\nbe backed out together, rebase history, ... (What about pushing\norphaned commits?)\n\nCode-wise, would that be more or less generic?\n\nNico\n-- \n"},{"id":"247996","messageId":"CAPc5daXKwW-T6PeYtFYdHYuBzvsDXWyYdNOYXXG867KGgHMKwA@mail.gmail.com","threadId":"37381","inReplyTo":"1408485987-3590-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 00/18] Signed push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-20T02:39:35Z","receivedAt":"2014-08-20T02:39:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, Aug 19, 2014 at 3:06 PM, Junio C Hamano wrote:\n>\n> If the server's GPG keychain and pre-receive hook are properly set\n> up, a \"git push --signed\" over an unauthenticated and unencrypted\n> communication channel (aka \"git daemon\") can be made as secure as,\n> and even more secure than, the authenticated \"git push ssh://\".\n>\n> With the signed push certificate, together with the connectivity\n> check done when the server accepts the packed data, we are assured\n> that the trusted user vouches for the history leading to the\n> proposed tips of refs (aka \"new-sha1\"s), and a man-in-the-middle\n> would not be able to make the server accept an update altered in\n> transit.\n\nThe above was written long after the series was done, but rethinking\nabout it within that frame of mind helped me find one nit in the push\ncertificate design.\n\nWe would need a PKT-LINE(\"pushed-to\" SP URL LF) in the header\npart to prevent the certificate and the same set of refs from getting\nreplayed. Otherwise, a different repository that happens to have the\nsame set of refs with same set of old-sha1 values can be made to\naccept a push that the signer never intended to make to it by replaying\nan earlier push to a different site the signer did make.\n"},{"id":"247997","messageId":"20140820024801.GA198803@vauxhall.crustytoothpaste.net","threadId":"37381","inReplyTo":"1408485987-3590-16-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 15/18] the beginning of the signed push","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-08-20T02:48:01Z","receivedAt":"2014-08-20T02:48:01Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Aug 19, 2014 at 03:06:24PM -0700, Junio C Hamano wrote:\n> While signed tags and commits assert that the objects thusly signed\n> came from you, who signed these objects, there is not a good way to\n> assert that you wanted to have a particular object at the tip of a\n> particular branch. My signing v2.0.1 tag only means I want to call\n> the version v2.0.1, and it does not mean I want to push it out to my\n> 'master' branch---it is likely that I only want it in 'maint'.\n> \n> Introduce a mechanism that allows you to sign a \"push certificate\"\n> (for the lack of better name) every time you push, asserting that\n> what object you are pushing to update which ref that used to point\n> at what other object. Think of it as a cryptographic protection for\n> ref updates, similar to signed tags/commits but working on an\n> orthogonal axis.\n> \n> The basic flow based on this mechanism goes like this:\n> \n> 1. You push out your work with \"git push -s\".\n\nYou wrote \"git push -s\", but the command below only seems to understand\n--signed, not -s. It should probably be consistent.\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index f50e3d5..ae56f73 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -506,6 +506,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BIT(0, \"no-verify\", &flags, N_(\"bypass pre-push hook\"), TRANSPORT_PUSH_NO_HOOK),\n> \t\tOPT_BIT(0, \"follow-tags\", &flags, N_(\"push missing but relevant tags\"),\n> \t\t\tTRANSPORT_PUSH_FOLLOW_TAGS),\n> +\t\tOPT_BIT(0, \"signed\", &flags, N_(\"GPG sign the push\"), TRANSPORT_PUSH_CERT),\n> \t\tOPT_END()\n> \t};\n> \n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"247998","messageId":"CAPc5daWJj1oM3ebc59sbpORggoigyq-hSOvfc0ueFHD=WeCYWg@mail.gmail.com","threadId":"37381","inReplyTo":"20140820011935.GC3104@localhost","subject":"Re: [PATCH 00/18] Signed push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-20T02:54:48Z","receivedAt":"2014-08-20T02:54:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sorry, but I cannot answer, as the only thing that I recall when\nI hear \"branch object\" was that I heard the phrase used but\nwithout much substance.\n\nI do not think I saw a clear explanation on what it is, how it is\nrepresented, how it is presented to the end user, how it is\npropagated across repositories (if it is to be propagated),\nhow it is stored (if it is to be stored), how it is to be used,\netc. and such crucial details necessary to judge the merit\nof thinking about introducing one. Perhaps it was explained\nin the old thread, but I don't recall, so I cannot judge how\nuseful it would be to solve the problem the push certificate is\ntrying to solve.\n\n\nOn Tue, Aug 19, 2014 at 6:19 PM, Nico Williams wrote:\n> On Tue, Aug 19, 2014 at 03:06:09PM -0700, Junio C Hamano wrote:\n>> While signed tags and commits assert that the objects thusly signed\n>> came from you, who signed these objects, there is not a good way to\n>> assert that you wanted to have a particular object at the tip of a\n>> particular branch. My signing v2.0.1 tag only means I want to call\n>> the version v2.0.1, and it does not mean I want to push it out to my\n>> 'master' branch---it is likely that I only want it in 'maint', so\n>> the signature on the object alone is insufficient.\n>>\n>> [...]\n>>\n>> This series introduces a cryptographic assurance for ref updates\n>> done by \"git push\" by introducing a mechanism that allows you to\n>> sign a \"push certificate\" (for the lack of better name) every time\n>> you push. Think of it as working on an axis orthogonal to the\n>> traditional \"signed tags\".\n>\n> Sounds a lot like the \"branch object\" concept I suggest earlier, where\n> each push would also push a commit to a branch object describing the\n> updates to the branch, including signing of the updates to the branch\n> (hey, it's just a signed commit), groups of commits pushed together / to\n> be backed out together, rebase history, ... (What about pushing\n> orphaned commits?)\n>\n> Code-wise, would that be more or less generic?\n>\n> Nico\n> --\n"},{"id":"248000","messageId":"CAPc5daWTeiW_4k9nCH_CQGPPva-dLtmWFu4KN-cxxw6J1MQFUw@mail.gmail.com","threadId":"37381","inReplyTo":"CAPc5daWJj1oM3ebc59sbpORggoigyq-hSOvfc0ueFHD=WeCYWg@mail.gmail.com","subject":"Re: [PATCH 00/18] Signed push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-20T05:57:56Z","receivedAt":"2014-08-20T05:57:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, Aug 19, 2014 at 7:54 PM, Junio C Hamano wrote:\n> Sorry, but I cannot answer, as the only thing that I recall when\n> I hear \"branch object\" was that I heard the phrase used but\n> without much substance.\n\nJust to avoid unnecessary misunderstanding, by the above, especially\nthe \"without much substance\" part, I do not mean that those without\ncode have no say in the way the project improves its product. It is true\nthat this project does not operate in such a way that visionaries throw\nideas for coding minions to implement. A person with an itch and idea\non how to scratch that itch is expected to lead the design and code,\npossibly with help from others. But in order to ask others evaluate and\njoin force to help with the design and make it real with code, you need\nto present your idea to sufficient level of detail to be actionable.\n\nBy \"actionable\", consider the level of detail in which the proposed log\nmessage (not code or documentation update) of PATCH 15/18. Even\nthough the message alone does not give any working code, or it does\nnot even spell out the byte-level detail of how the protocol messages\nlook like, people should be able to read enough details such as what\nkind of information a push certificate is to contain, when a certificate is\ncreated, how it is transferred, when and by whom it is received, how it\nwill be used by the receiver, how a server operator can tweak his or\nher system to make use of the information, and how the newly added\nsystem component would fit into the existing system. In other words,\nthe description should be sufficient to assess both how useful the end\nresult would be, how much new work needs to be done to get there,\nand how well the resulting system as a whole would fit together well.\n\nI went back to the old thread to re-read the mention of \"branch object\",\nbut I did not get an impression that the idea was presented to the\nactionable level of detail; at least not yet.\n"},{"id":"248001","messageId":"CAK3OfOjVfaRwcuqRd6VQ1T7HkHUd9Oe1raxSa1zz5PrKuWWBmA@mail.gmail.com","threadId":"37381","inReplyTo":"CAPc5daXKwW-T6PeYtFYdHYuBzvsDXWyYdNOYXXG867KGgHMKwA@mail.gmail.com","subject":"Re: [PATCH 00/18] Signed push","fromName":"Nico Williams","fromEmail":"nico@cryptonector.com","sentAt":"2014-08-20T06:28:39Z","receivedAt":"2014-08-20T06:28:39Z","isPatch":true,"sender":{"key":"nico@cryptonector.com","avatar":null},"body":"No code == no substance might be a stretch, but definitely fair enough.\n\nI thought the idea was clear enough, but I can flesh it out if\ndesired. The particular advantage I saw in it is that it would reuse\nthe existing object infrastructure, and extend to branches the\nfirst-class treatment that [signed] tags already get. I.e.,\ngenerality. Other benefits include the ability to fetch and view a\nremote branch's object and its history (which would represent branch\nhistory in detail and with metadata that would otherwise not be\navailable).\n\nI'm not interested in pushing something different when you already\nhave code that would achieve much of what I'd wanted. At this point\nmy interest is in seeing if the architecture would be \"purer\" (in some\nsense) by reusing existing infrastructure.\n\nNico\n--\n"},{"id":"248002","messageId":"CAKPyHN0Q2bCiNy0goxZ7YnmPjbU9fyjpWYmPF8gKz8T862jvAA@mail.gmail.com","threadId":"37381","inReplyTo":"1408485987-3590-16-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 15/18] the beginning of the signed push","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2014-08-20T06:57:51Z","receivedAt":"2014-08-20T06:57:51Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Aug 20, 2014 at 12:06 AM, Junio C Hamano wrote:\n> The basic flow based on this mechanism goes like this:\n>\n> 1. You push out your work with \"git push -s\".\n>\n> 2. The sending side learns where the remote refs are as usual,\n> together with what protocol extension the receiving end\n> supports. If the receiving end does not advertise the protocol\n> extension \"push-cert\", the sending side falls back to the normal\n> push that is not signed.\n>\n\nIs this fallback silently? If so I think it would be better to abort\nthe push, if the receiver does not support this but the user requested\nit.\n\nBert\n"},{"id":"248020","messageId":"1408553797.26173.3.camel@leckie","threadId":"37381","inReplyTo":"1408485987-3590-17-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 16/18] receive-pack: GPG-validate push certificates","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-08-20T16:56:37Z","receivedAt":"2014-08-20T16:56:37Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2014-08-19 at 15:06 -0700, Junio C Hamano wrote:\n> Reusing the GPG signature check helpers we already have, verify\n> the signature in receive-pack and give the results to the hooks\n> via GIT_PUSH_CERT_{SIGNER,KEY,STATUS} environment variables.\n> \n> Policy decisions, such as accepting or rejecting a good signature by\n> a key that is not fully trusted, is left to the hook and kept\n> outside of the core.\n\nIf I understand correctly, the hook does not have enough information to\nmake this decision, because it is missing the date from the signature.\nThis might allow an old signed push to be replayed, moving the head of a\nbranch to an older state (say, one lacking the latest security updates).\nI have not proven this, and it is entirely possible that I am wrong, but\nI think it would be worth either documenting why this is not possible,\nor fixing it if it is possible.\n"},{"id":"248021","messageId":"CAPc5daXLBQdjPaWNxmwZqStiWu8qHRfDG6=JS=bhbzA+c5Ww5g@mail.gmail.com","threadId":"37381","inReplyTo":"1408553797.26173.3.camel@leckie","subject":"Re: [PATCH 16/18] receive-pack: GPG-validate push certificates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-20T17:29:04Z","receivedAt":"2014-08-20T17:29:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Wed, Aug 20, 2014 at 9:56 AM, David Turner wrote:\n> On Tue, 2014-08-19 at 15:06 -0700, Junio C Hamano wrote:\n>> Reusing the GPG signature check helpers we already have, verify\n>> the signature in receive-pack and give the results to the hooks\n>> via GIT_PUSH_CERT_{SIGNER,KEY,STATUS} environment variables.\n>>\n>> Policy decisions, such as accepting or rejecting a good signature by\n>> a key that is not fully trusted, is left to the hook and kept\n>> outside of the core.\n>\n> If I understand correctly, the hook does not have enough information to\n> make this decision, because it is missing the date from the signature.\n\nThe full certificate is available to the hook so anything we can do the hook\nhas enough information to do ;-) But of course we should try to make it\neasier for the hook to validate the request.\n\nI am not opposed to extract the timestamp from pushed-by header in the cert\nand export it in another environment before calling the hook, but I am not sure\nit is worth it, as that is already a single liner text information.\n\n> This might allow an old signed push to be replayed, moving the head of a\n> branch to an older state (say, one lacking the latest security updates).\n\n... with old-sha1 recorded in the certificate?\n"},{"id":"248025","messageId":"1408557408.1282.5.camel@leckie","threadId":"37381","inReplyTo":"CAPc5daXLBQdjPaWNxmwZqStiWu8qHRfDG6=JS=bhbzA+c5Ww5g@mail.gmail.com","subject":"Re: [PATCH 16/18] receive-pack: GPG-validate push certificates","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-08-20T17:56:48Z","receivedAt":"2014-08-20T17:56:48Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2014-08-20 at 10:29 -0700, Junio C Hamano wrote:\n> On Wed, Aug 20, 2014 at 9:56 AM, David Turner wrote:\n> > On Tue, 2014-08-19 at 15:06 -0700, Junio C Hamano wrote:\n> >> Reusing the GPG signature check helpers we already have, verify\n> >> the signature in receive-pack and give the results to the hooks\n> >> via GIT_PUSH_CERT_{SIGNER,KEY,STATUS} environment variables.\n> >>\n> >> Policy decisions, such as accepting or rejecting a good signature by\n> >> a key that is not fully trusted, is left to the hook and kept\n> >> outside of the core.\n> >\n> > If I understand correctly, the hook does not have enough information to\n> > make this decision, because it is missing the date from the signature.\n> \n> The full certificate is available to the hook so anything we can do the hook\n> has enough information to do ;-) But of course we should try to make it\n> easier for the hook to validate the request.\n\nExcellent, then motivated hooks can do the right thing.\n\n> > This might allow an old signed push to be replayed, moving the head of a\n> > branch to an older state (say, one lacking the latest security updates).\n> \n> ... with old-sha1 recorded in the certificate?\n\nThat does prevent most replays, but it does not prevent resurrection of\na deleted branch by a replay of its initial creation (nor an undo of a\nforce-push to rollback). So I think we still need timestamps, but\nparsing them out of the cert is not terrible.\n"},{"id":"248028","messageId":"xmqqr40bt0i0.fsf@gitster.dls.corp.google.com","threadId":"37381","inReplyTo":"1408557408.1282.5.camel@leckie","subject":"Re: [PATCH 16/18] receive-pack: GPG-validate push certificates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-20T19:38:31Z","receivedAt":"2014-08-20T19:38:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> On Wed, 2014-08-20 at 10:29 -0700, Junio C Hamano wrote:\n>> On Wed, Aug 20, 2014 at 9:56 AM, David Turner wrote:\n>> > On Tue, 2014-08-19 at 15:06 -0700, Junio C Hamano wrote:\n>> >> Reusing the GPG signature check helpers we already have, verify\n>> >> the signature in receive-pack and give the results to the hooks\n>> >> via GIT_PUSH_CERT_{SIGNER,KEY,STATUS} environment variables.\n>> >>\n>> >> Policy decisions, such as accepting or rejecting a good signature by\n>> >> a key that is not fully trusted, is left to the hook and kept\n>> >> outside of the core.\n>> >\n>> > If I understand correctly, the hook does not have enough information to\n>> > make this decision, because it is missing the date from the signature.\n>> \n>> The full certificate is available to the hook so anything we can do the hook\n>> has enough information to do ;-) But of course we should try to make it\n>> easier for the hook to validate the request.\n>\n> Excellent, then motivated hooks can do the right thing.\n>\n>> > This might allow an old signed push to be replayed, moving the head of a\n>> > branch to an older state (say, one lacking the latest security updates).\n>> \n>> ... with old-sha1 recorded in the certificate?\n>\n> That does prevent most replays, but it does not prevent resurrection of\n> a deleted branch by a replay of its initial creation (nor an undo of a\n> force-push to rollback). So I think we still need timestamps, but\n> parsing them out of the cert is not terrible.\n\nAs I aleady mentioned elsewhere, a more problematic thing about the\npush certificate as presented in 15/18 is that it does not say\nanything about where the push is going. If you can capture a trial\npush to some random test repository I do with my signed push\ncertificate, you could replay it to my public repository hosted at\na more official site (say, k.org in the far distant future where it\ndoes not rely on ssh authentication to protect their services but\nuses the GPG signature on the push certificate to make sure it is I\nwho is pushing).\n\nWe can add a new \"pushed-to \" header line to the\ncertificate, next to \"pushed-by