-
-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CCID: Fix misleading read length #67
Conversation
Currently, when some extended APDU are processed, we get the following error: CCID_Receive() Can't read all data 49174kbytes In fact, the processing of the length of the APDU is not correct. For example: 00000002 commands.c:1726:CmdXfrBlockAPDU_short() T=0: 9 bytes 00000003 -> 000000 6F 09 00 00 00 00 2D 00 00 00 00 A4 09 04 04 3F 00 D0 03 00033873 <- 000000 80 02 00 00 00 00 2D 00 00 00 61 27 00 00 00 00 00 00 00 [...] 00002330 commands.c:1682:CCID_Receive() Can't read all data (49174 out of 2 expected) According to the log, the response should be correct but the reader may add too many zeros to the response. The actual response APDU is 61 27. If we check the complete logs/dumps, all the payload till the end is full of 00 .. 00. In fact, the check of the length of the data should be less strict. Let's be tolerant as along as we get enough payload. This fix has been tested with: - Gemalto (was Gemplus) GemPC Twin - Alcor Micro Corp. AU9540 - Feitian R502 Suggested-by: @godfreychung Fix: issue LudovicRousseau#66
Are you using a virtual machine? |
I would like to be able to reproduce and understand the issue first. |
Hi Ludovic, I spent some times since I wanted to double check using a Vanilla Ubuntu 20.04 (beta)
that includes the following versions:
from these packages:
Running baremetal, for all the following USB readers, all are OK using pcsk11-dump, without the enclosed patch from this pull request:
The previous test was done using a Ubuntu 18.04 running virtualized while running baremetal with Ubuntu 20.04/beta I do not face any issue. I am going to perform more tests in Virtualization mode since I need to get pcscd working properly within a virtualized environment. I do not get yet the sweet point of this issue then. I thought that the virtualized USB bus was solid enough to work for any cases. If you have any advice, they are welcomed. I'll keep you posted with some more tests. Just in case, assuming there is a bug with the virtualization of USB, still what's wrong with my patch knowing that it makes the drivers more tolerant when data are sent with some trailers that we should ignore ? best regards, |
What virtualisation software do you use? I had many issues with USB and virtual machines. But feel free to use a patched version. It is free software so that is one benefit to be able to modify the code to make it work better for you. |
I am using HyperV but I am pretty sure I had seen the same issue with Virtualbox and Qemu too ! I need to investigate. The goal is not to hide, but I guess we need to get it fine for the best of everyone. I'll keep you posted after some more investigations. |
One more comment, still using the same Ubuntu 20.04, I checked both slots of this smart card reader:
using pkcs11-dump. So, I get a total of 8 readers working fine, baremetal, with the vanilla pcscd and CCID. I'll spend some times during the upcoming days with the virtualization environment. I need to restore some systems. |
Currently, when some extended APDU are processed, we get the following error: CCID_Receive() Can't read all data 49174kbytes In fact, the processing of the length of the APDU is not correct. For example: 00000002 commands.c:1726:CmdXfrBlockAPDU_short() T=0: 9 bytes 00000003 -> 000000 6F 09 00 00 00 00 2D 00 00 00 00 A4 09 04 04 3F 00 D0 03 00033873 <- 000000 80 02 00 00 00 00 2D 00 00 00 61 27 00 00 00 00 00 00 00 [...] 00002330 commands.c:1682:CCID_Receive() Can't read all data (49174 out of 2 expected) According to the log, the response should be correct but the reader may add too many zeros to the response. The actual response APDU is 61 27. If we check the complete logs/dumps, all the payload till the end is full of 00 .. 00. In fact, the check of the length of the data should be less strict. Let's be tolerant as along as we get enough payload. This fix has been tested with: - Gemalto (was Gemplus) GemPC Twin - Alcor Micro Corp. AU9540 - Feitian R502 Suggested-by: @godfreychung Fix: issue LudovicRousseau#66
You should rebase your branch over upstream/master to have a cleaner patch history. |
right, it was just a local PoC in order to fight with the virtual USB driver till I can put my finger on a proper fix without this patch since it was not accepted upstream. thanks for your follow up, it is appreciated :D |
The problem comes from the virtualisation solution (HyperV here). |
I get the same problem with ESX and a Nutanix system that is using qemu under the hood. It is not specific to HyperV. |
It is not specific to HyperV but it happens only with virtualisation solutions (maybe not ALL the virtualisation solutions). Feel free to use your patch in your version of the CCID driver. That is one benefit of Free Software, you can patch it if you want/need. |
I do not understand your feedback: the benefit of open source for me is More than just having a patch I can play with on my own ; it has values when it becomes a shared one that is upstream-ed and integrated into the Linux distributions. |
Currently, when some extended APDU are processed, we get the following
error:
CCID_Receive() Can't read all data 49174kbytes
In fact, the processing of the length of the APDU is not correct. For
example:
00000002 commands.c:1726:CmdXfrBlockAPDU_short() T=0: 9 bytes
00000003 -> 000000 6F 09 00 00 00 00 2D 00 00 00 00 A4 09 04 04 3F 00 D0 03
00033873 <- 000000 80 02 00 00 00 00 2D 00 00 00 61 27 00 00 00 00 00 00 00
[...]
00002330 commands.c:1682:CCID_Receive() Can't read all data (49174 out of 2 expected)
According to the log, the response should be correct but the reader may
add too many zeros to the response. The actual response APDU is 61 27.
If we check the complete logs/dumps, all the payload till the end is
full of 00 .. 00.
In fact, the check of the length of the data should be less strict.
Let's be tolerant as along as we get enough payload.
This fix has been tested with:
Suggested-by: @godfreychung
Fix: issue #66