Skip to contentSkip to conversation

ssh signing: Support ECDSA as literal SSH keys

PATCH In review Andy Lindeman via GitGitGadget 4 participants 9 messages May 30, 2022 – Jun 13, 2022 Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ssh signing: Support ECDSA as literal SSH keys

From: Andy Lindeman <[email protected]>

Keys generated using ssh-keygen -t ecdsa or similar are being rejected as literal SSH keys because the prefix is ecdsa-sha2-nistp256, ecdsa-sha2-nistp384 or ecdsa-sha2-nistp521.

This was acknowledged as an issue [1] in the past, but hasn't yet been fixed.

[1]: https://github.com/git/git/pull/1041#issuecomment-971425601

---
ssh signing: Support ECDSA as literal SSH keys

Keys generated using ssh-keygen -t ecdsa or similar will currently be rejected as literal SSH keys because the prefix is ecdsa-sha2-nistp256, ecdsa-sha2-nistp384 or ecdsa-sha2-nistp521.

This was acknowledged as an issue in the past, but hasn't yet been fixed.

https://github.com/git/git/pull/1041#issuecomment-971425601

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1272%2Falindeman%2Fecdsa-sha2-keys-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1272/alindeman/ecdsa-sha2-keys-v1
Pull-Request: https://github.com/git/git/pull/1272

 gpg-interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
gpg-interface.c +1 -1
index 280f1fa1a58..086bd03b51d 100644
@@ -779,7 +779,7 @@ 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-")) {
+	if (starts_with(string, "ssh-") || starts_with(string, "ecdsa-sha2-")) {
 		*key = string;
 		return 1;
 	}

base-commit: 8ddf593a250e07d388059f7e3f471078e1d2ed5c

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 8 lines · Andy Lindeman via GitGitGadget

From: Andy Lindeman <[email protected]>

Keys generated using ssh-keygen -t ecdsa or similar are being rejected as literal SSH keys because the prefix is ecdsa-sha2-nistp256, ecdsa-sha2-nistp384 or ecdsa-sha2-nistp521.

This was acknowledged as an issue [1] in the past, but hasn't yet been fixed.

Hi Andy,
thanks for your report. We have decided in the past to not explicitly cater to every key prefix and instead use key:: for literal keys. See https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey

`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.`

Quoted 40 lines

[1]: https://github.com/git/git/pull/1041#issuecomment-971425601

---
ssh signing: Support ECDSA as literal SSH keys

Keys generated using ssh-keygen -t ecdsa or similar will currently be rejected as literal SSH keys because the prefix is ecdsa-sha2-nistp256, ecdsa-sha2-nistp384 or ecdsa-sha2-nistp521.

This was acknowledged as an issue in the past, but hasn't yet been fixed.

   https://github.com/git/git/pull/1041#issuecomment-971425601

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1272%2Falindeman%2Fecdsa-sha2-keys-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1272/alindeman/ecdsa-sha2-keys-v1
Pull-Request: https://github.com/git/git/pull/1272

gpg-interface.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
gpg-interface.c +1 -1
index 280f1fa1a58..086bd03b51d 100644
@@ -779,7 +779,7 @@ 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-")) {
+	if (starts_with(string, "ssh-") || starts_with(string, "ecdsa-sha2-")) {
 		*key = string;
 		return 1;
 	}

base-commit: 8ddf593a250e07d388059f7e3f471078e1d2ed5c

Signature · 1 line
gitgitgadget
#2 lore raw
Quoted 16 lines · Fabian Stelzer
Quoted 8 lines · Andy Lindeman via GitGitGadget

From: Andy Lindeman <[email protected]>

Keys generated using ssh-keygen -t ecdsa or similar are being rejected as literal SSH keys because the prefix is ecdsa-sha2-nistp256, ecdsa-sha2-nistp384 or ecdsa-sha2-nistp521.

This was acknowledged as an issue [1] in the past, but hasn't yet been fixed.

Hi Andy,
thanks for your report. We have decided in the past to not explicitly cater to every key prefix and instead use key:: for literal keys. See
https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey

Quoted 3 lines

`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.`

Thanks for replying, Fabian.

