[PATCH v5 2/7] ref-filter: add function to print single ref_array_item
From: Lukas Puehringer ref-filter functions are useful for printing git object information using a format specifier. However, some other modules may not want to use this functionality on a ref-array but only print a single item. Expose a pretty_print_ref function to create, pretty print and free individual ref-items. Signed-off-by: Lukas Puehringer --- ref-filter.c | 27 +-- ref-filter.h | 7 +++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 1a978405e..5f4b08792 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1361,7 +1361,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname, return ref; } -static int filter_ref_kind(struct ref_filter *filter, const char *refname) +static int ref_kind_from_refname(const char *refname) { unsigned int i; @@ -1374,11 +1374,7 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) { "refs/tags/", FILTER_REFS_TAGS} }; - if (filter->kind == FILTER_REFS_BRANCHES || - filter->kind == FILTER_REFS_REMOTES || - filter->kind == FILTER_REFS_TAGS) - return filter->kind; - else if (!strcmp(refname, "HEAD")) + if (!strcmp(refname, "HEAD")) return FILTER_REFS_DETACHED_HEAD; for (i = 0; i < ARRAY_SIZE(ref_kind); i++) { @@ -1389,6 +1385,15 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) return FILTER_REFS_OTHERS; } +static int filter_ref_kind(struct ref_filter *filter, const char *refname) +{ + if (filter->kind == FILTER_REFS_BRANCHES || + filter->kind == FILTER_REFS_REMOTES || + filter->kind == FILTER_REFS_TAGS) + return filter->kind; + return ref_kind_from_refname(refname); +} + /* * A call-back given to for_each_ref(). Filter refs and keep them for * later object processing. @@ -1671,6 +1676,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu putchar('\n'); } +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format) +{ + struct ref_array_item *ref_item; + ref_item = new_ref_array_item(name, sha1, 0); + ref_item->kind = ref_kind_from_refname(name); + show_ref_array_item(ref_item, format, 0); + free_array_item(ref_item); +} + /* If no sorting option is given, use refname to sort as default */ struct ref_sorting *ref_default_sorting(void) { diff --git a/ref-filter.h b/ref-filter.h index fc55fa357..7b05592ba 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -109,4 +109,11 @@ struct ref_sorting *ref_default_sorting(void); /* Function to parse --merged and --no-merged options */ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); +/* + * Print a single ref, outside of any ref-filter. Note that the + * name must be a fully qualified refname. + */ +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format); + #endif /* REF_FILTER_H */ -- 2.11.0
[PATCH v5 5/7] builtin/tag: add --format argument for tag -v
From: Lukas Puehringer Adding --format to git tag -v mutes the default output of the GPG verification and instead prints the formatted tag object. This allows callers to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. The callback function for for_each_tag_name() didn't allow callers to pass custom data to their callback functions. Add a new opaque pointer to each_tag_name_fn's parameter to allow this. Signed-off-by: Lukas Puehringer --- Documentation/git-tag.txt | 2 +- builtin/tag.c | 32 ++-- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 76cfe40d9..586aaa315 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -15,7 +15,7 @@ SYNOPSIS 'git tag' [-n[]] -l [--contains ] [--points-at ] [--column[=] | --no-column] [--create-reflog] [--sort=] [--format=] [--[no-]merged []] [...] -'git tag' -v ... +'git tag' -v [--format=] ... DESCRIPTION --- diff --git a/builtin/tag.c b/builtin/tag.c index 880677df5..9da11e0c2 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = { N_("git tag -d ..."), N_("git tag -l [-n[]] [--contains ] [--points-at ]" "\n\t\t[--format=] [--[no-]merged []] [...]"), - N_("git tag -v ..."), + N_("git tag -v [--format=] ..."), NULL }; @@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con } typedef int (*each_tag_name_fn)(const char *name, const char *ref, - const unsigned char *sha1); + const unsigned char *sha1, void *cb_data); -static int for_each_tag_name(const char **argv, each_tag_name_fn fn) +static int for_each_tag_name(const char **argv, each_tag_name_fn fn, + void *cb_data) { const char **p; char ref[PATH_MAX]; int had_error = 0; unsigned char sha1[20]; + for (p = argv; *p; p++) { if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p) >= sizeof(ref)) { @@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn) had_error = 1; continue; } - if (fn(*p, ref, sha1)) + if (fn(*p, ref, sha1, cb_data)) had_error = 1; } return had_error; } static int delete_tag(const char *name, const char *ref, - const unsigned char *sha1) + const unsigned char *sha1, void *cb_data) { if (delete_ref(ref, sha1, 0)) return 1; @@ -103,9 +105,16 @@ 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 unsigned char *sha1, void *cb_data) { - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); + int flags; + char *fmt_pretty = cb_data; + flags = GPG_VERIFY_VERBOSE; + + if (fmt_pretty) + flags = GPG_VERIFY_QUIET; + + return verify_and_format_tag(sha1, ref, fmt_pretty, flags); } static int do_sign(struct strbuf *buffer) @@ -428,9 +437,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (filter.merge_commit) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') - return for_each_tag_name(argv, delete_tag); - if (cmdmode == 'v') - return for_each_tag_name(argv, verify_tag); + return for_each_tag_name(argv, delete_tag, NULL); + if (cmdmode == 'v') { + if (format) + verify_ref_format(format); + return for_each_tag_name(argv, verify_tag, format); + } if (msg.given || msgfile) { if (msg.given && msgfile) -- 2.11.0
[PATCH v5 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag
From: Lukas Puehringer Functions that print git object information may require that the gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent print_signature_buffer from being called if flag is set. Signed-off-by: Lukas Puehringer --- gpg-interface.h | 1 + tag.c | 5 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gpg-interface.h b/gpg-interface.h index ea68885ad..85dc9820d 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -3,6 +3,7 @@ #define GPG_VERIFY_VERBOSE 1 #define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_QUIET 4 struct signature_check { char *payload; diff --git a/tag.c b/tag.c index d1dcd18cd..291073f6e 100644 --- a/tag.c +++ b/tag.c @@ -3,6 +3,7 @@ #include "commit.h" #include "tree.h" #include "blob.h" +#include "gpg-interface.h" const char *tag_type = "tag"; @@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) ret = check_signature(buf, payload_size, buf + payload_size, size - payload_size, &sigc); - print_signature_buffer(&sigc, flags); + + if (!(flags & GPG_VERIFY_QUIET)) + print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); return ret; -- 2.11.0
[PATCH v5 6/7] t/t7030-verify-tag: Add --format specifier tests
From: Santiago Torres Verify-tag now provides --format specifiers to inspect and ensure the contents of the tag are proper. We add two tests to ensure this functionality works as expected: the return value should indicate if verification passed, and the format specifiers must be respected. Signed-off-by: Santiago Torres --- t/t7030-verify-tag.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 07079a41c..d62ccbb98 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' ' test_cmp expect.stderr actual.stderr ' +test_expect_success 'verifying tag with --format' ' + cat >expect <<-\EOF + tagname : fourth-signed + EOF && + git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual && + test_cmp expect actual +' + +test_expect_success 'verifying a forged tag with --format fail and format accordingly' ' + cat >expect <<-\EOF + tagname : 7th forged-signed + EOF && + test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && + test_cmp expect actual-forged +' + test_done -- 2.11.0
[PATCH v5 7/7] t/t7004-tag: Add --format specifier tests
From: Santiago Torres tag -v now supports --format specifiers to inspect the contents of a tag upon verification. Add two tests to ensure this behavior is respected in future changes. Signed-off-by: Santiago Torres --- t/t7004-tag.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 07869b0c0..b2b81f203 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -874,6 +874,22 @@ test_expect_success GPG 'verifying a forged tag should fail' ' test_must_fail git tag -v forged-tag ' +test_expect_success 'verifying a proper tag with --format pass and format accordingly' ' + cat >expect <<-\EOF + tagname : signed-tag + EOF && + git tag -v --format="tagname : %(tag)" "signed-tag" >actual && + test_cmp expect actual +' + +test_expect_success 'verifying a forged tag with --format fail and format accordingly' ' + cat >expect <<-\EOF + tagname : forged-tag + EOF && + test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" >actual && + test_cmp expect actual +' + # blank and empty messages for signed tags: get_tag_header empty-signed-tag $commit commit $time >expect -- 2.11.0
[PATCH v5 0/7] Add --format to tag verification
From: Santiago Torres This is the fifth iteration of [1][2][3][4], and as a result of the discussion in [5]. The main goal of this patch series is to bring --format to git tag verification so that upper-layer tools can inspect the content of a tag and make decisions based on it. In this re-woll we: * Squashed Peff's first patch[6] with the second patch of the series. The commit message may need work. * Applied the relevant segment's of Peff's second patch[7] on the rest of the series * Rebased so these patches apply to the tip of the master branch. Thanks, -Santiago [1] http://public-inbox.org/git/20161007210721.20437-1-santi...@nyu.edu/ [2] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/ [3] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/ [4] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/ [5] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/ [6] http://public-inbox.org/git/20161019203546.dfqmi2czcxopg...@sigill.intra.peff.net/ [7] http://public-inbox.org/git/20161019203943.epjxnfci7vcqg...@sigill.intra.peff.net/ Lukas Puehringer (4): gpg-interface, tag: add GPG_VERIFY_QUIET flag ref-filter: add function to print single ref_array_item tag: add format specifier to gpg_verify_tag builtin/tag: add --format argument for tag -v Santiago Torres (3): builtin/verify-tag: add --format to verify-tag t/t7030-verify-tag: Add --format specifier tests t/t7004-tag: Add --format specifier tests Documentation/git-tag.txt| 2 +- Documentation/git-verify-tag.txt | 2 +- builtin/tag.c| 32 ++-- builtin/verify-tag.c | 13 +++-- gpg-interface.h | 1 + ref-filter.c | 27 +-- ref-filter.h | 7 +++ t/t7004-tag.sh | 16 t/t7030-verify-tag.sh| 16 tag.c| 22 +++--- tag.h| 4 ++-- 11 files changed, 113 insertions(+), 29 deletions(-) -- 2.11.0
[PATCH v5 3/7] tag: add format specifier to gpg_verify_tag
From: Lukas Puehringer Calling functions for gpg_verify_tag() may desire to print relevant information about the header for further verification. Add an optional format argument to print any desired information after GPG verification. Signed-off-by: Lukas Puehringer --- builtin/tag.c| 2 +- builtin/verify-tag.c | 2 +- tag.c| 17 +++-- tag.h| 4 ++-- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 73df72811..880677df5 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -105,7 +105,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) { - return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE); + return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148cf..de10198c4 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -51,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 (gpg_verify_tag(sha1, name, flags)) + else if (verify_and_format_tag(sha1, name, NULL, flags)) had_error = 1; } return had_error; diff --git a/tag.c b/tag.c index 291073f6e..d5a7cfb20 100644 --- a/tag.c +++ b/tag.c @@ -4,6 +4,7 @@ #include "tree.h" #include "blob.h" #include "gpg-interface.h" +#include "ref-filter.h" const char *tag_type = "tag"; @@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, - unsigned flags) +int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags) { enum object_type type; char *buf; @@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, 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 : + name ? + name : 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 : + name ? + name : find_unique_abbrev(sha1, DEFAULT_ABBREV)); ret = run_gpg_verify(buf, size, flags); free(buf); + + if (fmt_pretty) + pretty_print_ref(name, sha1, fmt_pretty); + return ret; } diff --git a/tag.h b/tag.h index a5721b673..896b9c2dc 100644 --- a/tag.h +++ b/tag.h @@ -17,7 +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); +extern int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags); #endif /* TAG_H */ -- 2.11.0
[PATCH v5 4/7] builtin/verify-tag: add --format to verify-tag
From: Santiago Torres Callers of verify-tag may want to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. This is to avoid tag refs that point to an incorrect object. Add a --format parameter to git verify-tag to print the formatted tag object header in addition to or instead of the --verbose or --raw GPG verification output. Signed-off-by: Santiago Torres --- Documentation/git-verify-tag.txt | 2 +- builtin/verify-tag.c | 13 +++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt index d590edceb..0b8075dad 100644 --- a/Documentation/git-verify-tag.txt +++ b/Documentation/git-verify-tag.txt @@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags SYNOPSIS [verse] -'git verify-tag' ... +'git verify-tag' [--format=] ... DESCRIPTION --- diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index de10198c4..212449f47 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -12,12 +12,14 @@ #include #include "parse-options.h" #include "gpg-interface.h" +#include "ref-filter.h" static const char * const verify_tag_usage[] = { - N_("git verify-tag [-v | --verbose] ..."), + N_("git verify-tag [-v | --verbose] [--format=] ..."), NULL }; + static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + char *fmt_pretty = NULL; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; + if (fmt_pretty) { + verify_ref_format(fmt_pretty); + flags |= GPG_VERIFY_QUIET; + } + 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_and_format_tag(sha1, name, NULL, flags)) + else if (verify_and_format_tag(sha1, name, fmt_pretty, flags)) had_error = 1; } return had_error; -- 2.11.0
[PATCH v6 0/6] Add --format to tag verification
From: Santiago Torres This is the sixth iteration of [1][2][3][4][5], and as a result of the discussion in [5]. The main goal of this patch series is to bring --format to git tag verification so that upper-layer tools can inspect the content of a tag and make decisions based on it. In this re-woll we: * Changed the call interface so printing is done outside of verification. * Fixed a couple of whitespace issues and whatnot. Thanks, -Santiago [1] http://public-inbox.org/git/20170115184705.10376-1-santi...@nyu.edu/ [2] http://public-inbox.org/git/20161007210721.20437-1-santi...@nyu.edu/ [3] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/ [4] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/ [5] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/ [6] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/ [7] http://public-inbox.org/git/20161019203546.dfqmi2czcxopg...@sigill.intra.peff.net/ [8] http://public-inbox.org/git/20161019203943.epjxnfci7vcqg...@sigill.intra.peff.net/ Lukas Puehringer (3): gpg-interface,tag: add GPG_VERIFY_OMIT_STATUS flag ref-filter: add function to print single ref_array_item builtin/tag: add --format argument for tag -v Santiago Torres (3): builtin/verify-tag: add --format to verify-tag t/t7030-verify-tag: Add --format specifier tests t/t7004-tag: Add --format specifier tests Documentation/git-tag.txt| 2 +- Documentation/git-verify-tag.txt | 2 +- builtin/tag.c| 38 -- builtin/verify-tag.c | 23 --- gpg-interface.h | 5 +++-- ref-filter.c | 27 +-- ref-filter.h | 7 +++ t/t7004-tag.sh | 16 t/t7030-verify-tag.sh| 16 tag.c| 5 - 10 files changed, 117 insertions(+), 24 deletions(-) -- 2.11.0
[PATCH v6 5/6] t/t7030-verify-tag: Add --format specifier tests
From: Santiago Torres Verify-tag now provides --format specifiers to inspect and ensure the contents of the tag are proper. We add two tests to ensure this functionality works as expected: the return value should indicate if verification passed, and the format specifiers must be respected. Signed-off-by: Santiago Torres --- t/t7030-verify-tag.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 07079a41c..d62ccbb98 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' ' test_cmp expect.stderr actual.stderr ' +test_expect_success 'verifying tag with --format' ' + cat >expect <<-\EOF + tagname : fourth-signed + EOF && + git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual && + test_cmp expect actual +' + +test_expect_success 'verifying a forged tag with --format fail and format accordingly' ' + cat >expect <<-\EOF + tagname : 7th forged-signed + EOF && + test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && + test_cmp expect actual-forged +' + test_done -- 2.11.0
[PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Lukas Puehringer Adding --format to git tag -v mutes the default output of the GPG verification and instead prints the formatted tag object. This allows callers to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. The callback function for for_each_tag_name() didn't allow callers to pass custom data to their callback functions. Add a new opaque pointer to each_tag_name_fn's parameter to allow this. Signed-off-by: Lukas Puehringer --- Documentation/git-tag.txt | 2 +- builtin/tag.c | 38 -- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 76cfe40d9..586aaa315 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -15,7 +15,7 @@ SYNOPSIS 'git tag' [-n[]] -l [--contains ] [--points-at ] [--column[=] | --no-column] [--create-reflog] [--sort=] [--format=] [--[no-]merged []] [...] -'git tag' -v ... +'git tag' -v [--format=] ... DESCRIPTION --- diff --git a/builtin/tag.c b/builtin/tag.c index 73df72811..b9da72761 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = { N_("git tag -d ..."), N_("git tag -l [-n[]] [--contains ] [--points-at ]" "\n\t\t[--format=] [--[no-]merged []] [...]"), - N_("git tag -v ..."), + N_("git tag -v [--format=] ..."), NULL }; @@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con } typedef int (*each_tag_name_fn)(const char *name, const char *ref, - const unsigned char *sha1); + const unsigned char *sha1, void *cb_data); -static int for_each_tag_name(const char **argv, each_tag_name_fn fn) +static int for_each_tag_name(const char **argv, each_tag_name_fn fn, + void *cb_data) { const char **p; char ref[PATH_MAX]; int had_error = 0; unsigned char sha1[20]; + for (p = argv; *p; p++) { if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p) >= sizeof(ref)) { @@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn) had_error = 1; continue; } - if (fn(*p, ref, sha1)) + if (fn(*p, ref, sha1, cb_data)) had_error = 1; } return had_error; } static int delete_tag(const char *name, const char *ref, - const unsigned char *sha1) + const unsigned char *sha1, void *cb_data) { if (delete_ref(ref, sha1, 0)) return 1; @@ -103,9 +105,22 @@ 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 unsigned char *sha1, void *cb_data) { - return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE); + int flags; + char *fmt_pretty = cb_data; + flags = GPG_VERIFY_VERBOSE; + + if (fmt_pretty) + flags = GPG_VERIFY_OMIT_STATUS; + + if (gpg_verify_tag(sha1, name, flags)) + return -1; + +if (fmt_pretty) + pretty_print_ref(name, sha1, fmt_pretty); + + return 0; } static int do_sign(struct strbuf *buffer) @@ -428,9 +443,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (filter.merge_commit) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') - return for_each_tag_name(argv, delete_tag); - if (cmdmode == 'v') - return for_each_tag_name(argv, verify_tag); + return for_each_tag_name(argv, delete_tag, NULL); + if (cmdmode == 'v') { + if (format) + verify_ref_format(format); + return for_each_tag_name(argv, verify_tag, format); + } if (msg.given || msgfile) { if (msg.given && msgfile) -- 2.11.0
[PATCH v6 3/6] builtin/verify-tag: add --format to verify-tag
From: Santiago Torres Callers of verify-tag may want to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. This is to avoid tag refs that point to an incorrect object. Add a --format parameter to git verify-tag to print the formatted tag object header in addition to or instead of the --verbose or --raw GPG verification output. Signed-off-by: Santiago Torres --- Documentation/git-verify-tag.txt | 2 +- builtin/verify-tag.c | 23 --- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt index d590edceb..0b8075dad 100644 --- a/Documentation/git-verify-tag.txt +++ b/Documentation/git-verify-tag.txt @@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags SYNOPSIS [verse] -'git verify-tag' ... +'git verify-tag' [--format=] ... DESCRIPTION --- diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148cf..18443bf9f 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -12,12 +12,14 @@ #include #include "parse-options.h" #include "gpg-interface.h" +#include "ref-filter.h" static const char * const verify_tag_usage[] = { - N_("git verify-tag [-v | --verbose] ..."), + N_("git verify-tag [-v | --verbose] [--format=] ..."), NULL }; + static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + char *fmt_pretty = NULL; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -46,13 +50,26 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; + if (fmt_pretty) { + verify_ref_format(fmt_pretty); + flags |= GPG_VERIFY_OMIT_STATUS; + } + while (i < argc) { unsigned char sha1[20]; const char *name = argv[i++]; - if (get_sha1(name, sha1)) + if (get_sha1(name, sha1)) { had_error = !!error("tag '%s' not found.", name); - else if (gpg_verify_tag(sha1, name, flags)) + continue; + } + + if (gpg_verify_tag(sha1, name, flags)) { had_error = 1; + continue; + } + + if (fmt_pretty) + pretty_print_ref(name, sha1, fmt_pretty); } return had_error; } -- 2.11.0
[PATCH v6 1/6] gpg-interface,tag: add GPG_VERIFY_OMIT_STATUS flag
From: Lukas Puehringer Functions that print git object information may require that the gpg-interface functions be silent. Add GPG_VERIFY_OMIT_STATUS flag and prevent print_signature_buffer from being called if flag is set. Signed-off-by: Lukas Puehringer --- gpg-interface.h | 5 +++-- tag.c | 5 - 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/gpg-interface.h b/gpg-interface.h index ea68885ad..d2d4fd3a6 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -1,8 +1,9 @@ #ifndef GPG_INTERFACE_H #define GPG_INTERFACE_H -#define GPG_VERIFY_VERBOSE 1 -#define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_VERBOSE 1 +#define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_OMIT_STATUS 4 struct signature_check { char *payload; diff --git a/tag.c b/tag.c index d1dcd18cd..243d1fdbb 100644 --- a/tag.c +++ b/tag.c @@ -3,6 +3,7 @@ #include "commit.h" #include "tree.h" #include "blob.h" +#include "gpg-interface.h" const char *tag_type = "tag"; @@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) ret = check_signature(buf, payload_size, buf + payload_size, size - payload_size, &sigc); - print_signature_buffer(&sigc, flags); + + if (!(flags & GPG_VERIFY_OMIT_STATUS)) + print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); return ret; -- 2.11.0
[PATCH v6 2/6] ref-filter: add function to print single ref_array_item
From: Lukas Puehringer ref-filter functions are useful for printing git object information using a format specifier. However, some other modules may not want to use this functionality on a ref-array but only print a single item. Expose a pretty_print_ref function to create, pretty print and free individual ref-items. Signed-off-by: Lukas Puehringer --- ref-filter.c | 27 +-- ref-filter.h | 7 +++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 1a978405e..5f4b08792 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1361,7 +1361,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname, return ref; } -static int filter_ref_kind(struct ref_filter *filter, const char *refname) +static int ref_kind_from_refname(const char *refname) { unsigned int i; @@ -1374,11 +1374,7 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) { "refs/tags/", FILTER_REFS_TAGS} }; - if (filter->kind == FILTER_REFS_BRANCHES || - filter->kind == FILTER_REFS_REMOTES || - filter->kind == FILTER_REFS_TAGS) - return filter->kind; - else if (!strcmp(refname, "HEAD")) + if (!strcmp(refname, "HEAD")) return FILTER_REFS_DETACHED_HEAD; for (i = 0; i < ARRAY_SIZE(ref_kind); i++) { @@ -1389,6 +1385,15 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) return FILTER_REFS_OTHERS; } +static int filter_ref_kind(struct ref_filter *filter, const char *refname) +{ + if (filter->kind == FILTER_REFS_BRANCHES || + filter->kind == FILTER_REFS_REMOTES || + filter->kind == FILTER_REFS_TAGS) + return filter->kind; + return ref_kind_from_refname(refname); +} + /* * A call-back given to for_each_ref(). Filter refs and keep them for * later object processing. @@ -1671,6 +1676,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu putchar('\n'); } +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format) +{ + struct ref_array_item *ref_item; + ref_item = new_ref_array_item(name, sha1, 0); + ref_item->kind = ref_kind_from_refname(name); + show_ref_array_item(ref_item, format, 0); + free_array_item(ref_item); +} + /* If no sorting option is given, use refname to sort as default */ struct ref_sorting *ref_default_sorting(void) { diff --git a/ref-filter.h b/ref-filter.h index fc55fa357..7b05592ba 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -109,4 +109,11 @@ struct ref_sorting *ref_default_sorting(void); /* Function to parse --merged and --no-merged options */ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); +/* + * Print a single ref, outside of any ref-filter. Note that the + * name must be a fully qualified refname. + */ +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format); + #endif /* REF_FILTER_H */ -- 2.11.0
[PATCH v6 6/6] t/t7004-tag: Add --format specifier tests
From: Santiago Torres tag -v now supports --format specifiers to inspect the contents of a tag upon verification. Add two tests to ensure this behavior is respected in future changes. Signed-off-by: Santiago Torres --- t/t7004-tag.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 07869b0c0..ba88b556b 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -874,6 +874,22 @@ test_expect_success GPG 'verifying a forged tag should fail' ' test_must_fail git tag -v forged-tag ' +test_expect_success 'verifying a proper tag with --format pass and format accordingly' ' + cat >expect <<-\EOF + tagname : signed-tag + EOF && + git tag -v --format="tagname : %(tag)" "signed-tag" >actual && + test_cmp expect actual +' + +test_expect_success 'verifying a forged tag with --format fail and format accordingly' ' + cat >expect <<-\EOF + tagname : forged-tag + EOF && + test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" >actual && + test_cmp expect actual +' + # blank and empty messages for signed tags: get_tag_header empty-signed-tag $commit commit $time >expect -- 2.11.0
[PATCH v4 3/7] tag: add format specifier to gpg_verify_tag
From: Lukas Puehringer Calling functions for gpg_verify_tag() may desire to print relevant information about the header for further verification. Add an optional format argument to print any desired information after GPG verification. Signed-off-by: Lukas Puehringer --- builtin/tag.c| 2 +- builtin/verify-tag.c | 2 +- tag.c| 17 +++-- tag.h| 4 ++-- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 50e4ae5..14f3b48 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -105,7 +105,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) { - return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE); + return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148..de10198 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -51,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 (gpg_verify_tag(sha1, name, flags)) + else if (verify_and_format_tag(sha1, name, NULL, flags)) had_error = 1; } return had_error; diff --git a/tag.c b/tag.c index 291073f..d3512c0 100644 --- a/tag.c +++ b/tag.c @@ -4,6 +4,7 @@ #include "tree.h" #include "blob.h" #include "gpg-interface.h" +#include "ref-filter.h" const char *tag_type = "tag"; @@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, - unsigned flags) +int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags) { enum object_type type; char *buf; @@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, 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 : + name ? + name : 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 : + name ? + name : find_unique_abbrev(sha1, DEFAULT_ABBREV)); ret = run_gpg_verify(buf, size, flags); free(buf); + + if (fmt_pretty) + pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS); + return ret; } diff --git a/tag.h b/tag.h index a5721b6..896b9c2 100644 --- a/tag.h +++ b/tag.h @@ -17,7 +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); +extern int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags); #endif /* TAG_H */ -- 2.10.0
[PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag
From: Santiago Torres Callers of verify-tag may want to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. This is to avoid tag refs that point to an incorrect object. Add a --format parameter to git verify-tag to print the formatted tag object header in addition to or instead of the --verbose or --raw GPG verification output. Signed-off-by: Santiago Torres --- Documentation/git-verify-tag.txt | 2 +- builtin/verify-tag.c | 13 +++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt index d590edc..0b8075d 100644 --- a/Documentation/git-verify-tag.txt +++ b/Documentation/git-verify-tag.txt @@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags SYNOPSIS [verse] -'git verify-tag' ... +'git verify-tag' [--format=] ... DESCRIPTION --- diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index de10198..745b6a6 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -12,12 +12,14 @@ #include #include "parse-options.h" #include "gpg-interface.h" +#include "ref-filter.h" static const char * const verify_tag_usage[] = { - N_("git verify-tag [-v | --verbose] ..."), + N_("git verify-tag [-v | --verbose] [--format=] ..."), NULL }; + static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + char *fmt_pretty = NULL; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; + if (fmt_pretty) { + verify_ref_format(fmt_pretty); + flags |= GPG_VERIFY_QUIET; + } + 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_and_format_tag(sha1, name, NULL, flags)) + else if (verify_and_format_tag(sha1, name, fmt_pretty, flags)) had_error = 1; } return had_error; -- 2.10.0
[PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag
From: Lukas Puehringer Functions that print git object information may require that the gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent print_signature_buffer from being called if flag is set. Signed-off-by: Lukas Puehringer --- gpg-interface.h | 1 + tag.c | 5 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gpg-interface.h b/gpg-interface.h index ea68885..85dc982 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -3,6 +3,7 @@ #define GPG_VERIFY_VERBOSE 1 #define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_QUIET 4 struct signature_check { char *payload; diff --git a/tag.c b/tag.c index d1dcd18..291073f 100644 --- a/tag.c +++ b/tag.c @@ -3,6 +3,7 @@ #include "commit.h" #include "tree.h" #include "blob.h" +#include "gpg-interface.h" const char *tag_type = "tag"; @@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) ret = check_signature(buf, payload_size, buf + payload_size, size - payload_size, &sigc); - print_signature_buffer(&sigc, flags); + + if (!(flags & GPG_VERIFY_QUIET)) + print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); return ret; -- 2.10.0
[PATCH v4 2/7] ref-filter: add function to print single ref_array_item
From: Lukas Puehringer ref-filter functions are useful for printing git object information using a format specifier. However, some other modules may not want to use this functionality on a ref-array but only print a single item. Expose a pretty_print_ref function to create, pretty print and free individual ref-items. Signed-off-by: Lukas Puehringer --- ref-filter.c | 10 ++ ref-filter.h | 3 +++ 2 files changed, 13 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 9adbb8a..cfbcd73 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1637,6 +1637,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu putchar('\n'); } +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format, unsigned kind) +{ + struct ref_array_item *ref_item; + ref_item = new_ref_array_item(name, sha1, 0); + ref_item->kind = kind; + show_ref_array_item(ref_item, format, 0); + free_array_item(ref_item); +} + /* If no sorting option is given, use refname to sort as default */ struct ref_sorting *ref_default_sorting(void) { diff --git a/ref-filter.h b/ref-filter.h index 14d435e..3d23090 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void); /* Function to parse --merged and --no-merged options */ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format, unsigned kind); + #endif /* REF_FILTER_H */ -- 2.10.0
[PATCH v4 0/7] Add --format to tag verification
From: Santiago Torres This is the fourth iteration of the series in [1][2][3], which comes as a result of the discussion in [4]. The main goal of this patch series is to bring --format to git tag verification so that upper-layer tools can inspect the content of a tag and make decisions based on those contents. In this re-woll we: * Fixed the author fields and signed off by's throughout the patch series * Added two more patches with unit tests to ensure the format specifier behaves as expected * Fixed a missing initialization for the format specifier in verify-tag which caused a crash. * Fixed an outdated git commit message that had the previous name of a function definition. Thanks, -Santiago [1] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/ [2] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/ [3] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/ [4] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/ Lukas Puehringer (4): tag: add format specifier to gpg_verify_tag gpg-interface, tag: add GPG_VERIFY_QUIET flag ref-filter: add function to print single ref_array_item builtin/tag: add --format argument for tag -v Santiago Torres (3): builtin/verify-tag: add --format to verify-tag t/t7030-verify-tag: Add --format specifier tests t/t7004-tag: Add --format specifier tests Documentation/git-tag.txt| 2 +- Documentation/git-verify-tag.txt | 2 +- builtin/tag.c| 34 +++--- builtin/verify-tag.c | 13 +++-- gpg-interface.h | 1 + ref-filter.c | 10 ++ ref-filter.h | 3 +++ t/t7004-tag.sh | 16 t/t7030-verify-tag.sh| 16 tag.c| 22 +++--- tag.h| 4 ++-- 11 files changed, 99 insertions(+), 24 deletions(-) -- 2.10.0
[PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests
From: Santiago Torres Verify-tag now provides --format specifiers to inspect and ensure the contents of the tag are proper. We add two tests to ensure this functionality works as expected: the return value should indicate if verification passed, and the format specifiers must be respected. Signed-off-by: Santiago Torres --- t/t7030-verify-tag.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 07079a4..ce37fd9 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' ' test_cmp expect.stderr actual.stderr ' +test_expect_success 'verifying tag with --format' ' + cat >expect <<-\EOF && + tagname : fourth-signed + EOF + git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual && + test_cmp expect actual +' + +test_expect_success 'verifying a forged tag with --format fail and format accordingly' ' + cat >expect <<-\EOF && + tagname : 7th forged-signed + EOF + test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && + test_cmp expect actual-forged +' + test_done -- 2.10.0
[PATCH v4 5/7] builtin/tag: add --format argument for tag -v
From: Lukas Puehringer Adding --format to git tag -v mutes the default output of the GPG verification and instead prints the formatted tag object. This allows callers to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. The callback function for for_each_tag_name() didn't allow callers to pass custom data to their callback functions. Add a new opaque pointer to each_tag_name_fn's parameter to allow this. Signed-off-by: Lukas Puehringer --- Documentation/git-tag.txt | 2 +- builtin/tag.c | 34 +++--- builtin/verify-tag.c | 2 +- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 7ecca8e..3bb5e3c 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -15,7 +15,7 @@ SYNOPSIS 'git tag' [-n[]] -l [--contains ] [--points-at ] [--column[=] | --no-column] [--create-reflog] [--sort=] [--format=] [--[no-]merged []] [...] -'git tag' -v ... +'git tag' -v [--format=] ... DESCRIPTION --- diff --git a/builtin/tag.c b/builtin/tag.c index 14f3b48..7730fd0 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = { N_("git tag -d ..."), N_("git tag -l [-n[]] [--contains ] [--points-at ]" "\n\t\t[--format=] [--[no-]merged []] [...]"), - N_("git tag -v ..."), + N_("git tag -v [--format=] ..."), NULL }; @@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con } typedef int (*each_tag_name_fn)(const char *name, const char *ref, - const unsigned char *sha1); + const unsigned char *sha1, void *cb_data); -static int for_each_tag_name(const char **argv, each_tag_name_fn fn) +static int for_each_tag_name(const char **argv, each_tag_name_fn fn, + void *cb_data) { const char **p; char ref[PATH_MAX]; int had_error = 0; unsigned char sha1[20]; + for (p = argv; *p; p++) { if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p) >= sizeof(ref)) { @@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn) had_error = 1; continue; } - if (fn(*p, ref, sha1)) + if (fn(*p, ref, sha1, cb_data)) had_error = 1; } return had_error; } static int delete_tag(const char *name, const char *ref, - const unsigned char *sha1) + const unsigned char *sha1, void *cb_data) { if (delete_ref(ref, sha1, 0)) return 1; @@ -103,9 +105,16 @@ 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 unsigned char *sha1, void *cb_data) { - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); + int flags; + char *fmt_pretty = cb_data; + flags = GPG_VERIFY_VERBOSE; + + if (fmt_pretty) + flags = GPG_VERIFY_QUIET; + + return verify_and_format_tag(sha1, name, fmt_pretty, flags); } static int do_sign(struct strbuf *buffer) @@ -334,7 +343,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) struct strbuf err = STRBUF_INIT; struct ref_filter filter; static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting; - const char *format = NULL; + char *format = NULL; struct option options[] = { OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'), { OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"), @@ -424,9 +433,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (filter.merge_commit) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') - return for_each_tag_name(argv, delete_tag); - if (cmdmode == 'v') - return for_each_tag_name(argv, verify_tag); + return for_each_tag_name(argv, delete_tag, NULL); + if (cmdmode == 'v') { + if (format) + verify_ref_format(format); + return for_each_tag_name(argv, verify_tag, format); + } if (msg.given || msgfile) { if (msg.given && msgfile) -- 2.10.0
[PATCH v4 7/7] t/t7004-tag: Add --format specifier tests
From: Santiago Torres tag -v now supports --format specifiers to inspect the contents of a tag upon verification. Add two tests to ensure this behavior is respected in future changes. Signed-off-by: Santiago Torres --- t/t7004-tag.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 8b0f71a..633b089 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -847,6 +847,22 @@ test_expect_success GPG 'verifying a forged tag should fail' ' test_must_fail git tag -v forged-tag ' +test_expect_success 'verifying a proper tag with --format pass and format accordingly' ' + cat >expect <<-\EOF && + tagname : signed-tag + EOF + git tag -v --format="tagname : %(tag)" "signed-tag" >actual && + test_cmp expect actual +' + +test_expect_success 'verifying a forged tag with --format fail and format accordingly' ' + cat >expect <<-\EOF && + tagname : forged-tag + EOF + test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" >actual && + test_cmp expect actual +' + # blank and empty messages for signed tags: get_tag_header empty-signed-tag $commit commit $time >expect -- 2.10.0
[RFC/PATCH] verify-tag: add --check-name flag
From: Santiago Torres Hello everyone, In a previous thread [1] we discussed about the possibility of having a --check-name flag, for the tag-verify command (and possibly git tag -v). Although many points were in the table, I don't think that it was conclusive as to whether having it or not. Also, I noticed that the, refactored interface requires really minmal changes to do this (see attached patch). This is the behavior I had in mind: santiago at ~/.../.../git ✗ ./git-verify-tag --check-name v2.7.0-rc2 gpg: Signature made Tue 22 Dec 2015 05:46:04 PM EST using RSA ... gpg: Good signature from "Junio C Hamano " [...] ... error: tag name doesn't match tag header!(v2.7.0-rc2) santiago at ~/.../.../git ✗ ./git-verify-tag --check-name v2.7.0-rc2 gpg: Signature made Tue 22 Dec 2015 05:46:04 PM EST using RSA ... gpg: Good signature from "Junio C Hamano " [...] ... The rationale behind this is the following: 1.- Using a tag ref as a check-out mechanism is pretty common by package managers and other tools. Verifying the tag signature provides authentication guarantees, but there is no feedback that the signature being verified belongs to the intended tag. 2.- The tuple tagname + signature can uniquely identify a tag. There are many tags that can have the same name, but this is mostly due to the naming policy. Having a tag-ref pointing to the right tag name with an appropriate signature provides tigther guarantees about the tag that's being checked-out. 3.- This follows concerns about other people who wish to provide a tighter binding between the refs and the tag objects. The git-evtag project is an example of this[2]. What I want to prevent is the following: santiago at ~/.../ ✔ pip install -e git+https://.../django/@1.8.3#egg=django Obtaining django from git+https://.../django/@1.8.3#egg=django [...] Successfully installed django santiago at ~/.../ ✔ django-admin.py --version 1.4.11 In this example, the tag retrieved is an annotated tag, signed by the right developer. In this case signature verification would pass, and there are no hints that something *might* have be wrong. Needless to say, Django 1.4.11 is deprecated and vulnerable to really nasty XSS and SQLi vectors... I acknowledge that it is possible to provide this by using the SHA1 of the tag object instead of the tag-ref, but this provides comparable guarantees while keeping a readable interface. Of course that the sha1 is the "tightest" binding, so this also allows for developers to remove tags during quick-fixes (as Junio pointed out in [1]) and other edge cases in which the SHA1 would break. Of course that using this flag needs to be integrated by package managers and other tools; I wouldn't mind reaching out and even proposing patches for the popular ones. A stub of the intended patch follows. I'll can make a cleaner patch (e.g., to include this in git tag -v) based on any feedback provided. [1] http://thread.gmane.org/gmane.comp.version-control.git/284757 [2] http://thread.gmane.org/gmane.comp.version-control.git/288439 --- builtin/verify-tag.c | 1 + tag.c| 8 tag.h| 1 + 3 files changed, 10 insertions(+) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148..947babe 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -33,6 +33,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_BIT(0, "check-name", &flags, N_("Verify the tag name"), TAG_VERIFY_NAME), OPT_END() }; diff --git a/tag.c b/tag.c index d1dcd18..591b31e 100644 --- a/tag.c +++ b/tag.c @@ -55,6 +55,14 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, ret = run_gpg_verify(buf, size, flags); + if (flags & TAG_VERIFY_NAME) { + struct tag tag_info; + ret += parse_tag_buffer(&tag_info, buf, size); + if strncmp(tag_info.tag, name_to_report, size) + ret += error("tag name doesn't match tag header!(%s)", + tag_info.tag); + } + free(buf); return ret; } diff --git a/tag.h b/tag.h index a5721b6..30c922e 100644 --- a/tag.h +++ b/tag.h @@ -2,6 +2,7 @@ #define TAG_H #include "object.h" +#define TAG_VERIFY_NAME 0x10 extern const char *tag_type; -- 2.8.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/6] Add --format to tag verification
From: Santiago Torres Hello everyone, This is a followup on [1]. There we discussed what would be the best way to provide automated scripts with mechanisms to inspect the contents of a tag upon verification. We struggled a little bit with how to make this fit the current git codebase in the best way. Specifically, we are not sure if adding the GPG_QUIET flags and/or exposing the primitives to allocate individual git_ref_item's would be the best way forward. This applies on the current HEAD as well as v 2.9.10. Thanks! -Santiago. P.S. Gmane seems to be broken for git after it was rebooted. Should we ping them about it? [1] http://lists-archives.com/git/869122-verify-tag-add-check-name-flag.html Lukas P (4): gpg-interface: add GPG_VERIFY_QUIET flag ref-filter: Expose wrappers for ref_item functions tag: add format specifier to gpg_verify_tag builtin/tag: add --format argument for tag -v Santiago Torres (2): builtin/tag: move format specifier to global var builtin/verify-tag: Add --format to verify-tag builtin/tag.c| 33 + builtin/verify-tag.c | 17 ++--- gpg-interface.c | 3 +++ gpg-interface.h | 1 + ref-filter.c | 20 ref-filter.h | 10 ++ tag.c| 14 -- tag.h| 4 ++-- 8 files changed, 83 insertions(+), 19 deletions(-) -- 2.10.0
[PATCH 6/6] builtin/tag: add --format argument for tag -v
From: Lukas P Adding --format to git tag -v mutes the default output of the GPG verification and instead prints the formatted tag object. This allows callers to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. Signed-off-by: Lukas Puehringer --- builtin/tag.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 94ed8a2..3dd1e65 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = { N_("git tag -d ..."), N_("git tag -l [-n[]] [--contains ] [--points-at ]" "\n\t\t[--format=] [--[no-]merged []] [...]"), - N_("git tag -v ..."), + N_("git tag -v [--format=] ..."), NULL }; @@ -106,7 +106,13 @@ static int delete_tag(const char *name, const char *ref, static int verify_tag(const char *name, const char *ref, const unsigned char *sha1) { - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); + int flags; + flags = GPG_VERIFY_VERBOSE; + + if (fmt_pretty) + flags = GPG_VERIFY_QUIET; + + return verify_and_format_tag(sha1, name, fmt_pretty, flags); } static int do_sign(struct strbuf *buffer) @@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') return for_each_tag_name(argv, delete_tag); - if (cmdmode == 'v') + if (cmdmode == 'v') { + if (fmt_pretty) + verify_ref_format(fmt_pretty); return for_each_tag_name(argv, verify_tag); + } if (msg.given || msgfile) { if (msg.given && msgfile) -- 2.10.0
[PATCH 3/6] ref-filter: Expose wrappers for ref_item functions
From: Lukas P Ref-filter functions are useful for printing git object information without a format specifier. However, some functions may not want to use a complete ref-array, and just a single item instead. Expose create/show/free functions for ref_array_items through wrappers around the original functions. Signed-off-by: Lukas Puehringer --- ref-filter.c | 20 ref-filter.h | 10 ++ 2 files changed, 30 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 9adbb8a..b013799 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1329,6 +1329,14 @@ static struct ref_array_item *new_ref_array_item(const char *refname, return ref; } +/* Wrapper: Create ref_array_item w/o referencing container in function name */ +struct ref_array_item *new_ref_item(const char *refname, +const unsigned char *objectname, +int flag) +{ + return new_ref_array_item(refname, objectname, flag); +} + static int filter_ref_kind(struct ref_filter *filter, const char *refname) { unsigned int i; @@ -1426,6 +1434,12 @@ static void free_array_item(struct ref_array_item *item) free(item); } +/* Wrapper: Free ref_array_item w/o referencing container in function name */ +void free_ref_item(struct ref_array_item *ref_item) +{ + free_array_item(ref_item); +} + /* Free all memory allocated for ref_array */ void ref_array_clear(struct ref_array *array) { @@ -1637,6 +1651,12 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu putchar('\n'); } +/* Wrapper: Show ref_array_item w/o referencing container in function name */ +void show_ref_item(struct ref_array_item *ref_item, const char *format, int quote_style) +{ + show_ref_array_item(ref_item, format, quote_style); +} + /* If no sorting option is given, use refname to sort as default */ struct ref_sorting *ref_default_sorting(void) { diff --git a/ref-filter.h b/ref-filter.h index 14d435e..0f0ffe9 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -107,4 +107,14 @@ struct ref_sorting *ref_default_sorting(void); /* Function to parse --merged and --no-merged options */ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); +/* + * Wrappers exposing the ref_array_item data structure independently + * of the container ref_array, e.g. to format-print individual refs. + */ +struct ref_array_item *new_ref_item(const char *refname, + const unsigned char *objectname, int flag); +void show_ref_item(struct ref_array_item *ref_item, const char *format, + int quote_style); +void free_ref_item(struct ref_array_item *ref_item); + #endif /* REF_FILTER_H */ -- 2.10.0
[PATCH 4/6] tag: add format specifier to gpg_verify_tag
From: Lukas P Calling functions for gpg_verify_tag() may desire to print relevant information about the header for further verification. Add an optional format argument to print any desired information after GPG verification. Signed-off-by: Lukas Puehringer --- builtin/tag.c| 2 +- builtin/verify-tag.c | 6 -- tag.c| 14 -- tag.h| 4 ++-- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index dbf271f..94ed8a2 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -106,7 +106,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) { - return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE); + return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148..7a1121b 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -51,8 +51,10 @@ 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 (gpg_verify_tag(sha1, name, flags)) - had_error = 1; + else { + if (verify_and_format_tag(sha1, name, NULL, flags)) + had_error = 1; + } } return had_error; } diff --git a/tag.c b/tag.c index d1dcd18..e08da51 100644 --- a/tag.c +++ b/tag.c @@ -3,6 +3,7 @@ #include "commit.h" #include "tree.h" #include "blob.h" +#include "ref-filter.h" const char *tag_type = "tag"; @@ -30,8 +31,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, - unsigned flags) +int verify_and_format_tag(const unsigned char *sha1, const char *name_to_report, + const char *fmt_pretty, unsigned flags) { enum object_type type; char *buf; @@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, ret = run_gpg_verify(buf, size, flags); free(buf); + + if (fmt_pretty) { + struct ref_array_item *ref_item; + ref_item = new_ref_item(name_to_report, sha1, 0); + ref_item->kind = FILTER_REFS_TAGS; + show_ref_item(ref_item, fmt_pretty, 0); + free_ref_item(ref_item); + } + return ret; } diff --git a/tag.h b/tag.h index a5721b6..460b6f9 100644 --- a/tag.h +++ b/tag.h @@ -17,7 +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); +extern int verify_and_format_tag(const unsigned char *sha1, + const char *name_to_report, const char *fmt_pretty, unsigned flags); #endif /* TAG_H */ -- 2.10.0
[PATCH 1/6] builtin/tag: move format specifier to global var
From: Santiago Torres The format specifier will be likely used in other functions throughout git tag. One likely candidate to require format strings in the future is the gpg_verify_tag function. However, changing the signature of functions such as for_each_ref or verify_tag would be quite burdensome. Instead, we move the format string specifier to a static global variable that modules can access in the same way git-log and other modules handle this case. Signed-off-by: Santiago Torres --- builtin/tag.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 50e4ae5..dbf271f 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -30,8 +30,9 @@ static const char * const git_tag_usage[] = { static unsigned int colopts; static int force_sign_annotate; +static const char *fmt_pretty; -static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format) +static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting) { struct ref_array array; char *to_free = NULL; @@ -42,23 +43,23 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con if (filter->lines == -1) filter->lines = 0; - if (!format) { + if (!fmt_pretty) { if (filter->lines) { to_free = xstrfmt("%s %%(contents:lines=%d)", "%(align:15)%(refname:strip=2)%(end)", filter->lines); - format = to_free; + fmt_pretty = to_free; } else - format = "%(refname:strip=2)"; + fmt_pretty = "%(refname:strip=2)"; } - verify_ref_format(format); + verify_ref_format(fmt_pretty); filter->with_commit_tag_algo = 1; filter_refs(&array, filter, FILTER_REFS_TAGS); ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) - show_ref_array_item(array.items[i], format, 0); + show_ref_array_item(array.items[i], fmt_pretty, 0); ref_array_clear(&array); free(to_free); @@ -334,7 +335,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix) struct strbuf err = STRBUF_INIT; struct ref_filter filter; static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting; - const char *format = NULL; struct option options[] = { OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'), { OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"), @@ -369,7 +369,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"), N_("print only tags of the object"), 0, parse_opt_object_name }, - OPT_STRING( 0 , "format", &format, N_("format"), N_("format to use for the output")), + OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -410,7 +410,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) run_column_filter(colopts, &copts); } filter.name_patterns = argv; - ret = list_tags(&filter, sorting, format); + ret = list_tags(&filter, sorting); if (column_active(colopts)) stop_column_filter(); return ret; -- 2.10.0
[PATCH 2/6] gpg-interface: add GPG_VERIFY_QUIET flag
From: Lukas P Functions that print git object information may require that the gpg-interface functions be silent. Add a GPG_VERIFY_QUIET to prevent functions such as `print_signature_buffer` from printing any output and only return whether signature verification passed or not. Signed-off-by: Lukas Puehringer --- gpg-interface.c | 3 +++ gpg-interface.h | 1 + 2 files changed, 4 insertions(+) diff --git a/gpg-interface.c b/gpg-interface.c index 8672eda..b82bc50 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -88,6 +88,9 @@ int check_signature(const char *payload, size_t plen, const char *signature, void print_signature_buffer(const struct signature_check *sigc, unsigned flags) { + if (flags & GPG_VERIFY_QUIET) + return; + const char *output = flags & GPG_VERIFY_RAW ? sigc->gpg_status : sigc->gpg_output; diff --git a/gpg-interface.h b/gpg-interface.h index ea68885..85dc982 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -3,6 +3,7 @@ #define GPG_VERIFY_VERBOSE 1 #define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_QUIET 4 struct signature_check { char *payload; -- 2.10.0
[PATCH 5/6] builtin/verify-tag: Add --format to verify-tag
From: Santiago Torres Callers of verify-tag may want to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. This is to avoid tag refs that point to an incorrect object. Add a --format parameter to git verify-tag to print the formatted tag object header in addition to or instead of the --verbose or --raw GPG verification output. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 7a1121b..319d469 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -12,12 +12,15 @@ #include #include "parse-options.h" #include "gpg-interface.h" +#include "ref-filter.h" static const char * const verify_tag_usage[] = { - N_("git verify-tag [-v | --verbose] ..."), + N_("git verify-tag [-v | --verbose] [--format=] ..."), NULL }; +char *fmt_pretty; + static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -46,13 +50,18 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; + if (fmt_pretty) { + verify_ref_format(fmt_pretty); + flags |= GPG_VERIFY_QUIET; + } + 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_and_format_tag(sha1, name, NULL, flags)) + if (verify_and_format_tag(sha1, name, fmt_pretty, flags)) had_error = 1; } } -- 2.10.0
[PATCH v2 3/5] tag: add format specifier to gpg_verify_tag
From: Lukas P Calling functions for gpg_verify_tag() may desire to print relevant information about the header for further verification. Add an optional format argument to print any desired information after GPG verification. Signed-off-by: Lukas P --- builtin/tag.c| 2 +- builtin/verify-tag.c | 2 +- tag.c| 17 +++-- tag.h| 4 ++-- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 50e4ae5..14f3b48 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -105,7 +105,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) { - return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE); + return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148..de10198 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -51,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 (gpg_verify_tag(sha1, name, flags)) + else if (verify_and_format_tag(sha1, name, NULL, flags)) had_error = 1; } return had_error; diff --git a/tag.c b/tag.c index 291073f..65e1a96 100644 --- a/tag.c +++ b/tag.c @@ -4,6 +4,7 @@ #include "tree.h" #include "blob.h" #include "gpg-interface.h" +#include "ref-filter.h" const char *tag_type = "tag"; @@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, - unsigned flags) +int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags) { enum object_type type; char *buf; @@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, 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 : + name ? + name : 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 : + name ? + name : find_unique_abbrev(sha1, DEFAULT_ABBREV)); ret = run_gpg_verify(buf, size, flags); free(buf); + + if (fmt_pretty) + format_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS); + return ret; } diff --git a/tag.h b/tag.h index a5721b6..0b6e458 100644 --- a/tag.h +++ b/tag.h @@ -17,7 +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); +extern int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags); #endif /* TAG_H */ -- 2.10.0
[PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag
From: Lukas P Functions that print git object information may require that the gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent print_signature_buffer from being called if flag is set. Signed-off-by: Lukas P --- gpg-interface.h | 1 + tag.c | 5 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gpg-interface.h b/gpg-interface.h index ea68885..85dc982 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -3,6 +3,7 @@ #define GPG_VERIFY_VERBOSE 1 #define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_QUIET 4 struct signature_check { char *payload; diff --git a/tag.c b/tag.c index d1dcd18..291073f 100644 --- a/tag.c +++ b/tag.c @@ -3,6 +3,7 @@ #include "commit.h" #include "tree.h" #include "blob.h" +#include "gpg-interface.h" const char *tag_type = "tag"; @@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) ret = check_signature(buf, payload_size, buf + payload_size, size - payload_size, &sigc); - print_signature_buffer(&sigc, flags); + + if (!(flags & GPG_VERIFY_QUIET)) + print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); return ret; -- 2.10.0
[PATCH v2 2/5] ref-filter: add function to print single ref_array_item
From: Lukas P ref-filter functions are useful for printing git object information using a format specifier. However, some other modules may not want to use this functionality on a ref-array but only print a single item. Expose a format_ref function to create, pretty print and free individual ref-items. Signed-off-by: Lukas P --- ref-filter.c | 10 ++ ref-filter.h | 4 2 files changed, 14 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index bc551a7..e0aaf5f 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1655,6 +1655,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu putchar('\n'); } +void format_ref(const char *name, const unsigned char *sha1, const char *format, + unsigned kind) +{ + struct ref_array_item *ref_item; + ref_item = new_ref_array_item(name, sha1, 0); + ref_item->kind = kind; + show_ref_array_item(ref_item, format, 0); + free_array_item(ref_item); +} + /* If no sorting option is given, use refname to sort as default */ struct ref_sorting *ref_default_sorting(void) { diff --git a/ref-filter.h b/ref-filter.h index 14d435e..1ef7999 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -107,4 +107,8 @@ struct ref_sorting *ref_default_sorting(void); /* Function to parse --merged and --no-merged options */ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); +/* Pretty-print a single ref */ +void format_ref(const char *name, const unsigned char *sha1, const char *format, + unsigned kind); + #endif /* REF_FILTER_H */ -- 2.10.0
[PATCH v2 5/5] builtin/tag: add --format argument for tag -v
From: Lukas P Adding --format to git tag -v mutes the default output of the GPG verification and instead prints the formatted tag object. This allows callers to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. Caveat: The change adds a format specifier argument to the (*each_tag_name_fn) function pointer, i.e. delete_tag now receives this too, although it does not need it. Signed-off-by: Lukas P --- builtin/tag.c | 30 -- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 14f3b48..f53227e 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = { N_("git tag -d ..."), N_("git tag -l [-n[]] [--contains ] [--points-at ]" "\n\t\t[--format=] [--[no-]merged []] [...]"), - N_("git tag -v ..."), + N_("git tag -v [--format=] ..."), NULL }; @@ -66,9 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con } typedef int (*each_tag_name_fn)(const char *name, const char *ref, - const unsigned char *sha1); + const unsigned char *sha1, const char *fmt_pretty); -static int for_each_tag_name(const char **argv, each_tag_name_fn fn) +static int for_each_tag_name(const char **argv, each_tag_name_fn fn, + const char *fmt_pretty) { const char **p; char ref[PATH_MAX]; @@ -87,14 +88,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn) had_error = 1; continue; } - if (fn(*p, ref, sha1)) + if (fn(*p, ref, sha1, fmt_pretty)) had_error = 1; } return had_error; } static int delete_tag(const char *name, const char *ref, - const unsigned char *sha1) + const unsigned char *sha1, const char *fmt_pretty) { if (delete_ref(ref, sha1, 0)) return 1; @@ -103,9 +104,15 @@ 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 unsigned char *sha1, const char *fmt_pretty) { - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); + int flags; + flags = GPG_VERIFY_VERBOSE; + + if (fmt_pretty) + flags = GPG_VERIFY_QUIET; + + return verify_and_format_tag(sha1, name, fmt_pretty, flags); } static int do_sign(struct strbuf *buffer) @@ -424,9 +431,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (filter.merge_commit) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') - return for_each_tag_name(argv, delete_tag); - if (cmdmode == 'v') - return for_each_tag_name(argv, verify_tag); + return for_each_tag_name(argv, delete_tag, NULL); + if (cmdmode == 'v') { + if (format) + verify_ref_format(format); + return for_each_tag_name(argv, verify_tag, format); + } if (msg.given || msgfile) { if (msg.given && msgfile) -- 2.10.0
[PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag
From: Santiago Torres Callers of verify-tag may want to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. This is to avoid tag refs that point to an incorrect object. Add a --format parameter to git verify-tag to print the formatted tag object header in addition to or instead of the --verbose or --raw GPG verification output. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index de10198..a941053 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -12,12 +12,15 @@ #include #include "parse-options.h" #include "gpg-interface.h" +#include "ref-filter.h" static const char * const verify_tag_usage[] = { - N_("git verify-tag [-v | --verbose] ..."), + N_("git verify-tag [-v | --verbose] [--format=] ..."), NULL }; +static char *fmt_pretty; + static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; + if (fmt_pretty) { + verify_ref_format(fmt_pretty); + flags |= GPG_VERIFY_QUIET; + } + 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_and_format_tag(sha1, name, NULL, flags)) + else if (verify_and_format_tag(sha1, name, fmt_pretty, flags)) had_error = 1; } return had_error; -- 2.10.0
[PATCH v2 0/5] Add --format to tag verification
From: Santiago Torres This is the second iteration of [1], and as a result of the discussion in [2]. In this re-roll we: * Dropped the commit to move the format string parameter to a global variable on builtin/tag. We had to change the signature of for_each_name_fn to do this. * Fixed the signed-off-by lines to match the author/committer [0001] * Moved the GPG_VERIFY_QUIET flag check into tag.c instead of gpg_interface [0002] * Changed the exported function to "format_ref". This way, we can print a single formatted ref element. This also made the patch cleaner/easier to read. [0003] * We fixed style/formatting errors that we had introduced This patch applies to 2.10.0 and master. [1] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/ [2] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/ Lukas P (4): gpg-interface, tag: add GPG_VERIFY_QUIET flag ref-filter: add function to print single ref_array_item tag: add format specifier to gpg_verify_tag builtin/tag: add --format argument for tag -v Santiago Torres (1): builtin/verify-tag: add --format to verify-tag builtin/tag.c| 30 -- builtin/verify-tag.c | 16 ++-- gpg-interface.h | 1 + ref-filter.c | 10 ++ ref-filter.h | 4 tag.c| 22 +++--- tag.h| 4 ++-- 7 files changed, 66 insertions(+), 21 deletions(-) -- 2.10.0
[PATCH v3 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag
From: Lukas Puehringer Functions that print git object information may require that the gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent print_signature_buffer from being called if flag is set. Signed-off-by: Lukas Puehringer --- gpg-interface.h | 1 + tag.c | 5 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gpg-interface.h b/gpg-interface.h index ea68885..85dc982 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -3,6 +3,7 @@ #define GPG_VERIFY_VERBOSE 1 #define GPG_VERIFY_RAW 2 +#define GPG_VERIFY_QUIET 4 struct signature_check { char *payload; diff --git a/tag.c b/tag.c index d1dcd18..291073f 100644 --- a/tag.c +++ b/tag.c @@ -3,6 +3,7 @@ #include "commit.h" #include "tree.h" #include "blob.h" +#include "gpg-interface.h" const char *tag_type = "tag"; @@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) ret = check_signature(buf, payload_size, buf + payload_size, size - payload_size, &sigc); - print_signature_buffer(&sigc, flags); + + if (!(flags & GPG_VERIFY_QUIET)) + print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); return ret; -- 2.10.0
[PATCH v3 0/5] Add --format to tag verification
From: Santiago Torres This is the third iteration of [1][2], and as a result of the discussion in [3]. In this re-roll we: * Fixed all the signed-off-by's [0002] * Renamed the function format_ref to pretty_print_ref instead, which is a more descriptive name [0004] * Added the respective line for the new --format parameter in the documentation. [0005] * Added mention of the --format flag in the documentation files. * Fixed the function signatures, now they take an opaque void *cb_data pointer so it can be used in a more general way (by e.g., delete_tag). This patch applies to 2.10.0 and master. [1] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/ [2] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/ [3] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/ Lukas Puehringer (4): gpg-interface, tag: add GPG_VERIFY_QUIET flag ref-filter: add function to print single ref_array_item tag: add format specifier to gpg_verify_tag builtin/tag: add --format argument for tag -v Santiago Torres (1): builtin/verify-tag: add --format to verify-tag Documentation/git-tag.txt| 2 +- Documentation/git-verify-tag.txt | 2 +- builtin/tag.c| 34 +++--- builtin/verify-tag.c | 13 +++-- gpg-interface.h | 1 + ref-filter.c | 10 ++ ref-filter.h | 3 +++ tag.c| 22 +++--- tag.h| 4 ++-- 9 files changed, 67 insertions(+), 24 deletions(-) -- 2.10.0
[PATCH v3 4/5] builtin/verify-tag: add --format to verify-tag
From: Santiago Torres Callers of verify-tag may want to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. This is to avoid tag refs that point to an incorrect object. Add a --format parameter to git verify-tag to print the formatted tag object header in addition to or instead of the --verbose or --raw GPG verification output. Signed-off-by: Santiago Torres --- Documentation/git-verify-tag.txt | 2 +- builtin/verify-tag.c | 13 +++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt index d590edc..0b8075d 100644 --- a/Documentation/git-verify-tag.txt +++ b/Documentation/git-verify-tag.txt @@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags SYNOPSIS [verse] -'git verify-tag' ... +'git verify-tag' [--format=] ... DESCRIPTION --- diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index de10198..745b6a6 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -12,12 +12,14 @@ #include #include "parse-options.h" #include "gpg-interface.h" +#include "ref-filter.h" static const char * const verify_tag_usage[] = { - N_("git verify-tag [-v | --verbose] ..."), + N_("git verify-tag [-v | --verbose] [--format=] ..."), NULL }; + static int git_verify_tag_config(const char *var, const char *value, void *cb) { int status = git_gpg_config(var, value, cb); @@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; +char *fmt_pretty; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), + OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")), OPT_END() }; @@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; + if (fmt_pretty) { + verify_ref_format(fmt_pretty); + flags |= GPG_VERIFY_QUIET; + } + 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_and_format_tag(sha1, name, NULL, flags)) + else if (verify_and_format_tag(sha1, name, fmt_pretty, flags)) had_error = 1; } return had_error; -- 2.10.0
[PATCH v3 3/5] tag: add format specifier to gpg_verify_tag
From: Lukas P Calling functions for gpg_verify_tag() may desire to print relevant information about the header for further verification. Add an optional format argument to print any desired information after GPG verification. Signed-off-by: Lukas Puehringer --- builtin/tag.c| 2 +- builtin/verify-tag.c | 2 +- tag.c| 17 +++-- tag.h| 4 ++-- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 50e4ae5..14f3b48 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -105,7 +105,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) { - return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE); + return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 99f8148..de10198 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -51,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 (gpg_verify_tag(sha1, name, flags)) + else if (verify_and_format_tag(sha1, name, NULL, flags)) had_error = 1; } return had_error; diff --git a/tag.c b/tag.c index 291073f..d3512c0 100644 --- a/tag.c +++ b/tag.c @@ -4,6 +4,7 @@ #include "tree.h" #include "blob.h" #include "gpg-interface.h" +#include "ref-filter.h" const char *tag_type = "tag"; @@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, - unsigned flags) +int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags) { enum object_type type; char *buf; @@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, 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 : + name ? + name : 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 : + name ? + name : find_unique_abbrev(sha1, DEFAULT_ABBREV)); ret = run_gpg_verify(buf, size, flags); free(buf); + + if (fmt_pretty) + pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS); + return ret; } diff --git a/tag.h b/tag.h index a5721b6..896b9c2 100644 --- a/tag.h +++ b/tag.h @@ -17,7 +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); +extern int verify_and_format_tag(const unsigned char *sha1, const char *name, + const char *fmt_pretty, unsigned flags); #endif /* TAG_H */ -- 2.10.0
[PATCH v3 5/5] builtin/tag: add --format argument for tag -v
From: Lukas Puehringer Adding --format to git tag -v mutes the default output of the GPG verification and instead prints the formatted tag object. This allows callers to cross-check the tagname from refs/tags with the tagname from the tag object header upon GPG verification. The callback function for for_each_tag_name() didn't allow callers to pass custom data to their callback functions. Add a new opaque pointer to each_tag_name_fn's parameter to allow this. Signed-off-by: Lukas Puehringer --- Documentation/git-tag.txt | 2 +- builtin/tag.c | 34 +++--- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 7ecca8e..3bb5e3c 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -15,7 +15,7 @@ SYNOPSIS 'git tag' [-n[]] -l [--contains ] [--points-at ] [--column[=] | --no-column] [--create-reflog] [--sort=] [--format=] [--[no-]merged []] [...] -'git tag' -v ... +'git tag' -v [--format=] ... DESCRIPTION --- diff --git a/builtin/tag.c b/builtin/tag.c index 14f3b48..7730fd0 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = { N_("git tag -d ..."), N_("git tag -l [-n[]] [--contains ] [--points-at ]" "\n\t\t[--format=] [--[no-]merged []] [...]"), - N_("git tag -v ..."), + N_("git tag -v [--format=] ..."), NULL }; @@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con } typedef int (*each_tag_name_fn)(const char *name, const char *ref, - const unsigned char *sha1); + const unsigned char *sha1, void *cb_data); -static int for_each_tag_name(const char **argv, each_tag_name_fn fn) +static int for_each_tag_name(const char **argv, each_tag_name_fn fn, + void *cb_data) { const char **p; char ref[PATH_MAX]; int had_error = 0; unsigned char sha1[20]; + for (p = argv; *p; p++) { if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p) >= sizeof(ref)) { @@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn) had_error = 1; continue; } - if (fn(*p, ref, sha1)) + if (fn(*p, ref, sha1, cb_data)) had_error = 1; } return had_error; } static int delete_tag(const char *name, const char *ref, - const unsigned char *sha1) + const unsigned char *sha1, void *cb_data) { if (delete_ref(ref, sha1, 0)) return 1; @@ -103,9 +105,16 @@ 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 unsigned char *sha1, void *cb_data) { - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE); + int flags; +char *fmt_pretty = cb_data; + flags = GPG_VERIFY_VERBOSE; + + if (fmt_pretty) + flags = GPG_VERIFY_QUIET; + + return verify_and_format_tag(sha1, name, fmt_pretty, flags); } static int do_sign(struct strbuf *buffer) @@ -334,7 +343,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) struct strbuf err = STRBUF_INIT; struct ref_filter filter; static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting; - const char *format = NULL; + char *format = NULL; struct option options[] = { OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'), { OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"), @@ -424,9 +433,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (filter.merge_commit) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') - return for_each_tag_name(argv, delete_tag); - if (cmdmode == 'v') - return for_each_tag_name(argv, verify_tag); + return for_each_tag_name(argv, delete_tag, NULL); + if (cmdmode == 'v') { + if (format) + verify_ref_format(format); + return for_each_tag_name(argv, verify_tag, format); + } if (msg.given || msgfile) { if (msg.given && msgfile) -- 2.10.0
[PATCH v3 2/5] ref-filter: add function to print single ref_array_item
From: Lukas Puehringer ref-filter functions are useful for printing git object information using a format specifier. However, some other modules may not want to use this functionality on a ref-array but only print a single item. Expose a format_ref function to create, pretty print and free individual ref-items. Signed-off-by: Lukas Puehringer --- ref-filter.c | 10 ++ ref-filter.h | 3 +++ 2 files changed, 13 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index bc551a7..ee3ed67 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1655,6 +1655,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu putchar('\n'); } +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format, unsigned kind) +{ + struct ref_array_item *ref_item; + ref_item = new_ref_array_item(name, sha1, 0); + ref_item->kind = kind; + show_ref_array_item(ref_item, format, 0); + free_array_item(ref_item); +} + /* If no sorting option is given, use refname to sort as default */ struct ref_sorting *ref_default_sorting(void) { diff --git a/ref-filter.h b/ref-filter.h index 14d435e..3d23090 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void); /* Function to parse --merged and --no-merged options */ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); +void pretty_print_ref(const char *name, const unsigned char *sha1, + const char *format, unsigned kind); + #endif /* REF_FILTER_H */ -- 2.10.0
[PATCH/RFC] builtin/tag: Changes argument format for verify
From: Santiago Torres The verify tag function converts the commit sha1 to hex and passes it as a command-line argument to builtin/verify-tag. Given that builtin/verify-tag already resolves the ref name sha1 equivalent, the sha1 to hex_sha1 conversion is unnecessary and the ref-name can be used instead. Signed-off-by: Santiago Torres --- builtin/tag.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..5de1161 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -105,8 +105,7 @@ 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); + "-v", name, NULL}; if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD)) return error(_("could not verify the tag '%s'"), name); -- 2.7.0.435.g70bd996.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] builtin/tag.c: move PGP verification inside builtin.
From: Santiago Torres The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification instide the tag builtin instead. Signed-off-by: Santiago Torres --- builtin/tag.c | 44 ++-- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..be5d7c7 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -30,6 +30,27 @@ static const char * const git_tag_usage[] = { static unsigned int colopts; +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) +{ + struct signature_check sigc; + int len; + int ret; + + memset(&sigc, 0, sizeof(sigc)); + + len = parse_signature(buf, size); + + if (size == len) { + write_in_full(1, buf, len); + } + + ret = check_signature(buf, len, buf + len, size - len, &sigc); + print_signature_buffer(&sigc, flags); + + signature_check_clear(&sigc); + return ret; +} + static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format) { struct ref_array array; @@ -104,13 +125,24 @@ 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; + enum object_type type; + unsigned long size; + const char* buf; + 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, typename(type)); + + buf = read_sha1_file(sha1, &type, &size); + if (!buf) + return error("%s: unable to read file.", name); + + ret = run_gpg_verify(buf, size, 0); + + return ret; } static int do_sign(struct strbuf *buffer) -- 2.7.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] tag.c: move PGP verification code from plumbing
From: Santiago Torres The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. To do this, the run_pgp_verify() and verify_tag() functions are moved to tag.c. The definition of verify_tag was changed to support extra arguments that match the builtin/tag and builtin/verify-tag modules. The SIGPIPE ignore call in tag-verify was also moved to a more sensible place, now that both modules need it. The function name was also changed to pgp_verify_tag to avoid conflicts with mktag.c's. Signed-off-by: Santiago Torres --- builtin/tag.c| 26 --- builtin/verify-tag.c | 60 ++-- gpg-interface.c | 6 ++ tag.c| 48 + tag.h| 2 ++ 5 files changed, 72 insertions(+), 70 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..af8f3ba 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -65,9 +65,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con } typedef int (*each_tag_name_fn)(const char *name, const char *ref, - const unsigned char *sha1); + const unsigned char *sha1, unsigned flags); -static int for_each_tag_name(const char **argv, each_tag_name_fn fn) +static int for_each_tag_name(const char **argv, each_tag_name_fn fn, + unsigned flags) { const char **p; char ref[PATH_MAX]; @@ -86,32 +87,22 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn) had_error = 1; continue; } - if (fn(*p, ref, sha1)) + if (fn(*p, ref, sha1, flags)) had_error = 1; } return had_error; } static int delete_tag(const char *name, const char *ref, - const unsigned char *sha1) + const unsigned char *sha1, unsigned flags) { - if (delete_ref(ref, sha1, 0)) + if (delete_ref(ref, sha1, flags)) return 1; printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, DEFAULT_ABBREV)); return 0; } -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; -} static int do_sign(struct strbuf *buffer) { @@ -424,9 +415,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (filter.merge_commit) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') - return for_each_tag_name(argv, delete_tag); + return for_each_tag_name(argv, delete_tag, 0); if (cmdmode == 'v') - return for_each_tag_name(argv, verify_tag); + return for_each_tag_name(argv, pgp_verify_tag, + GPG_VERIFY_VERBOSE); if (msg.given || msgfile) { if (msg.given && msgfile) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 00663f6..2e6a175 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -18,55 +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; - int len; - int ret; - - memset(&sigc, 0, sizeof(sigc)); - - len = parse_signature(buf, size); - - if (size == len) { - if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, len); - return error("no signature found"); - } - - ret = check_signature(buf, len, buf + len, size - len, &sigc); - print_signature_buffer(&sigc, flags); - - signature_check_clear(&sigc); - return ret; -} - -static int verify_tag(const char *name, 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.", -
[PATCH v3 0/4] tag: move PGP verification code to tag.c
This is a follow up of [1] and [2]: v3 (this): Thanks Eric, Jeff, for the feedback. * I separated the patch in multiple sub-patches. * I compared the behavior of previous git tag -v and git verify-tag invocations to make sure the behavior is the same * I dropped the multi-line comment, as suggested. * I fixed the issue with the missing brackets in the while (this is now detected by the test). v2: * I moved the pgp-verification code to tag.c * I added extra arguments so git tag -v and git verify-tag both work with the same function * Relocated the SIGPIPE handling code in verify-tag to gpg-interface v1: The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. This applies on v2.8.0. Thanks! -Santiago [1] http://git.661346.n2.nabble.com/PATCH-RFC-builtin-tag-c-move-PGP-verification-inside-builtin-td7651529.html#a7651547 [2] http://git.661346.n2.nabble.com/PATCH-tag-c-move-PGP-verification-code-from-plumbing-td7651562.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] t/t7030-verify-tag.sh: Adds validation for multiple tags
From: Santiago Torres The verify-tag command supports mutliple tag names as an argument. However, no previous tests try to verify multiple tags at once. This test runs the verify-tag command against three trusted tags (created previously), and ensures that: 1) Three tags are verified appropriately (grep GOODSIG) and 2) The three tags verified are indeed differently (uniq -u) Signed-off-by: Santiago Torres --- t/t7030-verify-tag.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 4608e71..5918f86 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -112,4 +112,12 @@ test_expect_success GPG 'verify signatures with --raw' ' ) ' +test_expect_success GPG 'verify multiple tags' ' + git verify-tag -v --raw fourth-signed sixth-signed seventh-signed 2>actual 1> tagnames && + grep -c "GOODSIG" actual > count && + ! grep "BADSIG" actual && + grep -E "tag\ .*" tagnames | uniq -u | wc - l | grep "3" +' + + test_done -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] tag: use pgp_verify_function in tag -v call
From: Santiago Torres Instead of running the verify-tag plumbing command, we use the pgp_verify_tag(). This avoids the usage of an extra fork call. To do this, we extend the number of parameters that tag.c takes, and verify-tag passes. Redundant calls done in the pgp_verify_tag function are removed. Signed-off-by: Santiago Torres --- Notes: - In this version I fixed the issues with the brackets (the old patch doesn't work in this case due to the new test. builtin/tag.c| 28 +--- builtin/verify-tag.c | 14 -- tag.c| 7 ++- tag.h| 3 ++- 4 files changed, 25 insertions(+), 27 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..3dffdff 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -65,9 +65,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con } typedef int (*each_tag_name_fn)(const char *name, const char *ref, - const unsigned char *sha1); + const unsigned char *sha1, unsigned flags); -static int for_each_tag_name(const char **argv, each_tag_name_fn fn) +static int for_each_tag_name(const char **argv, each_tag_name_fn fn, + unsigned flags) { const char **p; char ref[PATH_MAX]; @@ -86,33 +87,21 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn) had_error = 1; continue; } - if (fn(*p, ref, sha1)) + if (fn(*p, ref, sha1, flags)) had_error = 1; } return had_error; } static int delete_tag(const char *name, const char *ref, - const unsigned char *sha1) + const unsigned char *sha1, unsigned flags) { - if (delete_ref(ref, sha1, 0)) + if (delete_ref(ref, sha1, flags)) return 1; printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, DEFAULT_ABBREV)); return 0; } -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; -} - static int do_sign(struct strbuf *buffer) { return sign_buffer(buffer, buffer, get_signing_key()); @@ -424,9 +413,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (filter.merge_commit) die(_("--merged and --no-merged option are only allowed with -l")); if (cmdmode == 'd') - return for_each_tag_name(argv, delete_tag); + return for_each_tag_name(argv, delete_tag, 0); if (cmdmode == 'v') - return for_each_tag_name(argv, verify_tag); + return for_each_tag_name(argv, pgp_verify_tag, + GPG_VERIFY_VERBOSE); if (msg.given || msgfile) { if (msg.given && msgfile) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index f776778..8abc357 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -30,6 +30,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + unsigned char sha1[20]; + const char *name; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), @@ -46,8 +48,16 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; - while (i < argc) - if (pgp_verify_tag(argv[i++], flags)) + while (i < argc) { + name = argv[i++]; + if (get_sha1(name, sha1)) { + error("tag '%s' not found.", name); had_error = 1; + } + + if (pgp_verify_tag(name, NULL, sha1, flags)) + had_error = 1; + + } return had_error; } diff --git a/tag.c b/tag.c index 918ae39..2a0b24c 100644 --- a/tag.c +++ b/tag.c @@ -29,18 +29,15 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -int pgp_verify_tag(const char *name, unsigned flags) +int pgp_verify_tag(const char *name, const char *ref, + const unsi
[PATCH v3 3/4] builtin/verify-tag: move verification code to tag.c
From: Santiago Torres The PGP verification routine for tags could be accessed by other commands that require it. We do this by moving it to the common tag.c code. We rename the verify_tag() function to pgp_verify_tag() to avoid conflicts with the mktag.c function. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 51 +-- tag.c| 50 ++ tag.h| 1 + 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 77f070a..f776778 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -18,55 +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; - int len; - int ret; - - memset(&sigc, 0, sizeof(sigc)); - - len = parse_signature(buf, size); - - if (size == len) { - if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, len); - return error("no signature found"); - } - - ret = check_signature(buf, len, buf + len, size - len, &sigc); - print_signature_buffer(&sigc, flags); - - signature_check_clear(&sigc); - return ret; -} - -static int verify_tag(const char *name, 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)); - - buf = read_sha1_file(sha1, &type, &size); - if (!buf) - return error("%s: unable to read file.", name); - - 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); @@ -96,7 +47,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) flags |= GPG_VERIFY_VERBOSE; while (i < argc) - if (verify_tag(argv[i++], flags)) + if (pgp_verify_tag(argv[i++], flags)) had_error = 1; return had_error; } diff --git a/tag.c b/tag.c index d72f742..918ae39 100644 --- a/tag.c +++ b/tag.c @@ -6,6 +6,56 @@ const char *tag_type = "tag"; +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) +{ + struct signature_check sigc; + int payload_size; + int ret; + + memset(&sigc, 0, sizeof(sigc)); + + payload_size = parse_signature(buf, size); + + if (size == payload_size) { + write_in_full(1, buf, payload_size); + return error("No PGP signature found in this tag!"); + } + + 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 pgp_verify_tag(const char *name, unsigned flags) +{ + + enum object_type type; + unsigned long size; + unsigned char sha1[20]; + char* buf; + 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)); + + buf = read_sha1_file(sha1, &type, &size); + if (!buf) + return error("%s: unable to read file.", name); + + 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 f4580ae..09e71f9 100644 --- a/tag.h +++ b/tag.h @@ -17,5 +17,6 @@ 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 pgp_verify_tag(const char *name, unsigned flags); #endif /* TAG_H */ -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
From: Santiago Torres The verify_signed_buffer comand might cause a SIGPIPE signal 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 on the builtin/gpg-verify.c command to avoid this issue. However, any other caller who wanted to use the verify_signed_buffer command would have to include this signal call. Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the verify_signed_buffer call (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 --- Notes: I dropped the multiline comment altogheter. builtin/verify-tag.c | 3 --- gpg-interface.c | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 00663f6..77f070a 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 3dc2fe3..c1f6b2d 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -232,6 +232,8 @@ int verify_signed_buffer(const char *payload, size_t payload_size, if (gpg_output) gpg.err = -1; args_gpg[3] = path; + + sigchain_push(SIGPIPE, SIG_IGN); if (start_command(&gpg)) { unlink(path); return error(_("could not run gpg.")); @@ -250,6 +252,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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/6] builtin/verify-tag: move verification code to tag.c
From: Santiago Torres The PGP verification routine for tags could be accessed by other commands that require it. We do this by moving it to the common tag.c code. We rename the verify_tag() function to gpg_verify_tag() to avoid conflicts with the mktag.c function. Signed-off-by: Santiago Torres --- Note: In this version, I just blndly moved code around and renamed the function's name for ease of review. The following patch only renames variables. builtin/verify-tag.c | 51 +-- tag.c| 49 + tag.h| 1 + 3 files changed, 51 insertions(+), 50 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 77f070a..7e36d53 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -18,55 +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; - int len; - int ret; - - memset(&sigc, 0, sizeof(sigc)); - - len = parse_signature(buf, size); - - if (size == len) { - if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, len); - return error("no signature found"); - } - - ret = check_signature(buf, len, buf + len, size - len, &sigc); - print_signature_buffer(&sigc, flags); - - signature_check_clear(&sigc); - return ret; -} - -static int verify_tag(const char *name, 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)); - - buf = read_sha1_file(sha1, &type, &size); - if (!buf) - return error("%s: unable to read file.", name); - - 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); @@ -96,7 +47,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) flags |= GPG_VERIFY_VERBOSE; while (i < argc) - if (verify_tag(argv[i++], flags)) + if (gpg_verify_tag(argv[i++], flags)) had_error = 1; return had_error; } diff --git a/tag.c b/tag.c index d72f742..81e86e6 100644 --- a/tag.c +++ b/tag.c @@ -6,6 +6,55 @@ const char *tag_type = "tag"; +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) +{ + struct signature_check sigc; + int len; + int ret; + + memset(&sigc, 0, sizeof(sigc)); + + len = parse_signature(buf, size); + + if (size == len) { + if (flags & GPG_VERIFY_VERBOSE) + write_in_full(1, buf, len); + return error("no signature found"); + } + + ret = check_signature(buf, len, buf + len, size - len, &sigc); + print_signature_buffer(&sigc, flags); + + signature_check_clear(&sigc); + return ret; +} + +int gpg_verify_tag(const char *name, 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)); + + buf = read_sha1_file(sha1, &type, &size); + if (!buf) + return error("%s: unable to read file.", name); + + 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 f4580ae..877f180 100644 --- a/tag.h +++ b/tag.h @@ -17,5 +17,6 @@ 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 char *name, unsigned flags); #endif /* TAG_H */ -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/6] tag.c: Replace varialbe name for readability
From: Santiago Torres The run_gpg_verify function has two variables size, and len. This may come off as confusing when reading the code. We clarify which one pertains to the length of the tag headers by renaming len to payload_length. Signed-off-by: Santiago Torres --- Note: this used to be part of the previous patch on v3. I moved the renaming part to a single patch to simplify the review. tag.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tag.c b/tag.c index 81e86e6..f6443db 100644 --- a/tag.c +++ b/tag.c @@ -9,20 +9,21 @@ const char *tag_type = "tag"; static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) { struct signature_check sigc; - int len; + int payload_length; int ret; memset(&sigc, 0, sizeof(sigc)); - len = parse_signature(buf, size); + payload_length = parse_signature(buf, size); - if (size == len) { + if (size == payload_length) { if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, len); + write_in_full(1, buf, payload_length); return error("no signature found"); } - ret = check_signature(buf, len, buf + len, size - len, &sigc); + ret = check_signature(buf, payload_length, buf + payload_length, + size - payload_length, &sigc); print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 6/6] tag.c: Change gpg_verify_tag argument to sha1
From: Santiago Torres The gpg_verify_tag function resolves the ref for any existing object. However, git tag -v resolves to only tag-refs. We can provide support for sha1 by moving the refname resolution code out of gpg_verify_tag and allow for the object's sha1 as an argument. Signed-off-by: Santiago Torres --- builtin/tag.c| 2 +- builtin/verify-tag.c | 9 +++-- tag.c| 12 tag.h| 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index f4450f8..398c892 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -104,7 +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) { - return gpg_verify_tag(name, GPG_VERIFY_VERBOSE); + return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 7e36d53..2ff01d8 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -30,6 +30,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + unsigned char sha1[20]; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), @@ -46,8 +47,12 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; - while (i < argc) - if (gpg_verify_tag(argv[i++], flags)) + while (i < argc) { + if (get_sha1(argv[i++], sha1)) + return error("tag '%s' not found.", argv[i]); + + if (gpg_verify_tag(sha1, flags)) had_error = 1; + } return had_error; } diff --git a/tag.c b/tag.c index f6443db..8ac9de5 100644 --- a/tag.c +++ b/tag.c @@ -30,25 +30,21 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -int gpg_verify_tag(const char *name, unsigned flags) +int gpg_verify_tag(const unsigned char *sha1, 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)); + return error("cannot verify a non-tag object of type %s.", + typename(type)); buf = read_sha1_file(sha1, &type, &size); if (!buf) - return error("%s: unable to read file.", name); + return error("unable to read file."); ret = run_gpg_verify(buf, size, flags); diff --git a/tag.h b/tag.h index 877f180..cb643b9 100644 --- a/tag.h +++ b/tag.h @@ -17,6 +17,6 @@ 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 char *name, unsigned flags); +extern int gpg_verify_tag(const unsigned char *sha1, unsigned flags); #endif /* TAG_H */ -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/6] tag: move PGP verification code to tag.c
This is a follow up of [1], [2], and [3]: v4 (this): Thanks Eric, Peff, and Hannes for the feedback. * I relocated the sigchain_push call so it comes after the error on gpg-interface (thanks Hannnes for catching this). * I updated the unit test to match the discussion on [3]. Now it generates the expected output of the tag on the fly for comparison. (This is just copy and paste from [3], but I verified that it works by breaking the while) * I split moving the code and renaming the variables into two patches so these are easier to review. * I used an adapter on builtin/tag.c instead of redefining all the fn* declarations everywhere. This introduces an issue with the way git tag -v resolves refnames though. I added a new commit to restore the previous behavior of git-tag. I'm not sure if I should've split this into two commits though. v3: Thanks Eric, Jeff, for the feedback. * I separated the patch in multiple sub-patches. * I compared the behavior of previous git tag -v and git verify-tag invocations to make sure the behavior is the same * I dropped the multi-line comment, as suggested. * I fixed the issue with the missing brackets in the while (this is now detected by the test). v2: * I moved the pgp-verification code to tag.c * I added extra arguments so git tag -v and git verify-tag both work with the same function * Relocated the SIGPIPE handling code in verify-tag to gpg-interface v1: The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. This applies on v2.8.0. Thanks! -Santiago [1] http://git.661346.n2.nabble.com/PATCH-RFC-builtin-tag-c-move-PGP-verification-inside-builtin-td7651529.html#a7651547 [2] http://git.661346.n2.nabble.com/PATCH-tag-c-move-PGP-verification-code-from-plumbing-td7651562.html [3] http://git.661346.n2.nabble.com/PATCH-v3-0-4-tag-move-PGP-verification-code-to-tag-c-td7652334.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 5/6] tag: use pgp_verify_function in tag -v call
From: Santiago Torres Instead of running the verify-tag plumbing command, we use the pgp_verify_tag(). This avoids the usage of an extra fork call. To do this, we extend the number of parameters that tag.c takes, and verify-tag passes. Redundant calls done in the pgp_verify_tag function are removed. Signed-off-by: Santiago Torres --- Note: This follows Peff's suggestion to use an adapter, instead of changing the the fn* pointer structure to match gpg_verify_tag. builtin/tag.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..f4450f8 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(name, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/6] t/t7030-verify-tag.sh: Adds validation for multiple tags
From: Santiago Torres The verify-tag command supports mutliple tag names as an argument. However, no previous tests try to verify multiple tags at once. This test runs the verify-tag command against three tags separately and then compares the result against the invocation with the same three tags as an argument. The results shouldn't differ. Signed-off-by: Santiago Torres --- Note: In this version we don't check or uniq for the verify output, but instead compare against an invocation for each of the three tags indivdually. t/t7030-verify-tag.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 4608e71..f9161332 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -112,4 +112,16 @@ 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 GOODSIG expect.stderr && + git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 && + grep GOODSIG actual.stderr && + test_cmp expect.stdout actual.stdout && + test_cmp expect.stderr actual.stderr +' + test_done -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
From: Santiago Torres The verify_signed_buffer comand might cause a SIGPIPE signal 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 on the builtin/verify-tag.c command to avoid this issue. However, any other caller who wanted to use the verify_signed_buffer command would have to include this signal call. Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the verify_signed_buffer call (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 --- Note: On this version i moved the sigchain_push below the return error in the same way that sign_buffer does(). Thanks to Johanes Sixt for catching this. 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 00663f6..77f070a 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 3dc2fe3..2259938 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 6/6] tag: use gpg_verify_function in tag -v call
From: Santiago Torres Instead of running the verify-tag plumbing command, we use the gpg_verify_tag() function within the verify_tag function to avoid doing an additional fork call. Signed-off-by: Santiago Torres --- builtin/tag.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..398c892 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, GPG_VERIFY_VERBOSE); } static int do_sign(struct strbuf *buffer) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/6] builtin/verify-tag: change variable name for readability
From: Santiago Torres The run_gpg_verify function has two variables size, and len. This may come off as confusing when reading the code. We clarify which one pertains to the length of the tag headers by renaming len to payload_length. Signed-off-by: Santiago Torres --- 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 77f070a..1ca9a05 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; + int 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/6] builtin/verify-tag: replace name argument with sha1
From: Santiago Torres This change is meant to prepare verify_tag for libification. Many existing modules/commands already do the refname to sha1 resolution, so should avoid resolving the refname twice. To avoid breaking builtin/verify-tag, we move the refname resolution outside of the verify_tag() call. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 1ca9a05..7a7c376 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -42,25 +42,23 @@ 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, unsigned flags) { enum object_type type; - unsigned char sha1[20]; char *buf; + char *hex_sha1; unsigned long size; int ret; - if (get_sha1(name, sha1)) - return error("tag '%s' not found.", name); - + hex_sha1 = sha1_to_hex(sha1); 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)); + hex_sha1, 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.", hex_sha1); ret = run_gpg_verify(buf, size, flags); @@ -80,6 +78,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + unsigned char sha1[20]; + const char *name; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), @@ -96,8 +96,15 @@ 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) { + name = argv[i++]; + if (get_sha1(name, sha1)) { + error("tag '%s' not found.", name); had_error = 1; + continue; + } + if (verify_tag(sha1, flags)) + had_error = 1; + } return had_error; } -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
From: Santiago Torres The verify_signed_buffer comand might cause a SIGPIPE signal 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 on the builtin/verify-tag.c command to avoid this issue. However, any other caller who wanted to use the verify_signed_buffer command would have to include this signal call. Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the verify_signed_buffer call (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 --- 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 00663f6..77f070a 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 3dc2fe3..2259938 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/6] t7030-verify-tag: Adds validation for multiple tags
From: Santiago Torres The verify-tag command supports multiple tag names as an argument. However, existing tests only test for invocation with a single tag, so we add a test invoking with multiple tags. Helped-by: Jeff King Signed-off-by: Santiago Torres --- t/t7030-verify-tag.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 4608e71..c01621a 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -112,4 +112,16 @@ 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 -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 5/6] builtin/verify-tag: move verification code to tag.c
From: Santiago Torres The PGP verification routine for tags could be accessed by other commands that require it. We do this by moving it to the common tag.c module. We rename the verify_tag() function to gpg_verify_tag() to avoid conflicts with the mktag.c function. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 50 +- tag.c| 48 tag.h| 1 + 3 files changed, 50 insertions(+), 49 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 7a7c376..e9a2005 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -18,54 +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; - int 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, unsigned flags) -{ - enum object_type type; - char *buf; - char *hex_sha1; - unsigned long size; - int ret; - - hex_sha1 = sha1_to_hex(sha1); - type = sha1_object_info(sha1, NULL); - if (type != OBJ_TAG) - return error("%s: cannot verify a non-tag object of type %s.", - hex_sha1, typename(type)); - - buf = read_sha1_file(sha1, &type, &size); - if (!buf) - return error("%s: unable to read file.", hex_sha1); - - 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); @@ -103,7 +55,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) had_error = 1; continue; } - if (verify_tag(sha1, flags)) + if (gpg_verify_tag(sha1, flags)) had_error = 1; } return had_error; diff --git a/tag.c b/tag.c index d72f742..3f7669f 100644 --- a/tag.c +++ b/tag.c @@ -6,6 +6,54 @@ const char *tag_type = "tag"; +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) +{ + struct signature_check sigc; + int 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, unsigned flags) +{ + enum object_type type; + char *buf; + char *hex_sha1; + unsigned long size; + int ret; + + hex_sha1 = sha1_to_hex(sha1); + type = sha1_object_info(sha1, NULL); + if (type != OBJ_TAG) + return error("%s: cannot verify a non-tag object of type %s.", + hex_sha1, typename(type)); + + buf = read_sha1_file(sha1, &type, &size); + if (!buf) + return error("%s: unable to read file.", hex_sha1); + + 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 f4580ae..cb643b9 100644 --- a/tag.h +++ b/tag.h @@ -17,5 +17,6 @@ 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, unsigned flags); #endif /* TAG_H */ -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/6] tag: move PGP verification code to tag.c
From: Santiago Torres v5 (this): Added helpful feedback by Eric * Reordering of the patches, to avoid temporal inclusion of a regression * Fix typos here and there. * Review commit messages, as some weren't representative of what the patches were doing anymore. * Updated t7030 to include Peff's suggestion, and added a helped-by line here as it was mostly Peff's code. * Updated the error-handling/printing issues that were introduced when. libifying the verify_tag function. v4: Thanks Eric, Jeff, and Hannes for the feedback. * I relocated the sigchain_push call so it comes after the error on gpg-interface (thanks Hannnes for catching this). * I updated the unit test to match the discussion on [3]. Now it generates the expected output of the tag on the fly for comparison. (This is just copy and paste from [3], but I verified that it works by breaking the while) * I split moving the code and renaming the variables into two patches so these are easier to review. * I used an adapter on builtin/tag.c instead of redefining all the fn* declarations everywhere. This introduces an issue with the way git tag -v resolves refnames though. I added a new commit to restore the previous behavior of git-tag. I'm not sure if I should've split this into two commits though. v3: Thanks Eric, Jeff, for the feedback. * I separated the patch in multiple sub-patches. * I compared the behavior of previous git tag -v and git verify-tag invocations to make sure the behavior is the same * I dropped the multi-line comment, as suggested. * I fixed the issue with the missing brackets in the while (this is now detected by the test). v2: * I moved the pgp-verification code to tag.c * I added extra arguments so git tag -v and git verify-tag both work with the same function * Relocated the SIGPIPE handling code in verify-tag to gpg-interface v1: The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. This applies on v2.8.0. Thanks! -Santiago Santiago Torres (6): builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface t7030-verify-tag: Adds validation for multiple tags builtin/verify-tag: change variable name for readability builtin/verify-tag: replace name argument with sha1 builtin/verify-tag: move verification code to tag.c tag: use gpg_verify_function in tag -v call builtin/tag.c | 8 +-- builtin/verify-tag.c | 65 +-- gpg-interface.c | 2 ++ t/t7030-verify-tag.sh | 12 ++ tag.c | 48 + tag.h | 1 + 6 files changed, 75 insertions(+), 61 deletions(-) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/6] Move PGP verification out of verify-tag
From: Santiago Torres This is a follow up of [1], [2], [3], [4], and [5]. patches 1/6, 2/6 and 3/6 are the same as the corresponding commits in pu. v6: * As Junio suggested, updated 4/6, to include the name argument and the ternary operator to provide more descriptive error messages. I propagated these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column on 4/6, the ternary operator is rather long. * Updated and reviewed the commit messages based on Eric and Junio's feedback v5: Added helpful feedback by Eric * Reordering of the patches, to avoid temporal inclusion of a regression * Fix typos here and there. * Review commit messages, as some weren't representative of what the patches were doing anymore. * Updated t7030 to include Peff's suggestion, and added a helped-by line here as it was mostly Peff's code. * Updated the error-handling/printing issues that were introduced when. libifying the verify_tag function. v4: Thanks Eric, Jeff, and Hannes for the feedback. * I relocated the sigchain_push call so it comes after the error on gpg-interface (thanks Hannnes for catching this). * I updated the unit test to match the discussion on [3]. Now it generates the expected output of the tag on the fly for comparison. (This is just copy and paste from [3], but I verified that it works by breaking the while) * I split moving the code and renaming the variables into two patches so these are easier to review. * I used an adapter on builtin/tag.c instead of redefining all the fn* declarations everywhere. This introduces an issue with the way git tag -v resolves refnames though. I added a new commit to restore the previous behavior of git-tag. I'm not sure if I should've split this into two commits though. v3: Thanks Eric, Jeff, for the feedback. * I separated the patch in multiple sub-patches. * I compared the behavior of previous git tag -v and git verify-tag invocations to make sure the behavior is the same * I dropped the multi-line comment, as suggested. * I fixed the issue with the missing brackets in the while (this is now detected by the test). v2: * I moved the pgp-verification code to tag.c * I added extra arguments so git tag -v and git verify-tag both work with the same function * Relocated the SIGPIPE handling code in verify-tag to gpg-interface v1: The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. This applies on v2.8.0. Thanks! -Santiago [1] http://thread.gmane.org/gmane.comp.version-control.git/287649 [2] http://thread.gmane.org/gmane.comp.version-control.git/289836 [3] http://thread.gmane.org/gmane.comp.version-control.git/290608 [4] http://thread.gmane.org/gmane.comp.version-control.git/290731 [5] http://thread.gmane.org/gmane.comp.version-control.git/290790 Santiago Torres (6): builtin/verify-tag.c: ignore SIGPIPE in gpg-interface t7030: test verifying multiple tags verify-tag: change variable name for readability verify-tag: add sha1 argument to verify_tag() verify-tag: move verification code to tag.c tag -v: verfy directly rather than exec-ing verify-tag builtin/tag.c | 8 +-- builtin/verify-tag.c | 64 --- gpg-interface.c | 2 ++ t/t7030-verify-tag.sh | 13 +++ tag.c | 49 +++ tag.h | 3 ++- 6 files changed, 77 insertions(+), 62 deletions(-) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
From: Santiago Torres 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 00663f6..77f070a 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 3dc2fe3..2259938 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/6] verify-tag: change variable name for readability
From: Santiago Torres 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_length. 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 77f070a..010353c 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; + int 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/6] verify-tag: move verification code to tag.c
From: Santiago Torres 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 --- builtin/verify-tag.c | 51 +-- tag.c| 49 + tag.h| 3 ++- 3 files changed, 52 insertions(+), 51 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 1d1c5c2..4e3b643 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -18,55 +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; - int 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 *report_name, - 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.", - report_name ? report_name : find_unique_abbrev(sha1, DEFAULT_ABBREV), - typename(type)); - - buf = read_sha1_file(sha1, &type, &size); - if (!buf) - return error("%s: unable to read file.", - report_name ? report_name : 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); @@ -103,7 +54,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) error("tag '%s' not found.", name); had_error = 1; } - 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 d72f742..e7f22c6 100644 --- a/tag.c +++ b/tag.c @@ -6,6 +6,55 @@ const char *tag_type = "tag"; +static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) +{ + struct signature_check sigc; + int 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 *report_name, + 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.", + report_name ? report_name : find_unique_abbrev(sha1, DEFAULT_ABBREV), + typename(type)); + + buf = read_sha1_file(sha1, &type, &size); + if (!buf) + return error("%s: unable to read file.", + report_name ? report_name : 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 f4580ae..1a8d123 100644 --- a/tag.h +++ b/tag.h @@ -17,5 +17,6 @@ extern int parse_tag_buffer(struct tag *item,
[PATCH v6 2/6] t7030: test verifying multiple tags
From: Santiago Torres 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 4608e71..07079a4 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 -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/6] verify-tag: add sha1 argument to verify_tag()
From: Santiago Torres The current interface of verify_tag() resolves reference names to SHA1, which might be redundant as future callers may resolve the refname to SHA1 beforehand. Add a SHA1 parameter to use instead of the name parameter. We also replace the name argument to report_name and use it for error reporting only. Helped-by: Junio C Hamano Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 010353c..1d1c5c2 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -42,25 +42,24 @@ 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 *report_name, + 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)); + report_name ? report_name : 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.", + report_name ? report_name : find_unique_abbrev(sha1, DEFAULT_ABBREV)); ret = run_gpg_verify(buf, size, flags); @@ -80,6 +79,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + unsigned char sha1[20]; + const char *name; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), @@ -96,8 +97,14 @@ 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) { + name = argv[i++]; + if (get_sha1(name, sha1)) { + error("tag '%s' not found.", name); had_error = 1; + } + else if (verify_tag(sha1, name, flags)) + had_error = 1; + } return had_error; } -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 6/6] tag -v: verfy directly rather than exec-ing verify-tag
From: Santiago Torres tag -v forks into verify-tag, which only calls gpg_verify_tag(). Instead of forking to verify-tag, call gpg_verify_tag directly(). Helped-by: Eric Sunshine Signed-off-by: Santiago Torres --- builtin/tag.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..7b2918e 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) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/6] Move PGP verification out of verify-tag
From: Santiago Torres This is a follow up of [1], [2], [3], [4], [5], [6]. patches 1/6, 2/6, are the same as the corresponding commits in pu. v7: Mostly style/clarity changes mostly. Thanks Peff, Eric and Junio for the feedback! In summary: * Eric pointed out issues with 3/6's commit message. It doesn't match the one in pu though. I also took the opportunity to update payload_size to a size_t as Peff suggested. * 4/6 I updated report_name to name_to_report, I updated the commit message and addressed some nits in the code, one of the fixes removed all three nits that Eric pointed out. I updated 5/6 to match these changes * I gave the commit message on 6/6 another go. v6: * As Junio suggested, updated 4/6, to include the name argument and the ternary operator to provide more descriptive error messages. I propagated these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column on 4/6, the ternary operator is rather long. * Updated and reviewed the commit messages based on Eric and Junio's feedback v5: Added helpful feedback by Eric * Reordering of the patches, to avoid temporal inclusion of a regression * Fix typos here and there. * Review commit messages, as some weren't representative of what the patches were doing anymore. * Updated t7030 to include Peff's suggestion, and added a helped-by line here as it was mostly Peff's code. * Updated the error-handling/printing issues that were introduced when. libifying the verify_tag function. v4: Thanks Eric, Jeff, and Hannes for the feedback. * I relocated the sigchain_push call so it comes after the error on gpg-interface (thanks Hannnes for catching this). * I updated the unit test to match the discussion on [3]. Now it generates the expected output of the tag on the fly for comparison. (This is just copy and paste from [3], but I verified that it works by breaking the while) * I split moving the code and renaming the variables into two patches so these are easier to review. * I used an adapter on builtin/tag.c instead of redefining all the fn* declarations everywhere. This introduces an issue with the way git tag -v resolves refnames though. I added a new commit to restore the previous behavior of git-tag. I'm not sure if I should've split this into two commits though. v3: Thanks Eric, Jeff, for the feedback. * I separated the patch in multiple sub-patches. * I compared the behavior of previous git tag -v and git verify-tag invocations to make sure the behavior is the same * I dropped the multi-line comment, as suggested. * I fixed the issue with the missing brackets in the while (this is now detected by the test). v2: * I moved the pgp-verification code to tag.c * I added extra arguments so git tag -v and git verify-tag both work with the same function * Relocated the SIGPIPE handling code in verify-tag to gpg-interface v1: The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. This applies on v2.8.0. Thanks! -Santiago [1] http://thread.gmane.org/gmane.comp.version-control.git/287649 [2] http://thread.gmane.org/gmane.comp.version-control.git/289836 [3] http://thread.gmane.org/gmane.comp.version-control.git/290608 [4] http://thread.gmane.org/gmane.comp.version-control.git/290731 [5] http://thread.gmane.org/gmane.comp.version-control.git/290790 [6] http://thread.gmane.org/gmane.comp.version-control.git/291780 Santiago Torres (6): builtin/verify-tag.c: ignore SIGPIPE in gpg-interface t7030: test verifying multiple tags verify-tag: Update run_gpg_verfy len variable name and type verify-tag: prepare verify_tag for libification verify-tag: move tag verification code to tag.c tag -v: verfy directly rather than exec-ing verify-tag builtin/tag.c | 8 +-- builtin/verify-tag.c | 62 +++ gpg-interface.c | 2 ++ t/t7030-verify-tag.sh | 13 +++ tag.c | 53 +++ tag.h | 2 ++ 6 files changed, 79 insertions(+), 61 deletions(-) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/6] t7030: test verifying multiple tags
From: Santiago Torres 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 4608e71..07079a4 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 -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
From: Santiago Torres 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 00663f6..77f070a 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 3dc2fe3..2259938 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 4/6] verify-tag: prepare verify_tag for libification
From: Santiago Torres 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 --- builtin/verify-tag.c | 27 ++- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index fa26e40..01e956f 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); @@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + unsigned char sha1[20]; + const char *name; const struct option verify_tag_options[] = { OPT__VERBOSE(&verbose, N_("print tag contents")), OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW), @@ -96,8 +101,12 @@ 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) { + 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; } -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 5/6] verify-tag: move tag verification code to tag.c
From: Santiago Torres 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 --- 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 01e956f..714c899 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); @@ -105,7 +52,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) 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 d72f742..ace619a 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; +} + +extern 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);
[PATCH v7 6/6] tag -v: verfy directly rather than exec-ing verify-tag
From: Santiago Torres 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 --- builtin/tag.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..7b2918e 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) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 3/6] verify-tag: update variable name and type
From: Santiago Torres 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 77f070a..fa26e40 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 2/6] t7030: test verifying multiple tags
From: Santiago Torres 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 4608e71..07079a4 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 -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 4/6] verify-tag: prepare verify_tag for libification
From: Santiago Torres 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 --- 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 fa26e40..a3d3a43 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; } -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 3/6] verify-tag: update variable name and type
From: Santiago Torres 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 77f070a..fa26e40 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 0/6] Move PGP verification out of verify-tag
From: Santiago Torres This is a follow up of [1], [2], [3], [4], [5], [6], and [7]. patches 1/6, 2/6, and 3/6, are the same as the corresponding commits in pu. v8: Minor nits, I decided to quickly reroll to drop the extern qualifier in tag.c: * Eric pointed out that we could block-scope the declaration of name and sha1 in b/verify-tag.c, for 4/6 * There was a typo in 6/6 * I dropped the extern qualifier in tag.c for 5/6 as suggested by Ramsay Jones[8] v7: Mostly style/clarity changes. Thanks Peff, Eric and Junio for the feedback! In summary: * Eric pointed out issues with 3/6's commit message. It doesn't match the one in pu though. I also took the opportunity to update payload_size to a size_t as Peff suggested. * 4/6 I updated report_name to name_to_report, I updated the commit message and addressed some nits in the code, one of the fixes removed all three nits that Eric pointed out. I updated 5/6 to match these changes * I gave the commit message on 6/6 another go. v6: * As Junio suggested, updated 4/6, to include the name argument and the ternary operator to provide more descriptive error messages. I propagated these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column on 4/6, the ternary operator is rather long. * Updated and reviewed the commit messages based on Eric and Junio's feedback v5: Added helpful feedback by Eric * Reordering of the patches, to avoid temporal inclusion of a regression * Fix typos here and there. * Review commit messages, as some weren't representative of what the patches were doing anymore. * Updated t7030 to include Peff's suggestion, and added a helped-by line here as it was mostly Peff's code. * Updated the error-handling/printing issues that were introduced when. libifying the verify_tag function. v4: Thanks Eric, Jeff, and Hannes for the feedback. * I relocated the sigchain_push call so it comes after the error on gpg-interface (thanks Hannnes for catching this). * I updated the unit test to match the discussion on [3]. Now it generates the expected output of the tag on the fly for comparison. (This is just copy and paste from [3], but I verified that it works by breaking the while) * I split moving the code and renaming the variables into two patches so these are easier to review. * I used an adapter on builtin/tag.c instead of redefining all the fn* declarations everywhere. This introduces an issue with the way git tag -v resolves refnames though. I added a new commit to restore the previous behavior of git-tag. I'm not sure if I should've split this into two commits though. v3: Thanks Eric, Jeff, for the feedback. * I separated the patch in multiple sub-patches. * I compared the behavior of previous git tag -v and git verify-tag invocations to make sure the behavior is the same * I dropped the multi-line comment, as suggested. * I fixed the issue with the missing brackets in the while (this is now detected by the test). v2: * I moved the pgp-verification code to tag.c * I added extra arguments so git tag -v and git verify-tag both work with the same function * Relocated the SIGPIPE handling code in verify-tag to gpg-interface v1: The verify tag function is just a thin wrapper around the verify-tag command. We can avoid one fork call by doing the verification inside the tag builtin instead. This applies on v2.8.0. Thanks! -Santiago [1] http://thread.gmane.org/gmane.comp.version-control.git/287649 [2] http://thread.gmane.org/gmane.comp.version-control.git/289836 [3] http://thread.gmane.org/gmane.comp.version-control.git/290608 [4] http://thread.gmane.org/gmane.comp.version-control.git/290731 [5] http://thread.gmane.org/gmane.comp.version-control.git/290790 [6] http://thread.gmane.org/gmane.comp.version-control.git/291780 [7] http://thread.gmane.org/gmane.comp.version-control.git/291887 [8] http://thread.gmane.org/gmane.comp.version-control.git/292029 Santiago Torres (6): builtin/verify-tag.c: ignore SIGPIPE in gpg-interface t7030: test verifying multiple tags verify-tag: update variable name and type verify-tag: prepare verify_tag for libification verify-tag: move tag verification code to tag.c tag -v: verify directly rather than exec-ing verify-tag builtin/tag.c | 8 +-- builtin/verify-tag.c | 61 ++- gpg-interface.c | 2 ++ t/t7030-verify-tag.sh | 13 +++ tag.c | 53 tag.h | 2 ++ 6 files changed, 78 insertions(+), 61 deletions(-) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 6/6] tag -v: verify directly rather than exec-ing verify-tag
From: Santiago Torres 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 --- builtin/tag.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..7b2918e 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) -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 5/6] verify-tag: move tag verification code to tag.c
From: Santiago Torres 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 --- 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 a3d3a43..99f8148 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 d72f742..8363a0e 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, fl
[PATCH v8 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
From: Santiago Torres 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 00663f6..77f070a 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 3dc2fe3..2259938 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); -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] builtin:tag:verify_tag: allow gpg output + pretty
From: Santiago Torres On the git tag -v code, there is a guard to suppress gpg output if a pretty format is provided. The rationale for this is that the gpg output *and* the pretty formats together may conflict with each other. However, both outputs are directed to different output streams and, as such, they can safely coexist. Drop the guard clause and use GPG_VERIFY_VERBOSE regardless of the pretty format Signed-off-by: Santiago Torres --- builtin/tag.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 8c493a569..4b91c769c 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -110,9 +110,6 @@ static int verify_tag(const char *name, const char *ref, const struct ref_format *format = cb_data; flags = GPG_VERIFY_VERBOSE; - if (format->format) - flags = GPG_VERIFY_OMIT_STATUS; - if (gpg_verify_tag(oid, name, flags)) return -1; -- 2.21.0
[PATCH 0/2] tag verification: do not mute gpg output
From: Santiago Torres The default behavior of the tag verification functions used to quiet down the gpg output if --format was passed. The rationale for this was to avoid --format to be litterred by the gpg output. However, this may be unnecessary because the gpg output is already streamed to stderr and thus can be easily multiplexed. Santiago Torres (2): builtin/tag: do not omit -v gpg out for --format builtin/verify-tag: do not omit gpg on --format builtin/tag.c| 6 +++--- builtin/verify-tag.c | 6 ++ 2 files changed, 5 insertions(+), 7 deletions(-) -- 2.21.0
[PATCH 1/2] builtin/tag: do not omit -v gpg out for --format
From: Santiago Torres The current implementation of git tag -v omits the gpg output when the --format flag is passed. This may not be useful to users that want to see the gpg output *and* --format the output of the git tag -v. Instead, pass the default gpg interface output if --format is specified. Signed-off-by: Santiago Torres --- builtin/tag.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 02f6bd1279..449d91c13c 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -110,10 +110,10 @@ static int verify_tag(const char *name, const char *ref, { int flags; const struct ref_format *format = cb_data; - flags = GPG_VERIFY_VERBOSE; + flags = 0; - if (format->format) - flags = GPG_VERIFY_OMIT_STATUS; + if (!format->format) + flags = GPG_VERIFY_VERBOSE; if (gpg_verify_tag(oid, name, flags)) return -1; -- 2.21.0
[PATCH 2/2] builtin/verify-tag: do not omit gpg on --format
From: Santiago Torres The current implementation of git-verify-tag omits the gpg output when the --format flag is passed. This may not be useful to users that want to see the gpg output *and* --format the output of git verify-tag. Instead, respect the --raw flag or the default gpg output. Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index 6fa04b751a..262e73cb45 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -47,15 +47,13 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (argc <= i) usage_with_options(verify_tag_usage, verify_tag_options); - if (verbose) + if (verbose && !format.format) flags |= GPG_VERIFY_VERBOSE; - if (format.format) { + if (format.format) if (verify_ref_format(&format)) usage_with_options(verify_tag_usage, verify_tag_options); - flags |= GPG_VERIFY_OMIT_STATUS; - } while (i < argc) { struct object_id oid; -- 2.21.0
[RFC PATCH] t: lib-gpg: flush agent sockets on startup
From: Santiago Torres When running gpg-relevant tests, a gpg-daemon is ran for a trash_directory-specific GNUPGHOME. This daemon creates a unix socket on the target host, and it will be used on subsequent runs of the same test script. Add a call to kill the agent and flush the sockets of the relevant trash directory. Signed-off-by: Santiago Torres --- t/lib-gpg.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index ec2aa8f68..22ef2fa87 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -31,6 +31,7 @@ then chmod 0700 ./gpghome && GNUPGHOME="$(pwd)/gpghome" && export GNUPGHOME && + gpgconf --kill gpg-agent && gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \ "$TEST_DIRECTORY"/lib-gpg/keyring.gpg && gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \ -- 2.13.2
[PATCH] t: lib-gpg: flush gpg agent on startup
From: Santiago Torres When running gpg-relevant tests, a gpg-daemon is spawned for each GNUPGHOME used. This daemon may stay running after the test and cache file descriptors for the trash directories, even after the trash directory is removed. This leads to ENOENT errors when attempting to create files if tests are run multiple times. Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME (if any) before setting up the GPG relevant-environment. Helped-by: Junio C Hamano Signed-off-by: Santiago Torres --- t/lib-gpg.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index ec2aa8f68..7a6c7ee6f 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -31,6 +31,7 @@ then chmod 0700 ./gpghome && GNUPGHOME="$(pwd)/gpghome" && export GNUPGHOME && + gpgconf --kill gpg-agent 2>&1 >/dev/null gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \ "$TEST_DIRECTORY"/lib-gpg/keyring.gpg && gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \ -- 2.13.3
[PATCH v2] t: lib-gpg: flush gpg agent on startup
From: Santiago Torres When running gpg-relevant tests, a gpg-daemon is spawned for each GNUPGHOME used. This daemon may stay running after the test and cache file descriptors for the trash directories, even after the trash directory is removed. This leads to ENOENT errors when attempting to create files if tests are run multiple times. Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME (if any) before setting up the GPG relevant-environment. Helped-by: Junio C Hamano Signed-off-by: Santiago Torres --- t/lib-gpg.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index ec2aa8f68..43679a4c6 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -31,6 +31,7 @@ then chmod 0700 ./gpghome && GNUPGHOME="$(pwd)/gpghome" && export GNUPGHOME && + (gpgconf --kill gpg-agent 2>&1 >/dev/null || : ) && gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \ "$TEST_DIRECTORY"/lib-gpg/keyring.gpg && gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \ -- 2.13.3
Re: Commits are no longer gpg-signed in 2.9.0 when "commit.gpgsign" is enabled
Hi Nils, I just checked and I have commits made in 2.9 with this option set and I don't seem to have your issue. Here's what I did: santiago at ~/test-signing ✔ git init Initialized empty Git repository in /home/santiago/test-signing/.git/ santiago at ~/test-signing ✔ hub create Updating origin Enter passphrase for key '/home/santiago/.ssh/id_rsa': created repository: SantiagoTorres/test-signing santiago at ~/test-signing ✔ touch testfile santiago at ~/test-signing ✔ git add testfile santiago at ~/test-signing ✔ git commit i[master (root-commit) 6de1ad2] TEST: tests git autocommit setting 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 testfile santiago at ~/test-signing ✔ git log --show-signature commit 6de1ad20237f042433f8b94a3bf8c7cc41711d90 (HEAD -> refs/heads/master) gpg: Signature made Mon 11 Jul 2016 10:40:41 AM EDT using RSA key ID 468F122CE8162295 gpg: Good signature from "Santiago Torres " [ultimate] gpg: aka "Santiago Torres-Arias " [ultimate] Author: Santiago Torres Date: Mon Jul 11 10:40:32 2016 -0400 TEST: tests git autocommit setting santiago at ~/test-signing ✔ git push origin master Enter passphrase for key '/home/santiago/.ssh/id_rsa': Counting objects: 3, done. Writing objects: 100% (3/3), 879 bytes | 0 bytes/s, done. Total 3 (delta 0), reused 0 (delta 0) To g...@github.com:SantiagoTorres/test-signing.git * [new branch] master -> master santiago at ~/test-signing ✔ git --version git version 2.9.0 santiago at ~/test-signing ✔ You can check the github repository Do you have a mwe/.gitconfig so I can take a look at it? I wonder if this issue is similar to what happened in mutt, where gpg doesn't show the password prompt using gpg-agent and it silently failed. Thanks, -Santiago. On Mon, Jul 11, 2016 at 01:29:10PM +0200, Nils Fenner wrote: > Hey Git community, > > since Version 2.9.0, the configuration option "commit.gpgsign" doesn't work > as users would expect. By committing via 'git gui' (or usual 'git commit' > without further option), commits are not being auto-signed any longer, when > "commit.gpgSign" configuration option is set. I also couldn't find a flag to > "workaround" that situation in the GUI. To my understanding, I now have to > pass the '-S' option to 'git commit' every time and GUI becomes "impractical > to use". Surprisingly, nobody seems having noticed this behavioural change > since the release of 2.9. > > FYI: In the release log, there's a note stating, that this has been altered > in the context of 'git commit-tree'. Maybe this interferes with "normal" > behaviour. > > Would be happy hearing from you soon. Thanks! > > Cheers, > Nils > > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Commits are no longer gpg-signed in 2.9.0 when "commit.gpgsign" is enabled
On Mon, Jul 11, 2016 at 06:27:57PM +0200, Nils Fenner wrote: > Hi Santiago, > > repeated your test here and actually found something interesting. When > committing via 'git gui', commits are not being gpg-signed, while firing > a 'git commit' shows the passphrase dialog and signs the commit correctly. I have the feeling that this is an issue with your gpg configuration not playing well with the gui. Do you know which pinentry program is being used (e.g., type /usr/bin/pinentry). > What you mean by wme? Is it the gpg-agent's config or something? > Sorry, I meant a Minimum Working Example, although that'll be hard to reproduce in this case. Your configuration seems to be proper. > Hope that points in the right direction. I think that we can take further troubleshooting off-list. This doesn't seem to be behavior with git (although we can come back with a bug report if something is naturally-broken/could be improved) Cheers! -Santiago. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[OT] USENIX paper on Git
Hello everyone, I will be presenting a paper regarding the Git metadata issues that we discussed at the beginning on the year on USENIX '16. I'm writing To make everyone in this ML aware that this work exists and to bring everyone into the loop. I'm open for feedback and corrections. If anything seems odd imprecise to the community, I can make an errata in the presentation (at least). I'll also try to work towards making corrections anywhere if possible; this is my first publication, so I wasn't sure if it was possible to share things before they are published. Thankfully, this is OK in USENIX's book. Here's the link: http://i2.cdn.turner.com/cnnnext/dam/assets/160730192650-14new-week-in-politics-super-169.jpg I do mention of work towards fixing these issues in upcoming versions of Git. This is in reference to the issue with Git tags, although I hope to continue working on Git in general once I have more time for it. Thanks again for all the patience reviewing patches and discussing everything. Thanks! -Santiago. P.S. Let me know if anyone is going to USENIX. I'm looking forward to meeting! [1] http://thread.gmane.org/gmane.comp.version-control.git/287649 * I believe it to be this, but gmane seems to be down. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [OT] USENIX paper on Git
> share things before they are published. Thankfully, this is OK in > > USENIX's book. Here's the link: > > http://i2.cdn.turner.com/cnnnext/dam/assets/160730192650-14new-week-in-politics-super-169.jpg > > While I had a good laugh, I am wondering whether this is the correct link? Oh my god, sorry, I meant to p, not to ctrl + v. My head is all over the place as of late. Here's the correct link: http://isis.poly.edu/~jcappos/papers/torres_toto_usenixsec-2016.pdf Thanks! -Santiago. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [OT] USENIX paper on Git
On Wed, Aug 03, 2016 at 10:14:21AM -0700, Stefan Beller wrote: > On Wed, Aug 3, 2016 at 8:25 AM, Santiago Torres wrote: > > > share things before they are published. Thankfully, this is OK in > >> > USENIX's book. Here's the link: > >> > http://i2.cdn.turner.com/cnnnext/dam/assets/160730192650-14new-week-in-politics-super-169.jpg > >> > >> While I had a good laugh, I am wondering whether this is the correct link? > > > > Oh my god, sorry, I meant to p, not to ctrl + v. My head is all over the > > place as of late. > > > > Here's the correct link: > > > > http://isis.poly.edu/~jcappos/papers/torres_toto_usenixsec-2016.pdf > > In 4.1 you write: > > Finally, Git submodules are also vulnerable, as they automatically track > > a tag (or branch). If a build dependency is included in a project as a part > > of the submodule, a package might be vulnerable via an underlying library. > > Submodules actually track commits, not tags or branches. > > This is confusing for some users, e.g. the user intended to track > a library at version 1.1, but it tracks 1234abcd instead (which is what > 1.1 points at). I'm assuming that git submodule update does update where the ref points to, does it not? let me dig into this and try to take the necessary measures to correct this Thanks for the feedback! -Santiago. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [OT] USENIX paper on Git
e from start though. > > I think this is actually a generalization of the cloning issue > (where state "A" is simply "I have no existing state yet"). > > So it seems like there is room for better tooling around push-certs > (e.g., to fetch and verify the chaining automatically). Yeah, I think that in-band cert distribution and automatic validation may be a desirable feature, but there may be reasons as to why this is not appealing. (I already have some in mind). > I think git in general is quite weak in automatic tooling for > verifications. There are room for signatures in the data format and > tools for checking that the bytes haven't been touched, but there's > almost nothing to tell you that signatures make any sense, tools for > handling trust, etc. Yes, from our previous interactions, it seems that git's philosophy focuses on providing the right information to users/tools and let those tools make the call of whether something is fishy. I don't think this is necessarily bad. > > I think your solution also had some mechanisms for adding trusted keys > as part of the hash chain. I'm not convinced that's something that > should be part of git's solution in particular, and not an out-of-band > thing handled as part of the PKI. Because it's really a group key > management problem, and applies to anything you might sign. I see. What about, for example, having an official "overlay" on git for signing and verification of a repository? (e.g., similar to what monotone does). I see that other VCS's have a plugin mechanism, and they host official plugins. Thanks a lot! -Santiago. [1] https://isis.poly.edu/~jcappos/papers/kuppusamy_nsdi_16.pdf -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [OT] USENIX paper on Git
On Wed, Aug 03, 2016 at 10:35:54AM -0700, Junio C Hamano wrote: > Santiago Torres writes: > > >> Submodules actually track commits, not tags or branches. > >> > >> This is confusing for some users, e.g. the user intended to track a > >> library at version 1.1, but it tracks 1234abcd instead (which is > >> what 1.1 points at). > > > > I'm assuming that git submodule update does update where the ref > > points to, does it not? > > I think you may configure the command to do so, instead of the default > "detach at the commit recorded in the superproject". > > But then your tree immediately will be marked by "git status" as > "modified" at such a submodule, meaning "what you have in the working > tree is different from what the commit in the superproject wants you > to have", I would think. > Ah, I see where is my confusion. Thanks for the correction :) -Santiago. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html