diff options
| -rw-r--r-- | builtin/receive-pack.c | 4 | ||||
| -rw-r--r-- | builtin/tag.c | 16 | ||||
| -rw-r--r-- | commit.c | 83 | ||||
| -rw-r--r-- | commit.h | 12 | ||||
| -rw-r--r-- | fmt-merge-msg.c | 29 | ||||
| -rw-r--r-- | gpg-interface.c | 15 | ||||
| -rw-r--r-- | gpg-interface.h | 9 | ||||
| -rw-r--r-- | log-tree.c | 15 | ||||
| -rw-r--r-- | ref-filter.c | 33 | ||||
| -rwxr-xr-x | t/t7004-tag.sh | 25 | ||||
| -rwxr-xr-x | t/t7510-signed-commit.sh | 43 | ||||
| -rw-r--r-- | tag.c | 15 |
12 files changed, 225 insertions, 74 deletions
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index d49d050..b89ce31 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -764,7 +764,7 @@ static void prepare_push_cert_sha1(struct child_process *proc) memset(&sigcheck, '\0', sizeof(sigcheck)); - bogs = parse_signature(push_cert.buf, push_cert.len); + bogs = parse_signed_buffer(push_cert.buf, push_cert.len); check_signature(push_cert.buf, bogs, push_cert.buf + bogs, push_cert.len - bogs, &sigcheck); @@ -2050,7 +2050,7 @@ static void queue_commands_from_cert(struct command **tail, die("malformed push certificate %.*s", 100, push_cert->buf); else boc += 2; - eoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len); + eoc = push_cert->buf + parse_signed_buffer(push_cert->buf, push_cert->len); while (boc < eoc) { const char *eol = memchr(boc, '\n', eoc - boc); diff --git a/builtin/tag.c b/builtin/tag.c index e8b85ee..4237dc7 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -198,11 +198,17 @@ static void write_tag_body(int fd, const struct object_id *oid) { unsigned long size; enum object_type type; - char *buf, *sp; + char *buf, *sp, *orig; + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; - buf = read_object_file(oid, &type, &size); + orig = buf = read_object_file(oid, &type, &size); if (!buf) return; + if (parse_signature(buf, size, &payload, &signature)) { + buf = payload.buf; + size = payload.len; + } /* skip header */ sp = strstr(buf, "\n\n"); @@ -211,9 +217,11 @@ static void write_tag_body(int fd, const struct object_id *oid) return; } sp += 2; /* skip the 2 LFs */ - write_or_die(fd, sp, parse_signature(sp, buf + size - sp)); + write_or_die(fd, sp, buf + size - sp); - free(buf); + free(orig); + strbuf_release(&payload); + strbuf_release(&signature); } static int build_tag_object(struct strbuf *buf, int sign, struct object_id *result) @@ -995,7 +995,7 @@ static const char *gpg_sig_headers[] = { "gpgsig-sha256", }; -static int do_sign_commit(struct strbuf *buf, const char *keyid) +int sign_with_header(struct strbuf *buf, const char *keyid) { struct strbuf sig = STRBUF_INIT; int inspos, copypos; @@ -1035,21 +1035,32 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid) return 0; } + + int parse_signed_commit(const struct commit *commit, - struct strbuf *payload, struct strbuf *signature) + struct strbuf *payload, struct strbuf *signature, + const struct git_hash_algo *algop) { - unsigned long size; const char *buffer = get_commit_buffer(commit, &size); - int in_signature, saw_signature = -1; - const char *line, *tail; - const char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)]; - int gpg_sig_header_len = strlen(gpg_sig_header); + int ret = parse_buffer_signed_by_header(buffer, size, payload, signature, algop); + + unuse_commit_buffer(commit, buffer); + return ret; +} + +int parse_buffer_signed_by_header(const char *buffer, + unsigned long size, + struct strbuf *payload, + struct strbuf *signature, + const struct git_hash_algo *algop) +{ + int in_signature = 0, saw_signature = 0, other_signature = 0; + const char *line, *tail, *p; + const char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)]; line = buffer; tail = buffer + size; - in_signature = 0; - saw_signature = 0; while (line < tail) { const char *sig = NULL; const char *next = memchr(line, '\n', tail - line); @@ -1057,9 +1068,15 @@ int parse_signed_commit(const struct commit *commit, next = next ? next + 1 : tail; if (in_signature && line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) && - line[gpg_sig_header_len] == ' ') - sig = line + gpg_sig_header_len + 1; + else if (skip_prefix(line, gpg_sig_header, &p) && + *p == ' ') { + sig = line + strlen(gpg_sig_header) + 1; + other_signature = 0; + } + else if (starts_with(line, "gpgsig")) + other_signature = 1; + else if (other_signature && line[0] != ' ') + other_signature = 0; if (sig) { strbuf_add(signature, sig, next - sig); saw_signature = 1; @@ -1068,12 +1085,12 @@ int parse_signed_commit(const struct commit *commit, if (*line == '\n') /* dump the whole remainder of the buffer */ next = tail; - strbuf_add(payload, line, next - line); + if (!other_signature) + strbuf_add(payload, line, next - line); in_signature = 0; } line = next; } - unuse_commit_buffer(commit, buffer); return saw_signature; } @@ -1082,23 +1099,29 @@ int remove_signature(struct strbuf *buf) const char *line = buf->buf; const char *tail = buf->buf + buf->len; int in_signature = 0; - const char *sig_start = NULL; - const char *sig_end = NULL; + struct sigbuf { + const char *start; + const char *end; + } sigs[2], *sigp = &sigs[0]; + int i; + const char *orig_buf = buf->buf; + + memset(sigs, 0, sizeof(sigs)); while (line < tail) { const char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; if (in_signature && line[0] == ' ') - sig_end = next; + sigp->end = next; else if (starts_with(line, "gpgsig")) { int i; for (i = 1; i < GIT_HASH_NALGOS; i++) { const char *p; if (skip_prefix(line, gpg_sig_headers[i], &p) && *p == ' ') { - sig_start = line; - sig_end = next; + sigp->start = line; + sigp->end = next; in_signature = 1; } } @@ -1106,15 +1129,18 @@ int remove_signature(struct strbuf *buf) if (*line == '\n') /* dump the whole remainder of the buffer */ next = tail; + if (in_signature && sigp - sigs != ARRAY_SIZE(sigs)) + sigp++; in_signature = 0; } line = next; } - if (sig_start) - strbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start); + for (i = ARRAY_SIZE(sigs) - 1; i >= 0; i--) + if (sigs[i].start) + strbuf_remove(buf, sigs[i].start - orig_buf, sigs[i].end - sigs[i].start); - return sig_start != NULL; + return sigs[0].start != NULL; } static void handle_signed_tag(struct commit *parent, struct commit_extra_header ***tail) @@ -1122,8 +1148,10 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header struct merge_remote_desc *desc; struct commit_extra_header *mergetag; char *buf; - unsigned long size, len; + unsigned long size; enum object_type type; + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; desc = merge_remote_util(parent); if (!desc || !desc->obj) @@ -1131,8 +1159,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header buf = read_object_file(&desc->obj->oid, &type, &size); if (!buf || type != OBJ_TAG) goto free_return; - len = parse_signature(buf, size); - if (size == len) + if (!parse_signature(buf, size, &payload, &signature)) goto free_return; /* * We could verify this signature and either omit the tag when @@ -1151,6 +1178,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header **tail = mergetag; *tail = &mergetag->next; + strbuf_release(&payload); + strbuf_release(&signature); return; free_return: @@ -1165,7 +1194,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check * sigc->result = 'N'; - if (parse_signed_commit(commit, &payload, &signature) <= 0) + if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0) goto out; ret = check_signature(payload.buf, payload.len, signature.buf, signature.len, sigc); @@ -1515,7 +1544,7 @@ int commit_tree_extended(const char *msg, size_t msg_len, if (encoding_is_utf8 && !verify_utf8(&buffer)) fprintf(stderr, _(commit_utf8_warn)); - if (sign_commit && do_sign_commit(&buffer, sign_commit)) { + if (sign_commit && sign_with_header(&buffer, sign_commit)) { result = -1; goto out; } @@ -319,7 +319,8 @@ void set_merge_remote_desc(struct commit *commit, struct commit *get_merge_parent(const char *name); int parse_signed_commit(const struct commit *commit, - struct strbuf *message, struct strbuf *signature); + struct strbuf *message, struct strbuf *signature, + const struct git_hash_algo *algop); int remove_signature(struct strbuf *buf); /* @@ -361,4 +362,13 @@ int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void LAST_ARG_MUST_BE_NULL int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...); +/* Sign a commit or tag buffer, storing the result in a header. */ +int sign_with_header(struct strbuf *buf, const char *keyid); +/* Parse the signature out of a header. */ +int parse_buffer_signed_by_header(const char *buffer, + unsigned long size, + struct strbuf *payload, + struct strbuf *signature, + const struct git_hash_algo *algop); + #endif /* COMMIT_H */ diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index 46f6015..1e51492 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -510,22 +510,28 @@ static void fmt_merge_msg_sigs(struct strbuf *out) for (i = 0; i < origins.nr; i++) { struct object_id *oid = origins.items[i].util; enum object_type type; - unsigned long size, len; + unsigned long size; char *buf = read_object_file(oid, &type, &size); + char *origbuf = buf; + unsigned long len = size; struct signature_check sigc = { NULL }; - struct strbuf sig = STRBUF_INIT; + struct strbuf payload = STRBUF_INIT, sig = STRBUF_INIT; if (!buf || type != OBJ_TAG) goto next; - len = parse_signature(buf, size); - if (size == len) - ; /* merely annotated */ - else if (check_signature(buf, len, buf + len, size - len, &sigc) && - !sigc.gpg_output) - strbuf_addstr(&sig, "gpg verification failed.\n"); - else - strbuf_addstr(&sig, sigc.gpg_output); + if (!parse_signature(buf, size, &payload, &sig)) + ;/* merely annotated */ + else { + buf = payload.buf; + len = payload.len; + if (check_signature(payload.buf, payload.len, sig.buf, + sig.len, &sigc) && + !sigc.gpg_output) + strbuf_addstr(&sig, "gpg verification failed.\n"); + else + strbuf_addstr(&sig, sigc.gpg_output); + } signature_check_clear(&sigc); if (!tag_number++) { @@ -548,9 +554,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out) strlen(origins.items[i].string)); fmt_tag_signature(&tagbuf, &sig, buf, len); } + strbuf_release(&payload); strbuf_release(&sig); next: - free(buf); + free(origbuf); } if (tagbuf.len) { strbuf_addch(out, '\n'); diff --git a/gpg-interface.c b/gpg-interface.c index b499270..127aecf 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "commit.h" #include "config.h" #include "run-command.h" #include "strbuf.h" @@ -345,7 +346,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags) fputs(output, stderr); } -size_t parse_signature(const char *buf, size_t size) +size_t parse_signed_buffer(const char *buf, size_t size) { size_t len = 0; size_t match = size; @@ -361,6 +362,18 @@ size_t parse_signature(const char *buf, size_t size) return match; } +int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature) +{ + size_t match = parse_signed_buffer(buf, size); + if (match != size) { + strbuf_add(payload, buf, match); + remove_signature(payload); + strbuf_add(signature, buf + match, size - match); + return 1; + } + return 0; +} + void set_signing_key(const char *key) { free(configured_signing_key); diff --git a/gpg-interface.h b/gpg-interface.h index f4e9b4f..80567e4 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -38,12 +38,19 @@ struct signature_check { void signature_check_clear(struct signature_check *sigc); /* + * Look at a GPG signed tag object. If such a signature exists, store it in + * signature and the signed content in payload. Return 1 if a signature was + * found, and 0 otherwise. + */ +int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct strbuf *signature); + +/* * Look at GPG signed content (e.g. a signed tag object), whose * payload is followed by a detached signature on it. Return the * offset where the embedded detached signature begins, or the end of * the data when there is no such signature. */ -size_t parse_signature(const char *buf, size_t size); +size_t parse_signed_buffer(const char *buf, size_t size); /* * Create a detached signature for the contents of "buffer" and append @@ -502,7 +502,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit) struct signature_check sigc = { 0 }; int status; - if (parse_signed_commit(commit, &payload, &signature) <= 0) + if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0) goto out; status = check_signature(payload.buf, payload.len, signature.buf, @@ -548,7 +548,8 @@ static int show_one_mergetag(struct commit *commit, struct strbuf verify_message; struct signature_check sigc = { 0 }; int status, nth; - size_t payload_size; + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; hash_object_file(the_hash_algo, extra->value, extra->len, type_name(OBJ_TAG), &oid); @@ -571,13 +572,11 @@ static int show_one_mergetag(struct commit *commit, strbuf_addf(&verify_message, "parent #%d, tagged '%s'\n", nth + 1, tag->tag); - payload_size = parse_signature(extra->value, extra->len); status = -1; - if (extra->len > payload_size) { + if (parse_signature(extra->value, extra->len, &payload, &signature)) { /* could have a good signature */ - status = check_signature(extra->value, payload_size, - extra->value + payload_size, - extra->len - payload_size, &sigc); + status = check_signature(payload.buf, payload.len, + signature.buf, signature.len, &sigc); if (sigc.gpg_output) strbuf_addstr(&verify_message, sigc.gpg_output); else @@ -588,6 +587,8 @@ static int show_one_mergetag(struct commit *commit, show_sig_lines(opt, status, verify_message.buf); strbuf_release(&verify_message); + strbuf_release(&payload); + strbuf_release(&signature); return 0; } diff --git a/ref-filter.c b/ref-filter.c index fd994e1..bade652 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1210,12 +1210,20 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void } static void find_subpos(const char *buf, - const char **sub, unsigned long *sublen, - const char **body, unsigned long *bodylen, - unsigned long *nonsiglen, - const char **sig, unsigned long *siglen) + const char **sub, size_t *sublen, + const char **body, size_t *bodylen, + size_t *nonsiglen, + const char **sig, size_t *siglen) { + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; const char *eol; + const char *end = buf + strlen(buf); + const char *sigstart; + + /* parse signature first; we might not even have a subject line */ + parse_signature(buf, end - buf, &payload, &signature); + /* skip past header until we hit empty line */ while (*buf && *buf != '\n') { eol = strchrnul(buf, '\n'); @@ -1226,16 +1234,14 @@ static void find_subpos(const char *buf, /* skip any empty lines */ while (*buf == '\n') buf++; - - /* parse signature first; we might not even have a subject line */ - *sig = buf + parse_signature(buf, strlen(buf)); - *siglen = strlen(*sig); + *sig = strbuf_detach(&signature, siglen); + sigstart = buf + parse_signed_buffer(buf, strlen(buf)); /* subject is first non-empty line */ *sub = buf; /* subject goes to first empty line before signature begins */ if ((eol = strstr(*sub, "\n\n"))) { - eol = eol < *sig ? eol : *sig; + eol = eol < sigstart ? eol : sigstart; /* check if message uses CRLF */ } else if (! (eol = strstr(*sub, "\r\n\r\n"))) { /* treat whole message as subject */ @@ -1253,7 +1259,7 @@ static void find_subpos(const char *buf, buf++; *body = buf; *bodylen = strlen(buf); - *nonsiglen = *sig - buf; + *nonsiglen = sigstart - buf; } /* @@ -1285,12 +1291,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf) { int i; const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL; - unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0; + size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0; for (i = 0; i < used_atom_cnt; i++) { struct used_atom *atom = &used_atom[i]; const char *name = atom->name; struct atom_value *v = &val[i]; + if (!!deref != (*name == '*')) continue; if (deref) @@ -1322,7 +1329,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf) v->s = xmemdupz(sigpos, siglen); else if (atom->u.contents.option == C_LINES) { struct strbuf s = STRBUF_INIT; - const char *contents_end = bodylen + bodypos - siglen; + const char *contents_end = bodypos + nonsiglen; /* Size is the length of the message after removing the signature */ append_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines); @@ -1336,7 +1343,9 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf) v->s = strbuf_detach(&s, NULL); } else if (atom->u.contents.option == C_BARE) v->s = xstrdup(subpos); + } + free((void *)sigpos); } /* diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 943a7d5..400b83a 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -20,6 +20,13 @@ tag_exists () { git show-ref --quiet --verify refs/tags/"$1" } +test_expect_success 'setup' ' + test_oid_cache <<-EOM + othersigheader sha1:gpgsig-sha256 + othersigheader sha256:gpgsig + EOM +' + test_expect_success 'listing all tags in an empty tree should succeed' ' git tag -l && git tag @@ -1374,6 +1381,24 @@ test_expect_success GPG \ 'test_config gpg.program echo && test_must_fail git tag -s -m tail tag-gpg-failure' +# try to produce invalid signature +test_expect_success GPG 'git verifies tag is valid with double signature' ' + git tag -s -m tail tag-gpg-double-sig && + git cat-file tag tag-gpg-double-sig >tag && + othersigheader=$(test_oid othersigheader) && + sed -ne "/^\$/q;p" tag >new-tag && + cat <<-EOM >>new-tag && + $othersigheader -----BEGIN PGP SIGNATURE----- + someinvaliddata + -----END PGP SIGNATURE----- + EOM + sed -e "1,/^tagger/d" tag >>new-tag && + new_tag=$(git hash-object -t tag -w new-tag) && + git update-ref refs/tags/tag-gpg-double-sig $new_tag && + git verify-tag tag-gpg-double-sig && + git fsck +' + # try to sign with bad user.signingkey test_expect_success GPGSM \ 'git tag -s fails if gpgsm is misconfigured (bad key)' \ diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index f4bf925..8df5a74 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -175,7 +175,7 @@ test_expect_success GPG 'show signed commit with signature' ' git cat-file commit initial >cat && grep -v -e "gpg: " -e "Warning: " show >show.commit && grep -e "gpg: " -e "Warning: " show >show.gpg && - grep -v "^ " cat | grep -v "^$(test_oid header) " >cat.commit && + grep -v "^ " cat | grep -v "^gpgsig.* " >cat.commit && test_cmp show.commit commit && test_cmp show.gpg verify.2 && test_cmp cat.commit verify.1 @@ -337,4 +337,45 @@ test_expect_success GPG 'show double signature with custom format' ' test_cmp expect actual ' + +test_expect_success GPG 'verify-commit verifies multiply signed commits' ' + git init multiply-signed && + cd multiply-signed && + test_commit first && + echo 1 >second && + git add second && + tree=$(git write-tree) && + parent=$(git rev-parse HEAD^{commit}) && + git commit --gpg-sign -m second && + git cat-file commit HEAD && + # Avoid trailing whitespace. + sed -e "s/^Q//" -e "s/^Z/ /" >commit <<-EOF && + Qtree $tree + Qparent $parent + Qauthor A U Thor <author@example.com> 1112912653 -0700 + Qcommitter C O Mitter <committer@example.com> 1112912653 -0700 + Qgpgsig -----BEGIN PGP SIGNATURE----- + QZ + Q iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBDRYcY29tbWl0dGVy + Q QGV4YW1wbGUuY29tAAoJEBO29R7N3kMNd+8AoK1I8mhLHviPH+q2I5fIVgPsEtYC + Q AKCTqBh+VabJceXcGIZuF0Ry+udbBQ== + Q =tQ0N + Q -----END PGP SIGNATURE----- + Qgpgsig-sha256 -----BEGIN PGP SIGNATURE----- + QZ + Q iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBIBYcY29tbWl0dGVy + Q QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN/NEAn0XO9RYSBj2dFyozi0JKSbssYMtO + Q AJwKCQ1BQOtuwz//IjU8TiS+6S4iUw== + Q =pIwP + Q -----END PGP SIGNATURE----- + Q + Qsecond + EOF + head=$(git hash-object -t commit -w commit) && + git reset --hard $head && + git verify-commit $head 2>actual && + grep "Good signature from" actual && + ! grep "BAD signature from" actual +' + test_done @@ -13,26 +13,27 @@ const char *tag_type = "tag"; static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) { struct signature_check sigc; - size_t payload_size; + struct strbuf payload = STRBUF_INIT; + struct strbuf signature = STRBUF_INIT; int ret; memset(&sigc, 0, sizeof(sigc)); - payload_size = parse_signature(buf, size); - - if (size == payload_size) { + if (!parse_signature(buf, size, &payload, &signature)) { if (flags & GPG_VERIFY_VERBOSE) - write_in_full(1, buf, payload_size); + write_in_full(1, buf, size); return error("no signature found"); } - ret = check_signature(buf, payload_size, buf + payload_size, - size - payload_size, &sigc); + ret = check_signature(payload.buf, payload.len, signature.buf, + signature.len, &sigc); if (!(flags & GPG_VERIFY_OMIT_STATUS)) print_signature_buffer(&sigc, flags); signature_check_clear(&sigc); + strbuf_release(&payload); + strbuf_release(&signature); return ret; } |