From: Alex Wilson Date: Mon, 20 Apr 2020 11:42:02 +0000 (+1000) Subject: SCardReadCache/SCardWriteCache should actually cache data X-Git-Tag: 2.1.0~8 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6e73a9ecf229e4fc3588a4a8d4d541ee0f7e9016;p=platform%2Fupstream%2Ffreerdp.git SCardReadCache/SCardWriteCache should actually cache data 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. --- diff --git a/channels/smartcard/client/smartcard_operations.c b/channels/smartcard/client/smartcard_operations.c index ef03d7a..d1562b9 100644 --- a/channels/smartcard/client/smartcard_operations.c +++ b/channels/smartcard/client/smartcard_operations.c @@ -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, diff --git a/channels/smartcard/client/smartcard_pack.c b/channels/smartcard/client/smartcard_pack.c index 197345d..64a0c94 100644 --- a/channels/smartcard/client/smartcard_pack.c +++ b/channels/smartcard/client/smartcard_pack.c @@ -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, "}"); } diff --git a/winpr/libwinpr/smartcard/smartcard_pcsc.c b/winpr/libwinpr/smartcard/smartcard_pcsc.c index 3b98d92..f5f33a6 100644 --- a/winpr/libwinpr/smartcard/smartcard_pcsc.c +++ b/winpr/libwinpr/smartcard/smartcard_pcsc.c @@ -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);