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

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

[Junio C Hamano]

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

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

[Junio C Hamano]

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(-)

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

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

[Junio C Hamano]

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

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

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

[Junio C Hamano]

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(-)

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

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

[Junio C Hamano]

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(-)

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

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

[Junio C Hamano]

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(-)

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

[PATCH v2 6/7] refactor run_receive_hook()

[Junio C Hamano]

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(-)

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

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

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/receive-pack.c |   31 +++++++++++++++++++++++++++----
 1 files changed, 27 insertions(+), 4 deletions(-)

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

[PATCH v3 0/4] Signed push

[Junio C Hamano]

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

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

[Junio C Hamano]

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(-)

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

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

[Junio C Hamano]

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

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

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

[Junio C Hamano]

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(-)

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

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

[Junio C Hamano]

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(-)

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

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

[Junio C Hamano]

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(-)

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

Re: [PATCH v3 0/4] Signed push

[Junio C Hamano]

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.

Re: [PATCH v3 0/4] Signed push

[Sverre Rabbelier]

Heya,

On Sat, Sep 10, 2011 at 07:19, Junio C Hamano <gitster@pobox.com> wrote:
> 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

Re: [PATCH v3 0/4] Signed push

[Junio C Hamano]

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.

Re: [PATCH v3 0/4] Signed push

[Ted Ts'o]

On Sat, Sep 10, 2011 at 09:30:29AM -0700, Junio C Hamano wrote:
> 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

Re: [PATCH v3 0/4] Signed push

[Robin H. Johnson]

On Sat, Sep 10, 2011 at 09:30:29AM -0700,  Junio C Hamano wrote:
> 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

Re: [PATCH v3 0/4] Signed push

[Junio C Hamano]

Ted Ts'o <tytso@mit.edu> writes:

> I guess I'm confused about what the problem is with this?

Yeah, I have to agree.

> 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?

Re: [PATCH v3 0/4] Signed push

[Sverre Rabbelier]

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

Re: [PATCH v3 0/4] Signed push

[Ted Ts'o]

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