Bug 2187 - ssh-add unnecessarily prompts for PKCS#11 pin when removing key
Summary: ssh-add unnecessarily prompts for PKCS#11 pin when removing key
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh-add (show other bugs)
Version: 6.3p1
Hardware: All All
: P5 enhancement
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_6_5
  Show dependency treegraph
 
Reported: 2013-12-18 09:21 AEDT by jay
Modified: 2021-04-23 15:03 AEST (History)
3 users (show)

See Also:


Attachments
openssh-6.3p1-ssh-add.patch (646 bytes, text/plain)
2013-12-18 09:21 AEDT, jay
no flags Details
tweaked patch (943 bytes, patch)
2013-12-19 10:42 AEDT, Damien Miller
dtucker: ok+
Details | Diff
patch: free only on existing pin (271 bytes, patch)
2015-05-20 17:59 AEST, Jakub Jelen
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jay 2013-12-18 09:21:35 AEDT
Created attachment 2392 [details]
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.
Comment 1 Damien Miller 2013-12-19 10:42:29 AEDT
Created attachment 2393 [details]
tweaked patch

Thanks - that looks correct.

Here's a slightly tweaked patch. I think it should go in.
Comment 2 Damien Miller 2013-12-19 11:14:21 AEDT
committed - thanks. This will be in OpenSSH 6.5 due early next year.
Comment 3 Jakub Jelen 2015-05-20 17:59:26 AEST
Created attachment 2624 [details]
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.
Comment 4 Darren Tucker 2015-06-05 14:25:10 AEST
(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."
Comment 5 Damien Miller 2021-04-23 15:03:55 AEST
closing resolved bugs as of 8.6p1 release