From 711f5ef4e3a3a81f4f8fc64caeb49b3d3a98a004 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= Date: Thu, 15 Jun 2023 10:59:00 +0200 Subject: [PATCH 1/6] reader-pcsc: Get card status when timeout is received When the reader is removed and inserted back between two consecutive callings to SCardGetStatusChange(), the change is not reported. The card handle should be invalidated, which is detected by SCardStatus. When no reconnection is done afterwards, SCardGetStatusChange() will still return timeout and ard handle will be invalid. --- src/libopensc/reader-pcsc.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c index 723bcfd013..e5e492901d 100644 --- a/src/libopensc/reader-pcsc.c +++ b/src/libopensc/reader-pcsc.c @@ -375,14 +375,28 @@ static int refresh_attributes(sc_reader_t *reader) if (rv != SCARD_S_SUCCESS) { if (rv == (LONG)SCARD_E_TIMEOUT) { - /* Timeout, no change from previous recorded state. Make sure that - * changed flag is not set. */ - reader->flags &= ~SC_READER_CARD_CHANGED; + DWORD readers_len = 0, cstate = 0, prot, atr_len = SC_MAX_ATR_SIZE; + unsigned char atr[SC_MAX_ATR_SIZE]; + /* Make sure to preserve the CARD_PRESENT flag if the reader was * reattached and we called the refresh_attributes too recently */ if (priv->reader_state.dwEventState & SCARD_STATE_PRESENT) { reader->flags |= SC_READER_CARD_PRESENT; } + + rv = priv->gpriv->SCardStatus(priv->pcsc_card, NULL, &readers_len, &cstate, &prot, atr, &atr_len); + if (priv->pcsc_card != 0 + && (rv == (LONG)SCARD_W_REMOVED_CARD || rv == (LONG)SCARD_E_INVALID_VALUE || rv == (LONG)SCARD_E_INVALID_HANDLE)) { + /* When reader is removed between two subsequent calls to refresh_attributes, + * SCardGetStatusChange does not notice the change - set it here */ + reader->flags |= SC_READER_CARD_CHANGED; + /* If this happens, card must be reconnected, otherwise SCardGetStatusChange() will still return timeout + * and card handle will be invalid. */ + } else { + /* Otherwise timeout denotes no change from previous recorded state. Make sure that + * changed flag is not set. */ + reader->flags &= ~SC_READER_CARD_CHANGED; + } LOG_FUNC_RETURN(reader->ctx, SC_SUCCESS); } From 11258707b0e2dcc569ac85769dbb8be4f2c38bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= Date: Thu, 15 Jun 2023 11:05:58 +0200 Subject: [PATCH 2/6] reader-pcsc: Set flag denoting change when reader is removed When the removed reader is detected for the first time, set SC_READER_CARD_CHANGED denoting change. Remove the flag when the condition is hit again. Before this change, when reader was removed the reader->flags in refresh_attributes() was updated from 0x00000001 (SC_READER_CARD_PRESENT) to 0x00000020 (SC_READER_REMOVED). However, when the reader is still removed by another call to refresh_attributes(), the flags remained as 0x00000020 (SC_READER_REMOVED). Now the flags should change with SC_READER_CARD_CHANGED. --- src/libopensc/reader-pcsc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c index e5e492901d..6d275759d0 100644 --- a/src/libopensc/reader-pcsc.c +++ b/src/libopensc/reader-pcsc.c @@ -406,7 +406,13 @@ static int refresh_attributes(sc_reader_t *reader) || rv == (LONG)SCARD_E_NO_READERS_AVAILABLE #endif || rv == (LONG)SCARD_E_SERVICE_STOPPED) { - reader->flags &= ~(SC_READER_CARD_PRESENT); + /* Set flag when state changes */ + if (reader->flags & SC_READER_REMOVED) { + reader->flags &= ~SC_READER_CARD_CHANGED; + } else { + reader->flags |= SC_READER_CARD_CHANGED; + } + reader->flags &= ~SC_READER_CARD_PRESENT; reader->flags |= SC_READER_REMOVED; priv->gpriv->removed_reader = reader; SC_FUNC_RETURN(reader->ctx, SC_LOG_DEBUG_VERBOSE, SC_SUCCESS); From 639fa57c60ff07e07d3b9037074b964f243d6c9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= Date: Thu, 15 Jun 2023 11:16:12 +0200 Subject: [PATCH 3/6] reader-pcsc: remove flag for removed reader When SCardGetStatusChange returns value successfully, remove also SC_READER_REMOVED before setting the current state. --- src/libopensc/reader-pcsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c index 6d275759d0..7dc67772a4 100644 --- a/src/libopensc/reader-pcsc.c +++ b/src/libopensc/reader-pcsc.c @@ -437,7 +437,7 @@ static int refresh_attributes(sc_reader_t *reader) SC_FUNC_RETURN(reader->ctx, SC_LOG_DEBUG_VERBOSE, SC_SUCCESS); } - reader->flags &= ~(SC_READER_CARD_CHANGED|SC_READER_CARD_INUSE|SC_READER_CARD_EXCLUSIVE); + reader->flags &= ~(SC_READER_CARD_CHANGED|SC_READER_CARD_INUSE|SC_READER_CARD_EXCLUSIVE|SC_READER_REMOVED); if (state & SCARD_STATE_PRESENT) { reader->flags |= SC_READER_CARD_PRESENT; From b465e94c0911aa341413fcfe44323be3d15d0da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= Date: Thu, 15 Jun 2023 11:34:57 +0200 Subject: [PATCH 4/6] slot: remove infinite loop when card change happens When reader is removed and inserted back between two consecutive calls to PCSC SCardGetStatusChange() function, and no reconnection is done, the status will remain SC_READER_CARD_CHANGED. --- src/pkcs11/slot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pkcs11/slot.c b/src/pkcs11/slot.c index 0020a4002d..6457131ffd 100644 --- a/src/pkcs11/slot.c +++ b/src/pkcs11/slot.c @@ -215,6 +215,7 @@ CK_RV card_detect(sc_reader_t *reader) CK_RV rv; unsigned int i; int j; + int retry = 3; sc_log(context, "%s: Detecting smart card", reader->name); /* Check if someone inserted a card */ @@ -235,10 +236,10 @@ CK_RV card_detect(sc_reader_t *reader) sc_log(context, "%s: Card changed", reader->name); /* The following should never happen - but if it * does we'll be stuck in an endless loop. - * So better be fussy. - if (!retry--) - return CKR_TOKEN_NOT_PRESENT; */ + * So better be fussy.*/ card_removed(reader); + if (!retry--) + return CKR_TOKEN_NOT_PRESENT; goto again; } From 668ddda8651be1c09c67ffcd877e1b75dd9d10a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= Date: Thu, 15 Jun 2023 14:07:22 +0200 Subject: [PATCH 5/6] slot: remove relation between reader and slot The card_detect() can detect removed reader, it calls card_removed(), but card_detect_all() did not remove relation between slot and reader. --- src/pkcs11/slot.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/pkcs11/slot.c b/src/pkcs11/slot.c index 6457131ffd..3d3ae6f627 100644 --- a/src/pkcs11/slot.c +++ b/src/pkcs11/slot.c @@ -428,6 +428,17 @@ card_detect_all(void) } } card_detect(reader); + + /* If reader was removed, card_removed() was already called in card_detect(), + * remove only relation between reader and slot. */ + if (reader->flags & SC_READER_REMOVED) { + for (j = 0; jreader == reader) { + slot->reader = NULL; + } + } + } } } sc_log(context, "All cards detected"); From d1c5e0d13b8e99b1b84a25bb86a97b19bdf0da62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Veronika=20Hanul=C3=ADkov=C3=A1?= Date: Mon, 31 Jul 2023 11:04:52 +0200 Subject: [PATCH 6/6] Check for invalid pcsc card handle before calling SCardStatus --- src/libopensc/reader-pcsc.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/libopensc/reader-pcsc.c b/src/libopensc/reader-pcsc.c index 7dc67772a4..f0aa21da73 100644 --- a/src/libopensc/reader-pcsc.c +++ b/src/libopensc/reader-pcsc.c @@ -384,19 +384,20 @@ static int refresh_attributes(sc_reader_t *reader) reader->flags |= SC_READER_CARD_PRESENT; } - rv = priv->gpriv->SCardStatus(priv->pcsc_card, NULL, &readers_len, &cstate, &prot, atr, &atr_len); - if (priv->pcsc_card != 0 - && (rv == (LONG)SCARD_W_REMOVED_CARD || rv == (LONG)SCARD_E_INVALID_VALUE || rv == (LONG)SCARD_E_INVALID_HANDLE)) { + /* Timeout should denote no change from previous recorded state, + * but check for valid card handle */ + reader->flags &= ~SC_READER_CARD_CHANGED; + if (priv->pcsc_card != 0) { /* When reader is removed between two subsequent calls to refresh_attributes, - * SCardGetStatusChange does not notice the change - set it here */ - reader->flags |= SC_READER_CARD_CHANGED; + * SCardGetStatusChange does not notice the change, test the card handle with SCardStatus */ + rv = priv->gpriv->SCardStatus(priv->pcsc_card, NULL, &readers_len, &cstate, &prot, atr, &atr_len); + + if (rv == (LONG)SCARD_W_REMOVED_CARD || rv == (LONG)SCARD_E_INVALID_VALUE || rv == (LONG)SCARD_E_INVALID_HANDLE) + reader->flags |= SC_READER_CARD_CHANGED; /* If this happens, card must be reconnected, otherwise SCardGetStatusChange() will still return timeout * and card handle will be invalid. */ - } else { - /* Otherwise timeout denotes no change from previous recorded state. Make sure that - * changed flag is not set. */ - reader->flags &= ~SC_READER_CARD_CHANGED; } + LOG_FUNC_RETURN(reader->ctx, SC_SUCCESS); }