* [RFC PATCH 1/6] ssh signing: extend check_signature to accept payload metadata
2021-10-15 13:15 [RFC PATCH 0/6] ssh signing: verify key lifetime Fabian Stelzer
@ 2021-10-15 13:15 ` Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 2/6] ssh signing: add key lifetime test prereqs Fabian Stelzer
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Fabian Stelzer @ 2021-10-15 13:15 UTC (permalink / raw)
To: git; +Cc: Fabian Stelzer
Adds two new parameters to the check_signature api and passes them to the
internal verification ssh/gpg methods.
A payload timestamp that will be used to verify signatures at the time of their
objects creation if the signing method supports it (only ssh for now).
And a signer strbuf containing ident information about the signer that
we will need for implementing "Trust on first use" in a future patch
series.
Adding the ident info right now instead of a later patch series makes
certain choices in this patch series clearer. Only passing the timestamp
could be done a bit simpler on some call sites but i am certain that we
will need the ident info for "Trust on first use" no matter how exactly
it will be implemented later.
To start with we pass 0, NULL on all invocations to keep the current behaviour
as is.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
builtin/receive-pack.c | 5 +++--
commit.c | 2 +-
fmt-merge-msg.c | 4 ++--
gpg-interface.c | 35 +++++++++++++++++++++--------------
gpg-interface.h | 1 +
log-tree.c | 4 ++--
tag.c | 2 +-
7 files changed, 31 insertions(+), 22 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 49b846d960..761c70642b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -769,8 +769,9 @@ static void prepare_push_cert_sha1(struct child_process *proc)
memset(&sigcheck, '\0', sizeof(sigcheck));
bogs = parse_signed_buffer(push_cert.buf, push_cert.len);
- check_signature(push_cert.buf, bogs, push_cert.buf + bogs,
- push_cert.len - bogs, &sigcheck);
+ check_signature(push_cert.buf, bogs, 0, NULL,
+ push_cert.buf + bogs, push_cert.len - bogs,
+ &sigcheck);
nonce_status = check_nonce(push_cert.buf, bogs);
}
diff --git a/commit.c b/commit.c
index 551de4903c..1704d9df0a 100644
--- a/commit.c
+++ b/commit.c
@@ -1212,7 +1212,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
goto out;
- ret = check_signature(payload.buf, payload.len, signature.buf,
+ ret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,
signature.len, sigc);
out:
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 5216191488..d2cedad6b7 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -533,8 +533,8 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
else {
buf = payload.buf;
len = payload.len;
- if (check_signature(payload.buf, payload.len, sig.buf,
- sig.len, &sigc) &&
+ if (check_signature(payload.buf, payload.len, 0, NULL,
+ sig.buf, sig.len, &sigc) &&
!sigc.output)
strbuf_addstr(&sig, "gpg verification failed.\n");
else
diff --git a/gpg-interface.c b/gpg-interface.c
index 433482307c..5960202615 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -20,8 +20,9 @@ struct gpg_format {
const char **sigs;
int (*verify_signed_buffer)(struct signature_check *sigc,
struct gpg_format *fmt, const char *payload,
- size_t payload_size, const char *signature,
- size_t signature_size);
+ size_t payload_size, timestamp_t payload_timestamp,
+ struct strbuf *payload_signer,
+ const char *signature, size_t signature_size);
int (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature,
const char *signing_key);
const char *(*get_default_key)(void);
@@ -54,12 +55,14 @@ static const char *ssh_sigs[] = {
static int verify_gpg_signed_buffer(struct signature_check *sigc,
struct gpg_format *fmt, const char *payload,
- size_t payload_size, const char *signature,
- size_t signature_size);
+ size_t payload_size, timestamp_t payload_timestamp,
+ struct strbuf *payload_signer,
+ const char *signature, size_t signature_size);
static int verify_ssh_signed_buffer(struct signature_check *sigc,
struct gpg_format *fmt, const char *payload,
- size_t payload_size, const char *signature,
- size_t signature_size);
+ size_t payload_size, timestamp_t payload_timestamp,
+ struct strbuf *payload_signer,
+ const char *signature, size_t signature_size);
static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
const char *signing_key);
static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
@@ -315,8 +318,9 @@ static void parse_gpg_output(struct signature_check *sigc)
static int verify_gpg_signed_buffer(struct signature_check *sigc,
struct gpg_format *fmt, const char *payload,
- size_t payload_size, const char *signature,
- size_t signature_size)
+ size_t payload_size, timestamp_t payload_timestamp,
+ struct strbuf *payload_signer,
+ const char *signature, size_t signature_size)
{
struct child_process gpg = CHILD_PROCESS_INIT;
struct tempfile *temp;
@@ -421,8 +425,9 @@ static void parse_ssh_output(struct signature_check *sigc)
static int verify_ssh_signed_buffer(struct signature_check *sigc,
struct gpg_format *fmt, const char *payload,
- size_t payload_size, const char *signature,
- size_t signature_size)
+ size_t payload_size, timestamp_t payload_timestamp,
+ struct strbuf *payload_signer,
+ const char *signature, size_t signature_size)
{
struct child_process ssh_keygen = CHILD_PROCESS_INIT;
struct tempfile *buffer_file;
@@ -556,8 +561,9 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
return ret;
}
-int check_signature(const char *payload, size_t plen, const char *signature,
- size_t slen, struct signature_check *sigc)
+int check_signature(const char *payload, size_t plen, timestamp_t payload_timestamp,
+ struct strbuf *payload_signer, const char *signature,
+ size_t slen, struct signature_check *sigc)
{
struct gpg_format *fmt;
int status;
@@ -569,8 +575,9 @@ int check_signature(const char *payload, size_t plen, const char *signature,
if (!fmt)
die(_("bad/incompatible signature '%s'"), signature);
- status = fmt->verify_signed_buffer(sigc, fmt, payload, plen, signature,
- slen);
+ status = fmt->verify_signed_buffer(sigc, fmt, payload, plen,
+ payload_timestamp, payload_signer,
+ signature, slen);
if (status && !sigc->output)
return !!status;
diff --git a/gpg-interface.h b/gpg-interface.h
index beefacbb1e..a326845be8 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -71,6 +71,7 @@ const char *get_signing_key(void);
*/
const char *get_signing_key_id(void);
int check_signature(const char *payload, size_t plen,
+ timestamp_t payload_timestamp, struct strbuf *payload_signer,
const char *signature, size_t slen,
struct signature_check *sigc);
void print_signature_buffer(const struct signature_check *sigc,
diff --git a/log-tree.c b/log-tree.c
index 644893fd8c..3c3aec5c40 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -513,7 +513,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
goto out;
- status = check_signature(payload.buf, payload.len, signature.buf,
+ status = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,
signature.len, &sigc);
if (status && !sigc.output)
show_sig_lines(opt, status, "No signature\n");
@@ -583,7 +583,7 @@ static int show_one_mergetag(struct commit *commit,
status = -1;
if (parse_signature(extra->value, extra->len, &payload, &signature)) {
/* could have a good signature */
- status = check_signature(payload.buf, payload.len,
+ status = check_signature(payload.buf, payload.len, 0, NULL,
signature.buf, signature.len, &sigc);
if (sigc.output)
strbuf_addstr(&verify_message, sigc.output);
diff --git a/tag.c b/tag.c
index 3e18a41841..3459a0867c 100644
--- a/tag.c
+++ b/tag.c
@@ -25,7 +25,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
return error("no signature found");
}
- ret = check_signature(payload.buf, payload.len, signature.buf,
+ ret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,
signature.len, &sigc);
if (!(flags & GPG_VERIFY_OMIT_STATUS))
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC PATCH 2/6] ssh signing: add key lifetime test prereqs
2021-10-15 13:15 [RFC PATCH 0/6] ssh signing: verify key lifetime Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 1/6] ssh signing: extend check_signature to accept payload metadata Fabian Stelzer
@ 2021-10-15 13:15 ` Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 3/6] ssh signing: verify-commit/check_signature with commit date Fabian Stelzer
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Fabian Stelzer @ 2021-10-15 13:15 UTC (permalink / raw)
To: git; +Cc: Fabian Stelzer
if ssh-keygen supports -Overify-time, add test keys marked as expired,
not yet valid and valid both within the test_tick timeframe and outside of it.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
t/lib-gpg.sh | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index f99ef3e859..139e7a45ea 100644
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -90,6 +90,10 @@ test_lazy_prereq RFC1991 '
GPGSSH_KEY_PRIMARY="${GNUPGHOME}/ed25519_ssh_signing_key"
GPGSSH_KEY_SECONDARY="${GNUPGHOME}/rsa_2048_ssh_signing_key"
GPGSSH_KEY_UNTRUSTED="${GNUPGHOME}/untrusted_ssh_signing_key"
+GPGSSH_KEY_EXPIRED="${GNUPGHOME}/expired_ssh_signing_key"
+GPGSSH_KEY_NOTYETVALID="${GNUPGHOME}/notyetvalid_ssh_signing_key"
+GPGSSH_KEY_TIMEBOXEDVALID="${GNUPGHOME}/timeboxed_valid_ssh_signing_key"
+GPGSSH_KEY_TIMEBOXEDINVALID="${GNUPGHOME}/timeboxed_invalid_ssh_signing_key"
GPGSSH_KEY_WITH_PASSPHRASE="${GNUPGHOME}/protected_ssh_signing_key"
GPGSSH_KEY_PASSPHRASE="super_secret"
GPGSSH_ALLOWED_SIGNERS="${GNUPGHOME}/ssh.all_valid.allowedSignersFile"
@@ -112,7 +116,20 @@ test_lazy_prereq GPGSSH '
echo "\"principal with number 2\" $(cat "${GPGSSH_KEY_SECONDARY}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
ssh-keygen -t ed25519 -N "${GPGSSH_KEY_PASSPHRASE}" -C "git ed25519 encrypted key" -f "${GPGSSH_KEY_WITH_PASSPHRASE}" >/dev/null &&
echo "\"principal with number 3\" $(cat "${GPGSSH_KEY_WITH_PASSPHRASE}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
- ssh-keygen -t ed25519 -N "" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null
+ ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null
+'
+
+test_lazy_prereq GPGSSH_VERIFYTIME '
+ # Check if ssh-keygen has a verify-time option by passing an invalid date to it
+ ssh-keygen -Overify-time=INVALID -Y check-novalidate -s doesnotmatter 2>&1 | grep -q -F "Invalid \"verify-time\"" &&
+ ssh-keygen -t ed25519 -N "" -C "timeboxed valid key" -f "${GPGSSH_KEY_TIMEBOXEDVALID}" >/dev/null &&
+ echo "\"timeboxed valid key\" valid-after=\"20050407000000\",valid-before=\"200504100000\" $(cat "${GPGSSH_KEY_TIMEBOXEDVALID}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
+ ssh-keygen -t ed25519 -N "" -C "timeboxed invalid key" -f "${GPGSSH_KEY_TIMEBOXEDINVALID}" >/dev/null &&
+ echo "\"timeboxed invalid key\" valid-after=\"20050401000000\",valid-before=\"20050402000000\" $(cat "${GPGSSH_KEY_TIMEBOXEDINVALID}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
+ ssh-keygen -t ed25519 -N "" -C "expired key" -f "${GPGSSH_KEY_EXPIRED}" >/dev/null &&
+ echo "\"principal with expired key\" valid-before=\"20000101000000\" $(cat "${GPGSSH_KEY_EXPIRED}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
+ ssh-keygen -t ed25519 -N "" -C "not yet valid key" -f "${GPGSSH_KEY_NOTYETVALID}" >/dev/null &&
+ echo "\"principal with not yet valid key\" valid-after=\"29990101000000\" $(cat "${GPGSSH_KEY_NOTYETVALID}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}"
'
sanitize_pgp() {
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC PATCH 3/6] ssh signing: verify-commit/check_signature with commit date
2021-10-15 13:15 [RFC PATCH 0/6] ssh signing: verify key lifetime Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 1/6] ssh signing: extend check_signature to accept payload metadata Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 2/6] ssh signing: add key lifetime test prereqs Fabian Stelzer
@ 2021-10-15 13:15 ` Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 4/6] ssh signing: git log/check_signature " Fabian Stelzer
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Fabian Stelzer @ 2021-10-15 13:15 UTC (permalink / raw)
To: git; +Cc: Fabian Stelzer
If valid-before/after dates are configured for this signatures key in the
allowedSigners file then the verification will 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.
Therefore we provide parse_signed_buffer_metadata() for parsing both from
the payload buffer.
Passes commit date for verify-commit on to check_signature.
Adds test for keys being expired, not yet valid, having a commit date outside
of key validity and within.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
Documentation/config/gpg.txt | 5 ++++
commit.c | 12 ++++++++--
gpg-interface.c | 45 ++++++++++++++++++++++++++++++++++++
gpg-interface.h | 8 +++++++
t/t7528-signed-commit-ssh.sh | 42 +++++++++++++++++++++++++++++++++
5 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt
index 4f30c7dbdd..c9be554c73 100644
--- a/Documentation/config/gpg.txt
+++ b/Documentation/config/gpg.txt
@@ -64,6 +64,11 @@ A repository that only allows signed commits can store the file
in the repository itself using a path relative to the top-level of the working tree.
This way only committers with an already valid key can add or change keys in the keyring.
+
+Since OpensSSH 8.8 this file allows specifying a key lifetime using valid-after &
+valid-before options. Git will mark signatures as valid if the signing key was
+valid at the time of the signatures creation. This allows users to change a
+signing key without invalidating all previously made signatures.
++
Using a SSH CA key with the cert-authority option
(see ssh-keygen(1) "CERTIFICATES") is also valid.
diff --git a/commit.c b/commit.c
index 1704d9df0a..f70b233e00 100644
--- a/commit.c
+++ b/commit.c
@@ -1207,17 +1207,25 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
struct strbuf payload = STRBUF_INIT;
struct strbuf signature = STRBUF_INIT;
int ret = 1;
+ struct strbuf payload_signer = STRBUF_INIT;
+ timestamp_t payload_timestamp = 0;
sigc->result = 'N';
if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
goto out;
- ret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,
- signature.len, sigc);
+ if (parse_signed_buffer_metadata(payload.buf, "committer",
+ &payload_timestamp, &payload_signer))
+ goto out;
+
+ ret = check_signature(payload.buf, payload.len, payload_timestamp,
+ &payload_signer, signature.buf,
+ signature.len, sigc);
out:
strbuf_release(&payload);
strbuf_release(&signature);
+ strbuf_release(&payload_signer);
return ret;
}
diff --git a/gpg-interface.c b/gpg-interface.c
index 5960202615..a51d8e361c 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -439,6 +439,13 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
struct strbuf ssh_principals_err = STRBUF_INIT;
struct strbuf ssh_keygen_out = STRBUF_INIT;
struct strbuf ssh_keygen_err = STRBUF_INIT;
+ struct strbuf verify_time = STRBUF_INIT;
+ const struct date_mode verify_date_mode = {
+ .type = DATE_STRFTIME,
+ .strftime_fmt = "%Y%m%d%H%M%S",
+ /* SSH signing key validity has no timezone information - Use the local timezone */
+ .local = 1,
+ };
if (!ssh_allowed_signers) {
error(_("gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification"));
@@ -456,18 +463,27 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
return -1;
}
+ if (payload_timestamp != 0) {
+ strbuf_addf(&verify_time, "-Overify-time=%s",
+ show_date(payload_timestamp, 0, &verify_date_mode));
+ }
+
/* Find the principal from the signers */
strvec_pushl(&ssh_keygen.args, fmt->program,
"-Y", "find-principals",
"-f", ssh_allowed_signers,
"-s", buffer_file->filename.buf,
+ verify_time.buf,
NULL);
+
ret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,
&ssh_principals_err, 0);
+
if (ret && strstr(ssh_principals_err.buf, "usage:")) {
error(_("ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)"));
goto out;
}
+
if (ret || !ssh_principals_out.len) {
/*
* We did not find a matching principal in the allowedSigners
@@ -478,7 +494,9 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
"-Y", "check-novalidate",
"-n", "git",
"-s", buffer_file->filename.buf,
+ verify_time.buf,
NULL);
+
pipe_command(&ssh_keygen, payload, payload_size,
&ssh_keygen_out, 0, &ssh_keygen_err, 0);
@@ -512,6 +530,7 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
"-f", ssh_allowed_signers,
"-I", principal,
"-s", buffer_file->filename.buf,
+ verify_time.buf,
NULL);
if (ssh_revocation_file) {
@@ -557,10 +576,36 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
strbuf_release(&ssh_principals_err);
strbuf_release(&ssh_keygen_out);
strbuf_release(&ssh_keygen_err);
+ strbuf_release(&verify_time);
return ret;
}
+int parse_signed_buffer_metadata(const char *payload, const char *signer_header,
+ timestamp_t *payload_timestamp,
+ struct strbuf *payload_signer)
+{
+ const char *ident_line = NULL;
+ size_t ident_len;
+ struct ident_split ident;
+
+ ident_line = find_commit_header(payload, signer_header, &ident_len);
+ if (ident_line && ident_len) {
+ if (!split_ident_line(&ident, ident_line, ident_len)) {
+ if (payload_timestamp && ident.date_begin && ident.date_end)
+ *payload_timestamp = parse_timestamp(
+ ident.date_begin, NULL, 10);
+ if (payload_signer)
+ strbuf_add(payload_signer, ident.mail_begin,
+ (ident.mail_end - ident.mail_begin));
+
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
int check_signature(const char *payload, size_t plen, timestamp_t payload_timestamp,
struct strbuf *payload_signer, const char *signature,
size_t slen, struct signature_check *sigc)
diff --git a/gpg-interface.h b/gpg-interface.h
index a326845be8..27a34d06e8 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -65,6 +65,14 @@ int git_gpg_config(const char *, const char *, void *);
void set_signing_key(const char *);
const char *get_signing_key(void);
+/*
+ * Tries to find "signer_header" (usually committer or tagger) in the payload,
+ * and sets the corresponding timestamp and signer parameters if found.
+ */
+int parse_signed_buffer_metadata(const char *payload, const char *signer_header,
+ timestamp_t *payload_timestamp,
+ struct strbuf *payload_signer);
+
/*
* Returns a textual unique representation of the signing key in use
* Either a GPG KeyID or a SSH Key Fingerprint
diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh
index badf3ed320..dae76ded0c 100755
--- a/t/t7528-signed-commit-ssh.sh
+++ b/t/t7528-signed-commit-ssh.sh
@@ -76,6 +76,23 @@ test_expect_success GPGSSH 'create signed commits' '
git tag twelfth-signed-alt $(cat oid)
'
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '
+ test_when_finished "test_unconfig commit.gpgsign" &&
+ test_config gpg.format ssh &&
+
+ echo expired >file && test_tick && git commit -a -m expired -S"${GPGSSH_KEY_EXPIRED}" &&
+ git tag expired-signed &&
+
+ echo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S"${GPGSSH_KEY_NOTYETVALID}" &&
+ git tag notyetvalid-signed &&
+
+ echo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S"${GPGSSH_KEY_TIMEBOXEDVALID}" &&
+ git tag timeboxedvalid-signed &&
+
+ echo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S"${GPGSSH_KEY_TIMEBOXEDINVALID}" &&
+ git tag timeboxedinvalid-signed
+'
+
test_expect_success GPGSSH 'verify and show signatures' '
test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
test_config gpg.mintrustlevel UNDEFINED &&
@@ -122,6 +139,31 @@ test_expect_success GPGSSH 'verify-commit exits failure on untrusted signature'
grep "${GPGSSH_KEY_NOT_TRUSTED}" actual
'
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure on expired signature key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ test_must_fail git verify-commit expired-signed 2>actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure on not yet valid signature key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ test_must_fail git verify-commit notyetvalid-signed 2>actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit succeeds with commit date and key validity matching' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git verify-commit timeboxedvalid-signed 2>actual &&
+ grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
+ ! grep "${GPGSSH_BAD_SIGNATURE}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure with commit date outside of key validity' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ test_must_fail git verify-commit timeboxedinvalid-signed 2>actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
test_expect_success GPGSSH 'verify-commit exits success with matching minTrustLevel' '
test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
test_config gpg.minTrustLevel fully &&
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC PATCH 4/6] ssh signing: git log/check_signature with commit date
2021-10-15 13:15 [RFC PATCH 0/6] ssh signing: verify key lifetime Fabian Stelzer
` (2 preceding siblings ...)
2021-10-15 13:15 ` [RFC PATCH 3/6] ssh signing: verify-commit/check_signature with commit date Fabian Stelzer
@ 2021-10-15 13:15 ` Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 5/6] ssh signing: verify-tag/check_signature with tag date Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 6/6] ssh signing: fmt-merge-msg/check_signature " Fabian Stelzer
5 siblings, 0 replies; 7+ messages in thread
From: Fabian Stelzer @ 2021-10-15 13:15 UTC (permalink / raw)
To: git; +Cc: Fabian Stelzer
Pass the commit date and ident to check_signature when calling git log.
Implements the same tests as for verify-commit.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
log-tree.c | 23 +++++++++++++++++++----
t/t4202-log.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 3c3aec5c40..2fd7f0c398 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -509,12 +509,18 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
struct strbuf signature = STRBUF_INIT;
struct signature_check sigc = { 0 };
int status;
+ struct strbuf payload_signer = STRBUF_INIT;
+ timestamp_t payload_timestamp = 0;
if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
goto out;
- status = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,
- signature.len, &sigc);
+ if (parse_signed_buffer_metadata(payload.buf, "committer", &payload_timestamp, &payload_signer))
+ goto out;
+
+ status = check_signature(payload.buf, payload.len, payload_timestamp,
+ &payload_signer, signature.buf, signature.len,
+ &sigc);
if (status && !sigc.output)
show_sig_lines(opt, status, "No signature\n");
else
@@ -524,6 +530,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
out:
strbuf_release(&payload);
strbuf_release(&signature);
+ strbuf_release(&payload_signer);
}
static int which_parent(const struct object_id *oid, const struct commit *commit)
@@ -558,6 +565,8 @@ static int show_one_mergetag(struct commit *commit,
int status, nth;
struct strbuf payload = STRBUF_INIT;
struct strbuf signature = STRBUF_INIT;
+ struct strbuf payload_signer = STRBUF_INIT;
+ timestamp_t payload_timestamp = 0;
hash_object_file(the_hash_algo, extra->value, extra->len,
type_name(OBJ_TAG), &oid);
@@ -582,9 +591,14 @@ static int show_one_mergetag(struct commit *commit,
status = -1;
if (parse_signature(extra->value, extra->len, &payload, &signature)) {
+
+ if (parse_signed_buffer_metadata(payload.buf, "tagger", &payload_timestamp, &payload_signer))
+ strbuf_addstr(&verify_message, _("failed to parse timestamp and signer info from payload"));
+
/* could have a good signature */
- status = check_signature(payload.buf, payload.len, 0, NULL,
- signature.buf, signature.len, &sigc);
+ status = check_signature(payload.buf, payload.len, payload_timestamp,
+ &payload_signer, signature.buf,
+ signature.len, &sigc);
if (sigc.output)
strbuf_addstr(&verify_message, sigc.output);
else
@@ -597,6 +611,7 @@ static int show_one_mergetag(struct commit *commit,
strbuf_release(&verify_message);
strbuf_release(&payload);
strbuf_release(&signature);
+ strbuf_release(&payload_signer);
return 0;
}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6a650dacd6..2b12baab77 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1626,6 +1626,24 @@ test_expect_success GPGSSH 'setup sshkey signed branch' '
git commit -S -m signed_commit
'
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '
+ test_config gpg.format ssh &&
+ touch file &&
+ git add file &&
+
+ echo expired >file && test_tick && git commit -a -m expired -S"${GPGSSH_KEY_EXPIRED}" &&
+ git tag expired-signed &&
+
+ echo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S"${GPGSSH_KEY_NOTYETVALID}" &&
+ git tag notyetvalid-signed &&
+
+ echo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S"${GPGSSH_KEY_TIMEBOXEDVALID}" &&
+ git tag timeboxedvalid-signed &&
+
+ echo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S"${GPGSSH_KEY_TIMEBOXEDINVALID}" &&
+ git tag timeboxedinvalid-signed
+'
+
test_expect_success GPGSM 'log x509 fingerprint' '
echo "F8BF62E0693D0694816377099909C779FA23FD65 | " >expect &&
git log -n1 --format="%GF | %GP" signed-x509 >actual &&
@@ -1663,6 +1681,31 @@ test_expect_success GPGSSH 'log --graph --show-signature ssh' '
grep "${GOOD_SIGNATURE_TRUSTED}" actual
'
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log shows failure on expired signature key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git log --graph --show-signature -n1 expired-signed >actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log shows failure on not yet valid signature key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git log --graph --show-signature -n1 notyetvalid-signed >actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log show success with commit date and key validity matching' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git log --graph --show-signature -n1 timeboxedvalid-signed >actual &&
+ grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
+ ! grep "${GPGSSH_BAD_SIGNATURE}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log shows failure with commit date outside of key validity' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git log --graph --show-signature -n1 timeboxedinvalid-signed >actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
test_expect_success GPG 'log --graph --show-signature for merged tag' '
test_when_finished "git reset --hard && git checkout main" &&
git checkout -b plain main &&
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC PATCH 5/6] ssh signing: verify-tag/check_signature with tag date
2021-10-15 13:15 [RFC PATCH 0/6] ssh signing: verify key lifetime Fabian Stelzer
` (3 preceding siblings ...)
2021-10-15 13:15 ` [RFC PATCH 4/6] ssh signing: git log/check_signature " Fabian Stelzer
@ 2021-10-15 13:15 ` Fabian Stelzer
2021-10-15 13:15 ` [RFC PATCH 6/6] ssh signing: fmt-merge-msg/check_signature " Fabian Stelzer
5 siblings, 0 replies; 7+ messages in thread
From: Fabian Stelzer @ 2021-10-15 13:15 UTC (permalink / raw)
To: git; +Cc: Fabian Stelzer
Passes tag date and tagger ident to to check_signature.
Implements the same tests as for verify-commit.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
t/t7031-verify-tag-signed-ssh.sh | 42 ++++++++++++++++++++++++++++++++
tag.c | 12 +++++++--
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/t/t7031-verify-tag-signed-ssh.sh b/t/t7031-verify-tag-signed-ssh.sh
index 06c9dd6c93..1cb36b9ab8 100755
--- a/t/t7031-verify-tag-signed-ssh.sh
+++ b/t/t7031-verify-tag-signed-ssh.sh
@@ -48,6 +48,23 @@ test_expect_success GPGSSH 'create signed tags ssh' '
git tag -u"${GPGSSH_KEY_UNTRUSTED}" -m eighth eighth-signed-alt
'
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed tags with keys having defined lifetimes' '
+ test_when_finished "test_unconfig commit.gpgsign" &&
+ test_config gpg.format ssh &&
+
+ echo expired >file && test_tick && git commit -a -m expired -S"${GPGSSH_KEY_EXPIRED}" &&
+ git tag -s -u "${GPGSSH_KEY_EXPIRED}" -m expired-signed expired-signed &&
+
+ echo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S"${GPGSSH_KEY_NOTYETVALID}" &&
+ git tag -s -u "${GPGSSH_KEY_NOTYETVALID}" -m notyetvalid-signed notyetvalid-signed &&
+
+ echo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S"${GPGSSH_KEY_TIMEBOXEDVALID}" &&
+ git tag -s -u "${GPGSSH_KEY_TIMEBOXEDVALID}" -m timeboxedvalid-signed timeboxedvalid-signed &&
+
+ echo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S"${GPGSSH_KEY_TIMEBOXEDINVALID}" &&
+ git tag -s -u "${GPGSSH_KEY_TIMEBOXEDINVALID}" -m timeboxedinvalid-signed timeboxedinvalid-signed
+'
+
test_expect_success GPGSSH 'verify and show ssh signatures' '
test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
(
@@ -80,6 +97,31 @@ test_expect_success GPGSSH 'verify and show ssh signatures' '
)
'
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag exits failure on expired signature key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ test_must_fail git verify-tag expired-signed 2>actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag exits failure on not yet valid signature key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ test_must_fail git verify-tag notyetvalid-signed 2>actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag succeeds with tag date and key validity matching' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git verify-tag timeboxedvalid-signed 2>actual &&
+ grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
+ ! grep "${GPGSSH_BAD_SIGNATURE}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag failes with tag date outside of key validity' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ test_must_fail git verify-tag timeboxedinvalid-signed 2>actual &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
test_expect_success GPGSSH 'detect fudged ssh signature' '
test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
git cat-file tag seventh-signed >raw &&
diff --git a/tag.c b/tag.c
index 3459a0867c..1868e6b51d 100644
--- a/tag.c
+++ b/tag.c
@@ -16,6 +16,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
struct strbuf payload = STRBUF_INIT;
struct strbuf signature = STRBUF_INIT;
int ret;
+ struct strbuf payload_signer = STRBUF_INIT;
+ timestamp_t payload_timestamp = 0;
memset(&sigc, 0, sizeof(sigc));
@@ -25,8 +27,13 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
return error("no signature found");
}
- ret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,
- signature.len, &sigc);
+ if (parse_signed_buffer_metadata(payload.buf, "tagger", &payload_timestamp,
+ &payload_signer))
+ return error(_("failed to parse timestamp and signer info from payload"));
+
+ ret = check_signature(payload.buf, payload.len, payload_timestamp,
+ &payload_signer, signature.buf, signature.len,
+ &sigc);
if (!(flags & GPG_VERIFY_OMIT_STATUS))
print_signature_buffer(&sigc, flags);
@@ -34,6 +41,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
signature_check_clear(&sigc);
strbuf_release(&payload);
strbuf_release(&signature);
+ strbuf_release(&payload_signer);
return ret;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC PATCH 6/6] ssh signing: fmt-merge-msg/check_signature with tag date
2021-10-15 13:15 [RFC PATCH 0/6] ssh signing: verify key lifetime Fabian Stelzer
` (4 preceding siblings ...)
2021-10-15 13:15 ` [RFC PATCH 5/6] ssh signing: verify-tag/check_signature with tag date Fabian Stelzer
@ 2021-10-15 13:15 ` Fabian Stelzer
5 siblings, 0 replies; 7+ messages in thread
From: Fabian Stelzer @ 2021-10-15 13:15 UTC (permalink / raw)
To: git; +Cc: Fabian Stelzer
Pass the tag date and tagger ident to check_signature when generating merge
messages to verify merged tags signatures.
Implements the same tests as for verify-commit.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
fmt-merge-msg.c | 14 +++++++++--
t/t6200-fmt-merge-msg.sh | 54 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index d2cedad6b7..95b747e85a 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -524,6 +524,8 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
unsigned long len = size;
struct signature_check sigc = { NULL };
struct strbuf payload = STRBUF_INIT, sig = STRBUF_INIT;
+ struct strbuf payload_signer = STRBUF_INIT;
+ timestamp_t payload_timestamp = 0;
if (!buf || type != OBJ_TAG)
goto next;
@@ -533,9 +535,16 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
else {
buf = payload.buf;
len = payload.len;
- if (check_signature(payload.buf, payload.len, 0, NULL,
+
+ if (parse_signed_buffer_metadata(payload.buf, "tagger",
+ &payload_timestamp,
+ &payload_signer))
+ strbuf_addstr(&sig, _("failed to parse timestamp and signer info from payload"));
+
+ if (check_signature(payload.buf, payload.len,
+ payload_timestamp, &payload_signer,
sig.buf, sig.len, &sigc) &&
- !sigc.output)
+ !sigc.output)
strbuf_addstr(&sig, "gpg verification failed.\n");
else
strbuf_addstr(&sig, sigc.output);
@@ -564,6 +573,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
}
strbuf_release(&payload);
strbuf_release(&sig);
+ strbuf_release(&payload_signer);
next:
free(origbuf);
}
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 06c5fb5615..2dd2423643 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -91,6 +91,26 @@ test_expect_success GPGSSH 'created ssh signed commit and tag' '
git tag -s -u"${GPGSSH_KEY_UNTRUSTED}" -m signed-ssh-tag-msg-untrusted signed-untrusted-ssh-tag left
'
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed tags with keys having defined lifetimes' '
+ test_when_finished "test_unconfig commit.gpgsign" &&
+ test_config gpg.format ssh &&
+ git checkout -b signed-expiry-ssh &&
+ touch file &&
+ git add file &&
+
+ echo expired >file && test_tick && git commit -a -m expired -S"${GPGSSH_KEY_EXPIRED}" &&
+ git tag -s -u "${GPGSSH_KEY_EXPIRED}" -m expired-signed expired-signed &&
+
+ echo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S"${GPGSSH_KEY_NOTYETVALID}" &&
+ git tag -s -u "${GPGSSH_KEY_NOTYETVALID}" -m notyetvalid-signed notyetvalid-signed &&
+
+ echo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S"${GPGSSH_KEY_TIMEBOXEDVALID}" &&
+ git tag -s -u "${GPGSSH_KEY_TIMEBOXEDVALID}" -m timeboxedvalid-signed timeboxedvalid-signed &&
+
+ echo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S"${GPGSSH_KEY_TIMEBOXEDINVALID}" &&
+ git tag -s -u "${GPGSSH_KEY_TIMEBOXEDINVALID}" -m timeboxedinvalid-signed timeboxedinvalid-signed
+'
+
test_expect_success 'message for merging local branch' '
echo "Merge branch ${apos}left${apos}" >expected &&
@@ -137,6 +157,40 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh
! grep "${GPGSSH_BAD_SIGNATURE}" actual &&
grep "${GPGSSH_KEY_NOT_TRUSTED}" actual
'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by expired ssh key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git checkout main &&
+ git fetch . expired-signed &&
+ git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by not yet valid ssh key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git checkout main &&
+ git fetch . notyetvalid-signed &&
+ git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by valid timeboxed ssh key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git checkout main &&
+ git fetch . timeboxedvalid-signed &&
+ git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
+ grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
+ ! grep "${GPGSSH_BAD_SIGNATURE}" actual
+'
+
+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by invalid timeboxed ssh key' '
+ test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+ git checkout main &&
+ git fetch . timeboxedinvalid-signed &&
+ git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
+ ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
+'
+
test_expect_success 'message for merging external branch' '
echo "Merge branch ${apos}left${apos} of $(pwd)" >expected &&
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread