Skip to content
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

Closed
wants to merge 3 commits into from

Conversation

vjardin
Copy link

@vjardin vjardin commented Mar 27, 2020

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 #66

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
@LudovicRousseau
Copy link
Owner

Are you using a virtual machine?

@LudovicRousseau
Copy link
Owner

I would like to be able to reproduce and understand the issue first.
How do you reproduce the issue on your side?

@vjardin
Copy link
Author

vjardin commented Apr 14, 2020

Hi Ludovic,

I spent some times since I wanted to double check using a Vanilla Ubuntu 20.04 (beta)

cat /proc/version
Linux version 5.4.0-21-generic (buildd@lcy01-amd64-006) (gcc version 9.3.0 (Ubuntu 9.3.0-8ubuntu1)) #25-Ubuntu SMP Sat Mar 28 13:10:28 UTC 2020

that includes the following versions:

$ pcscd --version
pcsc-lite version 1.8.26.
Copyright (C) 1999-2002 by David Corcoran <[email protected]>.
Copyright (C) 2001-2018 by Ludovic Rousseau <[email protected]>.
Copyright (C) 2003-2004 by Damien Sauveron <[email protected]>.
Report bugs to <[email protected]>.
Enabled features: Linux x86_64-pc-linux-gnu libsystemd serial usb libudev usbdropdir=/usr/lib/pcsc/drivers ipcdir=/run/pcscd filter configdir=/etc/reader.conf.d

from these packages:

pcsc-tools/focal,now 1.5.5-1 amd64 [installed]
pcscd/focal,now 1.8.26-3 amd64 [installed]
libacsccid1/focal,now 1.1.8-1 amd64 [installed]
libccid/focal,now 1.4.31-1 amd64 [installed,automatic]

Running baremetal, for all the following USB readers, all are OK using pcsk11-dump, without the enclosed patch from this pull request:

OK 0a5c:5800 Broadcom Corp. BCM5880 Secure Applications Processor
OK 08e6:3437 Gemalto (was Gemplus) GemPC Twin SmartCard Reader
OK 04e6:5810 SCM Microsystems, Inc. uTrust 2700 R Smart Card Reader
OK 058f:9540 Alcor Micro Corp. AU9540 Smartcard Reader
OK 76b:3021 OmniKey AG CardMan 3121
OK 096e:060d Feitian Technologies, Inc
  Contact + Contactless
OK 076b:5422 OmniKey AG
  Contact + Contactless

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,
Vincent

@LudovicRousseau
Copy link
Owner

What virtualisation software do you use?

I had many issues with USB and virtual machines.
I do not plan to modify my code to hide bugs in USB virtualisation code. The bug should be corrected by the virtualisation software, and not hidden.

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.

@vjardin
Copy link
Author

vjardin commented Apr 14, 2020

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.

@vjardin
Copy link
Author

vjardin commented Apr 14, 2020

One more comment, still using the same Ubuntu 20.04, I checked both slots of this smart card reader:

OK 072f:8300 Advanced Card Systems, Ltd

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
@LudovicRousseau
Copy link
Owner

You should rebase your branch over upstream/master to have a cleaner patch history.

@vjardin
Copy link
Author

vjardin commented Aug 5, 2020

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

@LudovicRousseau
Copy link
Owner

The problem comes from the virtualisation solution (HyperV here).

@vjardin
Copy link
Author

vjardin commented Nov 1, 2020

I get the same problem with ESX and a Nutanix system that is using qemu under the hood. It is not specific to HyperV.

@LudovicRousseau
Copy link
Owner

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.

@vjardin
Copy link
Author

vjardin commented Nov 2, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants