From 89587fadb211e9c07c3bb2f05f0e90b3a57ac340 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 7 Sep 2011 21:19:47 -0700 Subject: [PATCH 1/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. - sign_buffer() takes a payload strbuf, a signature strbuf, and a signing key, runs "gpg" to produce a detached signature for the payload, and appends it to the signature strbuf. The contents of a signed tag that concatenates the payload and the detached signature can be produced by giving the same strbuf as payload and signature strbuf. - verify_signed_buffer() takes a payload and a detached signature as pairs, and runs "gpg --verify" to see if the payload matches the signature. It can optionally capture the output from GPG to allow the callers to pretty-print it in a way more suitable for their contexts. "verify-tag" (aka "tag -v") used to save the whole tag contents as if it is a detached signature, and fed gpg the payload part of the tag. It relied on gpg to fail when the given tag is not signed but just is annotated. The updated run_gpg_verify() function detects the lack of detached signature in the input, and errors out without bothering "gpg". Signed-off-by: Junio C Hamano --- Makefile | 2 + builtin/tag.c | 76 ++---------------------- builtin/verify-tag.c | 35 ++--------- gpg-interface.c | 138 +++++++++++++++++++++++++++++++++++++++++++ gpg-interface.h | 10 ++++ tag.c | 5 ++ 6 files changed, 166 insertions(+), 100 deletions(-) create mode 100644 gpg-interface.c create mode 100644 gpg-interface.h diff --git a/Makefile b/Makefile index 8d6d4515d2..2183223759 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 667515e527..fb0d4a1648 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 ] [-f] [-m |-F ] []", @@ -23,8 +24,6 @@ static const char * const git_tag_usage[] = { NULL }; -static char signingkey[1000]; - struct tag_filter { const char **patterns; int lines; @@ -208,60 +207,7 @@ 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; - - 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; + return sign_buffer(buffer, buffer, get_signing_key()); } static const char tag_template[] = @@ -270,21 +216,11 @@ static const char tag_template[] = "# Write a tag message\n" "#\n"); -static void set_signingkey(const char *value) -{ - if (strlcpy(signingkey, value, sizeof(signingkey)) >= sizeof(signingkey)) - die(_("signing key value too long (%.10s...)"), value); -} - static int git_tag_config(const char *var, const char *value, void *cb) { - if (!strcmp(var, "user.signingkey")) { - if (!value) - return config_error_nonbool(var); - set_signingkey(value); - return 0; - } - + int status = git_gpg_config(var, value, cb); + if (status) + return status; return git_default_config(var, value, cb); } @@ -463,7 +399,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (keyid) { sign = 1; - set_signingkey(keyid); + set_signing_key(keyid); } if (sign) annotate = 1; diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 3134766049..28c2174338 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -11,6 +11,7 @@ #include "run-command.h" #include #include "parse-options.h" +#include "gpg-interface.h" static const char * const verify_tag_usage[] = { "git verify-tag [-v|--verbose] ...", @@ -19,42 +20,16 @@ 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."); - } + if (size == len) + return error("no signature found"); - 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, len, buf + len, size - len, NULL); } static int verify_tag(const char *name, int verbose) diff --git a/gpg-interface.c b/gpg-interface.c new file mode 100644 index 0000000000..ff232c8c5d --- /dev/null +++ b/gpg-interface.c @@ -0,0 +1,138 @@ +#include "cache.h" +#include "run-command.h" +#include "strbuf.h" +#include "gpg-interface.h" +#include "sigchain.h" + +static char *configured_signing_key; + +void set_signing_key(const char *key) +{ + free(configured_signing_key); + configured_signing_key = xstrdup(key); +} + +int git_gpg_config(const char *var, const char *value, void *cb) +{ + if (!strcmp(var, "user.signingkey")) { + if (!value) + return config_error_nonbool(var); + set_signing_key(value); + } + return 0; +} + +const char *get_signing_key(void) +{ + if (configured_signing_key) + return configured_signing_key; + return git_committer_info(IDENT_ERROR_ON_NO_NAME|IDENT_NO_DATE); +} + +/* + * Create a detached signature for the contents of "buffer" and append + * it after "signature"; "buffer" and "signature" can be the same + * strbuf instance, which would cause the detached signature appended + * at the end. + */ +int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key) +{ + struct child_process gpg; + const char *args[4]; + ssize_t len; + size_t i, j, bottom; + + 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); + + bottom = signature->len; + len = strbuf_read(signature, 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 = bottom; i < signature->len; i++) + if (signature->buf[i] != '\r') { + if (i != j) + signature->buf[j] = signature->buf[i]; + j++; + } + strbuf_setlen(signature, j); + + return 0; +} + +/* + * Run "gpg" to see if the payload matches the detached signature. + * gpg_output_to tells where the output from "gpg" should go: + * < 0: /dev/null + * = 0: standard error of the calling process + * > 0: the specified file descriptor + */ +int verify_signed_buffer(const char *payload, size_t payload_size, + const char *signature, size_t signature_size, + struct strbuf *gpg_output) +{ + 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, signature, signature_size) < 0) + return error("failed writing detached signature to '%s': %s", + path, strerror(errno)); + close(fd); + + memset(&gpg, 0, sizeof(gpg)); + gpg.argv = args_gpg; + gpg.in = -1; + if (gpg_output) + gpg.err = -1; + args_gpg[2] = path; + if (start_command(&gpg)) { + unlink(path); + return error("could not run gpg."); + } + + write_in_full(gpg.in, payload, payload_size); + close(gpg.in); + + if (gpg_output) + strbuf_read(gpg_output, gpg.err, 0); + 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 0000000000..b9c36088ce --- /dev/null +++ b/gpg-interface.h @@ -0,0 +1,10 @@ +#ifndef GPG_INTERFACE_H +#define GPG_INTERFACE_H + +extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key); +extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output); +extern int git_gpg_config(const char *, const char *, void *); +extern void set_signing_key(const char *); +extern const char *get_signing_key(void); + +#endif diff --git a/tag.c b/tag.c index 7d38cc0f4d..3aa186df62 100644 --- a/tag.c +++ b/tag.c @@ -139,6 +139,11 @@ int parse_tag(struct tag *item) return ret; } +/* + * Look at a signed tag object, and return the offset where + * the embedded detached signature begins, or the end of the + * data when there is no such signature. + */ size_t parse_signature(const char *buf, unsigned long size) { char *eol; From 49b5f81713ef23f37a46e847d69327ad5765cd51 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 5 Oct 2011 17:23:20 -0700 Subject: [PATCH 2/7] commit: teach --gpg-sign option And this uses the gpg-interface.[ch] to allow signing the commit, i.e. $ git commit --gpg-sign -m foo You need a passphrase to unlock the secret key for user: "Junio C Hamano " 4096-bit RSA key, ID 96AFE6CB, created 2011-10-03 (main key ID 713660A7) [master 8457d13] foo 1 files changed, 1 insertions(+), 0 deletions(-) The lines of GPG detached signature are placed in new header lines, after the standard tree/parent/author/committer headers, instead of tucking the signature block at the end of the commit log message text (similar to how signed tag is done), for multiple reasons: - The signature won't clutter output from "git log" and friends if it is in the extra header. If we place it at the end of the log message, we would need to teach "git log" and friends to strip the signature block with an option. - Teaching new versions of "git log" and "gitk" to optionally verify and show signatures is cleaner if we structurally know where the signature block is (instead of scanning in the commit log message). - The signature needs to be stripped upon various commit rewriting operations, e.g. rebase, filter-branch, etc. They all already ignore unknown headers, but if we place signature in the log message, all of these tools (and third-party tools) also need to learn how a signature block would look like. - When we added the optional encoding header, all the tools (both in tree and third-party) that acts on the raw commit object should have been fixed to ignore headers they do not understand, so it is not like that new header would be more likely to break than extra text in the commit. A commit made with the above sample sequence would look like this: $ git cat-file commit HEAD tree 3cd71d90e3db4136e5260ab54599791c4f883b9d parent b87755351a47b09cb27d6913e6e0e17e6254a4d4 author Junio C Hamano 1317862251 -0700 committer Junio C Hamano 1317862251 -0700 gpgsig -----BEGIN PGP SIGNATURE----- gpgsig Version: GnuPG v1.4.10 (GNU/Linux) gpgsig gpgsig iQIcBAABAgAGBQJOjPtrAAoJELC16IaWr+bL4TMP/RSe2Y/jYnCkds9unO5JEnfG gpgsig ... gpgsig =dt98 gpgsig -----END PGP SIGNATURE----- foo but "git log" (unless you ask for it with --pretty=raw) output is not cluttered with the signature information. Signed-off-by: Junio C Hamano --- builtin/commit-tree.c | 24 +++++++++++++++++++++--- builtin/commit.c | 12 ++++++++++-- builtin/merge.c | 16 ++++++++++++++-- commit.c | 40 +++++++++++++++++++++++++++++++++++++++- commit.h | 2 +- notes-cache.c | 2 +- notes-merge.c | 2 +- 7 files changed, 87 insertions(+), 11 deletions(-) diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c index d083795e26..a17811f958 100644 --- a/builtin/commit-tree.c +++ b/builtin/commit-tree.c @@ -8,8 +8,9 @@ #include "tree.h" #include "builtin.h" #include "utf8.h" +#include "gpg-interface.h" -static const char commit_tree_usage[] = "git commit-tree [(-p )...] < changelog"; +static const char commit_tree_usage[] = "git commit-tree [-S] [(-p )...] < changelog"; static void new_parent(struct commit *parent, struct commit_list **parents_p) { @@ -25,6 +26,14 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p) commit_list_insert(parent, parents_p); } +static int commit_tree_config(const char *var, const char *value, void *cb) +{ + int status = git_gpg_config(var, value, NULL); + if (status) + return status; + return git_default_config(var, value, cb); +} + int cmd_commit_tree(int argc, const char **argv, const char *prefix) { int i; @@ -32,11 +41,19 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix) unsigned char tree_sha1[20]; unsigned char commit_sha1[20]; struct strbuf buffer = STRBUF_INIT; + const char *sign_commit = NULL; - git_config(git_default_config, NULL); + git_config(commit_tree_config, NULL); if (argc < 2 || !strcmp(argv[1], "-h")) usage(commit_tree_usage); + + if (!memcmp(argv[1], "-S", 2)) { + sign_commit = argv[1] + 2; + argv++; + argc--; + } + if (get_sha1(argv[1], tree_sha1)) die("Not a valid object name %s", argv[1]); @@ -56,7 +73,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix) if (strbuf_read(&buffer, 0, 0) < 0) die_errno("git commit-tree: failed to read"); - if (commit_tree(buffer.buf, tree_sha1, parents, commit_sha1, NULL)) { + if (commit_tree(buffer.buf, tree_sha1, parents, commit_sha1, + NULL, sign_commit)) { strbuf_release(&buffer); return 1; } diff --git a/builtin/commit.c b/builtin/commit.c index cbc9613ec6..90cf7e812b 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -26,6 +26,7 @@ #include "unpack-trees.h" #include "quote.h" #include "submodule.h" +#include "gpg-interface.h" static const char * const builtin_commit_usage[] = { "git commit [options] [--] ...", @@ -85,6 +86,8 @@ static int all, edit_flag, also, interactive, patch_interactive, only, amend, si static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship; static int no_post_rewrite, allow_empty_message; static char *untracked_files_arg, *force_date, *ignore_submodule_arg; +static char *sign_commit; + /* * The default commit message cleanup mode will remove the lines * beginning with # (shell comments) and leading and trailing @@ -144,6 +147,8 @@ static struct option builtin_commit_options[] = { OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"), OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"), OPT_BOOLEAN(0, "status", &include_status, "include status in commit message template"), + { OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id", + "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, /* end commit message options */ OPT_GROUP("Commit contents options"), @@ -1323,6 +1328,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1, static int git_commit_config(const char *k, const char *v, void *cb) { struct wt_status *s = cb; + int status; if (!strcmp(k, "commit.template")) return git_config_pathname(&template_file, k, v); @@ -1330,7 +1336,9 @@ static int git_commit_config(const char *k, const char *v, void *cb) include_status = git_config_bool(k, v); return 0; } - + status = git_gpg_config(k, v, NULL); + if (status) + return status; return git_status_config(k, v, s); } @@ -1481,7 +1489,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) } if (commit_tree(sb.buf, active_cache_tree->sha1, parents, sha1, - author_ident.buf)) { + author_ident.buf, sign_commit)) { rollback_index_files(); die(_("failed to write commit object")); } diff --git a/builtin/merge.c b/builtin/merge.c index ab4077f272..53cff0266c 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -26,6 +26,7 @@ #include "merge-recursive.h" #include "resolve-undo.h" #include "remote.h" +#include "gpg-interface.h" #define DEFAULT_TWOHEAD (1<<0) #define DEFAULT_OCTOPUS (1<<1) @@ -63,6 +64,7 @@ static int allow_rerere_auto; static int abort_current_merge; static int show_progress = -1; static int default_to_upstream; +static const char *sign_commit; static struct strategy all_strategy[] = { { "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL }, @@ -206,6 +208,8 @@ static struct option builtin_merge_options[] = { OPT_BOOLEAN(0, "abort", &abort_current_merge, "abort the current in-progress merge"), OPT_SET_INT(0, "progress", &show_progress, "force progress reporting", 1), + { OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id", + "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, OPT_END() }; @@ -525,6 +529,8 @@ static void parse_branch_merge_options(char *bmo) static int git_merge_config(const char *k, const char *v, void *cb) { + int status; + if (branch && !prefixcmp(k, "branch.") && !prefixcmp(k + 7, branch) && !strcmp(k + 7 + strlen(branch), ".mergeoptions")) { @@ -562,6 +568,10 @@ static int git_merge_config(const char *k, const char *v, void *cb) default_to_upstream = git_config_bool(k, v); return 0; } + + status = git_gpg_config(k, v, NULL); + if (status) + return status; return git_diff_ui_config(k, v, cb); } @@ -870,7 +880,8 @@ static int merge_trivial(void) parent->next->item = remoteheads->item; parent->next->next = NULL; run_prepare_commit_msg(); - commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL); + commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL, + sign_commit); finish(result_commit, "In-index merge"); drop_save(); return 0; @@ -900,7 +911,8 @@ static int finish_automerge(struct commit_list *common, free_commit_list(remoteheads); strbuf_addch(&merge_msg, '\n'); run_prepare_commit_msg(); - commit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL); + commit_tree(merge_msg.buf, result_tree, parents, result_commit, + NULL, sign_commit); strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy); finish(result_commit, buf.buf); strbuf_release(&buf); diff --git a/commit.c b/commit.c index 97b43279cd..4bff3cdaae 100644 --- a/commit.c +++ b/commit.c @@ -6,6 +6,7 @@ #include "diff.h" #include "revision.h" #include "notes.h" +#include "gpg-interface.h" int save_commit_buffer = 1; @@ -814,6 +815,40 @@ struct commit_list *reduce_heads(struct commit_list *heads) return result; } +static const char gpg_sig_header[] = "gpgsig "; +static const int gpg_sig_header_len = sizeof(gpg_sig_header) - 1; + +static int do_sign_commit(struct strbuf *buf, const char *keyid) +{ + struct strbuf sig = STRBUF_INIT; + int inspos, copypos; + + /* find the end of the header */ + inspos = strstr(buf->buf, "\n\n") - buf->buf + 1; + + if (!keyid || !*keyid) + keyid = get_signing_key(); + if (sign_buffer(buf, &sig, keyid)) { + strbuf_release(&sig); + return -1; + } + + for (copypos = 0; sig.buf[copypos]; ) { + const char *bol = sig.buf + copypos; + const char *eol = strchrnul(bol, '\n'); + int len = (eol - bol) + !!*eol; + + strbuf_insert(buf, inspos, gpg_sig_header, gpg_sig_header_len); + inspos += gpg_sig_header_len; + strbuf_insert(buf, inspos, bol, len); + inspos += len; + copypos += len; + } + strbuf_release(&sig); + return 0; +} + + static const char commit_utf8_warn[] = "Warning: commit message does not conform to UTF-8.\n" "You may want to amend it after fixing the message, or set the config\n" @@ -821,7 +856,7 @@ static const char commit_utf8_warn[] = int commit_tree(const char *msg, unsigned char *tree, struct commit_list *parents, unsigned char *ret, - const char *author) + const char *author, const char *sign_commit) { int result; int encoding_is_utf8; @@ -864,6 +899,9 @@ int commit_tree(const char *msg, unsigned char *tree, if (encoding_is_utf8 && !is_utf8(buffer.buf)) fprintf(stderr, commit_utf8_warn); + if (sign_commit && do_sign_commit(&buffer, sign_commit)) + return -1; + result = write_sha1_file(buffer.buf, buffer.len, commit_type, ret); strbuf_release(&buffer); return result; diff --git a/commit.h b/commit.h index 12d100b8b6..8c2419b51f 100644 --- a/commit.h +++ b/commit.h @@ -175,6 +175,6 @@ struct commit_list *reduce_heads(struct commit_list *heads); extern int commit_tree(const char *msg, unsigned char *tree, struct commit_list *parents, unsigned char *ret, - const char *author); + const char *author, const char *sign_commit); #endif /* COMMIT_H */ diff --git a/notes-cache.c b/notes-cache.c index 4c8984ede1..c36a960bc3 100644 --- a/notes-cache.c +++ b/notes-cache.c @@ -56,7 +56,7 @@ int notes_cache_write(struct notes_cache *c) if (write_notes_tree(&c->tree, tree_sha1)) return -1; - if (commit_tree(c->validity, tree_sha1, NULL, commit_sha1, NULL) < 0) + if (commit_tree(c->validity, tree_sha1, NULL, commit_sha1, NULL, NULL) < 0) return -1; if (update_ref("update notes cache", c->tree.ref, commit_sha1, NULL, 0, QUIET_ON_ERR) < 0) diff --git a/notes-merge.c b/notes-merge.c index e1aaf43b43..c29c434156 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -546,7 +546,7 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents, /* else: t->ref points to nothing, assume root/orphan commit */ } - if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL)) + if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL, NULL)) die("Failed to commit notes tree to database"); } From 93104a2742d01adf97f173bda63c12382c77aae2 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 18 Oct 2011 15:53:23 -0700 Subject: [PATCH 3/7] log: --show-signature This teaches the "log" family of commands to pass the GPG signature in the commit objects to "gpg --verify" via the verify_signed_buffer() interface used to verify signed tag objects. E.g. $ git show --show-signature -s HEAD shows GPG output in the header part of the output. Signed-off-by: Junio C Hamano --- commit.c | 34 ++++++++++++++++++++++++++++++++++ commit.h | 3 +++ log-tree.c | 39 +++++++++++++++++++++++++++++++++++++++ revision.c | 2 ++ revision.h | 1 + 5 files changed, 79 insertions(+) diff --git a/commit.c b/commit.c index 4bff3cdaae..93045a2cda 100644 --- a/commit.c +++ b/commit.c @@ -848,6 +848,40 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid) return 0; } +int parse_signed_commit(const unsigned char *sha1, + struct strbuf *payload, struct strbuf *signature) +{ + unsigned long size; + enum object_type type; + char *buffer = read_sha1_file(sha1, &type, &size); + int in_header, saw_signature = -1; + char *line; + + if (!buffer || type != OBJ_COMMIT) + goto cleanup; + + line = buffer; + in_header = 1; + saw_signature = 0; + while (*line) { + char *next = strchrnul(line, '\n'); + if (*next) + next++; + if (in_header && !prefixcmp(line, gpg_sig_header)) { + const char *sig = line + gpg_sig_header_len; + strbuf_add(signature, sig, next - sig); + saw_signature = 1; + } else { + strbuf_add(payload, line, next - line); + } + if (*line == '\n') + in_header = 0; + line = next; + } + cleanup: + free(buffer); + return saw_signature; +} static const char commit_utf8_warn[] = "Warning: commit message does not conform to UTF-8.\n" diff --git a/commit.h b/commit.h index 8c2419b51f..1885471b8b 100644 --- a/commit.h +++ b/commit.h @@ -177,4 +177,7 @@ extern int commit_tree(const char *msg, unsigned char *tree, struct commit_list *parents, unsigned char *ret, const char *author, const char *sign_commit); +extern int parse_signed_commit(const unsigned char *sha1, + struct strbuf *message, struct strbuf *signature); + #endif /* COMMIT_H */ diff --git a/log-tree.c b/log-tree.c index 24c295ea1d..f7b6976d9f 100644 --- a/log-tree.c +++ b/log-tree.c @@ -8,6 +8,7 @@ #include "refs.h" #include "string-list.h" #include "color.h" +#include "gpg-interface.h" struct decoration name_decoration = { "object names" }; @@ -395,6 +396,41 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit, *extra_headers_p = extra_headers; } +static void show_signature(struct rev_info *opt, struct commit *commit) +{ + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; + struct strbuf gpg_output = STRBUF_INIT; + int status; + const char *color, *reset, *bol, *eol; + + if (parse_signed_commit(commit->object.sha1, &payload, &signature) <= 0) + goto out; + + status = verify_signed_buffer(payload.buf, payload.len, + signature.buf, signature.len, + &gpg_output); + if (status && !gpg_output.len) + strbuf_addstr(&gpg_output, "No signature\n"); + + color = diff_get_color_opt(&opt->diffopt, + status ? DIFF_WHITESPACE : DIFF_FRAGINFO); + reset = diff_get_color_opt(&opt->diffopt, DIFF_RESET); + + bol = gpg_output.buf; + while (*bol) { + eol = strchrnul(bol, '\n'); + printf("%s%.*s%s%s", color, (int)(eol - bol), bol, reset, + *eol ? "\n" : ""); + bol = (*eol) ? (eol + 1) : eol; + } + + out: + strbuf_release(&gpg_output); + strbuf_release(&payload); + strbuf_release(&signature); +} + void show_log(struct rev_info *opt) { struct strbuf msgbuf = STRBUF_INIT; @@ -502,6 +538,9 @@ void show_log(struct rev_info *opt) } } + if (opt->show_signature) + show_signature(opt, commit); + if (!commit->buffer) return; diff --git a/revision.c b/revision.c index c46cfaa3e4..860a3128a3 100644 --- a/revision.c +++ b/revision.c @@ -1381,6 +1381,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs->show_notes = 1; revs->show_notes_given = 1; revs->notes_opt.use_default_notes = 1; + } else if (!strcmp(arg, "--show-signature")) { + revs->show_signature = 1; } else if (!prefixcmp(arg, "--show-notes=") || !prefixcmp(arg, "--notes=")) { struct strbuf buf = STRBUF_INIT; diff --git a/revision.h b/revision.h index 3d64adad18..198bb95894 100644 --- a/revision.h +++ b/revision.h @@ -89,6 +89,7 @@ struct rev_info { show_merge:1, show_notes:1, show_notes_given:1, + show_signature:1, pretty_given:1, abbrev_commit:1, abbrev_commit_given:1, From 009012833cea5a2e4b78529d1012e900664ccd0a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 19 Oct 2011 16:45:05 -0700 Subject: [PATCH 4/7] t7004: extract generic "GPG testing" bits Signed-off-by: Junio C Hamano --- t/lib-gpg.sh | 29 +++++++++++++++++++++++++++++ t/{t7004 => lib-gpg}/pubring.gpg | Bin t/{t7004 => lib-gpg}/random_seed | Bin t/{t7004 => lib-gpg}/secring.gpg | Bin t/{t7004 => lib-gpg}/trustdb.gpg | Bin t/t7004-tag.sh | 29 +---------------------------- 6 files changed, 30 insertions(+), 28 deletions(-) create mode 100644 t/lib-gpg.sh rename t/{t7004 => lib-gpg}/pubring.gpg (100%) rename t/{t7004 => lib-gpg}/random_seed (100%) rename t/{t7004 => lib-gpg}/secring.gpg (100%) rename t/{t7004 => lib-gpg}/trustdb.gpg (100%) diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh new file mode 100644 index 0000000000..eb09027175 --- /dev/null +++ b/t/lib-gpg.sh @@ -0,0 +1,29 @@ +#!/bin/sh + +# Check if gpg is available +gpg --version >/dev/null 2>/dev/null +if [ $? -eq 127 ]; then + say "# gpg not found - skipping tag signing and verification tests" +else + # As said here: http://www.gnupg.org/documentation/faqs.html#q6.19 + # the gpg version 1.0.6 didn't parse trust packets correctly, so for + # that version, creation of signed tags using the generated key fails. + case "$(gpg --version)" in + 'gpg (GnuPG) 1.0.6'*) + say "Skipping signed tag tests, because a bug in 1.0.6 version" + ;; + *) + test_set_prereq GPG + ;; + esac +fi + +# key generation info: gpg --homedir t/t7004 --gen-key +# Type DSA and Elgamal, size 2048 bits, no expiration date. +# Name and email: C O Mitter +# No password given, to enable non-interactive operation. + +cp -R "$TEST_DIRECTORY"/lib-gpg ./gpghome +chmod 0700 gpghome +GNUPGHOME="$(pwd)/gpghome" +export GNUPGHOME diff --git a/t/t7004/pubring.gpg b/t/lib-gpg/pubring.gpg similarity index 100% rename from t/t7004/pubring.gpg rename to t/lib-gpg/pubring.gpg diff --git a/t/t7004/random_seed b/t/lib-gpg/random_seed similarity index 100% rename from t/t7004/random_seed rename to t/lib-gpg/random_seed diff --git a/t/t7004/secring.gpg b/t/lib-gpg/secring.gpg similarity index 100% rename from t/t7004/secring.gpg rename to t/lib-gpg/secring.gpg diff --git a/t/t7004/trustdb.gpg b/t/lib-gpg/trustdb.gpg similarity index 100% rename from t/t7004/trustdb.gpg rename to t/lib-gpg/trustdb.gpg diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 097ce2bc83..ded5c86c30 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -8,6 +8,7 @@ test_description='git tag Tests for operations with tags.' . ./test-lib.sh +. "$TEST_DIRECTORY/lib-gpg.sh" # creating and listing lightweight tags: @@ -585,24 +586,6 @@ test_expect_success \ test_cmp expect actual ' -# subsequent tests require gpg; check if it is available -gpg --version >/dev/null 2>/dev/null -if [ $? -eq 127 ]; then - say "# gpg not found - skipping tag signing and verification tests" -else - # As said here: http://www.gnupg.org/documentation/faqs.html#q6.19 - # the gpg version 1.0.6 didn't parse trust packets correctly, so for - # that version, creation of signed tags using the generated key fails. - case "$(gpg --version)" in - 'gpg (GnuPG) 1.0.6'*) - say "Skipping signed tag tests, because a bug in 1.0.6 version" - ;; - *) - test_set_prereq GPG - ;; - esac -fi - # trying to verify annotated non-signed tags: test_expect_success GPG \ @@ -625,16 +608,6 @@ test_expect_success GPG \ # creating and verifying signed tags: -# key generation info: gpg --homedir t/t7004 --gen-key -# Type DSA and Elgamal, size 2048 bits, no expiration date. -# Name and email: C O Mitter -# No password given, to enable non-interactive operation. - -cp -R "$TEST_DIRECTORY"/t7004 ./gpghome -chmod 0700 gpghome -GNUPGHOME="$(pwd)/gpghome" -export GNUPGHOME - get_tag_header signed-tag $commit commit $time >expect echo 'A signed tag message' >>expect echo '-----BEGIN PGP SIGNATURE-----' >>expect From d542c72a8e7950dd943be51fc6e509ed37fbb4a2 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 19 Oct 2011 17:15:05 -0700 Subject: [PATCH 5/7] test "commit -S" and "log --show-signature" Signed-off-by: Junio C Hamano --- t/t7510-signed-commit.sh | 60 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100755 t/t7510-signed-commit.sh diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh new file mode 100755 index 0000000000..5c7475d818 --- /dev/null +++ b/t/t7510-signed-commit.sh @@ -0,0 +1,60 @@ +#!/bin/sh + +test_description='signed commit tests' +. ./test-lib.sh +. "$TEST_DIRECTORY/lib-gpg.sh" + +test_expect_success GPG 'create signed commits' ' + echo 1 >file && git add file && + test_tick && git commit -S -m initial && + git tag initial && + git branch side && + + echo 2 >file && test_tick && git commit -a -S -m second && + git tag second && + + git checkout side && + echo 3 >elif && git add elif && + test_tick && git commit -m "third on side" && + + git checkout master && + test_tick && git merge -S side && + git tag merge && + + echo 4 >file && test_tick && git commit -a -m "fourth unsigned" && + git tag fourth-unsigned && + + test_tick && git commit --amend -S -m "fourth signed" +' + +test_expect_success GPG 'show signatures' ' + ( + for commit in initial second merge master + do + git show --pretty=short --show-signature $commit >actual && + grep "Good signature from" actual || exit 1 + ! grep "BAD signature from" actual || exit 1 + echo $commit OK + done + ) && + ( + for commit in merge^2 fourth-unsigned + do + git show --pretty=short --show-signature $commit >actual && + grep "Good signature from" actual && exit 1 + ! grep "BAD signature from" actual || exit 1 + echo $commit OK + done + ) +' + +test_expect_success GPG 'detect fudged signature' ' + git cat-file commit master >raw && + sed -e "s/fourth signed/4th forged/" raw >forged && + git hash-object -w -t commit forged >forged.commit && + git show --pretty=short --show-signature $(cat forged.commit) >actual && + grep "BAD signature from" actual && + ! grep "Good signature from" actual +' + +test_done From 33a11a20eb7610771268e34211509cbbdee76b1e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 20 Oct 2011 11:37:59 -0700 Subject: [PATCH 6/7] parse_signed_commit: really use the entire commit log message ... even beyond the first NUL in the buffer, when checking the commit against the detached signature in the header. Signed-off-by: Junio C Hamano --- commit.c | 21 ++++++++++++--------- t/t7510-signed-commit.sh | 21 ++++++++++++++++----- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/commit.c b/commit.c index 93045a2cda..09693f78d9 100644 --- a/commit.c +++ b/commit.c @@ -854,28 +854,31 @@ int parse_signed_commit(const unsigned char *sha1, unsigned long size; enum object_type type; char *buffer = read_sha1_file(sha1, &type, &size); - int in_header, saw_signature = -1; - char *line; + int saw_signature = -1; + char *line, *tail; if (!buffer || type != OBJ_COMMIT) goto cleanup; line = buffer; - in_header = 1; + tail = buffer + size; saw_signature = 0; - while (*line) { - char *next = strchrnul(line, '\n'); - if (*next) + while (line < tail) { + char *next = memchr(line, '\n', tail - line); + if (!next) + next = tail; + else next++; - if (in_header && !prefixcmp(line, gpg_sig_header)) { + if (!prefixcmp(line, gpg_sig_header)) { const char *sig = line + gpg_sig_header_len; strbuf_add(signature, sig, next - sig); saw_signature = 1; } else { + if (*line == '\n') + /* dump the whole remainder of the buffer */ + next = tail; strbuf_add(payload, line, next - line); } - if (*line == '\n') - in_header = 0; line = next; } cleanup: diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 5c7475d818..30401ced07 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -50,11 +50,22 @@ test_expect_success GPG 'show signatures' ' test_expect_success GPG 'detect fudged signature' ' git cat-file commit master >raw && - sed -e "s/fourth signed/4th forged/" raw >forged && - git hash-object -w -t commit forged >forged.commit && - git show --pretty=short --show-signature $(cat forged.commit) >actual && - grep "BAD signature from" actual && - ! grep "Good signature from" actual + + sed -e "s/fourth signed/4th forged/" raw >forged1 && + git hash-object -w -t commit forged1 >forged1.commit && + git show --pretty=short --show-signature $(cat forged1.commit) >actual1 && + grep "BAD signature from" actual1 && + ! grep "Good signature from" actual1 +' + +test_expect_success GPG 'detect fudged signature with NUL' ' + git cat-file commit master >raw && + cat raw >forged2 && + echo Qwik | tr "Q" "\000" >>forged2 && + git hash-object -w -t commit forged2 >forged2.commit && + git show --pretty=short --show-signature $(cat forged2.commit) >actual2 && + grep "BAD signature from" actual2 && + ! grep "Good signature from" actual2 ' test_done From 5f549aa2f78314ac37bbd436c8f80aea4c752e07 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 21 Oct 2011 21:06:02 -0700 Subject: [PATCH 7/7] pretty: %G[?GS] placeholders Add new placeholders related to the GPG signature on signed commits. - %GG to show the raw verification message from GPG; - %G? to show either "G" for Good, "B" for Bad; - %GS to show the name of the signer. Signed-off-by: Junio C Hamano --- pretty.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/pretty.c b/pretty.c index f45eb54e4c..392d656595 100644 --- a/pretty.c +++ b/pretty.c @@ -9,6 +9,7 @@ #include "notes.h" #include "color.h" #include "reflog-walk.h" +#include "gpg-interface.h" static char *user_format; static struct cmt_fmt_map { @@ -640,6 +641,12 @@ struct format_commit_context { const struct pretty_print_context *pretty_ctx; unsigned commit_header_parsed:1; unsigned commit_message_parsed:1; + unsigned commit_signature_parsed:1; + struct { + char *gpg_output; + char good_bad; + char *signer; + } signature; char *message; size_t width, indent1, indent2; @@ -822,6 +829,59 @@ static void rewrap_message_tail(struct strbuf *sb, c->indent2 = new_indent2; } +static struct { + char result; + const char *check; +} signature_check[] = { + { 'G', ": Good signature from " }, + { 'B', ": BAD signature from " }, +}; + +static void parse_signature_lines(struct format_commit_context *ctx) +{ + const char *buf = ctx->signature.gpg_output; + int i; + + for (i = 0; i < ARRAY_SIZE(signature_check); i++) { + const char *found = strstr(buf, signature_check[i].check); + const char *next; + if (!found) + continue; + ctx->signature.good_bad = signature_check[i].result; + found += strlen(signature_check[i].check); + next = strchrnul(found, '\n'); + ctx->signature.signer = xmemdupz(found, next - found); + break; + } +} + +static void parse_commit_signature(struct format_commit_context *ctx) +{ + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; + struct strbuf gpg_output = STRBUF_INIT; + int status; + + ctx->commit_signature_parsed = 1; + + if (parse_signed_commit(ctx->commit->object.sha1, + &payload, &signature) <= 0) + goto out; + status = verify_signed_buffer(payload.buf, payload.len, + signature.buf, signature.len, + &gpg_output); + if (status && !gpg_output.len) + goto out; + ctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL); + parse_signature_lines(ctx); + + out: + strbuf_release(&gpg_output); + strbuf_release(&payload); + strbuf_release(&signature); +} + + static size_t format_commit_one(struct strbuf *sb, const char *placeholder, void *context) { @@ -974,6 +1034,30 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder, return 0; } + if (placeholder[0] == 'G') { + if (!c->commit_signature_parsed) + parse_commit_signature(c); + switch (placeholder[1]) { + case 'G': + if (c->signature.gpg_output) + strbuf_addstr(sb, c->signature.gpg_output); + break; + case '?': + switch (c->signature.good_bad) { + case 'G': + case 'B': + strbuf_addch(sb, c->signature.good_bad); + } + break; + case 'S': + if (c->signature.signer) + strbuf_addstr(sb, c->signature.signer); + break; + } + return 2; + } + + /* For the rest we have to parse the commit header. */ if (!c->commit_header_parsed) parse_commit_header(c); @@ -1114,6 +1198,8 @@ void format_commit_message(const struct commit *commit, if (context.message != commit->buffer) free(context.message); + free(context.signature.gpg_output); + free(context.signature.signer); } static void pp_header(const struct pretty_print_context *pp,