git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 3/7] ssh signing: make verify-commit consider key lifetime

From
Fabian Stelzer <fs@gigacodes.de>
Date
Nov 17, 2021, 09:35 UTC
Message-ID
<20211117093529.13953-4-fs@gigacodes.de>
In-Reply-To
<20211117093529.13953-1-fs@gigacodes.de>

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>
---
 Documentation/config/gpg.txt |  5 ++++
 commit.c                     |  1 +
 gpg-interface.c              | 53 ++++++++++++++++++++++++++++++++++++
 gpg-interface.h              |  9 ++++++
 t/t7528-signed-commit-ssh.sh | 42 ++++++++++++++++++++++++++++
 5 files changed, 110 insertions(+)
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 64e040a99b..a348f085b2 100644
--- a/commit.c
+++ b/commit.c
@@ -1213,6 +1213,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;
 
+	sigc->payload_type = SIGNATURE_PAYLOAD_COMMIT;
 	sigc->payload = strbuf_detach(&payload, &sigc->payload_len);
 	ret = check_signature(sigc, signature.buf, signature.len);
 
diff --git a/gpg-interface.c b/gpg-interface.c
index 75ab6faacb..330cfc5845 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,11 +463,16 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
 		return -1;
 	}
 
+	if (sigc->payload_timestamp)
+		strbuf_addf(&verify_time, "-Overify-time=%s",
+			show_date(sigc->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);
@@ -478,6 +490,7 @@ 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, sigc->payload, sigc->payload_len,
 				   &ssh_keygen_out, 0, &ssh_keygen_err, 0);
@@ -512,6 +525,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) {
@@ -556,10 +570,46 @@ 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;
 }
 
