Skip to content

Commit 930078b

Browse files
gitstergitster
committed
Merge branch 'hi/gpg-use-check-signature'
Hide lower-level verify_signed-buffer() API as a pure helper to implement the public check_signature() function, in order to encourage new callers to use the correct and more strict validation. * hi/gpg-use-check-signature: gpg-interface: prefer check_signature() for GPG verification
2 parents 08d2f46 + 72b006f commit 930078b

File tree

4 files changed

+72
-75
lines changed

4 files changed

+72
-75
lines changed

‎builtin/fmt-merge-msg.c‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ 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 };
497498
struct strbuf sig = STRBUF_INIT;
498499

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

503504
if (size == len)
504505
; /* merely annotated */
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-
}
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");
509512

510513
if (!tag_number++) {
511514
fmt_tag_signature(&tagbuf, &sig, buf, len);

‎gpg-interface.c‎

Lines changed: 49 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,55 @@ 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+
210259
int check_signature(const char *payload, size_t plen, const char *signature,
211260
size_t slen, struct signature_check *sigc)
212261
{
@@ -351,51 +400,3 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
351400

352401
return 0;
353402
}
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: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,6 @@ 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-
5849
int git_gpg_config(const char *, const char *, void *);
5950
void set_signing_key(const char *);
6051
const char *get_signing_key(void);

‎log-tree.c‎

Lines changed: 16 additions & 14 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 strbuf gpg_output = STRBUF_INIT;
452+
struct signature_check sigc = { 0 };
453453
int status;
454454

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

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);
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+
}
465466

466467
out:
467-
strbuf_release(&gpg_output);
468468
strbuf_release(&payload);
469469
strbuf_release(&signature);
470470
}
@@ -497,6 +497,7 @@ 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 };
500501
int status, nth;
501502
size_t payload_size, gpg_message_offset;
502503

@@ -525,12 +526,13 @@ static int show_one_mergetag(struct commit *commit,
525526
status = -1;
526527
if (extra->len > payload_size) {
527528
/* could have a good signature */
528-
if (!verify_signed_buffer(extra->value, payload_size,
529-
extra->value + payload_size,
530-
extra->len - payload_size,
531-
&verify_message, NULL))
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);
532534
status = 0; /* good */
533-
else if (verify_message.len <= gpg_message_offset)
535+
} else if (verify_message.len <= gpg_message_offset)
534536
strbuf_addstr(&verify_message, "No signature\n");
535537
/* otherwise we couldn't verify, which is shown as bad */
536538
}

0 commit comments

Comments
 (0)