ssh signing: preliminary refactoring and clean-up
Openssh v8.2p1 added some new options to ssh-keygen for signature creation and verification. These allow us to use ssh keys for git signatures easily. In our corporate environment we use PIV x509 Certs on Yubikeys for email signing/encryption and ssh keys which I think is quite common (at least for the email part). This way we can establish the correct trust for the SSH Keys without setting up a separate GPG Infrastructure (which is still quite painful for users) or implementing x509 signing support for git (which lacks good forwarding mechanisms). Using ssh agent forwarding makes this feature easily usable in todays development environments where code is often checked out in remote VMs / containers. In such a setup the keyring & revocationKeyring can be centrally generated from the x509 CA information and distributed to the users. To be able to implement new signing formats this commit: - makes the sigc structure more generic by renaming "gpg_output" to "output" - introduces function pointers in the gpg_format structure to call format specific signing and verification functions - moves format detection from verify_signed_buffer into the check_signature api function and calls the format specific verify - renames and wraps sign_buffer to handle format specific signing logic as well Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fabian Stelzer committed
Sep 10, 2021 at 20:07 UTC
b5726a5d9cabba0bd8fb6c1b25a887bc7ea4650d
5 files changed
+74
-50
fmt-merge-msg.c
+3
-3
@@ -526,11 +526,11 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
526
buf = payload.buf;
527
len = payload.len;
528
if (check_signature(payload.buf, payload.len, sig.buf,
529
- sig.len, &sigc) &&
530
- !sigc.gpg_output)
529
+ sig.len, &sigc) &&
530
+ !sigc.output)
531
strbuf_addstr(&sig, "gpg verification failed.\n");
532
else
533
- strbuf_addstr(&sig, sigc.gpg_output);
533
+ strbuf_addstr(&sig, sigc.output);
534
}
535
signature_check_clear(&sigc);
536
gpg-interface.c
+64
-40
@@ -15,6 +15,12 @@ struct gpg_format {
15
const char *program;
16
const char **verify_args;
17
const char **sigs;
18
+ int (*verify_signed_buffer)(struct signature_check *sigc,
19
+ struct gpg_format *fmt, const char *payload,
20
+ size_t payload_size, const char *signature,
21
+ size_t signature_size);
22
+ int (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature,
23
+ const char *signing_key);
24
};
25
26
static const char *openpgp_verify_args[] = {
@@ -35,14 +41,29 @@ static const char *x509_sigs[] = {
41
NULL
42
};
43
44
+static int verify_gpg_signed_buffer(struct signature_check *sigc,
45
+ struct gpg_format *fmt, const char *payload,
46
+ size_t payload_size, const char *signature,
47
+ size_t signature_size);
48
+static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
49
+ const char *signing_key);
50
+
51
static struct gpg_format gpg_format[] = {
39
- { .name = "openpgp", .program = "gpg",
40
- .verify_args = openpgp_verify_args,
41
- .sigs = openpgp_sigs
52
+ {
53
+ .name = "openpgp",
54
+ .program = "gpg",
55
+ .verify_args = openpgp_verify_args,
56
+ .sigs = openpgp_sigs,
57
+ .verify_signed_buffer = verify_gpg_signed_buffer,
58
+ .sign_buffer = sign_buffer_gpg,
59
},
43
- { .name = "x509", .program = "gpgsm",
44
- .verify_args = x509_verify_args,
45
- .sigs = x509_sigs
60
+ {
61
+ .name = "x509",
62
+ .program = "gpgsm",
63
+ .verify_args = x509_verify_args,
64
+ .sigs = x509_sigs,
65
+ .verify_signed_buffer = verify_gpg_signed_buffer,
66
+ .sign_buffer = sign_buffer_gpg,
67
},
68
};
69
@@ -72,7 +93,7 @@ static struct gpg_format *get_format_by_sig(const char *sig)
93
void signature_check_clear(struct signature_check *sigc)
94
{
95
FREE_AND_NULL(sigc->payload);
75
- FREE_AND_NULL(sigc->gpg_output);
96
+ FREE_AND_NULL(sigc->output);
97
FREE_AND_NULL(sigc->gpg_status);
98
FREE_AND_NULL(sigc->signer);
99
FREE_AND_NULL(sigc->key);
@@ -257,16 +278,16 @@ error:
278
FREE_AND_NULL(sigc->key);
279
}
280
260
-static int verify_signed_buffer(const char *payload, size_t payload_size,
261
- const char *signature, size_t signature_size,
262
- struct strbuf *gpg_output,
263
- struct strbuf *gpg_status)
281
+static int verify_gpg_signed_buffer(struct signature_check *sigc,
282
+ struct gpg_format *fmt, const char *payload,
283
+ size_t payload_size, const char *signature,
284
+ size_t signature_size)
285
{
286
struct child_process gpg = CHILD_PROCESS_INIT;
266
- struct gpg_format *fmt;
287
struct tempfile *temp;
288
int ret;
269
- struct strbuf buf = STRBUF_INIT;
289
+ struct strbuf gpg_stdout = STRBUF_INIT;
290
+ struct strbuf gpg_stderr = STRBUF_INIT;
291
292
temp = mks_tempfile_t(".git_vtag_tmpXXXXXX");
293
if (!temp)
@@ -279,10 +300,6 @@ static int verify_signed_buffer(const char *payload, size_t payload_size,
300
return -1;
301
}
302
282
- fmt = get_format_by_sig(signature);
283
- if (!fmt)
284
- BUG("bad signature '%s'", signature);
285
-
303
strvec_push(&gpg.args, fmt->program);
304
strvec_pushv(&gpg.args, fmt->verify_args);
305
strvec_pushl(&gpg.args,
@@ -290,18 +307,22 @@ static int verify_signed_buffer(const char *payload, size_t payload_size,
307
"--verify", temp->filename.buf, "-",
308
NULL);
309
293
- if (!gpg_status)
294
- gpg_status = &buf;
295
-
310
sigchain_push(SIGPIPE, SIG_IGN);
297
- ret = pipe_command(&gpg, payload, payload_size,
298
- gpg_status, 0, gpg_output, 0);
311
+ ret = pipe_command(&gpg, payload, payload_size, &gpg_stdout, 0,
312
+ &gpg_stderr, 0);
313
sigchain_pop(SIGPIPE);
314
315
delete_tempfile(&temp);
316
303
- ret |= !strstr(gpg_status->buf, "\n[GNUPG:] GOODSIG ");
304
- strbuf_release(&buf); /* no matter it was used or not */
317
+ ret |= !strstr(gpg_stdout.buf, "\n[GNUPG:] GOODSIG ");
318
+ sigc->payload = xmemdupz(payload, payload_size);
319
+ sigc->output = strbuf_detach(&gpg_stderr, NULL);
320
+ sigc->gpg_status = strbuf_detach(&gpg_stdout, NULL);
321
+
322
+ parse_gpg_output(sigc);
323
+
324
+ strbuf_release(&gpg_stdout);
325
+ strbuf_release(&gpg_stderr);
326
327
return ret;
328
}
@@ -309,35 +330,32 @@ static int verify_signed_buffer(const char *payload, size_t payload_size,
330
int check_signature(const char *payload, size_t plen, const char *signature,
331
size_t slen, struct signature_check *sigc)
332
{
312
- struct strbuf gpg_output = STRBUF_INIT;
313
- struct strbuf gpg_status = STRBUF_INIT;
333
+ struct gpg_format *fmt;
334
int status;
335
336
sigc->result = 'N';
337
sigc->trust_level = -1;
338
319
- status = verify_signed_buffer(payload, plen, signature, slen,
320
- &gpg_output, &gpg_status);
321
- if (status && !gpg_output.len)
322
- goto out;
323
- sigc->payload = xmemdupz(payload, plen);
324
- sigc->gpg_output = strbuf_detach(&gpg_output, NULL);
325
- sigc->gpg_status = strbuf_detach(&gpg_status, NULL);
326
- parse_gpg_output(sigc);
339
+ fmt = get_format_by_sig(signature);
340
+ if (!fmt)
341
+ die(_("bad/incompatible signature '%s'"), signature);
342
+
343
+ status = fmt->verify_signed_buffer(sigc, fmt, payload, plen, signature,
344
+ slen);
345
+
346
+ if (status && !sigc->output)
347
+ return !!status;
348
+
349
status |= sigc->result != 'G';
350
status |= sigc->trust_level < configured_min_trust_level;
351
330
- out:
331
- strbuf_release(&gpg_status);
332
- strbuf_release(&gpg_output);
333
-
352
return !!status;
353
}
354
355
void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
356
{
339
- const char *output = flags & GPG_VERIFY_RAW ?
340
- sigc->gpg_status : sigc->gpg_output;
357
+ const char *output = flags & GPG_VERIFY_RAW ? sigc->gpg_status :
358
+ sigc->output;
359
360
if (flags & GPG_VERIFY_VERBOSE && sigc->payload)
361
fputs(sigc->payload, stdout);
@@ -441,6 +459,12 @@ const char *get_signing_key(void)
459
}
460
461
int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
462
+{
463
+ return use_format->sign_buffer(buffer, signature, signing_key);
464
+}
465
+
466
+static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
467
+ const char *signing_key)
468
{
469
struct child_process gpg = CHILD_PROCESS_INIT;
470
int ret;
gpg-interface.h
+1
-1
@@ -17,7 +17,7 @@ enum signature_trust_level {
17
18
struct signature_check {
19
char *payload;
20
- char *gpg_output;
20
+ char *output;
21
char *gpg_status;
22
23
/*
log-tree.c
+4
-4
@@ -513,10 +513,10 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
513
514
status = check_signature(payload.buf, payload.len, signature.buf,
515
signature.len, &sigc);
516
- if (status && !sigc.gpg_output)
516
+ if (status && !sigc.output)
517
show_sig_lines(opt, status, "No signature\n");
518
else
519
- show_sig_lines(opt, status, sigc.gpg_output);
519
+ show_sig_lines(opt, status, sigc.output);
520
signature_check_clear(&sigc);
521
522
out:
@@ -583,8 +583,8 @@ static int show_one_mergetag(struct commit *commit,
583
/* could have a good signature */
584
status = check_signature(payload.buf, payload.len,
585
signature.buf, signature.len, &sigc);
586
- if (sigc.gpg_output)
587
- strbuf_addstr(&verify_message, sigc.gpg_output);
586
+ if (sigc.output)
587
+ strbuf_addstr(&verify_message, sigc.output);
588
else
589
strbuf_addstr(&verify_message, "No signature\n");
590
signature_check_clear(&sigc);
pretty.c
+2
-2
@@ -1432,8 +1432,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
1432
check_commit_signature(c->commit, &(c->signature_check));
1433
switch (placeholder[1]) {
1434
case 'G':
1435
- if (c->signature_check.gpg_output)
1436
- strbuf_addstr(sb, c->signature_check.gpg_output);
1435
+ if (c->signature_check.output)
1436
+ strbuf_addstr(sb, c->signature_check.output);
1437
break;
1438
case '?':
1439
switch (c->signature_check.result) {