ssh signing: use sigc struct to pass payload

To be able to extend the payload metadata with things like its creation timestamp or the creators ident we remove the payload parameters to check_signature() and use the already existing sigc->payload field instead, only adding the length field to the struct. This also allows us to get rid of the xmemdupz() calls in the verify functions. Since sigc is now used to input data as well as output the result move it to the front of the function list. - Add payload_length to struct signature_check - Populate sigc.payload/payload_len on all call sites - Remove payload parameters to check_signature() - Remove payload parameters to internal verify_* functions and use sigc instead - Remove xmemdupz() used for verbose output since payload is now already populated. Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Fabian Stelzer committed Dec 9, 2021 at 09:52 UTC 02769437e1421d837f6de27cfb5c14087cfec8dd
7 files changed +35 -35
builtin/receive-pack.c
+4 -2
@@ -769,8 +769,10 @@ static void prepare_push_cert_sha1(struct child_process *proc)
769 memset(&sigcheck, '\0', sizeof(sigcheck));
770
771 bogs = parse_signed_buffer(push_cert.buf, push_cert.len);
772 - check_signature(push_cert.buf, bogs, push_cert.buf + bogs,
773 - push_cert.len - bogs, &sigcheck);
772 + sigcheck.payload = xmemdupz(push_cert.buf, bogs);
773 + sigcheck.payload_len = bogs;
774 + check_signature(&sigcheck, push_cert.buf + bogs,
775 + push_cert.len - bogs);
776
777 nonce_status = check_nonce(push_cert.buf, bogs);
778 }
commit.c
+3 -2
@@ -1212,8 +1212,9 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
1212
1213 if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
1214 goto out;
1215 - ret = check_signature(payload.buf, payload.len, signature.buf,
1216 - signature.len, sigc);
1215 +
1216 + sigc->payload = strbuf_detach(&payload, &sigc->payload_len);
1217 + ret = check_signature(sigc, signature.buf, signature.len);
1218
1219 out:
1220 strbuf_release(&payload);
fmt-merge-msg.c
+2 -2
@@ -533,8 +533,8 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
533 else {
534 buf = payload.buf;
535 len = payload.len;
536 - if (check_signature(payload.buf, payload.len, sig.buf,
537 - sig.len, &sigc) &&
536 + sigc.payload = strbuf_detach(&payload, &sigc.payload_len);
537 + if (check_signature(&sigc, sig.buf, sig.len) &&
538 !sigc.output)
539 strbuf_addstr(&sig, "gpg verification failed.\n");
540 else
gpg-interface.c
+17 -20
@@ -19,8 +19,8 @@ struct gpg_format {
19 const char **verify_args;
20 const char **sigs;
21 int (*verify_signed_buffer)(struct signature_check *sigc,
22 - struct gpg_format *fmt, const char *payload,
23 - size_t payload_size, const char *signature,
22 + struct gpg_format *fmt,
23 + const char *signature,
24 size_t signature_size);
25 int (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature,
26 const char *signing_key);
@@ -53,12 +53,12 @@ static const char *ssh_sigs[] = {
53 };
54
55 static int verify_gpg_signed_buffer(struct signature_check *sigc,
56 - struct gpg_format *fmt, const char *payload,
57 - size_t payload_size, const char *signature,
56 + struct gpg_format *fmt,
57 + const char *signature,
58 size_t signature_size);
59 static int verify_ssh_signed_buffer(struct signature_check *sigc,
60 - struct gpg_format *fmt, const char *payload,
61 - size_t payload_size, const char *signature,
60 + struct gpg_format *fmt,
61 + const char *signature,
62 size_t signature_size);
63 static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
64 const char *signing_key);
@@ -314,8 +314,8 @@ error:
314 }
315
316 static int verify_gpg_signed_buffer(struct signature_check *sigc,
317 - struct gpg_format *fmt, const char *payload,
318 - size_t payload_size, const char *signature,
317 + struct gpg_format *fmt,
318 + const char *signature,
319 size_t signature_size)
320 {
321 struct child_process gpg = CHILD_PROCESS_INIT;
@@ -343,14 +343,13 @@ static int verify_gpg_signed_buffer(struct signature_check *sigc,
343 NULL);
344
345 sigchain_push(SIGPIPE, SIG_IGN);
346 - ret = pipe_command(&gpg, payload, payload_size, &gpg_stdout, 0,
346 + ret = pipe_command(&gpg, sigc->payload, sigc->payload_len, &gpg_stdout, 0,
347 &gpg_stderr, 0);
348 sigchain_pop(SIGPIPE);
349
350 delete_tempfile(&temp);
351
352 ret |= !strstr(gpg_stdout.buf, "\n[GNUPG:] GOODSIG ");
353 - sigc->payload = xmemdupz(payload, payload_size);
353 sigc->output = strbuf_detach(&gpg_stderr, NULL);
354 sigc->gpg_status = strbuf_detach(&gpg_stdout, NULL);
355
@@ -426,8 +425,8 @@ cleanup:
425 }
426
427 static int verify_ssh_signed_buffer(struct signature_check *sigc,
429 - struct gpg_format *fmt, const char *payload,
430 - size_t payload_size, const char *signature,
428 + struct gpg_format *fmt,
429 + const char *signature,
430 size_t signature_size)
431 {
432 struct child_process ssh_keygen = CHILD_PROCESS_INIT;
@@ -480,7 +479,7 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
479 "-n", "git",
480 "-s", buffer_file->filename.buf,
481 NULL);
483 - pipe_command(&ssh_keygen, payload, payload_size,
482 + pipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,
483 &ssh_keygen_out, 0, &ssh_keygen_err, 0);
484
485 /*
@@ -526,7 +525,7 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
525 }
526
527 sigchain_push(SIGPIPE, SIG_IGN);
529 - ret = pipe_command(&ssh_keygen, payload, payload_size,
528 + ret = pipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,
529 &ssh_keygen_out, 0, &ssh_keygen_err, 0);
530 sigchain_pop(SIGPIPE);
531
@@ -540,7 +539,6 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
539 }
540 }
541
543 - sigc->payload = xmemdupz(payload, payload_size);
542 strbuf_stripspace(&ssh_keygen_out, 0);
543 strbuf_stripspace(&ssh_keygen_err, 0);
544 /* Add stderr outputs to show the user actual ssh-keygen errors */
@@ -562,8 +560,8 @@ out:
560 return ret;
561 }
562
565 -int check_signature(const char *payload, size_t plen, const char *signature,
566 - size_t slen, struct signature_check *sigc)
563 +int check_signature(struct signature_check *sigc,
564 + const char *signature, size_t slen)
565 {
566 struct gpg_format *fmt;
567 int status;
@@ -575,8 +573,7 @@ int check_signature(const char *payload, size_t plen, const char *signature,
573 if (!fmt)
574 die(_("bad/incompatible signature '%s'"), signature);
575
578 - status = fmt->verify_signed_buffer(sigc, fmt, payload, plen, signature,
579 - slen);
576 + status = fmt->verify_signed_buffer(sigc, fmt, signature, slen);
577
578 if (status && !sigc->output)
579 return !!status;
@@ -593,7 +590,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
590 sigc->output;
591
592 if (flags & GPG_VERIFY_VERBOSE && sigc->payload)
596 - fputs(sigc->payload, stdout);
593 + fwrite(sigc->payload, 1, sigc->payload_len, stdout);
594
595 if (output)
596 fputs(output, stderr);
gpg-interface.h
+3 -3
@@ -17,6 +17,7 @@ enum signature_trust_level {
17
18 struct signature_check {
19 char *payload;
20 + size_t payload_len;
21 char *output;
22 char *gpg_status;
23
@@ -70,9 +71,8 @@ const char *get_signing_key(void);
71 * Either a GPG KeyID or a SSH Key Fingerprint
72 */
73 const char *get_signing_key_id(void);
73 -int check_signature(const char *payload, size_t plen,
74 - const char *signature, size_t slen,
75 - struct signature_check *sigc);
74 +int check_signature(struct signature_check *sigc,
75 + const char *signature, size_t slen);
76 void print_signature_buffer(const struct signature_check *sigc,
77 unsigned flags);
78
log-tree.c
+4 -4
@@ -513,8 +513,8 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
513 if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
514 goto out;
515
516 - status = check_signature(payload.buf, payload.len, signature.buf,
517 - signature.len, &sigc);
516 + sigc.payload = strbuf_detach(&payload, &sigc.payload_len);
517 + status = check_signature(&sigc, signature.buf, signature.len);
518 if (status && !sigc.output)
519 show_sig_lines(opt, status, "No signature\n");
520 else
@@ -583,8 +583,8 @@ static int show_one_mergetag(struct commit *commit,
583 status = -1;
584 if (parse_signature(extra->value, extra->len, &payload, &signature)) {
585 /* could have a good signature */
586 - status = check_signature(payload.buf, payload.len,
587 - signature.buf, signature.len, &sigc);
586 + sigc.payload = strbuf_detach(&payload, &sigc.payload_len);
587 + status = check_signature(&sigc, signature.buf, signature.len);
588 if (sigc.output)
589 strbuf_addstr(&verify_message, sigc.output);
590 else
tag.c
+2 -2
@@ -25,8 +25,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
25 return error("no signature found");
26 }
27
28 - ret = check_signature(payload.buf, payload.len, signature.buf,
29 - signature.len, &sigc);
28 + sigc.payload = strbuf_detach(&payload, &sigc.payload_len);
29 + ret = check_signature(&sigc, signature.buf, signature.len);
30
31 if (!(flags & GPG_VERIFY_OMIT_STATUS))
32 print_signature_buffer(&sigc, flags);