LudovicRousseau / pyscard

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

CardRequest waitforcard ATR can't see reconnected readers on Windows (regression) #123

Closed antonio-fr closed 2 years ago

antonio-fr commented 2 years ago

Working on a new version of PIVageant, I'm facing an issue when I update pyscard to 2.0.2.

Your system information

Please describe your issue in as much detail as possible:

There's regression in the v2.0.2. It is very similar to the issue #114. It was caused by this commit https://github.com/LudovicRousseau/pyscard/commit/5b43ef5006483e43e62f7497ef6ce466a0f67aaf

I think the issue is because the ScardServiceStopped error is now ignored and that change supposes a new connection will call getReaderNames(). But this is not always true. When a card connection happens through the ATR centric approach, the readers are not listed. When one adds readers() to list readers, it works. That was the workaround of the previous issue, and this workaround still applies : adding a fake listing of the readers allows pyscard to run without any issue. For this current issue, an easier workaround is to stay on (or downgrade to) pyscard v2.0.1 in Windows.

Describe what you expected should happen. We expect the same behavior as in 2.0.1. When all readers are previously disconnected, then the ATR cardrequest.waitforcard should detect and connect to an inserted reader.

Describe what did happen. The CardRequest service can't see any new reader connected. The effect is the CardRequestTimeoutException is raised, even when there's is an available reader and a card matching the expected ATR. Note that what happens is different from the previous issue, it throws CardRequestTimeoutException instead of a ListReadersException. It is not an out of bound exception, but now it is like the card was not present, pyscard can't see it. The user experience is the same, a dongle connected back can no more be found by pyscard.

Steps for reproducing this issue:

With 2.0.1 : works fine

Connect a YubiKey, and press RETURN

A YubiKey detected :
 - SN xxxxxxxxx

Disconnect all the reader, all Yubikeys, and press RETURN

Reconnect a Yubikey, and press RETURN

A YubiKey detected :
 - SN xxxxxxxxx

With 2.0.2 doesn't see the card reconnected

Connect a YubiKey, and press RETURN

A YubiKey detected :
 - SN xxxxxxxxx

Disconnect all the reader, all Yubikeys, and press RETURN

Reconnect a Yubikey, and press RETURN
[ timeout ]
No Yubikey detected

If you don't have any Yubikey, that code can probably be modified to run with other cards. Note that you have to disconnect the reader, not only the card. With Yubikey, the reader and card are packed together, and this is easy to manipulate for this issue. That is also why we face this issue as PIVageant works mostly with USB dongles.

I suggest an easy and simple solution is to revert the offending commit. But there may be more elaborated way t tackle this.

LudovicRousseau commented 2 years ago

Reverting 5b43ef5006483e43e62f7497ef6ce466a0f67aaf would reintroduce the problem that was fixed with the patch. Not a real solution.

I do not use Windows. If you can provide a patch that would help.

antonio-fr commented 2 years ago

What was the problem which was fixed by this commit? I don't see any issue using 2.0.1 on Windows. I don't tell there was no issue, just that I didn't face any, and I try to understand the issue to imagine a solution. Because I don't understand what was the problem.

I don't fully understand what the code is doing, and even less how the winscard API is working. Can we have a readers list output from SCardListReaders and having a non-null return code value? An other possibility is that the SCardListReaders returns SCARD_E_SERVICE_STOPPED the first time it is called, then it provides a list of the readers the second time. In waitforcard, there is a first call of getReaderNames() L130, then there is a loop while not evt.isSet() and not cardfound: calling getReaderNames() again.

There was absolutely no issue using PCSCCardRequest.waitforcard in 2.0.1 on Windows. Now with a change in 2.0.2, reconnected readers are not seen by pyscard. But at this point, I can't understand why it worked in 2.0.1.

I start to think that maybe I wasn't pointing and looking in the right direction. Can this be this change that introduced the regression ? https://github.com/LudovicRousseau/pyscard/commit/da43ddac3c3775566d50b9f860d1faa1fccea9b2 If so, that would mean a change like that in smartcard/pcsc/PCSCContext.py :

from sys import platform

...

            if platform == "windows" or not PCSCContext.instance:
                self.renewContext()

Considering the macOS fix in 2.0.2 and the regression in Windows, that means for now a product running on multiple platforms and using pyscard should instruct in its setup.py (in the case it uses waitforcard ATR).

'pyscard==2.0.2;platform_system=="Linux" or platform_system=="Darwin"',
'pyscard==2.0.1;platform_system=="Windows"',
antonio-fr commented 2 years ago

