ssh signing: make verify-commit consider key lifetime
If valid-before/after dates are configured for this signatures key in the allowedSigners file then the verification should check if the key was valid at the time the commit was made. This allows for graceful key rollover and revoking keys without invalidating all previous commits. This feature needs openssh > 8.8. Older ssh-keygen versions will simply ignore this flag and use the current time. Strictly speaking this feature is available in 8.7, but since 8.7 has a bug that makes it unusable in another needed call we require 8.8. Timestamp information is present on most invocations of check_signature. However signer ident is not. We will need the signer email / name to be able to implement "Trust on first use" functionality later. Since the payload contains all necessary information we can parse it from there. The caller only needs to provide us some info about the payload by setting payload_type in the signature_check struct. - Add payload_type field & enum and payload_timestamp to struct signature_check - Populate the timestamp when not already set if we know about the payload type - Pass -Overify-time={payload_timestamp} in the users timezone to all ssh-keygen verification calls - Set the payload type when verifying commits - Add tests for expired, not yet valid and keys having a commit date outside of key validity as well as within 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
6393c956f4e7061d6b19981bd8cd28ef037b911e
5 files changed
+110
Documentation/config/gpg.txt
+5
@@ -64,6 +64,11 @@ A repository that only allows signed commits can store the file
64
in the repository itself using a path relative to the top-level of the working tree.
65
This way only committers with an already valid key can add or change keys in the keyring.
66
+
67
+Since OpensSSH 8.8 this file allows specifying a key lifetime using valid-after &
68
+valid-before options. Git will mark signatures as valid if the signing key was
69
+valid at the time of the signatures creation. This allows users to change a
70
+signing key without invalidating all previously made signatures.
71
++
72
Using a SSH CA key with the cert-authority option
73
(see ssh-keygen(1) "CERTIFICATES") is also valid.
74
commit.c
+1
@@ -1213,6 +1213,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
1213
if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
1214
goto out;
1215
1216
+ sigc->payload_type = SIGNATURE_PAYLOAD_COMMIT;
1217
sigc->payload = strbuf_detach(&payload, &sigc->payload_len);
1218
ret = check_signature(sigc, signature.buf, signature.len);
1219
gpg-interface.c
+53
@@ -439,6 +439,13 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
439
struct strbuf ssh_principals_err = STRBUF_INIT;
440
struct strbuf ssh_keygen_out = STRBUF_INIT;
441
struct strbuf ssh_keygen_err = STRBUF_INIT;
442
+ struct strbuf verify_time = STRBUF_INIT;
443
+ const struct date_mode verify_date_mode = {
444
+ .type = DATE_STRFTIME,
445
+ .strftime_fmt = "%Y%m%d%H%M%S",
446
+ /* SSH signing key validity has no timezone information - Use the local timezone */
447
+ .local = 1,
448
+ };
449
450
if (!ssh_allowed_signers) {
451
error(_("gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification"));
@@ -456,11 +463,16 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
463
return -1;
464
}
465
466
+ if (sigc->payload_timestamp)
467
+ strbuf_addf(&verify_time, "-Overify-time=%s",
468
+ show_date(sigc->payload_timestamp, 0, &verify_date_mode));
469
+
470
/* Find the principal from the signers */
471
strvec_pushl(&ssh_keygen.args, fmt->program,
472
"-Y", "find-principals",
473
"-f", ssh_allowed_signers,
474
"-s", buffer_file->filename.buf,
475
+ verify_time.buf,
476
NULL);
477
ret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,
478
&ssh_principals_err, 0);
@@ -478,6 +490,7 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
490
"-Y", "check-novalidate",
491
"-n", "git",
492
"-s", buffer_file->filename.buf,
493
+ verify_time.buf,
494
NULL);
495
pipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,
496
&ssh_keygen_out, 0, &ssh_keygen_err, 0);
@@ -512,6 +525,7 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
525
"-f", ssh_allowed_signers,
526
"-I", principal,
527
"-s", buffer_file->filename.buf,
528
+ verify_time.buf,
529
NULL);
530
531
if (ssh_revocation_file) {
@@ -556,10 +570,46 @@ out:
570
strbuf_release(&ssh_principals_err);
571
strbuf_release(&ssh_keygen_out);
572
strbuf_release(&ssh_keygen_err);
573
+ strbuf_release(&verify_time);
574
575
return ret;
576
}
577
578
+static int parse_payload_metadata(struct signature_check *sigc)
579
+{
580
+ const char *ident_line = NULL;
581
+ size_t ident_len;
582
+ struct ident_split ident;
583
+ const char *signer_header;
584
+
585
+ switch (sigc->payload_type) {
586
+ case SIGNATURE_PAYLOAD_COMMIT:
587
+ signer_header = "committer";
588
+ break;
589
+ case SIGNATURE_PAYLOAD_TAG:
590
+ signer_header = "tagger";
591
+ break;
592
+ case SIGNATURE_PAYLOAD_UNDEFINED:
593
+ case SIGNATURE_PAYLOAD_PUSH_CERT:
594
+ /* Ignore payloads we don't want to parse */
595
+ return 0;
596
+ default:
597
+ BUG("invalid value for sigc->payload_type");
598
+ }
599
+
600
+ ident_line = find_commit_header(sigc->payload, signer_header, &ident_len);
601
+ if (!ident_line || !ident_len)
602
+ return 1;
603
+
604
+ if (split_ident_line(&ident, ident_line, ident_len))
605
+ return 1;
606
+
607
+ if (!sigc->payload_timestamp && ident.date_begin && ident.date_end)
608
+ sigc->payload_timestamp = parse_timestamp(ident.date_begin, NULL, 10);
609
+
610
+ return 0;
611
+}
612
+
613
int check_signature(struct signature_check *sigc,
614
const char *signature, size_t slen)
615
{
@@ -573,6 +623,9 @@ int check_signature(struct signature_check *sigc,
623
if (!fmt)
624
die(_("bad/incompatible signature '%s'"), signature);
625
626
+ if (parse_payload_metadata(sigc))
627
+ return 1;
628
+
629
status = fmt->verify_signed_buffer(sigc, fmt, signature, slen);
630
631
if (status && !sigc->output)
gpg-interface.h
+9
@@ -15,9 +15,18 @@ enum signature_trust_level {
15
TRUST_ULTIMATE,
16
};
17
18
+enum payload_type {
19
+ SIGNATURE_PAYLOAD_UNDEFINED,
20
+ SIGNATURE_PAYLOAD_COMMIT,
21
+ SIGNATURE_PAYLOAD_TAG,
22
+ SIGNATURE_PAYLOAD_PUSH_CERT,
23
+};
24
+
25
struct signature_check {
26
char *payload;
27
size_t payload_len;
28
+ enum payload_type payload_type;
29
+ timestamp_t payload_timestamp;
30
char *output;
31
char *gpg_status;
32
t/t7528-signed-commit-ssh.sh
+42
@@ -76,6 +76,23 @@ test_expect_success GPGSSH 'create signed commits' '
76
git tag twelfth-signed-alt $(cat oid)
77
'
78
79
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '
80
+ test_when_finished "test_unconfig commit.gpgsign" &&
81
+ test_config gpg.format ssh &&
82
+
83
+ echo expired >file && test_tick && git commit -a -m expired -S"${GPGSSH_KEY_EXPIRED}" &&
84
+ git tag expired-signed &&
85
+
86
+ echo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S"${GPGSSH_KEY_NOTYETVALID}" &&
87
+ git tag notyetvalid-signed &&
88
+
89
+ echo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S"${GPGSSH_KEY_TIMEBOXEDVALID}" &&
90
+ git tag timeboxedvalid-signed &&
91
+
92
+ echo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S"${GPGSSH_KEY_TIMEBOXEDINVALID}" &&
93
+ git tag timeboxedinvalid-signed
94
+'
95
+
96
test_expect_success GPGSSH 'verify and show signatures' '
97
test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
98
test_config gpg.mintrustlevel UNDEFINED &&
@@ -122,6 +139,31 @@ test_expect_success GPGSSH 'verify-commit exits failure on untrusted signature'
139
grep "${GPGSSH_KEY_NOT_TRUSTED}" actual
140
'
141
142
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure on expired signature key' '
143
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
144
+ test_must_fail git verify-commit expired-signed 2>actual &&
145
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
146
+'
147
+
148
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure on not yet valid signature key' '
149
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
150
+ test_must_fail git verify-commit notyetvalid-signed 2>actual &&
151
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
152
+'
153
+
154
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit succeeds with commit date and key validity matching' '
155
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
156
+ git verify-commit timeboxedvalid-signed 2>actual &&
157
+ grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
158
+ ! grep "${GPGSSH_BAD_SIGNATURE}" actual
159
+'
160
+
161
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure with commit date outside of key validity' '
162
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
163
+ test_must_fail git verify-commit timeboxedinvalid-signed 2>actual &&
164
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
165
+'
166
+
167
test_expect_success GPGSSH 'verify-commit exits success with matching minTrustLevel' '
168
test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
169
test_config gpg.minTrustLevel fully &&