git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* gpg.ssh.defaultKeyCommand docs bug?
@ 2023-10-06 17:14 matthew sporleder
  2023-10-09 20:43 ` Jeff King
  0 siblings, 1 reply; 4+ messages in thread
From: matthew sporleder @ 2023-10-06 17:14 UTC (permalink / raw)
  To: Git Mailing List

https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgsshdefaultKeyCommand

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

---

The command does not actually work (for me, git version 2.42.0) with
key:: prefixed.

It only works if I cat the public key as-is.

I only figured this out because the docs previously said it took the
format of ssh-add -L, which also doesn't not contain key::.

I am using this script for my "dynamic" key discovery:
#!/bin/sh
f=$(ssh -G $(git remote get-url $(git remote|head -1)|awk -F':' '{
print $1 }') |grep -E '^identityfile'|sed 's#^identityfile ##g')
cat $(eval realpath ${f}.pub)

Thanks,
Matt

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: gpg.ssh.defaultKeyCommand docs bug?
  2023-10-06 17:14 gpg.ssh.defaultKeyCommand docs bug? matthew sporleder
@ 2023-10-09 20:43 ` Jeff King
  2023-10-11 18:16   ` matthew sporleder
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2023-10-09 20:43 UTC (permalink / raw)
  To: matthew sporleder; +Cc: Fabian Stelzer, Git Mailing List

[+cc Fabian, who wrote this code]

On Fri, Oct 06, 2023 at 01:14:49PM -0400, matthew sporleder wrote:

> https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgsshdefaultKeyCommand
> 
> 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
> 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.
> 
> ---
> 
> The command does not actually work (for me, git version 2.42.0) with
> key:: prefixed.
> 
> It only works if I cat the public key as-is.
> 
> I only figured this out because the docs previously said it took the
> format of ssh-add -L, which also doesn't not contain key::.
> 
> I am using this script for my "dynamic" key discovery:
> #!/bin/sh
> f=$(ssh -G $(git remote get-url $(git remote|head -1)|awk -F':' '{
> print $1 }') |grep -E '^identityfile'|sed 's#^identityfile ##g')
> cat $(eval realpath ${f}.pub)

I'm not very familiar with this part of Git, but looking at the code
which parses the output of gpg.ssh.defaultKeyCommand, it splits it by
line and then calls is_literal_ssh_key() on it, which is:

  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;
  }

So your script works because the pub file starts with "ssh-rsa" or
similar (and so would "ssh-add -L" output).

The user.signingKey docs say:

  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.

From reading the commit messages here, I guess this is about supporting
non-ssh key types (e.g., my TPM-based key is ecdsa-sha2-nistp256 in the
"ssh-add -L" output). But I'm not sure who is supposed to be put "key::"
there.

You said it "does not actually work" with "key::" prefixed. What
happens? In the signing code we make a similar call to
is_literal_ssh_key() that wills trip off the "key::" prefix, so I'd
expect it work. But I could also believe there is a bug. :)

-Peff

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: gpg.ssh.defaultKeyCommand docs bug?
  2023-10-09 20:43 ` Jeff King
@ 2023-10-11 18:16   ` matthew sporleder
  2023-10-11 23:41     ` Jeff King
  0 siblings, 1 reply; 4+ messages in thread
From: matthew sporleder @ 2023-10-11 18:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Fabian Stelzer, Git Mailing List

On Mon, Oct 9, 2023 at 4:43 PM Jeff King <peff@peff.net> wrote:
>
> [+cc Fabian, who wrote this code]
>
> On Fri, Oct 06, 2023 at 01:14:49PM -0400, matthew sporleder wrote:
>
> > https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgsshdefaultKeyCommand
> >
> > 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
> > 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.
> >
> > ---
> >
> > The command does not actually work (for me, git version 2.42.0) with
> > key:: prefixed.
> >
> > It only works if I cat the public key as-is.
> >
> > I only figured this out because the docs previously said it took the
> > format of ssh-add -L, which also doesn't not contain key::.
> >
> > I am using this script for my "dynamic" key discovery:
> > #!/bin/sh
> > f=$(ssh -G $(git remote get-url $(git remote|head -1)|awk -F':' '{
> > print $1 }') |grep -E '^identityfile'|sed 's#^identityfile ##g')
> > cat $(eval realpath ${f}.pub)
>
> I'm not very familiar with this part of Git, but looking at the code
> which parses the output of gpg.ssh.defaultKeyCommand, it splits it by
> line and then calls is_literal_ssh_key() on it, which is:
>
>   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;
>   }
>
> So your script works because the pub file starts with "ssh-rsa" or
> similar (and so would "ssh-add -L" output).
>
> The user.signingKey docs say:
>
>   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.
>
> From reading the commit messages here, I guess this is about supporting
> non-ssh key types (e.g., my TPM-based key is ecdsa-sha2-nistp256 in the
> "ssh-add -L" output). But I'm not sure who is supposed to be put "key::"
> there.
>
> You said it "does not actually work" with "key::" prefixed. What
> happens? In the signing code we make a similar call to
> is_literal_ssh_key() that wills trip off the "key::" prefix, so I'd
> expect it work. But I could also believe there is a bug. :)
>
> -Peff

It gave very confusing errors!

key::ssh-rsa ABC123 me@localhost (no new line)
error: Load key "....: invalid format?

key::ABC123 (yes new line)
error: Couldn't load public key ...: No such file or directory?

key::ssh-rsa ABC123 me@localhost (yes new line)
works, I think

ssh-rsa ABC123 me@localhost (yes new line)
works (the script I provided)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: gpg.ssh.defaultKeyCommand docs bug?
  2023-10-11 18:16   ` matthew sporleder
@ 2023-10-11 23:41     ` Jeff King
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2023-10-11 23:41 UTC (permalink / raw)
  To: matthew sporleder; +Cc: Fabian Stelzer, Git Mailing List

On Wed, Oct 11, 2023 at 02:16:27PM -0400, matthew sporleder wrote:

> It gave very confusing errors!
> 
> key::ssh-rsa ABC123 me@localhost (no new line)
> error: Load key "....: invalid format?

It's hard to say without seeing the whole output, but I suspect this is
actually coming from ssh, not Git. We just dump the output into a
tempfile and feed it to "ssh-keygen -f".

Though I'd think we would see the same issue with user.signingKey in
that case.

So I'm not sure what's going on here (I haven't set up ssh signing to
play with yet).

> key::ABC123 (yes new line)
> error: Couldn't load public key ...: No such file or directory?

That one makes sense to me. The "ssh-rsa" part is important, because
without it, ssh-keygen has no idea what format it is in.

> key::ssh-rsa ABC123 me@localhost (yes new line)
> works, I think

This is the recommended format.

> ssh-rsa ABC123 me@localhost (yes new line)
> works (the script I provided)

And this is the historical one.

So I don't think the documentation is _wrong_ here, but I agree that it
is a bit on the confusing side (especially understanding that "key::"
came later, and that raw "ssh-rsa" is deprecated, which is only
mentioned in user.signingKey, not gpg.ssh.defaultKeyCommand.

And I'm still not sure what's going on with your no-new-line example,
which I'd have expected to work.

-Peff

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-10-11 23:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-06 17:14 gpg.ssh.defaultKeyCommand docs bug? matthew sporleder
2023-10-09 20:43 ` Jeff King
2023-10-11 18:16   ` matthew sporleder
2023-10-11 23:41     ` Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).