Revert "gpg-interface: prefer check_signature() for GPG verification"
This reverts commit 72b006f4bfd30b7c5037c163efaf279ab65bea9c, which breaks the end-user experience when merging a signed tag without having the public key. We should report "can't check because we have no public key", but the code with this change claimed that there was no signature.
Junio C Hamano committed
Feb 28, 2020 at 09:43 UTC
0106b1d4be166fd4f7bcf0b901d50940c9f539e2
4 files changed
+75
-72
builtin/fmt-merge-msg.c
+4
-7
@@ -495,7 +495,6 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
495
enum object_type type;
496
unsigned long size, len;
497
char *buf = read_object_file(oid, &type, &size);
498
- struct signature_check sigc = { 0 };
498
struct strbuf sig = STRBUF_INIT;
499
500
if (!buf || type != OBJ_TAG)
@@ -504,12 +503,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
503
504
if (size == len)
505
; /* merely annotated */
507
- else if (!check_signature(buf, len, buf + len, size - len,
508
- &sigc)) {
509
- strbuf_addstr(&sig, sigc.gpg_output);
510
- signature_check_clear(&sigc);
511
- } else
512
- strbuf_addstr(&sig, "gpg verification failed.\n");
506
+ else if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) {
507
+ if (!sig.len)
508
+ strbuf_addstr(&sig, "gpg verification failed.\n");
509
+ }
510
511
if (!tag_number++) {
512
fmt_tag_signature(&tagbuf, &sig, buf, len);
gpg-interface.c
+48
-49
@@ -207,55 +207,6 @@ found_duplicate_status:
207
FREE_AND_NULL(sigc->key);
208
}
209
210
-static int verify_signed_buffer(const char *payload, size_t payload_size,
211
- const char *signature, size_t signature_size,
212
- struct strbuf *gpg_output,
213
- struct strbuf *gpg_status)
214
-{
215
- struct child_process gpg = CHILD_PROCESS_INIT;
216
- struct gpg_format *fmt;
217
- struct tempfile *temp;
218
- int ret;
219
- struct strbuf buf = STRBUF_INIT;
220
-
221
- temp = mks_tempfile_t(".git_vtag_tmpXXXXXX");
222
- if (!temp)
223
- return error_errno(_("could not create temporary file"));
224
- if (write_in_full(temp->fd, signature, signature_size) < 0 ||
225
- close_tempfile_gently(temp) < 0) {
226
- error_errno(_("failed writing detached signature to '%s'"),
227
- temp->filename.buf);
228
- delete_tempfile(&temp);
229
- return -1;
230
- }
231
-
232
- fmt = get_format_by_sig(signature);
233
- if (!fmt)
234
- BUG("bad signature '%s'", signature);
235
-
236
- argv_array_push(&gpg.args, fmt->program);
237
- argv_array_pushv(&gpg.args, fmt->verify_args);
238
- argv_array_pushl(&gpg.args,
239
- "--status-fd=1",
240
- "--verify", temp->filename.buf, "-",
241
- NULL);
242
-
243
- if (!gpg_status)
244
- gpg_status = &buf;
245
-
246
- sigchain_push(SIGPIPE, SIG_IGN);
247
- ret = pipe_command(&gpg, payload, payload_size,
248
- gpg_status, 0, gpg_output, 0);
249
- sigchain_pop(SIGPIPE);
250
-
251
- delete_tempfile(&temp);
252
-
253
- ret |= !strstr(gpg_status->buf, "\n[GNUPG:] GOODSIG ");
254
- strbuf_release(&buf); /* no matter it was used or not */
255
-
256
- return ret;
257
-}
258
-
210
int check_signature(const char *payload, size_t plen, const char *signature,
211
size_t slen, struct signature_check *sigc)
212
{
@@ -400,3 +351,51 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
351
352
return 0;
353
}
354
+
355
+int verify_signed_buffer(const char *payload, size_t payload_size,
356
+ const char *signature, size_t signature_size,
357
+ struct strbuf *gpg_output, struct strbuf *gpg_status)
358
+{
359
+ struct child_process gpg = CHILD_PROCESS_INIT;
360
+ struct gpg_format *fmt;
361
+ struct tempfile *temp;
362
+ int ret;
363
+ struct strbuf buf = STRBUF_INIT;
364
+
365
+ temp = mks_tempfile_t(".git_vtag_tmpXXXXXX");
366
+ if (!temp)
367
+ return error_errno(_("could not create temporary file"));
368
+ if (write_in_full(temp->fd, signature, signature_size) < 0 ||
369
+ close_tempfile_gently(temp) < 0) {
370
+ error_errno(_("failed writing detached signature to '%s'"),
371
+ temp->filename.buf);
372
+ delete_tempfile(&temp);
373
+ return -1;
374
+ }
375
+
376
+ fmt = get_format_by_sig(signature);
377
+ if (!fmt)
378
+ BUG("bad signature '%s'", signature);
379
+
380
+ argv_array_push(&gpg.args, fmt->program);
381
+ argv_array_pushv(&gpg.args, fmt->verify_args);
382
+ argv_array_pushl(&gpg.args,
383
+ "--status-fd=1",
384
+ "--verify", temp->filename.buf, "-",
385
+ NULL);
386
+
387
+ if (!gpg_status)
388
+ gpg_status = &buf;
389
+
390
+ sigchain_push(SIGPIPE, SIG_IGN);
391
+ ret = pipe_command(&gpg, payload, payload_size,
392
+ gpg_status, 0, gpg_output, 0);
393
+ sigchain_pop(SIGPIPE);
394
+
395
+ delete_tempfile(&temp);
396
+
397
+ ret |= !strstr(gpg_status->buf, "\n[GNUPG:] GOODSIG ");
398
+ strbuf_release(&buf); /* no matter it was used or not */
399
+
400
+ return ret;
401
+}
gpg-interface.h
+9
@@ -46,6 +46,15 @@ size_t parse_signature(const char *buf, size_t size);
46
int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
47
const char *signing_key);
48
49
+/*
50
+ * Run "gpg" to see if the payload matches the detached signature.
51
+ * gpg_output, when set, receives the diagnostic output from GPG.
52
+ * gpg_status, when set, receives the status output from GPG.
53
+ */
54
+int verify_signed_buffer(const char *payload, size_t payload_size,
55
+ const char *signature, size_t signature_size,
56
+ struct strbuf *gpg_output, struct strbuf *gpg_status);
57
+
58
int git_gpg_config(const char *, const char *, void *);
59
void set_signing_key(const char *);
60
const char *get_signing_key(void);
log-tree.c
+14
-16
@@ -448,22 +448,22 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
448
{
449
struct strbuf payload = STRBUF_INIT;
450
struct strbuf signature = STRBUF_INIT;
451
- struct signature_check sigc = { 0 };
451
+ struct strbuf gpg_output = STRBUF_INIT;
452
int status;
453
454
if (parse_signed_commit(commit, &payload, &signature) <= 0)
455
goto out;
456
457
- status = check_signature(payload.buf, payload.len, signature.buf,
458
- signature.len, &sigc);
459
- if (status && sigc.result == 'N')
460
- show_sig_lines(opt, status, "No signature\n");
461
- else {
462
- show_sig_lines(opt, status, sigc.gpg_output);
463
- signature_check_clear(&sigc);
464
- }
457
+ status = verify_signed_buffer(payload.buf, payload.len,
458
+ signature.buf, signature.len,
459
+ &gpg_output, NULL);
460
+ if (status && !gpg_output.len)
461
+ strbuf_addstr(&gpg_output, "No signature\n");
462
+
463
+ show_sig_lines(opt, status, gpg_output.buf);
464
465
out:
466
+ strbuf_release(&gpg_output);
467
strbuf_release(&payload);
468
strbuf_release(&signature);
469
}
@@ -496,7 +496,6 @@ static int show_one_mergetag(struct commit *commit,
496
struct object_id oid;
497
struct tag *tag;
498
struct strbuf verify_message;
499
- struct signature_check sigc = { 0 };
499
int status, nth;
500
size_t payload_size, gpg_message_offset;
501
@@ -525,13 +524,12 @@ static int show_one_mergetag(struct commit *commit,
524
status = -1;
525
if (extra->len > payload_size) {
526
/* could have a good signature */
528
- if (!check_signature(extra->value, payload_size,
529
- extra->value + payload_size,
530
- extra->len - payload_size, &sigc)) {
531
- strbuf_addstr(&verify_message, sigc.gpg_output);
532
- signature_check_clear(&sigc);
527
+ if (!verify_signed_buffer(extra->value, payload_size,
528
+ extra->value + payload_size,
529
+ extra->len - payload_size,
530
+ &verify_message, NULL))
531
status = 0; /* good */
534
- } else if (verify_message.len <= gpg_message_offset)
532
+ else if (verify_message.len <= gpg_message_offset)
533
strbuf_addstr(&verify_message, "No signature\n");
534
/* otherwise we couldn't verify, which is shown as bad */
535
}