openssh bugs - May 2022 - [Bug 3429] New: Confusing error message from `ssh-keygen -Y sign` when private key is not in agent

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

            Bug ID: 3429
           Summary: Confusing error message from `ssh-keygen -Y sign` when
                    private key is not in agent
           Product: Portable OpenSSH
           Version: v9.0p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh-keygen
          Assignee: unassigned-bugs at mindrot.org
          Reporter: adaszko at gmail.com

Hi,

The `ssh-keygen -Y sign` command produces a confusing "invalid format"
message:

    $ ./ssh-keygen -Y sign -n git -f
/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpYT1apW
mac.c
    Load key
"/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpYT1apW":
invalid format

The key isn't in fact malformed -- it's a valid *public* key:

    $ cat
/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpYT1apW
    ssh-ed25519
AAAAC3NzaC1lZDI1NTE5AAAAIEC1BkuWP6vSf+4ud6IrkQz8TWlV6cQlwpKlgvpj0j/B
adaszko at gmail.com

The reason for this behavior is the fallback mechanism at [1]. 
Normally, the filename path passed as `-f` option is interpreted as a
*public* key, but when the corresponding *private* key is missing from
ssh-agent, ssh-keygen tries to interpret the file as a *private* key,
which fails with the above error message.  Everything works fine when
the private key is present in ssh-agent.

This becomes even more confusing when it's invoked by git to sign a
commit:

    $ git commit --amend -S --no-edit
    error: Load key
"/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpkArSj7":
invalid format?
    fatal: failed to write commit object

I'm happy to contribute a patch but it isn't entirely clear to me what
the best course of action would actually be in this case.  Displaying a
warning when the fallback fires?  Remove the fallback altogether? 
There's backward compatibility issues with the latter.  I'd appreciate
some input on the issue.

All the best
??Adam

[1]
https://github.com/openssh/openssh-portable/blob/457dce2cfef6a48f5442591cd8b21c7e8cba13f8/ssh-keygen.c#L2675-L2692

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org,
                   |                            |dtucker at dtucker.net
   Attachment #3589|                            |ok?(dtucker at dtucker.net)
              Flags|                            |

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Created attachment 3589
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3589&action=edit
improve error message

I think at the very least we can improve the error message. This patch
adjusts the error message to read:
> No private key found for "/tmp/id_ed25519"
which might give the user a chance to consider checking the agent.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3589|ok?(dtucker at dtucker.net)    |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

--- Comment #2 from Adam Szkoda <adaszko at gmail.com> ---
Thanks Damien.  Your patch is indeed an improvement.  Although the
error message coming from git is still going to be a little bit
confusing -- it's expressed in terms of a temporary file path that the
user has no control over.

Zooming out a little, I think the root cause is the double meaning of
the `-f` option: it's used for specifying both the private and the
public key which, in case anything goes wrong, makes it hard to guess
what the actual user intention was and thus produce an accurate error
message.

There are two ways that I see it could be improved further for git
users:

  1) An additional warning could be produced if ssh-keygen didn't find
the private key in the agent.  That I think would be a sufficient hint
to git user.  Though such a warning is a bit questionable given the
fact it's normal, documented behavior to perform a fallback to reading
a the private key from the file given to `-f`.

  2) Better IMO: It would have been clearer if for `-Y sign`, the `-f`
option was restricted to only to specify a *public* key.  An additional
option would need to be passed to indicate that `-f` is being given a
*private* key.  As it happens, such option already exists: `-U`, so it
seems like an ideal candidate.  I realize though, it's easier said than
done given backward compatibility.

Or perhaps 3): There's something on the git side that can be done to
make it clear the key is missing from ssh-agent?

Cheers

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3589|0                           |1
        is obsolete|                            |
   Attachment #3590|                            |ok?(dtucker at dtucker.net)
              Flags|                            |

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
Created attachment 3590
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3590&action=edit
Use prefer_agent

ha, I forgot that we had a flag for that. This patch make -U require
the agent for -Y sign operations.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3590|ok?(dtucker at dtucker.net)    |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |3418

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
These have all be committed and should be in OpenSSH 9.1, due in a few
months.

It should be safe to use -U even for older versions. It won't require
the agent (as openssh-9.1 will) but it won't cause an error.

Thanks again for the report and suggestions.

commit 0086a286ea6bbd11ca9b664ac3bb12b27443d6eb (HEAD -> master,
origin/master, origin/HEAD)
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Mon May 9 03:09:53 2022 +0000

    upstream: Allow existing -U (use agent) flag to work with "-Y
sign"

    operations, where it will be interpreted to require that the
private keys is
    hosted in an agent; bz3429, suggested by Adam Szkoda; ok dtucker@

    OpenBSD-Commit-ID: a7bc69873b99c32c42c7628ed9ea91565ba08c2f

commit cb010744cc98f651b1029bb09efa986eb54e4ccf
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Sun May 8 22:58:35 2022 +0000

    upstream: improve error message when 'ssh-keygen -Y sign' is unable
to

    load a private key; bz3429, reported by Adam Szkoda ok dtucker@

    OpenBSD-Commit-ID: bb57b285e67bea536ef81b1055467be2fc380e74


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3418
[Bug 3418] tracking bug for openssh-9.1
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

--- Comment #5 from Adam Szkoda <adaszko at gmail.com> ---
Wonderful!  Thank you for your time, Damien! :)

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
Closing bugs from openssh-9.1 release cycle

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #7 from Damien Miller <djm at mindrot.org> ---
OpenSSH 9.3 has been released. Close resolved bugs

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.