My main issue is that ecdsa-sha2-* keys currently seem incompatible with gpg.ssh.defaultKeyCommand = "ssh-add -L"

The git-config documentation of gpg.ssh.defaultKeyCommand says:

Quoted 1 line

To automatically use the first available key from your ssh-agent set this to "ssh-add -L".

But this does not work with ecdsa keys because each line of the output of the command is checked against is_literal_ssh_key. Because of that check, keys that do not begin with ssh- are skipped.

I could certainly write my own shell script for defaultKeyCommand that did something like ssh-add -L | sed 's/^/key::/' but it's a bit awkward.

The code that runs defaultKeyCommand states:

Quoted 4 lines

/*
* We only use is_literal_ssh_key here to check validity
* The prefix will be stripped when the key is used. */

but this is clearly not true because it is rejecting valid SSH keys.

Do you have thoughts on how to improve gpg.ssh.defaultKeyCommand for keys whose prefix is not ssh- ?

#3 lore raw
Quoted 26 lines · Andy Lindeman
Quoted 16 lines · Fabian Stelzer
Quoted 8 lines · Andy Lindeman via GitGitGadget

From: Andy Lindeman <[email protected]>

Keys generated using ssh-keygen -t ecdsa or similar are being rejected as literal SSH keys because the prefix is ecdsa-sha2-nistp256, ecdsa-sha2-nistp384 or ecdsa-sha2-nistp521.

This was acknowledged as an issue [1] in the past, but hasn't yet been fixed.

Hi Andy,
thanks for your report. We have decided in the past to not explicitly cater to every key prefix and instead use key:: for literal keys. See
https://git-scm.com/docs/git-config#Documentation/git-config.txt-usersigningKey

Quoted 3 lines

`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.`

Thanks for replying, Fabian.

My main issue is that ecdsa-sha2-* keys currently seem incompatible with gpg.ssh.defaultKeyCommand = "ssh-add -L"

Quoted 7 lines

The git-config documentation of gpg.ssh.defaultKeyCommand says:

Quoted 1 line

To automatically use the first available key from your ssh-agent set this to "ssh-add -L".

But this does not work with ecdsa keys because each line of the output of the command is checked against is_literal_ssh_key. Because of that check, keys that do not begin with ssh- are skipped.

True, this is a bug.

Quoted 4 lines

I could certainly write my own shell script for defaultKeyCommand that did something like ssh-add -L | sed 's/^/key::/' but it's a bit awkward.

I think this is at least a valid workaround for now.

Quoted 8 lines

The code that runs defaultKeyCommand states:

Quoted 4 lines

/*
* We only use is_literal_ssh_key here to check validity
* The prefix will be stripped when the key is used. */

Quoted 4 lines

but this is clearly not true because it is rejecting valid SSH keys.

Do you have thoughts on how to improve gpg.ssh.defaultKeyCommand for keys whose prefix is not ssh- ?

The problem is that we do not want to maintain all ssh keytypes in the git code. Thats why the key:: was added.
I'll have to think what we could do besides just skipping the check completely and just assuming the defaultKeyCommand will return a valid key.
ssh-add -L is not necessarily defined as having a parsable output and any additional messages it might print (or some pkcs11 provider) would at least be skipped with the ssh- prefix check.

#4 lore raw
Quoted 8 lines · Fabian Stelzer
Quoted 8 lines

Thanks for replying, Fabian.

My main issue is that ecdsa-sha2-* keys currently seem incompatible with gpg.ssh.defaultKeyCommand = "ssh-add -L"

The git-config documentation of gpg.ssh.defaultKeyCommand says:

Quoted 1 line

To automatically use the first available key from your ssh-agent set this to "ssh-add -L".

This is puzzling. One chooses the key to use when signing, and the key should go to the gpg.ssh.defaultkey, and also "ssh-add" is told about the key for convenient access. Asking "ssh-add -L" about the keys it knows about and randomly pick the first one it happens to tell you about sounds totally backwards to me.

I may have a key I use to sign, and one key each to go to various destinations, all of which "ssh-add -L" may know about. It alone cannot fundamentally tell because it does not know what you intend to use each key for.

