threads / patch / 28338

v2, 7 partssend-pack: typofix error message

Subject: [PATCH v2 1/7] send-pack: typofix error message

## tl;dr

22 messages between Sep 8, 2011 and Sep 11, 2011. Diffs are folded; open one to read it.

replies: 21people: 4as markdown or json

Junio C Hamano· Sep 8, 2011, 20:01 UTC · lore

[PATCH v2 0/7] "push -s"

Slightly re-rolled from last night.
 - Marked push-certificate format as version 0 while we are
   still experimenting;
 - The push certificate records new object name and the refname;
 - Add support for an external hook "pre-receive-signature".

One issue internally debated was if we want to list the refs that matched the pushing criteria but were found to be already up to date, and this can be argued both ways.

 - You can say that you are making assertion that you want to have a
   certain object at that ref, in which case you would want to include
   them.
 - On the other hand, for the purpose of leaving audit-trail, if the ref
   you tried to push already had the object you wanted to see at the tip
   of a ref, you weren't the person who made the ref point at the object,
   and it would be sensible not to include them.

Taking the latter stance is slightly easier on the end users, because "Everything up-to-date" case becomes a no-op as the natural consequence, and we do not have to ask them to unlock their GPG key in such a case. It however makes it impossible to say "Earlier I pushed that object to the tip of my branch but forgot to sign the push, and I want to make a signed push, even though I didn't add anything to my history."

People who configured to push out more than one branches with "git push" often work on one branch, run "git push" which ends up pushing that branch but not other branches, then work on another branch and run "git push" to push out that other branch, while the branch he earlier pushed out stays the same since his last push. For such people, the first "push" is not necessarily even an assertion that he wants to have both branches pointing at certain commits, and from that point of view, not including the latter branch he hasn't worked on (and stayed up-to-date) in the push certifiate is a sensible thing to do.

As there is no single right answer, this round of re-roll keeps the latter semantics to record only what you pushed out as the original series.

Junio C Hamano (7):
  send-pack: typofix error message
  Split GPG interface into its own helper library
  push -s: skeleton
  push -s: send signed push certificate
  push -s: receiving end
  refactor run_receive_hook()
  push -s: support pre-receive-signature hook
 Makefile               |    2 +
 builtin/push.c         |    1 +
 builtin/receive-pack.c |  206 +++++++++++++++++++++++++++++++++++++++++++-----
 builtin/send-pack.c    |   61 +++++++++++++-
 builtin/tag.c          |   60 ++------------
 builtin/verify-tag.c   |   35 +--------
 gpg-interface.c        |   94 ++++++++++++++++++++++
 gpg-interface.h        |   11 +++
 send-pack.h            |    1 +
 transport.c            |    4 +
 transport.h            |    4 +
 11 files changed, 369 insertions(+), 110 deletions(-)
 create mode 100644 gpg-interface.c
 create mode 100644 gpg-interface.h
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 8, 2011, 20:01 UTC · re: Junio C Hamano · lore

The message identifies the process as receive-pack when it cannot fork the sideband demultiplexer. We are actually a send-pack.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/send-pack.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin/send-pack.c +1 −1
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index c1f6ddd..87833f4 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -334,7 +334,7 @@ int send_pack(struct send_pack_args *args,
 		demux.data = fd;
 		demux.out = -1;
 		if (start_async(&demux))
-			die("receive-pack: unable to fork off sideband demultiplexer");
+			die("send-pack: unable to fork off sideband demultiplexer");
 		in = demux.out;
 	}
 
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 8, 2011, 20:01 UTC · re: Junio C Hamano · lore

[PATCH v2 2/7] Split GPG interface into its own helper library

This mostly moves existing code from builtin/tag.c (for signing) and builtin/verify-tag.c (for verifying) to a new gpg-interface.c file to provide a more generic library interface.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile             |    2 +
 builtin/tag.c        |   60 ++++----------------------------
 builtin/verify-tag.c |   35 ++-----------------
 gpg-interface.c      |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++
 gpg-interface.h      |   11 ++++++
 5 files changed, 117 insertions(+), 85 deletions(-)
 create mode 100644 gpg-interface.c
 create mode 100644 gpg-interface.h
Show changes to 5 files +117 −85

Makefile, builtin/tag.c, builtin/verify-tag.c, gpg-interface.c, gpg-interface.h

diff --git a/Makefile b/Makefile
index 8d6d451..2183223 100644
--- a/Makefile
+++ b/Makefile
@@ -530,6 +530,7 @@ LIB_H += exec_cmd.h
 LIB_H += fsck.h
 LIB_H += gettext.h
 LIB_H += git-compat-util.h
+LIB_H += gpg-interface.h
 LIB_H += graph.h
 LIB_H += grep.h
 LIB_H += hash.h
@@ -620,6 +621,7 @@ LIB_OBJS += entry.o
 LIB_OBJS += environment.o
 LIB_OBJS += exec_cmd.o
 LIB_OBJS += fsck.o
+LIB_OBJS += gpg-interface.o
 LIB_OBJS += graph.o
 LIB_OBJS += grep.o
 LIB_OBJS += hash.o
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..e9d36fa 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -14,6 +14,7 @@
 #include "parse-options.h"
 #include "diff.h"
 #include "revision.h"
+#include "gpg-interface.h"
 
 static const char * const git_tag_usage[] = {
 	"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
@@ -208,60 +209,13 @@ static int verify_tag(const char *name, const char *ref,
 
 static int do_sign(struct strbuf *buffer)
 {
-	struct child_process gpg;
-	const char *args[4];
-	char *bracket;
-	int len;
-	int i, j;
+	const char *key;
 
-	if (!*signingkey) {
-		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
-				sizeof(signingkey)) > sizeof(signingkey) - 1)
-			return error(_("committer info too long."));
-		bracket = strchr(signingkey, '>');
-		if (bracket)
-			bracket[1] = '\0';
-	}
-
-	/* When the username signingkey is bad, program could be terminated
-	 * because gpg exits without reading and then write gets SIGPIPE. */
-	signal(SIGPIPE, SIG_IGN);
-
-	memset(&gpg, 0, sizeof(gpg));
-	gpg.argv = args;
-	gpg.in = -1;
-	gpg.out = -1;
-	args[0] = "gpg";
-	args[1] = "-bsau";
-	args[2] = signingkey;
-	args[3] = NULL;
-
-	if (start_command(&gpg))
-		return error(_("could not run gpg."));
-
-	if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
-		close(gpg.in);
-		close(gpg.out);
-		finish_command(&gpg);
-		return error(_("gpg did not accept the tag data"));
-	}
-	close(gpg.in);
-	len = strbuf_read(buffer, gpg.out, 1024);
-	close(gpg.out);
-
-	if (finish_command(&gpg) || !len || len < 0)
-		return error(_("gpg failed to sign the tag"));
-
-	/* Strip CR from the line endings, in case we are on Windows. */
-	for (i = j = 0; i < buffer->len; i++)
-		if (buffer->buf[i] != '\r') {
-			if (i != j)
-				buffer->buf[j] = buffer->buf[i];
-			j++;
-		}
-	strbuf_setlen(buffer, j);
-
-	return 0;
+	if (*signingkey)
+		key = signingkey;
+	else
+		key = git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);
+	return sign_buffer(buffer, key);
 }
 
 static const char tag_template[] =
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 3134766..8b4f742 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -11,6 +11,7 @@
 #include "run-command.h"
 #include <signal.h>
 #include "parse-options.h"
