Skip to content

Commit 4e730fc

Browse files
gitstergitster
committed
Merge branch 'hi/gpg-use-check-signature' into maint
"git merge signed-tag" while lacking the public key started to say "No signature", which was utterly wrong. This regression has been reverted. * hi/gpg-use-check-signature: Revert "gpg-interface: prefer check_signature() for GPG verification"
2 parents 76ccbda + 0106b1d commit 4e730fc

File tree

4 files changed

+75
-72
lines changed

4 files changed

+75
-72
lines changed

‎builtin/fmt-merge-msg.c‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,6 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
494494
enum object_type type;
495495
unsigned long size, len;
496496
char *buf = read_object_file(oid, &type, &size);
497-
struct signature_check sigc = { 0 };
498497
struct strbuf sig = STRBUF_INIT;
499498

500499
if (!buf || type != OBJ_TAG)
@@ -503,12 +502,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
503502

504503
if (size == len)
505504
; /* merely annotated */
506-
else if (!check_signature(buf, len, buf + len, size - len,
507-
&sigc)) {
508-
strbuf_addstr(&sig, sigc.gpg_output);
509-
signature_check_clear(&sigc);
510-
} else
511-
strbuf_addstr(&sig, "gpg verification failed.\n");
505+
else if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) {
506+
if (!sig.len)
507+
strbuf_addstr(&sig, "gpg verification failed.\n");
508+
}
512509

513510
if (!tag_number++) {
514511
fmt_tag_signature(&tagbuf, &sig, buf, len);

‎gpg-interface.c‎

Lines changed: 48 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -207,55 +207,6 @@ static void parse_gpg_output(struct signature_check *sigc)
207207
FREE_AND_NULL(sigc->key);
208208
}
209209

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-
259210
int check_signature(const char *payload, size_t plen, const char *signature,
260211
size_t slen, struct signature_check *sigc)
261212
{
@@ -400,3 +351,51 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
400351

401352
return 0;
402353
}
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‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ size_t parse_signature(const char *buf, size_t size);
4646
int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
4747
const char *signing_key);
4848

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+
4958
int git_gpg_config(const char *, const char *, void *);
5059
void set_signing_key(const char *);
5160
const char *get_signing_key(void);

‎log-tree.c‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -449,22 +449,22 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
449449
{
450450
struct strbuf payload = STRBUF_INIT;
451451
struct strbuf signature = STRBUF_INIT;
452-
struct signature_check sigc = { 0 };
452+
struct strbuf gpg_output = STRBUF_INIT;
453453
int status;
454454

455455
if (parse_signed_commit(commit, &payload, &signature) <= 0)
456456
goto out;
457457

458-
status = check_signature(payload.buf, payload.len, signature.buf,
459-
signature.len, &sigc);
460-
if (status && sigc.result == 'N')
461-
show_sig_lines(opt, status, "No signature\n");
462-
else {
463-
show_sig_lines(opt, status, sigc.gpg_output);
464-
signature_check_clear(&sigc);
465-
}
458+
status = verify_signed_buffer(payload.buf, payload.len,
459+
signature.buf, signature.len,
460+
&gpg_output, NULL);
461+
if (status && !gpg_output.len)
462+
strbuf_addstr(&gpg_output, "No signature\n");
463+
464+
show_sig_lines(opt, status, gpg_output.buf);
466465

467466
out:
467+
strbuf_release(&gpg_output);
468468
strbuf_release(&payload);
469469
strbuf_release(&signature);
470470
}
@@ -497,7 +497,6 @@ static int show_one_mergetag(struct commit *commit,
497497
struct object_id oid;
498498
struct tag *tag;
499499
struct strbuf verify_message;
500-
struct signature_check sigc = { 0 };
501500
int status, nth;
502501
size_t payload_size, gpg_message_offset;
503502

@@ -526,13 +525,12 @@ static int show_one_mergetag(struct commit *commit,
526525
status = -1;
527526
if (extra->len > payload_size) {
528527
/* could have a good signature */
529-
if (!check_signature(extra->value, payload_size,
530-
extra->value + payload_size,
531-
extra->len - payload_size, &sigc)) {
532-
strbuf_addstr(&verify_message, sigc.gpg_output);
533-
signature_check_clear(&sigc);
528+
if (!verify_signed_buffer(extra->value, payload_size,
529+
extra->value + payload_size,
530+
extra->len - payload_size,
531+
&verify_message, NULL))
534532
status = 0; /* good */
535-
} else if (verify_message.len <= gpg_message_offset)
533+
else if (verify_message.len <= gpg_message_offset)
536534
strbuf_addstr(&verify_message, "No signature\n");
537535
/* otherwise we couldn't verify, which is shown as bad */
538536
}

0 commit comments

Comments
 (0)