undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'bc/signed-objects-with-both-hashes'

Browse Source 
Signed commits and tags now allow verification of objects, whose
two object names (one in SHA-1, the other in SHA-256) are both
signed.

* bc/signed-objects-with-both-hashes:
  gpg-interface: remove other signature headers before verifying
  ref-filter: hoist signature parsing
  commit: allow parsing arbitrary buffers with headers
  gpg-interface: improve interface for parsing tags
  commit: ignore additional signatures when parsing signed commits
  ref-filter: switch some uses of unsigned long to size_t
This commit is contained in:
Junio C Hamano 2021-02-22 16:12:42 -08:00
commit 15af6e6fee
12 changed files with 226 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
builtin/receive-pack.c
View File

@ -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);

16
builtin/tag.c
View File

@ -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)

85
commit.c
View File

@ -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)
{
int parse_signed_commit(const struct commit *commit,
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;
}

12
commit.h
View File

@ -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 */

29
fmt-merge-msg.c
View File

@ -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');

15
gpg-interface.c
View File

@ -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);

9
gpg-interface.h
View File

@ -37,13 +37,20 @@ 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

15
log-tree.c
View File

@ -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;
}

33
ref-filter.c
View File

@ -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);
}
/*

25
t/t7004-tag.sh
View File

@ -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)' \

43
t/t7510-signed-commit.sh
View File

@ -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

15
tag.c
View File

@ -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;
}