Of course, as your own custom script, defaultKeyCommand may know which keys you intend to use for connecting and which keys you intend to use for signing. It may even need to know which key you intend to use for each project you work with and your .git/config may have something to tell the script what "trait" the key to be used that appear in "ssh-add -L" output should have (perhaps the key is rotated very often so you cannot write the exact key in your configuration, but perhaps the comment at the end of each line have sufficient cue to tell them apart). So, the custom script would need to go line by line to find the key to use in the first place, and if it is computationally capable enough to do so, it should be easy to prefix key:: in front. IIRC, we designed the system in such a way that it is not an error to prefix key:: in front of ssh-* keys.

In any case, perhaps we should extend the documentation a bit. It generally is not sensible to just use "ssh-add -L" and pick one random key out of it, so we shouldn't be encouraging such a use, I suspect.

#5 lore raw
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Fabian Stelzer
Quoted 5 lines

Thanks for replying, Fabian.

My main issue is that ecdsa-sha2-* keys currently seem incompatible with gpg.ssh.defaultKeyCommand = "ssh-add -L"

Quoted 7 lines
Quoted 3 lines
Quoted 3 lines

The git-config documentation of gpg.ssh.defaultKeyCommand says:

Quoted 1 line

To automatically use the first available key from your ssh-agent set this to "ssh-add -L".

This is puzzling. One chooses the key to use when signing, and the key should go to the gpg.ssh.defaultkey, and also "ssh-add" is told about the key for convenient access.

I think you mean user.siningKey but yes, this is the best way to do this.

Quoted 23 lines

Asking "ssh-add -L" about the
keys it knows about and randomly pick the first one it happens to tell you about sounds totally backwards to me.

I may have a key I use to sign, and one key each to go to various destinations, all of which "ssh-add -L" may know about. It alone cannot fundamentally tell because it does not know what you intend to use each key for.

Of course, as your own custom script, defaultKeyCommand may know which keys you intend to use for connecting and which keys you intend to use for signing. It may even need to know which key you intend to use for each project you work with and your .git/config may have something to tell the script what "trait" the key to be used that appear in "ssh-add -L" output should have (perhaps the key is rotated very often so you cannot write the exact key in your configuration, but perhaps the comment at the end of each line have sufficient cue to tell them apart). So, the custom script would need to go line by line to find the key to use in the first place, and if it is computationally capable enough to do so, it should be easy to prefix key:: in front. IIRC, we designed the system in such a way that it is not an error to prefix key:: in front of ssh-* keys.

Quoted 4 lines

In any case, perhaps we should extend the documentation a bit. It generally is not sensible to just use "ssh-add -L" and pick one random key out of it, so we shouldn't be encouraging such a use, I suspect.

Yes, I think that reasonable. The script can do some advanced decision making / key lookup if needed. The use-case for me was to enforce/encourage use of the correct users keys on a shared development server in a corporate environment (i have a global directory of all the users keys and want to make sure everyone uses their correct one when signing).

I'll take a look at the docs and suggest a patch in a bit.

#6 lore raw
Quoted 10 lines · Fabian Stelzer
Quoted 9 lines · Junio C Hamano
Quoted 7 lines · Fabian Stelzer
Quoted 7 lines

Thanks for replying, Fabian.

My main issue is that ecdsa-sha2-* keys currently seem incompatible with gpg.ssh.defaultKeyCommand = "ssh-add -L"

The git-config documentation of gpg.ssh.defaultKeyCommand says:

Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

To automatically use the first available key from your ssh-agent set this to "ssh-add -L".

This is puzzling. One chooses the key to use when signing, and the key should go to the gpg.ssh.defaultkey, and also "ssh-add" is told about the key for convenient access.

I think you mean user.siningKey but yes, this is the best way to do this.

Thanks for seeing my intention through my mistake.

Quoted 14 lines
Quoted 4 lines

Asking "ssh-add -L" about the
keys it knows about and randomly pick the first one it happens to tell you about sounds totally backwards to me.

Quoted 9 lines

I may have a key I use to sign, and one key each to go to various destinations, all of which "ssh-add -L" may know about. It alone cannot fundamentally tell because it does not know what you intend to use each key for.
...
In any case, perhaps we should extend the documentation a bit. It generally is not sensible to just use "ssh-add -L" and pick one random key out of it, so we shouldn't be encouraging such a use, I suspect.

