{"thread":{"id":"28330","subject":"[PATCH 1/2] send-pack: typofix error message","startedAt":"2011-09-07T20:56:38Z","lastAt":"2011-09-09T19:12:40Z","messageCount":26,"participants":["Junio C Hamano","Shawn Pearce","Nguyen Thai Ngoc Duy","Robin H. Johnson","Johan Herland","Jeff King","Joey Hess","Drew Northup"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"175030","messageId":"7vfwk82hrt.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":null,"subject":"[PATCH 1/2] send-pack: typofix error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-07T20:56:38Z","receivedAt":"2011-09-07T20:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The message identifies the process as receive-pack when it cannot fork the\nsideband demultiplexer. We are actually a send-pack.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/send-pack.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex c1f6ddd..87833f4 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -334,7 +334,7 @@ int send_pack(struct send_pack_args *args,\n \t\tdemux.data = fd;\n \t\tdemux.out = -1;\n \t\tif (start_async(&demux))\n-\t\t\tdie(\"receive-pack: unable to fork off sideband demultiplexer\");\n+\t\t\tdie(\"send-pack: unable to fork off sideband demultiplexer\");\n \t\tin = demux.out;\n \t}\n \n-- \n1.7.7.rc0.186.g50963\n"},{"id":"175031","messageId":"7vbouw2hqg.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"7vfwk82hrt.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] push -s: skeleton","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-07T20:57:27Z","receivedAt":"2011-09-07T20:57:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If a tag is GPG-signed, and if you trust the cryptographic robustness of\nthe SHA-1 and GPG, you can guarantee that all the history leading to the\nsigned commit is not tampered with. However, it would be both cumbersome\nand cluttering to sign each and every commit. Especially if you strive to\nkeep your history clean by tweaking, rewriting and polishing your commits\nbefore pushing the resulting history out, many commits you will create\nlocally end up not mattering at all, and it is a waste of time to sign\nthem.\n\nA better alternative could be to sign a \"push certificate\" (for the lack\nof better name) every time you push, asserting that what commits you are\npushing to update which refs. The basic workflow goes like this:\n\n 1. You push out your work with \"git push -s\";\n\n 2. \"git push\", as usual, learns where the remote refs are and which refs\n are to be updated with this push. It prepares a text file in memory\n that looks like this using this information:\n\n\tPush-Certificate-Version: 1\n\tPusher: Junio C Hamano 1315427886 -0700\n\tUpdate: e83c51633... d4e58965f... refs/heads/master\n\tUpdate: 5a144a288... 7931f38a2... refs/heads/next\n\n An actual push certificate records full 40-char object name, but it is\n ellided for brevity here.\n\n The user then is asked to sign this push certificate using GPG. The\n result is carried to the other side (i.e. receive-pack). In the\n protocol exchange, this step comes immediately after the sender tells\n what the result of the push should be, before it sends the pack data.\n\n 3. The receiving end will keep the signed push certificate in core,\n receives the pack data and unpacks (or stores and runs index-pack)\n as usual.\n\n 4. A new phase to record the push certificate is introduced in the\n codepath after the receiving end runs receive_hook(). It is envisioned\n that this phase:\n\n a. parses the updated-to object names, and appends the push\n certificate (still GPG signed) to a note attached to each of the\n objects that will sit at the tip of the refs;\n\n b. verifies that the push certificate is signed with a GPG key that is\n authorized to push into this repository; and/or\n\n c. invokes pre-push-verify-signature hook, feeds the push\n certificate to it and asks it to veto the ref updates.\n\nAnd here is a skeleton to implement it. It has all the necessary protocol\nextensions implemented (although I do not know if we need separate\ncodepath for stateless RPC mode), but does not have subroutines to:\n\n - Sign the certificate with GPG key;\n\n - Parse the signed certificate to identify the updated-to objects, and\n add the certificate as notes to them;\n\n - Verify the certificate and find out what GPG key was used to sign it;\n or\n\n - Invoke and feed the certificate to pre-push-verify-hook.\n\nall of which should be fairly trivial. The places that needs to implement\nthese are clearly marked with large comments, so I'll leave it up to other\npeople who are interested in the topic to fill in the blanks ;-)\n\nSigned-off-by: Junio C Hamano \n---\n builtin/push.c | 1 +\n builtin/receive-pack.c | 54 +++++++++++++++++++++++++++++++++++++++-\n builtin/send-pack.c | 65 +++++++++++++++++++++++++++++++++++++++++++++---\n send-pack.h | 1 +\n transport.c | 4 +++\n transport.h | 4 +++\n 6 files changed, 124 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 35cce53..2238f4e 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -261,6 +261,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('u', \"set-upstream\", &flags, \"set upstream for git pull/status\",\n \t\t\tTRANSPORT_PUSH_SET_UPSTREAM),\n \t\tOPT_BOOLEAN(0, \"progress\", &progress, \"force progress reporting\"),\n+\t\tOPT_BIT('s', \"signed\", &flags, \"GPG sign the push\", TRANSPORT_PUSH_SIGNED),\n \t\tOPT_END()\n \t};\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ae164da..307fc3b 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -30,12 +30,14 @@ static int receive_unpack_limit = -1;\n static int transfer_unpack_limit = -1;\n static int unpack_limit = 100;\n static int report_status;\n+static int signed_push;\n static int use_sideband;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n static int auto_gc = 1;\n static const char *head_name;\n static int sent_capabilities;\n+static char *push_certificate;\n \n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n@@ -114,7 +116,7 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void\n \telse\n \t\tpacket_write(1, \"%s %s%c%s%s\\n\",\n \t\t\t sha1_to_hex(sha1), path, 0,\n-\t\t\t \" report-status delete-refs side-band-64k\",\n+\t\t\t \" report-status delete-refs side-band-64k signed-push\",\n \t\t\t prefer_ofs_delta ? \" ofs-delta\" : \"\");\n \tsent_capabilities = 1;\n \treturn 0;\n@@ -579,6 +581,31 @@ static void check_aliased_updates(struct command *commands)\n \tstring_list_clear(&ref_list, 0);\n }\n \n+static int record_signed_push(char *cert)\n+{\n+\t/*\n+\t * This is the place for you to parse the signed push\n+\t * certificate, grab the commit object names the push updates\n+\t * refs to, and append the certificate to the notes to these\n+\t * commits.\n+\t *\n+\t * You could also feed the signed push certificate to GPG,\n+\t * verify the signer identity, and all the other fun stuff,\n+\t * including feeding it to \"pre-push-verify-signature\" hook.\n+\t *\n+\t * Here we just throw it to stderr to demonstrate that the\n+\t * codepath is being exercised.\n+\t */\n+\tchar *cp, *ep;\n+\tfor (cp = cert; *cp; cp = ep) {\n+\t\tep = strchrnul(cp, '\\n');\n+\t\tif (*ep == '\\n')\n+\t\t\tep++;\n+\t\tfprintf(stderr, \"RSP: %.*s\", (int)(ep - cp), cp);\n+\t}\n+\treturn 0;\n+}\n+\n static void execute_commands(struct command *commands, const char *unpacker_error)\n {\n \tstruct command *cmd;\n@@ -596,6 +623,12 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n \t\treturn;\n \t}\n \n+\tif (push_certificate && record_signed_push(push_certificate)) {\n+\t\tfor (cmd = commands; cmd; cmd = cmd->next)\n+\t\t\tcmd->error_string = \"n/a (push signature error)\";\n+\t\treturn;\n+\t}\n+\n \tcheck_aliased_updates(commands);\n \n \thead_name = resolve_ref(\"HEAD\", sha1, 0, NULL);\n@@ -636,6 +669,8 @@ static struct command *read_head_info(void)\n \t\t\t\treport_status = 1;\n \t\t\tif (strstr(refname + reflen + 1, \"side-band-64k\"))\n \t\t\t\tuse_sideband = LARGE_PACKET_MAX;\n+\t\t\tif (strstr(refname + reflen + 1, \"signed-push\"))\n+\t\t\t\tsigned_push = 1;\n \t\t}\n \t\tcmd = xcalloc(1, sizeof(struct command) + len - 80);\n \t\thashcpy(cmd->old_sha1, old_sha1);\n@@ -731,6 +766,21 @@ static const char *unpack(void)\n \t}\n }\n \n+static char *receive_push_certificate(void)\n+{\n+\tstruct strbuf cert = STRBUF_INIT;\n+\tfor (;;) {\n+\t\tchar line[1000];\n+\t\tint len;\n+\n+\t\tlen = packet_read_line(0, line, sizeof(line));\n+\t\tif (!len)\n+\t\t\tbreak;\n+\t\tstrbuf_add(&cert, line, len);\n+\t}\n+\treturn strbuf_detach(&cert, NULL);\n+}\n+\n static void report(struct command *commands, const char *unpack_status)\n {\n \tstruct command *cmd;\n@@ -846,6 +896,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \tif ((commands = read_head_info()) != NULL) {\n \t\tconst char *unpack_status = NULL;\n \n+\t\tif (signed_push)\n+\t\t\tpush_certificate = receive_push_certificate();\n \t\tif (!delete_only(commands))\n \t\t\tunpack_status = unpack();\n \t\texecute_commands(commands, unpack_status);\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 87833f4..3193f34 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -237,6 +237,27 @@ static int sideband_demux(int in, int out, void *data)\n \treturn ret;\n }\n \n+static void sign_push_certificate(struct strbuf *cert)\n+{\n+\t/*\n+\t * Here, take the contents of cert->buf, and have the user GPG\n+\t * sign it, and read it back in the strbuf.\n+\t *\n+\t * You may want to append some extra info to cert before giving\n+\t * it to GPG, possibly via a hook.\n+\t *\n+\t * Here we upcase them just to demonstrate that the codepath\n+\t * is being exercised.\n+\t */\n+\tchar *cp;\n+\tfor (cp = cert->buf; *cp; cp++) {\n+\t\tint ch = *cp;\n+\t\tif ('a' <= ch && ch <= 'z')\n+\t\t\t*cp = toupper(ch);\n+\t}\n+\treturn;\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@@ -250,9 +271,11 @@ int send_pack(struct send_pack_args *args,\n \tint allow_deleting_refs = 0;\n \tint status_report = 0;\n \tint use_sideband = 0;\n+\tint signed_push = 0;\n \tunsigned cmds_sent = 0;\n \tint ret;\n \tstruct async demux;\n+\tstruct strbuf push_cert = STRBUF_INIT;\n \n \t/* Does the other end support the reporting? */\n \tif (server_supports(\"report-status\"))\n@@ -270,6 +293,19 @@ int send_pack(struct send_pack_args *args,\n \t\treturn 0;\n \t}\n \n+\tif (args->signed_push) {\n+\t\tif (server_supports(\"signed-push\"))\n+\t\t\tsigned_push = !args->dry_run;\n+\t\telse\n+\t\t\twarning(\"The receiving side does not support signed-push\");\n+\t}\n+\n+\tif (signed_push) {\n+\t\tconst char *committer_info = git_committer_info(0);\n+\t\tstrbuf_addstr(&push_cert, \"Push-Certificate-Version: 1\\n\");\n+\t\tstrbuf_addf(&push_cert, \"Pusher: %s\\n\", committer_info);\n+\t}\n+\n \t/*\n \t * Finally, tell the other end!\n \t */\n@@ -301,15 +337,19 @@ int send_pack(struct send_pack_args *args,\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 && (status_report || use_sideband)) {\n-\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s\",\n+\t\t\tif (!cmds_sent &&\n+\t\t\t (status_report || use_sideband || signed_push))\n+\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s\",\n \t\t\t\t\told_hex, new_hex, ref->name, 0,\n \t\t\t\t\tstatus_report ? \" report-status\" : \"\",\n-\t\t\t\t\tuse_sideband ? \" side-band-64k\" : \"\");\n-\t\t\t}\n+\t\t\t\t\tuse_sideband ? \" side-band-64k\" : \"\",\n+\t\t\t\t\tsigned_push ? \" signed-push\" : \"\");\n \t\t\telse\n \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n \t\t\t\t\told_hex, new_hex, ref->name);\n+\t\t\tif (signed_push && hashcmp(ref->old_sha1, ref->new_sha1))\n+\t\t\t\tstrbuf_addf(&push_cert, \"Update: %s %s %s\\n\",\n+\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@@ -326,6 +366,23 @@ int send_pack(struct send_pack_args *args,\n \t\tsafe_write(out, req_buf.buf, req_buf.len);\n \t\tpacket_flush(out);\n \t}\n+\n+\tif (signed_push) {\n+\t\tchar *cp, *ep;\n+\n+\t\tsign_push_certificate(&push_cert);\n+\t\tstrbuf_reset(&req_buf);\n+\t\tfor (cp = push_cert.buf; *cp; cp = ep) {\n+\t\t\tep = strchrnul(cp, '\\n');\n+\t\t\tif (*ep == '\\n')\n+\t\t\t\tep++;\n+\t\t\tpacket_buf_write(&req_buf, \"%.*s\",\n+\t\t\t\t\t (int)(ep - cp), cp);\n+\t\t}\n+\t\t/* Do we need anything funky for stateless rpc? */\n+\t\tsafe_write(out, req_buf.buf, req_buf.len);\n+\t\tpacket_flush(out);\n+\t}\n \tstrbuf_release(&req_buf);\n \n \tif (use_sideband && cmds_sent) {\ndiff --git a/send-pack.h b/send-pack.h\nindex 05d7ab1..754943e 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\tsigned_push:1,\n \t\tstateless_rpc:1;\n };\n \ndiff --git a/transport.c b/transport.c\nindex fa279d5..7a7ffe4 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -476,6 +476,9 @@ static int set_git_option(struct git_transport_options *opts,\n \t\telse\n \t\t\topts->depth = atoi(value);\n \t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_SIGNED_PUSH)) {\n+\t\topts->signed_push = !!value;\n+\t\treturn 0;\n \t}\n \treturn 1;\n }\n@@ -793,6 +796,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.signed_push = !!(flags & TRANSPORT_PUSH_SIGNED);\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 059b330..d2fa478 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -8,6 +8,7 @@ struct git_transport_options {\n \tunsigned thin : 1;\n \tunsigned keep : 1;\n \tunsigned followtags : 1;\n+\tunsigned signed_push : 1;\n \tint depth;\n \tconst char *uploadpack;\n \tconst char *receivepack;\n@@ -102,6 +103,7 @@ struct transport {\n #define TRANSPORT_PUSH_PORCELAIN 16\n #define TRANSPORT_PUSH_SET_UPSTREAM 32\n #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64\n+#define TRANSPORT_PUSH_SIGNED 128\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n \n@@ -128,6 +130,8 @@ struct transport *transport_get(struct remote *, const char *);\n /* Aggressively fetch annotated tags if possible */\n #define TRANS_OPT_FOLLOWTAGS \"followtags\"\n \n+#define TRANS_OPT_SIGNED_PUSH \"signedpush\"\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-- \n1.7.7.rc0.186.g50963\n"},{"id":"175037","messageId":"CAJo=hJtz6fa4XfC-4ghryP_nfg3sbcrE2bKauj+F7w2Z_8Ckvw@mail.gmail.com","threadId":"28330","inReplyTo":"7vbouw2hqg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2011-09-07T21:18:52Z","receivedAt":"2011-09-07T21:18:52Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Wed, Sep 7, 2011 at 13:57, Junio C Hamano wrote:\n> If a tag is GPG-signed, and if you trust the cryptographic robustness of\n> the SHA-1 and GPG, you can guarantee that all the history leading to the\n> signed commit is not tampered with. However, it would be both cumbersome\n> and cluttering to sign each and every commit. Especially if you strive to\n> keep your history clean by tweaking, rewriting and polishing your commits\n> before pushing the resulting history out, many commits you will create\n> locally end up not mattering at all, and it is a waste of time to sign\n> them.\n>\n> A better alternative could be to sign a \"push certificate\" (for the lack\n> of better name) every time you push, asserting that what commits you are\n> pushing to update which refs. The basic workflow goes like this:\n>\n>  1. You push out your work with \"git push -s\";\n\nYay!\n\n> And here is a skeleton to implement it. It has all the necessary protocol\n> extensions implemented (although I do not know if we need separate\n> codepath for stateless RPC mode), but does not have subroutines to:\n\nYea, its broken for stateless RPC. See below.\n\n> +static char *receive_push_certificate(void)\n> +{\n> +       struct strbuf cert = STRBUF_INIT;\n> +       for (;;) {\n> +               char line[1000];\n\n1000 isn't enough for some certificates. Imagine pushing a Gerrit Code\nReview managed repository with 2M worth of advertisement data at once.\nYou can't sign that in 1000 bytes.\n\n> @@ -326,6 +366,23 @@ int send_pack(struct send_pack_args *args,\n>                safe_write(out, req_buf.buf, req_buf.len);\n>                packet_flush(out);\n>        }\n> +\n> +       if (signed_push) {\n> +               char *cp, *ep;\n> +\n> +               sign_push_certificate(&push_cert);\n> +               strbuf_reset(&req_buf);\n> +               for (cp = push_cert.buf; *cp; cp = ep) {\n> +                       ep = strchrnul(cp, '\\n');\n> +                       if (*ep == '\\n')\n> +                               ep++;\n> +                       packet_buf_write(&req_buf, \"%.*s\",\n> +                                        (int)(ep - cp), cp);\n> +               }\n> +               /* Do we need anything funky for stateless rpc? */\n\nYes. Above we flushed the req_buf and send that in an HTTP request.\nYou need to hoist this block above the \"if (args->stateless_rpc)\"\nsegment.\n\n-- \nShawn.\n"},{"id":"175048","messageId":"CACsJy8Cy_Nn3EExV0D=RWtft+1pc9RBdJgpmES4AeQgYsUfU3A@mail.gmail.com","threadId":"28330","inReplyTo":"7vbouw2hqg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-09-07T22:21:05Z","receivedAt":"2011-09-07T22:21:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Sep 8, 2011 at 6:57 AM, Junio C Hamano wrote:\n> A better alternative could be to sign a \"push certificate\" (for the lack\n> of better name) every time you push, asserting that what commits you are\n> pushing to update which refs. The basic workflow goes like this:\n>\n>  1. You push out your work with \"git push -s\";\n>\n>  2. \"git push\", as usual, learns where the remote refs are and which refs\n>    are to be updated with this push. It prepares a text file in memory\n>    that looks like this using this information:\n>\n>        Push-Certificate-Version: 1\n>        Pusher: Junio C Hamano 1315427886 -0700\n>        Update: e83c51633... d4e58965f... refs/heads/master\n>        Update: 5a144a288... 7931f38a2... refs/heads/next\n>\n>    An actual push certificate records full 40-char object name, but it is\n>    ellided for brevity here.\n>\n>    The user then is asked to sign this push certificate using GPG. The\n>    result is carried to the other side (i.e. receive-pack). In the\n>    protocol exchange, this step comes immediately after the sender tells\n>    what the result of the push should be, before it sends the pack data.\n>\n>  3. The receiving end will keep the signed push certificate in core,\n>    receives the pack data and unpacks (or stores and runs index-pack)\n>    as usual.\n>\n>  4. A new phase to record the push certificate is introduced in the\n>    codepath after the receiving end runs receive_hook(). It is envisioned\n>    that this phase:\n>\n>    a. parses the updated-to object names, and appends the push\n>       certificate (still GPG signed) to a note attached to each of the\n>       objects that will sit at the tip of the refs;\n\nI recall Gentoo wanted something like this (recording who pushes\nwhat). Pulling Robin in if he has any comments.\n-- \nDuy\n"},{"id":"175047","messageId":"7vpqjc0zaf.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"CAJo=hJtz6fa4XfC-4ghryP_nfg3sbcrE2bKauj+F7w2Z_8Ckvw@mail.gmail.com","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-07T22:21:12Z","receivedAt":"2011-09-07T22:21:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> Yes. Above we flushed the req_buf and send that in an HTTP request.\n> You need to hoist this block above the \"if (args->stateless_rpc)\"\n> segment.\n\nWhat do you mean by \"hoist\"? For the req advertisement, it seems that you\nare not hoisting anything but duplicating the code, turning safe_write()\nfollowed by flush into packet-buf-flush and sending the result over the\nsideband. Shouldn't this new data be sent over the sideband-to-http the\nsame way?\n\nUnless you do not want signed push over http, that is...\n\n builtin/send-pack.c | 10 +++++++---\n 1 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 3193f34..37e0313 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -379,9 +379,13 @@ int send_pack(struct send_pack_args *args,\n \t\t\tpacket_buf_write(&req_buf, \"%.*s\",\n \t\t\t\t\t (int)(ep - cp), cp);\n \t\t}\n-\t\t/* Do we need anything funky for stateless rpc? */\n-\t\tsafe_write(out, req_buf.buf, req_buf.len);\n-\t\tpacket_flush(out);\n+\t\tif (args->stateless_rpc) {\n+\t\t\tpacket_buf_flush(&req_buf);\n+\t\t\tsend_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);\n+\t\t} else {\n+\t\t\tsafe_write(out, req_buf.buf, req_buf.len);\n+\t\t\tpacket_flush(out);\n+\t\t}\n \t}\n \tstrbuf_release(&req_buf);\n \n"},{"id":"175049","messageId":"7vliu00yei.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"CACsJy8Cy_Nn3EExV0D=RWtft+1pc9RBdJgpmES4AeQgYsUfU3A@mail.gmail.com","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-07T22:40:21Z","receivedAt":"2011-09-07T22:40:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n>> ...\n>> 4. A new phase to record the push certificate is introduced in the\n>> codepath after the receiving end runs receive_hook(). It is envisioned\n>> that this phase:\n>>\n>> a. parses the updated-to object names, and appends the push\n>> certificate (still GPG signed) to a note attached to each of the\n>> objects that will sit at the tip of the refs;\n>\n> I recall Gentoo wanted something like this (recording who pushes\n> what). Pulling Robin in if he has any comments.\n\nAs the beauty of this approach is that we can update and tailor what the\nreceiving end does using the information given from the server, it is a\nstrange thing to do to chomp this list in the middle at a funny place\nhere.\n"},{"id":"175052","messageId":"CAJo=hJsLx1Q9ZDoxGn=dww5J-rO9GitH47rEme_1L8Lg0RmAqw@mail.gmail.com","threadId":"28330","inReplyTo":"7vpqjc0zaf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2011-09-07T23:23:13Z","receivedAt":"2011-09-07T23:23:13Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Wed, Sep 7, 2011 at 15:21, Junio C Hamano wrote:\n> Shawn Pearce writes:\n>\n>> Yes. Above we flushed the req_buf and send that in an HTTP request.\n>> You need to hoist this block above the \"if (args->stateless_rpc)\"\n>> segment.\n>\n> What do you mean by \"hoist\"? For the req advertisement, it seems that you\n> are not hoisting anything but duplicating the code, turning safe_write()\n> followed by flush into packet-buf-flush and sending the result over the\n> sideband. Shouldn't this new data be sent over the sideband-to-http the\n> same way?\n>\n> Unless you do not want signed push over http, that is...\n\nWe do.\n\n> diff --git a/builtin/send-pack.c b/builtin/send-pack.c\n> index 3193f34..37e0313 100644\n> --- a/builtin/send-pack.c\n> +++ b/builtin/send-pack.c\n> @@ -379,9 +379,13 @@ int send_pack(struct send_pack_args *args,\n>                        packet_buf_write(&req_buf, \"%.*s\",\n>                                         (int)(ep - cp), cp);\n>                }\n> -               /* Do we need anything funky for stateless rpc? */\n> -               safe_write(out, req_buf.buf, req_buf.len);\n> -               packet_flush(out);\n> +               if (args->stateless_rpc) {\n> +                       packet_buf_flush(&req_buf);\n> +                       send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);\n> +               } else {\n> +                       safe_write(out, req_buf.buf, req_buf.len);\n> +                       packet_flush(out);\n> +               }\n\nThis sounds too late to me. I think you just caused 2 HTTP POSTs, one\na partial one with the commands and no pack data, and another with the\npush certificate and the pack. Neither is useful.\n\n-- \nShawn.\n"},{"id":"175055","messageId":"robbat2-20110907T234637-463765607Z@orbis-terrarum.net","threadId":"28330","inReplyTo":"7vbouw2hqg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2011-09-07T23:55:44Z","receivedAt":"2011-09-07T23:55:44Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Wed, Sep 07, 2011 at 01:57:27PM -0700, Junio C Hamano wrote:\n> If a tag is GPG-signed, and if you trust the cryptographic robustness of\n> the SHA-1 and GPG, you can guarantee that all the history leading to the\n> signed commit is not tampered with. However, it would be both cumbersome\n> and cluttering to sign each and every commit. Especially if you strive to\n> keep your history clean by tweaking, rewriting and polishing your commits\n> before pushing the resulting history out, many commits you will create\n> locally end up not mattering at all, and it is a waste of time to sign\n> them.\nThanks to pcloud for including me on the thread. I do find the idea of\nthese push-certificates very interesting and useful, but I think they\nwill do best to augment signed commits, not replace them.\n\nThere's a couple of related things we've been considering on the Gentoo\nside:\n- detached signatures of blobs (either the SHA1 of the blob or the blob\n itself)\n- The signature covering the message+blob details, but NOT the chain of\n history: this opens up the ability to cherry-pick and rebase iff there\n are no conflicts and the blobs are identical, all while preserving the\n signature.\n- concerns about a pre-image attack against Git. tl;dr version:\n 1. Attacker prepares decoy file in advance, that hashes to the same as\n the malicious file.\n 2. Attacker sends decoy in as an innocuous real commit.\n 3. Months later, the attacker breaks into the system and alters the\n packfile to include the new malicious file.\n 4. All new clones from that point forward get the malicious version.\n\nRe your comment on always needing to resign commits above, we'd been\nconsidering post-signing commits, not when they are initially made.\nAfter your commit is clean and ready to ship, you can fire the commit\nids into the signature tool, which can generate a detached signature\nnote for each commit.\n\n-- \nRobin Hugh Johnson\nGentoo Linux: Developer, Trustee & Infrastructure Lead\nE-Mail : robbat2@gentoo.org\nGnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85\n"},{"id":"175062","messageId":"7vk49jzm0h.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"7vbouw2hqg.fsf@alter.siamese.dyndns.org","subject":"[PATCH 3/2] Split GPG interface into its own helper library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T04:37:24Z","receivedAt":"2011-09-08T04:37:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This moves existing code from builtin/tag.c (for signing) and\nbuiltin/verify-tag.c (for verifying) to a new gpg-interface.c file to\nprovide a more generic library interface.\n\nSigned-off-by: Junio C Hamano \n---\n Makefile | 2 +\n builtin/tag.c | 60 ++++----------------------------\n builtin/verify-tag.c | 35 ++-----------------\n gpg-interface.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++\n gpg-interface.h | 11 ++++++\n 5 files changed, 117 insertions(+), 85 deletions(-)\n create mode 100644 gpg-interface.c\n create mode 100644 gpg-interface.h\n\ndiff --git a/Makefile b/Makefile\nindex 8d6d451..2183223 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -530,6 +530,7 @@ LIB_H += exec_cmd.h\n LIB_H += fsck.h\n LIB_H += gettext.h\n LIB_H += git-compat-util.h\n+LIB_H += gpg-interface.h\n LIB_H += graph.h\n LIB_H += grep.h\n LIB_H += hash.h\n@@ -620,6 +621,7 @@ LIB_OBJS += entry.o\n LIB_OBJS += environment.o\n LIB_OBJS += exec_cmd.o\n LIB_OBJS += fsck.o\n+LIB_OBJS += gpg-interface.o\n LIB_OBJS += graph.o\n LIB_OBJS += grep.o\n LIB_OBJS += hash.o\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 667515e..e9d36fa 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -14,6 +14,7 @@\n #include \"parse-options.h\"\n #include \"diff.h\"\n #include \"revision.h\"\n+#include \"gpg-interface.h\"\n \n static const char * const git_tag_usage[] = {\n \t\"git tag [-a|-s|-u ] [-f] [-m |-F ] []\",\n@@ -208,60 +209,13 @@ static int verify_tag(const char *name, const char *ref,\n \n static int do_sign(struct strbuf *buffer)\n {\n-\tstruct child_process gpg;\n-\tconst char *args[4];\n-\tchar *bracket;\n-\tint len;\n-\tint i, j;\n+\tconst char *key;\n \n-\tif (!*signingkey) {\n-\t\tif (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),\n-\t\t\t\tsizeof(signingkey)) > sizeof(signingkey) - 1)\n-\t\t\treturn error(_(\"committer info too long.\"));\n-\t\tbracket = strchr(signingkey, '>');\n-\t\tif (bracket)\n-\t\t\tbracket[1] = '\\0';\n-\t}\n-\n-\t/* When the username signingkey is bad, program could be terminated\n-\t * because gpg exits without reading and then write gets SIGPIPE. */\n-\tsignal(SIGPIPE, SIG_IGN);\n-\n-\tmemset(&gpg, 0, sizeof(gpg));\n-\tgpg.argv = args;\n-\tgpg.in = -1;\n-\tgpg.out = -1;\n-\targs[0] = \"gpg\";\n-\targs[1] = \"-bsau\";\n-\targs[2] = signingkey;\n-\targs[3] = NULL;\n-\n-\tif (start_command(&gpg))\n-\t\treturn error(_(\"could not run gpg.\"));\n-\n-\tif (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {\n-\t\tclose(gpg.in);\n-\t\tclose(gpg.out);\n-\t\tfinish_command(&gpg);\n-\t\treturn error(_(\"gpg did not accept the tag data\"));\n-\t}\n-\tclose(gpg.in);\n-\tlen = strbuf_read(buffer, gpg.out, 1024);\n-\tclose(gpg.out);\n-\n-\tif (finish_command(&gpg) || !len || len < 0)\n-\t\treturn error(_(\"gpg failed to sign the tag\"));\n-\n-\t/* Strip CR from the line endings, in case we are on Windows. */\n-\tfor (i = j = 0; i < buffer->len; i++)\n-\t\tif (buffer->buf[i] != '\\r') {\n-\t\t\tif (i != j)\n-\t\t\t\tbuffer->buf[j] = buffer->buf[i];\n-\t\t\tj++;\n-\t\t}\n-\tstrbuf_setlen(buffer, j);\n-\n-\treturn 0;\n+\tif (*signingkey)\n+\t\tkey = signingkey;\n+\telse\n+\t\tkey = git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);\n+\treturn sign_buffer(buffer, key);\n }\n \n static const char tag_template[] =\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex 3134766..8b4f742 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -11,6 +11,7 @@\n #include \"run-command.h\"\n #include \n #include \"parse-options.h\"\n+#include \"gpg-interface.h\"\n \n static const char * const verify_tag_usage[] = {\n \t\t\"git verify-tag [-v|--verbose] ...\",\n@@ -19,42 +20,12 @@ static const char * const verify_tag_usage[] = {\n \n static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n {\n-\tstruct child_process gpg;\n-\tconst char *args_gpg[] = {\"gpg\", \"--verify\", \"FILE\", \"-\", NULL};\n-\tchar path[PATH_MAX];\n-\tsize_t len;\n-\tint fd, ret;\n+\tint len;\n \n-\tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n-\tif (fd < 0)\n-\t\treturn error(\"could not create temporary file '%s': %s\",\n-\t\t\t\t\t\tpath, strerror(errno));\n-\tif (write_in_full(fd, buf, size) < 0)\n-\t\treturn error(\"failed writing temporary file '%s': %s\",\n-\t\t\t\t\t\tpath, strerror(errno));\n-\tclose(fd);\n-\n-\t/* find the length without signature */\n \tlen = parse_signature(buf, size);\n \tif (verbose)\n \t\twrite_in_full(1, buf, len);\n-\n-\tmemset(&gpg, 0, sizeof(gpg));\n-\tgpg.argv = args_gpg;\n-\tgpg.in = -1;\n-\targs_gpg[2] = path;\n-\tif (start_command(&gpg)) {\n-\t\tunlink(path);\n-\t\treturn error(\"could not run gpg.\");\n-\t}\n-\n-\twrite_in_full(gpg.in, buf, len);\n-\tclose(gpg.in);\n-\tret = finish_command(&gpg);\n-\n-\tunlink_or_warn(path);\n-\n-\treturn ret;\n+\treturn verify_signed_buffer(buf, size, len);\n }\n \n static int verify_tag(const char *name, int verbose)\ndiff --git a/gpg-interface.c b/gpg-interface.c\nnew file mode 100644\nindex 0000000..b83cca1\n--- /dev/null\n+++ b/gpg-interface.c\n@@ -0,0 +1,94 @@\n+/*\n+ * Copyright (c) 2011, Google Inc.\n+ */\n+#include \"cache.h\"\n+#include \"run-command.h\"\n+#include \"strbuf.h\"\n+#include \"gpg-interface.h\"\n+#include \"sigchain.h\"\n+\n+int sign_buffer(struct strbuf *buffer, const char *signing_key)\n+{\n+\tstruct child_process gpg;\n+\tconst char *args[4];\n+\tssize_t len;\n+\tint i, j;\n+\n+\tmemset(&gpg, 0, sizeof(gpg));\n+\tgpg.argv = args;\n+\tgpg.in = -1;\n+\tgpg.out = -1;\n+\targs[0] = \"gpg\";\n+\targs[1] = \"-bsau\";\n+\targs[2] = signing_key;\n+\targs[3] = NULL;\n+\n+\tif (start_command(&gpg))\n+\t\treturn error(_(\"could not run gpg.\"));\n+\n+\t/*\n+\t * When the username signingkey is bad, program could be terminated\n+\t * because gpg exits without reading and then write gets SIGPIPE.\n+\t */\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\n+\tif (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {\n+\t\tclose(gpg.in);\n+\t\tclose(gpg.out);\n+\t\tfinish_command(&gpg);\n+\t\treturn error(_(\"gpg did not accept the data\"));\n+\t}\n+\tclose(gpg.in);\n+\tlen = strbuf_read(buffer, gpg.out, 1024);\n+\tclose(gpg.out);\n+\n+\tsigchain_pop(SIGPIPE);\n+\n+\tif (finish_command(&gpg) || !len || len < 0)\n+\t\treturn error(_(\"gpg failed to sign the data\"));\n+\n+\t/* Strip CR from the line endings, in case we are on Windows. */\n+\tfor (i = j = 0; i < buffer->len; i++)\n+\t\tif (buffer->buf[i] != '\\r') {\n+\t\t\tif (i != j)\n+\t\t\t\tbuffer->buf[j] = buffer->buf[i];\n+\t\t\tj++;\n+\t\t}\n+\tstrbuf_setlen(buffer, j);\n+\n+\treturn 0;\n+}\n+\n+int verify_signed_buffer(const char *buf, size_t total, size_t payload)\n+{\n+\tstruct child_process gpg;\n+\tconst char *args_gpg[] = {\"gpg\", \"--verify\", \"FILE\", \"-\", NULL};\n+\tchar path[PATH_MAX];\n+\tint fd, ret;\n+\n+\tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n+\tif (fd < 0)\n+\t\treturn error(\"could not create temporary file '%s': %s\",\n+\t\t\t path, strerror(errno));\n+\tif (write_in_full(fd, buf, total) < 0)\n+\t\treturn error(\"failed writing temporary file '%s': %s\",\n+\t\t\t path, strerror(errno));\n+\tclose(fd);\n+\n+\tmemset(&gpg, 0, sizeof(gpg));\n+\tgpg.argv = args_gpg;\n+\tgpg.in = -1;\n+\targs_gpg[2] = path;\n+\tif (start_command(&gpg)) {\n+\t\tunlink(path);\n+\t\treturn error(\"could not run gpg.\");\n+\t}\n+\n+\twrite_in_full(gpg.in, buf, payload);\n+\tclose(gpg.in);\n+\tret = finish_command(&gpg);\n+\n+\tunlink_or_warn(path);\n+\n+\treturn ret;\n+}\ndiff --git a/gpg-interface.h b/gpg-interface.h\nnew file mode 100644\nindex 0000000..7689357\n--- /dev/null\n+++ b/gpg-interface.h\n@@ -0,0 +1,11 @@\n+#ifndef GPG_INTERFACE_H\n+#define GPG_INTERFACE_H\n+\n+/*\n+ * Copyright (c) 2011, Google Inc.\n+ */\n+\n+extern int sign_buffer(struct strbuf *buffer, const char *signing_key);\n+extern int verify_signed_buffer(const char *buffer, size_t total, size_t payload);\n+\n+#endif\n-- \n1.7.7.rc0.188.g3793ac\n"},{"id":"175063","messageId":"7vehzrzm0e.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"7vbouw2hqg.fsf@alter.siamese.dyndns.org","subject":"[PATCH 4/2] push -s: send signed push certificate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T04:38:14Z","receivedAt":"2011-09-08T04:38:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"And this uses the GPG interface to sign the push certificate. The format\nof the signed certificate is very similar to a signed tag, in that the\nresult is a concatenation of the payload, immediately followed by a\ndetached signature.\n\nThis places the same constraint as an annotated tag on the push\ncertificate payload; it has to be a text file and the final line\nmust not be an incomplete line.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/send-pack.c | 29 ++++++++++++-----------------\n 1 files changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 3193f34..298e181 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -8,6 +8,7 @@\n #include \"send-pack.h\"\n #include \"quote.h\"\n #include \"transport.h\"\n+#include \"gpg-interface.h\"\n \n static const char send_pack_usage[] =\n \"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=] [--verbose] [--thin] [:] [...]\\n\"\n@@ -237,25 +238,18 @@ static int sideband_demux(int in, int out, void *data)\n \treturn ret;\n }\n \n-static void sign_push_certificate(struct strbuf *cert)\n+/*\n+ * Take the contents of cert->buf, and have the user GPG sign it, and\n+ * read it back in the strbuf.\n+ */\n+static int sign_push_certificate(struct strbuf *cert)\n {\n \t/*\n-\t * Here, take the contents of cert->buf, and have the user GPG\n-\t * sign it, and read it back in the strbuf.\n-\t *\n-\t * You may want to append some extra info to cert before giving\n-\t * it to GPG, possibly via a hook.\n-\t *\n-\t * Here we upcase them just to demonstrate that the codepath\n-\t * is being exercised.\n+\t * You may want to append some extra info to cert before\n+\t * giving it to GPG, possibly via a hook, here.\n \t */\n-\tchar *cp;\n-\tfor (cp = cert->buf; *cp; cp++) {\n-\t\tint ch = *cp;\n-\t\tif ('a' <= ch && ch <= 'z')\n-\t\t\t*cp = toupper(ch);\n-\t}\n-\treturn;\n+\n+\treturn sign_buffer(cert, git_committer_info(IDENT_NO_DATE));\n }\n \n int send_pack(struct send_pack_args *args,\n@@ -370,7 +364,8 @@ int send_pack(struct send_pack_args *args,\n \tif (signed_push) {\n \t\tchar *cp, *ep;\n \n-\t\tsign_push_certificate(&push_cert);\n+\t\tif (sign_push_certificate(&push_cert))\n+\t\t\treturn error(_(\"failed to sign push certificate\"));\n \t\tstrbuf_reset(&req_buf);\n \t\tfor (cp = push_cert.buf; *cp; cp = ep) {\n \t\t\tep = strchrnul(cp, '\\n');\n-- \n1.7.7.rc0.188.g3793ac\n"},{"id":"175065","messageId":"7v7h5jzj8o.fsf_-_@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"7vehzrzm0e.fsf@alter.siamese.dyndns.org","subject":"[PATCH 5/2] push -s: receiving end","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T05:38:31Z","receivedAt":"2011-09-08T05:38:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This stores the GPG signed push certificate in the receiving repository\nusing the notes mechanism. The certificate is appended to a note in the\nrefs/notes/signed-push tree for each object that appears on the right\nhand side of the push certificate, i.e. the object that was pushed to\nupdate the tip of a ref.\n\nSigned-off-by: Junio C Hamano \n---\n * This is largely untested, so take it with a large grain of salt.\n This concludes tonight's hacking session for me.\n\n builtin/receive-pack.c | 74 +++++++++++++++++++++++++++++++++++++++++++----\n 1 files changed, 67 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 307fc3b..257f2a5 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -11,6 +11,11 @@\n #include \"transport.h\"\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n+#include \"gpg-interface.h\"\n+#include \"notes.h\"\n+#include \"notes-merge.h\"\n+#include \"blob.h\"\n+#include \"tag.h\"\n \n static const char receive_pack_usage[] = \"git receive-pack \";\n \n@@ -156,6 +161,7 @@ struct command {\n };\n \n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n+static const char pre_receive_signature_hook[] = \"hooks/pre-receive-signature\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 2)));\n@@ -581,6 +587,22 @@ static void check_aliased_updates(struct command *commands)\n \tstring_list_clear(&ref_list, 0);\n }\n \n+static void get_note_text(struct strbuf *buf, struct notes_tree *t,\n+\t\t\t const unsigned char *object)\n+{\n+\tconst unsigned char *sha1 = get_note(t, object);\n+\tchar *text;\n+\tunsigned long len;\n+\tenum object_type type;\n+\n+\tif (!sha1)\n+\t\treturn;\n+\ttext = read_sha1_file(sha1, &type, &len);\n+\tif (text && len && type == OBJ_BLOB)\n+\t\tstrbuf_add(buf, text, len);\n+\tfree(text);\n+}\n+\n static int record_signed_push(char *cert)\n {\n \t/*\n@@ -591,19 +613,57 @@ static int record_signed_push(char *cert)\n \t *\n \t * You could also feed the signed push certificate to GPG,\n \t * verify the signer identity, and all the other fun stuff,\n-\t * including feeding it to \"pre-push-verify-signature\" hook.\n-\t *\n-\t * Here we just throw it to stderr to demonstrate that the\n-\t * codepath is being exercised.\n+\t * including feeding it to \"pre-receive-signature\" hook.\n \t */\n+\tsize_t total, payload;\n \tchar *cp, *ep;\n-\tfor (cp = cert; *cp; cp = ep) {\n+\tint ret = 0;\n+\tstruct notes_tree *t;\n+\tstruct strbuf nbuf = STRBUF_INIT;\n+\n+\tinit_notes(NULL, \"refs/notes/signed-push\", NULL, 0);\n+\tt = &default_notes_tree;\n+\n+\ttotal = strlen(cert);\n+\tpayload = parse_signature(cert, total);\n+\tfor (cp = cert; cp < cert + payload; cp = ep) {\n+\t\tunsigned char sha1[20], nsha1[20];\n+\n \t\tep = strchrnul(cp, '\\n');\n \t\tif (*ep == '\\n')\n \t\t\tep++;\n-\t\tfprintf(stderr, \"RSP: %.*s\", (int)(ep - cp), cp);\n+\t\tif (prefixcmp(cp, \"Update: \"))\n+\t\t\tcontinue;\n+\t\tcp += strlen(\"Update: \");\n+\t\tif (get_sha1_hex(cp, sha1) || cp[40] != ' ')\n+\t\t\tcontinue;\n+\t\tcp += 41;\n+\t\tif (get_sha1_hex(cp, sha1) || cp[40] != ' ')\n+\t\t\tcontinue;\n+\n+\t\tget_note_text(&nbuf, t, sha1);\n+\t\tif (nbuf.len)\n+\t\t\tstrbuf_addch(&nbuf, '\\n');\n+\t\tstrbuf_add(&nbuf, cert, total);\n+\t\tif (write_sha1_file(nbuf.buf, nbuf.len, blob_type, nsha1) ||\n+\t\t add_note(t, sha1, nsha1, NULL))\n+\t\t\tret = error(_(\"unable to write note object\"));\n+\t\tstrbuf_reset(&nbuf);\n \t}\n-\treturn 0;\n+\n+\tif (!ret) {\n+\t\tunsigned char commit[20];\n+\t\tunsigned char parent[20];\n+\t\tstruct ref_lock *lock;\n+\n+\t\tresolve_ref(t->ref, parent, 0, NULL);\n+\t\tlock = lock_any_ref_for_update(t->ref, parent, 0);\n+\t\tcreate_notes_commit(t, NULL, \"push\", commit);\n+\t\tret = write_ref_sha1(lock, commit, \"push\");\n+\t}\n+\tfree_notes(t);\n+\n+\treturn ret;\n }\n \n static void execute_commands(struct command *commands, const char *unpacker_error)\n-- \n1.7.7.rc0.188.g3793ac\n"},{"id":"175082","messageId":"201109081131.58362.johan@herland.net","threadId":"28330","inReplyTo":"7v7h5jzj8o.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/2] push -s: receiving end","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-09-08T09:31:58Z","receivedAt":"2011-09-08T09:31:58Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thursday 8. September 2011, Junio C Hamano wrote:\n> This stores the GPG signed push certificate in the receiving\n> repository using the notes mechanism. The certificate is appended to\n> a note in the refs/notes/signed-push tree for each object that\n> appears on the right hand side of the push certificate, i.e. the\n> object that was pushed to update the tip of a ref.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n\n(...)\n\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 307fc3b..257f2a5 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -156,6 +161,7 @@ struct command {\n> };\n> \n> static const char pre_receive_hook[] = \"hooks/pre-receive\";\n> +static const char pre_receive_signature_hook[] =\n> \"hooks/pre-receive-signature\"; static const char post_receive_hook[]\n> = \"hooks/post-receive\";\n> \n> static void rp_error(const char *err, ...) __attribute__((format\n> (printf, 1, 2))); @@ -581,6 +587,22 @@ static void\n> check_aliased_updates(struct command *commands)\n> string_list_clear(&ref_list, 0);\n> }\n> \n> +static void get_note_text(struct strbuf *buf, struct notes_tree *t,\n> +\t\t\t const unsigned char *object)\n> +{\n> +\tconst unsigned char *sha1 = get_note(t, object);\n> +\tchar *text;\n> +\tunsigned long len;\n> +\tenum object_type type;\n> +\n> +\tif (!sha1)\n> +\t\treturn;\n> +\ttext = read_sha1_file(sha1, &type, &len);\n> +\tif (text && len && type == OBJ_BLOB)\n> +\t\tstrbuf_add(buf, text, len);\n> +\tfree(text);\n> +}\n> +\n\nWhat about adding this function to notes.h as a convenience to other \nusers of the notes API?\n\nOtherwise the code looks good to me.\n\n\nHave fun! :)\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"175113","messageId":"7v39g7ypcg.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"CAJo=hJsLx1Q9ZDoxGn=dww5J-rO9GitH47rEme_1L8Lg0RmAqw@mail.gmail.com","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T16:24:15Z","receivedAt":"2011-09-08T16:24:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> This sounds too late to me. I think you just caused 2 HTTP POSTs, one\n> a partial one with the commands and no pack data, and another with the\n> push certificate and the pack. Neither is useful.\n\nWell, then I'd stop looking at this area and let others make it useful\nwhile I hack in other areas.\n"},{"id":"175116","messageId":"7vvct3x9vn.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"201109081131.58362.johan@herland.net","subject":"Re: [PATCH 5/2] push -s: receiving end","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T16:43:40Z","receivedAt":"2011-09-08T16:43:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n>> +static void get_note_text(struct strbuf *buf, struct notes_tree *t,\n>> +\t\t\t const unsigned char *object)\n>> +{\n>> +\tconst unsigned char *sha1 = get_note(t, object);\n>> +\tchar *text;\n>> +\tunsigned long len;\n>> +\tenum object_type type;\n>> +\n>> +\tif (!sha1)\n>> +\t\treturn;\n>> +\ttext = read_sha1_file(sha1, &type, &len);\n>> +\tif (text && len && type == OBJ_BLOB)\n>> +\t\tstrbuf_add(buf, text, len);\n>> +\tfree(text);\n>> +}\n>> +\n>\n> What about adding this function to notes.h as a convenience to other \n> users of the notes API?\n\nI actually was hoping to hear that I do not have to do this \"check\nexisting and concatenate\", and should let the add_note() function\nrun its default combine_notes method to do the concatenation.\n\nI found a few things I wasn't quite sure in the notes/notes-merge API, by\nthe way.\n\n - The combine_notes callback is run when a note is inserted into the\n in-core notes tree. I felt that this is way too early if you want to\n avoid racing with another process (and the patch tries to wrap\n create-notes-commit with lock-ref/write-ref-sha1 pair), but perhaps\n this is to deal with a case where the calling program calls add_notes()\n on the same object multiple times.\n\n - create_notes_commit() dies under a few conditions, but some callers\n that are recording advisory/optional notes might want to get an error\n and continue.\n\nI think ideally this patch should handle notes like the following, which\nis not quite how I coded it:\n\n - initialize in-core notes tree;\n\n - add bunch of notes, without regard to the existing ones, to in-core\n notes tree by calling add_notes();\n\n - lock the notes ref and read the \"parent\"; we may want to add \"wait and\n retry for a few times until we get the lock\" support at lockfile API\n level, but doing it at the application level would be fine.\n\n - call create-notes-commit, which in turn merges the in-core\n notes with what collides with those already in \"parent\" by\n calling the combine-notes callback, merges and re-balances\n the notes tree, and makes a notes commit object;\n\n - update the notes ref with that notes commit, releasing the lock on\n the ref.\n"},{"id":"175129","messageId":"20110908193555.GC16064@sigill.intra.peff.net","threadId":"28330","inReplyTo":"7vbouw2hqg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-08T19:35:55Z","receivedAt":"2011-09-08T19:35:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 07, 2011 at 01:57:27PM -0700, Junio C Hamano wrote:\n\n> If a tag is GPG-signed, and if you trust the cryptographic robustness of\n> the SHA-1 and GPG, you can guarantee that all the history leading to the\n> signed commit is not tampered with. However, it would be both cumbersome\n> and cluttering to sign each and every commit. Especially if you strive to\n> keep your history clean by tweaking, rewriting and polishing your commits\n> before pushing the resulting history out, many commits you will create\n> locally end up not mattering at all, and it is a waste of time to sign\n> them.\n> \n> A better alternative could be to sign a \"push certificate\" (for the lack\n> of better name) every time you push, asserting that what commits you are\n> pushing to update which refs. The basic workflow goes like this:\n\nI think this is the right direction, but I was a little turned off by\nthe idea that it needs a protocol extension. As I see it, there are two\nways to care about the contents of a push certificate:\n\n 1. The server might care, because it only wants to accept pushes that\n are accompanied by a certificate matching a certain key.\n\n 2. A client fetching from the server might care, because they want the\n integrity and authenticity of the data to be ensured by the gpg key\n of the pusher, not by trusting the server.\n\nI think (1) is actually not all that interesting. The server already has\ncredentials for each user via ssh or http. So it knows who each pusher\nis already. It can't relay that information cryptographically to a\nclient who fetches later, of course, but we are just talking about\nwhether or not to accept the push at this moment.\n\nBut if you really did want to do that, it seems like a pre-receive hook\nwould be sufficient.\n\nFor (2), you don't want to trust the server, so the user's\nauthentication to the server isn't enough. You want a cryptographic\nchain leading back to the original pusher. But the server doesn't\nactually need to see or understand that cryptographic chain for this\npurpose. If it were stored in a notes-tree or other format pointed to by\na ref, then a client could pull down those notes and do the verification\nthemselves.\n\nWhich means you can start using this immediately, without having to care\nabout whether your hosting provider supports it or not (or even whether\nyour provider supports the git protocol. Such a system would Just Work\nacross dumb http, local clones, sneakernet bundles, etc).\n\nThe only issue I foresee is one of atomicity. IIRC, we never have a\nwhole-repo lock during push, so it's possible that a client might\nsucceed in pushing the ref with the certificate, but fail at one or more\nrefs that the certificate mentions. And maybe a protocol extension is\nrequired for that. You've looked much more closely at this than I have,\nso maybe you already considered something simpler.\n\n-Peff\n"},{"id":"175132","messageId":"20110908200343.GD16064@sigill.intra.peff.net","threadId":"28330","inReplyTo":"robbat2-20110907T234637-463765607Z@orbis-terrarum.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-08T20:03:43Z","receivedAt":"2011-09-08T20:03:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 07, 2011 at 11:55:44PM +0000, Robin H. Johnson wrote:\n\n> There's a couple of related things we've been considering on the Gentoo\n> side:\n> - detached signatures of blobs (either the SHA1 of the blob or the blob\n> itself)\n\nThere's not much point in signing the blob itself and not the sha1; the\nfirst thing any signature algorithm will do is make a fixed-size digest\nof the content anyway. So it is only useful if you don't trust sha1 as\nyour digest algorithm (which maybe is a reasonable concern these\ndays...).\n\n> - The signature covering the message+blob details, but NOT the chain of\n> history: this opens up the ability to cherry-pick and rebase iff there\n> are no conflicts and the blobs are identical, all while preserving the\n> signature.\n\nThe problem is that many of the blobs won't be identical, because\nthey'll have new content from the new commits you rebased on top of. So\n_some_ blobs will be the same, but you'll end up with a half-signed\ncommit. I think you're better to just re-sign the new history.\n\nBut I'd have to see a longer description of your scheme to really\ncritique it. I'm not 100% sure what your security goals are here.\n\n> - concerns about a pre-image attack against Git. tl;dr version:\n> 1. Attacker prepares decoy file in advance, that hashes to the same as\n> the malicious file.\n> 2. Attacker sends decoy in as an innocuous real commit.\n> 3. Months later, the attacker breaks into the system and alters the\n> packfile to include the new malicious file.\n> 4. All new clones from that point forward get the malicious version.\n\nNit: I think you mean \"collision attack\" here. Pre-image attacks are\nmatching a malicious file to what is already in the tree, but are much\nharder to execute.\n\nBut yeah, it is a potential problem. I don't keep up very well with that\nsort of news anymore, but AFAIK, we still don't have any actual\ncollisions in sha1. Wikipedia seems to seem to think the best attacks\nare in the 2^50-ish range, but nobody has successfully found one. So we\nmay still be a few years away from a realistic attack. If the attacks\nare anything like the MD5 attacks, the decoy and malicious files will\nneed to have a bunch of random garbage in them. Which may be hard to\ndisguise, depending on your repo contents.\n\nI think, though, that the sane fix at that point is not to start trying\nto make per-blob signatures or anything like that, but to consider \"git\nversion 2\" with SHA-256, or whatever ends up becoming SHA-3 next year.\nIt would involve rewriting all of your history and dropping support for\nolder git clients, of course, but it may be worth it at the point that\nsha1 is completely broken.\n\n> Re your comment on always needing to resign commits above, we'd been\n> considering post-signing commits, not when they are initially made.\n> After your commit is clean and ready to ship, you can fire the commit\n> ids into the signature tool, which can generate a detached signature\n> note for each commit.\n\nAgreed. This is just an interface problem, not a cryptographic or\ntechnical one. However, I do think there's a subtle difference between\nthe two ideas. Signing each commit individually just indicates some\napproval of particular commits. But signing a push certificate is about\nassociating particular commits with particular refs (e.g., saying \"move\n'master' from commit X to commit Y). I think there may be uses for both\nforms.\n\n-Peff\n"},{"id":"175118","messageId":"7vy5xywyk8.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"20110908193555.GC16064@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T20:48:07Z","receivedAt":"2011-09-08T20:48:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think (1) is actually not all that interesting. The server already has\n> credentials for each user via ssh or http. So it knows who each pusher\n> is already. It can't relay that information cryptographically to a\n> client who fetches later, of course, but we are just talking about\n> whether or not to accept the push at this moment.\n>\n> But if you really did want to do that, it seems like a pre-receive hook\n> would be sufficient.\n\nI see two flaws in that reasoning. The server's authentication may be\nfound not trustworthy for some reason long after commits hit the tree, and\nGPG signature made by the _pusher_ would assert the integrity. Also this\nwill open the door to accept push over an unauthenticated connection and\nallowing only signed pushes.\n\n> For (2), you don't want to trust the server, so the user's\n> authentication to the server isn't enough. You want a cryptographic\n> chain leading back to the original pusher. But the server doesn't\n> actually need to see or understand that cryptographic chain for this\n> purpose.\n\nExactly. That is why the signed push certificate is stored without the\nserver doing anything funky, only to annotate the pushed commits in the\nnotes tree---the fetchers can peek the notes and verify the GPG signature.\nBut not _forcing_ that the push certificate be placed in a notes tree on\nthe client side allows different server hosting sites to additionally do\ndifferent things using that data.\n\n> The only issue I foresee is one of atomicity.\n\nThe very initial thinking was to create a notes tree commit on the client\nside and push that along with what is pushed, but that approach has an\ninherent flaw of causing unnecessary collisions between two people who are\npushing to unrelated branches, and that is why I decided to let the server\nside handle it.\n"},{"id":"175131","messageId":"20110908210217.GA32522@sigill.intra.peff.net","threadId":"28330","inReplyTo":"7vy5xywyk8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-08T21:02:17Z","receivedAt":"2011-09-08T21:02:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2011 at 01:48:07PM -0700, Junio C Hamano wrote:\n\n> > I think (1) is actually not all that interesting. The server already has\n> > credentials for each user via ssh or http. So it knows who each pusher\n> > is already. It can't relay that information cryptographically to a\n> > client who fetches later, of course, but we are just talking about\n> > whether or not to accept the push at this moment.\n> >\n> > But if you really did want to do that, it seems like a pre-receive hook\n> > would be sufficient.\n> \n> I see two flaws in that reasoning. The server's authentication may be\n> found not trustworthy for some reason long after commits hit the tree, and\n> GPG signature made by the _pusher_ would assert the integrity.\n\nRight, but that is not about (1), but rather about (2). IOW, there are\ntwo times to authenticate: at the moment you take the push, and every\nmoment thereafter, no matter who you are. But I think we are just\nsplitting hairs. We both agree that signed pushes are a good thing.\n\nWhen I said \"a pre-receive hook is sufficient\", I emphatically _didn't_\nmean that the server should sign in the hook, or create a cryptographic\ntrail starting there. I meant that git doesn't need to carry the code\ninternally, and that a pre-receive hook can check the push certificate\nitself, just as a client would.\n\nBTW, is the name \"push certificate\" right? It seems like they are not\nnecessarily about pushing, but about signing \"I would like to move ref X\nfrom Y to Z\". Is there value to making such signatures locally (i.e.,\nnot over the git protocol, but such that you could later check the\nintegrity of what's on the disk cryptographically)? Would it not be\npossible to generate this information in one step, and then have a\nremote fetch from you, with no pushes at all?\n\n> Also this will open the door to accept push over an unauthenticated\n> connection and allowing only signed pushes.\n\nYes, though a pre-receive hook could do that, too.\n\n> Exactly. That is why the signed push certificate is stored without the\n> server doing anything funky, only to annotate the pushed commits in the\n> notes tree---the fetchers can peek the notes and verify the GPG signature.\n> But not _forcing_ that the push certificate be placed in a notes tree on\n> the client side allows different server hosting sites to additionally do\n> different things using that data.\n\nHmm. So you seem to take the approach of:\n\n 1. Client needs only know \"I am pushing with a signed cert\".\n\n 2. Server can convert that signed cert into other formats as they see\n fit, including breaking the signature out into a notes ref.\n\n 3. Other clients fetch from the server, seeing the notes ref (or not).\n\nBut that seems backwards to me. In a decentralized system, the endpoints\nare what is important. So the pushing client in 1 should be the one\ndeciding what other clients see, no? Otherwise, if I care about what's\nin the notes tree, I have to care whether I am pulling from kernel.org\nor github.com, or whatever. The server stops being dumb storage.\n\n> > The only issue I foresee is one of atomicity.\n> \n> The very initial thinking was to create a notes tree commit on the client\n> side and push that along with what is pushed, but that approach has an\n> inherent flaw of causing unnecessary collisions between two people who are\n> pushing to unrelated branches, and that is why I decided to let the server\n> side handle it.\n\nYeah, it is a potential problem, but it just seems wrong to put too much\npolicy work onto the server. Perhaps it would make more sense to keep\none notes tree per ref, which would also resolve that locking issue?\n\n-Peff\n"},{"id":"175103","messageId":"7v7h5iwub9.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"20110908210217.GA32522@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-08T22:19:54Z","receivedAt":"2011-09-08T22:19:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah, it is a potential problem, but it just seems wrong to put too much\n> policy work onto the server.\n\nMy take on it is somewhat different. The only thing in the end result we\nwant to see is that the pushed commits are annotated with GPG signatures\nin the notes tree, and there is no reason for us to cast in stone that\nthere has to be any significance in the commit history of the notes tree.\n\nIn a busy hosting site that has many branches being pushed simultaneously,\nit is entirely plausible that the server side may just want to store each\nreceived push certificate in a new flat file in a filesystem, and have\nasynchronous process sweep the new certificates to update the notes tree,\npossibly creating a single notes tree commit that records updates by\nmultiple pushes, for performance purposes, in its implementation of\nrecord_signed_push() in receive-pack.\n\nIf you forced the clients to also prepare notes and push the notes tree to\nthe server, you are forcing the ordering in the history of the notes, and\nclosing the door for such a server implementation. I would consider it an\nunnecessary and/or premature policy decision.\n"},{"id":"175141","messageId":"robbat2-20110909T004300-810527870Z@orbis-terrarum.net","threadId":"28330","inReplyTo":"20110908200343.GD16064@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2011-09-09T01:30:09Z","receivedAt":"2011-09-09T01:30:09Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Thu, Sep 08, 2011 at 04:03:43PM -0400, Jeff King wrote:\n> On Wed, Sep 07, 2011 at 11:55:44PM +0000, Robin H. Johnson wrote:\n> > There's a couple of related things we've been considering on the Gentoo\n> > side:\n> > - detached signatures of blobs (either the SHA1 of the blob or the blob\n> > itself)\n> There's not much point in signing the blob itself and not the sha1; the\n> first thing any signature algorithm will do is make a fixed-size digest\n> of the content anyway. So it is only useful if you don't trust sha1 as\n> your digest algorithm (which maybe is a reasonable concern these\n> days...).\nTo clarify here, there's two things gained by this over signing the\nSHA1:\n1. Ability to use a different digest algorithm right now, without\n having to trust SHA1.\n2. Ability to use HMAC.\n\nI agree that both of these are bandaids to the security of SHA*.\n\n> > - The signature covering the message+blob details, but NOT the chain of\n> > history: this opens up the ability to cherry-pick and rebase iff there\n> > are no conflicts and the blobs are identical, all while preserving the\n> > signature.\n> The problem is that many of the blobs won't be identical, because\n> they'll have new content from the new commits you rebased on top of. So\n> _some_ blobs will be the same, but you'll end up with a half-signed\n> commit. I think you're better to just re-sign the new history.\nPossibly, the rebase/cherry-pick isn't critical.\n\n> But I'd have to see a longer description of your scheme to really\n> critique it. I'm not 100% sure what your security goals are here.\nThe primary goal is that a developer can certify their commits when they\nare ready to push them, regardless of the means of how they are going to\npush them (I've had requests for some git-bundle usage help in pushes).\n\n> > - concerns about a pre-image attack against Git. tl;dr version:\n> > 1. Attacker prepares decoy file in advance, that hashes to the same as\n> > the malicious file.\n> > 2. Attacker sends decoy in as an innocuous real commit.\n> > 3. Months later, the attacker breaks into the system and alters the\n> > packfile to include the new malicious file.\n> > 4. All new clones from that point forward get the malicious version.\n> Nit: I think you mean \"collision attack\" here. Pre-image attacks are\n> matching a malicious file to what is already in the tree, but are much\n> harder to execute.\nMy bad, it was really late when I was writing the above.\n\n> But yeah, it is a potential problem. I don't keep up very well with that\n> sort of news anymore, but AFAIK, we still don't have any actual\n> collisions in sha1. Wikipedia seems to seem to think the best attacks\n> are in the 2^50-ish range, but nobody has successfully found one. So we\n> may still be a few years away from a realistic attack. If the attacks\n> are anything like the MD5 attacks, the decoy and malicious files will\n> need to have a bunch of random garbage in them. Which may be hard to\n> disguise, depending on your repo contents.\nJoey Hess discussed this two years ago, and again last week:\nhttp://kitenet.net/~joey/blog/entry/size_of_the_git_sha1_collision_attack_surface/\nhttp://kitenet.net/~joey/blog/entry/sha-1/\n\nThis is easy in the kernel tree, it's got lots of eyeballs and only few\nbinary files. This isn't true for lots of other Git trees, a tree with a\nJPEG image or a gzip file would be a great target.\n\nIt's 2^53 last I saw, which is only ~250 days of cputime on my i7\ndesktop. Also notably that is under the complexity of the 2^56 EFF DES\ncracker was built do to (and the more recent COPACOBANA unit).\n\n> I think, though, that the sane fix at that point is not to start trying\n> to make per-blob signatures or anything like that, but to consider \"git\n> version 2\" with SHA-256, or whatever ends up becoming SHA-3 next year.\n> It would involve rewriting all of your history and dropping support for\n> older git clients, of course, but it may be worth it at the point that\n> sha1 is completely broken.\nI think this is needed sooner rather than later.\n\n> > Re your comment on always needing to resign commits above, we'd been\n> > considering post-signing commits, not when they are initially made.\n> > After your commit is clean and ready to ship, you can fire the commit\n> > ids into the signature tool, which can generate a detached signature\n> > note for each commit.\n> Agreed. This is just an interface problem, not a cryptographic or\n> technical one. However, I do think there's a subtle difference between\n> the two ideas. Signing each commit individually just indicates some\n> approval of particular commits. But signing a push certificate is about\n> associating particular commits with particular refs (e.g., saying \"move\n> 'master' from commit X to commit Y). I think there may be uses for both\n> forms.\nYes, there is use for both forms. The additional one is that you can\nseparate who pushes from who commits.\n\n-- \nRobin Hugh Johnson\nGentoo Linux: Developer, Trustee & Infrastructure Lead\nE-Mail : robbat2@gentoo.org\nGnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85\n"},{"id":"175198","messageId":"20110909152248.GA28480@sigill.intra.peff.net","threadId":"28330","inReplyTo":"CAJo=hJsQvRN3Z0xJg9q37Km1g_1qUdJKNQ6n8=a9mv3YjugyVw@mail.gmail.com","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-09T15:22:48Z","receivedAt":"2011-09-09T15:22:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2011 at 03:07:25PM -0700, Shawn O. Pearce wrote:\n\n> A notes tree per ref is ugly when you have a lot of branches. Its a\n> problem when you merge commits from say \"maint\" over to \"master\" 2\n> days after they were initially pushed. Ideally the notes tree for\n> master also includes what you brought over.\n\nI agree you can end up with a lot of refs if you have a lot of branches,\nand that may get a bit unwieldy. I don't see how the merge thing is a\nproblem, though. If you do the merge at a client, then those commits\nwill hit master by push, and you'll get entries in the cert tree for\nmaster. If you do the merge locally, then there's not going to be a\nsignature on those commits hitting the master ref, no matter what the\nstorage scheme.\n\n> However, maybe it is reasonable for a protocol extension to support\n> \"auto-notes-merge\" on a refs/notes/ ref if the client asks for it in\n> the send-pack/receive-pack command stream? Clients could push notes\n> and have the server automatically merge the client's pushed commit\n> into the notes tree, either by fast-forward or by performing an\n> automatic notes merge, with concat being applied to non-identical\n> notes on the same SHA-1. This would allow the client to prepare his\n> local certificate, and push that notes tree, while still working\n> around the race on the server side.\n> \n> If the server doesn't support the protocol extension, the client can\n> still push his signed notes, he just may run into a race with another\n> concurrent user pushing into the same repository. Which then means\n> that an upgrade of the server is really only important/necessary if\n> you have \"central repository\" model that a lot of users push into. If\n> you use the traditional workflow that GitHub encourages of per-user\n> repositories, you would never have a race on the server, and wouldn't\n> need the upgraded server binary with \"auto-notes-merge\".\n\nI like this approach, as the protocol extension provides the minimal\nbuilding block that can be used to implement this, or any other\nnotes-related scheme.\n\n-Peff\n"},{"id":"175200","messageId":"20110909153441.GB28480@sigill.intra.peff.net","threadId":"28330","inReplyTo":"7v7h5iwub9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-09T15:34:41Z","receivedAt":"2011-09-09T15:34:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2011 at 03:19:54PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Yeah, it is a potential problem, but it just seems wrong to put too much\n> > policy work onto the server.\n> \n> My take on it is somewhat different. The only thing in the end result we\n> want to see is that the pushed commits are annotated with GPG signatures\n> in the notes tree, and there is no reason for us to cast in stone that\n> there has to be any significance in the commit history of the notes tree.\n\nHmm. Is order really irrelevant? If you push a commit to master, moving\nit from X to Y, then push-rewind it back to X, then push a new commit Z,\nhow do I cryptographically determine the correct final state of master?\n\nI'll see two push-certs, one going X..Y and one X..Z. They'll have\ntimestamps, which can be used for ordering. But what if the first and\nthird actions above are done by different people. Now you're trusting\nthat their clocks are synced to order them properly.\n\nNote that simply keeping an unsigned but ordered notes history doesn't\nsolve this problem, either; you'd probably want a parent pointer in your\npush cert saying \"and this is the previous push cert I am based on\".\n\nAnd maybe this is a use case we don't care about. Maybe it's enough for\nthe push-cert to say \"at some point in time, I thought it was a good\nidea to push these commits into master; signed, me\".\n\nBut I'm not really clear on exactly what the security goals are. The\nseries you sent looks interesting, but I haven't seen the verification\nside of these signatures. What are they going to be used for? What\nguarantees are we attempting to provide? For that matter, what is our\nthreat model? What are attackers capable of?\n\n> In a busy hosting site that has many branches being pushed simultaneously,\n> it is entirely plausible that the server side may just want to store each\n> received push certificate in a new flat file in a filesystem, and have\n> asynchronous process sweep the new certificates to update the notes tree,\n> possibly creating a single notes tree commit that records updates by\n> multiple pushes, for performance purposes, in its implementation of\n> record_signed_push() in receive-pack.\n\nOK, I see. It is not \"the server can do whatever it likes with the\ninformation\" as much as \"the server can do whatever it likes, but at the\nvery least should eventually create a notes tree of a given form\".\n\n-Peff\n"},{"id":"175208","messageId":"20110909160301.GA9707@gnu.kitenet.net","threadId":"28330","inReplyTo":"robbat2-20110909T004300-810527870Z@orbis-terrarum.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-09-09T16:03:01Z","receivedAt":"2011-09-09T16:03:01Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Robin H. Johnson wrote:\n> Joey Hess discussed this two years ago, and again last week:\n> http://kitenet.net/~joey/blog/entry/size_of_the_git_sha1_collision_attack_surface/\n> \n> This is easy in the kernel tree, it's got lots of eyeballs and only few\n> binary files. This isn't true for lots of other Git trees, a tree with a\n> JPEG image or a gzip file would be a great target.\n\nThe most credible attack I have so far does not involve binary files in\ntree. Someone pointed out that git log, git show, etc stop printing\ncommit messages at NULL. So colliding binary garbage can be put in a\ncommit message and be unlikely to be noticed, and the commit can\nlater be altered to point to a different tree.\n\nhttps://github.com/joeyh/supercollider\n\njoey@gnu:~/tmp/supercollider>git log\ncommit 24f30db5790b209fa412ce81c5ef2bf8af5fd4d7\nAuthor: Joey Hess \nDate: Fri Sep 9 11:49:21 2011 -0400\n\n an innocent commit\n \n If this were a sha1 colliding attack, there would be some sort of binary\n garbage below. Which there isn't. So this can be safely merged.\njoey@gnu:~/tmp/supercollider>git cat-file commit 24f30db5790b209fa412ce81c5ef2bf8af5fd4d7\ntree 735a7633237c07b398856005de3bc9ea00446747\nauthor Joey Hess 1315583361 -0400\ncommitter Joey Hess 1315583361 -0400\n\nan innocent commit\n\nIf this were a sha1 colliding attack, there would be some sort of binary\ngarbage below. Which there isn't. So this can be safely merged.\n\n\n\n??b???\u001f[?i??ͯ?t?\f2??\u0002????os?\u0014?Q??H?޸\u0016*zl\u001a?RA˂q?E\f?\u0006\u0016E7??\u001b?\u0003\\?m???U?\u001e>MU\u000b\tGY?d)?ȼ??'g?~D??ɯhQ?\u0013???/\"E\u0004??X?m???^͸??S?D\u0013??;w6(?`??>?\u0010縘?\u0007AѲ?*!??@v????>?8??2\b?\u0014!??=*?J\t\u001b\r\r???\u0001ynH\u0010???c?w?\\??K7??\u001c?N?6??\u001c???A5?FM?wZ?~?pK\u0002Y?R???s7??(?\u0007ƶ?_\"??m\u0011%????1a??ʀ??K[\rt??\u0011??\u000e!A0?ΈfT.?T?w\u0007?򁛵ƌ\u000b?р???aco?V/2\u0014??nَ?\n?}?6?\u0019_?z?{\n\nIt might be worth ameloriating that attack by making git log always\nshow the full buffer. Or it would be easy to write a tool that finds\nany commits that have a NULL in their message.\n\n-- \nsee shy jo\n"},{"id":"175212","messageId":"1315584873.18331.103.camel@ddn-tmpdesk.its.maine.edu","threadId":"28330","inReplyTo":"20110909160301.GA9707@gnu.kitenet.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Drew Northup","fromEmail":"drew.northup@maine.edu","sentAt":"2011-09-09T16:14:33Z","receivedAt":"2011-09-09T16:14:33Z","isPatch":true,"sender":{"key":"drew.northup@maine.edu","avatar":"https://avatars.githubusercontent.com/u/18331571?v=4"},"body":"\nOn Fri, 2011-09-09 at 12:03 -0400, Joey Hess wrote:\n\n> It might be worth ameloriating that attack by making git log always\n> show the full buffer. Or it would be easy to write a tool that finds\n> any commits that have a NULL in their message.\n\nJust be aware that fixing the problem by diallowing NULLs in the commit\nmessage makes it impossible to include UTF-16 text in the commit message\n(which isn't currently handled very well anyway). Granted, I'm not sure\nwhat the most decent way of dealing with that otherwise might be as I've\nnot taken the time to think about it...\n\n-- \n-Drew Northup\n________________________________________________\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"175216","messageId":"7v8vpxvcyi.fsf@alter.siamese.dyndns.org","threadId":"28330","inReplyTo":"20110909153441.GB28480@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-09T17:32:21Z","receivedAt":"2011-09-09T17:32:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Sep 08, 2011 at 03:19:54PM -0700, Junio C Hamano wrote:\n>\n>> My take on it is somewhat different. The only thing in the end result we\n>> want to see is that the pushed commits are annotated with GPG signatures\n>> in the notes tree, and there is no reason for us to cast in stone that\n>> there has to be any significance in the commit history of the notes tree.\n>\n> Hmm. Is order really irrelevant? If you push a commit to master, moving\n> it from X to Y, then push-rewind it back to X, then push a new commit Z,\n> how do I cryptographically determine the correct final state of master?\n\nYou don't, as the certs are more about \"up to this point, the pusher\ntrusts the history\". I should have made it clearer in the cover letter to\nthe rerolled series, but the push certificate does not record the old\nvalue of the ref in the reroll, because the point of signed-push is not\nabout signing the information that is equivalent to the server side\nreflog.\n\nYou would have a signed push record that pushed Y, X and Z, and commit Z\nsitting at the tip of 'master'. A few days may pass and then you run\n\n $ git log --show-notes=refs/notes/push-signature master\n\nto find that the first commit with a push signature by somebody whose\njudgement you trust is Z. Then you would need to inspect only commits that\nare not ancestors of Z even if you suspect that some commits near the tip\nof 'master' at the server side were tampered with.\n\nYou may at the same time find commits signed by the trusted people that\nare meant for the same branch but are not contained in the history of\n'master' (e.g. Y), which might indicate that the branch was rewound,\npossibly by an intruder.\n\nAnother possible scenario. Later you and the pusher of Z may find that\nwhen the pusher created Z, he merged something questionable and Z may now\nhave to be in \"untrustable\" set. You can dig further to find X at that\npoint.\n\n> OK, I see. It is not \"the server can do whatever it likes with the\n> information\" as much as \"the server can do whatever it likes, but at the\n> very least should eventually create a notes tree of a given form\".\n\nYes, examples of things the server side might want to additionally do in\npre-receive-signature hook are to read the push certificate to implement\nauthorization (and it can be per-branch if you wanted to) and to forward\nit immediately to offsite storage for safekeeping (the storage does not\nhave to use git notes to implement it).\n"},{"id":"175229","messageId":"20110909191240.GA30019@sigill.intra.peff.net","threadId":"28330","inReplyTo":"20110909160301.GA9707@gnu.kitenet.net","subject":"Re: [PATCH 2/2] push -s: skeleton","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-09T19:12:40Z","receivedAt":"2011-09-09T19:12:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 09, 2011 at 12:03:01PM -0400, Joey Hess wrote:\n\n> The most credible attack I have so far does not involve binary files in\n> tree. Someone pointed out that git log, git show, etc stop printing\n> commit messages at NULL.\n\nIt was me.\n\n> It might be worth ameloriating that attack by making git log always\n> show the full buffer. Or it would be easy to write a tool that finds\n> any commits that have a NULL in their message.\n\nUnfortunately, that is going to involve a pretty huge code audit of git,\nas the \"tack a \\0 to the end of an object just in case\" code dates back\nquite a while (e871b64, unpack_sha1_file: zero-pad the unpacked\nobject, 2005-05-25). So I suspect there is a lot of code built on\ntop of the assumption that commit messages are NUL-terminated strings.\n\nBesides which, that is only one form of hiding. If collision attacks\nagainst sha1 become a possibility, I think we are better to talk about\nmoving to a new hash. Even sha-256 truncated to 160 bits would be better\nthan sha-1 (AFAIK, that family of SHA does not suffer from the same\nattacks, so we would still be in the 2^80 range for collision attacks).\n\n-Peff\n"}]}