openssh bugs - Dec 2013 - [Bug 2187] New: ssh-add unnecessarily prompts for PKCS#11 pin when removing key

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

            Bug ID: 2187
           Summary: ssh-add unnecessarily prompts for PKCS#11 pin when
                    removing key
           Product: Portable OpenSSH
           Version: 6.3p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh-add
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jay at slushpupie.com

Created attachment 2392
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2392&action=edit
openssh-6.3p1-ssh-add.patch

Although PROTOCOL.agent specifies that when performing
SSH_AGENTC_REMOVE_SMARTCARD_KEY the pin is used to determine which
smartcards to remove, in implementation the pin is never used.  I think
this is due to the fact the pin is never stored, so there is nothing to
compare to.  Although the pin is never used during the remove
operation, ssh-add prompts for the pin, which is frustrating to some
users

The attached patch causes ssh-add to not prompt for a pin while
removing a PKCS#11 library.

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2392|0                           |1
        is obsolete|                            |
   Attachment #2393|                            |ok?(dtucker at zip.com.au)
              Flags|                            |
                 CC|                            |djm at mindrot.org,
                   |                            |dtucker at zip.com.au
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org

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

Thanks - that looks correct.

Here's a slightly tweaked patch. I think it should go in.

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

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2393|ok?(dtucker at zip.com.au)     |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=2187

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
committed - thanks. This will be in OpenSSH 6.5 due early next year.

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

Jakub Jelen <jjelen at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jjelen at redhat.com

--- Comment #3 from Jakub Jelen <jjelen at redhat.com> ---
Created attachment 2624
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2624&action=edit
patch: free only on existing pin

This works, but unfortunately, if you are removing card, you call free
on NULL pointer, which is ... not good.

We had this fixed in our version, but probably forgot to report back
upstream last year.

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

Jakub Jelen <jjelen at redhat.com> changed:

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

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

Darren Tucker <dtucker at zip.com.au> changed:

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

--- Comment #4 from Darren Tucker <dtucker at zip.com.au> ---
(In reply to Jakub Jelen from comment #3)
> This works, but unfortunately, if you are removing card, you call
> free on NULL pointer, which is ... not good.
Nope, free(NULL) is fine.
>From the free(3) man page: "If ptr is NULL, no operation is
performed."
and SuSv2: http://pubs.opengroup.org/onlinepubs/007908799/xsh/free.html
"If ptr is a null pointer, no action occurs."

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

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 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.