Quoted 2 lines

Yes, I think that reasonable. The script can do some advanced decision making / key lookup if needed.

OK.

Quoted 4 lines

The use-case for me was to enforce/encourage use of the correct users keys on a shared development server in a corporate environment (i have a global directory of all the users keys and want to make sure everyone uses their correct one when signing).

I actually wanted to hear more about the reasoning along that line.

IOW, "sure, theoretically, you should start from 'this is the key I want to use' and you shouldn't be asking 'ssh-add -L' about it, but here is a real-world workflow that makes it cumbersome" was what I wanted to see, both in the discussion *and* in the documentation update.

For example, there may be corporate environment where key is frequently rotated, e.g. every morning an employee may have to "corp login" to talk to a central key server and get the ssh key stored in their hardware token refreshed. In such an environment, it would not be surprising if the employee does not even know what the fingerprint or the public part of the key looks like before asking 'ssh-add -L' to query the hardware token, so it may be impractical to follow the "set your key to user.signingKey and add that to the agent". Asking the agent about the key may make perfect sense (but you'd probably need to find which key among its output lines) in such a case.

#7 lore raw

PATCH 1/1 gpg docs: explain better use of ssh.defaultKeyCommand

In reply to Junio C Hamano

Using ssh-add -L for gpg.ssh.defaultKeyCommand is not a good recommendation. It might switch keys depending on the order of known keys and it only supports ssh-* and no ecdsa or other keys. Clarify that we expect a literal key prefixed by key::, give valid example use cases and refer to user.signingKey as the preferred option.

---
 Documentation/config/gpg.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Documentation/config/gpg.txt +6 -3
index 86892ada77..86f6308c4c 100644
@@ -36,9 +36,12 @@ gpg.minTrustLevel::
 
 gpg.ssh.defaultKeyCommand::
 	This command that will be run when user.signingkey is not set and a ssh
-	signature is requested. On successful exit a valid ssh public key is
-	expected in the first line of its output. To automatically use the first
-	available key from your ssh-agent set this to "ssh-add -L".
+	signature is requested. On successful exit a valid ssh public key
+	prefixed with `key::` is expected in the first line of its output.
+	This allows for a script doing a dynamic lookup of the correct public
+	key when it is impractical to statically configure `user.signingKey`.
+	For example when keys or SSH Certificates are rotated frequently or
+	selection of the right key depends on external factors unknown to git.
 
 gpg.ssh.allowedSignersFile::
 	A file containing ssh public keys which you are willing to trust.
Signature · 1 line
2.35.3
#8 lore raw

Re: [PATCH] gpg docs: explain better use of ssh.defaultKeyCommand

Quoted 34 lines · Fabian Stelzer

Using ssh-add -L for gpg.ssh.defaultKeyCommand is not a good recommendation. It might switch keys depending on the order of known keys and it only supports ssh-* and no ecdsa or other keys. Clarify that we expect a literal key prefixed by key::, give valid example use cases and refer to user.signingKey as the preferred option.

---
 Documentation/config/gpg.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Documentation/config/gpg.txt +6 -4
index 86892ada77..86f6308c4c 100644
@@ -36,9 +36,12 @@ gpg.minTrustLevel::
 
 gpg.ssh.defaultKeyCommand::
        This command that will be run when user.signingkey is not set and a ssh
-       signature is requested. On successful exit a valid ssh public key is
-       expected in the first line of its output. To automatically use the first
-       available key from your ssh-agent set this to "ssh-add -L".
+       signature is requested. On successful exit a valid ssh public key
+       prefixed with `key::` is expected in the first line of its output.
+       This allows for a script doing a dynamic lookup of the correct public
+       key when it is impractical to statically configure `user.signingKey`.
+       For example when keys or SSH Certificates are rotated frequently or
+       selection of the right key depends on external factors unknown to git.
 
 gpg.ssh.allowedSignersFile::
        A file containing ssh public keys which you are willing to trust.
--
Quoted 2 lines

2.35.3

Nice. This makes sense to me.