{"thread":{"id":"54974","subject":"[PATCH 1/5] commit: ignore additional signatures when parsing signed commits","startedAt":"2021-01-11T00:38:36Z","lastAt":"2021-02-11T08:00:47Z","messageCount":39,"participants":["brian m. carlson","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"413994","messageId":"20210111003740.1319996-2-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH 1/5] commit: ignore additional signatures when parsing signed commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:34Z","receivedAt":"2021-01-11T00:38:36Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we create a commit with multiple signatures, neither of these\nsignatures includes the other. Consequently, when we produce the\npayload which has been signed so we can verify the commit, we must strip\noff any other signatures, or the payload will differ from what was\nsigned. Do so, and in preparation for verifying with multiple\nalgorithms, pass the algorithm we want to verify into\nparse_signed_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 52 ++++++++++++++++++++++++----------------\n commit.h | 3 ++-\n log-tree.c | 2 +-\n t/t7510-signed-commit.sh | 43 ++++++++++++++++++++++++++++++++-\n 4 files changed, 77 insertions(+), 23 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex f128f18a9b..93faaad764 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1036,20 +1036,18 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n }\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *payload, struct strbuf *signature)\n+\t\t\tstruct strbuf *payload, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop)\n {\n \n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n-\tint in_signature, saw_signature = -1;\n-\tconst char *line, *tail;\n-\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];\n-\tint gpg_sig_header_len = strlen(gpg_sig_header);\n+\tint in_signature = 0, saw_signature = 0, other_signature = 0;\n+\tconst char *line, *tail, *p;\n+\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n \n \tline = buffer;\n \ttail = buffer + size;\n-\tin_signature = 0;\n-\tsaw_signature = 0;\n \twhile (line < tail) {\n \t\tconst char *sig = NULL;\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n@@ -1057,9 +1055,15 @@ int parse_signed_commit(const struct commit *commit,\n \t\tnext = next ? next + 1 : tail;\n \t\tif (in_signature && line[0] == ' ')\n \t\t\tsig = line + 1;\n-\t\telse if (starts_with(line, gpg_sig_header) &&\n-\t\t\t line[gpg_sig_header_len] == ' ')\n-\t\t\tsig = line + gpg_sig_header_len + 1;\n+\t\telse if (skip_prefix(line, gpg_sig_header, &p) &&\n+\t\t\t *p == ' ') {\n+\t\t\tsig = line + strlen(gpg_sig_header) + 1;\n+\t\t\tother_signature = 0;\n+\t\t}\n+\t\telse if (starts_with(line, \"gpgsig\"))\n+\t\t\tother_signature = 1;\n+\t\telse if (other_signature && line[0] != ' ')\n+\t\t\tother_signature = 0;\n \t\tif (sig) {\n \t\t\tstrbuf_add(signature, sig, next - sig);\n \t\t\tsaw_signature = 1;\n@@ -1068,7 +1072,8 @@ int parse_signed_commit(const struct commit *commit,\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n-\t\t\tstrbuf_add(payload, line, next - line);\n+\t\t\tif (!other_signature)\n+\t\t\t\tstrbuf_add(payload, line, next - line);\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n@@ -1082,23 +1087,27 @@ int remove_signature(struct strbuf *buf)\n \tconst char *line = buf->buf;\n \tconst char *tail = buf->buf + buf->len;\n \tint in_signature = 0;\n-\tconst char *sig_start = NULL;\n-\tconst char *sig_end = NULL;\n+\tstruct sigbuf {\n+\t\tconst char *start;\n+\t\tconst char *end;\n+\t} sigs[2] = { 0 }, *sigp = &sigs[0];\n+\tint i;\n+\tconst char *orig_buf = buf->buf;\n \n \twhile (line < tail) {\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n \t\tnext = next ? next + 1 : tail;\n \n \t\tif (in_signature && line[0] == ' ')\n-\t\t\tsig_end = next;\n+\t\t\tsigp->end = next;\n \t\telse if (starts_with(line, \"gpgsig\")) {\n \t\t\tint i;\n \t\t\tfor (i = 1; i < GIT_HASH_NALGOS; i++) {\n \t\t\t\tconst char *p;\n \t\t\t\tif (skip_prefix(line, gpg_sig_headers[i], &p) &&\n \t\t\t\t *p == ' ') {\n-\t\t\t\t\tsig_start = line;\n-\t\t\t\t\tsig_end = next;\n+\t\t\t\t\tsigp->start = line;\n+\t\t\t\t\tsigp->end = next;\n \t\t\t\t\tin_signature = 1;\n \t\t\t\t}\n \t\t\t}\n@@ -1106,15 +1115,18 @@ int remove_signature(struct strbuf *buf)\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n+\t\t\tif (in_signature && sigp - sigs != ARRAY_SIZE(sigs))\n+\t\t\t\tsigp++;\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n \t}\n \n-\tif (sig_start)\n-\t\tstrbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start);\n+\tfor (i = ARRAY_SIZE(sigs) - 1; i >= 0; i--)\n+\t\tif (sigs[i].start)\n+\t\t\tstrbuf_remove(buf, sigs[i].start - orig_buf, sigs[i].end - sigs[i].start);\n \n-\treturn sig_start != NULL;\n+\treturn sigs[0].start != NULL;\n }\n \n static void handle_signed_tag(struct commit *parent, struct commit_extra_header ***tail)\n@@ -1165,7 +1177,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \n \tsigc->result = 'N';\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \tret = check_signature(payload.buf, payload.len, signature.buf,\n \t\tsignature.len, sigc);\ndiff --git a/commit.h b/commit.h\nindex f4e7b0158e..030aa65ab8 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -317,7 +317,8 @@ void set_merge_remote_desc(struct commit *commit,\n struct commit *get_merge_parent(const char *name);\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *message, struct strbuf *signature);\n+\t\t\tstruct strbuf *message, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop);\n int remove_signature(struct strbuf *buf);\n \n /*\ndiff --git a/log-tree.c b/log-tree.c\nindex fd0dde97ec..7e0335e548 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -502,7 +502,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \tstruct signature_check sigc = { 0 };\n \tint status;\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n \tstatus = check_signature(payload.buf, payload.len, signature.buf,\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 6baaa1ad91..d78319d5c8 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -172,7 +172,7 @@ test_expect_success GPG 'show signed commit with signature' '\n \tgit cat-file commit initial >cat &&\n \tgrep -v -e \"gpg: \" -e \"Warning: \" show >show.commit &&\n \tgrep -e \"gpg: \" -e \"Warning: \" show >show.gpg &&\n-\tgrep -v \"^ \" cat | grep -v \"^$(test_oid header) \" >cat.commit &&\n+\tgrep -v \"^ \" cat | grep -v \"^gpgsig.* \" >cat.commit &&\n \ttest_cmp show.commit commit &&\n \ttest_cmp show.gpg verify.2 &&\n \ttest_cmp cat.commit verify.1\n@@ -334,4 +334,45 @@ test_expect_success GPG 'show double signature with custom format' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success GPG 'verify-commit verifies multiply signed commits' '\n+\tgit init multiply-signed &&\n+\tcd multiply-signed &&\n+\ttest_commit first &&\n+\techo 1 >second &&\n+\tgit add second &&\n+\ttree=$(git write-tree) &&\n+\tparent=$(git rev-parse HEAD^{commit}) &&\n+\tgit commit --gpg-sign -m second &&\n+\tgit cat-file commit HEAD &&\n+\t# Avoid trailing whitespace.\n+\tsed -e \"s/^Q//\" -e \"s/^Z/ /\" >commit <<-EOF &&\n+\tQtree $tree\n+\tQparent $parent\n+\tQauthor A U Thor 1112912653 -0700\n+\tQcommitter C O Mitter 1112912653 -0700\n+\tQgpgsig -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBDRYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMNd+8AoK1I8mhLHviPH+q2I5fIVgPsEtYC\n+\tQ AKCTqBh+VabJceXcGIZuF0Ry+udbBQ==\n+\tQ =tQ0N\n+\tQ -----END PGP SIGNATURE-----\n+\tQgpgsig-sha256 -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBIBYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN/NEAn0XO9RYSBj2dFyozi0JKSbssYMtO\n+\tQ AJwKCQ1BQOtuwz//IjU8TiS+6S4iUw==\n+\tQ =pIwP\n+\tQ -----END PGP SIGNATURE-----\n+\tQ\n+\tQsecond\n+\tEOF\n+\thead=$(git hash-object -t commit -w commit) &&\n+\tgit reset --hard $head &&\n+\tgit verify-commit $head 2>actual &&\n+\tgrep \"Good signature from\" actual &&\n+\t! grep \"BAD signature from\" actual\n+'\n+\n test_done\n"},{"id":"413995","messageId":"20210111003740.1319996-4-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH 3/5] commit: allow parsing arbitrary buffers with headers","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:36Z","receivedAt":"2021-01-11T00:38:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently only commits are signed with headers. However, in the future,\nwe'll also sign tags with headers as well. Let's refactor out a\nfunction called parse_buffer_signed_by_header which does exactly that.\nIn addition, since we'll want to sign things other than commits this\nway, let's call the function sign_with_header instead of do_sign_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 21 +++++++++++++++++----\n commit.h | 9 +++++++++\n 2 files changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 794dc8b593..7bbab5add4 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -995,7 +995,7 @@ static const char *gpg_sig_headers[] = {\n \t\"gpgsig-sha256\",\n };\n \n-static int do_sign_commit(struct strbuf *buf, const char *keyid)\n+int sign_with_header(struct strbuf *buf, const char *keyid)\n {\n \tstruct strbuf sig = STRBUF_INIT;\n \tint inspos, copypos;\n@@ -1035,16 +1035,30 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n \treturn 0;\n }\n \n+\n+\n int parse_signed_commit(const struct commit *commit,\n \t\t\tstruct strbuf *payload, struct strbuf *signature,\n \t\t\tconst struct git_hash_algo *algop)\n {\n-\n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n+\tint ret = parse_buffer_signed_by_header(buffer, size, payload, signature, algop);\n+\n+\tunuse_commit_buffer(commit, buffer);\n+\treturn ret;\n+}\n+\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop)\n+{\n \tint in_signature = 0, saw_signature = 0, other_signature = 0;\n \tconst char *line, *tail, *p;\n \tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n+\tint gpg_sig_header_len = strlen(gpg_sig_header);\n \n \tline = buffer;\n \ttail = buffer + size;\n@@ -1078,7 +1092,6 @@ int parse_signed_commit(const struct commit *commit,\n \t\t}\n \t\tline = next;\n \t}\n-\tunuse_commit_buffer(commit, buffer);\n \treturn saw_signature;\n }\n \n@@ -1530,7 +1543,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,\n \tif (encoding_is_utf8 && !verify_utf8(&buffer))\n \t\tfprintf(stderr, _(commit_utf8_warn));\n \n-\tif (sign_commit && do_sign_commit(&buffer, sign_commit)) {\n+\tif (sign_commit && sign_with_header(&buffer, sign_commit)) {\n \t\tresult = -1;\n \t\tgoto out;\n \t}\ndiff --git a/commit.h b/commit.h\nindex 030aa65ab8..e2856ce8ef 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -360,4 +360,13 @@ int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void\n LAST_ARG_MUST_BE_NULL\n int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);\n \n+/* Sign a commit or tag buffer, storing the result in a header. */\n+int sign_with_header(struct strbuf *buf, const char *keyid);\n+/* Parse the signature out of a header. */\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop);\n+\n #endif /* COMMIT_H */\n"},{"id":"413996","messageId":"20210111003740.1319996-3-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH 2/5] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:35Z","receivedAt":"2021-01-11T00:38:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have a function which parses a buffer with a signature at the end,\nparse_signature, and this function is used for signed tags. However,\nthe transition plan has SHA-256 tags using a header, which is a\nmaterially different syntax. The current interface is not suitable for\nparsing such tags.\n\nAdjust the parse_signature interface to store the parsed data in two\nstrbufs and turn the existing function into parse_signed_buffer. The\nlatter is still used in places where we want to strip off the signature\nin a SHA-1 tag or in places where we know we always have a signed\nbuffer, such as push certs.\n\nAdjust all the callers to deal with this new interface.\n\nSigned-off-by: brian m. carlson \n---\n builtin/receive-pack.c | 4 ++--\n builtin/tag.c | 16 ++++++++++++----\n commit.c | 9 ++++++---\n fmt-merge-msg.c | 8 +++++---\n gpg-interface.c | 13 ++++++++++++-\n gpg-interface.h | 9 ++++++++-\n log-tree.c | 13 +++++++------\n ref-filter.c | 18 ++++++++++++++----\n tag.c | 15 ++++++++-------\n 9 files changed, 74 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d49d050e6e..b89ce31bf2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -764,7 +764,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \n-\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tbogs = parse_signed_buffer(push_cert.buf, push_cert.len);\n \t\tcheck_signature(push_cert.buf, bogs, push_cert.buf + bogs,\n \t\t\t\tpush_cert.len - bogs, &sigcheck);\n \n@@ -2050,7 +2050,7 @@ static void queue_commands_from_cert(struct command **tail,\n \t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n \telse\n \t\tboc += 2;\n-\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\teoc = push_cert->buf + parse_signed_buffer(push_cert->buf, push_cert->len);\n \n \twhile (boc < eoc) {\n \t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ecf011776d..7162f4ccc5 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -174,11 +174,17 @@ static void write_tag_body(int fd, const struct object_id *oid)\n {\n \tunsigned long size;\n \tenum object_type type;\n-\tchar *buf, *sp;\n+\tchar *buf, *sp, *orig;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n-\tbuf = read_object_file(oid, &type, &size);\n+\torig = buf = read_object_file(oid, &type, &size);\n \tif (!buf)\n \t\treturn;\n+\tif (parse_signature(buf, size, &payload, &signature)) {\n+\t\tbuf = payload.buf;\n+\t\tsize = payload.len;\n+\t}\n \t/* skip header */\n \tsp = strstr(buf, \"\\n\\n\");\n \n@@ -187,9 +193,11 @@ static void write_tag_body(int fd, const struct object_id *oid)\n \t\treturn;\n \t}\n \tsp += 2; /* skip the 2 LFs */\n-\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp));\n+\twrite_or_die(fd, sp, buf + size - sp);\n \n-\tfree(buf);\n+\tfree(orig);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n }\n \n static int build_tag_object(struct strbuf *buf, int sign, struct object_id *result)\ndiff --git a/commit.c b/commit.c\nindex 93faaad764..794dc8b593 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1134,8 +1134,10 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tstruct merge_remote_desc *desc;\n \tstruct commit_extra_header *mergetag;\n \tchar *buf;\n-\tunsigned long size, len;\n+\tunsigned long size;\n \tenum object_type type;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \tdesc = merge_remote_util(parent);\n \tif (!desc || !desc->obj)\n@@ -1143,8 +1145,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tbuf = read_object_file(&desc->obj->oid, &type, &size);\n \tif (!buf || type != OBJ_TAG)\n \t\tgoto free_return;\n-\tlen = parse_signature(buf, size);\n-\tif (size == len)\n+\tif (!parse_signature(buf, size, &payload, &signature))\n \t\tgoto free_return;\n \t/*\n \t * We could verify this signature and either omit the tag when\n@@ -1163,6 +1164,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \n \t**tail = mergetag;\n \t*tail = &mergetag->next;\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn;\n \n free_return:\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 9a664a4a58..b96e6e5926 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -512,15 +512,16 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\tunsigned long size, len;\n \t\tchar *buf = read_object_file(oid, &type, &size);\n \t\tstruct signature_check sigc = { NULL };\n-\t\tstruct strbuf sig = STRBUF_INIT;\n+\t\tstruct strbuf payload = STRBUF_INIT, sig = STRBUF_INIT;\n \n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n-\t\tlen = parse_signature(buf, size);\n+\t\tlen = parse_signature(buf, size, &payload, &sig);\n \n \t\tif (size == len)\n \t\t\t; /* merely annotated */\n-\t\telse if (check_signature(buf, len, buf + len, size - len, &sigc) &&\n+\t\telse if (check_signature(payload.buf, payload.len, sig.buf,\n+\t\t\t\t\t sig.len, &sigc) &&\n \t\t\t!sigc.gpg_output)\n \t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n \t\telse\n@@ -547,6 +548,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\t\t\tstrlen(origins.items[i].string));\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n+\t\tstrbuf_release(&payload);\n \t\tstrbuf_release(&sig);\n \tnext:\n \t\tfree(buf);\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex b499270836..c6274c14af 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -345,7 +345,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-size_t parse_signature(const char *buf, size_t size)\n+size_t parse_signed_buffer(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n \tsize_t match = size;\n@@ -361,6 +361,17 @@ size_t parse_signature(const char *buf, size_t size)\n \treturn match;\n }\n \n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature)\n+{\n+\tsize_t match = parse_signed_buffer(buf, size);\n+\tif (match != size) {\n+\t\tstrbuf_add(payload, buf, match);\n+\t\tstrbuf_add(signature, buf + match, size - match);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n void set_signing_key(const char *key)\n {\n \tfree(configured_signing_key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex f4e9b4f371..80567e4894 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -37,13 +37,20 @@ struct signature_check {\n \n void signature_check_clear(struct signature_check *sigc);\n \n+/*\n+ * Look at a GPG signed tag object. If such a signature exists, store it in\n+ * signature and the signed content in payload. Return 1 if a signature was\n+ * found, and 0 otherwise.\n+ */\n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature);\n+\n /*\n * Look at GPG signed content (e.g. a signed tag object), whose\n * payload is followed by a detached signature on it. Return the\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n */\n-size_t parse_signature(const char *buf, size_t size);\n+size_t parse_signed_buffer(const char *buf, size_t size);\n \n /*\n * Create a detached signature for the contents of \"buffer\" and append\ndiff --git a/log-tree.c b/log-tree.c\nindex 7e0335e548..b025c8da93 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -548,7 +548,8 @@ static int show_one_mergetag(struct commit *commit,\n \tstruct strbuf verify_message;\n \tstruct signature_check sigc = { 0 };\n \tint status, nth;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \thash_object_file(the_hash_algo, extra->value, extra->len,\n \t\t\t type_name(OBJ_TAG), &oid);\n@@ -571,13 +572,11 @@ static int show_one_mergetag(struct commit *commit,\n \t\tstrbuf_addf(&verify_message,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \n-\tpayload_size = parse_signature(extra->value, extra->len);\n \tstatus = -1;\n-\tif (extra->len > payload_size) {\n+\tif (parse_signature(extra->value, extra->len, &payload, &signature)) {\n \t\t/* could have a good signature */\n-\t\tstatus = check_signature(extra->value, payload_size,\n-\t\t\t\t\t extra->value + payload_size,\n-\t\t\t\t\t extra->len - payload_size, &sigc);\n+\t\tstatus = check_signature(payload.buf, payload.len,\n+\t\t\t\t\t signature.buf, signature.len, &sigc);\n \t\tif (sigc.gpg_output)\n \t\t\tstrbuf_addstr(&verify_message, sigc.gpg_output);\n \t\telse\n@@ -588,6 +587,8 @@ static int show_one_mergetag(struct commit *commit,\n \n \tshow_sig_lines(opt, status, verify_message.buf);\n \tstrbuf_release(&verify_message);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex aa260bfd09..8d8baec1b5 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1215,7 +1215,13 @@ static void find_subpos(const char *buf,\n \t\t\tunsigned long *nonsiglen,\n \t\t\tconst char **sig, unsigned long *siglen)\n {\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tconst char *eol;\n+\tconst char *end = buf + strlen(buf);\n+\tconst char *sigstart;\n+\n+\n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n \t\teol = strchrnul(buf, '\\n');\n@@ -1228,14 +1234,15 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \n \t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n-\t*siglen = strlen(*sig);\n+\tparse_signature(buf, end - buf, &payload, &signature);\n+\t*sig = strbuf_detach(&signature, siglen);\n+\tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n \t/* subject is first non-empty line */\n \t*sub = buf;\n \t/* subject goes to first empty line before signature begins */\n \tif ((eol = strstr(*sub, \"\\n\\n\"))) {\n-\t\teol = eol < *sig ? eol : *sig;\n+\t\teol = eol < sigstart ? eol : sigstart;\n \t/* check if message uses CRLF */\n \t} else if (! (eol = strstr(*sub, \"\\r\\n\\r\\n\"))) {\n \t\t/* treat whole message as subject */\n@@ -1253,7 +1260,7 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \t*body = buf;\n \t*bodylen = strlen(buf);\n-\t*nonsiglen = *sig - buf;\n+\t*nonsiglen = sigstart - buf;\n }\n \n /*\n@@ -1291,6 +1298,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\tstruct used_atom *atom = &used_atom[i];\n \t\tconst char *name = atom->name;\n \t\tstruct atom_value *v = &val[i];\n+\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -1336,6 +1344,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n+\n+\t\tfree((void *)sigpos);\n \t}\n }\n \ndiff --git a/tag.c b/tag.c\nindex 1ed2684e45..3e18a41841 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -13,26 +13,27 @@ const char *tag_type = \"tag\";\n static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n {\n \tstruct signature_check sigc;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tint ret;\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n-\tpayload_size = parse_signature(buf, size);\n-\n-\tif (size == payload_size) {\n+\tif (!parse_signature(buf, size, &payload, &signature)) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n-\t\t\twrite_in_full(1, buf, payload_size);\n+\t\t\twrite_in_full(1, buf, size);\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(buf, payload_size, buf + payload_size,\n-\t\t\t\tsize - payload_size, &sigc);\n+\tret = check_signature(payload.buf, payload.len, signature.buf,\n+\t\t\t\tsignature.len, &sigc);\n \n \tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n \t\tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn ret;\n }\n \n"},{"id":"413997","messageId":"20210111003740.1319996-5-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH 4/5] ref-filter: hoist signature parsing","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:37Z","receivedAt":"2021-01-11T00:38:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we parse a signature in the ref-filter code, we continually\nincrement the buffer pointer. Hoist the signature parsing above the\nblank line delimiting headers and body so we can find the signature when\nusing a header to sign the buffer.\n\nSigned-off-by: brian m. carlson \n---\n ref-filter.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8d8baec1b5..32ed4d5111 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1221,6 +1221,8 @@ static void find_subpos(const char *buf,\n \tconst char *end = buf + strlen(buf);\n \tconst char *sigstart;\n \n+\t/* parse signature first; we might not even have a subject line */\n+\tparse_signature(buf, end - buf, &payload, &signature);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n@@ -1232,9 +1234,6 @@ static void find_subpos(const char *buf,\n \t/* skip any empty lines */\n \twhile (*buf == '\\n')\n \t\tbuf++;\n-\n-\t/* parse signature first; we might not even have a subject line */\n-\tparse_signature(buf, end - buf, &payload, &signature);\n \t*sig = strbuf_detach(&signature, siglen);\n \tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n@@ -1330,7 +1329,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = xmemdupz(sigpos, siglen);\n \t\telse if (atom->u.contents.option == C_LINES) {\n \t\t\tstruct strbuf s = STRBUF_INIT;\n-\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n+\t\t\tconst char *contents_end = bodypos + nonsiglen;\n \n \t\t\t/* Size is the length of the message after removing the signature */\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n"},{"id":"413998","messageId":"20210111003740.1319996-1-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":null,"subject":"[PATCH 0/5] Support for commits signed by multiple algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:33Z","receivedAt":"2021-01-11T00:39:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces support for verifying commits and tags signed by\nmultiple algorithms.\n\nOriginally, we had planned for SHA-256 tags to stuff the signature in a\nheader instead of using a trailing signature, and a patch to do that was\nsent out in part 1/3. Unfortunately, for whatever reason, that patch\ndidn't make it into the master branch, and so we use trailing signatures\nthere.\n\nWe can't change this now, because otherwise it would be ambiguous\nwhether the trailing signature on a SHA-256 object was for the SHA-256\ncontents or whether the contents were a rewritten SHA-1 object with no\nSHA-256 signature at all. To do the next best thing, let's use the\ntrailing signature for the preferred hash algorithm and use a header for\nthe other variant. This permits round-tripping, but has the downside\nthat tags signed with multiple algorithms can't be verified with older\nversions of Git. However, signatures created with older versions of Git\ncontinue to be accepted.\n\nFor commits, let's accept a commit that has two signatures. We\npreviously created the commits correctly but didn't strip the extra\nheader off when verifying, so our verification indicated the signature\nwas bad.\n\nBoth these situations allow for signing commits and tags that can be\nround-tripped through both SHA-1 and SHA-256. We verify only the\nsignature using the current hash algorithm, since we currently don't\nrewrite objects.\n\nbrian m. carlson (5):\n commit: ignore additional signatures when parsing signed commits\n gpg-interface: improve interface for parsing tags\n commit: allow parsing arbitrary buffers with headers\n ref-filter: hoist signature parsing\n gpg-interface: remove other signature headers before verifying\n\n builtin/receive-pack.c | 4 +-\n builtin/tag.c | 16 ++++++--\n commit.c | 82 +++++++++++++++++++++++++++-------------\n commit.h | 12 +++++-\n fmt-merge-msg.c | 8 ++--\n gpg-interface.c | 15 +++++++-\n gpg-interface.h | 9 ++++-\n log-tree.c | 15 ++++----\n ref-filter.c | 23 +++++++----\n t/t7004-tag.sh | 25 ++++++++++++\n t/t7510-signed-commit.sh | 43 ++++++++++++++++++++-\n tag.c | 15 ++++----\n 12 files changed, 206 insertions(+), 61 deletions(-)\n\n"},{"id":"413999","messageId":"20210111003740.1319996-6-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH 5/6] fixup! commit: ignore additional signatures when parsing signed commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:38Z","receivedAt":"2021-01-11T00:39:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"---\n commit.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/commit.c b/commit.c\nindex 7bbab5add4..23020c0bca 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1058,7 +1058,6 @@ int parse_buffer_signed_by_header(const char *buffer,\n \tint in_signature = 0, saw_signature = 0, other_signature = 0;\n \tconst char *line, *tail, *p;\n \tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n-\tint gpg_sig_header_len = strlen(gpg_sig_header);\n \n \tline = buffer;\n \ttail = buffer + size;\n"},{"id":"414000","messageId":"20210111003740.1319996-7-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH 5/5] gpg-interface: remove other signature headers before verifying","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:39Z","receivedAt":"2021-01-11T00:39:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we have a multiply signed commit, we need to remove the signature\nin the header before verifying the object, since the trailing signature\nwill not be over both pieces of data. Do so, and verify that we\nvalidate the signature appropriately.\n\nSigned-off-by: brian m. carlson \n---\n gpg-interface.c | 2 ++\n t/t7004-tag.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c6274c14af..127aecfc2b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"commit.h\"\n #include \"config.h\"\n #include \"run-command.h\"\n #include \"strbuf.h\"\n@@ -366,6 +367,7 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct\n \tsize_t match = parse_signed_buffer(buf, size);\n \tif (match != size) {\n \t\tstrbuf_add(payload, buf, match);\n+\t\tremove_signature(payload);\n \t\tstrbuf_add(signature, buf + match, size - match);\n \t\treturn 1;\n \t}\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 05f411c821..6fb4e3cf11 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -17,6 +17,13 @@ tag_exists () {\n \tgit show-ref --quiet --verify refs/tags/\"$1\"\n }\n \n+test_expect_success 'setup' '\n+\ttest_oid_cache <<-EOM\n+\tothersigheader sha1:gpgsig-sha256\n+\tothersigheader sha256:gpgsig\n+\tEOM\n+'\n+\n test_expect_success 'listing all tags in an empty tree should succeed' '\n \tgit tag -l &&\n \tgit tag\n@@ -1371,6 +1378,24 @@ test_expect_success GPG \\\n \t'test_config gpg.program echo &&\n \t test_must_fail git tag -s -m tail tag-gpg-failure'\n \n+# try to produce invalid signature\n+test_expect_success GPG 'git verifies tag is valid with double signature' '\n+\tgit tag -s -m tail tag-gpg-double-sig &&\n+\tgit cat-file tag tag-gpg-double-sig >tag &&\n+\tothersigheader=$(test_oid othersigheader) &&\n+\tsed -ne \"/^\\$/q;p\" tag >new-tag &&\n+\tcat <<-EOM >>new-tag &&\n+\t$othersigheader -----BEGIN PGP SIGNATURE-----\n+\t someinvaliddata\n+\t -----END PGP SIGNATURE-----\n+\tEOM\n+\tsed -e \"1,/^tagger/d\" tag >>new-tag &&\n+\tnew_tag=$(git hash-object -t tag -w new-tag) &&\n+\tgit update-ref refs/tags/tag-gpg-double-sig $new_tag &&\n+\tgit verify-tag tag-gpg-double-sig &&\n+\tgit fsck\n+'\n+\n # try to sign with bad user.signingkey\n test_expect_success GPGSM \\\n \t'git tag -s fails if gpgsm is misconfigured (bad key)' \\\n"},{"id":"414001","messageId":"20210111003740.1319996-8-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH 6/6] gpg-interface: remove other signature headers before verifying","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:37:40Z","receivedAt":"2021-01-11T00:39:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we have a multiply signed commit, we need to remove the signature\nin the header before verifying the object, since the trailing signature\nwill not be over both pieces of data. Do so, and verify that we\nvalidate the signature appropriately.\n\nSigned-off-by: brian m. carlson \n---\n gpg-interface.c | 2 ++\n t/t7004-tag.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c6274c14af..127aecfc2b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"commit.h\"\n #include \"config.h\"\n #include \"run-command.h\"\n #include \"strbuf.h\"\n@@ -366,6 +367,7 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct\n \tsize_t match = parse_signed_buffer(buf, size);\n \tif (match != size) {\n \t\tstrbuf_add(payload, buf, match);\n+\t\tremove_signature(payload);\n \t\tstrbuf_add(signature, buf + match, size - match);\n \t\treturn 1;\n \t}\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 05f411c821..6fb4e3cf11 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -17,6 +17,13 @@ tag_exists () {\n \tgit show-ref --quiet --verify refs/tags/\"$1\"\n }\n \n+test_expect_success 'setup' '\n+\ttest_oid_cache <<-EOM\n+\tothersigheader sha1:gpgsig-sha256\n+\tothersigheader sha256:gpgsig\n+\tEOM\n+'\n+\n test_expect_success 'listing all tags in an empty tree should succeed' '\n \tgit tag -l &&\n \tgit tag\n@@ -1371,6 +1378,24 @@ test_expect_success GPG \\\n \t'test_config gpg.program echo &&\n \t test_must_fail git tag -s -m tail tag-gpg-failure'\n \n+# try to produce invalid signature\n+test_expect_success GPG 'git verifies tag is valid with double signature' '\n+\tgit tag -s -m tail tag-gpg-double-sig &&\n+\tgit cat-file tag tag-gpg-double-sig >tag &&\n+\tothersigheader=$(test_oid othersigheader) &&\n+\tsed -ne \"/^\\$/q;p\" tag >new-tag &&\n+\tcat <<-EOM >>new-tag &&\n+\t$othersigheader -----BEGIN PGP SIGNATURE-----\n+\t someinvaliddata\n+\t -----END PGP SIGNATURE-----\n+\tEOM\n+\tsed -e \"1,/^tagger/d\" tag >>new-tag &&\n+\tnew_tag=$(git hash-object -t tag -w new-tag) &&\n+\tgit update-ref refs/tags/tag-gpg-double-sig $new_tag &&\n+\tgit verify-tag tag-gpg-double-sig &&\n+\tgit fsck\n+'\n+\n # try to sign with bad user.signingkey\n test_expect_success GPGSM \\\n \t'git tag -s fails if gpgsm is misconfigured (bad key)' \\\n"},{"id":"414002","messageId":"X/ufMz0Xq2NhFKY0@camp.crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-6-sandals@crustytoothpaste.net","subject":"Re: [PATCH 5/6] fixup! commit: ignore additional signatures when parsing signed commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T00:43:31Z","receivedAt":"2021-01-11T00:44:49Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-01-11 at 00:37:38, brian m. carlson wrote:\n> ---\n> commit.c | 1 -\n> 1 file changed, 1 deletion(-)\n> \n\nPlease ignore this patch. I squashed it in, but neglected to clean my\npatches directory before sending. My apologies for the error.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"414004","messageId":"20210111035840.2437737-1-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111003740.1319996-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 0/5] Support for commits signed by multiple algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T03:58:35Z","receivedAt":"2021-01-11T03:59:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces support for verifying commits and tags signed by\nmultiple algorithms.\n\nOriginally, we had planned for SHA-256 tags to stuff the signature in a\nheader instead of using a trailing signature, and a patch to do that was\nsent out in part 1/3. Unfortunately, for whatever reason, that patch\ndidn't make it into the master branch, and so we use trailing signatures\nthere.\n\nWe can't change this now, because otherwise it would be ambiguous\nwhether the trailing signature on a SHA-256 object was for the SHA-256\ncontents or whether the contents were a rewritten SHA-1 object with no\nSHA-256 signature at all. To do the next best thing, let's use the\ntrailing signature for the preferred hash algorithm and use a header for\nthe other variant. This permits round-tripping, but has the downside\nthat tags signed with multiple algorithms can't be verified with older\nversions of Git. However, signatures created with older versions of Git\ncontinue to be accepted.\n\nFor commits, let's accept a commit that has two signatures. We\npreviously created the commits correctly but didn't strip the extra\nheader off when verifying, so our verification indicated the signature\nwas bad.\n\nBoth these situations allow for signing commits and tags that can be\nround-tripped through both SHA-1 and SHA-256. We verify only the\nsignature using the current hash algorithm, since we currently don't\nrewrite objects.\n\nChanges from v1:\n* Fix brown paper bag bug where some tests failed due to a bad rebase.\n\nbrian m. carlson (5):\n commit: ignore additional signatures when parsing signed commits\n gpg-interface: improve interface for parsing tags\n commit: allow parsing arbitrary buffers with headers\n ref-filter: hoist signature parsing\n gpg-interface: remove other signature headers before verifying\n\n builtin/receive-pack.c | 4 +-\n builtin/tag.c | 16 ++++++--\n commit.c | 83 ++++++++++++++++++++++++++--------------\n commit.h | 12 +++++-\n fmt-merge-msg.c | 29 ++++++++------\n gpg-interface.c | 15 +++++++-\n gpg-interface.h | 9 ++++-\n log-tree.c | 15 ++++----\n ref-filter.c | 23 +++++++----\n t/t7004-tag.sh | 25 ++++++++++++\n t/t7510-signed-commit.sh | 43 ++++++++++++++++++++-\n tag.c | 15 ++++----\n 12 files changed, 219 insertions(+), 70 deletions(-)\n\n"},{"id":"414005","messageId":"20210111035840.2437737-4-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111035840.2437737-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 3/5] commit: allow parsing arbitrary buffers with headers","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T03:58:38Z","receivedAt":"2021-01-11T03:59:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently only commits are signed with headers. However, in the future,\nwe'll also sign tags with headers as well. Let's refactor out a\nfunction called parse_buffer_signed_by_header which does exactly that.\nIn addition, since we'll want to sign things other than commits this\nway, let's call the function sign_with_header instead of do_sign_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 20 ++++++++++++++++----\n commit.h | 9 +++++++++\n 2 files changed, 25 insertions(+), 4 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 794dc8b593..23020c0bca 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -995,7 +995,7 @@ static const char *gpg_sig_headers[] = {\n \t\"gpgsig-sha256\",\n };\n \n-static int do_sign_commit(struct strbuf *buf, const char *keyid)\n+int sign_with_header(struct strbuf *buf, const char *keyid)\n {\n \tstruct strbuf sig = STRBUF_INIT;\n \tint inspos, copypos;\n@@ -1035,13 +1035,26 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n \treturn 0;\n }\n \n+\n+\n int parse_signed_commit(const struct commit *commit,\n \t\t\tstruct strbuf *payload, struct strbuf *signature,\n \t\t\tconst struct git_hash_algo *algop)\n {\n-\n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n+\tint ret = parse_buffer_signed_by_header(buffer, size, payload, signature, algop);\n+\n+\tunuse_commit_buffer(commit, buffer);\n+\treturn ret;\n+}\n+\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop)\n+{\n \tint in_signature = 0, saw_signature = 0, other_signature = 0;\n \tconst char *line, *tail, *p;\n \tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n@@ -1078,7 +1091,6 @@ int parse_signed_commit(const struct commit *commit,\n \t\t}\n \t\tline = next;\n \t}\n-\tunuse_commit_buffer(commit, buffer);\n \treturn saw_signature;\n }\n \n@@ -1530,7 +1542,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,\n \tif (encoding_is_utf8 && !verify_utf8(&buffer))\n \t\tfprintf(stderr, _(commit_utf8_warn));\n \n-\tif (sign_commit && do_sign_commit(&buffer, sign_commit)) {\n+\tif (sign_commit && sign_with_header(&buffer, sign_commit)) {\n \t\tresult = -1;\n \t\tgoto out;\n \t}\ndiff --git a/commit.h b/commit.h\nindex 030aa65ab8..e2856ce8ef 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -360,4 +360,13 @@ int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void\n LAST_ARG_MUST_BE_NULL\n int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);\n \n+/* Sign a commit or tag buffer, storing the result in a header. */\n+int sign_with_header(struct strbuf *buf, const char *keyid);\n+/* Parse the signature out of a header. */\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop);\n+\n #endif /* COMMIT_H */\n"},{"id":"414006","messageId":"20210111035840.2437737-5-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111035840.2437737-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 4/5] ref-filter: hoist signature parsing","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T03:58:39Z","receivedAt":"2021-01-11T03:59:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we parse a signature in the ref-filter code, we continually\nincrement the buffer pointer. Hoist the signature parsing above the\nblank line delimiting headers and body so we can find the signature when\nusing a header to sign the buffer.\n\nSigned-off-by: brian m. carlson \n---\n ref-filter.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8d8baec1b5..32ed4d5111 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1221,6 +1221,8 @@ static void find_subpos(const char *buf,\n \tconst char *end = buf + strlen(buf);\n \tconst char *sigstart;\n \n+\t/* parse signature first; we might not even have a subject line */\n+\tparse_signature(buf, end - buf, &payload, &signature);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n@@ -1232,9 +1234,6 @@ static void find_subpos(const char *buf,\n \t/* skip any empty lines */\n \twhile (*buf == '\\n')\n \t\tbuf++;\n-\n-\t/* parse signature first; we might not even have a subject line */\n-\tparse_signature(buf, end - buf, &payload, &signature);\n \t*sig = strbuf_detach(&signature, siglen);\n \tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n@@ -1330,7 +1329,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = xmemdupz(sigpos, siglen);\n \t\telse if (atom->u.contents.option == C_LINES) {\n \t\t\tstruct strbuf s = STRBUF_INIT;\n-\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n+\t\t\tconst char *contents_end = bodypos + nonsiglen;\n \n \t\t\t/* Size is the length of the message after removing the signature */\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n"},{"id":"414007","messageId":"20210111035840.2437737-3-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111035840.2437737-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 2/5] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T03:58:37Z","receivedAt":"2021-01-11T03:59:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have a function which parses a buffer with a signature at the end,\nparse_signature, and this function is used for signed tags. However,\nwe'll need to store values for multiple algorithms, and we'll do this by\nusing a header for the non-default algorithm.\n\nAdjust the parse_signature interface to store the parsed data in two\nstrbufs and turn the existing function into parse_signed_buffer. The\nlatter is still used in places where we know we always have a signed\nbuffer, such as push certs.\n\nAdjust all the callers to deal with this new interface.\n\nSigned-off-by: brian m. carlson \n---\n builtin/receive-pack.c | 4 ++--\n builtin/tag.c | 16 ++++++++++++----\n commit.c | 9 ++++++---\n fmt-merge-msg.c | 29 ++++++++++++++++++-----------\n gpg-interface.c | 13 ++++++++++++-\n gpg-interface.h | 9 ++++++++-\n log-tree.c | 13 +++++++------\n ref-filter.c | 18 ++++++++++++++----\n tag.c | 15 ++++++++-------\n 9 files changed, 87 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d49d050e6e..b89ce31bf2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -764,7 +764,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \n-\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tbogs = parse_signed_buffer(push_cert.buf, push_cert.len);\n \t\tcheck_signature(push_cert.buf, bogs, push_cert.buf + bogs,\n \t\t\t\tpush_cert.len - bogs, &sigcheck);\n \n@@ -2050,7 +2050,7 @@ static void queue_commands_from_cert(struct command **tail,\n \t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n \telse\n \t\tboc += 2;\n-\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\teoc = push_cert->buf + parse_signed_buffer(push_cert->buf, push_cert->len);\n \n \twhile (boc < eoc) {\n \t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ecf011776d..7162f4ccc5 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -174,11 +174,17 @@ static void write_tag_body(int fd, const struct object_id *oid)\n {\n \tunsigned long size;\n \tenum object_type type;\n-\tchar *buf, *sp;\n+\tchar *buf, *sp, *orig;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n-\tbuf = read_object_file(oid, &type, &size);\n+\torig = buf = read_object_file(oid, &type, &size);\n \tif (!buf)\n \t\treturn;\n+\tif (parse_signature(buf, size, &payload, &signature)) {\n+\t\tbuf = payload.buf;\n+\t\tsize = payload.len;\n+\t}\n \t/* skip header */\n \tsp = strstr(buf, \"\\n\\n\");\n \n@@ -187,9 +193,11 @@ static void write_tag_body(int fd, const struct object_id *oid)\n \t\treturn;\n \t}\n \tsp += 2; /* skip the 2 LFs */\n-\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp));\n+\twrite_or_die(fd, sp, buf + size - sp);\n \n-\tfree(buf);\n+\tfree(orig);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n }\n \n static int build_tag_object(struct strbuf *buf, int sign, struct object_id *result)\ndiff --git a/commit.c b/commit.c\nindex 93faaad764..794dc8b593 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1134,8 +1134,10 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tstruct merge_remote_desc *desc;\n \tstruct commit_extra_header *mergetag;\n \tchar *buf;\n-\tunsigned long size, len;\n+\tunsigned long size;\n \tenum object_type type;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \tdesc = merge_remote_util(parent);\n \tif (!desc || !desc->obj)\n@@ -1143,8 +1145,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tbuf = read_object_file(&desc->obj->oid, &type, &size);\n \tif (!buf || type != OBJ_TAG)\n \t\tgoto free_return;\n-\tlen = parse_signature(buf, size);\n-\tif (size == len)\n+\tif (!parse_signature(buf, size, &payload, &signature))\n \t\tgoto free_return;\n \t/*\n \t * We could verify this signature and either omit the tag when\n@@ -1163,6 +1164,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \n \t**tail = mergetag;\n \t*tail = &mergetag->next;\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn;\n \n free_return:\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 9a664a4a58..7fd99f0ac1 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -509,22 +509,28 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \tfor (i = 0; i < origins.nr; i++) {\n \t\tstruct object_id *oid = origins.items[i].util;\n \t\tenum object_type type;\n-\t\tunsigned long size, len;\n+\t\tunsigned long size;\n \t\tchar *buf = read_object_file(oid, &type, &size);\n+\t\tchar *origbuf = buf;\n+\t\tunsigned long len = size;\n \t\tstruct signature_check sigc = { NULL };\n-\t\tstruct strbuf sig = STRBUF_INIT;\n+\t\tstruct strbuf payload = STRBUF_INIT, sig = STRBUF_INIT;\n \n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n-\t\tlen = parse_signature(buf, size);\n \n-\t\tif (size == len)\n-\t\t\t; /* merely annotated */\n-\t\telse if (check_signature(buf, len, buf + len, size - len, &sigc) &&\n-\t\t\t!sigc.gpg_output)\n-\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n-\t\telse\n-\t\t\tstrbuf_addstr(&sig, sigc.gpg_output);\n+\t\tif (!parse_signature(buf, size, &payload, &sig))\n+\t\t\t;/* merely annotated */\n+\t\telse {\n+\t\t\tbuf = payload.buf;\n+\t\t\tlen = payload.len;\n+\t\t\tif (check_signature(payload.buf, payload.len, sig.buf,\n+\t\t\t\t\t sig.len, &sigc) &&\n+\t\t\t\t!sigc.gpg_output)\n+\t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n+\t\t\telse\n+\t\t\t\tstrbuf_addstr(&sig, sigc.gpg_output);\n+\t\t}\n \t\tsignature_check_clear(&sigc);\n \n \t\tif (!tag_number++) {\n@@ -547,9 +553,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\t\t\tstrlen(origins.items[i].string));\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n+\t\tstrbuf_release(&payload);\n \t\tstrbuf_release(&sig);\n \tnext:\n-\t\tfree(buf);\n+\t\tfree(origbuf);\n \t}\n \tif (tagbuf.len) {\n \t\tstrbuf_addch(out, '\\n');\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex b499270836..c6274c14af 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -345,7 +345,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-size_t parse_signature(const char *buf, size_t size)\n+size_t parse_signed_buffer(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n \tsize_t match = size;\n@@ -361,6 +361,17 @@ size_t parse_signature(const char *buf, size_t size)\n \treturn match;\n }\n \n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature)\n+{\n+\tsize_t match = parse_signed_buffer(buf, size);\n+\tif (match != size) {\n+\t\tstrbuf_add(payload, buf, match);\n+\t\tstrbuf_add(signature, buf + match, size - match);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n void set_signing_key(const char *key)\n {\n \tfree(configured_signing_key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex f4e9b4f371..80567e4894 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -37,13 +37,20 @@ struct signature_check {\n \n void signature_check_clear(struct signature_check *sigc);\n \n+/*\n+ * Look at a GPG signed tag object. If such a signature exists, store it in\n+ * signature and the signed content in payload. Return 1 if a signature was\n+ * found, and 0 otherwise.\n+ */\n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature);\n+\n /*\n * Look at GPG signed content (e.g. a signed tag object), whose\n * payload is followed by a detached signature on it. Return the\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n */\n-size_t parse_signature(const char *buf, size_t size);\n+size_t parse_signed_buffer(const char *buf, size_t size);\n \n /*\n * Create a detached signature for the contents of \"buffer\" and append\ndiff --git a/log-tree.c b/log-tree.c\nindex 7e0335e548..b025c8da93 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -548,7 +548,8 @@ static int show_one_mergetag(struct commit *commit,\n \tstruct strbuf verify_message;\n \tstruct signature_check sigc = { 0 };\n \tint status, nth;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \thash_object_file(the_hash_algo, extra->value, extra->len,\n \t\t\t type_name(OBJ_TAG), &oid);\n@@ -571,13 +572,11 @@ static int show_one_mergetag(struct commit *commit,\n \t\tstrbuf_addf(&verify_message,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \n-\tpayload_size = parse_signature(extra->value, extra->len);\n \tstatus = -1;\n-\tif (extra->len > payload_size) {\n+\tif (parse_signature(extra->value, extra->len, &payload, &signature)) {\n \t\t/* could have a good signature */\n-\t\tstatus = check_signature(extra->value, payload_size,\n-\t\t\t\t\t extra->value + payload_size,\n-\t\t\t\t\t extra->len - payload_size, &sigc);\n+\t\tstatus = check_signature(payload.buf, payload.len,\n+\t\t\t\t\t signature.buf, signature.len, &sigc);\n \t\tif (sigc.gpg_output)\n \t\t\tstrbuf_addstr(&verify_message, sigc.gpg_output);\n \t\telse\n@@ -588,6 +587,8 @@ static int show_one_mergetag(struct commit *commit,\n \n \tshow_sig_lines(opt, status, verify_message.buf);\n \tstrbuf_release(&verify_message);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex aa260bfd09..8d8baec1b5 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1215,7 +1215,13 @@ static void find_subpos(const char *buf,\n \t\t\tunsigned long *nonsiglen,\n \t\t\tconst char **sig, unsigned long *siglen)\n {\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tconst char *eol;\n+\tconst char *end = buf + strlen(buf);\n+\tconst char *sigstart;\n+\n+\n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n \t\teol = strchrnul(buf, '\\n');\n@@ -1228,14 +1234,15 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \n \t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n-\t*siglen = strlen(*sig);\n+\tparse_signature(buf, end - buf, &payload, &signature);\n+\t*sig = strbuf_detach(&signature, siglen);\n+\tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n \t/* subject is first non-empty line */\n \t*sub = buf;\n \t/* subject goes to first empty line before signature begins */\n \tif ((eol = strstr(*sub, \"\\n\\n\"))) {\n-\t\teol = eol < *sig ? eol : *sig;\n+\t\teol = eol < sigstart ? eol : sigstart;\n \t/* check if message uses CRLF */\n \t} else if (! (eol = strstr(*sub, \"\\r\\n\\r\\n\"))) {\n \t\t/* treat whole message as subject */\n@@ -1253,7 +1260,7 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \t*body = buf;\n \t*bodylen = strlen(buf);\n-\t*nonsiglen = *sig - buf;\n+\t*nonsiglen = sigstart - buf;\n }\n \n /*\n@@ -1291,6 +1298,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\tstruct used_atom *atom = &used_atom[i];\n \t\tconst char *name = atom->name;\n \t\tstruct atom_value *v = &val[i];\n+\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -1336,6 +1344,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n+\n+\t\tfree((void *)sigpos);\n \t}\n }\n \ndiff --git a/tag.c b/tag.c\nindex 1ed2684e45..3e18a41841 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -13,26 +13,27 @@ const char *tag_type = \"tag\";\n static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n {\n \tstruct signature_check sigc;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tint ret;\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n-\tpayload_size = parse_signature(buf, size);\n-\n-\tif (size == payload_size) {\n+\tif (!parse_signature(buf, size, &payload, &signature)) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n-\t\t\twrite_in_full(1, buf, payload_size);\n+\t\t\twrite_in_full(1, buf, size);\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(buf, payload_size, buf + payload_size,\n-\t\t\t\tsize - payload_size, &sigc);\n+\tret = check_signature(payload.buf, payload.len, signature.buf,\n+\t\t\t\tsignature.len, &sigc);\n \n \tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n \t\tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn ret;\n }\n \n"},{"id":"414008","messageId":"20210111035840.2437737-6-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111035840.2437737-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 5/5] gpg-interface: remove other signature headers before verifying","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T03:58:40Z","receivedAt":"2021-01-11T04:00:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we have a multiply signed commit, we need to remove the signature\nin the header before verifying the object, since the trailing signature\nwill not be over both pieces of data. Do so, and verify that we\nvalidate the signature appropriately.\n\nSigned-off-by: brian m. carlson \n---\n gpg-interface.c | 2 ++\n t/t7004-tag.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c6274c14af..127aecfc2b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"commit.h\"\n #include \"config.h\"\n #include \"run-command.h\"\n #include \"strbuf.h\"\n@@ -366,6 +367,7 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct\n \tsize_t match = parse_signed_buffer(buf, size);\n \tif (match != size) {\n \t\tstrbuf_add(payload, buf, match);\n+\t\tremove_signature(payload);\n \t\tstrbuf_add(signature, buf + match, size - match);\n \t\treturn 1;\n \t}\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 05f411c821..6fb4e3cf11 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -17,6 +17,13 @@ tag_exists () {\n \tgit show-ref --quiet --verify refs/tags/\"$1\"\n }\n \n+test_expect_success 'setup' '\n+\ttest_oid_cache <<-EOM\n+\tothersigheader sha1:gpgsig-sha256\n+\tothersigheader sha256:gpgsig\n+\tEOM\n+'\n+\n test_expect_success 'listing all tags in an empty tree should succeed' '\n \tgit tag -l &&\n \tgit tag\n@@ -1371,6 +1378,24 @@ test_expect_success GPG \\\n \t'test_config gpg.program echo &&\n \t test_must_fail git tag -s -m tail tag-gpg-failure'\n \n+# try to produce invalid signature\n+test_expect_success GPG 'git verifies tag is valid with double signature' '\n+\tgit tag -s -m tail tag-gpg-double-sig &&\n+\tgit cat-file tag tag-gpg-double-sig >tag &&\n+\tothersigheader=$(test_oid othersigheader) &&\n+\tsed -ne \"/^\\$/q;p\" tag >new-tag &&\n+\tcat <<-EOM >>new-tag &&\n+\t$othersigheader -----BEGIN PGP SIGNATURE-----\n+\t someinvaliddata\n+\t -----END PGP SIGNATURE-----\n+\tEOM\n+\tsed -e \"1,/^tagger/d\" tag >>new-tag &&\n+\tnew_tag=$(git hash-object -t tag -w new-tag) &&\n+\tgit update-ref refs/tags/tag-gpg-double-sig $new_tag &&\n+\tgit verify-tag tag-gpg-double-sig &&\n+\tgit fsck\n+'\n+\n # try to sign with bad user.signingkey\n test_expect_success GPGSM \\\n \t'git tag -s fails if gpgsm is misconfigured (bad key)' \\\n"},{"id":"414009","messageId":"20210111035840.2437737-2-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111035840.2437737-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 1/5] commit: ignore additional signatures when parsing signed commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T03:58:36Z","receivedAt":"2021-01-11T04:00:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we create a commit with multiple signatures, neither of these\nsignatures includes the other. Consequently, when we produce the\npayload which has been signed so we can verify the commit, we must strip\noff any other signatures, or the payload will differ from what was\nsigned. Do so, and in preparation for verifying with multiple\nalgorithms, pass the algorithm we want to verify into\nparse_signed_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 52 ++++++++++++++++++++++++----------------\n commit.h | 3 ++-\n log-tree.c | 2 +-\n t/t7510-signed-commit.sh | 43 ++++++++++++++++++++++++++++++++-\n 4 files changed, 77 insertions(+), 23 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex f128f18a9b..93faaad764 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1036,20 +1036,18 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n }\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *payload, struct strbuf *signature)\n+\t\t\tstruct strbuf *payload, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop)\n {\n \n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n-\tint in_signature, saw_signature = -1;\n-\tconst char *line, *tail;\n-\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];\n-\tint gpg_sig_header_len = strlen(gpg_sig_header);\n+\tint in_signature = 0, saw_signature = 0, other_signature = 0;\n+\tconst char *line, *tail, *p;\n+\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n \n \tline = buffer;\n \ttail = buffer + size;\n-\tin_signature = 0;\n-\tsaw_signature = 0;\n \twhile (line < tail) {\n \t\tconst char *sig = NULL;\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n@@ -1057,9 +1055,15 @@ int parse_signed_commit(const struct commit *commit,\n \t\tnext = next ? next + 1 : tail;\n \t\tif (in_signature && line[0] == ' ')\n \t\t\tsig = line + 1;\n-\t\telse if (starts_with(line, gpg_sig_header) &&\n-\t\t\t line[gpg_sig_header_len] == ' ')\n-\t\t\tsig = line + gpg_sig_header_len + 1;\n+\t\telse if (skip_prefix(line, gpg_sig_header, &p) &&\n+\t\t\t *p == ' ') {\n+\t\t\tsig = line + strlen(gpg_sig_header) + 1;\n+\t\t\tother_signature = 0;\n+\t\t}\n+\t\telse if (starts_with(line, \"gpgsig\"))\n+\t\t\tother_signature = 1;\n+\t\telse if (other_signature && line[0] != ' ')\n+\t\t\tother_signature = 0;\n \t\tif (sig) {\n \t\t\tstrbuf_add(signature, sig, next - sig);\n \t\t\tsaw_signature = 1;\n@@ -1068,7 +1072,8 @@ int parse_signed_commit(const struct commit *commit,\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n-\t\t\tstrbuf_add(payload, line, next - line);\n+\t\t\tif (!other_signature)\n+\t\t\t\tstrbuf_add(payload, line, next - line);\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n@@ -1082,23 +1087,27 @@ int remove_signature(struct strbuf *buf)\n \tconst char *line = buf->buf;\n \tconst char *tail = buf->buf + buf->len;\n \tint in_signature = 0;\n-\tconst char *sig_start = NULL;\n-\tconst char *sig_end = NULL;\n+\tstruct sigbuf {\n+\t\tconst char *start;\n+\t\tconst char *end;\n+\t} sigs[2] = { 0 }, *sigp = &sigs[0];\n+\tint i;\n+\tconst char *orig_buf = buf->buf;\n \n \twhile (line < tail) {\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n \t\tnext = next ? next + 1 : tail;\n \n \t\tif (in_signature && line[0] == ' ')\n-\t\t\tsig_end = next;\n+\t\t\tsigp->end = next;\n \t\telse if (starts_with(line, \"gpgsig\")) {\n \t\t\tint i;\n \t\t\tfor (i = 1; i < GIT_HASH_NALGOS; i++) {\n \t\t\t\tconst char *p;\n \t\t\t\tif (skip_prefix(line, gpg_sig_headers[i], &p) &&\n \t\t\t\t *p == ' ') {\n-\t\t\t\t\tsig_start = line;\n-\t\t\t\t\tsig_end = next;\n+\t\t\t\t\tsigp->start = line;\n+\t\t\t\t\tsigp->end = next;\n \t\t\t\t\tin_signature = 1;\n \t\t\t\t}\n \t\t\t}\n@@ -1106,15 +1115,18 @@ int remove_signature(struct strbuf *buf)\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n+\t\t\tif (in_signature && sigp - sigs != ARRAY_SIZE(sigs))\n+\t\t\t\tsigp++;\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n \t}\n \n-\tif (sig_start)\n-\t\tstrbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start);\n+\tfor (i = ARRAY_SIZE(sigs) - 1; i >= 0; i--)\n+\t\tif (sigs[i].start)\n+\t\t\tstrbuf_remove(buf, sigs[i].start - orig_buf, sigs[i].end - sigs[i].start);\n \n-\treturn sig_start != NULL;\n+\treturn sigs[0].start != NULL;\n }\n \n static void handle_signed_tag(struct commit *parent, struct commit_extra_header ***tail)\n@@ -1165,7 +1177,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \n \tsigc->result = 'N';\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \tret = check_signature(payload.buf, payload.len, signature.buf,\n \t\tsignature.len, sigc);\ndiff --git a/commit.h b/commit.h\nindex f4e7b0158e..030aa65ab8 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -317,7 +317,8 @@ void set_merge_remote_desc(struct commit *commit,\n struct commit *get_merge_parent(const char *name);\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *message, struct strbuf *signature);\n+\t\t\tstruct strbuf *message, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop);\n int remove_signature(struct strbuf *buf);\n \n /*\ndiff --git a/log-tree.c b/log-tree.c\nindex fd0dde97ec..7e0335e548 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -502,7 +502,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \tstruct signature_check sigc = { 0 };\n \tint status;\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n \tstatus = check_signature(payload.buf, payload.len, signature.buf,\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 6baaa1ad91..d78319d5c8 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -172,7 +172,7 @@ test_expect_success GPG 'show signed commit with signature' '\n \tgit cat-file commit initial >cat &&\n \tgrep -v -e \"gpg: \" -e \"Warning: \" show >show.commit &&\n \tgrep -e \"gpg: \" -e \"Warning: \" show >show.gpg &&\n-\tgrep -v \"^ \" cat | grep -v \"^$(test_oid header) \" >cat.commit &&\n+\tgrep -v \"^ \" cat | grep -v \"^gpgsig.* \" >cat.commit &&\n \ttest_cmp show.commit commit &&\n \ttest_cmp show.gpg verify.2 &&\n \ttest_cmp cat.commit verify.1\n@@ -334,4 +334,45 @@ test_expect_success GPG 'show double signature with custom format' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success GPG 'verify-commit verifies multiply signed commits' '\n+\tgit init multiply-signed &&\n+\tcd multiply-signed &&\n+\ttest_commit first &&\n+\techo 1 >second &&\n+\tgit add second &&\n+\ttree=$(git write-tree) &&\n+\tparent=$(git rev-parse HEAD^{commit}) &&\n+\tgit commit --gpg-sign -m second &&\n+\tgit cat-file commit HEAD &&\n+\t# Avoid trailing whitespace.\n+\tsed -e \"s/^Q//\" -e \"s/^Z/ /\" >commit <<-EOF &&\n+\tQtree $tree\n+\tQparent $parent\n+\tQauthor A U Thor 1112912653 -0700\n+\tQcommitter C O Mitter 1112912653 -0700\n+\tQgpgsig -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBDRYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMNd+8AoK1I8mhLHviPH+q2I5fIVgPsEtYC\n+\tQ AKCTqBh+VabJceXcGIZuF0Ry+udbBQ==\n+\tQ =tQ0N\n+\tQ -----END PGP SIGNATURE-----\n+\tQgpgsig-sha256 -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBIBYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN/NEAn0XO9RYSBj2dFyozi0JKSbssYMtO\n+\tQ AJwKCQ1BQOtuwz//IjU8TiS+6S4iUw==\n+\tQ =pIwP\n+\tQ -----END PGP SIGNATURE-----\n+\tQ\n+\tQsecond\n+\tEOF\n+\thead=$(git hash-object -t commit -w commit) &&\n+\tgit reset --hard $head &&\n+\tgit verify-commit $head 2>actual &&\n+\tgrep \"Good signature from\" actual &&\n+\t! grep \"BAD signature from\" actual\n+'\n+\n test_done\n"},{"id":"414080","messageId":"xmqq5z436rwe.fsf@gitster.c.googlers.com","threadId":"54974","inReplyTo":"20210111035840.2437737-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 0/5] Support for commits signed by multiple algorithms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-11T22:16:33Z","receivedAt":"2021-01-11T22:17:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> This series introduces support for verifying commits and tags signed by\n> multiple algorithms.\n>\n> Originally, we had planned for SHA-256 tags to stuff the signature in a\n> header instead of using a trailing signature, and a patch to do that was\n> sent out in part 1/3. Unfortunately, for whatever reason, that patch\n> didn't make it into the master branch, and so we use trailing signatures\n> there.\n>\n> We can't change this now, because otherwise it would be ambiguous\n> whether the trailing signature on a SHA-256 object was for the SHA-256\n> contents or whether the contents were a rewritten SHA-1 object with no\n> SHA-256 signature at all.\n\nHow widely are SHA-256 tags in use in the real world, though? Is it\nreally too late to fix that already?\n"},{"id":"414083","messageId":"X/zfdA8uUAdx0oEu@camp.crustytoothpaste.net","threadId":"54974","inReplyTo":"xmqq5z436rwe.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 0/5] Support for commits signed by multiple algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-11T23:29:56Z","receivedAt":"2021-01-12T00:27:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-01-11 at 22:16:33, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> \n> > This series introduces support for verifying commits and tags signed by\n> > multiple algorithms.\n> >\n> > Originally, we had planned for SHA-256 tags to stuff the signature in a\n> > header instead of using a trailing signature, and a patch to do that was\n> > sent out in part 1/3. Unfortunately, for whatever reason, that patch\n> > didn't make it into the master branch, and so we use trailing signatures\n> > there.\n> >\n> > We can't change this now, because otherwise it would be ambiguous\n> > whether the trailing signature on a SHA-256 object was for the SHA-256\n> > contents or whether the contents were a rewritten SHA-1 object with no\n> > SHA-256 signature at all.\n> \n> How widely are SHA-256 tags in use in the real world, though? Is it\n> really too late to fix that already?\n\nI don't know. I don't know of any major hosting platform that supports\nthem, but of course many people may be using them independently on\nself-hosted instances.\n\nI don't think it matters one way or the other, honestly, because the\nfunctionality is the same either way, whether we always put SHA-256 in a\nheader or whether we put the non-default algorithm in the header.\nMultiply signed commits and tags are still unverifiable on older\nversions because the older versions consider the header to be part of\nthe payload and not something to be stripped.\n\nI just noticed this because I'm now getting to the case where we write\n(and sign) both SHA-1 and SHA-256 versions of commits and I thought I'd\nbetter send in a patch sooner rather than later, since there's a lot\nmore prep work (surprise) before we get to anything interesting.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"414087","messageId":"xmqqft366her.fsf@gitster.c.googlers.com","threadId":"54974","inReplyTo":"X/zfdA8uUAdx0oEu@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 0/5] Support for commits signed by multiple algorithms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T02:03:08Z","receivedAt":"2021-01-12T02:03:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> I just noticed this because I'm now getting to the case where we write\n> (and sign) both SHA-1 and SHA-256 versions of commits and I thought I'd\n> better send in a patch sooner rather than later, since there's a lot\n> more prep work (surprise) before we get to anything interesting.\n\nUncomfortably excited to hear this ;-)\n"},{"id":"414088","messageId":"X/0IaVkxqbYxKJBf@camp.crustytoothpaste.net","threadId":"54974","inReplyTo":"xmqqft366her.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 0/5] Support for commits signed by multiple algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-12T02:24:41Z","receivedAt":"2021-01-12T02:25:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-01-12 at 02:03:08, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> \n> > I just noticed this because I'm now getting to the case where we write\n> > (and sign) both SHA-1 and SHA-256 versions of commits and I thought I'd\n> > better send in a patch sooner rather than later, since there's a lot\n> > more prep work (surprise) before we get to anything interesting.\n> \n> Uncomfortably excited to hear this ;-)\n\nHere's a brief summary of what's ahead:\n\n* struct object_id is going to have an algorithm member.\n* Consequently, there will be per-algorithm null OIDs.\n* To efficiently compare and copy OIDs of all sizes (notably in khash\n tables, where things otherwise become tricky), we'll zero-pad SHA-1\n OIDs and always compare the full buffer.\n* For all of these reasons, oidread (or similar) will become practically\n required.\n* Objects will be mapped into the loose object store when written, and\n each type of object will have a function to convert it between formats\n if required.\n* The testsuite will learn a mode not to stuff invalid OIDs into things,\n since those will no longer work (because those OIDs can't be mapped\n and so we can't create SHA-1 versions of them). That will necessitate\n another large set of prerequisite additions in the testsuite.\n\nIn progress work can be seen on the transition-interop branch at\nhttps://github.com/bk2204/git.git. I should point out that it is very\nin progress; the tip will definitely fail the testsuite in certain\ncases.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"414093","messageId":"xmqqbldu699q.fsf@gitster.c.googlers.com","threadId":"54974","inReplyTo":"20210111003740.1319996-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH 2/5] gpg-interface: improve interface for parsing tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T04:58:57Z","receivedAt":"2021-01-12T04:59:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index aa260bfd09..8d8baec1b5 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1215,7 +1215,13 @@ static void find_subpos(const char *buf,\n> \t\t\tunsigned long *nonsiglen,\n> \t\t\tconst char **sig, unsigned long *siglen)\n> {\n> +\tstruct strbuf payload = STRBUF_INIT;\n> +\tstruct strbuf signature = STRBUF_INIT;\n> \tconst char *eol;\n> +\tconst char *end = buf + strlen(buf);\n> +\tconst char *sigstart;\n> +\n> +\n> \t/* skip past header until we hit empty line */\n> \twhile (*buf && *buf != '\\n') {\n> \t\teol = strchrnul(buf, '\\n');\n> @@ -1228,14 +1234,15 @@ static void find_subpos(const char *buf,\n> \t\tbuf++;\n> \n> \t/* parse signature first; we might not even have a subject line */\n> -\t*sig = buf + parse_signature(buf, strlen(buf));\n> -\t*siglen = strlen(*sig);\n> +\tparse_signature(buf, end - buf, &payload, &signature);\n> +\t*sig = strbuf_detach(&signature, siglen);\n\n\"unsigned long *siglen\" may not be the same as \"size_t *siglen\", and\nthe latter is what strbuf_detach() expects to see. This breaks\n32-bit builds e.g. [*1*].\n\nI suspect that all these ${foo}len, including the parameter to this\nfunction but also the four local variables in its sole caller, would\nwant to become size_t.\n\nThanks.\n\n\n[Reference]\n\n*1* https://github.com/git/git/runs/1685453231?check_suite_focus=true#step:5:519\n"},{"id":"414094","messageId":"xmqq7doi682z.fsf@gitster.c.googlers.com","threadId":"54974","inReplyTo":"20210111035840.2437737-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 2/5] gpg-interface: improve interface for parsing tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-12T05:24:36Z","receivedAt":"2021-01-12T05:25:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index aa260bfd09..8d8baec1b5 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1215,7 +1215,13 @@ static void find_subpos(const char *buf,\n> \t\t\tunsigned long *nonsiglen,\n> \t\t\tconst char **sig, unsigned long *siglen)\n> {\n\nI think a preliminary clean-up should look like this.\n\nTonight's tip of 'seen' contains this at the tip of your topic.\n\n https://github.com/git/git/actions/runs/479421349\n\nThanks.\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 32ed4d5111..e6c8106377 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1210,10 +1210,10 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n }\n \n static void find_subpos(const char *buf,\n-\t\t\tconst char **sub, unsigned long *sublen,\n-\t\t\tconst char **body, unsigned long *bodylen,\n-\t\t\tunsigned long *nonsiglen,\n-\t\t\tconst char **sig, unsigned long *siglen)\n+\t\t\tconst char **sub, size_t *sublen,\n+\t\t\tconst char **body, size_t *bodylen,\n+\t\t\tsize_t *nonsiglen,\n+\t\t\tconst char **sig, size_t *siglen)\n {\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n@@ -1291,7 +1291,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n-\tunsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n+\tsize_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n"},{"id":"414181","messageId":"20210112170344.GX8396@szeder.dev","threadId":"54974","inReplyTo":"20210111035840.2437737-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 1/5] commit: ignore additional signatures when parsing signed commits","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-01-12T17:03:44Z","receivedAt":"2021-01-12T17:04:36Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jan 11, 2021 at 03:58:36AM +0000, brian m. carlson wrote:\n> diff --git a/commit.c b/commit.c\n> index f128f18a9b..93faaad764 100644\n> --- a/commit.c\n> +++ b/commit.c\n\n> @@ -1082,23 +1087,27 @@ int remove_signature(struct strbuf *buf)\n> \tconst char *line = buf->buf;\n> \tconst char *tail = buf->buf + buf->len;\n> \tint in_signature = 0;\n> -\tconst char *sig_start = NULL;\n> -\tconst char *sig_end = NULL;\n> +\tstruct sigbuf {\n> +\t\tconst char *start;\n> +\t\tconst char *end;\n> +\t} sigs[2] = { 0 }, *sigp = &sigs[0];\n\nVarious Clang versions issue the following warning about this\ninitialization:\n\n CC commit.o\ncommit.c:1105:16: error: suggest braces around initialization of subobject\n [-Werror,-Wmissing-braces]\n } sigs[2] = { 0 }, *sigp = &sigs[0];\n ^\n {}\n1 error generated.\n\n"},{"id":"414398","messageId":"YADRP22NG15AHoAB@camp.crustytoothpaste.net","threadId":"54974","inReplyTo":"xmqqbldu699q.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/5] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-14T23:18:23Z","receivedAt":"2021-01-14T23:19:41Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-01-12 at 04:58:57, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> \n> > diff --git a/ref-filter.c b/ref-filter.c\n> > index aa260bfd09..8d8baec1b5 100644\n> > --- a/ref-filter.c\n> > +++ b/ref-filter.c\n> > @@ -1215,7 +1215,13 @@ static void find_subpos(const char *buf,\n> > \t\t\tunsigned long *nonsiglen,\n> > \t\t\tconst char **sig, unsigned long *siglen)\n> > {\n> > +\tstruct strbuf payload = STRBUF_INIT;\n> > +\tstruct strbuf signature = STRBUF_INIT;\n> > \tconst char *eol;\n> > +\tconst char *end = buf + strlen(buf);\n> > +\tconst char *sigstart;\n> > +\n> > +\n> > \t/* skip past header until we hit empty line */\n> > \twhile (*buf && *buf != '\\n') {\n> > \t\teol = strchrnul(buf, '\\n');\n> > @@ -1228,14 +1234,15 @@ static void find_subpos(const char *buf,\n> > \t\tbuf++;\n> > \n> > \t/* parse signature first; we might not even have a subject line */\n> > -\t*sig = buf + parse_signature(buf, strlen(buf));\n> > -\t*siglen = strlen(*sig);\n> > +\tparse_signature(buf, end - buf, &payload, &signature);\n> > +\t*sig = strbuf_detach(&signature, siglen);\n> \n> \"unsigned long *siglen\" may not be the same as \"size_t *siglen\", and\n> the latter is what strbuf_detach() expects to see. This breaks\n> 32-bit builds e.g. [*1*].\n> \n> I suspect that all these ${foo}len, including the parameter to this\n> function but also the four local variables in its sole caller, would\n> want to become size_t.\n> \n> Thanks.\n\nI'll reroll with the fixes for size_t and unsigned long. Feel free to\ndrop the series if it's causing CI to fail until I get around to that\n(likely this weekend).\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"414414","messageId":"xmqqzh1bug2s.fsf@gitster.c.googlers.com","threadId":"54974","inReplyTo":"YADRP22NG15AHoAB@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/5] gpg-interface: improve interface for parsing tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-15T01:47:07Z","receivedAt":"2021-01-15T01:47:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> I'll reroll with the fixes for size_t and unsigned long. Feel free to\n> drop the series if it's causing CI to fail until I get around to that\n> (likely this weekend).\n\nNo worries. You would have found my tentative fix-up applied in\n'seen' already if you updated from me when you got the message you\nare responding to.\n\nThanks.\n"},{"id":"414638","messageId":"20210118234915.2036197-1-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210111035840.2437737-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 0/6] Support for commits signed by multiple algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-18T23:49:09Z","receivedAt":"2021-01-18T23:50:53Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces support for verifying commits and tags signed by\nmultiple algorithms.\n\nThese commits allow for signing commits and tags that can be\nround-tripped through both SHA-1 and SHA-256. We verify only the\nsignature using the current hash algorithm, since we currently don't\nrewrite objects.\n\nChanges from v2:\n* Fix problems with size_t and unsigned long on 32-bit systems by\n creating a cleanup patch at the beginning of the series.\n* Fix a clang warning with initialization by using memset.\n\nChanges from v1:\n* Fix brown paper bag bug where some tests failed due to a bad rebase.\n\nbrian m. carlson (6):\n ref-filter: switch some uses of unsigned long to size_t\n commit: ignore additional signatures when parsing signed commits\n gpg-interface: improve interface for parsing tags\n commit: allow parsing arbitrary buffers with headers\n ref-filter: hoist signature parsing\n gpg-interface: remove other signature headers before verifying\n\n builtin/receive-pack.c | 4 +-\n builtin/tag.c | 16 ++++++--\n commit.c | 85 +++++++++++++++++++++++++++-------------\n commit.h | 12 +++++-\n fmt-merge-msg.c | 29 ++++++++------\n gpg-interface.c | 15 ++++++-\n gpg-interface.h | 9 ++++-\n log-tree.c | 15 +++----\n ref-filter.c | 33 ++++++++++------\n t/t7004-tag.sh | 25 ++++++++++++\n t/t7510-signed-commit.sh | 43 +++++++++++++++++++-\n tag.c | 15 +++----\n 12 files changed, 226 insertions(+), 75 deletions(-)\n\n"},{"id":"414639","messageId":"20210118234915.2036197-2-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210118234915.2036197-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 1/6] ref-filter: switch some uses of unsigned long to size_t","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-18T23:49:10Z","receivedAt":"2021-01-18T23:51:22Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In the future, we'll want to pass some of the arguments of find_subpos\nto strbuf_detach, which takes a size_t. This is fine on systems where\nthat's the same size as unsigned long, but that isn't the case on all\nsystems. Moreover, size_t makes sense since it's not possible to use a\nbuffer here that's larger than memory anyway.\n\nLet's switch each use to size_t for these lengths in\ngrab_sub_body_contents and find_subpos.\n\nSigned-off-by: brian m. carlson \n---\n ref-filter.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex aa260bfd09..606f638ab1 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1210,10 +1210,10 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n }\n \n static void find_subpos(const char *buf,\n-\t\t\tconst char **sub, unsigned long *sublen,\n-\t\t\tconst char **body, unsigned long *bodylen,\n-\t\t\tunsigned long *nonsiglen,\n-\t\t\tconst char **sig, unsigned long *siglen)\n+\t\t\tconst char **sub, size_t *sublen,\n+\t\t\tconst char **body, size_t *bodylen,\n+\t\t\tsize_t *nonsiglen,\n+\t\t\tconst char **sig, size_t *siglen)\n {\n \tconst char *eol;\n \t/* skip past header until we hit empty line */\n@@ -1285,7 +1285,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n-\tunsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n+\tsize_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n"},{"id":"414640","messageId":"20210118234915.2036197-3-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210118234915.2036197-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 2/6] commit: ignore additional signatures when parsing signed commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-18T23:49:11Z","receivedAt":"2021-01-18T23:51:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we create a commit with multiple signatures, neither of these\nsignatures includes the other. Consequently, when we produce the\npayload which has been signed so we can verify the commit, we must strip\noff any other signatures, or the payload will differ from what was\nsigned. Do so, and in preparation for verifying with multiple\nalgorithms, pass the algorithm we want to verify into\nparse_signed_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 54 +++++++++++++++++++++++++---------------\n commit.h | 3 ++-\n log-tree.c | 2 +-\n t/t7510-signed-commit.sh | 43 +++++++++++++++++++++++++++++++-\n 4 files changed, 79 insertions(+), 23 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex bab8d5ab07..1006c85ca8 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1036,20 +1036,18 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n }\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *payload, struct strbuf *signature)\n+\t\t\tstruct strbuf *payload, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop)\n {\n \n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n-\tint in_signature, saw_signature = -1;\n-\tconst char *line, *tail;\n-\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];\n-\tint gpg_sig_header_len = strlen(gpg_sig_header);\n+\tint in_signature = 0, saw_signature = 0, other_signature = 0;\n+\tconst char *line, *tail, *p;\n+\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n \n \tline = buffer;\n \ttail = buffer + size;\n-\tin_signature = 0;\n-\tsaw_signature = 0;\n \twhile (line < tail) {\n \t\tconst char *sig = NULL;\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n@@ -1057,9 +1055,15 @@ int parse_signed_commit(const struct commit *commit,\n \t\tnext = next ? next + 1 : tail;\n \t\tif (in_signature && line[0] == ' ')\n \t\t\tsig = line + 1;\n-\t\telse if (starts_with(line, gpg_sig_header) &&\n-\t\t\t line[gpg_sig_header_len] == ' ')\n-\t\t\tsig = line + gpg_sig_header_len + 1;\n+\t\telse if (skip_prefix(line, gpg_sig_header, &p) &&\n+\t\t\t *p == ' ') {\n+\t\t\tsig = line + strlen(gpg_sig_header) + 1;\n+\t\t\tother_signature = 0;\n+\t\t}\n+\t\telse if (starts_with(line, \"gpgsig\"))\n+\t\t\tother_signature = 1;\n+\t\telse if (other_signature && line[0] != ' ')\n+\t\t\tother_signature = 0;\n \t\tif (sig) {\n \t\t\tstrbuf_add(signature, sig, next - sig);\n \t\t\tsaw_signature = 1;\n@@ -1068,7 +1072,8 @@ int parse_signed_commit(const struct commit *commit,\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n-\t\t\tstrbuf_add(payload, line, next - line);\n+\t\t\tif (!other_signature)\n+\t\t\t\tstrbuf_add(payload, line, next - line);\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n@@ -1082,23 +1087,29 @@ int remove_signature(struct strbuf *buf)\n \tconst char *line = buf->buf;\n \tconst char *tail = buf->buf + buf->len;\n \tint in_signature = 0;\n-\tconst char *sig_start = NULL;\n-\tconst char *sig_end = NULL;\n+\tstruct sigbuf {\n+\t\tconst char *start;\n+\t\tconst char *end;\n+\t} sigs[2], *sigp = &sigs[0];\n+\tint i;\n+\tconst char *orig_buf = buf->buf;\n+\n+\tmemset(sigs, 0, sizeof(sigs));\n \n \twhile (line < tail) {\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n \t\tnext = next ? next + 1 : tail;\n \n \t\tif (in_signature && line[0] == ' ')\n-\t\t\tsig_end = next;\n+\t\t\tsigp->end = next;\n \t\telse if (starts_with(line, \"gpgsig\")) {\n \t\t\tint i;\n \t\t\tfor (i = 1; i < GIT_HASH_NALGOS; i++) {\n \t\t\t\tconst char *p;\n \t\t\t\tif (skip_prefix(line, gpg_sig_headers[i], &p) &&\n \t\t\t\t *p == ' ') {\n-\t\t\t\t\tsig_start = line;\n-\t\t\t\t\tsig_end = next;\n+\t\t\t\t\tsigp->start = line;\n+\t\t\t\t\tsigp->end = next;\n \t\t\t\t\tin_signature = 1;\n \t\t\t\t}\n \t\t\t}\n@@ -1106,15 +1117,18 @@ int remove_signature(struct strbuf *buf)\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n+\t\t\tif (in_signature && sigp - sigs != ARRAY_SIZE(sigs))\n+\t\t\t\tsigp++;\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n \t}\n \n-\tif (sig_start)\n-\t\tstrbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start);\n+\tfor (i = ARRAY_SIZE(sigs) - 1; i >= 0; i--)\n+\t\tif (sigs[i].start)\n+\t\t\tstrbuf_remove(buf, sigs[i].start - orig_buf, sigs[i].end - sigs[i].start);\n \n-\treturn sig_start != NULL;\n+\treturn sigs[0].start != NULL;\n }\n \n static void handle_signed_tag(struct commit *parent, struct commit_extra_header ***tail)\n@@ -1165,7 +1179,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \n \tsigc->result = 'N';\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \tret = check_signature(payload.buf, payload.len, signature.buf,\n \t\tsignature.len, sigc);\ndiff --git a/commit.h b/commit.h\nindex f4e7b0158e..030aa65ab8 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -317,7 +317,8 @@ void set_merge_remote_desc(struct commit *commit,\n struct commit *get_merge_parent(const char *name);\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *message, struct strbuf *signature);\n+\t\t\tstruct strbuf *message, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop);\n int remove_signature(struct strbuf *buf);\n \n /*\ndiff --git a/log-tree.c b/log-tree.c\nindex fd0dde97ec..7e0335e548 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -502,7 +502,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \tstruct signature_check sigc = { 0 };\n \tint status;\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n \tstatus = check_signature(payload.buf, payload.len, signature.buf,\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 6baaa1ad91..d78319d5c8 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -172,7 +172,7 @@ test_expect_success GPG 'show signed commit with signature' '\n \tgit cat-file commit initial >cat &&\n \tgrep -v -e \"gpg: \" -e \"Warning: \" show >show.commit &&\n \tgrep -e \"gpg: \" -e \"Warning: \" show >show.gpg &&\n-\tgrep -v \"^ \" cat | grep -v \"^$(test_oid header) \" >cat.commit &&\n+\tgrep -v \"^ \" cat | grep -v \"^gpgsig.* \" >cat.commit &&\n \ttest_cmp show.commit commit &&\n \ttest_cmp show.gpg verify.2 &&\n \ttest_cmp cat.commit verify.1\n@@ -334,4 +334,45 @@ test_expect_success GPG 'show double signature with custom format' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success GPG 'verify-commit verifies multiply signed commits' '\n+\tgit init multiply-signed &&\n+\tcd multiply-signed &&\n+\ttest_commit first &&\n+\techo 1 >second &&\n+\tgit add second &&\n+\ttree=$(git write-tree) &&\n+\tparent=$(git rev-parse HEAD^{commit}) &&\n+\tgit commit --gpg-sign -m second &&\n+\tgit cat-file commit HEAD &&\n+\t# Avoid trailing whitespace.\n+\tsed -e \"s/^Q//\" -e \"s/^Z/ /\" >commit <<-EOF &&\n+\tQtree $tree\n+\tQparent $parent\n+\tQauthor A U Thor 1112912653 -0700\n+\tQcommitter C O Mitter 1112912653 -0700\n+\tQgpgsig -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBDRYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMNd+8AoK1I8mhLHviPH+q2I5fIVgPsEtYC\n+\tQ AKCTqBh+VabJceXcGIZuF0Ry+udbBQ==\n+\tQ =tQ0N\n+\tQ -----END PGP SIGNATURE-----\n+\tQgpgsig-sha256 -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBIBYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN/NEAn0XO9RYSBj2dFyozi0JKSbssYMtO\n+\tQ AJwKCQ1BQOtuwz//IjU8TiS+6S4iUw==\n+\tQ =pIwP\n+\tQ -----END PGP SIGNATURE-----\n+\tQ\n+\tQsecond\n+\tEOF\n+\thead=$(git hash-object -t commit -w commit) &&\n+\tgit reset --hard $head &&\n+\tgit verify-commit $head 2>actual &&\n+\tgrep \"Good signature from\" actual &&\n+\t! grep \"BAD signature from\" actual\n+'\n+\n test_done\n"},{"id":"414641","messageId":"20210118234915.2036197-5-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210118234915.2036197-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 4/6] commit: allow parsing arbitrary buffers with headers","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-18T23:49:13Z","receivedAt":"2021-01-18T23:51:38Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently only commits are signed with headers. However, in the future,\nwe'll also sign tags with headers as well. Let's refactor out a\nfunction called parse_buffer_signed_by_header which does exactly that.\nIn addition, since we'll want to sign things other than commits this\nway, let's call the function sign_with_header instead of do_sign_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 20 ++++++++++++++++----\n commit.h | 9 +++++++++\n 2 files changed, 25 insertions(+), 4 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex ccb912b9b5..2c57b6ded9 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -995,7 +995,7 @@ static const char *gpg_sig_headers[] = {\n \t\"gpgsig-sha256\",\n };\n \n-static int do_sign_commit(struct strbuf *buf, const char *keyid)\n+int sign_with_header(struct strbuf *buf, const char *keyid)\n {\n \tstruct strbuf sig = STRBUF_INIT;\n \tint inspos, copypos;\n@@ -1035,13 +1035,26 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n \treturn 0;\n }\n \n+\n+\n int parse_signed_commit(const struct commit *commit,\n \t\t\tstruct strbuf *payload, struct strbuf *signature,\n \t\t\tconst struct git_hash_algo *algop)\n {\n-\n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n+\tint ret = parse_buffer_signed_by_header(buffer, size, payload, signature, algop);\n+\n+\tunuse_commit_buffer(commit, buffer);\n+\treturn ret;\n+}\n+\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop)\n+{\n \tint in_signature = 0, saw_signature = 0, other_signature = 0;\n \tconst char *line, *tail, *p;\n \tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n@@ -1078,7 +1091,6 @@ int parse_signed_commit(const struct commit *commit,\n \t\t}\n \t\tline = next;\n \t}\n-\tunuse_commit_buffer(commit, buffer);\n \treturn saw_signature;\n }\n \n@@ -1532,7 +1544,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,\n \tif (encoding_is_utf8 && !verify_utf8(&buffer))\n \t\tfprintf(stderr, _(commit_utf8_warn));\n \n-\tif (sign_commit && do_sign_commit(&buffer, sign_commit)) {\n+\tif (sign_commit && sign_with_header(&buffer, sign_commit)) {\n \t\tresult = -1;\n \t\tgoto out;\n \t}\ndiff --git a/commit.h b/commit.h\nindex 030aa65ab8..e2856ce8ef 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -360,4 +360,13 @@ int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void\n LAST_ARG_MUST_BE_NULL\n int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);\n \n+/* Sign a commit or tag buffer, storing the result in a header. */\n+int sign_with_header(struct strbuf *buf, const char *keyid);\n+/* Parse the signature out of a header. */\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop);\n+\n #endif /* COMMIT_H */\n"},{"id":"414642","messageId":"20210118234915.2036197-4-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210118234915.2036197-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 3/6] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-18T23:49:12Z","receivedAt":"2021-01-18T23:51:40Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have a function which parses a buffer with a signature at the end,\nparse_signature, and this function is used for signed tags. However,\nwe'll need to store values for multiple algorithms, and we'll do this by\nusing a header for the non-default algorithm.\n\nAdjust the parse_signature interface to store the parsed data in two\nstrbufs and turn the existing function into parse_signed_buffer. The\nlatter is still used in places where we know we always have a signed\nbuffer, such as push certs.\n\nAdjust all the callers to deal with this new interface.\n\nSigned-off-by: brian m. carlson \n---\n builtin/receive-pack.c | 4 ++--\n builtin/tag.c | 16 ++++++++++++----\n commit.c | 9 ++++++---\n fmt-merge-msg.c | 29 ++++++++++++++++++-----------\n gpg-interface.c | 13 ++++++++++++-\n gpg-interface.h | 9 ++++++++-\n log-tree.c | 13 +++++++------\n ref-filter.c | 18 ++++++++++++++----\n tag.c | 15 ++++++++-------\n 9 files changed, 87 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d49d050e6e..b89ce31bf2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -764,7 +764,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \n-\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tbogs = parse_signed_buffer(push_cert.buf, push_cert.len);\n \t\tcheck_signature(push_cert.buf, bogs, push_cert.buf + bogs,\n \t\t\t\tpush_cert.len - bogs, &sigcheck);\n \n@@ -2050,7 +2050,7 @@ static void queue_commands_from_cert(struct command **tail,\n \t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n \telse\n \t\tboc += 2;\n-\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\teoc = push_cert->buf + parse_signed_buffer(push_cert->buf, push_cert->len);\n \n \twhile (boc < eoc) {\n \t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex ecf011776d..7162f4ccc5 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -174,11 +174,17 @@ static void write_tag_body(int fd, const struct object_id *oid)\n {\n \tunsigned long size;\n \tenum object_type type;\n-\tchar *buf, *sp;\n+\tchar *buf, *sp, *orig;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n-\tbuf = read_object_file(oid, &type, &size);\n+\torig = buf = read_object_file(oid, &type, &size);\n \tif (!buf)\n \t\treturn;\n+\tif (parse_signature(buf, size, &payload, &signature)) {\n+\t\tbuf = payload.buf;\n+\t\tsize = payload.len;\n+\t}\n \t/* skip header */\n \tsp = strstr(buf, \"\\n\\n\");\n \n@@ -187,9 +193,11 @@ static void write_tag_body(int fd, const struct object_id *oid)\n \t\treturn;\n \t}\n \tsp += 2; /* skip the 2 LFs */\n-\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp));\n+\twrite_or_die(fd, sp, buf + size - sp);\n \n-\tfree(buf);\n+\tfree(orig);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n }\n \n static int build_tag_object(struct strbuf *buf, int sign, struct object_id *result)\ndiff --git a/commit.c b/commit.c\nindex 1006c85ca8..ccb912b9b5 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1136,8 +1136,10 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tstruct merge_remote_desc *desc;\n \tstruct commit_extra_header *mergetag;\n \tchar *buf;\n-\tunsigned long size, len;\n+\tunsigned long size;\n \tenum object_type type;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \tdesc = merge_remote_util(parent);\n \tif (!desc || !desc->obj)\n@@ -1145,8 +1147,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tbuf = read_object_file(&desc->obj->oid, &type, &size);\n \tif (!buf || type != OBJ_TAG)\n \t\tgoto free_return;\n-\tlen = parse_signature(buf, size);\n-\tif (size == len)\n+\tif (!parse_signature(buf, size, &payload, &signature))\n \t\tgoto free_return;\n \t/*\n \t * We could verify this signature and either omit the tag when\n@@ -1165,6 +1166,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \n \t**tail = mergetag;\n \t*tail = &mergetag->next;\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn;\n \n free_return:\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 9a664a4a58..7fd99f0ac1 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -509,22 +509,28 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \tfor (i = 0; i < origins.nr; i++) {\n \t\tstruct object_id *oid = origins.items[i].util;\n \t\tenum object_type type;\n-\t\tunsigned long size, len;\n+\t\tunsigned long size;\n \t\tchar *buf = read_object_file(oid, &type, &size);\n+\t\tchar *origbuf = buf;\n+\t\tunsigned long len = size;\n \t\tstruct signature_check sigc = { NULL };\n-\t\tstruct strbuf sig = STRBUF_INIT;\n+\t\tstruct strbuf payload = STRBUF_INIT, sig = STRBUF_INIT;\n \n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n-\t\tlen = parse_signature(buf, size);\n \n-\t\tif (size == len)\n-\t\t\t; /* merely annotated */\n-\t\telse if (check_signature(buf, len, buf + len, size - len, &sigc) &&\n-\t\t\t!sigc.gpg_output)\n-\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n-\t\telse\n-\t\t\tstrbuf_addstr(&sig, sigc.gpg_output);\n+\t\tif (!parse_signature(buf, size, &payload, &sig))\n+\t\t\t;/* merely annotated */\n+\t\telse {\n+\t\t\tbuf = payload.buf;\n+\t\t\tlen = payload.len;\n+\t\t\tif (check_signature(payload.buf, payload.len, sig.buf,\n+\t\t\t\t\t sig.len, &sigc) &&\n+\t\t\t\t!sigc.gpg_output)\n+\t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n+\t\t\telse\n+\t\t\t\tstrbuf_addstr(&sig, sigc.gpg_output);\n+\t\t}\n \t\tsignature_check_clear(&sigc);\n \n \t\tif (!tag_number++) {\n@@ -547,9 +553,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\t\t\tstrlen(origins.items[i].string));\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n+\t\tstrbuf_release(&payload);\n \t\tstrbuf_release(&sig);\n \tnext:\n-\t\tfree(buf);\n+\t\tfree(origbuf);\n \t}\n \tif (tagbuf.len) {\n \t\tstrbuf_addch(out, '\\n');\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex b499270836..c6274c14af 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -345,7 +345,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-size_t parse_signature(const char *buf, size_t size)\n+size_t parse_signed_buffer(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n \tsize_t match = size;\n@@ -361,6 +361,17 @@ size_t parse_signature(const char *buf, size_t size)\n \treturn match;\n }\n \n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature)\n+{\n+\tsize_t match = parse_signed_buffer(buf, size);\n+\tif (match != size) {\n+\t\tstrbuf_add(payload, buf, match);\n+\t\tstrbuf_add(signature, buf + match, size - match);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n void set_signing_key(const char *key)\n {\n \tfree(configured_signing_key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex f4e9b4f371..80567e4894 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -37,13 +37,20 @@ struct signature_check {\n \n void signature_check_clear(struct signature_check *sigc);\n \n+/*\n+ * Look at a GPG signed tag object. If such a signature exists, store it in\n+ * signature and the signed content in payload. Return 1 if a signature was\n+ * found, and 0 otherwise.\n+ */\n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature);\n+\n /*\n * Look at GPG signed content (e.g. a signed tag object), whose\n * payload is followed by a detached signature on it. Return the\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n */\n-size_t parse_signature(const char *buf, size_t size);\n+size_t parse_signed_buffer(const char *buf, size_t size);\n \n /*\n * Create a detached signature for the contents of \"buffer\" and append\ndiff --git a/log-tree.c b/log-tree.c\nindex 7e0335e548..b025c8da93 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -548,7 +548,8 @@ static int show_one_mergetag(struct commit *commit,\n \tstruct strbuf verify_message;\n \tstruct signature_check sigc = { 0 };\n \tint status, nth;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \thash_object_file(the_hash_algo, extra->value, extra->len,\n \t\t\t type_name(OBJ_TAG), &oid);\n@@ -571,13 +572,11 @@ static int show_one_mergetag(struct commit *commit,\n \t\tstrbuf_addf(&verify_message,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \n-\tpayload_size = parse_signature(extra->value, extra->len);\n \tstatus = -1;\n-\tif (extra->len > payload_size) {\n+\tif (parse_signature(extra->value, extra->len, &payload, &signature)) {\n \t\t/* could have a good signature */\n-\t\tstatus = check_signature(extra->value, payload_size,\n-\t\t\t\t\t extra->value + payload_size,\n-\t\t\t\t\t extra->len - payload_size, &sigc);\n+\t\tstatus = check_signature(payload.buf, payload.len,\n+\t\t\t\t\t signature.buf, signature.len, &sigc);\n \t\tif (sigc.gpg_output)\n \t\t\tstrbuf_addstr(&verify_message, sigc.gpg_output);\n \t\telse\n@@ -588,6 +587,8 @@ static int show_one_mergetag(struct commit *commit,\n \n \tshow_sig_lines(opt, status, verify_message.buf);\n \tstrbuf_release(&verify_message);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 606f638ab1..bba4877745 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1215,7 +1215,13 @@ static void find_subpos(const char *buf,\n \t\t\tsize_t *nonsiglen,\n \t\t\tconst char **sig, size_t *siglen)\n {\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tconst char *eol;\n+\tconst char *end = buf + strlen(buf);\n+\tconst char *sigstart;\n+\n+\n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n \t\teol = strchrnul(buf, '\\n');\n@@ -1228,14 +1234,15 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \n \t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n-\t*siglen = strlen(*sig);\n+\tparse_signature(buf, end - buf, &payload, &signature);\n+\t*sig = strbuf_detach(&signature, siglen);\n+\tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n \t/* subject is first non-empty line */\n \t*sub = buf;\n \t/* subject goes to first empty line before signature begins */\n \tif ((eol = strstr(*sub, \"\\n\\n\"))) {\n-\t\teol = eol < *sig ? eol : *sig;\n+\t\teol = eol < sigstart ? eol : sigstart;\n \t/* check if message uses CRLF */\n \t} else if (! (eol = strstr(*sub, \"\\r\\n\\r\\n\"))) {\n \t\t/* treat whole message as subject */\n@@ -1253,7 +1260,7 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \t*body = buf;\n \t*bodylen = strlen(buf);\n-\t*nonsiglen = *sig - buf;\n+\t*nonsiglen = sigstart - buf;\n }\n \n /*\n@@ -1291,6 +1298,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\tstruct used_atom *atom = &used_atom[i];\n \t\tconst char *name = atom->name;\n \t\tstruct atom_value *v = &val[i];\n+\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -1336,6 +1344,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n+\n+\t\tfree((void *)sigpos);\n \t}\n }\n \ndiff --git a/tag.c b/tag.c\nindex 1ed2684e45..3e18a41841 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -13,26 +13,27 @@ const char *tag_type = \"tag\";\n static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n {\n \tstruct signature_check sigc;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tint ret;\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n-\tpayload_size = parse_signature(buf, size);\n-\n-\tif (size == payload_size) {\n+\tif (!parse_signature(buf, size, &payload, &signature)) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n-\t\t\twrite_in_full(1, buf, payload_size);\n+\t\t\twrite_in_full(1, buf, size);\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(buf, payload_size, buf + payload_size,\n-\t\t\t\tsize - payload_size, &sigc);\n+\tret = check_signature(payload.buf, payload.len, signature.buf,\n+\t\t\t\tsignature.len, &sigc);\n \n \tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n \t\tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn ret;\n }\n \n"},{"id":"414643","messageId":"20210118234915.2036197-7-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210118234915.2036197-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 6/6] gpg-interface: remove other signature headers before verifying","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-18T23:49:15Z","receivedAt":"2021-01-18T23:52:25Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we have a multiply signed commit, we need to remove the signature\nin the header before verifying the object, since the trailing signature\nwill not be over both pieces of data. Do so, and verify that we\nvalidate the signature appropriately.\n\nSigned-off-by: brian m. carlson \n---\n gpg-interface.c | 2 ++\n t/t7004-tag.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c6274c14af..127aecfc2b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"commit.h\"\n #include \"config.h\"\n #include \"run-command.h\"\n #include \"strbuf.h\"\n@@ -366,6 +367,7 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct\n \tsize_t match = parse_signed_buffer(buf, size);\n \tif (match != size) {\n \t\tstrbuf_add(payload, buf, match);\n+\t\tremove_signature(payload);\n \t\tstrbuf_add(signature, buf + match, size - match);\n \t\treturn 1;\n \t}\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 05f411c821..6fb4e3cf11 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -17,6 +17,13 @@ tag_exists () {\n \tgit show-ref --quiet --verify refs/tags/\"$1\"\n }\n \n+test_expect_success 'setup' '\n+\ttest_oid_cache <<-EOM\n+\tothersigheader sha1:gpgsig-sha256\n+\tothersigheader sha256:gpgsig\n+\tEOM\n+'\n+\n test_expect_success 'listing all tags in an empty tree should succeed' '\n \tgit tag -l &&\n \tgit tag\n@@ -1371,6 +1378,24 @@ test_expect_success GPG \\\n \t'test_config gpg.program echo &&\n \t test_must_fail git tag -s -m tail tag-gpg-failure'\n \n+# try to produce invalid signature\n+test_expect_success GPG 'git verifies tag is valid with double signature' '\n+\tgit tag -s -m tail tag-gpg-double-sig &&\n+\tgit cat-file tag tag-gpg-double-sig >tag &&\n+\tothersigheader=$(test_oid othersigheader) &&\n+\tsed -ne \"/^\\$/q;p\" tag >new-tag &&\n+\tcat <<-EOM >>new-tag &&\n+\t$othersigheader -----BEGIN PGP SIGNATURE-----\n+\t someinvaliddata\n+\t -----END PGP SIGNATURE-----\n+\tEOM\n+\tsed -e \"1,/^tagger/d\" tag >>new-tag &&\n+\tnew_tag=$(git hash-object -t tag -w new-tag) &&\n+\tgit update-ref refs/tags/tag-gpg-double-sig $new_tag &&\n+\tgit verify-tag tag-gpg-double-sig &&\n+\tgit fsck\n+'\n+\n # try to sign with bad user.signingkey\n test_expect_success GPGSM \\\n \t'git tag -s fails if gpgsm is misconfigured (bad key)' \\\n"},{"id":"414644","messageId":"20210118234915.2036197-6-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210118234915.2036197-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 5/6] ref-filter: hoist signature parsing","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-18T23:49:14Z","receivedAt":"2021-01-18T23:52:41Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we parse a signature in the ref-filter code, we continually\nincrement the buffer pointer. Hoist the signature parsing above the\nblank line delimiting headers and body so we can find the signature when\nusing a header to sign the buffer.\n\nSigned-off-by: brian m. carlson \n---\n ref-filter.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bba4877745..e6c8106377 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1221,6 +1221,8 @@ static void find_subpos(const char *buf,\n \tconst char *end = buf + strlen(buf);\n \tconst char *sigstart;\n \n+\t/* parse signature first; we might not even have a subject line */\n+\tparse_signature(buf, end - buf, &payload, &signature);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n@@ -1232,9 +1234,6 @@ static void find_subpos(const char *buf,\n \t/* skip any empty lines */\n \twhile (*buf == '\\n')\n \t\tbuf++;\n-\n-\t/* parse signature first; we might not even have a subject line */\n-\tparse_signature(buf, end - buf, &payload, &signature);\n \t*sig = strbuf_detach(&signature, siglen);\n \tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n@@ -1330,7 +1329,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = xmemdupz(sigpos, siglen);\n \t\telse if (atom->u.contents.option == C_LINES) {\n \t\t\tstruct strbuf s = STRBUF_INIT;\n-\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n+\t\t\tconst char *contents_end = bodypos + nonsiglen;\n \n \t\t\t/* Size is the length of the message after removing the signature */\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n"},{"id":"416694","messageId":"20210211020806.288523-1-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210118234915.2036197-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 0/6] Support for commits signed by multiple algorithms","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-02-11T02:08:00Z","receivedAt":"2021-02-11T02:09:27Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces support for verifying commits and tags signed by\nmultiple algorithms.\n\nThese commits allow for signing commits and tags that can be\nround-tripped through both SHA-1 and SHA-256. We verify only the\nsignature using the current hash algorithm, since we currently don't\nrewrite objects.\n\nChanges from v3:\n* Fix a double free.\n\nChanges from v2:\n* Fix problems with size_t and unsigned long on 32-bit systems by\n creating a cleanup patch at the beginning of the series.\n* Fix a clang warning with initialization by using memset.\n\nChanges from v1:\n* Fix brown paper bag bug where some tests failed due to a bad rebase.\n\nbrian m. carlson (6):\n ref-filter: switch some uses of unsigned long to size_t\n commit: ignore additional signatures when parsing signed commits\n gpg-interface: improve interface for parsing tags\n commit: allow parsing arbitrary buffers with headers\n ref-filter: hoist signature parsing\n gpg-interface: remove other signature headers before verifying\n\n builtin/receive-pack.c | 4 +-\n builtin/tag.c | 16 ++++++--\n commit.c | 85 +++++++++++++++++++++++++++-------------\n commit.h | 12 +++++-\n fmt-merge-msg.c | 29 ++++++++------\n gpg-interface.c | 15 ++++++-\n gpg-interface.h | 9 ++++-\n log-tree.c | 15 +++----\n ref-filter.c | 33 ++++++++++------\n t/t7004-tag.sh | 25 ++++++++++++\n t/t7510-signed-commit.sh | 43 +++++++++++++++++++-\n tag.c | 15 +++----\n 12 files changed, 226 insertions(+), 75 deletions(-)\n\n"},{"id":"416695","messageId":"20210211020806.288523-5-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210211020806.288523-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 4/6] commit: allow parsing arbitrary buffers with headers","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-02-11T02:08:04Z","receivedAt":"2021-02-11T02:09:27Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently only commits are signed with headers. However, in the future,\nwe'll also sign tags with headers as well. Let's refactor out a\nfunction called parse_buffer_signed_by_header which does exactly that.\nIn addition, since we'll want to sign things other than commits this\nway, let's call the function sign_with_header instead of do_sign_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 20 ++++++++++++++++----\n commit.h | 9 +++++++++\n 2 files changed, 25 insertions(+), 4 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex ccb912b9b5..2c57b6ded9 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -995,7 +995,7 @@ static const char *gpg_sig_headers[] = {\n \t\"gpgsig-sha256\",\n };\n \n-static int do_sign_commit(struct strbuf *buf, const char *keyid)\n+int sign_with_header(struct strbuf *buf, const char *keyid)\n {\n \tstruct strbuf sig = STRBUF_INIT;\n \tint inspos, copypos;\n@@ -1035,13 +1035,26 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n \treturn 0;\n }\n \n+\n+\n int parse_signed_commit(const struct commit *commit,\n \t\t\tstruct strbuf *payload, struct strbuf *signature,\n \t\t\tconst struct git_hash_algo *algop)\n {\n-\n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n+\tint ret = parse_buffer_signed_by_header(buffer, size, payload, signature, algop);\n+\n+\tunuse_commit_buffer(commit, buffer);\n+\treturn ret;\n+}\n+\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop)\n+{\n \tint in_signature = 0, saw_signature = 0, other_signature = 0;\n \tconst char *line, *tail, *p;\n \tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n@@ -1078,7 +1091,6 @@ int parse_signed_commit(const struct commit *commit,\n \t\t}\n \t\tline = next;\n \t}\n-\tunuse_commit_buffer(commit, buffer);\n \treturn saw_signature;\n }\n \n@@ -1532,7 +1544,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,\n \tif (encoding_is_utf8 && !verify_utf8(&buffer))\n \t\tfprintf(stderr, _(commit_utf8_warn));\n \n-\tif (sign_commit && do_sign_commit(&buffer, sign_commit)) {\n+\tif (sign_commit && sign_with_header(&buffer, sign_commit)) {\n \t\tresult = -1;\n \t\tgoto out;\n \t}\ndiff --git a/commit.h b/commit.h\nindex 030aa65ab8..e2856ce8ef 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -360,4 +360,13 @@ int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void\n LAST_ARG_MUST_BE_NULL\n int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);\n \n+/* Sign a commit or tag buffer, storing the result in a header. */\n+int sign_with_header(struct strbuf *buf, const char *keyid);\n+/* Parse the signature out of a header. */\n+int parse_buffer_signed_by_header(const char *buffer,\n+\t\t\t\t unsigned long size,\n+\t\t\t\t struct strbuf *payload,\n+\t\t\t\t struct strbuf *signature,\n+\t\t\t\t const struct git_hash_algo *algop);\n+\n #endif /* COMMIT_H */\n"},{"id":"416696","messageId":"20210211020806.288523-2-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210211020806.288523-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 1/6] ref-filter: switch some uses of unsigned long to size_t","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-02-11T02:08:01Z","receivedAt":"2021-02-11T02:09:38Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In the future, we'll want to pass some of the arguments of find_subpos\nto strbuf_detach, which takes a size_t. This is fine on systems where\nthat's the same size as unsigned long, but that isn't the case on all\nsystems. Moreover, size_t makes sense since it's not possible to use a\nbuffer here that's larger than memory anyway.\n\nLet's switch each use to size_t for these lengths in\ngrab_sub_body_contents and find_subpos.\n\nSigned-off-by: brian m. carlson \n---\n ref-filter.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex fd994e1874..cf617a62e1 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1210,10 +1210,10 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n }\n \n static void find_subpos(const char *buf,\n-\t\t\tconst char **sub, unsigned long *sublen,\n-\t\t\tconst char **body, unsigned long *bodylen,\n-\t\t\tunsigned long *nonsiglen,\n-\t\t\tconst char **sig, unsigned long *siglen)\n+\t\t\tconst char **sub, size_t *sublen,\n+\t\t\tconst char **body, size_t *bodylen,\n+\t\t\tsize_t *nonsiglen,\n+\t\t\tconst char **sig, size_t *siglen)\n {\n \tconst char *eol;\n \t/* skip past header until we hit empty line */\n@@ -1285,7 +1285,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n-\tunsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n+\tsize_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct used_atom *atom = &used_atom[i];\n"},{"id":"416697","messageId":"20210211020806.288523-3-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210211020806.288523-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 2/6] commit: ignore additional signatures when parsing signed commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-02-11T02:08:02Z","receivedAt":"2021-02-11T02:09:42Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we create a commit with multiple signatures, neither of these\nsignatures includes the other. Consequently, when we produce the\npayload which has been signed so we can verify the commit, we must strip\noff any other signatures, or the payload will differ from what was\nsigned. Do so, and in preparation for verifying with multiple\nalgorithms, pass the algorithm we want to verify into\nparse_signed_commit.\n\nSigned-off-by: brian m. carlson \n---\n commit.c | 54 +++++++++++++++++++++++++---------------\n commit.h | 3 ++-\n log-tree.c | 2 +-\n t/t7510-signed-commit.sh | 43 +++++++++++++++++++++++++++++++-\n 4 files changed, 79 insertions(+), 23 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex bab8d5ab07..1006c85ca8 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1036,20 +1036,18 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)\n }\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *payload, struct strbuf *signature)\n+\t\t\tstruct strbuf *payload, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop)\n {\n \n \tunsigned long size;\n \tconst char *buffer = get_commit_buffer(commit, &size);\n-\tint in_signature, saw_signature = -1;\n-\tconst char *line, *tail;\n-\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];\n-\tint gpg_sig_header_len = strlen(gpg_sig_header);\n+\tint in_signature = 0, saw_signature = 0, other_signature = 0;\n+\tconst char *line, *tail, *p;\n+\tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];\n \n \tline = buffer;\n \ttail = buffer + size;\n-\tin_signature = 0;\n-\tsaw_signature = 0;\n \twhile (line < tail) {\n \t\tconst char *sig = NULL;\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n@@ -1057,9 +1055,15 @@ int parse_signed_commit(const struct commit *commit,\n \t\tnext = next ? next + 1 : tail;\n \t\tif (in_signature && line[0] == ' ')\n \t\t\tsig = line + 1;\n-\t\telse if (starts_with(line, gpg_sig_header) &&\n-\t\t\t line[gpg_sig_header_len] == ' ')\n-\t\t\tsig = line + gpg_sig_header_len + 1;\n+\t\telse if (skip_prefix(line, gpg_sig_header, &p) &&\n+\t\t\t *p == ' ') {\n+\t\t\tsig = line + strlen(gpg_sig_header) + 1;\n+\t\t\tother_signature = 0;\n+\t\t}\n+\t\telse if (starts_with(line, \"gpgsig\"))\n+\t\t\tother_signature = 1;\n+\t\telse if (other_signature && line[0] != ' ')\n+\t\t\tother_signature = 0;\n \t\tif (sig) {\n \t\t\tstrbuf_add(signature, sig, next - sig);\n \t\t\tsaw_signature = 1;\n@@ -1068,7 +1072,8 @@ int parse_signed_commit(const struct commit *commit,\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n-\t\t\tstrbuf_add(payload, line, next - line);\n+\t\t\tif (!other_signature)\n+\t\t\t\tstrbuf_add(payload, line, next - line);\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n@@ -1082,23 +1087,29 @@ int remove_signature(struct strbuf *buf)\n \tconst char *line = buf->buf;\n \tconst char *tail = buf->buf + buf->len;\n \tint in_signature = 0;\n-\tconst char *sig_start = NULL;\n-\tconst char *sig_end = NULL;\n+\tstruct sigbuf {\n+\t\tconst char *start;\n+\t\tconst char *end;\n+\t} sigs[2], *sigp = &sigs[0];\n+\tint i;\n+\tconst char *orig_buf = buf->buf;\n+\n+\tmemset(sigs, 0, sizeof(sigs));\n \n \twhile (line < tail) {\n \t\tconst char *next = memchr(line, '\\n', tail - line);\n \t\tnext = next ? next + 1 : tail;\n \n \t\tif (in_signature && line[0] == ' ')\n-\t\t\tsig_end = next;\n+\t\t\tsigp->end = next;\n \t\telse if (starts_with(line, \"gpgsig\")) {\n \t\t\tint i;\n \t\t\tfor (i = 1; i < GIT_HASH_NALGOS; i++) {\n \t\t\t\tconst char *p;\n \t\t\t\tif (skip_prefix(line, gpg_sig_headers[i], &p) &&\n \t\t\t\t *p == ' ') {\n-\t\t\t\t\tsig_start = line;\n-\t\t\t\t\tsig_end = next;\n+\t\t\t\t\tsigp->start = line;\n+\t\t\t\t\tsigp->end = next;\n \t\t\t\t\tin_signature = 1;\n \t\t\t\t}\n \t\t\t}\n@@ -1106,15 +1117,18 @@ int remove_signature(struct strbuf *buf)\n \t\t\tif (*line == '\\n')\n \t\t\t\t/* dump the whole remainder of the buffer */\n \t\t\t\tnext = tail;\n+\t\t\tif (in_signature && sigp - sigs != ARRAY_SIZE(sigs))\n+\t\t\t\tsigp++;\n \t\t\tin_signature = 0;\n \t\t}\n \t\tline = next;\n \t}\n \n-\tif (sig_start)\n-\t\tstrbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start);\n+\tfor (i = ARRAY_SIZE(sigs) - 1; i >= 0; i--)\n+\t\tif (sigs[i].start)\n+\t\t\tstrbuf_remove(buf, sigs[i].start - orig_buf, sigs[i].end - sigs[i].start);\n \n-\treturn sig_start != NULL;\n+\treturn sigs[0].start != NULL;\n }\n \n static void handle_signed_tag(struct commit *parent, struct commit_extra_header ***tail)\n@@ -1165,7 +1179,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \n \tsigc->result = 'N';\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \tret = check_signature(payload.buf, payload.len, signature.buf,\n \t\tsignature.len, sigc);\ndiff --git a/commit.h b/commit.h\nindex f4e7b0158e..030aa65ab8 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -317,7 +317,8 @@ void set_merge_remote_desc(struct commit *commit,\n struct commit *get_merge_parent(const char *name);\n \n int parse_signed_commit(const struct commit *commit,\n-\t\t\tstruct strbuf *message, struct strbuf *signature);\n+\t\t\tstruct strbuf *message, struct strbuf *signature,\n+\t\t\tconst struct git_hash_algo *algop);\n int remove_signature(struct strbuf *buf);\n \n /*\ndiff --git a/log-tree.c b/log-tree.c\nindex e048467650..0f9ef836d4 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -502,7 +502,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \tstruct signature_check sigc = { 0 };\n \tint status;\n \n-\tif (parse_signed_commit(commit, &payload, &signature) <= 0)\n+\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n \tstatus = check_signature(payload.buf, payload.len, signature.buf,\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex f4bf925bdd..8df5a74f1d 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -175,7 +175,7 @@ test_expect_success GPG 'show signed commit with signature' '\n \tgit cat-file commit initial >cat &&\n \tgrep -v -e \"gpg: \" -e \"Warning: \" show >show.commit &&\n \tgrep -e \"gpg: \" -e \"Warning: \" show >show.gpg &&\n-\tgrep -v \"^ \" cat | grep -v \"^$(test_oid header) \" >cat.commit &&\n+\tgrep -v \"^ \" cat | grep -v \"^gpgsig.* \" >cat.commit &&\n \ttest_cmp show.commit commit &&\n \ttest_cmp show.gpg verify.2 &&\n \ttest_cmp cat.commit verify.1\n@@ -337,4 +337,45 @@ test_expect_success GPG 'show double signature with custom format' '\n \ttest_cmp expect actual\n '\n \n+\n+test_expect_success GPG 'verify-commit verifies multiply signed commits' '\n+\tgit init multiply-signed &&\n+\tcd multiply-signed &&\n+\ttest_commit first &&\n+\techo 1 >second &&\n+\tgit add second &&\n+\ttree=$(git write-tree) &&\n+\tparent=$(git rev-parse HEAD^{commit}) &&\n+\tgit commit --gpg-sign -m second &&\n+\tgit cat-file commit HEAD &&\n+\t# Avoid trailing whitespace.\n+\tsed -e \"s/^Q//\" -e \"s/^Z/ /\" >commit <<-EOF &&\n+\tQtree $tree\n+\tQparent $parent\n+\tQauthor A U Thor 1112912653 -0700\n+\tQcommitter C O Mitter 1112912653 -0700\n+\tQgpgsig -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBDRYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMNd+8AoK1I8mhLHviPH+q2I5fIVgPsEtYC\n+\tQ AKCTqBh+VabJceXcGIZuF0Ry+udbBQ==\n+\tQ =tQ0N\n+\tQ -----END PGP SIGNATURE-----\n+\tQgpgsig-sha256 -----BEGIN PGP SIGNATURE-----\n+\tQZ\n+\tQ iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBIBYcY29tbWl0dGVy\n+\tQ QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN/NEAn0XO9RYSBj2dFyozi0JKSbssYMtO\n+\tQ AJwKCQ1BQOtuwz//IjU8TiS+6S4iUw==\n+\tQ =pIwP\n+\tQ -----END PGP SIGNATURE-----\n+\tQ\n+\tQsecond\n+\tEOF\n+\thead=$(git hash-object -t commit -w commit) &&\n+\tgit reset --hard $head &&\n+\tgit verify-commit $head 2>actual &&\n+\tgrep \"Good signature from\" actual &&\n+\t! grep \"BAD signature from\" actual\n+'\n+\n test_done\n"},{"id":"416698","messageId":"20210211020806.288523-4-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210211020806.288523-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 3/6] gpg-interface: improve interface for parsing tags","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-02-11T02:08:03Z","receivedAt":"2021-02-11T02:09:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have a function which parses a buffer with a signature at the end,\nparse_signature, and this function is used for signed tags. However,\nwe'll need to store values for multiple algorithms, and we'll do this by\nusing a header for the non-default algorithm.\n\nAdjust the parse_signature interface to store the parsed data in two\nstrbufs and turn the existing function into parse_signed_buffer. The\nlatter is still used in places where we know we always have a signed\nbuffer, such as push certs.\n\nAdjust all the callers to deal with this new interface.\n\nSigned-off-by: brian m. carlson \n---\n builtin/receive-pack.c | 4 ++--\n builtin/tag.c | 16 ++++++++++++----\n commit.c | 9 ++++++---\n fmt-merge-msg.c | 29 ++++++++++++++++++-----------\n gpg-interface.c | 13 ++++++++++++-\n gpg-interface.h | 9 ++++++++-\n log-tree.c | 13 +++++++------\n ref-filter.c | 18 ++++++++++++++----\n tag.c | 15 ++++++++-------\n 9 files changed, 87 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d49d050e6e..b89ce31bf2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -764,7 +764,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \n-\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tbogs = parse_signed_buffer(push_cert.buf, push_cert.len);\n \t\tcheck_signature(push_cert.buf, bogs, push_cert.buf + bogs,\n \t\t\t\tpush_cert.len - bogs, &sigcheck);\n \n@@ -2050,7 +2050,7 @@ static void queue_commands_from_cert(struct command **tail,\n \t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n \telse\n \t\tboc += 2;\n-\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\teoc = push_cert->buf + parse_signed_buffer(push_cert->buf, push_cert->len);\n \n \twhile (boc < eoc) {\n \t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e8b85eefd8..4237dc724c 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -198,11 +198,17 @@ static void write_tag_body(int fd, const struct object_id *oid)\n {\n \tunsigned long size;\n \tenum object_type type;\n-\tchar *buf, *sp;\n+\tchar *buf, *sp, *orig;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n-\tbuf = read_object_file(oid, &type, &size);\n+\torig = buf = read_object_file(oid, &type, &size);\n \tif (!buf)\n \t\treturn;\n+\tif (parse_signature(buf, size, &payload, &signature)) {\n+\t\tbuf = payload.buf;\n+\t\tsize = payload.len;\n+\t}\n \t/* skip header */\n \tsp = strstr(buf, \"\\n\\n\");\n \n@@ -211,9 +217,11 @@ static void write_tag_body(int fd, const struct object_id *oid)\n \t\treturn;\n \t}\n \tsp += 2; /* skip the 2 LFs */\n-\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp));\n+\twrite_or_die(fd, sp, buf + size - sp);\n \n-\tfree(buf);\n+\tfree(orig);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n }\n \n static int build_tag_object(struct strbuf *buf, int sign, struct object_id *result)\ndiff --git a/commit.c b/commit.c\nindex 1006c85ca8..ccb912b9b5 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1136,8 +1136,10 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tstruct merge_remote_desc *desc;\n \tstruct commit_extra_header *mergetag;\n \tchar *buf;\n-\tunsigned long size, len;\n+\tunsigned long size;\n \tenum object_type type;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \tdesc = merge_remote_util(parent);\n \tif (!desc || !desc->obj)\n@@ -1145,8 +1147,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tbuf = read_object_file(&desc->obj->oid, &type, &size);\n \tif (!buf || type != OBJ_TAG)\n \t\tgoto free_return;\n-\tlen = parse_signature(buf, size);\n-\tif (size == len)\n+\tif (!parse_signature(buf, size, &payload, &signature))\n \t\tgoto free_return;\n \t/*\n \t * We could verify this signature and either omit the tag when\n@@ -1165,6 +1166,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \n \t**tail = mergetag;\n \t*tail = &mergetag->next;\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn;\n \n free_return:\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 46f6015c44..1e51492a05 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -510,22 +510,28 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \tfor (i = 0; i < origins.nr; i++) {\n \t\tstruct object_id *oid = origins.items[i].util;\n \t\tenum object_type type;\n-\t\tunsigned long size, len;\n+\t\tunsigned long size;\n \t\tchar *buf = read_object_file(oid, &type, &size);\n+\t\tchar *origbuf = buf;\n+\t\tunsigned long len = size;\n \t\tstruct signature_check sigc = { NULL };\n-\t\tstruct strbuf sig = STRBUF_INIT;\n+\t\tstruct strbuf payload = STRBUF_INIT, sig = STRBUF_INIT;\n \n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n-\t\tlen = parse_signature(buf, size);\n \n-\t\tif (size == len)\n-\t\t\t; /* merely annotated */\n-\t\telse if (check_signature(buf, len, buf + len, size - len, &sigc) &&\n-\t\t\t!sigc.gpg_output)\n-\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n-\t\telse\n-\t\t\tstrbuf_addstr(&sig, sigc.gpg_output);\n+\t\tif (!parse_signature(buf, size, &payload, &sig))\n+\t\t\t;/* merely annotated */\n+\t\telse {\n+\t\t\tbuf = payload.buf;\n+\t\t\tlen = payload.len;\n+\t\t\tif (check_signature(payload.buf, payload.len, sig.buf,\n+\t\t\t\t\t sig.len, &sigc) &&\n+\t\t\t\t!sigc.gpg_output)\n+\t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n+\t\t\telse\n+\t\t\t\tstrbuf_addstr(&sig, sigc.gpg_output);\n+\t\t}\n \t\tsignature_check_clear(&sigc);\n \n \t\tif (!tag_number++) {\n@@ -548,9 +554,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t\t\t\tstrlen(origins.items[i].string));\n \t\t\tfmt_tag_signature(&tagbuf, &sig, buf, len);\n \t\t}\n+\t\tstrbuf_release(&payload);\n \t\tstrbuf_release(&sig);\n \tnext:\n-\t\tfree(buf);\n+\t\tfree(origbuf);\n \t}\n \tif (tagbuf.len) {\n \t\tstrbuf_addch(out, '\\n');\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex b499270836..c6274c14af 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -345,7 +345,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-size_t parse_signature(const char *buf, size_t size)\n+size_t parse_signed_buffer(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n \tsize_t match = size;\n@@ -361,6 +361,17 @@ size_t parse_signature(const char *buf, size_t size)\n \treturn match;\n }\n \n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature)\n+{\n+\tsize_t match = parse_signed_buffer(buf, size);\n+\tif (match != size) {\n+\t\tstrbuf_add(payload, buf, match);\n+\t\tstrbuf_add(signature, buf + match, size - match);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n void set_signing_key(const char *key)\n {\n \tfree(configured_signing_key);\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex f4e9b4f371..80567e4894 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -37,13 +37,20 @@ struct signature_check {\n \n void signature_check_clear(struct signature_check *sigc);\n \n+/*\n+ * Look at a GPG signed tag object. If such a signature exists, store it in\n+ * signature and the signed content in payload. Return 1 if a signature was\n+ * found, and 0 otherwise.\n+ */\n+int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature);\n+\n /*\n * Look at GPG signed content (e.g. a signed tag object), whose\n * payload is followed by a detached signature on it. Return the\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n */\n-size_t parse_signature(const char *buf, size_t size);\n+size_t parse_signed_buffer(const char *buf, size_t size);\n \n /*\n * Create a detached signature for the contents of \"buffer\" and append\ndiff --git a/log-tree.c b/log-tree.c\nindex 0f9ef836d4..60bbe1d5df 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -548,7 +548,8 @@ static int show_one_mergetag(struct commit *commit,\n \tstruct strbuf verify_message;\n \tstruct signature_check sigc = { 0 };\n \tint status, nth;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \n \thash_object_file(the_hash_algo, extra->value, extra->len,\n \t\t\t type_name(OBJ_TAG), &oid);\n@@ -571,13 +572,11 @@ static int show_one_mergetag(struct commit *commit,\n \t\tstrbuf_addf(&verify_message,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \n-\tpayload_size = parse_signature(extra->value, extra->len);\n \tstatus = -1;\n-\tif (extra->len > payload_size) {\n+\tif (parse_signature(extra->value, extra->len, &payload, &signature)) {\n \t\t/* could have a good signature */\n-\t\tstatus = check_signature(extra->value, payload_size,\n-\t\t\t\t\t extra->value + payload_size,\n-\t\t\t\t\t extra->len - payload_size, &sigc);\n+\t\tstatus = check_signature(payload.buf, payload.len,\n+\t\t\t\t\t signature.buf, signature.len, &sigc);\n \t\tif (sigc.gpg_output)\n \t\t\tstrbuf_addstr(&verify_message, sigc.gpg_output);\n \t\telse\n@@ -588,6 +587,8 @@ static int show_one_mergetag(struct commit *commit,\n \n \tshow_sig_lines(opt, status, verify_message.buf);\n \tstrbuf_release(&verify_message);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex cf617a62e1..040a24402e 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1215,7 +1215,13 @@ static void find_subpos(const char *buf,\n \t\t\tsize_t *nonsiglen,\n \t\t\tconst char **sig, size_t *siglen)\n {\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tconst char *eol;\n+\tconst char *end = buf + strlen(buf);\n+\tconst char *sigstart;\n+\n+\n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n \t\teol = strchrnul(buf, '\\n');\n@@ -1228,14 +1234,15 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \n \t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n-\t*siglen = strlen(*sig);\n+\tparse_signature(buf, end - buf, &payload, &signature);\n+\t*sig = strbuf_detach(&signature, siglen);\n+\tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n \t/* subject is first non-empty line */\n \t*sub = buf;\n \t/* subject goes to first empty line before signature begins */\n \tif ((eol = strstr(*sub, \"\\n\\n\"))) {\n-\t\teol = eol < *sig ? eol : *sig;\n+\t\teol = eol < sigstart ? eol : sigstart;\n \t/* check if message uses CRLF */\n \t} else if (! (eol = strstr(*sub, \"\\r\\n\\r\\n\"))) {\n \t\t/* treat whole message as subject */\n@@ -1253,7 +1260,7 @@ static void find_subpos(const char *buf,\n \t\tbuf++;\n \t*body = buf;\n \t*bodylen = strlen(buf);\n-\t*nonsiglen = *sig - buf;\n+\t*nonsiglen = sigstart - buf;\n }\n \n /*\n@@ -1291,6 +1298,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\tstruct used_atom *atom = &used_atom[i];\n \t\tconst char *name = atom->name;\n \t\tstruct atom_value *v = &val[i];\n+\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n@@ -1336,7 +1344,9 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\tv->s = xstrdup(subpos);\n+\n \t}\n+\tfree((void *)sigpos);\n }\n \n /*\ndiff --git a/tag.c b/tag.c\nindex 1ed2684e45..3e18a41841 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -13,26 +13,27 @@ const char *tag_type = \"tag\";\n static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n {\n \tstruct signature_check sigc;\n-\tsize_t payload_size;\n+\tstruct strbuf payload = STRBUF_INIT;\n+\tstruct strbuf signature = STRBUF_INIT;\n \tint ret;\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n-\tpayload_size = parse_signature(buf, size);\n-\n-\tif (size == payload_size) {\n+\tif (!parse_signature(buf, size, &payload, &signature)) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n-\t\t\twrite_in_full(1, buf, payload_size);\n+\t\t\twrite_in_full(1, buf, size);\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(buf, payload_size, buf + payload_size,\n-\t\t\t\tsize - payload_size, &sigc);\n+\tret = check_signature(payload.buf, payload.len, signature.buf,\n+\t\t\t\tsignature.len, &sigc);\n \n \tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n \t\tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n+\tstrbuf_release(&payload);\n+\tstrbuf_release(&signature);\n \treturn ret;\n }\n \n"},{"id":"416699","messageId":"20210211020806.288523-6-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210211020806.288523-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 5/6] ref-filter: hoist signature parsing","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-02-11T02:08:05Z","receivedAt":"2021-02-11T02:09:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we parse a signature in the ref-filter code, we continually\nincrement the buffer pointer. Hoist the signature parsing above the\nblank line delimiting headers and body so we can find the signature when\nusing a header to sign the buffer.\n\nSigned-off-by: brian m. carlson \n---\n ref-filter.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 040a24402e..bade6528ee 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1221,6 +1221,8 @@ static void find_subpos(const char *buf,\n \tconst char *end = buf + strlen(buf);\n \tconst char *sigstart;\n \n+\t/* parse signature first; we might not even have a subject line */\n+\tparse_signature(buf, end - buf, &payload, &signature);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n@@ -1232,9 +1234,6 @@ static void find_subpos(const char *buf,\n \t/* skip any empty lines */\n \twhile (*buf == '\\n')\n \t\tbuf++;\n-\n-\t/* parse signature first; we might not even have a subject line */\n-\tparse_signature(buf, end - buf, &payload, &signature);\n \t*sig = strbuf_detach(&signature, siglen);\n \tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n \n@@ -1330,7 +1329,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tv->s = xmemdupz(sigpos, siglen);\n \t\telse if (atom->u.contents.option == C_LINES) {\n \t\t\tstruct strbuf s = STRBUF_INIT;\n-\t\t\tconst char *contents_end = bodylen + bodypos - siglen;\n+\t\t\tconst char *contents_end = bodypos + nonsiglen;\n \n \t\t\t/* Size is the length of the message after removing the signature */\n \t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n"},{"id":"416700","messageId":"20210211020806.288523-7-sandals@crustytoothpaste.net","threadId":"54974","inReplyTo":"20210211020806.288523-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 6/6] gpg-interface: remove other signature headers before verifying","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-02-11T02:08:06Z","receivedAt":"2021-02-11T02:10:00Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we have a multiply signed commit, we need to remove the signature\nin the header before verifying the object, since the trailing signature\nwill not be over both pieces of data. Do so, and verify that we\nvalidate the signature appropriately.\n\nSigned-off-by: brian m. carlson \n---\n gpg-interface.c | 2 ++\n t/t7004-tag.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c6274c14af..127aecfc2b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"commit.h\"\n #include \"config.h\"\n #include \"run-command.h\"\n #include \"strbuf.h\"\n@@ -366,6 +367,7 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct\n \tsize_t match = parse_signed_buffer(buf, size);\n \tif (match != size) {\n \t\tstrbuf_add(payload, buf, match);\n+\t\tremove_signature(payload);\n \t\tstrbuf_add(signature, buf + match, size - match);\n \t\treturn 1;\n \t}\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 943a7d5c1d..400b83a49e 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -20,6 +20,13 @@ tag_exists () {\n \tgit show-ref --quiet --verify refs/tags/\"$1\"\n }\n \n+test_expect_success 'setup' '\n+\ttest_oid_cache <<-EOM\n+\tothersigheader sha1:gpgsig-sha256\n+\tothersigheader sha256:gpgsig\n+\tEOM\n+'\n+\n test_expect_success 'listing all tags in an empty tree should succeed' '\n \tgit tag -l &&\n \tgit tag\n@@ -1374,6 +1381,24 @@ test_expect_success GPG \\\n \t'test_config gpg.program echo &&\n \t test_must_fail git tag -s -m tail tag-gpg-failure'\n \n+# try to produce invalid signature\n+test_expect_success GPG 'git verifies tag is valid with double signature' '\n+\tgit tag -s -m tail tag-gpg-double-sig &&\n+\tgit cat-file tag tag-gpg-double-sig >tag &&\n+\tothersigheader=$(test_oid othersigheader) &&\n+\tsed -ne \"/^\\$/q;p\" tag >new-tag &&\n+\tcat <<-EOM >>new-tag &&\n+\t$othersigheader -----BEGIN PGP SIGNATURE-----\n+\t someinvaliddata\n+\t -----END PGP SIGNATURE-----\n+\tEOM\n+\tsed -e \"1,/^tagger/d\" tag >>new-tag &&\n+\tnew_tag=$(git hash-object -t tag -w new-tag) &&\n+\tgit update-ref refs/tags/tag-gpg-double-sig $new_tag &&\n+\tgit verify-tag tag-gpg-double-sig &&\n+\tgit fsck\n+'\n+\n # try to sign with bad user.signingkey\n test_expect_success GPGSM \\\n \t'git tag -s fails if gpgsm is misconfigured (bad key)' \\\n"},{"id":"416725","messageId":"xmqqtuqjxd1o.fsf@gitster.c.googlers.com","threadId":"54974","inReplyTo":"20210211020806.288523-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v4 0/6] Support for commits signed by multiple algorithms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-11T07:45:55Z","receivedAt":"2021-02-11T08:00:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> This series introduces support for verifying commits and tags signed by\n> multiple algorithms.\n\nThanks.\n"}]}