SCardReadCache/SCardWriteCache should actually cache data
authorAlex Wilson <alex@cooperi.net>
Mon, 20 Apr 2020 11:42:02 +0000 (21:42 +1000)
committerakallabeth <akallabeth@users.noreply.github.com>
Tue, 28 Apr 2020 12:03:19 +0000 (14:03 +0200)
Currently since the hash/keyCompare/keyClone members on the
context->cache were never being set, we were using the
HashTable_Pointer* variants, meaning that lookup always
failed (since we never ask for the same *pointer* twice).

This also revealed that the logic for autoallocate on these ops
was a bit backwards, and some error codes and support for the
"freshness" counter were missing.

In Win10 (at least with some card minidrivers) the freshness
counter is load-bearing and smartcard login won't work without
implementing a very basic version of it.

channels/smartcard/client/smartcard_operations.c
channels/smartcard/client/smartcard_pack.c
winpr/libwinpr/smartcard/smartcard_pcsc.c

index ef03d7a..d1562b9 100644 (file)
@@ -959,7 +959,7 @@ static LONG smartcard_ReadCacheA_Call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OPE
        if (!call->Common.fPbDataIsNULL)
        {
                ret.cbDataLen = call->Common.cbDataLen;
-               if (autoalloc)
+               if (!autoalloc)
                {
                        ret.pbData = malloc(ret.cbDataLen);
                        if (!ret.pbData)
@@ -975,7 +975,11 @@ static LONG smartcard_ReadCacheA_Call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OPE
                ret.ReturnCode = SCardReadCacheA(operation->hContext, call->Common.CardIdentifier,
                                                 call->Common.FreshnessCounter, call->szLookupName,
                                                 ret.pbData, &ret.cbDataLen);
-       log_status_error(TAG, "SCardReadCacheA", ret.ReturnCode);
+       if ((ret.ReturnCode != SCARD_W_CACHE_ITEM_NOT_FOUND) &&
+           (ret.ReturnCode != SCARD_W_CACHE_ITEM_STALE))
+       {
+               log_status_error(TAG, "SCardReadCacheA", ret.ReturnCode);
+       }
        free(call->szLookupName);
        free(call->Common.CardIdentifier);
 
@@ -1000,7 +1004,7 @@ static LONG smartcard_ReadCacheW_Call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OPE
        if (!call->Common.fPbDataIsNULL)
        {
                ret.cbDataLen = call->Common.cbDataLen;
-               if (autoalloc)
+               if (!autoalloc)
                {
                        ret.pbData = malloc(ret.cbDataLen);
                        if (!ret.pbData)
@@ -1016,7 +1020,11 @@ static LONG smartcard_ReadCacheW_Call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OPE
                ret.ReturnCode = SCardReadCacheW(operation->hContext, call->Common.CardIdentifier,
                                                 call->Common.FreshnessCounter, call->szLookupName,
                                                 ret.pbData, &ret.cbDataLen);
-       log_status_error(TAG, "SCardReadCacheW", ret.ReturnCode);
+       if ((ret.ReturnCode != SCARD_W_CACHE_ITEM_NOT_FOUND) &&
+           (ret.ReturnCode != SCARD_W_CACHE_ITEM_STALE))
+       {
+               log_status_error(TAG, "SCardReadCacheA", ret.ReturnCode);
+       }
        free(call->szLookupName);
        free(call->Common.CardIdentifier);
 
@@ -2685,7 +2693,8 @@ LONG smartcard_irp_device_control_call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OP
        }
 
        if ((result != SCARD_S_SUCCESS) && (result != SCARD_E_TIMEOUT) &&
-           (result != SCARD_E_NO_READERS_AVAILABLE) && (result != SCARD_E_NO_SERVICE))
+           (result != SCARD_E_NO_READERS_AVAILABLE) && (result != SCARD_E_NO_SERVICE) &&
+           (result != SCARD_W_CACHE_ITEM_NOT_FOUND) && (result != SCARD_W_CACHE_ITEM_STALE))
        {
                WLog_WARN(TAG, "IRP failure: %s (0x%08" PRIX32 "), status: %s (0x%08" PRIX32 ")",
                          smartcard_get_ioctl_string(ioControlCode, TRUE), ioControlCode,
index 197345d..64a0c94 100644 (file)
@@ -712,7 +712,10 @@ static void smartcard_trace_read_cache_return(SMARTCARD_DEVICE* smartcard,
 
        if (ret->ReturnCode == SCARD_S_SUCCESS)
        {
+               char buffer[1024];
                WLog_LVL(TAG, g_LogLevel, " cbDataLen=%" PRId32, ret->cbDataLen);
+               WLog_LVL(TAG, g_LogLevel, "  cbData: %s",
+                        smartcard_array_dump(ret->pbData, ret->cbDataLen, buffer, sizeof(buffer)));
        }
        WLog_LVL(TAG, g_LogLevel, "}");
 }
index 3b98d92..f5f33a6 100644 (file)
@@ -187,6 +187,7 @@ typedef struct _PCSC_SCARDHANDLE PCSC_SCARDHANDLE;
 typedef struct
 {
        DWORD len;
+       DWORD freshness;
        BYTE* data;
 } PCSC_CACHE_ITEM;
 
@@ -396,6 +397,9 @@ static PCSC_SCARDCONTEXT* PCSC_EstablishCardContext(SCARDCONTEXT hContext)
        if (!pContext->cache)
                goto errors;
 
+       pContext->cache->hash = HashTable_StringHash;
+       pContext->cache->keyCompare = HashTable_StringCompare;
+       pContext->cache->keyClone = HashTable_StringClone;
        pContext->cache->keyFree = free;
        pContext->cache->valueFree = pcsc_cache_item_free;
        if (!g_CardContexts)
@@ -2660,14 +2664,19 @@ static LONG WINAPI PCSC_SCardReadCacheA(SCARDCONTEXT hContext, UUID* CardIdentif
        PCSC_CACHE_ITEM* data;
        PCSC_SCARDCONTEXT* ctx = PCSC_GetCardContextData(hContext);
        char* id = card_id_and_name_a(CardIdentifier, LookupName);
-       WINPR_UNUSED(FreshnessCounter);
 
        data = HashTable_GetItemValue(ctx->cache, id);
        free(id);
        if (!data)
        {
                *DataLen = 0;
-               return SCARD_S_SUCCESS;
+               return SCARD_W_CACHE_ITEM_NOT_FOUND;
+       }
+
+       if (FreshnessCounter != data->freshness)
+       {
+               *DataLen = 0;
+               return SCARD_W_CACHE_ITEM_STALE;
        }
 
        if (*DataLen == SCARD_AUTOALLOCATE)
@@ -2705,7 +2714,6 @@ static LONG WINAPI PCSC_SCardReadCacheW(SCARDCONTEXT hContext, UUID* CardIdentif
        PCSC_CACHE_ITEM* data;
        PCSC_SCARDCONTEXT* ctx = PCSC_GetCardContextData(hContext);
        char* id = card_id_and_name_w(CardIdentifier, LookupName);
-       WINPR_UNUSED(FreshnessCounter);
 
        data = HashTable_GetItemValue(ctx->cache, id);
        free(id);
@@ -2713,7 +2721,13 @@ static LONG WINAPI PCSC_SCardReadCacheW(SCARDCONTEXT hContext, UUID* CardIdentif
        if (!data)
        {
                *DataLen = 0;
-               return SCARD_S_SUCCESS;
+               return SCARD_W_CACHE_ITEM_NOT_FOUND;
+       }
+
+       if (FreshnessCounter != data->freshness)
+       {
+               *DataLen = 0;
+               return SCARD_W_CACHE_ITEM_STALE;
        }
 
        if (*DataLen == SCARD_AUTOALLOCATE)
@@ -2752,8 +2766,6 @@ static LONG WINAPI PCSC_SCardWriteCacheA(SCARDCONTEXT hContext, UUID* CardIdenti
        PCSC_SCARDCONTEXT* ctx = PCSC_GetCardContextData(hContext);
        char* id = card_id_and_name_a(CardIdentifier, LookupName);
 
-       WINPR_UNUSED(FreshnessCounter);
-
        if (!id)
                return SCARD_E_NO_MEMORY;
 
@@ -2771,6 +2783,7 @@ static LONG WINAPI PCSC_SCardWriteCacheA(SCARDCONTEXT hContext, UUID* CardIdenti
                return SCARD_E_NO_MEMORY;
        }
        data->len = DataLen;
+       data->freshness = FreshnessCounter;
        memcpy(data->data, Data, data->len);
 
        HashTable_Remove(ctx->cache, id);
@@ -2787,8 +2800,6 @@ static LONG WINAPI PCSC_SCardWriteCacheW(SCARDCONTEXT hContext, UUID* CardIdenti
        PCSC_SCARDCONTEXT* ctx = PCSC_GetCardContextData(hContext);
        char* id = card_id_and_name_w(CardIdentifier, LookupName);
 
-       WINPR_UNUSED(FreshnessCounter);
-
        if (!id)
                return SCARD_E_NO_MEMORY;
 
@@ -2806,6 +2817,7 @@ static LONG WINAPI PCSC_SCardWriteCacheW(SCARDCONTEXT hContext, UUID* CardIdenti
                return SCARD_E_NO_MEMORY;
        }
        data->len = DataLen;
+       data->freshness = FreshnessCounter;
        memcpy(data->data, Data, data->len);
 
        HashTable_Remove(ctx->cache, id);