+#include "gpg-interface.h"
 
 static const char * const verify_tag_usage[] = {
 		"git verify-tag [-v|--verbose] <tag>...",
@@ -19,42 +20,12 @@ static const char * const verify_tag_usage[] = {
 
 static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 {
-	struct child_process gpg;
-	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
-	char path[PATH_MAX];
-	size_t len;
-	int fd, ret;
+	int len;
 
-	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
-	if (fd < 0)
-		return error("could not create temporary file '%s': %s",
-						path, strerror(errno));
-	if (write_in_full(fd, buf, size) < 0)
-		return error("failed writing temporary file '%s': %s",
-						path, strerror(errno));
-	close(fd);
-
-	/* find the length without signature */
 	len = parse_signature(buf, size);
 	if (verbose)
 		write_in_full(1, buf, len);
-
-	memset(&gpg, 0, sizeof(gpg));
-	gpg.argv = args_gpg;
-	gpg.in = -1;
-	args_gpg[2] = path;
-	if (start_command(&gpg)) {
-		unlink(path);
-		return error("could not run gpg.");
-	}
-
-	write_in_full(gpg.in, buf, len);
-	close(gpg.in);
-	ret = finish_command(&gpg);
-
-	unlink_or_warn(path);
-
-	return ret;
+	return verify_signed_buffer(buf, size, len);
 }
 
 static int verify_tag(const char *name, int verbose)
diff --git a/gpg-interface.c b/gpg-interface.c
new file mode 100644
index 0000000..b83cca1
--- /dev/null
+++ b/gpg-interface.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "gpg-interface.h"
+#include "sigchain.h"
+
+int sign_buffer(struct strbuf *buffer, const char *signing_key)
+{
+	struct child_process gpg;
+	const char *args[4];
+	ssize_t len;
+	int i, j;
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args;
+	gpg.in = -1;
+	gpg.out = -1;
+	args[0] = "gpg";
+	args[1] = "-bsau";
+	args[2] = signing_key;
+	args[3] = NULL;
+
+	if (start_command(&gpg))
+		return error(_("could not run gpg."));
+
+	/*
+	 * When the username signingkey is bad, program could be terminated
+	 * because gpg exits without reading and then write gets SIGPIPE.
+	 */
+	sigchain_push(SIGPIPE, SIG_IGN);
+
+	if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
+		close(gpg.in);
+		close(gpg.out);
+		finish_command(&gpg);
+		return error(_("gpg did not accept the data"));
+	}
+	close(gpg.in);
+	len = strbuf_read(buffer, gpg.out, 1024);
+	close(gpg.out);
+
+	sigchain_pop(SIGPIPE);
+
+	if (finish_command(&gpg) || !len || len < 0)
+		return error(_("gpg failed to sign the data"));
+
+	/* Strip CR from the line endings, in case we are on Windows. */
+	for (i = j = 0; i < buffer->len; i++)
+		if (buffer->buf[i] != '\r') {
+			if (i != j)
+				buffer->buf[j] = buffer->buf[i];
+			j++;
+		}
+	strbuf_setlen(buffer, j);
+
+	return 0;
+}
+
+int verify_signed_buffer(const char *buf, size_t total, size_t payload)
+{
+	struct child_process gpg;
+	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
+	char path[PATH_MAX];
+	int fd, ret;
+
+	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
+	if (fd < 0)
+		return error("could not create temporary file '%s': %s",
+			     path, strerror(errno));
+	if (write_in_full(fd, buf, total) < 0)
+		return error("failed writing temporary file '%s': %s",
+			     path, strerror(errno));
+	close(fd);
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args_gpg;
+	gpg.in = -1;
+	args_gpg[2] = path;
+	if (start_command(&gpg)) {
+		unlink(path);
+		return error("could not run gpg.");
+	}
+
+	write_in_full(gpg.in, buf, payload);
+	close(gpg.in);
+	ret = finish_command(&gpg);
+
+	unlink_or_warn(path);
+
+	return ret;
+}
diff --git a/gpg-interface.h b/gpg-interface.h
new file mode 100644
index 0000000..7689357
--- /dev/null
+++ b/gpg-interface.h
@@ -0,0 +1,11 @@
+#ifndef GPG_INTERFACE_H
+#define GPG_INTERFACE_H
+
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+
+extern int sign_buffer(struct strbuf *buffer, const char *signing_key);
+extern int verify_signed_buffer(const char *buffer, size_t total, size_t payload);
+
+#endif
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 8, 2011, 20:01 UTC · re: Junio C Hamano · lore

[PATCH v2 3/7] push -s: skeleton

If a tag is GPG-signed, and if you trust the cryptographic robustness of the SHA-1 hash and GPG, you can sleep well knowing that all the history leading to the signed commit cannot be tampered with. However, it would be both cumbersome and cluttering to sign each and every commit. Especially if you strive to keep your history clean by tweaking, rewriting and polishing your commits before pushing the resulting history out, many commits you will create locally end up not mattering at all, and it is a waste of time to sign them all.

A better alternative could be to sign a "push certificate" (for the lack of better name) every time you push, asserting that what commits you are pushing to update which refs.

The basic workflow based on this idea would go like this:
 1. You push out your work with "git push -s";
 2. "git push", as usual, learns where the remote refs are and which refs
    are to be updated with this push. It prepares a text file in core,
    that looks like the following:
	Push-Certificate-Version: 0
	Pusher: Junio C Hamano <gitster@pobox.com> 1315427886 -0700
	Update: 3793ac56b4c4f9bf0bddc306a0cec21118683728 refs/heads/master
	Update: 12850bec0c24b529c9a9df6a95ad4bdeea39373e refs/heads/next
    Each "Update" line shows the new object name at the tip of the ref
    this push tries to update.
    The user then is asked to sign this push certificate using GPG. The
    result is carried to the other side (i.e. receive-pack). In the
    protocol exchange, this step comes immediately after the sender tells
    what the result of the push should be, before it sends the pack data.
 3. The receiving end keeps the signed push certificate in core, receives
    the pack data and unpacks (or stores and indexes) it as usual.
 4. A new phase to record the push certificate is introduced in the
    codepath after the receiving end runs receive_hook(). It is envisioned
    that this phase:
    a. parses the updated-to object names, and appends the push
       certificate (still GPG signed) to a note attached to each of the
       objects that will sit at the tip of the refs;
    b. verifies that the push certificate is signed with a GPG key that is
       authorized to push into this repository; and/or
    c. invokes pre-receive-signature hook, feeds the push certificate to it
       and asks it to veto the ref updates.

And here is a skeleton to implement this. The patch has the necessary protocol extensions implemented (although I do not know if we need separate codepath for stateless RPC mode), but does not have subroutines to:

 - Sign the certificate with GPG key;
 - Parse the signed certificate to identify the updated-to objects, and
   add the certificate as notes to them;
 - Verify the certificate and find out what GPG key was used to sign it;
   or
 - Invoke and feed the certificate to pre-receive-signature hook.

all of which should be fairly trivial. The places that needs to implement these are clearly marked with large comments, so I'll leave it up to other people who are interested in the topic to fill in the blanks ;-)

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c         |    1 +
 builtin/receive-pack.c |   54 +++++++++++++++++++++++++++++++++++++++-
 builtin/send-pack.c    |   64 +++++++++++++++++++++++++++++++++++++++++++++---
 send-pack.h            |    1 +
 transport.c            |    4 +++
 transport.h            |    4 +++
 6 files changed, 123 insertions(+), 5 deletions(-)
Show changes to 6 files +123 −5

builtin/push.c, builtin/receive-pack.c, builtin/send-pack.c, send-pack.h, transport.c, transport.h

diff --git a/builtin/push.c b/builtin/push.c
index 35cce53..2238f4e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -261,6 +261,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
 			TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
+		OPT_BIT('s', "signed", &flags, "GPG sign the push", TRANSPORT_PUSH_SIGNED),
 		OPT_END()
 	};
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ae164da..20b6799 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -30,12 +30,14 @@ static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
 static int unpack_limit = 100;
 static int report_status;
+static int signed_push;
 static int use_sideband;
 static int prefer_ofs_delta = 1;
 static int auto_update_server_info;
 static int auto_gc = 1;
 static const char *head_name;
 static int sent_capabilities;
+static char *push_certificate;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -114,7 +116,7 @@ static int show_ref(const char *path, const unsigned char *sha1, int flag, void
 	else
 		packet_write(1, "%s %s%c%s%s\n",
 			     sha1_to_hex(sha1), path, 0,
-			     " report-status delete-refs side-band-64k",
+			     " report-status delete-refs side-band-64k signed-push",
 			     prefer_ofs_delta ? " ofs-delta" : "");
 	sent_capabilities = 1;
 	return 0;
@@ -579,6 +581,31 @@ static void check_aliased_updates(struct command *commands)
 	string_list_clear(&ref_list, 0);
 }
 
+static int record_signed_push(char *cert)
+{
+	/*
+	 * This is the place for you to parse the signed push
+	 * certificate, grab the commit object names the push updates
+	 * refs to, and append the certificate to the notes to these
+	 * commits.
+	 *
+	 * You could also feed the signed push certificate to GPG,
+	 * verify the signer identity, and all the other fun stuff,
+	 * including feeding it to "pre-receive-signature" hook.
+	 *
+	 * Here we just throw it to stderr to demonstrate that the
+	 * codepath is being exercised.
+	 */
+	char *cp, *ep;
+	for (cp = cert; *cp; cp = ep) {
+		ep = strchrnul(cp, '\n');
+		if (*ep == '\n')
+			ep++;
+		fprintf(stderr, "RSP: %.*s", (int)(ep - cp), cp);
+	}
+	return 0;
+}
+
 static void execute_commands(struct command *commands, const char *unpacker_error)
 {
 	struct command *cmd;
@@ -596,6 +623,12 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
+	if (push_certificate && record_signed_push(push_certificate)) {
+		for (cmd = commands; cmd; cmd = cmd->next)
+			cmd->error_string = "n/a (push signature error)";
+		return;
+	}
+
 	check_aliased_updates(commands);
 
 	head_name = resolve_ref("HEAD", sha1, 0, NULL);
@@ -636,6 +669,8 @@ static struct command *read_head_info(void)
 				report_status = 1;
 			if (strstr(refname + reflen + 1, "side-band-64k"))
 				use_sideband = LARGE_PACKET_MAX;
+			if (strstr(refname + reflen + 1, "signed-push"))
+				signed_push = 1;
 		}
 		cmd = xcalloc(1, sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
@@ -731,6 +766,21 @@ static const char *unpack(void)
 	}
 }
 
