commit: ignore additional signatures when parsing signed commits
When we create a commit with multiple signatures, neither of these signatures includes the other. Consequently, when we produce the payload which has been signed so we can verify the commit, we must strip off any other signatures, or the payload will differ from what was signed. Do so, and in preparation for verifying with multiple algorithms, pass the algorithm we want to verify into parse_signed_commit. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson committed
Jan 18, 2021 at 23:49 UTC
1fb5cf0da657ef046c4eb4d0de6f2defb2fb09c6
4 files changed
+79
-23
commit.c
+34
-20
@@ -1036,20 +1036,18 @@ static int do_sign_commit(struct strbuf *buf, const char *keyid)
1036
}
1037
1038
int parse_signed_commit(const struct commit *commit,
1039
- struct strbuf *payload, struct strbuf *signature)
1039
+ struct strbuf *payload, struct strbuf *signature,
1040
+ const struct git_hash_algo *algop)
1041
{
1042
1043
unsigned long size;
1044
const char *buffer = get_commit_buffer(commit, &size);
1044
- int in_signature, saw_signature = -1;
1045
- const char *line, *tail;
1046
- const char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(the_hash_algo)];
1047
- int gpg_sig_header_len = strlen(gpg_sig_header);
1045
+ int in_signature = 0, saw_signature = 0, other_signature = 0;
1046
+ const char *line, *tail, *p;
1047
+ const char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algop)];
1048
1049
line = buffer;
1050
tail = buffer + size;
1051
- in_signature = 0;
1052
- saw_signature = 0;
1051
while (line < tail) {
1052
const char *sig = NULL;
1053
const char *next = memchr(line, '\n', tail - line);
@@ -1057,9 +1055,15 @@ int parse_signed_commit(const struct commit *commit,
1055
next = next ? next + 1 : tail;
1056
if (in_signature && line[0] == ' ')
1057
sig = line + 1;
1060
- else if (starts_with(line, gpg_sig_header) &&
1061
- line[gpg_sig_header_len] == ' ')
1062
- sig = line + gpg_sig_header_len + 1;
1058
+ else if (skip_prefix(line, gpg_sig_header, &p) &&
1059
+ *p == ' ') {
1060
+ sig = line + strlen(gpg_sig_header) + 1;
1061
+ other_signature = 0;
1062
+ }
1063
+ else if (starts_with(line, "gpgsig"))
1064
+ other_signature = 1;
1065
+ else if (other_signature && line[0] != ' ')
1066
+ other_signature = 0;
1067
if (sig) {
1068
strbuf_add(signature, sig, next - sig);
1069
saw_signature = 1;
@@ -1068,7 +1072,8 @@ int parse_signed_commit(const struct commit *commit,
1072
if (*line == '\n')
1073
/* dump the whole remainder of the buffer */
1074
next = tail;
1071
- strbuf_add(payload, line, next - line);
1075
+ if (!other_signature)
1076
+ strbuf_add(payload, line, next - line);
1077
in_signature = 0;
1078
}
1079
line = next;
@@ -1082,23 +1087,29 @@ int remove_signature(struct strbuf *buf)
1087
const char *line = buf->buf;
1088
const char *tail = buf->buf + buf->len;
1089
int in_signature = 0;
1085
- const char *sig_start = NULL;
1086
- const char *sig_end = NULL;
1090
+ struct sigbuf {
1091
+ const char *start;
1092
+ const char *end;
1093
+ } sigs[2], *sigp = &sigs[0];
1094
+ int i;
1095
+ const char *orig_buf = buf->buf;
1096
+
1097
+ memset(sigs, 0, sizeof(sigs));
1098
1099
while (line < tail) {
1100
const char *next = memchr(line, '\n', tail - line);
1101
next = next ? next + 1 : tail;
1102
1103
if (in_signature && line[0] == ' ')
1093
- sig_end = next;
1104
+ sigp->end = next;
1105
else if (starts_with(line, "gpgsig")) {
1106
int i;
1107
for (i = 1; i < GIT_HASH_NALGOS; i++) {
1108
const char *p;
1109
if (skip_prefix(line, gpg_sig_headers[i], &p) &&
1110
*p == ' ') {
1100
- sig_start = line;
1101
- sig_end = next;
1111
+ sigp->start = line;
1112
+ sigp->end = next;
1113
in_signature = 1;
1114
}
1115
}
@@ -1106,15 +1117,18 @@ int remove_signature(struct strbuf *buf)
1117
if (*line == '\n')
1118
/* dump the whole remainder of the buffer */
1119
next = tail;
1120
+ if (in_signature && sigp - sigs != ARRAY_SIZE(sigs))
1121
+ sigp++;
1122
in_signature = 0;
1123
}
1124
line = next;
1125
}
1126
1114
- if (sig_start)
1115
- strbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start);
1127
+ for (i = ARRAY_SIZE(sigs) - 1; i >= 0; i--)
1128
+ if (sigs[i].start)
1129
+ strbuf_remove(buf, sigs[i].start - orig_buf, sigs[i].end - sigs[i].start);
1130
1117
- return sig_start != NULL;
1131
+ return sigs[0].start != NULL;
1132
}
1133
1134
static void handle_signed_tag(struct commit *parent, struct commit_extra_header ***tail)
@@ -1165,7 +1179,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
1179
1180
sigc->result = 'N';
1181
1168
- if (parse_signed_commit(commit, &payload, &signature) <= 0)
1182
+ if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
1183
goto out;
1184
ret = check_signature(payload.buf, payload.len, signature.buf,
1185
signature.len, sigc);
commit.h
+2
-1
@@ -317,7 +317,8 @@ void set_merge_remote_desc(struct commit *commit,
317
struct commit *get_merge_parent(const char *name);
318
319
int parse_signed_commit(const struct commit *commit,
320
- struct strbuf *message, struct strbuf *signature);
320
+ struct strbuf *message, struct strbuf *signature,
321
+ const struct git_hash_algo *algop);
322
int remove_signature(struct strbuf *buf);
323
324
/*
log-tree.c
+1
-1
@@ -502,7 +502,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
502
struct signature_check sigc = { 0 };
503
int status;
504
505
- if (parse_signed_commit(commit, &payload, &signature) <= 0)
505
+ if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
506
goto out;
507
508
status = check_signature(payload.buf, payload.len, signature.buf,
t/t7510-signed-commit.sh
+42
-1
@@ -172,7 +172,7 @@ test_expect_success GPG 'show signed commit with signature' '
172
git cat-file commit initial >cat &&
173
grep -v -e "gpg: " -e "Warning: " show >show.commit &&
174
grep -e "gpg: " -e "Warning: " show >show.gpg &&
175
- grep -v "^ " cat | grep -v "^$(test_oid header) " >cat.commit &&
175
+ grep -v "^ " cat | grep -v "^gpgsig.* " >cat.commit &&
176
test_cmp show.commit commit &&
177
test_cmp show.gpg verify.2 &&
178
test_cmp cat.commit verify.1
@@ -334,4 +334,45 @@ test_expect_success GPG 'show double signature with custom format' '
334
test_cmp expect actual
335
'
336
337
+
338
+test_expect_success GPG 'verify-commit verifies multiply signed commits' '
339
+ git init multiply-signed &&
340
+ cd multiply-signed &&
341
+ test_commit first &&
342
+ echo 1 >second &&
343
+ git add second &&
344
+ tree=$(git write-tree) &&
345
+ parent=$(git rev-parse HEAD^{commit}) &&
346
+ git commit --gpg-sign -m second &&
347
+ git cat-file commit HEAD &&
348
+ # Avoid trailing whitespace.
349
+ sed -e "s/^Q//" -e "s/^Z/ /" >commit <<-EOF &&
350
+ Qtree $tree
351
+ Qparent $parent
352
+ Qauthor A U Thor <author@example.com> 1112912653 -0700
353
+ Qcommitter C O Mitter <committer@example.com> 1112912653 -0700
354
+ Qgpgsig -----BEGIN PGP SIGNATURE-----
355
+ QZ
356
+ Q iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBDRYcY29tbWl0dGVy
357
+ Q QGV4YW1wbGUuY29tAAoJEBO29R7N3kMNd+8AoK1I8mhLHviPH+q2I5fIVgPsEtYC
358
+ Q AKCTqBh+VabJceXcGIZuF0Ry+udbBQ==
359
+ Q =tQ0N
360
+ Q -----END PGP SIGNATURE-----
361
+ Qgpgsig-sha256 -----BEGIN PGP SIGNATURE-----
362
+ QZ
363
+ Q iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCX/uBIBYcY29tbWl0dGVy
364
+ Q QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN/NEAn0XO9RYSBj2dFyozi0JKSbssYMtO
365
+ Q AJwKCQ1BQOtuwz//IjU8TiS+6S4iUw==
366
+ Q =pIwP
367
+ Q -----END PGP SIGNATURE-----
368
+ Q
369
+ Qsecond
370
+ EOF
371
+ head=$(git hash-object -t commit -w commit) &&
372
+ git reset --hard $head &&
373
+ git verify-commit $head 2>actual &&
374
+ grep "Good signature from" actual &&
375
+ ! grep "BAD signature from" actual
376
+'
377
+
378
test_done