[PATCH v8 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen

["Fabian Stelzer via GitGitGadget" <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
   functionality

v8:

 * 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