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

Add kapelse readers support in COMPOSITE_AS_MULTISLOT mode #118

Closed

Conversation

bmoraine
Copy link
Contributor

Hello,

Please find 2 patches regarding COMPOSITE_AS_MULTISLOT mode to simplify its implementation and to add support for Kapelse readers supports.

Best Regards

Baptiste Moraine

@bmoraine
Copy link
Contributor Author

Build issue when --enable-strict --enable-composite-as-multislot used for configuration is now fixed by last commit

Copy link
Owner

@LudovicRousseau LudovicRousseau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed your fist commit with changes in https://github.com/LudovicRousseau/CCID-debug/

Please re-submit your patch in this -debug repo.

src/ccid_usb.c Show resolved Hide resolved

case KAPELSE_KAPLIN2 :
/* KAP&LINK2 : only 3 first interfaces are ccid ones */
if(config_desc->bNumInterfaces > 3)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this test?
In my list the KAP-LINK2 has 3 CCID interfaces https://ccid.apdu.fr/ccid/shouldwork.html#0x29470x0105

Do you have another model with a different configuration but using the same idProduct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is needed as KAP-LINK2 is a USB composite device presenting either 3 CCID interfaces or 3 CCID interfaces (the 3 first ones) and a CDC-ACM interface according user configuration

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.
It should work fine if you something like use:

max_interface_number = 3; /* up to 4 interfaces */
num_CCID_interfaces = 3;  /* 3 CCID interfaces */

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmoraine Is it possible for the KAP-LINK2 to NOT have 3 CCID interfaces?

Why can't we use max_interface_number = 2; in all cases without checking the value of config_desc->bNumInterfaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LudovicRousseau Effectively, KAP-LINK2 has always 3 CCID interfaces (the 3 first ones) and an optional 4th CDC-ACM interface. So, you are right, we can use max_interface_number = 2; in all cases without checking the value of config_desc->bNumInterfaces

@LudovicRousseau
Copy link
Owner

Can you also give me the output of the following command for each kapelse readers please

lsusb -d 2947: -v | grep 'bInterfaceClass\|idVendor\|idProduct'

I need to know what other interfaces are present and in which position.

For example the Gemalto Prox DU reader has 2 CCID interfaces and 1 HID interface. The HID interface is in the first position so max_num_interfaces is not the same as the number of "slots" as you suggested in your patch. The code you removed looked like a duplication but was not exactly the same.

@bmoraine
Copy link
Contributor Author

bmoraine commented Nov 6, 2023

Please find below the output of the command :
lsusb -d 2947: -v | grep 'bInterfaceClass\|idVendor\|idProduct'

  1. For Kap&Link2
  • When enumerating 3 CCID interfaces :
    $ lsusb -d 2947: -v | grep 'bInterfaceClass|idVendor|idProduct'
    idVendor 0x2947
    idProduct 0x0105
    bInterfaceClass 11 Chip/SmartCard
    bInterfaceClass 11 Chip/SmartCard
    bInterfaceClass 11 Chip/SmartCard

  • When enumerating 3 CCID interfaces and 1 CDC ACM Interface :
    $ lsusb -d 2947: -v | grep 'bInterfaceClass|idVendor|idProduct'
    idVendor 0x2947
    idProduct 0x0105
    bInterfaceClass 11 Chip/SmartCard
    bInterfaceClass 11 Chip/SmartCard
    bInterfaceClass 11 Chip/SmartCard
    bInterfaceClass 2 Communications
    bInterfaceClass 10 CDC Data

  1. For KAP-eCV :

$ lsusb -d 2947: -v | grep 'bInterfaceClass|idVendor|idProduct'
idVendor 0x2947
idProduct 0x0112
bInterfaceClass 11 Chip/SmartCard
bInterfaceClass 3 Human Interface Device
bInterfaceClass 2 Communications
bInterfaceClass 10 CDC Data

  1. For all others Kapelse readers there are as many CCID interfaces as USB interfaces as for Kap&Link :
    $ lsusb -d 2947: -v | grep 'bInterfaceClass|idVendor|idProduct'
    idVendor 0x2947
    idProduct 0x0101
    bInterfaceClass 11 Chip/SmartCard
    bInterfaceClass 11 Chip/SmartCard

@bmoraine
Copy link
Contributor Author

bmoraine commented Nov 7, 2023

It's unclear for me. Do you want me to submit Kapelse reader suport in COMPOSITE_AS_MULTISLOT mode patch in this -debug repo? If yes, via a pull request?

@LudovicRousseau
Copy link
Owner

I made modifications to your first patch in the CCID-debug repo.
You now have to define 2 values: max_interface_number and num_CCID_interfaces.

Since the CCID interfaces on the kapelse readers are in the first interfaces num_CCID_interfaces should contain the number of CCID interfaces (easy) and max_interface_number should be num_CCID_interfaces -1 because it starts at 0.

Yes, I ask you to propose a Pull Request on this repo to add support of kapelse readers.
I do not have any of the kapelse readers so I can't test the code myself.
I don't want to publish untested code.

@bmoraine
Copy link
Contributor Author

bmoraine commented Nov 9, 2023

For information, I have created a Pull Request as recommended :
LudovicRousseau/CCID-debug#1

@LudovicRousseau
Copy link
Owner

Good.
have you checked your patches works fine on macOS with your readers?

@bmoraine
Copy link
Contributor Author

bmoraine commented Nov 9, 2023

No, I tested only on linux using "./configure --enable-strict --enable-composite-as-multislot " before compiling and installing driver.

In my understanding, driver should behave the same as on macos.

@LudovicRousseau
Copy link
Owner

I was expecting an answer like that :-)
I asked because your patch does not correctly set (I think) the max_interface_number value.
pcsc-lite on GNU/Linux does support composite devices. So you can't check your code on Linux.

You have to use macOS to test your changes work fine on macOS.

Please also test connecting 2 Kapelse readers and check all the CCID interfaces are visible using pcsctest.
The CCID driver has code to reset static_interface value when all the CCID interfaces have been used, so the driver is ready to connect another "composite-as-multislot" reader.
https://github.com/LudovicRousseau/CCID/blob/master/src/ccid_usb.c#L732

@LudovicRousseau
Copy link
Owner

@bmoraine you can also send me Kapelse sample readers so I can make tests on my side.
Contact me by private email.

@bmoraine
Copy link
Contributor Author

@LudovicRousseau I will test Kapelse readers on macOS Sonoma and ,in the same time, send you some readers.

Regarding ccid driver generatioin on macos, I always slightly modified ccid driver source in SmartcardCCID :
https://github.com/apple-oss-distributions/SmartcardCCID/ and never update the entire ccid driver source files.

In my undestanding, Apple applies patches on top of cdid driver and use specific parameters when configuring ccid project according :
https://github.com/apple-oss-distributions/SmartcardCCID/blob/main/ccid/Makefile.
I assume that if I replace ccid tar.gz with the content of CCID-debug and I execute the makefile, it should be OK. Do you confirm?

@LudovicRousseau
Copy link
Owner

I assume that if I replace ccid tar.gz with the content of CCID-debug and I execute the makefile, it should be OK. Do you confirm?

I also assume it should work.
I never used the Apple process to build my CCID driver on macOS, so you are the expert on this point.

@bmoraine
Copy link
Contributor Author

What is your process to build CCID driver on macOS?

@LudovicRousseau
Copy link
Owner

@LudovicRousseau
Copy link
Owner

Fixed in 73d9d3a and later patches

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