+static char *receive_push_certificate(void)
+{
+	struct strbuf cert = STRBUF_INIT;
+	for (;;) {
+		char line[1000];
+		int len;
+
+		len = packet_read_line(0, line, sizeof(line));
+		if (!len)
+			break;
+		strbuf_add(&cert, line, len);
+	}
+	return strbuf_detach(&cert, NULL);
+}
+
 static void report(struct command *commands, const char *unpack_status)
 {
 	struct command *cmd;
@@ -846,6 +896,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	if ((commands = read_head_info()) != NULL) {
 		const char *unpack_status = NULL;
 
+		if (signed_push)
+			push_certificate = receive_push_certificate();
 		if (!delete_only(commands))
 			unpack_status = unpack();
 		execute_commands(commands, unpack_status);
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 87833f4..7f4778c 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -237,6 +237,27 @@ static int sideband_demux(int in, int out, void *data)
 	return ret;
 }
 
+static void sign_push_certificate(struct strbuf *cert)
+{
+	/*
+	 * Here, take the contents of cert->buf, and have the user GPG
+	 * sign it, and read it back in the strbuf.
+	 *
+	 * You may want to append some extra info to cert before giving
+	 * it to GPG, possibly via a hook.
+	 *
+	 * Here we upcase them just to demonstrate that the codepath
+	 * is being exercised.
+	 */
+	char *cp;
+	for (cp = cert->buf; *cp; cp++) {
+		int ch = *cp;
+		if ('a' <= ch && ch <= 'z')
+			*cp = toupper(ch);
+	}
+	return;
+}
+
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
 	      struct ref *remote_refs,
@@ -250,9 +271,11 @@ int send_pack(struct send_pack_args *args,
 	int allow_deleting_refs = 0;
 	int status_report = 0;
 	int use_sideband = 0;
+	int signed_push = 0;
 	unsigned cmds_sent = 0;
 	int ret;
 	struct async demux;
+	struct strbuf push_cert = STRBUF_INIT;
 
 	/* Does the other end support the reporting? */
 	if (server_supports("report-status"))
@@ -270,6 +293,18 @@ int send_pack(struct send_pack_args *args,
 		return 0;
 	}
 
+	if (args->signed_push) {
+		if (server_supports("signed-push"))
+			signed_push = !args->dry_run;
+		else
+			warning("The receiving side does not support signed-push");
+	}
+
+	if (signed_push) {
+		strbuf_addstr(&push_cert, "Push-Certificate-Version: 0\n");
+		strbuf_addf(&push_cert, "Pusher: %s\n", git_committer_info(0));
+	}
+
 	/*
 	 * Finally, tell the other end!
 	 */
@@ -301,15 +336,19 @@ int send_pack(struct send_pack_args *args,
 			char *old_hex = sha1_to_hex(ref->old_sha1);
 			char *new_hex = sha1_to_hex(ref->new_sha1);
 
-			if (!cmds_sent && (status_report || use_sideband)) {
-				packet_buf_write(&req_buf, "%s %s %s%c%s%s",
+			if (!cmds_sent &&
+			    (status_report || use_sideband || signed_push))
+				packet_buf_write(&req_buf, "%s %s %s%c%s%s%s",
 					old_hex, new_hex, ref->name, 0,
 					status_report ? " report-status" : "",
-					use_sideband ? " side-band-64k" : "");
-			}
+					use_sideband ? " side-band-64k" : "",
+					signed_push ? " signed-push" : "");
 			else
 				packet_buf_write(&req_buf, "%s %s %s",
 					old_hex, new_hex, ref->name);
+			if (signed_push)
+				strbuf_addf(&push_cert, "Update: %s %s\n",
+					    new_hex, ref->name);
 			ref->status = status_report ?
 				REF_STATUS_EXPECTING_REPORT :
 				REF_STATUS_OK;
@@ -326,6 +365,23 @@ int send_pack(struct send_pack_args *args,
 		safe_write(out, req_buf.buf, req_buf.len);
 		packet_flush(out);
 	}
+
+	if (signed_push && cmds_sent) {
+		char *cp, *ep;
+
+		sign_push_certificate(&push_cert);
+		strbuf_reset(&req_buf);
+		for (cp = push_cert.buf; *cp; cp = ep) {
+			ep = strchrnul(cp, '\n');
+			if (*ep == '\n')
+				ep++;
+			packet_buf_write(&req_buf, "%.*s",
+					 (int)(ep - cp), cp);
+		}
+		/* Do we need anything funky for stateless rpc? */
+		safe_write(out, req_buf.buf, req_buf.len);
+		packet_flush(out);
+	}
 	strbuf_release(&req_buf);
 
 	if (use_sideband && cmds_sent) {
diff --git a/send-pack.h b/send-pack.h
index 05d7ab1..754943e 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -11,6 +11,7 @@ struct send_pack_args {
 		use_thin_pack:1,
 		use_ofs_delta:1,
 		dry_run:1,
+		signed_push:1,
 		stateless_rpc:1;
 };
 
diff --git a/transport.c b/transport.c
index fa279d5..7a7ffe4 100644
--- a/transport.c
+++ b/transport.c
@@ -476,6 +476,9 @@ static int set_git_option(struct git_transport_options *opts,
 		else
 			opts->depth = atoi(value);
 		return 0;
+	} else if (!strcmp(name, TRANS_OPT_SIGNED_PUSH)) {
+		opts->signed_push = !!value;
+		return 0;
 	}
 	return 1;
 }
@@ -793,6 +796,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	args.progress = transport->progress;
 	args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
 	args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
+	args.signed_push = !!(flags & TRANSPORT_PUSH_SIGNED);
 
 	ret = send_pack(&args, data->fd, data->conn, remote_refs,
 			&data->extra_have);
diff --git a/transport.h b/transport.h
index 059b330..d2fa478 100644
--- a/transport.h
+++ b/transport.h
@@ -8,6 +8,7 @@ struct git_transport_options {
 	unsigned thin : 1;
 	unsigned keep : 1;
 	unsigned followtags : 1;
+	unsigned signed_push : 1;
 	int depth;
 	const char *uploadpack;
 	const char *receivepack;
@@ -102,6 +103,7 @@ struct transport {
 #define TRANSPORT_PUSH_PORCELAIN 16
 #define TRANSPORT_PUSH_SET_UPSTREAM 32
 #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
+#define TRANSPORT_PUSH_SIGNED 128
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 
@@ -128,6 +130,8 @@ struct transport *transport_get(struct remote *, const char *);
 /* Aggressively fetch annotated tags if possible */
 #define TRANS_OPT_FOLLOWTAGS "followtags"
 
+#define TRANS_OPT_SIGNED_PUSH "signedpush"
+
 /**
  * Returns 0 if the option was used, non-zero otherwise. Prints a
  * message to stderr if the option is not used.
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 8, 2011, 20:01 UTC · re: Junio C Hamano · lore

[PATCH v2 4/7] push -s: send signed push certificate

And this uses the GPG interface to sign the push certificate. The format of the signed certificate is very similar to a signed tag, in that the result is a concatenation of the payload, immediately followed by a detached signature.

This places the same constraint as an annotated tag on the push certificate payload; it has to be a text file and the final line must not be an incomplete line.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/send-pack.c |   29 ++++++++++++-----------------
 1 files changed, 12 insertions(+), 17 deletions(-)
Show changes to builtin/send-pack.c +12 −17
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7f4778c..f715324 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -8,6 +8,7 @@
 #include "send-pack.h"
 #include "quote.h"
 #include "transport.h"
+#include "gpg-interface.h"
 
 static const char send_pack_usage[] =
 "git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
@@ -237,25 +238,18 @@ static int sideband_demux(int in, int out, void *data)
 	return ret;
 }
 
-static void sign_push_certificate(struct strbuf *cert)
+/*
+ * Take the contents of cert->buf, and have the user GPG sign it, and
+ * read it back in the strbuf.
+ */
+static int sign_push_certificate(struct strbuf *cert)
 {
 	/*
-	 * Here, take the contents of cert->buf, and have the user GPG
-	 * sign it, and read it back in the strbuf.
-	 *
-	 * You may want to append some extra info to cert before giving
-	 * it to GPG, possibly via a hook.
-	 *
-	 * Here we upcase them just to demonstrate that the codepath
-	 * is being exercised.
+	 * You may want to append some extra info to cert before
+	 * giving it to GPG, possibly via a hook, here.
 	 */
-	char *cp;
-	for (cp = cert->buf; *cp; cp++) {
-		int ch = *cp;
-		if ('a' <= ch && ch <= 'z')
-			*cp = toupper(ch);
-	}
-	return;
+
+	return sign_buffer(cert, git_committer_info(IDENT_NO_DATE));
 }
 
 int send_pack(struct send_pack_args *args,
@@ -369,7 +363,8 @@ int send_pack(struct send_pack_args *args,
 	if (signed_push && cmds_sent) {
 		char *cp, *ep;
 
-		sign_push_certificate(&push_cert);
+		if (sign_push_certificate(&push_cert))
+			return error(_("failed to sign push certificate"));
 		strbuf_reset(&req_buf);
 		for (cp = push_cert.buf; *cp; cp = ep) {
 			ep = strchrnul(cp, '\n');
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 8, 2011, 20:01 UTC · re: Junio C Hamano · lore

[PATCH v2 5/7] push -s: receiving end

This stores the GPG signed push certificate in the receiving repository using the notes mechanism. The certificate is appended to a note in the refs/notes/signed-push tree for each object that appears on the right hand side of the push certificate, i.e. the object that was pushed to update the tip of a ref.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/receive-pack.c |   72 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 65 insertions(+), 7 deletions(-)
Show changes to builtin/receive-pack.c +65 −7
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 20b6799..344660e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -11,6 +11,11 @@
 #include "transport.h"
 #include "string-list.h"
 #include "sha1-array.h"
+#include "gpg-interface.h"
+#include "notes.h"
+#include "notes-merge.h"
+#include "blob.h"
+#include "tag.h"
 
 static const char receive_pack_usage[] = "git receive-pack <git-dir>";
 
@@ -581,6 +586,22 @@ static void check_aliased_updates(struct command *commands)
 	string_list_clear(&ref_list, 0);
 }
 
+static void get_note_text(struct strbuf *buf, struct notes_tree *t,
+			  const unsigned char *object)
+{
+	const unsigned char *sha1 = get_note(t, object);
+	char *text;
+	unsigned long len;
+	enum object_type type;
+
+	if (!sha1)
+		return;
+	text = read_sha1_file(sha1, &type, &len);
+	if (text && len && type == OBJ_BLOB)
+		strbuf_add(buf, text, len);
+	free(text);
+}
+
 static int record_signed_push(char *cert)
 {
 	/*
@@ -592,18 +613,55 @@ static int record_signed_push(char *cert)
 	 * You could also feed the signed push certificate to GPG,
 	 * verify the signer identity, and all the other fun stuff,
 	 * including feeding it to "pre-receive-signature" hook.
-	 *
-	 * Here we just throw it to stderr to demonstrate that the
-	 * codepath is being exercised.
 	 */
+	size_t total, payload;
 	char *cp, *ep;
-	for (cp = cert; *cp; cp = ep) {
+	int ret = 0;
+	struct notes_tree *t;
+	struct strbuf nbuf = STRBUF_INIT;
+
+	if (!cert)
+		return 0;
+
+	init_notes(NULL, "refs/notes/signed-push", NULL, 0);
+	t = &default_notes_tree;
+
+	total = strlen(cert);
+	payload = parse_signature(cert, total);
+	for (cp = cert; cp < cert + payload; cp = ep) {
+		unsigned char sha1[20], nsha1[20];
+
 		ep = strchrnul(cp, '\n');
 		if (*ep == '\n')
 			ep++;
-		fprintf(stderr, "RSP: %.*s", (int)(ep - cp), cp);
+		if (prefixcmp(cp, "Update: "))
+			continue;
+		cp += strlen("Update: ");
+		if (get_sha1_hex(cp, sha1) || cp[40] != ' ')
+			continue;
+
+		get_note_text(&nbuf, t, sha1);
+		if (nbuf.len)
+			strbuf_addch(&nbuf, '\n');
+		strbuf_add(&nbuf, cert, total);
+		if (write_sha1_file(nbuf.buf, nbuf.len, blob_type, nsha1) ||
+		    add_note(t, sha1, nsha1, NULL))
+			ret = error(_("unable to write note object"));
+		strbuf_reset(&nbuf);
 	}
-	return 0;
+
+	if (!ret) {
+		unsigned char commit[20];
+		unsigned char parent[20];
+		struct ref_lock *lock;
+
+		resolve_ref(t->ref, parent, 0, NULL);
+		lock = lock_any_ref_for_update(t->ref, parent, 0);
+		create_notes_commit(t, NULL, "push", commit);
+		ret = write_ref_sha1(lock, commit, "push");
+	}
+	free_notes(t);
+	return ret;
 }
 
 static void execute_commands(struct command *commands, const char *unpacker_error)
@@ -623,7 +681,7 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
-	if (push_certificate && record_signed_push(push_certificate)) {
+	if (record_signed_push(push_certificate)) {
 		for (cmd = commands; cmd; cmd = cmd->next)
 			cmd->error_string = "n/a (push signature error)";
 		return;
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 8, 2011, 20:01 UTC · re: Junio C Hamano · lore

[PATCH v2 6/7] refactor run_receive_hook()

Running a hook has to make complex set-up to establish web of communication between child process and multiplexer, which is common regardless of what kind of data is fed to the hook. Refactor the parts that is specific to the data fed to the particular set of hooks from the part that runs the hook, so that the code can be reused to drive hooks that take different kind of data.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/receive-pack.c |   71 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 52 insertions(+), 19 deletions(-)
Show changes to builtin/receive-pack.c +52 −19
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 344660e..939b867 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -212,21 +212,15 @@ static int copy_to_sideband(int in, int out, void *arg)
 	return 0;
 }
 
-static int run_receive_hook(struct command *commands, const char *hook_name)
+typedef int (*feed_fn)(void *, const char **, size_t *);
+static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_state)
 {
-	static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
-	struct command *cmd;
 	struct child_process proc;
 	struct async muxer;
 	const char *argv[2];
-	int have_input = 0, code;
-
-	for (cmd = commands; !have_input && cmd; cmd = cmd->next) {
-		if (!cmd->error_string)
-			have_input = 1;
-	}
+	int code;
 
-	if (!have_input || access(hook_name, X_OK) < 0)
+	if (access(hook_name, X_OK) < 0)
 		return 0;
 
 	argv[0] = hook_name;
@@ -254,15 +248,13 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
 		return code;
 	}
 
-	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (!cmd->error_string) {
-			size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
-				sha1_to_hex(cmd->old_sha1),
-				sha1_to_hex(cmd->new_sha1),
-				cmd->ref_name);
-			if (write_in_full(proc.in, buf, n) != n)
-				break;
-		}
+	while (1) {
+		const char *buf;
+		size_t n;
+		if (feed(feed_state, &buf, &n))
+			break;
+		if (write_in_full(proc.in, buf, n) != n)
+			break;
 	}
 	close(proc.in);
 	if (use_sideband)
@@ -270,6 +262,47 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
 	return finish_command(&proc);
 }
 
+struct receive_hook_feed_state {
+	struct command *cmd;
+	struct strbuf buf;
+};
+
+static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
+{
+	struct receive_hook_feed_state *state = state_;
+	struct command *cmd = state->cmd;
+
+	while (cmd && cmd->error_string)
+		cmd = cmd->next;
+	if (!cmd)
+		return -1; /* EOF */
+	strbuf_reset(&state->buf);
+	strbuf_addf(&state->buf, "%s %s %s\n",
+		    sha1_to_hex(cmd->old_sha1), sha1_to_hex(cmd->new_sha1),
+		    cmd->ref_name);
+	state->cmd = cmd->next;
+	if (bufp) {
+		*bufp = state->buf.buf;
+		*sizep = state->buf.len;
+	}
+	return 0;
+}
+
+static int run_receive_hook(struct command *commands, const char *hook_name)
+{
+	struct receive_hook_feed_state state;
+	int status;
+
+	strbuf_init(&state.buf, 0);
+	state.cmd = commands;
+	if (feed_receive_hook(&state, NULL, NULL))
+		return 0;
+	state.cmd = commands;
+	status = run_and_feed_hook(hook_name, feed_receive_hook, &state);
+	strbuf_release(&state.buf);
+	return status;
+}
+
 static int run_update_hook(struct command *cmd)
 {
 	static const char update_hook[] = "hooks/update";
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 8, 2011, 20:01 UTC · re: Junio C Hamano · lore

[PATCH v2 7/7] push -s: support pre-receive-signature hook

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/receive-pack.c |   31 +++++++++++++++++++++++++++----
 1 files changed, 27 insertions(+), 4 deletions(-)
Show changes to builtin/receive-pack.c +27 −4
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 939b867..b5a54e7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -303,6 +303,27 @@ static int run_receive_hook(struct command *commands, const char *hook_name)
 	return status;
 }
 
+static int feed_signature_hook(void *state_, const char **bufp, size_t *sizep)
+{
+	const char **cert_p = state_;
+
+	if (!*cert_p)
+		return -1; /* EOF */
+	*bufp = *cert_p;
+	*cert_p = NULL; /* just return once */
+	*sizep = strlen(*bufp);
+	return 0;
+}
+
+static int run_receive_signature_hook(const char *cert)
+{
+	static const char hook[] = "hooks/pre-receive-signature";
+
+	if (!cert)
+		return 0;
+	return run_and_feed_hook(hook, feed_signature_hook, &cert);
+}
+
 static int run_update_hook(struct command *cmd)
 {
 	static const char update_hook[] = "hooks/update";
@@ -642,10 +663,6 @@ static int record_signed_push(char *cert)
 	 * certificate, grab the commit object names the push updates
 	 * refs to, and append the certificate to the notes to these
 	 * commits.
-	 *
-	 * You could also feed the signed push certificate to GPG,
-	 * verify the signer identity, and all the other fun stuff,
-	 * including feeding it to "pre-receive-signature" hook.
 	 */
 	size_t total, payload;
 	char *cp, *ep;
@@ -714,6 +731,12 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
 		return;
 	}
 
+	if (run_receive_signature_hook(push_certificate)) {
+		for (cmd = commands; cmd; cmd = cmd->next)
+			cmd->error_string = "n/a (pre-receive-signature hook declined)";
+		return;
+	}
+
 	if (record_signed_push(push_certificate)) {
 		for (cmd = commands; cmd; cmd = cmd->next)
 			cmd->error_string = "n/a (push signature error)";
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 9, 2011, 20:41 UTC · re: Junio C Hamano · lore

[PATCH v3 0/4] Signed push

This is based an alternative design we discussed on the list to prepare and push the notes tree on the sending side.

Junio C Hamano (4):
  send-pack: typofix error message
  Split GPG interface into its own helper library
  rename "match_refs()" to "match_push_refs()"
  push -s: signed push
 Makefile             |    2 +
 builtin/push.c       |    1 +
 builtin/remote.c     |    4 +-
 builtin/send-pack.c  |    4 +-
 builtin/tag.c        |   60 +++---------------------
 builtin/verify-tag.c |   35 +------------
 gpg-interface.c      |   94 ++++++++++++++++++++++++++++++++++++
 gpg-interface.h      |   11 ++++
 http-push.c          |    4 +-
 notes.c              |   16 ++++++
 notes.h              |    2 +
 remote.c             |   13 +++--
 remote.h             |    4 +-
 transport.c          |  128 +++++++++++++++++++++++++++++++++++++++++++++++++-
 transport.h          |    5 ++
 15 files changed, 283 insertions(+), 100 deletions(-)
 create mode 100644 gpg-interface.c
 create mode 100644 gpg-interface.h
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 9, 2011, 20:41 UTC · re: Junio C Hamano · lore

[PATCH v3 1/4] send-pack: typofix error message

The message identifies the process as receive-pack when it cannot fork the sideband demultiplexer. We are actually a send-pack.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/send-pack.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin/send-pack.c +1 −1
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index c1f6ddd..87833f4 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -334,7 +334,7 @@ int send_pack(struct send_pack_args *args,
 		demux.data = fd;
 		demux.out = -1;
 		if (start_async(&demux))
-			die("receive-pack: unable to fork off sideband demultiplexer");
+			die("send-pack: unable to fork off sideband demultiplexer");
 		in = demux.out;
 	}
 
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 9, 2011, 20:41 UTC · re: Junio C Hamano · lore

[PATCH v3 2/4] Split GPG interface into its own helper library

This mostly moves existing code from builtin/tag.c (for signing) and builtin/verify-tag.c (for verifying) to a new gpg-interface.c file to provide a more generic library interface.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile             |    2 +
 builtin/tag.c        |   60 ++++----------------------------
 builtin/verify-tag.c |   35 ++-----------------
 gpg-interface.c      |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++
 gpg-interface.h      |   11 ++++++
 5 files changed, 117 insertions(+), 85 deletions(-)
 create mode 100644 gpg-interface.c
 create mode 100644 gpg-interface.h
Show changes to 5 files +117 −85

Makefile, builtin/tag.c, builtin/verify-tag.c, gpg-interface.c, gpg-interface.h

diff --git a/Makefile b/Makefile
index 8d6d451..2183223 100644
--- a/Makefile
+++ b/Makefile
@@ -530,6 +530,7 @@ LIB_H += exec_cmd.h
 LIB_H += fsck.h
 LIB_H += gettext.h
 LIB_H += git-compat-util.h
+LIB_H += gpg-interface.h
 LIB_H += graph.h
 LIB_H += grep.h
 LIB_H += hash.h
@@ -620,6 +621,7 @@ LIB_OBJS += entry.o
 LIB_OBJS += environment.o
 LIB_OBJS += exec_cmd.o
 LIB_OBJS += fsck.o
+LIB_OBJS += gpg-interface.o
 LIB_OBJS += graph.o
 LIB_OBJS += grep.o
 LIB_OBJS += hash.o
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..e9d36fa 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -14,6 +14,7 @@
 #include "parse-options.h"
 #include "diff.h"
 #include "revision.h"
+#include "gpg-interface.h"
 
 static const char * const git_tag_usage[] = {
 	"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
@@ -208,60 +209,13 @@ static int verify_tag(const char *name, const char *ref,
 
 static int do_sign(struct strbuf *buffer)
 {
-	struct child_process gpg;
-	const char *args[4];
-	char *bracket;
-	int len;
-	int i, j;
+	const char *key;
 
-	if (!*signingkey) {
-		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
-				sizeof(signingkey)) > sizeof(signingkey) - 1)
-			return error(_("committer info too long."));
-		bracket = strchr(signingkey, '>');
-		if (bracket)
-			bracket[1] = '\0';
-	}
-
-	/* When the username signingkey is bad, program could be terminated
-	 * because gpg exits without reading and then write gets SIGPIPE. */
-	signal(SIGPIPE, SIG_IGN);
-
-	memset(&gpg, 0, sizeof(gpg));
-	gpg.argv = args;
-	gpg.in = -1;
-	gpg.out = -1;
-	args[0] = "gpg";
-	args[1] = "-bsau";
-	args[2] = signingkey;
-	args[3] = NULL;
-
-	if (start_command(&gpg))
-		return error(_("could not run gpg."));
-
-	if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
-		close(gpg.in);
-		close(gpg.out);
-		finish_command(&gpg);
-		return error(_("gpg did not accept the tag data"));
-	}
-	close(gpg.in);
-	len = strbuf_read(buffer, gpg.out, 1024);
-	close(gpg.out);
-
-	if (finish_command(&gpg) || !len || len < 0)
-		return error(_("gpg failed to sign the tag"));
-
-	/* Strip CR from the line endings, in case we are on Windows. */
-	for (i = j = 0; i < buffer->len; i++)
-		if (buffer->buf[i] != '\r') {
-			if (i != j)
-				buffer->buf[j] = buffer->buf[i];
-			j++;
-		}
-	strbuf_setlen(buffer, j);
-
-	return 0;
+	if (*signingkey)
+		key = signingkey;
+	else
+		key = git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE);
+	return sign_buffer(buffer, key);
 }
 
 static const char tag_template[] =
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 3134766..8b4f742 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -11,6 +11,7 @@
 #include "run-command.h"
 #include <signal.h>
 #include "parse-options.h"
+#include "gpg-interface.h"
 
 static const char * const verify_tag_usage[] = {
 		"git verify-tag [-v|--verbose] <tag>...",
@@ -19,42 +20,12 @@ static const char * const verify_tag_usage[] = {
 
 static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 {
-	struct child_process gpg;
-	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
-	char path[PATH_MAX];
-	size_t len;
-	int fd, ret;
+	int len;
 
-	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
-	if (fd < 0)
-		return error("could not create temporary file '%s': %s",
-						path, strerror(errno));
-	if (write_in_full(fd, buf, size) < 0)
-		return error("failed writing temporary file '%s': %s",
-						path, strerror(errno));
-	close(fd);
-
-	/* find the length without signature */
 	len = parse_signature(buf, size);
 	if (verbose)
 		write_in_full(1, buf, len);
-
-	memset(&gpg, 0, sizeof(gpg));
-	gpg.argv = args_gpg;
-	gpg.in = -1;
-	args_gpg[2] = path;
-	if (start_command(&gpg)) {
-		unlink(path);
-		return error("could not run gpg.");
-	}
-
-	write_in_full(gpg.in, buf, len);
-	close(gpg.in);
-	ret = finish_command(&gpg);
-
-	unlink_or_warn(path);
-
-	return ret;
+	return verify_signed_buffer(buf, size, len);
 }
 
 static int verify_tag(const char *name, int verbose)
diff --git a/gpg-interface.c b/gpg-interface.c
new file mode 100644
index 0000000..b83cca1
--- /dev/null
+++ b/gpg-interface.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "gpg-interface.h"
+#include "sigchain.h"
+
+int sign_buffer(struct strbuf *buffer, const char *signing_key)
+{
+	struct child_process gpg;
+	const char *args[4];
+	ssize_t len;
+	int i, j;
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args;
+	gpg.in = -1;
+	gpg.out = -1;
+	args[0] = "gpg";
+	args[1] = "-bsau";
+	args[2] = signing_key;
+	args[3] = NULL;
+
+	if (start_command(&gpg))
+		return error(_("could not run gpg."));
+
+	/*
+	 * When the username signingkey is bad, program could be terminated
+	 * because gpg exits without reading and then write gets SIGPIPE.
+	 */
+	sigchain_push(SIGPIPE, SIG_IGN);
+
+	if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
+		close(gpg.in);
+		close(gpg.out);
+		finish_command(&gpg);
+		return error(_("gpg did not accept the data"));
+	}
+	close(gpg.in);
+	len = strbuf_read(buffer, gpg.out, 1024);
+	close(gpg.out);
+
+	sigchain_pop(SIGPIPE);
+
+	if (finish_command(&gpg) || !len || len < 0)
+		return error(_("gpg failed to sign the data"));
+
+	/* Strip CR from the line endings, in case we are on Windows. */
+	for (i = j = 0; i < buffer->len; i++)
+		if (buffer->buf[i] != '\r') {
+			if (i != j)
+				buffer->buf[j] = buffer->buf[i];
+			j++;
+		}
+	strbuf_setlen(buffer, j);
+
+	return 0;
+}
+
+int verify_signed_buffer(const char *buf, size_t total, size_t payload)
+{
+	struct child_process gpg;
+	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
+	char path[PATH_MAX];
+	int fd, ret;
+
+	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
+	if (fd < 0)
+		return error("could not create temporary file '%s': %s",
+			     path, strerror(errno));
+	if (write_in_full(fd, buf, total) < 0)
+		return error("failed writing temporary file '%s': %s",
+			     path, strerror(errno));
+	close(fd);
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args_gpg;
+	gpg.in = -1;
+	args_gpg[2] = path;
+	if (start_command(&gpg)) {
+		unlink(path);
+		return error("could not run gpg.");
+	}
+
+	write_in_full(gpg.in, buf, payload);
+	close(gpg.in);
+	ret = finish_command(&gpg);
+
+	unlink_or_warn(path);
+
+	return ret;
+}
diff --git a/gpg-interface.h b/gpg-interface.h
new file mode 100644
index 0000000..7689357
--- /dev/null
+++ b/gpg-interface.h
@@ -0,0 +1,11 @@
+#ifndef GPG_INTERFACE_H
+#define GPG_INTERFACE_H
+
+/*
+ * Copyright (c) 2011, Google Inc.
+ */
+
+extern int sign_buffer(struct strbuf *buffer, const char *signing_key);
+extern int verify_signed_buffer(const char *buffer, size_t total, size_t payload);
+
+#endif
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 9, 2011, 20:41 UTC · re: Junio C Hamano · lore

[PATCH v3 3/4] rename "match_refs()" to "match_push_refs()"

Yes, there is a warning that says the function is only used by push in big red letters in front of this function, but it didn't say a more important thing it should have said: what the function is for and what it does.

Rename it and document it to avoid future confusion.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/remote.c    |    4 ++--
 builtin/send-pack.c |    2 +-
 http-push.c         |    4 ++--
 remote.c            |   13 ++++++++-----
 remote.h            |    4 ++--
 transport.c         |    4 ++--
 6 files changed, 17 insertions(+), 14 deletions(-)
Show changes to 6 files +17 −14

builtin/remote.c, builtin/send-pack.c, http-push.c, remote.c, remote.h, transport.c

diff --git a/builtin/remote.c b/builtin/remote.c
index f2a9c26..f16b544 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -389,8 +389,8 @@ static int get_push_ref_states(const struct ref *remote_refs,
 	local_refs = get_local_heads();
 	push_map = copy_ref_list(remote_refs);
 
-	match_refs(local_refs, &push_map, remote->push_refspec_nr,
-		   remote->push_refspec, MATCH_REFS_NONE);
+	match_push_refs(local_refs, &push_map, remote->push_refspec_nr,
+			remote->push_refspec, MATCH_REFS_NONE);
 
 	states->push.strdup_strings = 1;
 	for (ref = push_map; ref; ref = ref->next) {
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 87833f4..e0b8030 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -509,7 +509,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 		flags |= MATCH_REFS_MIRROR;
 
 	/* match them up */
-	if (match_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))
+	if (match_push_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))
 		return -1;
 
 	set_ref_status_for_push(remote_refs, args.send_mirror,
diff --git a/http-push.c b/http-push.c
index 376331a..02f46a5 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1877,8 +1877,8 @@ int main(int argc, char **argv)
 	}
 
 	/* match them up */
-	if (match_refs(local_refs, &remote_refs,
-		       nr_refspec, (const char **) refspec, push_all)) {
+	if (match_push_refs(local_refs, &remote_refs,
+			    nr_refspec, (const char **) refspec, push_all)) {
 		rc = -1;
 		goto cleanup;
 	}
diff --git a/remote.c b/remote.c
index b8ecfa5..536ffa3 100644
--- a/remote.c
+++ b/remote.c
@@ -1170,12 +1170,15 @@ static struct ref **tail_ref(struct ref **head)
 }
 
 /*
- * Note. This is used only by "push"; refspec matching rules for
- * push and fetch are subtly different, so do not try to reuse it
- * without thinking.
+ * Given the set of refs the local repository has, the set of refs the
+ * remote repository has, and the refspec used for push, determine
+ * what remote refs we will update and with what value by setting
+ * peer_ref (which object is being pushed) and force (if the push is
+ * forced) in elements of "dst". The function may add new elements to
+ * dst (e.g. pushing to a new branch, done in match_explicit_refs).
  */
-int match_refs(struct ref *src, struct ref **dst,
-	       int nr_refspec, const char **refspec, int flags)
+int match_push_refs(struct ref *src, struct ref **dst,
+		    int nr_refspec, const char **refspec, int flags)
 {
 	struct refspec *rs;
 	int send_all = flags & MATCH_REFS_ALL;
diff --git a/remote.h b/remote.h
index 9a30a9d..6729477 100644
--- a/remote.h
+++ b/remote.h
@@ -96,8 +96,8 @@ void free_refspec(int nr_refspec, struct refspec *refspec);
 char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
 		     const char *name);
 
-int match_refs(struct ref *src, struct ref **dst,
-	       int nr_refspec, const char **refspec, int all);
+int match_push_refs(struct ref *src, struct ref **dst,
+		    int nr_refspec, const char **refspec, int all);
 void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 	int force_update);
 
diff --git a/transport.c b/transport.c
index fa279d5..740a739 100644
--- a/transport.c
+++ b/transport.c
@@ -1033,8 +1033,8 @@ int transport_push(struct transport *transport,
 		if (flags & TRANSPORT_PUSH_MIRROR)
 			match_flags |= MATCH_REFS_MIRROR;
 
-		if (match_refs(local_refs, &remote_refs,
-			       refspec_nr, refspec, match_flags)) {
+		if (match_push_refs(local_refs, &remote_refs,
+				    refspec_nr, refspec, match_flags)) {
 			return -1;
 		}
 
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 9, 2011, 20:41 UTC · re: Junio C Hamano · lore

[PATCH v3 4/4] push -s: signed push

If a tag is GPG-signed, and if you trust the cryptographic robustness of the SHA-1 hash and GPG, you can sleep well knowing that all the history leading to the signed commit cannot be tampered with. However, it would be both cumbersome and cluttering to sign each and every commit. Especially if you strive to keep your history clean by tweaking, rewriting and polishing your commits before pushing the resulting history out, many commits you will create locally end up not mattering at all, and it is a waste of time to sign them all.

A better alternative could be to sign a "push certificate" (for the lack of better name) every time you push, asserting that what commits you are pushing to update which refs.

The basic workflow based on this idea would go like this:
 1. You push out your work with "git push -s";
 2. "git push", as usual, learns where the remote refs are and which refs
    are to be updated with this push. It prepares a text file in core,
    that looks like the following:
	Push-Certificate-Version: 0
	Pusher: Junio C Hamano <gitster@pobox.com> 1315427886 -0700
	Update: 3793ac56b4c4f9bf0bddc306a0cec21118683728 refs/heads/master
	Update: 12850bec0c24b529c9a9df6a95ad4bdeea39373e refs/heads/next
    Each "Update" line shows the new object name at the tip of the ref
    this push tries to update.
    The user then is asked to sign this push certificate using GPG.
 3. The signed push certificate is added as notes in the "signed-push"
    notes tree to the objects listed in the certificate. The push refspec
    is altered to push this notes tree to the other side.

Compared to the alternative design posted earlier on the list, this does not require changes in the receiving end, as the signed push certificate is added to the notes tree on the sending side. A possible downside is that it may become more likely that a push is refused due to a conflict while updating the notes tree if the receiving repository is pushed into frequently and by multiple people.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |    1 +
 notes.c        |   16 +++++++
 notes.h        |    2 +
 transport.c    |  124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.h    |    5 ++
 5 files changed, 148 insertions(+), 0 deletions(-)
Show changes to 5 files +148 −0

builtin/push.c, notes.c, notes.h, transport.c, transport.h

diff --git a/builtin/push.c b/builtin/push.c
index 35cce53..2238f4e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -261,6 +261,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
 			TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
+		OPT_BIT('s', "signed", &flags, "GPG sign the push", TRANSPORT_PUSH_SIGNED),
 		OPT_END()
 	};
 
diff --git a/notes.c b/notes.c
index 93e9868..d081e7c 100644
--- a/notes.c
+++ b/notes.c
@@ -1296,3 +1296,19 @@ void expand_notes_ref(struct strbuf *sb)
 	else
 		strbuf_insert(sb, 0, "refs/notes/", 11);
 }
+
+void get_note_text(struct strbuf *buf, struct notes_tree *t,
+		   const unsigned char *object)
+{
+	const unsigned char *sha1 = get_note(t, object);
+	char *text;
+	unsigned long len;
+	enum object_type type;
+
+	if (!sha1)
+		return;
+	text = read_sha1_file(sha1, &type, &len);
+	if (text && len && type == OBJ_BLOB)
+		strbuf_add(buf, text, len);
+	free(text);
+}
diff --git a/notes.h b/notes.h
index c716694..5141e13 100644
--- a/notes.h
+++ b/notes.h
@@ -312,4 +312,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,
 /* Expand inplace a note ref like "foo" or "notes/foo" into "refs/notes/foo" */
 void expand_notes_ref(struct strbuf *sb);
 
+void get_note_text(struct strbuf *, struct notes_tree *, const unsigned char *);
+
 #endif
diff --git a/transport.c b/transport.c
index 740a739..de61669 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,12 @@
 #include "branch.h"
 #include "url.h"
 #include "submodule.h"
+#include "gpg-interface.h"
+#include "commit.h"
+#include "notes.h"
+#include "notes-merge.h"
+#include "blob.h"
+#include "tag.h"
 
 /* rsync support */
 
@@ -476,6 +482,9 @@ static int set_git_option(struct git_transport_options *opts,
 		else
 			opts->depth = atoi(value);
 		return 0;
+	} else if (!strcmp(name, TRANS_OPT_SIGNED_PUSH)) {
+		opts->signed_push = !!value;
+		return 0;
 	}
 	return 1;
 }
@@ -1004,6 +1013,112 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 	transport->progress = force_progress || (verbosity >= 0 && isatty(2));
 }
 
+static int is_ref_pushed(const struct ref *ref)
+{
+	if (!ref->peer_ref || ref->deletion)
+		return 0;
+
+	/* Filter out unchanged ones */
+	switch (ref->status) {
+	case REF_STATUS_REJECT_NONFASTFORWARD:
+	case REF_STATUS_UPTODATE:
+		return 0;
+	default:
+		; /* ok */
+	}
+
+	return 1;
+}
+
+static const char push_signature_note[] = "refs/notes/signed-push";
+
+static int add_push_signature_note(struct ref *signature_note,
+				   struct ref *ref,
+				   struct strbuf *cert)
+{
+	struct notes_tree *notes_tree;
+	struct strbuf nbuf = STRBUF_INIT;
+	int ret = 0;
+	unsigned char parent[20], commit[20];
+	struct ref_lock *lock;
+
+	init_notes(NULL, push_signature_note, NULL, 0);
+	notes_tree = &default_notes_tree;
+
+	resolve_ref(notes_tree->ref, parent, 0, NULL);
+	lock = lock_any_ref_for_update(notes_tree->ref, parent, 0);
+
+	for ( ; ref; ref = ref->next) {
+		unsigned char nsha1[20];
+
+		if ((ref == signature_note) || !is_ref_pushed(ref))
+			continue;
+		get_note_text(&nbuf, notes_tree, ref->new_sha1);
+		if (nbuf.len)
+			strbuf_addch(&nbuf, '\n');
+		strbuf_add(&nbuf, cert->buf, cert->len);
+		if (write_sha1_file(nbuf.buf, nbuf.len, blob_type, nsha1) ||
+		    add_note(notes_tree, ref->new_sha1, nsha1, NULL))
+			ret = error(_("unable to write note object"));
+		strbuf_reset(&nbuf);
+	}
+
+	if (!ret) {
+		create_notes_commit(notes_tree, NULL, "push", commit);
+		ret = write_ref_sha1(lock, commit, "signed push");
+	}
+	free_notes(notes_tree);
+
+	if (!ret) {
+		hashcpy(signature_note->new_sha1, commit);
+		if (!signature_note->peer_ref)
+			signature_note->peer_ref = alloc_ref(push_signature_note);
+	}
+	return ret;
+}
+
+static int sign_push_certificate(struct strbuf *cert)
+{
+	return sign_buffer(cert, git_committer_info(IDENT_NO_DATE));
+}
+
+static int sign_push(struct transport *transport,
+		     struct ref *remote_refs,
+		     int flags)
+{
+	struct ref *ref, *tail = NULL, *signature_note = NULL;
+	struct strbuf push_cert = STRBUF_INIT;
+	int updates = 0, ret = 0;
+
+	if (flags & TRANSPORT_PUSH_DRY_RUN)
+		return 0;
+
+	strbuf_addstr(&push_cert, "Push-Certificate-Version: 0\n");
+	strbuf_addf(&push_cert, "Pusher: %s\n", git_committer_info(0));
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		tail = ref;
+		if (!strcmp(ref->name, push_signature_note))
+			signature_note = ref;
+		if (!is_ref_pushed(ref))
+			continue;
+		updates++;
+		strbuf_addf(&push_cert, "Update: %s %s\n",
+			    sha1_to_hex(ref->new_sha1), ref->name);
+	}
+
+	if (updates && !sign_push_certificate(&push_cert)) {
+		if (!signature_note) {
+			signature_note = alloc_ref(push_signature_note);
+			tail->next = signature_note;
+		}
+		ret = add_push_signature_note(signature_note,
+					      remote_refs, &push_cert);
+	}
+	strbuf_release(&push_cert);
+	return ret;
+}
+
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
 		   int *nonfastforward)
@@ -1015,6 +1130,9 @@ int transport_push(struct transport *transport,
 		/* Maybe FIXME. But no important transport uses this case. */
 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			die("This transport does not support using --set-upstream");
+		/* Likewise */
+		if (flags & TRANSPORT_PUSH_SIGNED)
+			die("This transport does not support using --signed");
 
 		return transport->push(transport, refspec_nr, refspec, flags);
 	} else if (transport->push_refs) {
@@ -1050,7 +1168,13 @@ int transport_push(struct transport *transport,
 					die("There are unpushed submodules, aborting.");
 		}
 
+		if (flags & TRANSPORT_PUSH_SIGNED) {
+			if (sign_push(transport, remote_refs, flags))
+				return -1;
+		}
+
 		push_ret = transport->push_refs(transport, remote_refs, flags);
+
 		err = push_had_errors(remote_refs);
 		ret = push_ret | err;
 
diff --git a/transport.h b/transport.h
index 059b330..034ff5a 100644
--- a/transport.h
+++ b/transport.h
@@ -8,6 +8,7 @@ struct git_transport_options {
 	unsigned thin : 1;
 	unsigned keep : 1;
 	unsigned followtags : 1;
+	unsigned signed_push : 1;
 	int depth;
 	const char *uploadpack;
 	const char *receivepack;
@@ -102,6 +103,7 @@ struct transport {
 #define TRANSPORT_PUSH_PORCELAIN 16
 #define TRANSPORT_PUSH_SET_UPSTREAM 32
 #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
+#define TRANSPORT_PUSH_SIGNED 128
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 
@@ -128,6 +130,9 @@ struct transport *transport_get(struct remote *, const char *);
 /* Aggressively fetch annotated tags if possible */
 #define TRANS_OPT_FOLLOWTAGS "followtags"
 
+/* Signed push */
+#define TRANS_OPT_SIGNED_PUSH "signedpush"
+
 /**
  * Returns 0 if the option was used, non-zero otherwise. Prints a
  * message to stderr if the option is not used.
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 9, 2011, 21:16 UTC · re: Junio C Hamano · lore

[PATCH v3.1 4/4] push -s: signed push

The earlier one had some unnecessary bits specific to git transport copied from the v2 patch, but this alternative design is transport agnostic, so here is a replacemnt to remove them.

-- >8 --
Subject: [PATCH] push -s: signed push

If a tag is GPG-signed, and if you trust the cryptographic robustness of the SHA-1 hash and GPG, you can sleep well knowing that all the history leading to the signed commit cannot be tampered with. However, it would be both cumbersome and cluttering to sign each and every commit. Especially if you strive to keep your history clean by tweaking, rewriting and polishing your commits before pushing the resulting history out, many commits you will create locally end up not mattering at all, and it is a waste of time to sign them all.

A better alternative could be to sign a "push certificate" (for the lack of better name) every time you push, asserting that what commits you are pushing to update which refs.

The basic workflow based on this idea would go like this:
 1. You push out your work with "git push -s";
 2. "git push", as usual, learns where the remote refs are and which refs
    are to be updated with this push. It prepares a text file in core,
    that looks like the following:
	Push-Certificate-Version: 0
	Pusher: Junio C Hamano <gitster@pobox.com> 1315427886 -0700
	Update: 3793ac56b4c4f9bf0bddc306a0cec21118683728 refs/heads/master
	Update: 12850bec0c24b529c9a9df6a95ad4bdeea39373e refs/heads/next
    Each "Update" line shows the new object name at the tip of the ref
    this push tries to update.
    The user then is asked to sign this push certificate using GPG.
 3. The signed push certificate is added as notes in the "signed-push"
    notes tree to the objects listed in the certificate. The push refspec
    is altered to push this notes tree to the other side.

Compared to the alternative design posted earlier on the list, this does not require changes in the receiving end, as the signed push certificate is added to the notes tree on the sending side. A possible downside is that it may become more likely that a push is refused due to a conflict while updating the notes tree if the receiving repository is pushed into frequently and by multiple people.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c |    1 +
 notes.c        |   16 +++++++
 notes.h        |    2 +
 transport.c    |  120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.h    |    2 +
 5 files changed, 141 insertions(+), 0 deletions(-)
Show changes to 5 files +141 −0

builtin/push.c, notes.c, notes.h, transport.c, transport.h

diff --git a/builtin/push.c b/builtin/push.c
index 35cce53..2238f4e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -261,6 +261,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
 			TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
+		OPT_BIT('s', "signed", &flags, "GPG sign the push", TRANSPORT_PUSH_SIGNED),
 		OPT_END()
 	};
 
diff --git a/notes.c b/notes.c
index 93e9868..d081e7c 100644
--- a/notes.c
+++ b/notes.c
@@ -1296,3 +1296,19 @@ void expand_notes_ref(struct strbuf *sb)
 	else
 		strbuf_insert(sb, 0, "refs/notes/", 11);
 }
+
+void get_note_text(struct strbuf *buf, struct notes_tree *t,
+		   const unsigned char *object)
+{
+	const unsigned char *sha1 = get_note(t, object);
+	char *text;
+	unsigned long len;
+	enum object_type type;
+
+	if (!sha1)
+		return;
+	text = read_sha1_file(sha1, &type, &len);
+	if (text && len && type == OBJ_BLOB)
+		strbuf_add(buf, text, len);
+	free(text);
+}
diff --git a/notes.h b/notes.h
index c716694..5141e13 100644
--- a/notes.h
+++ b/notes.h
@@ -312,4 +312,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,
 /* Expand inplace a note ref like "foo" or "notes/foo" into "refs/notes/foo" */
 void expand_notes_ref(struct strbuf *sb);
 
+void get_note_text(struct strbuf *, struct notes_tree *, const unsigned char *);
+
 #endif
diff --git a/transport.c b/transport.c
index 740a739..702d438 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,12 @@
 #include "branch.h"
 #include "url.h"
 #include "submodule.h"
+#include "gpg-interface.h"
+#include "commit.h"
+#include "notes.h"
+#include "notes-merge.h"
+#include "blob.h"
+#include "tag.h"
 
 /* rsync support */
 
@@ -1004,6 +1010,112 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 	transport->progress = force_progress || (verbosity >= 0 && isatty(2));
 }
 
+static int is_ref_pushed(const struct ref *ref)
+{
+	if (!ref->peer_ref || ref->deletion)
+		return 0;
+
+	/* Filter out unchanged ones */
+	switch (ref->status) {
+	case REF_STATUS_REJECT_NONFASTFORWARD:
+	case REF_STATUS_UPTODATE:
+		return 0;
+	default:
+		; /* ok */
+	}
+
+	return 1;
+}
+
+static const char push_signature_note[] = "refs/notes/signed-push";
+
+static int add_push_signature_note(struct ref *signature_note,
+				   struct ref *ref,
+				   struct strbuf *cert)
+{
+	struct notes_tree *notes_tree;
+	struct strbuf nbuf = STRBUF_INIT;
+	int ret = 0;
+	unsigned char parent[20], commit[20];
+	struct ref_lock *lock;
+
+	init_notes(NULL, push_signature_note, NULL, 0);
+	notes_tree = &default_notes_tree;
+
+	resolve_ref(notes_tree->ref, parent, 0, NULL);
+	lock = lock_any_ref_for_update(notes_tree->ref, parent, 0);
+
+	for ( ; ref; ref = ref->next) {
+		unsigned char nsha1[20];
+
+		if ((ref == signature_note) || !is_ref_pushed(ref))
+			continue;
+		get_note_text(&nbuf, notes_tree, ref->new_sha1);
+		if (nbuf.len)
+			strbuf_addch(&nbuf, '\n');
+		strbuf_add(&nbuf, cert->buf, cert->len);
+		if (write_sha1_file(nbuf.buf, nbuf.len, blob_type, nsha1) ||
+		    add_note(notes_tree, ref->new_sha1, nsha1, NULL))
+			ret = error(_("unable to write note object"));
+		strbuf_reset(&nbuf);
+	}
+
+	if (!ret) {
+		create_notes_commit(notes_tree, NULL, "push", commit);
+		ret = write_ref_sha1(lock, commit, "signed push");
+	}
+	free_notes(notes_tree);
+
+	if (!ret) {
+		hashcpy(signature_note->new_sha1, commit);
+		if (!signature_note->peer_ref)
+			signature_note->peer_ref = alloc_ref(push_signature_note);
+	}
+	return ret;
+}
+
+static int sign_push_certificate(struct strbuf *cert)
+{
+	return sign_buffer(cert, git_committer_info(IDENT_NO_DATE));
+}
+
+static int sign_push(struct transport *transport,
+		     struct ref *remote_refs,
+		     int flags)
+{
+	struct ref *ref, *tail = NULL, *signature_note = NULL;
+	struct strbuf push_cert = STRBUF_INIT;
+	int updates = 0, ret = 0;
+
+	if (flags & TRANSPORT_PUSH_DRY_RUN)
+		return 0;
+
+	strbuf_addstr(&push_cert, "Push-Certificate-Version: 0\n");
+	strbuf_addf(&push_cert, "Pusher: %s\n", git_committer_info(0));
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		tail = ref;
+		if (!strcmp(ref->name, push_signature_note))
+			signature_note = ref;
+		if (!is_ref_pushed(ref))
+			continue;
+		updates++;
+		strbuf_addf(&push_cert, "Update: %s %s\n",
+			    sha1_to_hex(ref->new_sha1), ref->name);
+	}
+
+	if (updates && !sign_push_certificate(&push_cert)) {
+		if (!signature_note) {
+			signature_note = alloc_ref(push_signature_note);
+			tail->next = signature_note;
+		}
+		ret = add_push_signature_note(signature_note,
+					      remote_refs, &push_cert);
+	}
+	strbuf_release(&push_cert);
+	return ret;
+}
+
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
 		   int *nonfastforward)
@@ -1015,6 +1127,9 @@ int transport_push(struct transport *transport,
 		/* Maybe FIXME. But no important transport uses this case. */
 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			die("This transport does not support using --set-upstream");
+		/* Likewise */
+		if (flags & TRANSPORT_PUSH_SIGNED)
+			die("This transport does not support using --signed");
 
 		return transport->push(transport, refspec_nr, refspec, flags);
 	} else if (transport->push_refs) {
@@ -1050,6 +1165,11 @@ int transport_push(struct transport *transport,
 					die("There are unpushed submodules, aborting.");
 		}
 
+		if (flags & TRANSPORT_PUSH_SIGNED) {
+			if (sign_push(transport, remote_refs, flags))
+				return -1;
+		}
+
 		push_ret = transport->push_refs(transport, remote_refs, flags);
 		err = push_had_errors(remote_refs);
 		ret = push_ret | err;
diff --git a/transport.h b/transport.h
index 059b330..e525d07 100644
--- a/transport.h
+++ b/transport.h
@@ -8,6 +8,7 @@ struct git_transport_options {
 	unsigned thin : 1;
 	unsigned keep : 1;
 	unsigned followtags : 1;
+	unsigned signed_push : 1;
 	int depth;
 	const char *uploadpack;
 	const char *receivepack;
@@ -102,6 +103,7 @@ struct transport {
 #define TRANSPORT_PUSH_PORCELAIN 16
 #define TRANSPORT_PUSH_SET_UPSTREAM 32
 #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
+#define TRANSPORT_PUSH_SIGNED 128
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 
-- 
1.7.7.rc0.188.g3793ac
Junio C Hamano· Sep 10, 2011, 05:19 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/4] Signed push

Now that I have tried two concrete implementations and had a chance to play with both ideas, even though I haven't started pushing to other repositories with signature for real, I can make some observations with a certain degree of confidence. I have to say that there is one thing I do not really like about this.

During my typical work day, I usually run at least one, but possibly up to three to four integration cycles, and each cycle is concluded with these actions (in this order):

 - Push maint/master/next/pu integration branches and optionaly todo
   branch to k.org;
 - Fetch html/man branches from k.org that are auto-updated as the result
   of the above push, if necessary;
 - Push maint/master/next/pu/html/man branches and optionally todo branch
   to repo.or.cz and code.google.com;
 - Push maint/master/html/man branches and optionally todo branch to
   sourceforge.net and sourceforge.jp; and finally
 - Mirror-push everything to github.com.
First let's make it clear what is _not_ what I do _not_ like.

I do not mind having to unlock my GPG key for each of the above set of pushes. After all, these four classes of repositories:

 (1) the primary public repository at k.org;
 (2) its two mirrors at repo and google;
 (3) the secondary partial mirrors at two sourceforge sites; and
 (4) full mirror at github

receive different sets of branches. It is not just I do not mind, but I actively would prefer, making separate assertions to say "These commits this particular site has were placed here by me to these branches with this push", and I think it is preferrable that the assertions are customized for destination sites. So it is natural to generate separate push certificates to make separate assertions, and that is why unlocking the key multiple times nor signing one document per push destination is not a problem.

Now, with that behind us, let's discuss what I do _not_ like.

The v3 design that prepares and pushes the signed-push notes tree locally has one major downside that I did not anticipate before starting to really think about using signed pushes.

Think about what is contained in the signed-push notes tree that is pushed to sourceforge during one cycle of my work day described above. The site gets neither next nor pu branch, but because

 (1) I push out the same commit at the tip of master as I push to k.org,
     repo and sourceforge;
 (2) the push to sourceforge happens after the pushes to k.org and repo
     happens; and
 (3) in v3 design, I have to keep the notes tree prepared locally before
     pushing things out,

the push certificate note attached to the commit object at the tip of the 'master' branch becomes a concatenation of three pushes, and some of them describe the branches that sourceforge does not get (namely, next and pu that I pushed out to k.org and repo).

Worse yet, after I conclude this round by pushing all the topics to github with signature, my signed-push notes history has push certificates that describe my previous push of _all topics_ to github repository, and on top of that notes tree, signed-push notes to describe the next push to k.org is built, and it is sent to k.org at the beginning of the next cycle.

Now imagine a hypothetical universe where my k.org repository is the "open source Git" (eh, that is not hypothetical), but some of the topic branches that are not merged to pu were the "proprietary extensions" published only to the github repository. Further imagine that in this hypothetical universe, my github repository is not the "open source" one, but is a proprietary "partner server" I use to work with other "Git proprietary extensions" closed source commercial project.

You can easily understand why the behaviour of the natural consequence of the v3 design is a horrible mistake. In such a set-up, even branch names may contain sensitive information, yet we are exposing the record of pushes to github to the shared signed-push notes tree at k.org during the next round. The presense of such a partner server may even be a sensitive piece of information that shouldn't be devulged to the open source k.org repository.

There is no such problem with the v2 design, where the reciever site is solely responsible to keep the push certificate and does not force me to have copies of all the push certificates locally to preserve ordering. The signed-push notes for each hosting site contain _only_ push certificates that pertain to pushes to _that_ site in the v2 design.

And I think that really is the right way to do a signed push.

The primary motivation of the signed push is for me to assert "These commits this particular site has were placed here by me to these branches with this push", so that people can be sure that the commits near the tip of my branches since the latest tagged commits (which are signed by my GPG key) did come from me. In order to achieve that goal, k.org repository has no business knowing what I pushed to github repository, and github repository has no business knowing what I pushed to k.org repository.

Even under v2 design, if somebody who has access to both k.org (public) and github (proprietary in the hypothetical universe) would want to combine the signed-push notes to see a unified picture (perhaps I push to these two sites with different frequencies), he can fetch signed-push notes from both sites and merge them himself. But v3 design also allows anybody who has access to k.org (which is public so by definition that truly is anybody) to peek into signed-push notes at k.org to learn more than he should be able to.

It is cumbersome, if not impossible, to achieve the same separation v2 design gives us by default. The pusher of course can maintain separate signed-push branches per push destinations, but I think that is merely a workaround.

To begin with, there is no real reason for _me_ to keep _any_ signed-push record for places _I_ pushed. The assertion I am making by signing my pushes is for people who get commits beyond my last tagged commits to be sure that they do come from me, and while it is fine to have back-up copies of them for your own use, there is no reason to _require_ me to do so. But the "locally prepare notes tree and push that out along with what you wanted to push out" design does exactly that.

Sverre Rabbelier· Sep 10, 2011, 15:17 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/4] Signed push

Heya,
On Sat, Sep 10, 2011 at 07:19, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> Even under v2 design, if somebody who has access to both k.org (public)
> and github (proprietary in the hypothetical universe) would want to
> combine the signed-push notes to see a unified picture (perhaps I push to
> these two sites with different frequencies), he can fetch signed-push
> notes from both sites and merge them himself. But v3 design also allows
> anybody who has access to k.org (which is public so by definition that
> truly is anybody) to peek into signed-push notes at k.org to learn more
> than he should be able to.

I think this is also some further motivation to have a refs/remotes/github/notes/signed-push and a refs/remotes/korg/notes/signed-push, rather than have everything automatically go into refs/notes/signed-push when fetching from a remote.

(If I misremembered and it's already that way, please ignore this message :P)
-- 
Cheers,

Sverre Rabbelier
Junio C Hamano· Sep 10, 2011, 16:30 UTC · re: Sverre Rabbelier · lore

Re: [PATCH v3 0/4] Signed push

Sverre Rabbelier <srabbelier@gmail.com> writes:
> I think this is also some further motivation to have a

Did you miss that I already mentioned that workaround? It does not _fix_ the fundamental breakage, which is that you are _forcing_ the sending side to keep copies, though.

Ted Ts'o· Sep 10, 2011, 19:22 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/4] Signed push

On Sat, Sep 10, 2011 at 09:30:29AM -0700, Junio C Hamano wrote:
Show 7 quoted lines
> Sverre Rabbelier <srabbelier@gmail.com> writes:
> 
> > I think this is also some further motivation to have a
> 
> Did you miss that I already mentioned that workaround?  It does not _fix_
> the fundamental breakage, which is that you are _forcing_ the sending side
> to keep copies, though.
I guess I'm confused about what the problem is with this?
If I do something like this:

git tag -s -m for_linus-20110910 for_linus-20110910 git push github git push --tags github

I'm "forcing" the sending side to keep the signed tag, no? Isn't that kind of implicit in allowing someone to push to your repo?

     		    	     	     	     	- Ted
Junio C Hamano· Sep 11, 2011, 01:42 UTC · re: Ted Ts'o · lore

Re: [PATCH v3 0/4] Signed push

Ted Ts'o <tytso@mit.edu> writes:
> I guess I'm confused about what the problem is with this?
Yeah, I have to agree.
Show 7 quoted lines
> If I do something like this:
>
> git tag -s -m for_linus-20110910 for_linus-20110910
> git push github
> git push --tags github
>
> I'm "forcing" the sending side to keep the signed tag, no?

No, you are not forced to _keep_ it. After pushing you can delete it locally.

The reason your "tag" example is fundamentally different is because a tag like for_linus_20110910 is a one-shot thing and you can choose to remove it from your local namespace once you are done pushing. It does not affect your ability to make another signed tag for_linus_20110911 before pushing tomorrow.

The point in this round of "signed push" topic is to allow people not tag every time before they push, making it easier to sign their pushes to encourage it, so that other people can have a way to verify the commits near the tip of branches that are not still tagged in between releases. Instead of contaminating refs/tags/ namespace with daily tags, the idea was to keep a single "signed-push" notes tree on the receiving end (which is the distribution point for consumers) that contain the signed record of pushes.

The original "signed push" (what I called v2) design was for the sender to prepare the record that goes into the notes tree, but record the notes tree at the receiving end (this does _not_ prevent the sender from fetching it back to keep his local copy, but the sender is _not_ required to do so). It needs updates to both sending and receiving end.

An alternative idea (which I implemented as v3) that came up during the discussion was to instead have the sender add this record locally to the signed-push notes tree, and push it out along with the branches. For this push not to lose _existing_ records of pushes at the receiving end, the pusher is required to have an up-to-date copy of signed-push notes tree, and add the new record to it before pushing it out. One upside is that this does not need updates to receiving end.

I do not know if you read the message Sverre was responding to, but the "you have to have local copy" requirement has another and potentially bigger downside (which Sverre did not quote) for people who push out to multiple places.

Perhaps we shouldn't worry about tag namespace contamination to make things easier and simpler and stop using notes tree?

Sverre Rabbelier· Sep 11, 2011, 08:53 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/4] Signed push

Heya,
On Sun, Sep 11, 2011 at 03:42, Junio C Hamano <gitster@pobox.com> wrote:
> I do not know if you read the message Sverre was responding to, but the
> "you have to have local copy" requirement has another and potentially
> bigger downside (which Sverre did not quote) for people who push out to
> multiple places.

That's not what I meant though. I was responding to your "than you can later _inspect_ the certificates from multiple locations". I was indicating that it would be easier to do such inspection if you can optionally fetch the notes from different remotes to different locations in the refs/remotes namespace.

-- 
Cheers,

Sverre Rabbelier
Ted Ts'o· Sep 11, 2011, 15:51 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/4] Signed push

On Sat, Sep 10, 2011 at 06:42:58PM -0700, Junio C Hamano wrote:
> Perhaps we shouldn't worry about tag namespace contamination to make
> things easier and simpler and stop using notes tree?

With the appropriate conventions, such as using a tag name such as signed-<email>-<timestamp>" we can at least avoid name conflicts, at least for all practical purposes.

There is the additional problem that "git tag -l" gets painful. At least for me, though, it's already mostly useless:

   % git tag -l | grep ^v[23] | wc -l
   858

I can work around this with git aliases that filter out certain prefixes that I normally don't care about, but maybe that's something that should be directly supported in git-tag with some git-config parameters.

The final issue that I'd worry about with using tags is performance. If we have hundreds of thousands or millions of tags of the form signed-<email>-<timestamp>, is this going to be a problem? This does seem like something that could be worked around --- in the worst case there could just be a locally maintained reverse index from git commit id's to tag names. (Although as I recall Linus objected to having something like this for time skews, so maybe he'd object to this too.)

	       	    	     	       	     	  - Ted
Robin H. Johnson· Sep 10, 2011, 20:05 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/4] Signed push

On Sat, Sep 10, 2011 at 09:30:29AM -0700,  Junio C Hamano wrote:
Show 7 quoted lines
> Sverre Rabbelier <srabbelier@gmail.com> writes:
> 
> > I think this is also some further motivation to have a
> 
> Did you miss that I already mentioned that workaround?  It does not _fix_
> the fundamental breakage, which is that you are _forcing_ the sending side
> to keep copies, though.

In the case of a shared Git repo, I'd like to pull copies added by other developers, so that I can verify the content locally as well. In that case, I'd need to have copies of the certificates I created as well.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

← back to recent threads