Re: How safe are signed git tags? Only as safe as SHA-1 or somehow safer?

[Duy Nguyen <pclouds@gmail.com>]

On Mon, Nov 24, 2014 at 06:44:10PM +0700, Duy Nguyen wrote:
> >> I wonder if we can have an option to sign all blob content of the tree
> >> associated to a commit, and the content of parent commit(s). It's more
> >> expensive than signing just commit/tag content. But it's also safer
> >> without completely ditching SHA-1.
> >>
> >
> > This amounts to hashing the blob content with whatever hash you told
> > your gpg to use (hopefully not sha1 ;) ) and signing that.
> >
> > You're free to do that now and store the signature wherever your
> > toolchain deems fit, say in a note or an annotated tag. But that
> > approach won't sign the history, that is: If you are concerned about the
> > breakability of sha1, then history is "possibly broken" no matter how
> > you sign a commit object whose "parent" entry is based on the sha1 of
> > its parent object.
> 
> If you store the singature in commit message, and if you hash the
> parent commit messages as well, which are also signed, then you have
> the same chaining effect that we have with SHA-1. I think this could
> be done with some hooks already, except maybe for the verification
> part.

To demonstrate, a hook like this can take commit object from stdin and
produce some hash lines, which are appended at the end of the commit
message before the new commit object is created. So if I commit "foo"
the final commit message would be

  foo

  SHA512: <long hash>

This script uses sha512sum, but you can add as many hashes as you want
(and pay the penalty at commit time, of course). I think it covers
enough content to validate history up to the last signed commit.

-- 8< --
#!/bin/bash
# commit content
cat >$GIT_DIR/tmp
# parent commit content
sed '/^$/q' $GIT_DIR/tmp |
    grep '^parent ' |
    cut -c 8- |
    xargs -n1 git cat-file commit >>$GIT_DIR/tmp
# all blobs
sed '/^$/q' $GIT_DIR/tmp |
    grep '^tree ' |
    cut -c 6- |
    xargs -n1 git ls-tree -r |
    cut -c 13-52 |
    git cat-file --batch >>$GIT_DIR/tmp
echo
echo "SHA512: `sha512sum < $GIT_DIR/tmp`"
-- 8< --

An extra patch is required to hook this in final commit steps.

-- 8< --
diff --git a/commit.c b/commit.c
index 19cf8f9..c447c1d 100644
--- a/commit.c
+++ b/commit.c
@@ -11,6 +11,8 @@
 #include "commit-slab.h"
 #include "prio-queue.h"
 #include "sha1-lookup.h"
+#include "run-command.h"
+#include "sigchain.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
 
@@ -1064,6 +1066,36 @@ struct commit_list *reduce_heads(struct commit_list *heads)
 	return result;
 }
 
+static int run_sign_commit_hook(struct strbuf *buf)
+{
+	struct child_process hook = CHILD_PROCESS_INIT;
+	const char *p = find_hook("sign-commit");
+	int len;
+
+	if (!p)
+		return 0;
+
+	argv_array_push(&hook.args, p);
+	hook.in = -1;
+	hook.out = -1;
+	if (start_command(&hook))
+		return error(_("could not run sign-commit hook"));
+	sigchain_push(SIGPIPE, SIG_IGN);
+	if (write_in_full(hook.in, buf->buf, buf->len) != buf->len) {
+		close(hook.in);
+		close(hook.out);
+		finish_command(&hook);
+		return error(_("sign-commit hook did not accept the data"));
+	}
+	close(hook.in);
+	len = strbuf_read(buf, hook.out, 1024);
+	close(hook.out);
+	sigchain_pop(SIGPIPE);
+	if (finish_command(&hook) || len <= 0)
+		return error(_("sign-commit hook failed to sign the data"));
+	return 0;
+}
+
 static const char gpg_sig_header[] = "gpgsig";
 static const int gpg_sig_header_len = sizeof(gpg_sig_header) - 1;
 
@@ -1555,6 +1587,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 	if (encoding_is_utf8 && !verify_utf8(&buffer))
 		fprintf(stderr, commit_utf8_warn);
 
+	if (run_sign_commit_hook(&buffer))
+		return -1;
+
 	if (sign_commit && do_sign_commit(&buffer, sign_commit))
 		return -1;
 
-- 8< --