ssh signing: support non ssh-* keytypes

The user.signingKey config for ssh signing supports either a path to a
file containing the key or for the sake of convenience a literal string
with the ssh public key. To differentiate between those two cases we
check if the first few characters contain "ssh-" which is unlikely to be
the start of a path. ssh supports other key types which are not prefixed
with "ssh-" and will currently be treated as a file path and therefore
fail to load. To remedy this we move the prefix check into its own
function and introduce the prefix `key::` for literal ssh keys. This way
we don't need to add new key types when they become available. The
existing `ssh-` prefix is retained for compatibility with current user
configs but removed from the official documentation to discourage its
use.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt
index ad78dce9ecbfc6faf633be133f8f88218844bd9a..ec9233b060a82c41dd9a01785a6f1e748ddf0ad9 100644 (file)
--- a/Documentation/config/user.txt
+++ b/Documentation/config/user.txt
@@ -36,10 +36,13 @@ user.signingKey::
        commit, you can override the default selection with this variable.
        This option is passed unchanged to gpg's --local-user parameter,
        so you may specify a key using any method that gpg supports.
-       If gpg.format is set to "ssh" this can contain the literal ssh public
-       key (e.g.: "ssh-rsa XXXXXX identifier") or a file which contains it and
-       corresponds to the private key used for signing. The private key
-       needs to be available via ssh-agent. Alternatively it can be set to
-       a file containing a private key directly. If not set git will call
-       gpg.ssh.defaultKeyCommand (e.g.: "ssh-add -L") and try to use the first
-       key available.
+       If gpg.format is set to `ssh` this can contain the path to either
+       your private ssh key or the public key when ssh-agent is used.
+       Alternatively it can contain a public key prefixed with `key::`
+       directly (e.g.: "key::ssh-rsa XXXXXX identifier"). The private key
+       needs to be available via ssh-agent. If not set git will call
+       gpg.ssh.defaultKeyCommand (e.g.: "ssh-add -L") and try to use the
+       first key available. For backward compatibility, a raw key which
+       begins with "ssh-", such as "ssh-rsa XXXXXX identifier", is treated
+       as "key::ssh-rsa XXXXXX identifier", but this form is deprecated;
+       use the `key::` form instead.

diff --git a/gpg-interface.c b/gpg-interface.c
index 3e7255a2a91103323232f55a6ae7770266595f9a..73554ea998b4b65ceb5c30662d8fb2aaf2c3edf6 100644 (file)
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -707,6 +707,21 @@ int git_gpg_config(const char *var, const char *value, void *cb)
        return 0;
 }
 
