From d281b45d754477e79a8e0228c8f5ce4195079238 Mon Sep 17 00:00:00 2001 From: Santiago Torres Date: Tue, 5 Apr 2016 12:07:24 -0400 Subject: [PATCH 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface The verify_signed_buffer() function may trigger a SIGPIPE when the GPG child process terminates early (due to a bad keyid, for example) and Git tries to write to it afterwards. Previously, ignoring SIGPIPE was done in builtin/verify-tag.c to avoid this issue. However, any other caller who wants to call verify_signed_buffer() would have to do the same. Use sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(), pretty much like in sign_buffer(), so that any caller is not required to perform this task. This will avoid possible mistakes by further developers using verify_signed_buffer(). Signed-off-by: Santiago Torres Reviewed-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/verify-tag.c | 3 --- gpg-interface.c | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 00663f6a30..77f070a02a 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; - /* sometimes the program was terminated because this signal - * was received in the process of writing the gpg input: */ - signal(SIGPIPE, SIG_IGN); while (i < argc) if (verify_tag(argv[i++], flags)) had_error = 1; diff --git a/gpg-interface.c b/gpg-interface.c index 3dc2fe397e..2259938236 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size, return error(_("could not run gpg.")); } + sigchain_push(SIGPIPE, SIG_IGN); write_in_full(gpg.in, payload, payload_size); close(gpg.in); @@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size, close(gpg.out); ret = finish_command(&gpg); + sigchain_pop(SIGPIPE); unlink_or_warn(path); From 3e1e7454cc6737ceff0311451eba7a698da9d8c9 Mon Sep 17 00:00:00 2001 From: Santiago Torres Date: Sun, 17 Apr 2016 18:26:57 -0400 Subject: [PATCH 2/6] t7030: test verifying multiple tags The verify-tag command supports multiple tag names to verify, but existing tests only test for invocation with a single tag. Add a test invoking it with multiple tags. Helped-by: Jeff King Signed-off-by: Santiago Torres Reviewed-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t7030-verify-tag.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 4608e71343..07079a41c4 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -112,4 +112,17 @@ test_expect_success GPG 'verify signatures with --raw' ' ) ' +test_expect_success GPG 'verify multiple tags' ' + tags="fourth-signed sixth-signed seventh-signed" && + for i in $tags + do + git verify-tag -v --raw $i || return 1 + done >expect.stdout 2>expect.stderr.1 && + grep "^.GNUPG:." expect.stderr && + git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 && + grep "^.GNUPG:." actual.stderr && + test_cmp expect.stdout actual.stdout && + test_cmp expect.stderr actual.stderr +' + test_done From 20972f54d3f17007904cacb829c1feefb84a68dd Mon Sep 17 00:00:00 2001 From: Santiago Torres Date: Tue, 19 Apr 2016 13:47:18 -0400 Subject: [PATCH 3/6] verify-tag: update variable name and type The run_gpg_verify() function has two variables, size and len. This may come off as confusing when reading the code. Clarify which one pertains to the length of the tag headers by renaming len to payload_size. Additionally, change the type of payload_size to size_t to match the return type of parse_signature. Signed-off-by: Santiago Torres Reviewed-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/verify-tag.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 77f070a02a..fa26e407c8 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = { static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) { struct signature_check sigc; - int len; + size_t payload_size; int ret; memset(&sigc, 0, sizeof(sigc)); - len = parse_signature(buf, size); + payload_size = parse_signature(buf, size); - if (size == len) { + if (size == payload_size) { if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, len); + write_in_full(1, buf, payload_size); return error("no signature found"); } - ret = check_signature(buf, len, buf + len, size - len, &sigc); + ret = check_signature(buf, payload_size, buf + payload_size, + size - payload_size, &sigc); print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); From 78ccd4419525562a2c9d2b7cebddba0914bde151 Mon Sep 17 00:00:00 2001 From: Santiago Torres Date: Tue, 19 Apr 2016 13:47:19 -0400 Subject: [PATCH 4/6] verify-tag: prepare verify_tag for libification The current interface of verify_tag() resolves reference names to SHA1, however, the plan is to make this functionality public and the current interface is cumbersome for callers: they are expected to supply the textual representation of a sha1/refname. In many cases, this requires them to turn the sha1 to hex representation, just to be converted back inside verify_tag. Add a SHA1 parameter to use instead of the name parameter, and rename the name parameter to "name_to_report" for reporting purposes only. Helped-by: Junio C Hamano Signed-off-by: Santiago Torres Reviewed-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/verify-tag.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index fa26e407c8..a3d3a43826 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -42,25 +42,28 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -static int verify_tag(const char *name, unsigned flags) +static int verify_tag(const unsigned char *sha1, const char *name_to_report, + unsigned flags) { enum object_type type; - unsigned char sha1[20]; char *buf; unsigned long size; int ret; - if (get_sha1(name, sha1)) - return error("tag '%s' not found.", name); - type = sha1_object_info(sha1, NULL); if (type != OBJ_TAG) return error("%s: cannot verify a non-tag object of type %s.", - name, typename(type)); + name_to_report ? + name_to_report : + find_unique_abbrev(sha1, DEFAULT_ABBREV), + typename(type)); buf = read_sha1_file(sha1, &type, &size); if (!buf) - return error("%s: unable to read file.", name); + return error("%s: unable to read file.", + name_to_report ? + name_to_report : + find_unique_abbrev(sha1, DEFAULT_ABBREV)); ret = run_gpg_verify(buf, size, flags); @@ -96,8 +99,13 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; - while (i < argc) - if (verify_tag(argv[i++], flags)) + while (i < argc) { + unsigned char sha1[20]; + const char *name = argv[i++]; + if (get_sha1(name, sha1)) + had_error = !!error("tag '%s' not found.", name); + else if (verify_tag(sha1, name, flags)) had_error = 1; + } return had_error; } From 45a227ef769df9fc0d198f323d2f919aa74375e1 Mon Sep 17 00:00:00 2001 From: Santiago Torres Date: Fri, 22 Apr 2016 10:52:04 -0400 Subject: [PATCH 5/6] verify-tag: move tag verification code to tag.c The PGP verification routine for tags could be accessed by other modules that require to do so. Publish the verify_tag function in tag.c and rename it to gpg_verify_tag so it does not conflict with builtin/mktag's static function. Helped-by: Junio C Hamano Signed-off-by: Santiago Torres Reviewed-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/verify-tag.c | 55 +------------------------------------------- tag.c | 53 ++++++++++++++++++++++++++++++++++++++++++ tag.h | 2 ++ 3 files changed, 56 insertions(+), 54 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index a3d3a43826..99f8148cf7 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -18,59 +18,6 @@ static const char * const verify_tag_usage[] = { NULL }; -static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) -{ - struct signature_check sigc; - size_t payload_size; - int ret; - - memset(&sigc, 0, sizeof(sigc)); - - payload_size = parse_signature(buf, size); - - if (size == payload_size) { - if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, payload_size); - return error("no signature found"); - } - - ret = check_signature(buf, payload_size, buf + payload_size, - size - payload_size, &sigc); - print_signature_buffer(&sigc, flags); - - signature_check_clear(&sigc); - return ret; -} - -static int verify_tag(const unsigned char *sha1, const char *name_to_report, - unsigned flags) -{ - enum object_type type; - char *buf; - unsigned long size; - int ret; - - type = sha1_object_info(sha1, NULL); - if (type != OBJ_TAG) - return error("%s: cannot verify a non-tag object of type %s.", - name_to_report ? - name_to_report : - find_unique_abbrev(sha1, DEFAULT_ABBREV), - typename(type)); - - buf = read_sha1_file(sha1, &type, &size); - if (!buf) - return error("%s: unable to read file.", - name_to_report ? - name_to_report : - find_unique_abbrev(sha1, DEFAULT_ABBREV)); - - ret = run_gpg_verify(buf, size, flags); - - free(buf); - return ret; -} - static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -104,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) const char *name = argv[i++]; if (get_sha1(name, sha1)) had_error = !!error("tag '%s' not found.", name); - else if (verify_tag(sha1, name, flags)) + else if (gpg_verify_tag(sha1, name, flags)) had_error = 1; } return had_error; diff --git a/tag.c b/tag.c index d72f742af9..d1dcd18cd7 100644 --- a/tag.c +++ b/tag.c @@ -6,6 +6,59 @@ const char *tag_type = "tag"; +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) +{ + struct signature_check sigc; + size_t payload_size; + int ret; + + memset(&sigc, 0, sizeof(sigc)); + + payload_size = parse_signature(buf, size); + + if (size == payload_size) { + if (flags & GPG_VERIFY_VERBOSE) + write_in_full(1, buf, payload_size); + return error("no signature found"); + } + + ret = check_signature(buf, payload_size, buf + payload_size, + size - payload_size, &sigc); + print_signature_buffer(&sigc, flags); + + signature_check_clear(&sigc); + return ret; +} + +int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, + unsigned flags) +{ + enum object_type type; + char *buf; + unsigned long size; + int ret; + + type = sha1_object_info(sha1, NULL); + if (type != OBJ_TAG) + return error("%s: cannot verify a non-tag object of type %s.", + name_to_report ? + name_to_report : + find_unique_abbrev(sha1, DEFAULT_ABBREV), + typename(type)); + + buf = read_sha1_file(sha1, &type, &size); + if (!buf) + return error("%s: unable to read file.", + name_to_report ? + name_to_report : + find_unique_abbrev(sha1, DEFAULT_ABBREV)); + + ret = run_gpg_verify(buf, size, flags); + + free(buf); + return ret; +} + struct object *deref_tag(struct object *o, const char *warn, int warnlen) { while (o && o->type == OBJ_TAG) diff --git a/tag.h b/tag.h index f4580aea42..a5721b6731 100644 --- a/tag.h +++ b/tag.h @@ -17,5 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si extern int parse_tag(struct tag *item); extern struct object *deref_tag(struct object *, const char *, int); extern struct object *deref_tag_noverify(struct object *); +extern int gpg_verify_tag(const unsigned char *sha1, + const char *name_to_report, unsigned flags); #endif /* TAG_H */ From bef234b09e5c3b2f63429fb49aff983f6f617f22 Mon Sep 17 00:00:00 2001 From: Santiago Torres Date: Fri, 22 Apr 2016 10:52:05 -0400 Subject: [PATCH 6/6] tag -v: verify directly rather than exec-ing verify-tag Instead of having tag -v fork to run verify-tag, use the gpg_verify_tag() function directly. Helped-by: Eric Sunshine Signed-off-by: Santiago Torres Reviewed-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/tag.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94665..7b2918ef38 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -104,13 +104,7 @@ static int delete_tag(const char *name, const char *ref, static int verify_tag(const char *name, const char *ref, const unsigned char *sha1) { - const char *argv_verify_tag[] = {"verify-tag", - "-v", "SHA1_HEX", NULL}; - argv_verify_tag[2] = sha1_to_hex(sha1); - - if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD)) - return error(_("could not verify the tag '%s'"), name); - return 0; + return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer)