[PATCH v8 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen
- From
Fabian Stelzer via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Sep 10, 2021, 20:07 UTC
- Message-ID
- <pull.1041.v8.git.git.1631304462.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1041.v7.git.git.1627998358.gitgitgadget@gmail.com>
openssh 8.7 will add valid-after, valid-before options to the allowed keys keyring. This allows us to pass the commit timestamp to the verification call and make key rollover possible and still be able to verify older commits. Set valid-after to the current date when adding your key to the keyring and set valid-before to make it fail if used after a certain date. Software like gitolite/github or corporate automation can do this automatically when ssh push keys are addded / removed I will add this feature in a follow up patch afterwards since the released 8.7 version has a broken ssh-keygen implementation which will break ssh signing completely.
v7:
* change unknown signing key behavior to fail verify-commit/tag just like
gpg does
* add test for unknown signing keys for ssh & gpg
* made default signing key retrieval configurable
(gpg.ssh.defaultKeyCommand). We could default this to "ssh-add -L" but
would risk some users signing with a wrong key
* die() instead of error in case of incompatible signatures to match
current BUG() behaviour more
* various review fixes (early return for config parse, missing free,
comments)
* got rid of strcmp("ssh") branches and used format configurable callbacks
everywhere
* moved documentation changes into the commits adding the specific
functionalityv8:
* fixes a bug around find-principals buffer i was releasing while still iterating over it. Uses separate strbufs now. * rename a wrong variable in the tests * use git_config_pathname instead of string where applicable
Fabian Stelzer (9): ssh signing: preliminary refactoring and clean-up ssh signing: add test prereqs ssh signing: add ssh key format and signing code ssh signing: retrieve a default key from ssh-agent ssh signing: provide a textual signing_key_id ssh signing: verify signatures using ssh-keygen ssh signing: duplicate t7510 tests for commits ssh signing: tests for logs, tags & push certs ssh signing: test that gpg fails for unknown keys
Documentation/config/gpg.txt | 45 ++- Documentation/config/user.txt | 7 + builtin/receive-pack.c | 4 + fmt-merge-msg.c | 6 +- gpg-interface.c | 577 ++++++++++++++++++++++++++++--- gpg-interface.h | 8 +- log-tree.c | 8 +- pretty.c | 4 +- send-pack.c | 8 +- t/lib-gpg.sh | 28 ++ t/t4202-log.sh | 23 ++ t/t5534-push-signed.sh | 101 ++++++ t/t7031-verify-tag-signed-ssh.sh | 161 +++++++++ t/t7510-signed-commit.sh | 29 +- t/t7528-signed-commit-ssh.sh | 398 +++++++++++++++++++++ 15 files changed, 1341 insertions(+), 66 deletions(-) create mode 100755 t/t7031-verify-tag-signed-ssh.sh create mode 100755 t/t7528-signed-commit-ssh.sh
base-commit: 8463beaeb69fe0b7f651065813def4aa6827cd5d Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1041%2FFStelzer%2Fsshsign-v8 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1041/FStelzer/sshsign-v8 Pull-Request: https://github.com/git/git/pull/1041
Range-diff vs v7:
1: 91fd0159e1f = 1: b0bee197a05 ssh signing: preliminary refactoring and clean-up
2: fe98052a3ea = 2: d08327ecb25 ssh signing: add test prereqs
3: 80d2d55d22e = 3: c1e9bba8da0 ssh signing: add ssh key format and signing code
4: 83ece42e1de = 4: 8c430fc7a1b ssh signing: retrieve a default key from ssh-agent
5: 76bc9eb4079 = 5: 0864ed04670 ssh signing: provide a textual signing_key_id
6: dc092c79796 ! 6: cfd66180249 ssh signing: verify signatures using ssh-keygen
@@ gpg-interface.c: static int verify_gpg_signed_buffer(struct signature_check *sig
+ const char *line;
+ size_t trust_size;
+ char *principal;
++ struct strbuf ssh_principals_out = STRBUF_INIT;
++ struct strbuf ssh_principals_err = STRBUF_INIT;
+ struct strbuf ssh_keygen_out = STRBUF_INIT;
+ struct strbuf ssh_keygen_err = STRBUF_INIT;
+
@@ gpg-interface.c: static int verify_gpg_signed_buffer(struct signature_check *sig
+ "-f", ssh_allowed_signers,
+ "-s", buffer_file->filename.buf,
+ NULL);
-+ ret = pipe_command(&ssh_keygen, NULL, 0, &ssh_keygen_out, 0,
-+ &ssh_keygen_err, 0);
-+ if (ret && strstr(ssh_keygen_err.buf, "usage:")) {
++ 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_keygen_out.len) {
++ if (ret || !ssh_principals_out.len) {
+ /*
+ * We did not find a matching principal in the allowedSigners
+ * Check without validation
@@ gpg-interface.c: static int verify_gpg_signed_buffer(struct signature_check *sig
+ ret = -1;
+ } else {
+ /* Check every principal we found (one per line) */
-+ for (line = ssh_keygen_out.buf; *line;
++ for (line = ssh_principals_out.buf; *line;
+ line = strchrnul(line + 1, '\n')) {
+ while (*line == '\n')
+ line++;
@@ gpg-interface.c: static int verify_gpg_signed_buffer(struct signature_check *sig
+ sigc->payload = xmemdupz(payload, payload_size);
+ strbuf_stripspace(&ssh_keygen_out, 0);
+ strbuf_stripspace(&ssh_keygen_err, 0);
++ /* Add stderr outputs to show the user actual ssh-keygen errors */
++ strbuf_add(&ssh_keygen_out, ssh_principals_err.buf, ssh_principals_err.len);
+ strbuf_add(&ssh_keygen_out, ssh_keygen_err.buf, ssh_keygen_err.len);
+ sigc->output = strbuf_detach(&ssh_keygen_out, NULL);
+ sigc->gpg_status = xstrdup(sigc->output);
@@ gpg-interface.c: static int verify_gpg_signed_buffer(struct signature_check *sig
+out:
+ if (buffer_file)
+ delete_tempfile(&buffer_file);
++ strbuf_release(&ssh_principals_out);
++ strbuf_release(&ssh_principals_err);
+ strbuf_release(&ssh_keygen_out);
+ strbuf_release(&ssh_keygen_err);
+
@@ gpg-interface.c: int git_gpg_config(const char *var, const char *value, void *cb
+ if (!strcmp(var, "gpg.ssh.allowedsignersfile")) {
+ if (!value)
+ return config_error_nonbool(var);
-+ return git_config_string(&ssh_allowed_signers, var, value);
++ return git_config_pathname(&ssh_allowed_signers, var, value);
+ }
+
+ if (!strcmp(var, "gpg.ssh.revocationfile")) {
+ if (!value)
+ return config_error_nonbool(var);
-+ return git_config_string(&ssh_revocation_file, var, value);
++ return git_config_pathname(&ssh_revocation_file, var, value);
+ }
+
if (!strcmp(var, "gpg.program") || !strcmp(var, "gpg.openpgp.program"))
7: c17441566d9 ! 7: c8e21dc97f1 ssh signing: duplicate t7510 tests for commits
@@ t/t7528-signed-commit-ssh.sh (new)
+ git show --pretty=short --show-signature $commit >actual &&
+ grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual &&
+ ! grep "${GPGSSH_BAD_SIGNATURE}" actual &&
-+ grep "${KEY_NOT_TRUSTED}" actual &&
++ grep "${GPGSSH_KEY_NOT_TRUSTED}" actual &&
+ echo $commit OK || exit 1
+ done
+ )
@@ t/t7528-signed-commit-ssh.sh (new)
+ test_must_fail git verify-commit eighth-signed-alt 2>actual &&
+ grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual &&
+ ! grep "${GPGSSH_BAD_SIGNATURE}" actual &&
-+ grep "${KEY_NOT_TRUSTED}" actual
++ grep "${GPGSSH_KEY_NOT_TRUSTED}" actual
+'
+
+test_expect_success GPGSSH 'verify-commit exits success with matching minTrustLevel' '
8: 0763517d62d = 8: b66e3e0284c ssh signing: tests for logs, tags & push certs
9: a5add98197a ! 9: 07afb94ed83 ssh signing: test that gpg fails for unkown keys
@@ Metadata
Author: Fabian Stelzer <fs@gigacodes.de>
## Commit message ##
- ssh signing: test that gpg fails for unkown keys
+ ssh signing: test that gpg fails for unknown keys
Test that verify-commit/tag will fail when a gpg key is completely
unknown. To do this we have to generate a key, use it for a signature-- gitgitgadget