+static int parse_payload_metadata(struct signature_check *sigc)
+{
+	const char *ident_line = NULL;
+	size_t ident_len;
+	struct ident_split ident;
+	const char *signer_header;
+
+	switch (sigc->payload_type) {
+	case SIGNATURE_PAYLOAD_COMMIT:
+		signer_header = "committer";
+		break;
+	case SIGNATURE_PAYLOAD_TAG:
+		signer_header = "tagger";
+		break;
+	case SIGNATURE_PAYLOAD_UNDEFINED:
+	case SIGNATURE_PAYLOAD_PUSH_CERT:
+		/* Ignore payloads we don't want to parse */
+		return 0;
+	default:
+		BUG("invalid value for sigc->payload_type");
+	}
+
+	ident_line = find_commit_header(sigc->payload, signer_header, &ident_len);
+	if (!ident_line || !ident_len)
+		return 1;
+
+	if (split_ident_line(&ident, ident_line, ident_len))
+		return 1;
+
+	if (!sigc->payload_timestamp && ident.date_begin && ident.date_end)
+		sigc->payload_timestamp = parse_timestamp(ident.date_begin, NULL, 10);
+
+	return 0;
+}
+
 int check_signature(struct signature_check *sigc,
 		    const char *signature, size_t slen)
 {
@@ -573,6 +623,9 @@ int check_signature(struct signature_check *sigc,
 	if (!fmt)
 		die(_("bad/incompatible signature '%s'"), signature);
 
+	if (parse_payload_metadata(sigc))
+		return 1;
+
 	status = fmt->verify_signed_buffer(sigc, fmt, signature, slen);
 
 	if (status && !sigc->output)
diff --git a/gpg-interface.h b/gpg-interface.h
index 5ee7d8b6b9..b30cbdcd3d 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -15,9 +15,18 @@ enum signature_trust_level {
 	TRUST_ULTIMATE,
 };
 
+enum payload_type {
+	SIGNATURE_PAYLOAD_UNDEFINED,
+	SIGNATURE_PAYLOAD_COMMIT,
+	SIGNATURE_PAYLOAD_TAG,
+	SIGNATURE_PAYLOAD_PUSH_CERT,
+};
+
 struct signature_check {
 	char *payload;
 	size_t payload_len;
+	enum payload_type payload_type;
+	timestamp_t payload_timestamp;
 	char *output;
 	char *gpg_status;
 
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
Previous: Fabian StelzerNext: Fabian Stelzer
Message 10 of 60 in “ssh signing: verify key lifetime”
  1. 0/6 ssh signing: verify key lifetimeFabian Stelzer, Oct 27, 2021
  2. 1/6 ssh signing: use sigc struct to pass payloadFabian Stelzer, Oct 27, 2021
  3. 2/6 ssh signing: add key lifetime test prereqsFabian Stelzer, Oct 27, 2021
  4. 3/6 ssh signing: make verify-commit consider key lifetimeFabian Stelzer, Oct 27, 2021
  5. Junio C HamanoOct 27, 2021
  6. Fabian StelzerOct 28, 2021
  7. 0/7 ssh signing: verify key lifetimeFabian Stelzer, Nov 17, 2021
  8. 1/7 ssh signing: use sigc struct to pass payloadFabian Stelzer, Nov 17, 2021
  9. 2/7 ssh signing: add key lifetime test prereqsFabian Stelzer, Nov 17, 2021
  10. 3/7 ssh signing: make verify-commit consider key lifetimeFabian Stelzer, Nov 17, 2021
  11. 4/7 ssh signing: make git log verify key lifetimeFabian Stelzer, Nov 17, 2021
  12. 5/7 ssh signing: make verify-tag consider key lifetimeFabian Stelzer, Nov 17, 2021
  13. 6/7 ssh signing: make fmt-merge-msg consider key lifetimeFabian Stelzer, Nov 17, 2021
  14. 7/7 ssh signing: verify ssh-keygen in test prereqFabian Stelzer, Nov 17, 2021
  15. Junio C HamanoNov 19, 2021
  16. 0/7 ssh signing: verify key lifetimeFabian Stelzer, Nov 30, 2021
  17. 1/7 ssh signing: use sigc struct to pass payloadFabian Stelzer, Nov 30, 2021
  18. 2/7 ssh signing: add key lifetime test prereqsFabian Stelzer, Nov 30, 2021
  19. 3/7 ssh signing: make verify-commit consider key lifetimeFabian Stelzer, Nov 30, 2021
  20. 4/7 ssh signing: make git log verify key lifetimeFabian Stelzer, Nov 30, 2021
  21. 5/7 ssh signing: make verify-tag consider key lifetimeFabian Stelzer, Nov 30, 2021
  22. 6/7 ssh signing: make fmt-merge-msg consider key lifetimeFabian Stelzer, Nov 30, 2021
  23. SZEDER GáborDec 5, 2021
  24. Fabian StelzerDec 8, 2021
  25. 7/7 ssh signing: verify ssh-keygen in test prereqFabian Stelzer, Nov 30, 2021
  26. Junio C HamanoDec 2, 2021
  27. Fabian StelzerDec 2, 2021
  28. Junio C HamanoDec 2, 2021
  29. Ævar Arnfjörð BjarmasonDec 3, 2021
  30. Fabian StelzerDec 3, 2021
  31. Junio C HamanoDec 3, 2021
  32. 0/8 ssh signing: verify key lifetimeFabian Stelzer, Dec 8, 2021
  33. 1/8 ssh signing: use sigc struct to pass payloadFabian Stelzer, Dec 8, 2021
  34. 2/8 ssh signing: add key lifetime test prereqsFabian Stelzer, Dec 8, 2021
  35. 3/8 ssh signing: make verify-commit consider key lifetimeFabian Stelzer, Dec 8, 2021
  36. 4/8 ssh signing: make git log verify key lifetimeFabian Stelzer, Dec 8, 2021
  37. 5/8 ssh signing: make verify-tag consider key lifetimeFabian Stelzer, Dec 8, 2021
  38. 6/8 ssh signing: make fmt-merge-msg consider key lifetimeFabian Stelzer, Dec 8, 2021
  39. 7/8 ssh signing: verify ssh-keygen in test prereqFabian Stelzer, Dec 8, 2021
  40. 8/8 t/fmt-merge-msg: make gpg/ssh tests more specificFabian Stelzer, Dec 8, 2021
  41. Junio C HamanoDec 8, 2021
  42. Fabian StelzerDec 9, 2021
  43. 0/9 ssh signing: verify key lifetimeFabian Stelzer, Dec 9, 2021
  44. 2/9 t/fmt-merge-msg: make gpgssh tests more specificFabian Stelzer, Dec 9, 2021
  45. 1/9 t/fmt-merge-msg: do not redirect stderrFabian Stelzer, Dec 9, 2021
  46. 3/9 ssh signing: use sigc struct to pass payloadFabian Stelzer, Dec 9, 2021
  47. 4/9 ssh signing: add key lifetime test prereqsFabian Stelzer, Dec 9, 2021
  48. 5/9 ssh signing: make verify-commit consider key lifetimeFabian Stelzer, Dec 9, 2021
  49. 6/9 ssh signing: make git log verify key lifetimeFabian Stelzer, Dec 9, 2021
  50. 7/9 ssh signing: make verify-tag consider key lifetimeFabian Stelzer, Dec 9, 2021
  51. 8/9 ssh signing: make fmt-merge-msg consider key lifetimeFabian Stelzer, Dec 9, 2021
  52. 9/9 ssh signing: verify ssh-keygen in test prereqFabian Stelzer, Dec 9, 2021
  53. 6/6 ssh signing: make fmt-merge-msg consider key lifetimeFabian Stelzer, Oct 27, 2021
  54. 4/6 ssh signing: make git log verify key lifetimeFabian Stelzer, Oct 27, 2021
  55. 5/6 ssh signing: make verify-tag consider key lifetimeFabian Stelzer, Oct 27, 2021
  56. Adam DinwoodieNov 3, 2021
  57. Fabian StelzerNov 3, 2021
  58. Adam DinwoodieNov 4, 2021
  59. Fabian StelzerNov 4, 2021
  60. Adam DinwoodieNov 4, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.