{"thread":{"id":"56721","subject":"[RFC PATCH 0/6] ssh signing: verify key lifetime","startedAt":"2021-10-15T13:15:12Z","lastAt":"2021-10-15T13:15:25Z","messageCount":7,"participants":["Fabian Stelzer"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"438847","messageId":"20211015131507.1773153-1-fs@gigacodes.de","threadId":"56721","inReplyTo":null,"subject":"[RFC PATCH 0/6] ssh signing: verify key lifetime","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-15T13:15:01Z","receivedAt":"2021-10-15T13:15:12Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"This series adds key lifetime validity checks by passing commit/tag\ndates to the check operaion. The whole series is functional and has\ntests. I'm sending this as an RFC since i'm unsure about the way i'm\nparsing payload info via parse_signed_buffer_metadata().\npayload was just an opaque buffer before. Now gpg-interface actually\nlooks at it (thats wy i used a seperate api function for it and did not\nhide it in check_signature).\n\nA cleaner approach would maybe be introducing a check_object_signature\nwhich takes a struct object instead of the payload directly to avoid the\nparse function altogether. However only some call sites already have\nthis struct. Tags & fmt-merge-msg use different ways to produce the\npayload and i'm not sure how involved its creation would be or what\nside-effects this could have(performance?). Since the push-certs use case\nwill never produce a object struct we would still have to keep the\nexisting check_signature function anyway.\nThe now used parse function also opens the usage up to sites already having\nboth infos (date & ident), although there is currently none.\n\nFabian Stelzer (6):\n  ssh signing: extend check_signature to accept payload metadata\n  ssh signing: add key lifetime test prereqs\n  ssh signing: verify-commit/check_signature with commit date\n  ssh signing: git log/check_signature with commit date\n  ssh signing: verify-tag/check_signature with tag date\n  ssh signing: fmt-merge-msg/check_signature with tag date\n\n Documentation/config/gpg.txt     |  5 ++\n builtin/receive-pack.c           |  5 +-\n commit.c                         | 12 ++++-\n fmt-merge-msg.c                  | 16 +++++--\n gpg-interface.c                  | 80 ++++++++++++++++++++++++++------\n gpg-interface.h                  |  9 ++++\n log-tree.c                       | 23 +++++++--\n t/lib-gpg.sh                     | 19 +++++++-\n t/t4202-log.sh                   | 43 +++++++++++++++++\n t/t6200-fmt-merge-msg.sh         | 54 +++++++++++++++++++++\n t/t7031-verify-tag-signed-ssh.sh | 42 +++++++++++++++++\n t/t7528-signed-commit-ssh.sh     | 42 +++++++++++++++++\n tag.c                            | 12 ++++-\n 13 files changed, 334 insertions(+), 28 deletions(-)\n\n\nbase-commit: 33379063c9546476a80d42c704efc4ea5d0d95e5\n-- \n2.31.1\n\n"},{"id":"438848","messageId":"20211015131507.1773153-2-fs@gigacodes.de","threadId":"56721","inReplyTo":"20211015131507.1773153-1-fs@gigacodes.de","subject":"[RFC PATCH 1/6] ssh signing: extend check_signature to accept payload metadata","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-15T13:15:02Z","receivedAt":"2021-10-15T13:15:15Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"Adds two new parameters to the check_signature api and passes them to the\ninternal verification ssh/gpg methods.\nA payload timestamp that will be used to verify signatures at the time of their\nobjects creation if the signing method supports it (only ssh for now).\nAnd a signer strbuf containing ident information about the signer that\nwe will need for implementing \"Trust on first use\" in a future patch\nseries.\n\nAdding the ident info right now instead of a later patch series makes\ncertain choices in this patch series clearer. Only passing the timestamp\ncould be done a bit simpler on some call sites but i am certain that we\nwill need the ident info for \"Trust on first use\" no matter how exactly\nit will be implemented later.\n\nTo start with we pass 0, NULL on all invocations to keep the current behaviour\nas is.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n builtin/receive-pack.c |  5 +++--\n commit.c               |  2 +-\n fmt-merge-msg.c        |  4 ++--\n gpg-interface.c        | 35 +++++++++++++++++++++--------------\n gpg-interface.h        |  1 +\n log-tree.c             |  4 ++--\n tag.c                  |  2 +-\n 7 files changed, 31 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 49b846d960..761c70642b 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -769,8 +769,9 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \n \t\tbogs = parse_signed_buffer(push_cert.buf, push_cert.len);\n-\t\tcheck_signature(push_cert.buf, bogs, push_cert.buf + bogs,\n-\t\t\t\tpush_cert.len - bogs, &sigcheck);\n+\t\tcheck_signature(push_cert.buf, bogs, 0, NULL,\n+\t\t\t\tpush_cert.buf + bogs, push_cert.len - bogs,\n+\t\t\t\t&sigcheck);\n \n \t\tnonce_status = check_nonce(push_cert.buf, bogs);\n \t}\ndiff --git a/commit.c b/commit.c\nindex 551de4903c..1704d9df0a 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1212,7 +1212,7 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \n \tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n-\tret = check_signature(payload.buf, payload.len, signature.buf,\n+\tret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,\n \t\tsignature.len, sigc);\n \n  out:\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 5216191488..d2cedad6b7 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -533,8 +533,8 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\telse {\n \t\t\tbuf = payload.buf;\n \t\t\tlen = payload.len;\n-\t\t\tif (check_signature(payload.buf, payload.len, sig.buf,\n-\t\t\t\t\t    sig.len, &sigc) &&\n+\t\t\tif (check_signature(payload.buf, payload.len, 0, NULL,\n+\t\t\t\t\t    sig.buf, sig.len, &sigc) &&\n \t\t\t    !sigc.output)\n \t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n \t\t\telse\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 433482307c..5960202615 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -20,8 +20,9 @@ struct gpg_format {\n \tconst char **sigs;\n \tint (*verify_signed_buffer)(struct signature_check *sigc,\n \t\t\t\t    struct gpg_format *fmt, const char *payload,\n-\t\t\t\t    size_t payload_size, const char *signature,\n-\t\t\t\t    size_t signature_size);\n+\t\t\t\t    size_t payload_size, timestamp_t payload_timestamp,\n+\t\t\t\t    struct strbuf *payload_signer,\n+\t\t\t\t    const char *signature, size_t signature_size);\n \tint (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature,\n \t\t\t   const char *signing_key);\n \tconst char *(*get_default_key)(void);\n@@ -54,12 +55,14 @@ static const char *ssh_sigs[] = {\n \n static int verify_gpg_signed_buffer(struct signature_check *sigc,\n \t\t\t\t    struct gpg_format *fmt, const char *payload,\n-\t\t\t\t    size_t payload_size, const char *signature,\n-\t\t\t\t    size_t signature_size);\n+\t\t\t\t    size_t payload_size, timestamp_t payload_timestamp,\n+\t\t\t\t    struct strbuf *payload_signer,\n+\t\t\t\t    const char *signature, size_t signature_size);\n static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\t\t\t    struct gpg_format *fmt, const char *payload,\n-\t\t\t\t    size_t payload_size, const char *signature,\n-\t\t\t\t    size_t signature_size);\n+\t\t\t\t    size_t payload_size, timestamp_t payload_timestamp,\n+\t\t\t\t    struct strbuf *payload_signer,\n+\t\t\t\t    const char *signature, size_t signature_size);\n static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t\t\t   const char *signing_key);\n static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,\n@@ -315,8 +318,9 @@ static void parse_gpg_output(struct signature_check *sigc)\n \n static int verify_gpg_signed_buffer(struct signature_check *sigc,\n \t\t\t\t    struct gpg_format *fmt, const char *payload,\n-\t\t\t\t    size_t payload_size, const char *signature,\n-\t\t\t\t    size_t signature_size)\n+\t\t\t\t    size_t payload_size, timestamp_t payload_timestamp,\n+\t\t\t\t    struct strbuf *payload_signer,\n+\t\t\t\t    const char *signature, size_t signature_size)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tstruct tempfile *temp;\n@@ -421,8 +425,9 @@ static void parse_ssh_output(struct signature_check *sigc)\n \n static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\t\t\t    struct gpg_format *fmt, const char *payload,\n-\t\t\t\t    size_t payload_size, const char *signature,\n-\t\t\t\t    size_t signature_size)\n+\t\t\t\t    size_t payload_size, timestamp_t payload_timestamp,\n+\t\t\t\t    struct strbuf *payload_signer,\n+\t\t\t\t    const char *signature, size_t signature_size)\n {\n \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n \tstruct tempfile *buffer_file;\n@@ -556,8 +561,9 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \treturn ret;\n }\n \n-int check_signature(const char *payload, size_t plen, const char *signature,\n-\tsize_t slen, struct signature_check *sigc)\n+int check_signature(const char *payload, size_t plen, timestamp_t payload_timestamp,\n+\t\t    struct strbuf *payload_signer, const char *signature,\n+\t\t    size_t slen, struct signature_check *sigc)\n {\n \tstruct gpg_format *fmt;\n \tint status;\n@@ -569,8 +575,9 @@ int check_signature(const char *payload, size_t plen, const char *signature,\n \tif (!fmt)\n \t\tdie(_(\"bad/incompatible signature '%s'\"), signature);\n \n-\tstatus = fmt->verify_signed_buffer(sigc, fmt, payload, plen, signature,\n-\t\t\t\t\t   slen);\n+\tstatus = fmt->verify_signed_buffer(sigc, fmt, payload, plen,\n+\t\t\t\t\t   payload_timestamp, payload_signer,\n+\t\t\t\t\t   signature, slen);\n \n \tif (status && !sigc->output)\n \t\treturn !!status;\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex beefacbb1e..a326845be8 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -71,6 +71,7 @@ const char *get_signing_key(void);\n  */\n const char *get_signing_key_id(void);\n int check_signature(const char *payload, size_t plen,\n+\t\t    timestamp_t payload_timestamp, struct strbuf *payload_signer,\n \t\t    const char *signature, size_t slen,\n \t\t    struct signature_check *sigc);\n void print_signature_buffer(const struct signature_check *sigc,\ndiff --git a/log-tree.c b/log-tree.c\nindex 644893fd8c..3c3aec5c40 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -513,7 +513,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n-\tstatus = check_signature(payload.buf, payload.len, signature.buf,\n+\tstatus = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,\n \t\t\t\t signature.len, &sigc);\n \tif (status && !sigc.output)\n \t\tshow_sig_lines(opt, status, \"No signature\\n\");\n@@ -583,7 +583,7 @@ static int show_one_mergetag(struct commit *commit,\n \tstatus = -1;\n \tif (parse_signature(extra->value, extra->len, &payload, &signature)) {\n \t\t/* could have a good signature */\n-\t\tstatus = check_signature(payload.buf, payload.len,\n+\t\tstatus = check_signature(payload.buf, payload.len, 0, NULL,\n \t\t\t\t\t signature.buf, signature.len, &sigc);\n \t\tif (sigc.output)\n \t\t\tstrbuf_addstr(&verify_message, sigc.output);\ndiff --git a/tag.c b/tag.c\nindex 3e18a41841..3459a0867c 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -25,7 +25,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(payload.buf, payload.len, signature.buf,\n+\tret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,\n \t\t\t\tsignature.len, &sigc);\n \n \tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n-- \n2.31.1\n\n"},{"id":"438849","messageId":"20211015131507.1773153-3-fs@gigacodes.de","threadId":"56721","inReplyTo":"20211015131507.1773153-1-fs@gigacodes.de","subject":"[RFC PATCH 2/6] ssh signing: add key lifetime test prereqs","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-15T13:15:03Z","receivedAt":"2021-10-15T13:15:16Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"if ssh-keygen supports -Overify-time, add test keys marked as expired,\nnot yet valid and valid both within the test_tick timeframe and outside of it.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n t/lib-gpg.sh | 19 ++++++++++++++++++-\n 1 file changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex f99ef3e859..139e7a45ea 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -90,6 +90,10 @@ test_lazy_prereq RFC1991 '\n GPGSSH_KEY_PRIMARY=\"${GNUPGHOME}/ed25519_ssh_signing_key\"\n GPGSSH_KEY_SECONDARY=\"${GNUPGHOME}/rsa_2048_ssh_signing_key\"\n GPGSSH_KEY_UNTRUSTED=\"${GNUPGHOME}/untrusted_ssh_signing_key\"\n+GPGSSH_KEY_EXPIRED=\"${GNUPGHOME}/expired_ssh_signing_key\"\n+GPGSSH_KEY_NOTYETVALID=\"${GNUPGHOME}/notyetvalid_ssh_signing_key\"\n+GPGSSH_KEY_TIMEBOXEDVALID=\"${GNUPGHOME}/timeboxed_valid_ssh_signing_key\"\n+GPGSSH_KEY_TIMEBOXEDINVALID=\"${GNUPGHOME}/timeboxed_invalid_ssh_signing_key\"\n GPGSSH_KEY_WITH_PASSPHRASE=\"${GNUPGHOME}/protected_ssh_signing_key\"\n GPGSSH_KEY_PASSPHRASE=\"super_secret\"\n GPGSSH_ALLOWED_SIGNERS=\"${GNUPGHOME}/ssh.all_valid.allowedSignersFile\"\n@@ -112,7 +116,20 @@ test_lazy_prereq GPGSSH '\n \techo \"\\\"principal with number 2\\\" $(cat \"${GPGSSH_KEY_SECONDARY}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n \tssh-keygen -t ed25519 -N \"${GPGSSH_KEY_PASSPHRASE}\" -C \"git ed25519 encrypted key\" -f \"${GPGSSH_KEY_WITH_PASSPHRASE}\" >/dev/null &&\n \techo \"\\\"principal with number 3\\\" $(cat \"${GPGSSH_KEY_WITH_PASSPHRASE}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n-\tssh-keygen -t ed25519 -N \"\" -f \"${GPGSSH_KEY_UNTRUSTED}\" >/dev/null\n+\tssh-keygen -t ed25519 -N \"\" -C \"git ed25519 key\" -f \"${GPGSSH_KEY_UNTRUSTED}\" >/dev/null\n+'\n+\n+test_lazy_prereq GPGSSH_VERIFYTIME '\n+\t# Check if ssh-keygen has a verify-time option by passing an invalid date to it\n+\tssh-keygen -Overify-time=INVALID -Y check-novalidate -s doesnotmatter 2>&1 | grep -q -F \"Invalid \\\"verify-time\\\"\" &&\n+\tssh-keygen -t ed25519 -N \"\" -C \"timeboxed valid key\" -f \"${GPGSSH_KEY_TIMEBOXEDVALID}\" >/dev/null &&\n+\techo \"\\\"timeboxed valid key\\\" valid-after=\\\"20050407000000\\\",valid-before=\\\"200504100000\\\" $(cat \"${GPGSSH_KEY_TIMEBOXEDVALID}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tssh-keygen -t ed25519 -N \"\" -C \"timeboxed invalid key\" -f \"${GPGSSH_KEY_TIMEBOXEDINVALID}\" >/dev/null &&\n+\techo \"\\\"timeboxed invalid key\\\" valid-after=\\\"20050401000000\\\",valid-before=\\\"20050402000000\\\" $(cat \"${GPGSSH_KEY_TIMEBOXEDINVALID}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tssh-keygen -t ed25519 -N \"\" -C \"expired key\" -f \"${GPGSSH_KEY_EXPIRED}\" >/dev/null &&\n+\techo \"\\\"principal with expired key\\\" valid-before=\\\"20000101000000\\\" $(cat \"${GPGSSH_KEY_EXPIRED}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tssh-keygen -t ed25519 -N \"\" -C \"not yet valid key\" -f \"${GPGSSH_KEY_NOTYETVALID}\" >/dev/null &&\n+\techo \"\\\"principal with not yet valid key\\\" valid-after=\\\"29990101000000\\\" $(cat \"${GPGSSH_KEY_NOTYETVALID}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\"\n '\n \n sanitize_pgp() {\n-- \n2.31.1\n\n"},{"id":"438850","messageId":"20211015131507.1773153-5-fs@gigacodes.de","threadId":"56721","inReplyTo":"20211015131507.1773153-1-fs@gigacodes.de","subject":"[RFC PATCH 4/6] ssh signing: git log/check_signature with commit date","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-15T13:15:05Z","receivedAt":"2021-10-15T13:15:18Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"Pass the commit date and ident to check_signature when calling git log.\nImplements the same tests as for verify-commit.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n log-tree.c     | 23 +++++++++++++++++++----\n t/t4202-log.sh | 43 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 4 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 3c3aec5c40..2fd7f0c398 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -509,12 +509,18 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \tstruct strbuf signature = STRBUF_INIT;\n \tstruct signature_check sigc = { 0 };\n \tint status;\n+\tstruct strbuf payload_signer = STRBUF_INIT;\n+\ttimestamp_t payload_timestamp = 0;\n \n \tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n-\tstatus = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,\n-\t\t\t\t signature.len, &sigc);\n+\tif (parse_signed_buffer_metadata(payload.buf, \"committer\", &payload_timestamp, &payload_signer))\n+\t\tgoto out;\n+\n+\tstatus = check_signature(payload.buf, payload.len, payload_timestamp,\n+\t\t\t\t &payload_signer, signature.buf, signature.len,\n+\t\t\t\t &sigc);\n \tif (status && !sigc.output)\n \t\tshow_sig_lines(opt, status, \"No signature\\n\");\n \telse\n@@ -524,6 +530,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n  out:\n \tstrbuf_release(&payload);\n \tstrbuf_release(&signature);\n+\tstrbuf_release(&payload_signer);\n }\n \n static int which_parent(const struct object_id *oid, const struct commit *commit)\n@@ -558,6 +565,8 @@ static int show_one_mergetag(struct commit *commit,\n \tint status, nth;\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n+\tstruct strbuf payload_signer = STRBUF_INIT;\n+\ttimestamp_t payload_timestamp = 0;\n \n \thash_object_file(the_hash_algo, extra->value, extra->len,\n \t\t\t type_name(OBJ_TAG), &oid);\n@@ -582,9 +591,14 @@ static int show_one_mergetag(struct commit *commit,\n \n \tstatus = -1;\n \tif (parse_signature(extra->value, extra->len, &payload, &signature)) {\n+\n+\t\tif (parse_signed_buffer_metadata(payload.buf, \"tagger\", &payload_timestamp, &payload_signer))\n+\t\t\tstrbuf_addstr(&verify_message, _(\"failed to parse timestamp and signer info from payload\"));\n+\n \t\t/* could have a good signature */\n-\t\tstatus = check_signature(payload.buf, payload.len, 0, NULL,\n-\t\t\t\t\t signature.buf, signature.len, &sigc);\n+\t\tstatus = check_signature(payload.buf, payload.len, payload_timestamp,\n+\t\t\t\t\t &payload_signer, signature.buf,\n+\t\t\t\t\t signature.len, &sigc);\n \t\tif (sigc.output)\n \t\t\tstrbuf_addstr(&verify_message, sigc.output);\n \t\telse\n@@ -597,6 +611,7 @@ static int show_one_mergetag(struct commit *commit,\n \tstrbuf_release(&verify_message);\n \tstrbuf_release(&payload);\n \tstrbuf_release(&signature);\n+\tstrbuf_release(&payload_signer);\n \treturn 0;\n }\n \ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 6a650dacd6..2b12baab77 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1626,6 +1626,24 @@ test_expect_success GPGSSH 'setup sshkey signed branch' '\n \tgit commit -S -m signed_commit\n '\n \n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '\n+\ttest_config gpg.format ssh &&\n+\ttouch file &&\n+\tgit add file &&\n+\n+\techo expired >file && test_tick && git commit -a -m expired -S\"${GPGSSH_KEY_EXPIRED}\" &&\n+\tgit tag expired-signed &&\n+\n+\techo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S\"${GPGSSH_KEY_NOTYETVALID}\" &&\n+\tgit tag notyetvalid-signed &&\n+\n+\techo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S\"${GPGSSH_KEY_TIMEBOXEDVALID}\" &&\n+\tgit tag timeboxedvalid-signed &&\n+\n+\techo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S\"${GPGSSH_KEY_TIMEBOXEDINVALID}\" &&\n+\tgit tag timeboxedinvalid-signed\n+'\n+\n test_expect_success GPGSM 'log x509 fingerprint' '\n \techo \"F8BF62E0693D0694816377099909C779FA23FD65 | \" >expect &&\n \tgit log -n1 --format=\"%GF | %GP\" signed-x509 >actual &&\n@@ -1663,6 +1681,31 @@ test_expect_success GPGSSH 'log --graph --show-signature ssh' '\n \tgrep \"${GOOD_SIGNATURE_TRUSTED}\" actual\n '\n \n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log shows failure on expired signature key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit log --graph --show-signature -n1 expired-signed >actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log shows failure on not yet valid signature key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit log --graph --show-signature -n1 notyetvalid-signed >actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log show success with commit date and key validity matching' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit log --graph --show-signature -n1 timeboxedvalid-signed >actual &&\n+\tgrep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual &&\n+\t! grep \"${GPGSSH_BAD_SIGNATURE}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'log shows failure with commit date outside of key validity' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit log --graph --show-signature -n1 timeboxedinvalid-signed >actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n test_expect_success GPG 'log --graph --show-signature for merged tag' '\n \ttest_when_finished \"git reset --hard && git checkout main\" &&\n \tgit checkout -b plain main &&\n-- \n2.31.1\n\n"},{"id":"438851","messageId":"20211015131507.1773153-4-fs@gigacodes.de","threadId":"56721","inReplyTo":"20211015131507.1773153-1-fs@gigacodes.de","subject":"[RFC PATCH 3/6] ssh signing: verify-commit/check_signature with commit date","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-15T13:15:04Z","receivedAt":"2021-10-15T13:15:19Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"If valid-before/after dates are configured for this signatures key in the\nallowedSigners file then the verification will check if the key was valid at\nthe time the commit was made. This allows for graceful key rollover and\nrevoking keys without invalidating all previous commits.\nThis feature needs openssh > 8.8. Older ssh-keygen versions will simply\nignore this flag and use the current time.\nStrictly speaking this feature is available in 8.7, but since 8.7 has a\nbug that makes it unusable in another needed call we require 8.8.\n\nTimestamp information is present on most invocations of check_signature.\nHowever signer ident is not. We will need the signer email / name to be able\nto implement \"Trust on first use\" functionality.\nTherefore we provide parse_signed_buffer_metadata() for parsing both from\nthe payload buffer.\n\nPasses commit date for verify-commit on to check_signature.\nAdds test for keys being expired, not yet valid, having a commit date outside\nof key validity and within.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n Documentation/config/gpg.txt |  5 ++++\n commit.c                     | 12 ++++++++--\n gpg-interface.c              | 45 ++++++++++++++++++++++++++++++++++++\n gpg-interface.h              |  8 +++++++\n t/t7528-signed-commit-ssh.sh | 42 +++++++++++++++++++++++++++++++++\n 5 files changed, 110 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt\nindex 4f30c7dbdd..c9be554c73 100644\n--- a/Documentation/config/gpg.txt\n+++ b/Documentation/config/gpg.txt\n@@ -64,6 +64,11 @@ A repository that only allows signed commits can store the file\n in the repository itself using a path relative to the top-level of the working tree.\n This way only committers with an already valid key can add or change keys in the keyring.\n +\n+Since OpensSSH 8.8 this file allows specifying a key lifetime using valid-after &\n+valid-before options. Git will mark signatures as valid if the signing key was\n+valid at the time of the signatures creation. This allows users to change a\n+signing key without invalidating all previously made signatures.\n++\n Using a SSH CA key with the cert-authority option\n (see ssh-keygen(1) \"CERTIFICATES\") is also valid.\n \ndiff --git a/commit.c b/commit.c\nindex 1704d9df0a..f70b233e00 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1207,17 +1207,25 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n \tint ret = 1;\n+\tstruct strbuf payload_signer = STRBUF_INIT;\n+\ttimestamp_t payload_timestamp = 0;\n \n \tsigc->result = 'N';\n \n \tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n-\tret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,\n-\t\tsignature.len, sigc);\n+\tif (parse_signed_buffer_metadata(payload.buf, \"committer\",\n+\t\t\t\t\t &payload_timestamp, &payload_signer))\n+\t\tgoto out;\n+\n+\tret = check_signature(payload.buf, payload.len, payload_timestamp,\n+\t\t\t      &payload_signer, signature.buf,\n+\t\t\t      signature.len, sigc);\n \n  out:\n \tstrbuf_release(&payload);\n \tstrbuf_release(&signature);\n+\tstrbuf_release(&payload_signer);\n \n \treturn ret;\n }\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 5960202615..a51d8e361c 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -439,6 +439,13 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \tstruct strbuf ssh_principals_err = STRBUF_INIT;\n \tstruct strbuf ssh_keygen_out = STRBUF_INIT;\n \tstruct strbuf ssh_keygen_err = STRBUF_INIT;\n+\tstruct strbuf verify_time = STRBUF_INIT;\n+\tconst struct date_mode verify_date_mode = {\n+\t\t.type = DATE_STRFTIME,\n+\t\t.strftime_fmt = \"%Y%m%d%H%M%S\",\n+\t\t/* SSH signing key validity has no timezone information - Use the local timezone */\n+\t\t.local = 1,\n+\t};\n \n \tif (!ssh_allowed_signers) {\n \t\terror(_(\"gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification\"));\n@@ -456,18 +463,27 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\treturn -1;\n \t}\n \n+\tif (payload_timestamp != 0) {\n+\t\tstrbuf_addf(&verify_time, \"-Overify-time=%s\",\n+\t\t\t    show_date(payload_timestamp, 0, &verify_date_mode));\n+\t}\n+\n \t/* Find the principal from the signers */\n \tstrvec_pushl(&ssh_keygen.args, fmt->program,\n \t\t     \"-Y\", \"find-principals\",\n \t\t     \"-f\", ssh_allowed_signers,\n \t\t     \"-s\", buffer_file->filename.buf,\n+\t\t     verify_time.buf,\n \t\t     NULL);\n+\n \tret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,\n \t\t\t   &ssh_principals_err, 0);\n+\n \tif (ret && strstr(ssh_principals_err.buf, \"usage:\")) {\n \t\terror(_(\"ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)\"));\n \t\tgoto out;\n \t}\n+\n \tif (ret || !ssh_principals_out.len) {\n \t\t/*\n \t\t * We did not find a matching principal in the allowedSigners\n@@ -478,7 +494,9 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\t\t     \"-Y\", \"check-novalidate\",\n \t\t\t     \"-n\", \"git\",\n \t\t\t     \"-s\", buffer_file->filename.buf,\n+\t\t\t     verify_time.buf,\n \t\t\t     NULL);\n+\n \t\tpipe_command(&ssh_keygen, payload, payload_size,\n \t\t\t\t   &ssh_keygen_out, 0, &ssh_keygen_err, 0);\n \n@@ -512,6 +530,7 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\t\t\t     \"-f\", ssh_allowed_signers,\n \t\t\t\t     \"-I\", principal,\n \t\t\t\t     \"-s\", buffer_file->filename.buf,\n+\t\t\t\t     verify_time.buf,\n \t\t\t\t     NULL);\n \n \t\t\tif (ssh_revocation_file) {\n@@ -557,10 +576,36 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \tstrbuf_release(&ssh_principals_err);\n \tstrbuf_release(&ssh_keygen_out);\n \tstrbuf_release(&ssh_keygen_err);\n+\tstrbuf_release(&verify_time);\n \n \treturn ret;\n }\n \n+int parse_signed_buffer_metadata(const char *payload, const char *signer_header,\n+\t\t\t\t timestamp_t *payload_timestamp,\n+\t\t\t\t struct strbuf *payload_signer)\n+{\n+\tconst char *ident_line = NULL;\n+\tsize_t ident_len;\n+\tstruct ident_split ident;\n+\n+\tident_line = find_commit_header(payload, signer_header, &ident_len);\n+\tif (ident_line && ident_len) {\n+\t\tif (!split_ident_line(&ident, ident_line, ident_len)) {\n+\t\t\tif (payload_timestamp && ident.date_begin && ident.date_end)\n+\t\t\t\t*payload_timestamp = parse_timestamp(\n+\t\t\t\t\tident.date_begin, NULL, 10);\n+\t\t\tif (payload_signer)\n+\t\t\t\tstrbuf_add(payload_signer, ident.mail_begin,\n+\t\t\t\t\t(ident.mail_end - ident.mail_begin));\n+\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n+\treturn 1;\n+}\n+\n int check_signature(const char *payload, size_t plen, timestamp_t payload_timestamp,\n \t\t    struct strbuf *payload_signer, const char *signature,\n \t\t    size_t slen, struct signature_check *sigc)\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex a326845be8..27a34d06e8 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -65,6 +65,14 @@ int git_gpg_config(const char *, const char *, void *);\n void set_signing_key(const char *);\n const char *get_signing_key(void);\n \n+/*\n+ * Tries to find \"signer_header\" (usually committer or tagger) in the payload,\n+ * and sets the corresponding timestamp and signer parameters if found.\n+ */\n+int parse_signed_buffer_metadata(const char *payload, const char *signer_header,\n+\t\t\t\t timestamp_t *payload_timestamp,\n+\t\t\t\t struct strbuf *payload_signer);\n+\n /*\n  * Returns a textual unique representation of the signing key in use\n  * Either a GPG KeyID or a SSH Key Fingerprint\ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex badf3ed320..dae76ded0c 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -76,6 +76,23 @@ test_expect_success GPGSSH 'create signed commits' '\n \tgit tag twelfth-signed-alt $(cat oid)\n '\n \n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '\n+\ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n+\ttest_config gpg.format ssh &&\n+\n+\techo expired >file && test_tick && git commit -a -m expired -S\"${GPGSSH_KEY_EXPIRED}\" &&\n+\tgit tag expired-signed &&\n+\n+\techo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S\"${GPGSSH_KEY_NOTYETVALID}\" &&\n+\tgit tag notyetvalid-signed &&\n+\n+\techo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S\"${GPGSSH_KEY_TIMEBOXEDVALID}\" &&\n+\tgit tag timeboxedvalid-signed &&\n+\n+\techo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S\"${GPGSSH_KEY_TIMEBOXEDINVALID}\" &&\n+\tgit tag timeboxedinvalid-signed\n+'\n+\n test_expect_success GPGSSH 'verify and show signatures' '\n \ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n \ttest_config gpg.mintrustlevel UNDEFINED &&\n@@ -122,6 +139,31 @@ test_expect_success GPGSSH 'verify-commit exits failure on untrusted signature'\n \tgrep \"${GPGSSH_KEY_NOT_TRUSTED}\" actual\n '\n \n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure on expired signature key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\ttest_must_fail git verify-commit expired-signed 2>actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure on not yet valid signature key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\ttest_must_fail git verify-commit notyetvalid-signed 2>actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit succeeds with commit date and key validity matching' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit verify-commit timeboxedvalid-signed 2>actual &&\n+\tgrep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual &&\n+\t! grep \"${GPGSSH_BAD_SIGNATURE}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-commit exits failure with commit date outside of key validity' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\ttest_must_fail git verify-commit timeboxedinvalid-signed 2>actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n test_expect_success GPGSSH 'verify-commit exits success with matching minTrustLevel' '\n \ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n \ttest_config gpg.minTrustLevel fully &&\n-- \n2.31.1\n\n"},{"id":"438852","messageId":"20211015131507.1773153-7-fs@gigacodes.de","threadId":"56721","inReplyTo":"20211015131507.1773153-1-fs@gigacodes.de","subject":"[RFC PATCH 6/6] ssh signing: fmt-merge-msg/check_signature with tag date","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-15T13:15:07Z","receivedAt":"2021-10-15T13:15:23Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"Pass the tag date and tagger ident to check_signature when generating merge\nmessages to verify merged tags signatures.\nImplements the same tests as for verify-commit.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n fmt-merge-msg.c          | 14 +++++++++--\n t/t6200-fmt-merge-msg.sh | 54 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 66 insertions(+), 2 deletions(-)\n\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex d2cedad6b7..95b747e85a 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -524,6 +524,8 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\tunsigned long len = size;\n \t\tstruct signature_check sigc = { NULL };\n \t\tstruct strbuf payload = STRBUF_INIT, sig = STRBUF_INIT;\n+\t\tstruct strbuf payload_signer = STRBUF_INIT;\n+\t\ttimestamp_t payload_timestamp = 0;\n \n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n@@ -533,9 +535,16 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\telse {\n \t\t\tbuf = payload.buf;\n \t\t\tlen = payload.len;\n-\t\t\tif (check_signature(payload.buf, payload.len, 0, NULL,\n+\n+\t\t\tif (parse_signed_buffer_metadata(payload.buf, \"tagger\",\n+\t\t\t\t\t\t\t &payload_timestamp,\n+\t\t\t\t\t\t\t &payload_signer))\n+\t\t\t\tstrbuf_addstr(&sig, _(\"failed to parse timestamp and signer info from payload\"));\n+\n+\t\t\tif (check_signature(payload.buf, payload.len,\n+\t\t\t\t\t    payload_timestamp, &payload_signer,\n \t\t\t\t\t    sig.buf, sig.len, &sigc) &&\n-\t\t\t    !sigc.output)\n+\t\t\t!sigc.output)\n \t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n \t\t\telse\n \t\t\t\tstrbuf_addstr(&sig, sigc.output);\n@@ -564,6 +573,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\t}\n \t\tstrbuf_release(&payload);\n \t\tstrbuf_release(&sig);\n+\t\tstrbuf_release(&payload_signer);\n \tnext:\n \t\tfree(origbuf);\n \t}\ndiff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh\nindex 06c5fb5615..2dd2423643 100755\n--- a/t/t6200-fmt-merge-msg.sh\n+++ b/t/t6200-fmt-merge-msg.sh\n@@ -91,6 +91,26 @@ test_expect_success GPGSSH 'created ssh signed commit and tag' '\n \tgit tag -s -u\"${GPGSSH_KEY_UNTRUSTED}\" -m signed-ssh-tag-msg-untrusted signed-untrusted-ssh-tag left\n '\n \n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed tags with keys having defined lifetimes' '\n+\ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n+\ttest_config gpg.format ssh &&\n+\tgit checkout -b signed-expiry-ssh &&\n+\ttouch file &&\n+\tgit add file &&\n+\n+\techo expired >file && test_tick && git commit -a -m expired -S\"${GPGSSH_KEY_EXPIRED}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_EXPIRED}\" -m expired-signed expired-signed &&\n+\n+\techo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S\"${GPGSSH_KEY_NOTYETVALID}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_NOTYETVALID}\" -m notyetvalid-signed notyetvalid-signed &&\n+\n+\techo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S\"${GPGSSH_KEY_TIMEBOXEDVALID}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_TIMEBOXEDVALID}\" -m timeboxedvalid-signed timeboxedvalid-signed &&\n+\n+\techo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S\"${GPGSSH_KEY_TIMEBOXEDINVALID}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_TIMEBOXEDINVALID}\" -m timeboxedinvalid-signed timeboxedinvalid-signed\n+'\n+\n test_expect_success 'message for merging local branch' '\n \techo \"Merge branch ${apos}left${apos}\" >expected &&\n \n@@ -137,6 +157,40 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh\n \t! grep \"${GPGSSH_BAD_SIGNATURE}\" actual &&\n \tgrep \"${GPGSSH_KEY_NOT_TRUSTED}\" actual\n '\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by expired ssh key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit checkout main &&\n+\tgit fetch . expired-signed &&\n+\tgit fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by not yet valid ssh key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit checkout main &&\n+\tgit fetch . notyetvalid-signed &&\n+\tgit fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by valid timeboxed ssh key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit checkout main &&\n+\tgit fetch . timeboxedvalid-signed &&\n+\tgit fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&\n+\tgrep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual &&\n+\t! grep \"${GPGSSH_BAD_SIGNATURE}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by invalid timeboxed ssh key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit checkout main &&\n+\tgit fetch . timeboxedinvalid-signed &&\n+\tgit fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n test_expect_success 'message for merging external branch' '\n \techo \"Merge branch ${apos}left${apos} of $(pwd)\" >expected &&\n \n-- \n2.31.1\n\n"},{"id":"438853","messageId":"20211015131507.1773153-6-fs@gigacodes.de","threadId":"56721","inReplyTo":"20211015131507.1773153-1-fs@gigacodes.de","subject":"[RFC PATCH 5/6] ssh signing: verify-tag/check_signature with tag date","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-15T13:15:06Z","receivedAt":"2021-10-15T13:15:25Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"Passes tag date and tagger ident to to check_signature.\nImplements the same tests as for verify-commit.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n t/t7031-verify-tag-signed-ssh.sh | 42 ++++++++++++++++++++++++++++++++\n tag.c                            | 12 +++++++--\n 2 files changed, 52 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7031-verify-tag-signed-ssh.sh b/t/t7031-verify-tag-signed-ssh.sh\nindex 06c9dd6c93..1cb36b9ab8 100755\n--- a/t/t7031-verify-tag-signed-ssh.sh\n+++ b/t/t7031-verify-tag-signed-ssh.sh\n@@ -48,6 +48,23 @@ test_expect_success GPGSSH 'create signed tags ssh' '\n \tgit tag -u\"${GPGSSH_KEY_UNTRUSTED}\" -m eighth eighth-signed-alt\n '\n \n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed tags with keys having defined lifetimes' '\n+\ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n+\ttest_config gpg.format ssh &&\n+\n+\techo expired >file && test_tick && git commit -a -m expired -S\"${GPGSSH_KEY_EXPIRED}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_EXPIRED}\" -m expired-signed expired-signed &&\n+\n+\techo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S\"${GPGSSH_KEY_NOTYETVALID}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_NOTYETVALID}\" -m notyetvalid-signed notyetvalid-signed &&\n+\n+\techo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S\"${GPGSSH_KEY_TIMEBOXEDVALID}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_TIMEBOXEDVALID}\" -m timeboxedvalid-signed timeboxedvalid-signed &&\n+\n+\techo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S\"${GPGSSH_KEY_TIMEBOXEDINVALID}\" &&\n+\tgit tag -s -u \"${GPGSSH_KEY_TIMEBOXEDINVALID}\" -m timeboxedinvalid-signed timeboxedinvalid-signed\n+'\n+\n test_expect_success GPGSSH 'verify and show ssh signatures' '\n \ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n \t(\n@@ -80,6 +97,31 @@ test_expect_success GPGSSH 'verify and show ssh signatures' '\n \t)\n '\n \n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag exits failure on expired signature key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\ttest_must_fail git verify-tag expired-signed 2>actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag exits failure on not yet valid signature key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\ttest_must_fail git verify-tag notyetvalid-signed 2>actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag succeeds with tag date and key validity matching' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit verify-tag timeboxedvalid-signed 2>actual &&\n+\tgrep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual &&\n+\t! grep \"${GPGSSH_BAD_SIGNATURE}\" actual\n+'\n+\n+test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag failes with tag date outside of key validity' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\ttest_must_fail git verify-tag timeboxedinvalid-signed 2>actual &&\n+\t! grep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual\n+'\n+\n test_expect_success GPGSSH 'detect fudged ssh signature' '\n \ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n \tgit cat-file tag seventh-signed >raw &&\ndiff --git a/tag.c b/tag.c\nindex 3459a0867c..1868e6b51d 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -16,6 +16,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n \tint ret;\n+\tstruct strbuf payload_signer = STRBUF_INIT;\n+\ttimestamp_t payload_timestamp = 0;\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n@@ -25,8 +27,13 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \t\treturn error(\"no signature found\");\n \t}\n \n-\tret = check_signature(payload.buf, payload.len, 0, NULL, signature.buf,\n-\t\t\t\tsignature.len, &sigc);\n+\tif (parse_signed_buffer_metadata(payload.buf, \"tagger\", &payload_timestamp,\n+\t\t\t\t\t &payload_signer))\n+\t\treturn error(_(\"failed to parse timestamp and signer info from payload\"));\n+\n+\tret = check_signature(payload.buf, payload.len, payload_timestamp,\n+\t\t\t      &payload_signer, signature.buf, signature.len,\n+\t\t\t      &sigc);\n \n \tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n \t\tprint_signature_buffer(&sigc, flags);\n@@ -34,6 +41,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \tsignature_check_clear(&sigc);\n \tstrbuf_release(&payload);\n \tstrbuf_release(&signature);\n+\tstrbuf_release(&payload_signer);\n \treturn ret;\n }\n \n-- \n2.31.1\n\n"}]}