LudovicRousseau / pyscard

pyscard smartcard library for python
http://pyscard.sourceforge.net/
GNU Lesser General Public License v2.1
383 stars 110 forks source link

T0/T1 marshaling confusion #31

Closed leifj closed 7 years ago

leifj commented 7 years ago

I've been tracking a strange issue that I believe is due to a problem in the swig code for scard. I'm talking to a SLE5542 using an ACR39U reader and I've got a piece of C code for reading memory off of that card that works but the corresponding python code using the scard interface does not. All of this was done using fresh clones of pcsc-lite and pyscard from github.

The C code is in https://gist.github.com/leifj/140f9c79d99af078c5167f587cd81b69

This code works as expected and it depends on using the T0 protocol which triggers the correct processing of "pseudo" APDUs in the driver. So far so good. However ...

Using scard I expect code like this to work:

hresult, hcard, dwActiveProtocol = SCardConnect(
                hcontext,
                reader,
                SCARD_SHARE_DIRECT, SCARD_PROTOCOL_T0 | SCARD_PROTOCOL_T1)

SELECT_CARD_TYPE = [0xFF,0xA4,0x00,0x00,0x01,0x06]
hresult, response = SCardTransmit(hcard, SCARD_PCI_T0, SELECT_CARD_TYPE)
if hresult != SCARD_S_SUCCESS:
    raise error, 'Failed to transmit: ' + SCardGetErrorMessage(hresult)

Using this code I get a protocol mismatch (between the card protocol and requested protocol) and when I step through pcscd with gdb while running this python code, I find that the message string received by ContextThread in winscard_svc.c has a '1' in the pioSendPciProtocol field instead of the expected 0. A sample gdb snippet:

13553973 winscard_svc.c:351:ContextThread() Received command: TRANSMIT from client 14
353                     switch (header.command)
(gdb) 
621                                     READ_BODY(trStr)
(gdb) 
623                                     if (MSGCheckHandleAssociation(trStr.hCard, threadContext))
(gdb) p trStr
$3 = {hCard = 1821360693, ioSendPciProtocol = 1, ioSendPciLength = 16, cbSendLength = 6, ioRecvPciProtocol = 3, ioRecvPciLength = 16, 
  pcbRecvLength = 65548, rv = 0}
(gdb) c
Continuing.

Continuing to step through this I confirm that the subsequent call to SCardTransmit is attempted using the T1 protocol which of course results in a protocol mismatch later on.

There seems to be a marshalling error somewhere in pyscard.

leifj commented 7 years ago

A bit more research... The '1' in dwProtocol for T0 is is clearly correct. The question is why 0 is sent from that piece of C-code, because that seems to be why the C code works.... Here is the same snippet of gdb from stepping through pcscd when calling the C program:

03558356 winscard_svc.c:351:ContextThread() Received command: TRANSMIT from client 14
353                     switch (header.command)
(gdb) 
621                                     READ_BODY(trStr)
(gdb) 
623                                     if (MSGCheckHandleAssociation(trStr.hCard, threadContext))
(gdb) p trStr
$1 = {hCard = 781293158, ioSendPciProtocol = 0, ioSendPciLength = 6299704, cbSendLength = 6, ioRecvPciProtocol = 3, ioRecvPciLength = 16, 
  pcbRecvLength = 266, rv = 0}
LudovicRousseau commented 7 years ago

Why don't you use the value returned by SCardConnect() in dwActiveProtocol to use it in SCardTransmit() ?

LudovicRousseau commented 7 years ago

You are using SCARD_SHARE_DIRECT. This imply a different protocol negotiation. But that does not explain why the C code "works".

leifj commented 7 years ago

Sure but the case statement in _Transmit in scard.i makes that impossible

leifj commented 7 years ago

Yes but I use SCARD_SHARE_DIRECT in the pyscard code too - I just left that out for brevity

leifj commented 7 years ago

dwActiveProtocol is 0 btw which hits the default case in _Transmit and causes an Invalid Parameter exception. This seems inconsistent with the behaviour of the C library where passing 0 while using DIRECT triggers the desired behavior in the driver

leifj commented 7 years ago

