{"thread":{"id":"56928","subject":"[PATCH] ssh signing: support non ssh-* keytypes","startedAt":"2021-11-17T16:27:37Z","lastAt":"2021-11-19T15:07:21Z","messageCount":12,"participants":["Fabian Stelzer","Taylor Blau","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"441507","messageId":"20211117162727.650857-1-fs@gigacodes.de","threadId":"56928","inReplyTo":null,"subject":"[PATCH] ssh signing: support non ssh-* keytypes","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-17T16:27:27Z","receivedAt":"2021-11-17T16:27:37Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"The user.signingKey config for ssh signing supports either a path to a\nfile containing the key or for the sake of convenience a literal string\nwith the ssh public key. To differentiate between those two cases we\ncheck if the first few characters contain \"ssh-\" which is unlikely to be\nthe start of a path. ssh supports other key types which are not prefixed\nwith \"ssh-\" and will currently be treated as a file path and therefore\nfail to load. To remedy this we move the prefix check into its own\nfunction and add the other key types. \"ssh -Q key\" can be used to show a\nlist of all supported types.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n gpg-interface.c | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 3e7255a2a9..dd1df9f4ee 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -707,6 +707,16 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n+/* Determines wether key contains a literal ssh key or a path to a file */\n+static int is_literal_ssh_key(const char *key) {\n+\treturn (\n+\t\tstarts_with(key, \"ssh-\") ||\n+\t\tstarts_with(key, \"ecdsa-\") ||\n+\t\tstarts_with(key, \"sk-ssh-\") ||\n+\t\tstarts_with(key, \"sk-ecdsa-\")\n+\t);\n+}\n+\n static char *get_ssh_key_fingerprint(const char *signing_key)\n {\n \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n@@ -719,7 +729,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \t * With SSH Signing this can contain a filename or a public key\n \t * For textual representation we usually want a fingerprint\n \t */\n-\tif (starts_with(signing_key, \"ssh-\")) {\n+\tif (is_literal_ssh_key(signing_key)) {\n \t\tstrvec_pushl(&ssh_keygen.args, \"ssh-keygen\", \"-lf\", \"-\", NULL);\n \t\tret = pipe_command(&ssh_keygen, signing_key,\n \t\t\t\t   strlen(signing_key), &fingerprint_stdout, 0,\n@@ -774,7 +784,7 @@ static const char *get_default_ssh_signing_key(void)\n \n \tif (!ret) {\n \t\tkeys = strbuf_split_max(&key_stdout, '\\n', 2);\n-\t\tif (keys[0] && starts_with(keys[0]->buf, \"ssh-\")) {\n+\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf)) {\n \t\t\tdefault_key = strbuf_detach(keys[0], NULL);\n \t\t} else {\n \t\t\twarning(_(\"gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s\"),\n@@ -894,7 +904,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,\n \t\treturn error(\n \t\t\t_(\"user.signingkey needs to be set for ssh signing\"));\n \n-\tif (starts_with(signing_key, \"ssh-\")) {\n+\tif (is_literal_ssh_key(signing_key)) {\n \t\t/* A literal ssh key */\n \t\tkey_file = mks_tempfile_t(\".git_signing_key_tmpXXXXXX\");\n \t\tif (!key_file)\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\n-- \n2.31.1\n\n"},{"id":"441513","messageId":"YZVBODo2BgrXd8XK@nand.local","threadId":"56928","inReplyTo":"20211117162727.650857-1-fs@gigacodes.de","subject":"Re: [PATCH] ssh signing: support non ssh-* keytypes","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-11-17T17:51:52Z","receivedAt":"2021-11-17T17:51:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Nov 17, 2021 at 05:27:27PM +0100, Fabian Stelzer wrote:\n> The user.signingKey config for ssh signing supports either a path to a\n> file containing the key or for the sake of convenience a literal string\n> with the ssh public key. To differentiate between those two cases we\n> check if the first few characters contain \"ssh-\" which is unlikely to be\n> the start of a path. ssh supports other key types which are not prefixed\n> with \"ssh-\" and will currently be treated as a file path and therefore\n> fail to load. To remedy this we move the prefix check into its own\n> function and add the other key types. \"ssh -Q key\" can be used to show a\n> list of all supported types.\n>\n> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>\n> ---\n>  gpg-interface.c | 16 +++++++++++++---\n>  1 file changed, 13 insertions(+), 3 deletions(-)\n>\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 3e7255a2a9..dd1df9f4ee 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -707,6 +707,16 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n>  \treturn 0;\n>  }\n>\n> +/* Determines wether key contains a literal ssh key or a path to a file */\n\nNit: s/wether/whether.\n\nI had to re-read this comment before I realized that the \"or a path to a\nfile\" isn't checked by this function, but is what we assume to be true\nif this function returns 0.\n\nSo I don't think anything you wrote there is wrong, but it may be\nclearer to just say \"Returns 1 if `key` contains a literal SSH\nkey, 0 otherwise.\"\n\n> +static int is_literal_ssh_key(const char *key) {\n> +\treturn (\n> +\t\tstarts_with(key, \"ssh-\") ||\n> +\t\tstarts_with(key, \"ecdsa-\") ||\n> +\t\tstarts_with(key, \"sk-ssh-\") ||\n> +\t\tstarts_with(key, \"sk-ecdsa-\")\n> +\t);\n> +}\n\nThe outer-most parenthesis are unnecessary, but help with line wrapping.\nEqually OK would have been:\n\n    return starts_with(...) ||\n      starts_with(...) ||\n\nand so on, but it doesn't matter much one way or the other.\n\n> +\n>  static char *get_ssh_key_fingerprint(const char *signing_key)\n>  {\n>  \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n> @@ -719,7 +729,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n>  \t * With SSH Signing this can contain a filename or a public key\n>  \t * For textual representation we usually want a fingerprint\n>  \t */\n> -\tif (starts_with(signing_key, \"ssh-\")) {\n> +\tif (is_literal_ssh_key(signing_key)) {\n\nThis (and all other replacements) are straightforward and exhaustive. It\nwould be nice to see an additional test confirming that we treat, for\ne.g., literal ECDSA keys correctly.\n\nThanks,\nTaylor\n"},{"id":"441559","messageId":"xmqq4k8a2m97.fsf@gitster.g","threadId":"56928","inReplyTo":"20211117162727.650857-1-fs@gigacodes.de","subject":"Re: [PATCH] ssh signing: support non ssh-* keytypes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-18T03:09:08Z","receivedAt":"2021-11-18T03:09:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Stelzer <fs@gigacodes.de> writes:\n\n> +/* Determines wether key contains a literal ssh key or a path to a file */\n> +static int is_literal_ssh_key(const char *key) {\n> +\treturn (\n> +\t\tstarts_with(key, \"ssh-\") ||\n> +\t\tstarts_with(key, \"ecdsa-\") ||\n> +\t\tstarts_with(key, \"sk-ssh-\") ||\n> +\t\tstarts_with(key, \"sk-ecdsa-\")\n> +\t);\n> +}\n\nA more forward looking thing you could do is to \n\n (1) grandfather the convention \"any string that begins with 'ssh-'\n     is taken as a ssh literal key\".\n\n (2) refrain from spreading such an unstructured mess by picking a\n     reserved prefix, say \"ssh-key::\" and have all other kinds of\n     ssh keys use the convention.\n\nmaking the above function look more like\n\n    static int is_literal_ssh_key(const char *string, const char **key)\n    {\n\tif (skip_prefix(string, \"ssh-key::\", key)\n\t    return 1;\n\tif (starts_with(string, \"ssh-\")) {\n\t    key = string;\n\t    return 1;\n\t}\n\treturn 0;\n    }\n\nso that the caller can extract the literal key from the string that\nspecifies either the literal key or path to the file.  This will\nfutureproof us in two axis.  When SSH adds types of keys using new\nalgo, we do not have to add it to is_literal_ssh_key() function.\nAlso when another crypto suite other than GPG and SSH comes, we\nwon't repeat the \"bare 'ssh-' prefix is reserved by ssh, and\ndifferent kind in the same suite may have to consume more reserved\nprefixes\" mistake---it would make it more natural for us to pick\n\"literal keys from any variant of that new FOO crypto suite are\nwritten with 'foo-key::' prefix\" if we did so right now.  It would\nhave been better if we didn't have to do the grandfathering, but I\nam assuming that the ship has already sailed?\n\n> @@ -719,7 +729,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n>  \t * With SSH Signing this can contain a filename or a public key\n>  \t * For textual representation we usually want a fingerprint\n>  \t */\n> -\tif (starts_with(signing_key, \"ssh-\")) {\n> +\tif (is_literal_ssh_key(signing_key)) {\n> \t\tstrvec_pushl(&ssh_keygen.args, \"ssh-keygen\", \"-lf\", \"-\", NULL);\n> \t\tret = pipe_command(&ssh_keygen, signing_key,\n> \t\t\t\t   strlen(signing_key), &fingerprint_stdout, 0,\n\nThis part needs a bit of adjustment if we go the\n\"is_literal_ssh_key() is not just a boolean but is used to strip the\nprefix to signal the kind of key\" route, but the necessary\nadjustment should be trivial.\n"},{"id":"441572","messageId":"xmqqh7caynlf.fsf@gitster.g","threadId":"56928","inReplyTo":"xmqq4k8a2m97.fsf@gitster.g","subject":"Re: [PATCH] ssh signing: support non ssh-* keytypes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-18T06:39:08Z","receivedAt":"2021-11-18T06:39:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Fabian Stelzer <fs@gigacodes.de> writes:\n>\n>> +/* Determines wether key contains a literal ssh key or a path to a file */\n>> +static int is_literal_ssh_key(const char *key) {\n>> +\treturn (\n>> +\t\tstarts_with(key, \"ssh-\") ||\n>> +\t\tstarts_with(key, \"ecdsa-\") ||\n>> +\t\tstarts_with(key, \"sk-ssh-\") ||\n>> +\t\tstarts_with(key, \"sk-ecdsa-\")\n>> +\t);\n>> +}\n>\n> A more forward looking thing you could do is to \n>\n>  (1) grandfather the convention \"any string that begins with 'ssh-'\n>      is taken as a ssh literal key\".\n>\n>  (2) refrain from spreading such an unstructured mess by picking a\n>      reserved prefix, say \"ssh-key::\" and have all other kinds of\n>      ssh keys use the convention.\n>\n> making the above function look more like\n>\n>     static int is_literal_ssh_key(const char *string, const char **key)\n>     {\n> \tif (skip_prefix(string, \"ssh-key::\", key)\n> \t    return 1;\n> \tif (starts_with(string, \"ssh-\")) {\n> \t    key = string;\n> \t    return 1;\n> \t}\n> \treturn 0;\n>     }\n\nGiven that this ONLY gets called from ssh codepath, I think the\nspecial prefix can just be \"key::\", and when a new crypto suite\nis introduced to sit next to GPG and SSH, presumably the code\nstructure to support it will be similar to that of ssh's, and it\ncan also use \"key::\" prefix for their literal keys.  That design\nmay be cleaner.\n\nThanks.\n"},{"id":"441609","messageId":"20211118151607.3mgkptt33ktrb2eh@fs","threadId":"56928","inReplyTo":"xmqqh7caynlf.fsf@gitster.g","subject":"Re: [PATCH] ssh signing: support non ssh-* keytypes","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-18T15:16:07Z","receivedAt":"2021-11-18T15:16:12Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 17.11.2021 22:39, Junio C Hamano wrote:\n>Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Fabian Stelzer <fs@gigacodes.de> writes:\n>>\n>>> +/* Determines wether key contains a literal ssh key or a path to a file */\n>>> +static int is_literal_ssh_key(const char *key) {\n>>> +\treturn (\n>>> +\t\tstarts_with(key, \"ssh-\") ||\n>>> +\t\tstarts_with(key, \"ecdsa-\") ||\n>>> +\t\tstarts_with(key, \"sk-ssh-\") ||\n>>> +\t\tstarts_with(key, \"sk-ecdsa-\")\n>>> +\t);\n>>> +}\n>>\n>> A more forward looking thing you could do is to\n>>\n>>  (1) grandfather the convention \"any string that begins with 'ssh-'\n>>      is taken as a ssh literal key\".\n>>\n>>  (2) refrain from spreading such an unstructured mess by picking a\n>>      reserved prefix, say \"ssh-key::\" and have all other kinds of\n>>      ssh keys use the convention.\n>>\n>> making the above function look more like\n>>\n>>     static int is_literal_ssh_key(const char *string, const char **key)\n>>     {\n>> \tif (skip_prefix(string, \"ssh-key::\", key)\n>> \t    return 1;\n>> \tif (starts_with(string, \"ssh-\")) {\n>> \t    key = string;\n>> \t    return 1;\n>> \t}\n>> \treturn 0;\n>>     }\n>\n>Given that this ONLY gets called from ssh codepath, I think the\n>special prefix can just be \"key::\", and when a new crypto suite\n>is introduced to sit next to GPG and SSH, presumably the code\n>structure to support it will be similar to that of ssh's, and it\n>can also use \"key::\" prefix for their literal keys.  That design\n>may be cleaner.\n>\n>Thanks.\n\nThanks both for your review. I will use the key:: suggestion and also\nadd tests for this. For now i guess we will have to keep the ssh- since\nit's already out there :/ Will reroll soon.\n\nFabian\n"},{"id":"441646","messageId":"20211118171411.147568-1-fs@gigacodes.de","threadId":"56928","inReplyTo":"20211117162727.650857-1-fs@gigacodes.de","subject":"[PATCH v2 1/2] ssh signing: support non ssh-* keytypes","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-18T17:14:10Z","receivedAt":"2021-11-18T17:14:24Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"The user.signingKey config for ssh signing supports either a path to a\nfile containing the key or for the sake of convenience a literal string\nwith the ssh public key. To differentiate between those two cases we\ncheck if the first few characters contain \"ssh-\" which is unlikely to be\nthe start of a path. ssh supports other key types which are not prefixed\nwith \"ssh-\" and will currently be treated as a file path and therefore\nfail to load. To remedy this we move the prefix check into its own\nfunction and introduce the prefix `key::` for literal ssh keys. This way\nwe don't need to add new key types when they become available. The\nexisting `ssh-` prefix is retained for compatibility with current user\nconfigs but removed from the official documentation to discourage its\nuse.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\nWhen writing the tests for this i remembered why we had none for literal\nkeys. Those need a running ssh-agent with the private key present.\nPlease let me know if the tests provide a safer way to start the\nadditional agent and make sure it gets cleaned up correctly. I tried\ngrepping for spawn,kill and similar things but did not find much else.\n\n Documentation/config/user.txt | 14 +++++++-------\n gpg-interface.c               | 36 ++++++++++++++++++++++++++++-------\n t/lib-gpg.sh                  |  3 +++\n t/t7528-signed-commit-ssh.sh  | 24 ++++++++++++++++++++++-\n 4 files changed, 62 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/config/user.txt b/Documentation/config/user.txt\nindex ad78dce9ec..4de700f651 100644\n--- a/Documentation/config/user.txt\n+++ b/Documentation/config/user.txt\n@@ -36,10 +36,10 @@ user.signingKey::\n \tcommit, you can override the default selection with this variable.\n \tThis option is passed unchanged to gpg's --local-user parameter,\n \tso you may specify a key using any method that gpg supports.\n-\tIf gpg.format is set to \"ssh\" this can contain the literal ssh public\n-\tkey (e.g.: \"ssh-rsa XXXXXX identifier\") or a file which contains it and\n-\tcorresponds to the private key used for signing. The private key\n-\tneeds to be available via ssh-agent. Alternatively it can be set to\n-\ta file containing a private key directly. If not set git will call\n-\tgpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the first\n-\tkey available.\n+\tIf gpg.format is set to `ssh` this can contain the path to either\n+\tyour private ssh key or the public key when ssh-agent is used.\n+\tAlternatively it can contain a public key prefixed with `key::`\n+\tdirectly (e.g.: \"key::ssh-rsa XXXXXX identifier\"). The private key\n+\tneeds to be available via ssh-agent. If not set git will call\n+\tgpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the\n+\tfirst key available.\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 3e7255a2a9..73554ea998 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -707,6 +707,21 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n+/*\n+ * Returns 1 if `string` contains a literal ssh key, 0 otherwise\n+ * `key` will be set to the start of the actual key if a prefix is present.\n+ */\n+static int is_literal_ssh_key(const char *string, const char **key)\n+{\n+\tif (skip_prefix(string, \"key::\", key))\n+\t\treturn 1;\n+\tif (starts_with(string, \"ssh-\")) {\n+\t\t*key = string;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static char *get_ssh_key_fingerprint(const char *signing_key)\n {\n \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n@@ -714,15 +729,16 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \tstruct strbuf fingerprint_stdout = STRBUF_INIT;\n \tstruct strbuf **fingerprint;\n \tchar *fingerprint_ret;\n+\tconst char *literal_key = NULL;\n \n \t/*\n \t * With SSH Signing this can contain a filename or a public key\n \t * For textual representation we usually want a fingerprint\n \t */\n-\tif (starts_with(signing_key, \"ssh-\")) {\n+\tif (is_literal_ssh_key(signing_key, &literal_key)) {\n \t\tstrvec_pushl(&ssh_keygen.args, \"ssh-keygen\", \"-lf\", \"-\", NULL);\n-\t\tret = pipe_command(&ssh_keygen, signing_key,\n-\t\t\t\t   strlen(signing_key), &fingerprint_stdout, 0,\n+\t\tret = pipe_command(&ssh_keygen, literal_key,\n+\t\t\t\t   strlen(literal_key), &fingerprint_stdout, 0,\n \t\t\t\t   NULL, 0);\n \t} else {\n \t\tstrvec_pushl(&ssh_keygen.args, \"ssh-keygen\", \"-lf\",\n@@ -757,6 +773,7 @@ static const char *get_default_ssh_signing_key(void)\n \tconst char **argv;\n \tint n;\n \tchar *default_key = NULL;\n+\tconst char *literal_key = NULL;\n \n \tif (!ssh_default_key_command)\n \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n@@ -774,7 +791,11 @@ static const char *get_default_ssh_signing_key(void)\n \n \tif (!ret) {\n \t\tkeys = strbuf_split_max(&key_stdout, '\\n', 2);\n-\t\tif (keys[0] && starts_with(keys[0]->buf, \"ssh-\")) {\n+\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n+\t\t\t/*\n+\t\t\t * We only use `is_literal_ssh_key` here to check validity\n+\t\t\t * The prefix will be stripped when the key is used.\n+\t\t\t */\n \t\t\tdefault_key = strbuf_detach(keys[0], NULL);\n \t\t} else {\n \t\t\twarning(_(\"gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s\"),\n@@ -889,19 +910,20 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,\n \tstruct tempfile *key_file = NULL, *buffer_file = NULL;\n \tchar *ssh_signing_key_file = NULL;\n \tstruct strbuf ssh_signature_filename = STRBUF_INIT;\n+\tconst char *literal_key = NULL;\n \n \tif (!signing_key || signing_key[0] == '\\0')\n \t\treturn error(\n \t\t\t_(\"user.signingkey needs to be set for ssh signing\"));\n \n-\tif (starts_with(signing_key, \"ssh-\")) {\n+\tif (is_literal_ssh_key(signing_key, &literal_key)) {\n \t\t/* A literal ssh key */\n \t\tkey_file = mks_tempfile_t(\".git_signing_key_tmpXXXXXX\");\n \t\tif (!key_file)\n \t\t\treturn error_errno(\n \t\t\t\t_(\"could not create temporary file\"));\n-\t\tkeylen = strlen(signing_key);\n-\t\tif (write_in_full(key_file->fd, signing_key, keylen) < 0 ||\n+\t\tkeylen = strlen(literal_key);\n+\t\tif (write_in_full(key_file->fd, literal_key, keylen) < 0 ||\n \t\t    close_tempfile_gently(key_file) < 0) {\n \t\t\terror_errno(_(\"failed writing ssh signing key to '%s'\"),\n \t\t\t\t    key_file->filename.buf);\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex a3f285f515..6434feb6c1 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -91,6 +91,7 @@ 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_WITH_PASSPHRASE=\"${GNUPGHOME}/protected_ssh_signing_key\"\n+GPGSSH_KEY_ECDSA=\"${GNUPGHOME}/ecdsa_ssh_signing_key\"\n GPGSSH_KEY_PASSPHRASE=\"super_secret\"\n GPGSSH_ALLOWED_SIGNERS=\"${GNUPGHOME}/ssh.all_valid.allowedSignersFile\"\n \n@@ -119,6 +120,8 @@ 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 ecdsa -N \"\" -f \"${GPGSSH_KEY_ECDSA}\" >/dev/null\n+\techo \"\\\"principal with number 4\\\" $(cat \"${GPGSSH_KEY_ECDSA}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n \tssh-keygen -t ed25519 -N \"\" -f \"${GPGSSH_KEY_UNTRUSTED}\" >/dev/null\n '\n \ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex badf3ed320..455eafa15c 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -73,7 +73,29 @@ test_expect_success GPGSSH 'create signed commits' '\n \tgit tag eleventh-signed $(cat oid) &&\n \techo 12 | git commit-tree --gpg-sign=\"${GPGSSH_KEY_UNTRUSTED}\" HEAD^{tree} >oid &&\n \ttest_line_count = 1 oid &&\n-\tgit tag twelfth-signed-alt $(cat oid)\n+\tgit tag twelfth-signed-alt $(cat oid) &&\n+\n+\techo 13>file && test_tick && git commit -a -m thirteenth -S\"${GPGSSH_KEY_ECDSA}\" &&\n+\tgit tag thirteenth-signed-ecdsa\n+'\n+\n+test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agent' '\n+\ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n+\ttest_config gpg.format ssh &&\n+\teval $(ssh-agent) &&\n+\ttest_when_finished \"kill ${SSH_AGENT_PID}\" &&\n+\tssh-add \"${GPGSSH_KEY_PRIMARY}\" &&\n+\techo 1 >file && git add file &&\n+\tgit commit -a -m rsa-inline -S\"$(cat \"${GPGSSH_KEY_PRIMARY}.pub\")\" &&\n+\techo 2 >file &&\n+\ttest_config user.signingkey \"$(cat \"${GPGSSH_KEY_PRIMARY}.pub\")\" &&\n+\tgit commit -a -m rsa-config -S &&\n+\tssh-add \"${GPGSSH_KEY_ECDSA}\" &&\n+\techo 3 >file &&\n+\tgit commit -a -m ecdsa-inline -S\"key::$(cat \"${GPGSSH_KEY_ECDSA}.pub\")\" &&\n+\techo 4 >file &&\n+\ttest_config user.signingkey \"key::$(cat \"${GPGSSH_KEY_ECDSA}.pub\")\" &&\n+\tgit commit -a -m ecdsa-config -S\n '\n \n test_expect_success GPGSSH 'verify and show signatures' '\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\n-- \n2.31.1\n\n"},{"id":"441647","messageId":"20211118171411.147568-2-fs@gigacodes.de","threadId":"56928","inReplyTo":"20211118171411.147568-1-fs@gigacodes.de","subject":"[PATCH v2 2/2] ssh signing: make sign/amend test more resilient","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-18T17:14:11Z","receivedAt":"2021-11-18T17:14:29Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"The test `amending already signed commit` is using git checkout to\nselect a specific commit to amend. In case an earlier test fails and\nleaves behind a dirty index/worktree this test would fail as well.\nUsing `checkout -f` will avoid interference by most other tests.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n t/t7510-signed-commit.sh     | 2 +-\n t/t7528-signed-commit-ssh.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex d65a0171f2..9882b69ae2 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -228,7 +228,7 @@ test_expect_success GPG 'detect fudged signature with NUL' '\n '\n \n test_expect_success GPG 'amending already signed commit' '\n-\tgit checkout fourth-signed^0 &&\n+\tgit checkout -f fourth-signed^0 &&\n \tgit commit --amend -S --no-edit &&\n \tgit verify-commit HEAD &&\n \tgit show -s --show-signature HEAD >actual &&\ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex 455eafa15c..0ec5a6d764 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -239,7 +239,7 @@ test_expect_success GPGSSH 'amending already signed commit' '\n \ttest_config gpg.format ssh &&\n \ttest_config user.signingkey \"${GPGSSH_KEY_PRIMARY}\" &&\n \ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n-\tgit checkout fourth-signed^0 &&\n+\tgit checkout -f fourth-signed^0 &&\n \tgit commit --amend -S --no-edit &&\n \tgit verify-commit HEAD &&\n \tgit show -s --show-signature HEAD >actual &&\n-- \n2.31.1\n\n"},{"id":"441668","messageId":"CAPig+cTVX5yYp-1eUjCgj6aox9vYpzm+JFvson37M0R_pnxRvg@mail.gmail.com","threadId":"56928","inReplyTo":"20211118171411.147568-1-fs@gigacodes.de","subject":"Re: [PATCH v2 1/2] ssh signing: support non ssh-* keytypes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-11-18T22:14:16Z","receivedAt":"2021-11-18T22:14:31Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 18, 2021 at 12:14 PM Fabian Stelzer <fs@gigacodes.de> wrote:\n> The user.signingKey config for ssh signing supports either a path to a\n> file containing the key or for the sake of convenience a literal string\n> with the ssh public key. To differentiate between those two cases we\n> check if the first few characters contain \"ssh-\" which is unlikely to be\n> the start of a path. ssh supports other key types which are not prefixed\n> with \"ssh-\" and will currently be treated as a file path and therefore\n> fail to load. To remedy this we move the prefix check into its own\n> function and introduce the prefix `key::` for literal ssh keys. This way\n> we don't need to add new key types when they become available. The\n> existing `ssh-` prefix is retained for compatibility with current user\n> configs but removed from the official documentation to discourage its\n> use.\n\nI think we usually avoid removing documentation for something which is\nstill supported (even if deprecated) for the very real reason that\npeople will still encounter the old form in the wild, whether in\nconfiguration files, in blogs, or elsewhere, and may be perplexed to\ndiscover that the form is not documented (thus not understand how or\nwhy it seems to be working). Instead, we can discourage its use by\nmentioning clearly that it is deprecated and that `key::` should be\nused instead.\n\n> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>\n> ---\n> diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt\n> @@ -36,10 +36,10 @@ user.signingKey::\n>         This option is passed unchanged to gpg's --local-user parameter,\n>         so you may specify a key using any method that gpg supports.\n> -       If gpg.format is set to \"ssh\" this can contain the literal ssh public\n> -       key (e.g.: \"ssh-rsa XXXXXX identifier\") or a file which contains it and\n> -       corresponds to the private key used for signing. The private key\n> -       needs to be available via ssh-agent. Alternatively it can be set to\n> -       a file containing a private key directly. If not set git will call\n> -       gpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the first\n> -       key available.\n> +       If gpg.format is set to `ssh` this can contain the path to either\n> +       your private ssh key or the public key when ssh-agent is used.\n> +       Alternatively it can contain a public key prefixed with `key::`\n> +       directly (e.g.: \"key::ssh-rsa XXXXXX identifier\"). The private key\n> +       needs to be available via ssh-agent. If not set git will call\n> +       gpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the\n> +       first key available.\n\nThus, perhaps this text could end with:\n\n    For backward compatibility, a raw key which begins with \"ssh-\",\n    such as \"ssh-rsa XXXXXX identifier\", is treated as \"key::ssh-rsa\n    XXXXXX identifier\", but this form is deprecated; use the `key::`\n    form instead.\n"},{"id":"441707","messageId":"20211119090549.uggwmaxjq3yg3fyj@fs","threadId":"56928","inReplyTo":"CAPig+cTVX5yYp-1eUjCgj6aox9vYpzm+JFvson37M0R_pnxRvg@mail.gmail.com","subject":"Re: [PATCH v2 1/2] ssh signing: support non ssh-* keytypes","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-19T09:05:49Z","receivedAt":"2021-11-19T09:05:54Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 18.11.2021 17:14, Eric Sunshine wrote:\n>On Thu, Nov 18, 2021 at 12:14 PM Fabian Stelzer <fs@gigacodes.de> wrote:\n>> The user.signingKey config for ssh signing supports either a path to a\n>> file containing the key or for the sake of convenience a literal string\n>> with the ssh public key. To differentiate between those two cases we\n>> check if the first few characters contain \"ssh-\" which is unlikely to be\n>> the start of a path. ssh supports other key types which are not prefixed\n>> with \"ssh-\" and will currently be treated as a file path and therefore\n>> fail to load. To remedy this we move the prefix check into its own\n>> function and introduce the prefix `key::` for literal ssh keys. This way\n>> we don't need to add new key types when they become available. The\n>> existing `ssh-` prefix is retained for compatibility with current user\n>> configs but removed from the official documentation to discourage its\n>> use.\n>\n>I think we usually avoid removing documentation for something which is\n>still supported (even if deprecated) for the very real reason that\n>people will still encounter the old form in the wild, whether in\n>configuration files, in blogs, or elsewhere, and may be perplexed to\n>discover that the form is not documented (thus not understand how or\n>why it seems to be working). Instead, we can discourage its use by\n>mentioning clearly that it is deprecated and that `key::` should be\n>used instead.\n>\n\nI thought since this only existed in the docs since the 2.34\nrelease a few days ago i could get away with it ;)\nBut since we keep the support for `ssh-` in the code we should document\nit as such. I'll add your suggestion below to it.\nThanks for your help.\n\n>> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>\n>> ---\n>> diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt\n>> @@ -36,10 +36,10 @@ user.signingKey::\n>>         This option is passed unchanged to gpg's --local-user parameter,\n>>         so you may specify a key using any method that gpg supports.\n>> -       If gpg.format is set to \"ssh\" this can contain the literal ssh public\n>> -       key (e.g.: \"ssh-rsa XXXXXX identifier\") or a file which contains it and\n>> -       corresponds to the private key used for signing. The private key\n>> -       needs to be available via ssh-agent. Alternatively it can be set to\n>> -       a file containing a private key directly. If not set git will call\n>> -       gpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the first\n>> -       key available.\n>> +       If gpg.format is set to `ssh` this can contain the path to either\n>> +       your private ssh key or the public key when ssh-agent is used.\n>> +       Alternatively it can contain a public key prefixed with `key::`\n>> +       directly (e.g.: \"key::ssh-rsa XXXXXX identifier\"). The private key\n>> +       needs to be available via ssh-agent. If not set git will call\n>> +       gpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the\n>> +       first key available.\n>\n>Thus, perhaps this text could end with:\n>\n>    For backward compatibility, a raw key which begins with \"ssh-\",\n>    such as \"ssh-rsa XXXXXX identifier\", is treated as \"key::ssh-rsa\n>    XXXXXX identifier\", but this form is deprecated; use the `key::`\n>    form instead.\n"},{"id":"441745","messageId":"20211119150707.3924636-1-fs@gigacodes.de","threadId":"56928","inReplyTo":"20211117162727.650857-1-fs@gigacodes.de","subject":"[PATCH v3 0/2] ssh signing: support non ssh-* keytypes","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-19T15:07:05Z","receivedAt":"2021-11-19T15:07:15Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"support generic ssh keytypes using a `key::` prefix instead of relying\non ssh- prefixes.\n\nchanges since v2:\n - no longer hide that we still support `ssh-` prefixes for literal\n   keys.\n\nFabian Stelzer (2):\n  ssh signing: support non ssh-* keytypes\n  ssh signing: make sign/amend test more resilient\n\n Documentation/config/user.txt | 17 ++++++++++-------\n gpg-interface.c               | 36 ++++++++++++++++++++++++++++-------\n t/lib-gpg.sh                  |  3 +++\n t/t7510-signed-commit.sh      |  2 +-\n t/t7528-signed-commit-ssh.sh  | 26 +++++++++++++++++++++++--\n 5 files changed, 67 insertions(+), 17 deletions(-)\n\nRange-diff against v2:\n1:  ea032f98f0 ! 1:  865a32d37c ssh signing: support non ssh-* keytypes\n    @@ Documentation/config/user.txt: user.signingKey::\n     +\tdirectly (e.g.: \"key::ssh-rsa XXXXXX identifier\"). The private key\n     +\tneeds to be available via ssh-agent. If not set git will call\n     +\tgpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the\n    -+\tfirst key available.\n    ++\tfirst key available. For backward compatibility, a raw key which\n    ++\tbegins with \"ssh-\", such as \"ssh-rsa XXXXXX identifier\", is treated\n    ++\tas \"key::ssh-rsa XXXXXX identifier\", but this form is deprecated;\n    ++\tuse the `key::` form instead.\n     \n      ## gpg-interface.c ##\n     @@ gpg-interface.c: int git_gpg_config(const char *var, const char *value, void *cb)\n2:  5054ff0e76 = 2:  868cb7f524 ssh signing: make sign/amend test more resilient\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\n-- \n2.31.1\n\n"},{"id":"441746","messageId":"20211119150707.3924636-2-fs@gigacodes.de","threadId":"56928","inReplyTo":"20211119150707.3924636-1-fs@gigacodes.de","subject":"[PATCH v3 1/2] ssh signing: support non ssh-* keytypes","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-19T15:07:06Z","receivedAt":"2021-11-19T15:07:17Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"The user.signingKey config for ssh signing supports either a path to a\nfile containing the key or for the sake of convenience a literal string\nwith the ssh public key. To differentiate between those two cases we\ncheck if the first few characters contain \"ssh-\" which is unlikely to be\nthe start of a path. ssh supports other key types which are not prefixed\nwith \"ssh-\" and will currently be treated as a file path and therefore\nfail to load. To remedy this we move the prefix check into its own\nfunction and introduce the prefix `key::` for literal ssh keys. This way\nwe don't need to add new key types when they become available. The\nexisting `ssh-` prefix is retained for compatibility with current user\nconfigs but removed from the official documentation to discourage its\nuse.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n Documentation/config/user.txt | 17 ++++++++++-------\n gpg-interface.c               | 36 ++++++++++++++++++++++++++++-------\n t/lib-gpg.sh                  |  3 +++\n t/t7528-signed-commit-ssh.sh  | 24 ++++++++++++++++++++++-\n 4 files changed, 65 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/config/user.txt b/Documentation/config/user.txt\nindex ad78dce9ec..ec9233b060 100644\n--- a/Documentation/config/user.txt\n+++ b/Documentation/config/user.txt\n@@ -36,10 +36,13 @@ user.signingKey::\n \tcommit, you can override the default selection with this variable.\n \tThis option is passed unchanged to gpg's --local-user parameter,\n \tso you may specify a key using any method that gpg supports.\n-\tIf gpg.format is set to \"ssh\" this can contain the literal ssh public\n-\tkey (e.g.: \"ssh-rsa XXXXXX identifier\") or a file which contains it and\n-\tcorresponds to the private key used for signing. The private key\n-\tneeds to be available via ssh-agent. Alternatively it can be set to\n-\ta file containing a private key directly. If not set git will call\n-\tgpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the first\n-\tkey available.\n+\tIf gpg.format is set to `ssh` this can contain the path to either\n+\tyour private ssh key or the public key when ssh-agent is used.\n+\tAlternatively it can contain a public key prefixed with `key::`\n+\tdirectly (e.g.: \"key::ssh-rsa XXXXXX identifier\"). The private key\n+\tneeds to be available via ssh-agent. If not set git will call\n+\tgpg.ssh.defaultKeyCommand (e.g.: \"ssh-add -L\") and try to use the\n+\tfirst key available. For backward compatibility, a raw key which\n+\tbegins with \"ssh-\", such as \"ssh-rsa XXXXXX identifier\", is treated\n+\tas \"key::ssh-rsa XXXXXX identifier\", but this form is deprecated;\n+\tuse the `key::` form instead.\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 3e7255a2a9..73554ea998 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -707,6 +707,21 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n+/*\n+ * Returns 1 if `string` contains a literal ssh key, 0 otherwise\n+ * `key` will be set to the start of the actual key if a prefix is present.\n+ */\n+static int is_literal_ssh_key(const char *string, const char **key)\n+{\n+\tif (skip_prefix(string, \"key::\", key))\n+\t\treturn 1;\n+\tif (starts_with(string, \"ssh-\")) {\n+\t\t*key = string;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static char *get_ssh_key_fingerprint(const char *signing_key)\n {\n \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n@@ -714,15 +729,16 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \tstruct strbuf fingerprint_stdout = STRBUF_INIT;\n \tstruct strbuf **fingerprint;\n \tchar *fingerprint_ret;\n+\tconst char *literal_key = NULL;\n \n \t/*\n \t * With SSH Signing this can contain a filename or a public key\n \t * For textual representation we usually want a fingerprint\n \t */\n-\tif (starts_with(signing_key, \"ssh-\")) {\n+\tif (is_literal_ssh_key(signing_key, &literal_key)) {\n \t\tstrvec_pushl(&ssh_keygen.args, \"ssh-keygen\", \"-lf\", \"-\", NULL);\n-\t\tret = pipe_command(&ssh_keygen, signing_key,\n-\t\t\t\t   strlen(signing_key), &fingerprint_stdout, 0,\n+\t\tret = pipe_command(&ssh_keygen, literal_key,\n+\t\t\t\t   strlen(literal_key), &fingerprint_stdout, 0,\n \t\t\t\t   NULL, 0);\n \t} else {\n \t\tstrvec_pushl(&ssh_keygen.args, \"ssh-keygen\", \"-lf\",\n@@ -757,6 +773,7 @@ static const char *get_default_ssh_signing_key(void)\n \tconst char **argv;\n \tint n;\n \tchar *default_key = NULL;\n+\tconst char *literal_key = NULL;\n \n \tif (!ssh_default_key_command)\n \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n@@ -774,7 +791,11 @@ static const char *get_default_ssh_signing_key(void)\n \n \tif (!ret) {\n \t\tkeys = strbuf_split_max(&key_stdout, '\\n', 2);\n-\t\tif (keys[0] && starts_with(keys[0]->buf, \"ssh-\")) {\n+\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n+\t\t\t/*\n+\t\t\t * We only use `is_literal_ssh_key` here to check validity\n+\t\t\t * The prefix will be stripped when the key is used.\n+\t\t\t */\n \t\t\tdefault_key = strbuf_detach(keys[0], NULL);\n \t\t} else {\n \t\t\twarning(_(\"gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s\"),\n@@ -889,19 +910,20 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,\n \tstruct tempfile *key_file = NULL, *buffer_file = NULL;\n \tchar *ssh_signing_key_file = NULL;\n \tstruct strbuf ssh_signature_filename = STRBUF_INIT;\n+\tconst char *literal_key = NULL;\n \n \tif (!signing_key || signing_key[0] == '\\0')\n \t\treturn error(\n \t\t\t_(\"user.signingkey needs to be set for ssh signing\"));\n \n-\tif (starts_with(signing_key, \"ssh-\")) {\n+\tif (is_literal_ssh_key(signing_key, &literal_key)) {\n \t\t/* A literal ssh key */\n \t\tkey_file = mks_tempfile_t(\".git_signing_key_tmpXXXXXX\");\n \t\tif (!key_file)\n \t\t\treturn error_errno(\n \t\t\t\t_(\"could not create temporary file\"));\n-\t\tkeylen = strlen(signing_key);\n-\t\tif (write_in_full(key_file->fd, signing_key, keylen) < 0 ||\n+\t\tkeylen = strlen(literal_key);\n+\t\tif (write_in_full(key_file->fd, literal_key, keylen) < 0 ||\n \t\t    close_tempfile_gently(key_file) < 0) {\n \t\t\terror_errno(_(\"failed writing ssh signing key to '%s'\"),\n \t\t\t\t    key_file->filename.buf);\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex a3f285f515..6434feb6c1 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -91,6 +91,7 @@ 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_WITH_PASSPHRASE=\"${GNUPGHOME}/protected_ssh_signing_key\"\n+GPGSSH_KEY_ECDSA=\"${GNUPGHOME}/ecdsa_ssh_signing_key\"\n GPGSSH_KEY_PASSPHRASE=\"super_secret\"\n GPGSSH_ALLOWED_SIGNERS=\"${GNUPGHOME}/ssh.all_valid.allowedSignersFile\"\n \n@@ -119,6 +120,8 @@ 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 ecdsa -N \"\" -f \"${GPGSSH_KEY_ECDSA}\" >/dev/null\n+\techo \"\\\"principal with number 4\\\" $(cat \"${GPGSSH_KEY_ECDSA}.pub\")\" >> \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n \tssh-keygen -t ed25519 -N \"\" -f \"${GPGSSH_KEY_UNTRUSTED}\" >/dev/null\n '\n \ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex badf3ed320..455eafa15c 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -73,7 +73,29 @@ test_expect_success GPGSSH 'create signed commits' '\n \tgit tag eleventh-signed $(cat oid) &&\n \techo 12 | git commit-tree --gpg-sign=\"${GPGSSH_KEY_UNTRUSTED}\" HEAD^{tree} >oid &&\n \ttest_line_count = 1 oid &&\n-\tgit tag twelfth-signed-alt $(cat oid)\n+\tgit tag twelfth-signed-alt $(cat oid) &&\n+\n+\techo 13>file && test_tick && git commit -a -m thirteenth -S\"${GPGSSH_KEY_ECDSA}\" &&\n+\tgit tag thirteenth-signed-ecdsa\n+'\n+\n+test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agent' '\n+\ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n+\ttest_config gpg.format ssh &&\n+\teval $(ssh-agent) &&\n+\ttest_when_finished \"kill ${SSH_AGENT_PID}\" &&\n+\tssh-add \"${GPGSSH_KEY_PRIMARY}\" &&\n+\techo 1 >file && git add file &&\n+\tgit commit -a -m rsa-inline -S\"$(cat \"${GPGSSH_KEY_PRIMARY}.pub\")\" &&\n+\techo 2 >file &&\n+\ttest_config user.signingkey \"$(cat \"${GPGSSH_KEY_PRIMARY}.pub\")\" &&\n+\tgit commit -a -m rsa-config -S &&\n+\tssh-add \"${GPGSSH_KEY_ECDSA}\" &&\n+\techo 3 >file &&\n+\tgit commit -a -m ecdsa-inline -S\"key::$(cat \"${GPGSSH_KEY_ECDSA}.pub\")\" &&\n+\techo 4 >file &&\n+\ttest_config user.signingkey \"key::$(cat \"${GPGSSH_KEY_ECDSA}.pub\")\" &&\n+\tgit commit -a -m ecdsa-config -S\n '\n \n test_expect_success GPGSSH 'verify and show signatures' '\n-- \n2.31.1\n\n"},{"id":"441747","messageId":"20211119150707.3924636-3-fs@gigacodes.de","threadId":"56928","inReplyTo":"20211119150707.3924636-1-fs@gigacodes.de","subject":"[PATCH v3 2/2] ssh signing: make sign/amend test more resilient","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-11-19T15:07:07Z","receivedAt":"2021-11-19T15:07:21Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"The test `amending already signed commit` is using git checkout to\nselect a specific commit to amend. In case an earlier test fails and\nleaves behind a dirty index/worktree this test would fail as well.\nUsing `checkout -f` will avoid interference by most other tests.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n t/t7510-signed-commit.sh     | 2 +-\n t/t7528-signed-commit-ssh.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex d65a0171f2..9882b69ae2 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -228,7 +228,7 @@ test_expect_success GPG 'detect fudged signature with NUL' '\n '\n \n test_expect_success GPG 'amending already signed commit' '\n-\tgit checkout fourth-signed^0 &&\n+\tgit checkout -f fourth-signed^0 &&\n \tgit commit --amend -S --no-edit &&\n \tgit verify-commit HEAD &&\n \tgit show -s --show-signature HEAD >actual &&\ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex 455eafa15c..0ec5a6d764 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -239,7 +239,7 @@ test_expect_success GPGSSH 'amending already signed commit' '\n \ttest_config gpg.format ssh &&\n \ttest_config user.signingkey \"${GPGSSH_KEY_PRIMARY}\" &&\n \ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n-\tgit checkout fourth-signed^0 &&\n+\tgit checkout -f fourth-signed^0 &&\n \tgit commit --amend -S --no-edit &&\n \tgit verify-commit HEAD &&\n \tgit show -s --show-signature HEAD >actual &&\n-- \n2.31.1\n\n"}]}