+/*
+ * Returns 1 if `string` contains a literal ssh key, 0 otherwise
+ * `key` will be set to the start of the actual key if a prefix is present.
+ */
+static int is_literal_ssh_key(const char *string, const char **key)
+{
+       if (skip_prefix(string, "key::", key))
+               return 1;
+       if (starts_with(string, "ssh-")) {
+               *key = string;
+               return 1;
+       }
+       return 0;
+}
+
 static char *get_ssh_key_fingerprint(const char *signing_key)
 {
        struct child_process ssh_keygen = CHILD_PROCESS_INIT;
@@ -714,15 +729,16 @@ static char *get_ssh_key_fingerprint(const char *signing_key)
        struct strbuf fingerprint_stdout = STRBUF_INIT;
        struct strbuf **fingerprint;
        char *fingerprint_ret;
+       const char *literal_key = NULL;
 
        /*
         * With SSH Signing this can contain a filename or a public key
         * For textual representation we usually want a fingerprint
         */
-       if (starts_with(signing_key, "ssh-")) {
+       if (is_literal_ssh_key(signing_key, &literal_key)) {
                strvec_pushl(&ssh_keygen.args, "ssh-keygen", "-lf", "-", NULL);
-               ret = pipe_command(&ssh_keygen, signing_key,
-                                  strlen(signing_key), &fingerprint_stdout, 0,
+               ret = pipe_command(&ssh_keygen, literal_key,
+                                  strlen(literal_key), &fingerprint_stdout, 0,
                                   NULL, 0);
        } else {
                strvec_pushl(&ssh_keygen.args, "ssh-keygen", "-lf",
@@ -757,6 +773,7 @@ static const char *get_default_ssh_signing_key(void)
        const char **argv;
        int n;
        char *default_key = NULL;
+       const char *literal_key = NULL;
 
        if (!ssh_default_key_command)
                die(_("either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured"));
@@ -774,7 +791,11 @@ static const char *get_default_ssh_signing_key(void)
 
        if (!ret) {
                keys = strbuf_split_max(&key_stdout, '\n', 2);
-               if (keys[0] && starts_with(keys[0]->buf, "ssh-")) {
+               if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {
+                       /*
+                        * We only use `is_literal_ssh_key` here to check validity
+                        * The prefix will be stripped when the key is used.
+                        */
                        default_key = strbuf_detach(keys[0], NULL);
                } else {
                        warning(_("gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s"),
@@ -889,19 +910,20 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
        struct tempfile *key_file = NULL, *buffer_file = NULL;
        char *ssh_signing_key_file = NULL;
        struct strbuf ssh_signature_filename = STRBUF_INIT;
+       const char *literal_key = NULL;
 
        if (!signing_key || signing_key[0] == '\0')
                return error(
                        _("user.signingkey needs to be set for ssh signing"));
 
-       if (starts_with(signing_key, "ssh-")) {
+       if (is_literal_ssh_key(signing_key, &literal_key)) {
                /* A literal ssh key */
                key_file = mks_tempfile_t(".git_signing_key_tmpXXXXXX");
                if (!key_file)
                        return error_errno(
                                _("could not create temporary file"));
-               keylen = strlen(signing_key);
-               if (write_in_full(key_file->fd, signing_key, keylen) < 0 ||
+               keylen = strlen(literal_key);
+               if (write_in_full(key_file->fd, literal_key, keylen) < 0 ||
                    close_tempfile_gently(key_file) < 0) {
                        error_errno(_("failed writing ssh signing key to '%s'"),
                                    key_file->filename.buf);

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index a3f285f5156670bb956494f2778891145339eaba..6434feb6c14a86b715522c52b71cfd701015a169 100644 (file)
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -91,6 +91,7 @@ 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_WITH_PASSPHRASE="${GNUPGHOME}/protected_ssh_signing_key"
+GPGSSH_KEY_ECDSA="${GNUPGHOME}/ecdsa_ssh_signing_key"
 GPGSSH_KEY_PASSPHRASE="super_secret"
 GPGSSH_ALLOWED_SIGNERS="${GNUPGHOME}/ssh.all_valid.allowedSignersFile"
 
@@ -119,6 +120,8 @@ 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 ecdsa -N "" -f "${GPGSSH_KEY_ECDSA}" >/dev/null
+       echo "\"principal with number 4\" $(cat "${GPGSSH_KEY_ECDSA}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
        ssh-keygen -t ed25519 -N "" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null
 '
 

diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh
index badf3ed320406ea5550e7b24b82216d147a48e80..455eafa15c0d16127f59a3dc2b3dfe2c9b9e7b46 100755 (executable)
--- a/t/t7528-signed-commit-ssh.sh
+++ b/t/t7528-signed-commit-ssh.sh
@@ -73,7 +73,29 @@ test_expect_success GPGSSH 'create signed commits' '
        git tag eleventh-signed $(cat oid) &&
        echo 12 | git commit-tree --gpg-sign="${GPGSSH_KEY_UNTRUSTED}" HEAD^{tree} >oid &&
        test_line_count = 1 oid &&
-       git tag twelfth-signed-alt $(cat oid)
+       git tag twelfth-signed-alt $(cat oid) &&
+
+       echo 13>file && test_tick && git commit -a -m thirteenth -S"${GPGSSH_KEY_ECDSA}" &&
+       git tag thirteenth-signed-ecdsa
+'
+
+test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agent' '
+       test_when_finished "test_unconfig commit.gpgsign" &&
+       test_config gpg.format ssh &&
+       eval $(ssh-agent) &&
+       test_when_finished "kill ${SSH_AGENT_PID}" &&
+       ssh-add "${GPGSSH_KEY_PRIMARY}" &&
+       echo 1 >file && git add file &&
+       git commit -a -m rsa-inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
+       echo 2 >file &&
+       test_config user.signingkey "$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
+       git commit -a -m rsa-config -S &&
+       ssh-add "${GPGSSH_KEY_ECDSA}" &&
+       echo 3 >file &&
+       git commit -a -m ecdsa-inline -S"key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
+       echo 4 >file &&
+       test_config user.signingkey "key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
+       git commit -a -m ecdsa-config -S
 '
 
 test_expect_success GPGSSH 'verify and show signatures' '