Here is a patch that "fixes" this. Review pls...

diff --git a/smartcard/scard/scard.i b/smartcard/scard/scard.i
index 76f63ad..87f38fa 100644
--- a/smartcard/scard/scard.i
+++ b/smartcard/scard/scard.i
@@ -672,8 +672,12 @@ static SCARDRETCODE _Transmit(
 )
 {
     PSCARD_IO_REQUEST piorequest=NULL;
+    SCARD_IO_REQUEST unknown;
     long ret;

+    unknown.dwProtocol = 0;
+    unknown.cbPciLength = 0;
+
     pblRecvBuffer->ab = (unsigned char*)mem_Malloc(MAX_BUFFER_SIZE_EXTENDED*sizeof(unsigned char));
     pblRecvBuffer->cBytes = MAX_BUFFER_SIZE_EXTENDED;

@@ -692,6 +696,10 @@ static SCARDRETCODE _Transmit(
             piorequest=(PSCARD_IO_REQUEST)myg_prgSCardRawPci;
             break;

+        case 0x00:
+            piorequest = &unknown;
+            break;
+
         default:
             return SCARD_E_INVALID_PARAMETER
LudovicRousseau commented 7 years ago

I guess your C code works because you go in the default case and pioSendPci is not initialized. With luck you have pioSendPci.dwProtocol = 0 and 0 is SCARD_PROTOCOL_UNDEFINED.

What is the value of dwActiveProtocolafter the SCardConnect()?

If you add a the line pioSendPci.dwProtocol = SCARD_PROTOCOL_T0 before the switch(dwActiveProtocol) the C program should also fail. Do you confirm?

leifj commented 7 years ago

Skickat från min iPhone

21 okt. 2016 kl. 21:28 skrev Ludovic Rousseau notifications@github.com:

I guess your C code works because you go in the default case and pioSendPci is not initialized. With luck you have pioSendPci.dwProtocol = 0 and 0 is SCARD_PROTOCOL_UNDEFINED.

What is the value of dwActiveProtocolafter the SCardConnect()?

If you add a the line pioSendPci.dwProtocol = SCARD_PROTOCOL_T0 before the switch(dwActiveProtocol) the C program should also fail. Do you confirm?

Oh I'm absolutely sure you're right!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

LudovicRousseau commented 7 years ago

What is the value of dwActiveProtocol after the SCardConnect()?

You forgot to answer this question.

LudovicRousseau commented 7 years ago

I got no answer? I need to understand the problem in order to fix it correctly.

leifj commented 7 years ago

On 2016-11-07 20:28, Ludovic Rousseau wrote:

I got no answer? I need to understand the problem in order to fix it correctly.

sorry, got several NMIs

leifj commented 7 years ago

Really sorry for the delay!

repr(dwActiveProtocol) == 0L after ScardConnect

LudovicRousseau commented 7 years ago

It should be fixed with https://github.com/LudovicRousseau/pyscard/commit/fa3addbad5ba66e2d6e5b4bef95238a72138c80e

Please confirm it works for you.

LudovicRousseau commented 7 years ago

Can you please confirm the fix works for you? The idea is to close this issue and release a new version of PySCard.

leifj commented 7 years ago

Actually it doesn't. I still needed my patch to make it work.

LudovicRousseau commented 7 years ago

Your patch should not apply without modifications on the latest version.

leifj commented 7 years ago

Skickat från min iPhone

5 jan. 2017 kl. 16:09 skrev Ludovic Rousseau notifications@github.com:

Your patch should not apply without modifications on the latest version.

Are you really using the latest PySCard git version?

Yeah I'm pretty sure

If yes can you send me your updated patch for review? Exactly the same one the one in my PR

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

LudovicRousseau commented 7 years ago

OK. You PR can apply cleanly. But your patch should have NO effect since the case 0 (or SCARD_PROTOCOL_UNDEFINED) is already managed by the current code.

I also note that your repository at https://github.com/SUNET/pyscard/commits/master is NOT up to date.

LudovicRousseau commented 7 years ago

This bug should be fixed upstream. If your problem is still present then please open a new issue with details and traces.