An other workaround to get a bug free experience with pyscard on multiple platforms is also to add a call to smartcard.System.readers() right before a CardRequest. As it was done here. Note this software is only for Windows and this workaround applies for pyscard versions before 2.0.1. This workaround is removed in the in-development version using pyscard 2.0.1. But then this current issue came for 2.0.2, and the same workaround can work for this current issue. But we chose to specify the pyscard version being 2.0.1, instead of putting back this workaround in this Windows app.

LudovicRousseau commented 2 years ago

As indicated in https://github.com/LudovicRousseau/pyscard/commit/db52d271240568145be43b8d726d8a552c48c18e the initial problem is https://github.com/LudovicRousseau/pyscard/issues/74

LudovicRousseau commented 2 years ago

I can reproduce your issue (I think it is the same problem) using https://github.com/LudovicRousseau/pyscard/blob/master/smartcard/Examples/framework/sample_MonitorCards.py

On Windows 10, If I remove the reader+card and insert it again the new reader is not detected.

antonio-fr commented 2 years ago

Thanks for your fast care about this issue.

The issue #74 was fully fixed by https://github.com/LudovicRousseau/pyscard/commit/db52d271240568145be43b8d726d8a552c48c18e, present in 2.0.1. But the next version 2.0.2 made a regression about this topic. To me, https://github.com/LudovicRousseau/pyscard/commit/5b43ef5006483e43e62f7497ef6ce466a0f67aaf wasn't fixing any issue. I can just tell that a couple of changes between 2.0.1 and 2.0.2 introduced this current issue : when the last reader is disconnected, any new reader connected is not found.

LudovicRousseau commented 2 years ago

The problem fixed in https://github.com/LudovicRousseau/pyscard/commit/5b43ef5006483e43e62f7497ef6ce466a0f67aaf is that getReaderNames() will now not return the exception for SCARD_E_SERVICE_STOPPED when the last reader is removed.

I should have added the test sample code + output in the git commit message. :-(

See also https://ludovicrousseau.blogspot.com/2021/12/windows-pcsc-and-scardeservicestopped.html

LudovicRousseau commented 2 years ago

I have a solution in https://ludovicrousseau.blogspot.com/2021/12/windows-pcsc-and-scardeservicestopped_27.html

LudovicRousseau commented 2 years ago

Please check the problem is fixed for you. You can get pre-built packages from https://ci.appveyor.com/project/LudovicRousseau/pyscard/build/job/38basdl4rln7bf6m/artifacts

antonio-fr commented 2 years ago

I built and performed a test drive using : https://gist.github.com/antonio-fr/56e84dd019302d5bbebe6456b053a43a

I can confirm the latest version works as expected.

Testing platform : Microsoft Windows Version 10.0.19042.1237 64 bit Python 3.9.7 (tags/v3.9.7:1016ef3, Aug 30 2021, 20:19:38) [MSC v.1929 64 bit (AMD64)]

2.0.0 : Scard Mgmr shut down exception (issue #114 )

Testing pyscard Windows
pyscard version 2.0.0

Connect a YubiKey, and press RETURN

A YubiKey detected :
 - SN XXX

Disconnect all the reader, all Yubikeys, and press RETURN

Reconnect a Yubikey, and press RETURN

Error !!!
Failed to list readers: The Smart Card Resource Manager has shut down.  (0x-7FEFFFE2)

Now with readers list
A YubiKey detected :
 - SN XXX

2.0.1 : βœ… πŸ‘Œ

Testing pyscard Windows
pyscard version 2.0.1

Connect a YubiKey, and press RETURN

A YubiKey detected :
 - SN XXX

Disconnect all the reader, all Yubikeys, and press RETURN

Reconnect a Yubikey, and press RETURN

A YubiKey detected :
 - SN XXX

Now with readers list
A YubiKey detected :
 - SN XXX

2.0.2 : No reader detected (this current issue)

Testing pyscard Windows
pyscard version 2.0.2

Connect a YubiKey, and press RETURN

A YubiKey detected :
 - SN XXX

Disconnect all the reader, all Yubikeys, and press RETURN

Reconnect a Yubikey, and press RETURN

No Yubikey detected !!!

Now with readers list
A YubiKey detected :
 - SN XXX

2.0.2+dev : βœ… πŸ‘Œ From this package


Testing pyscard Windows
pyscard version 2.0.2 [a2aaa1c9]

Connect a YubiKey, and press RETURN

A YubiKey detected :
 - SN XXX

Disconnect all the reader, all Yubikeys, and press RETURN

Reconnect a Yubikey, and press RETURN

A YubiKey detected :
 - SN XXX

Now with readers list
A YubiKey detected :
 - SN XXX

Thanks for the fix. πŸ‘

LudovicRousseau commented 2 years ago

Thanks for the feedback. I will make a new release "soon".