Mailing List Archive: [Bug 1506] rationalize agent behavior on smartcard removal/reattachment

[Bug 1506] rationalize agent behavior on smartcard removal/reattachment

Feb 1, 2009, 3:02 PM

Post #1 of 3 (683 views)

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

--- Comment #6 from Damien Miller <djm@mindrot.org> 2009-02-02 10:01:59 ---
(From update of attachment 1561)
>diff -ruN openssh-5.1p1/scard-opensc.c openssh-5.1p1.dkg/scard-opensc.c
>--- openssh-5.1p1/scard-opensc.c 2007-03-12 16:35:39.000000000 -0400
>+++ openssh-5.1p1.dkg/scard-opensc.c 2008-08-19 21:35:31.000000000 -0400
>@@ -124,7 +121,9 @@
> struct sc_pkcs15_prkey_info *key;
> struct sc_pkcs15_object *pin_obj;
> struct sc_pkcs15_pin_info *pin;
>+ int detach_retry = 1;
>
>+ detachretry:
> priv = (struct sc_priv_data *) RSA_get_app_data(rsa);
> if (priv == NULL)
> return -1;
>@@ -162,6 +161,13 @@
> }
> pin = pin_obj->data;
> r = sc_lock(card);
>+ if (r == SC_ERROR_READER_DETACHED) {

Shouldn't this be (r == SC_ERROR_READER_DETACHED && detach_retry)

>diff -ruN openssh-5.1p1/ssh-agent.c openssh-5.1p1.dkg/ssh-agent.c
>--- openssh-5.1p1/ssh-agent.c 2008-07-04 09:10:49.000000000 -0400
>+++ openssh-5.1p1.dkg/ssh-agent.c 2008-08-21 11:49:47.000000000 -0400
>@@ -136,6 +136,11 @@
> /* Default lifetime (0 == forever) */
> static int lifetime = 0;
>
>+#ifdef SMARTCARD
>+/* forward declaration needed */
>+static void remove_all_smartcard_keys();
>+#endif /* SMARTCARD */
>+
> static void
> close_socket(SocketEntry *e)
> {
>@@ -330,8 +335,14 @@
> key = key_from_blob(blob, blen);
> if (key != NULL) {
> Identity *id = lookup_identity(key, 2);
>- if (id != NULL && (!id->confirm || confirm_key(id) == 0))
>+ if (id != NULL && (!id->confirm || confirm_key(id) == 0)) {
> ok = key_sign(id->key, &signature, &slen, data, dlen);
>+#ifdef SMARTCARD
>+ if ((ok != 0) && (id->key->flags &= KEY_FLAG_EXT)) {
>+ remove_all_smartcard_keys();
>+ }
>+#endif /* SMARTCARD */

Could this be moved into scard-opensc.c somehow?

--
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- 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.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs

[Bug 1506] rationalize agent behavior on smartcard removal/reattachment [ In reply to ]

bugzilla-daemon at bugzilla

Feb 1, 2009, 4:14 PM

Post #2 of 3 (644 views)

Permalink

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

--- Comment #7 from Daniel Kahn Gillmor <dkg@fifthhorseman.net> 2009-02-02 11:14:17 ---
Damien wrote:

> Shouldn't this be (r == SC_ERROR_READER_DETACHED && detach_retry)

How embarrassing! You are right, of course.

> [...]
> Could this be moved into scard-opensc.c somehow?

It's been a while since i wrote this. i'll dig back into it and see if
there's a way to pull this off. It does seem like an agent-specific
action, though, since none of the other tools actively cache
connections to smartcards.

--
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- 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.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs

[Bug 1506] rationalize agent behavior on smartcard removal/reattachment [ In reply to ]

bugzilla-daemon at bugzilla

Feb 3, 2009, 10:23 AM

Post #3 of 3 (622 views)

Permalink

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

Daniel Kahn Gillmor <dkg@fifthhorseman.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #1561|0 |1
is obsolete| |

--- Comment #8 from Daniel Kahn Gillmor <dkg@fifthhorseman.net> 2009-02-04 05:23:13 ---
Created an attachment (id=1600)
--> (http://bugzilla.mindrot.org/attachment.cgi?id=1600)
retry smartcard at most once on detached reader, and purge PIN and all
H/W keys from agent on failure

I've updated the attached patch with damien's fix.

I just spent a bit of time looking at the agent and the scard-opensc
code, and it's not clear to me how to reasonably move those later hunks
out of ssh-agent.c and into scard-opensc.c. Please point out if i'm
missing something or misunderstanding something.

Basically, we'd need to do something like having one possible side
effect of the sc_sign() call be to disable the RSA* object (which
ssh-agent sees as identity.key.rsa). How would such a disabling work?
RSA_free() doesn't make sense, because it looks like we wouldn't be
able to propagate that information back to the Key structure, as it
holds the pointer to the RSA object which would be invalid after an
RSA_free().

Even if we can figure out how to invalidate a key in a safe way, we'd
then need to update not only the agent, but also all the other code
that ever calls key_sign() to be aware of the possibility that a side
effect of key_sign() could be the disabling of the passed Key object.

So my current preference is to leave the code in ssh-agent, though i
could probably be convinced otherwise if a good technique was
suggested.

--
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- 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.
_______________________________________________
openssh-bugs mailing list
openssh-bugs@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-bugs