openssh bugs - Apr 2016 - [Bug 2564] New: ssh_config AddKeysToAgent doesn't set key name/path

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

            Bug ID: 2564
           Summary: ssh_config AddKeysToAgent doesn't set key name/path
           Product: Portable OpenSSH
           Version: 7.2p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: alves.rjc at gmail.com

Hi everyone,

This request is a small enhancement to the feature introduced in
https://bugzilla.mindrot.org/show_bug.cgi?id=1699 to include the name
or path of/to the ssh key.

Currently if a key is added automatically by having the option
AddKeysToAgent enabled the name/path to the key is not included.
This contrasts with keys added manually via ssh-add which include this
information. So when using "ssh-add -l" the following is seen:

  2048 SHA256:aF4h47lpohn7aXTMtlWFMfEXWtT7zN1CMh2M/NGXgTM  (RSA)
  2048 SHA256:ihNn8ml5z5rLXULYnXibgBSSZ7PiHVXzgCQywXqGi6I .ssh/test
(RSA)

The first line was added automatically by use of AddKeysToAgent. The
second was added by calling "ssh-add .ssh/test".

The missing information could come from the "IdentityFile" in
ssh_config or the "-i" parameter if specified in the command line.

Having the name of the key is useful to know which keys are currently
loaded on the agent.

Thanks,
Renato

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Created attachment 2885
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2885&action=edit
probable fix

Could you please try this patch?

-- 
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=2564

--- Comment #2 from alves.rjc at gmail.com ---
Hi Damien,

The patch doesn't seem to affect anything.
I get exactly the same output as before in both situations.

-- 
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=2564

Vincent Brillault <git at lerya.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |git at lerya.net

--- Comment #3 from Vincent Brillault <git at lerya.net> ---
Created attachment 2894
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2894&action=edit
AddKeysToAgent: Use filename when comment is empty

I took a look at this bug by chance and I think I found the 'issue':
ssh-add has a check verifying if the 'comment' of the key is NULL or
contains only '\0' and replace it with the filename in that case (see
https://github.com/openssh/openssh-portable/blob/master/ssh-add.c#L261-L262,
seems to have been added in
https://github.com/openssh/openssh-portable/commit/2681cdb6e0de7c1af549dac37a9531af202b4434)

The attached patch simply duplicate that behaviour (looking at
sshkey_parse_private_fileblob_type, RSA key always have a 'NULL'
comment).
It seems to fix the issue for me, can you confirm?

-- 
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=2564

--- Comment #4 from alves.rjc at gmail.com ---
(In reply to Vincent Brillault from comment #3)
> Created attachment 2894 [details]
You have a small typo (dupplicate) in the commit message, otherwise the
patch works great and fixes the issue.

Can we get it reviewed and merged?

Thanks,
Renato

-- 
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=2564

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
A similar fix has been committed and will be in OpenSSH 8.2


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3079
[Bug 3079] Tracking bug for 8.2 release
-- 
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=2564

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 release

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