pyscard mutes/hides user created exceptions in `on_insert()` hook.

[Torxed]

Your system information

• Operating system used: Arch Linux
• pyscard version: 2.0.9
• Python version: 3.11.8

Please describe your issue in as much detail as possible:

• https://github.com/LudovicRousseau/pyscard/blob/e0d000cff79b541702a8830c161c7b8487e929e3/smartcard/CardMonitoring.py#L189-L192

The following exception handling mutes issues originating outside of pyscard. Given the traceback below, the following would have been muted:

Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/smartcard/CardMonitoring.py", line 180, in run
    self.observable.notifyObservers(
  File "/usr/lib/python3.11/site-packages/smartcard/Observer.py", line 59, in notifyObservers
    observer.update(self, arg)
  File "test.py", line 24, in update
    self.on_insert(addedcards)
  File "test.py", line 117, in on_insert
    self.on_correct_identity(self)
  File "test.py", line 150, in _load_shares
    print(session.use_in_some_way())
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "test_lib.py", line 19, in shares
    print(session.sign(payload, privkey))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: UserSession.sign() takes 2 positional arguments but 3 were given

The correct thing here would be to catch it in test_lib.py obviously. But the error will silently go by until a explicit try/catch is put into place in test_lib.py, making it quite difficult to know where to even put the exception handling. Debugging these errors become quite tedious even with proper breakpoints unless the exception becomes visible to the developing user.

Either pyscard shouldn't do blind exception handling with pass as the result, or the exception of dealing with __del__() should be dealt with conditional checks rather than relying on handling the exception.

Steps for reproducing this issue:

I know this isn't ideal, but I'm in no able to create a minimal working example at the moment due to time. I can come back later and add one, but the exception itself should be clear enough to understand that the exception handling situation isn't ideal.

Apologies for this, but time is extremely limited at the moment.

[LudovicRousseau]

How did you get the traceback then? You used a modified pyscard?

Can you share your test.py source code?

[Torxed]

Will do in a day or so. But yes, modified the above linked code to look like this:

  except TypeError:
-     pass
+     traceback.print_exc()
  except AttributeError:
-     pass
+     traceback.print_exc()

A random shot in the dark is, that it's happening when you're running "external dependencies" in threads but they relate to the on_insert call in pyscard. I know this is a fuzzy description and I'll describe it better later - or even better with some code in a day or so.

[Torxed]

Here's an example of where the above "muted error" occurs. This is as small as I could make the example with the time I had.

To run it:

1. Ensure pcscd is running: /usr/bin/pcscd --foreground --auto-exit $PCSCD_ARGS
2. Insert your PKCS#11 compliant smartcard (since pyscard supports treating it as "injected" on first execution)
3. Make sure pkcs11-tool can find your smartcard: pkcs11-tool -L
4. python <code below>.py

Without the patched code, you'll get:

And after patching it, you'll hopefully get:

(in the screenshot above I just added print() instead of traceback.print_exc() because I was in a rush) And here's the code for testing/verifying, and I used a YubiKey to load the privkey, cert and everything else using the PIV module:

import time
import threading
import cryptography.x509
import cryptography.hazmat.primitives
import smartcard
import smartcard.CardMonitoring
from PyKCS11 import *
from PyKCS11.LowLevel import CKA_ID

yubikey_pin = "123456"
pkcs11_lib = '/usr/lib/opensc-pkcs11.so'
test = {
    "alive": True
}

class CardInsertedHook(smartcard.CardMonitoring.CardObserver):
    def __init__(self, on_insert=None, on_eject=None):
        self.on_insert = on_insert
        self.on_eject = on_eject

    def update(self, observable, actions):
        (addedcards, removedcards) = actions
        if addedcards and self.on_insert:
            self.on_insert(addedcards)
        if removedcards and self.on_eject:
            self.on_eject(removedcards)

class CardMonitor(threading.Thread):
    def __init__(self, on_insert=None, on_eject=None):
        # Create an instance of the CardMonitor
        cardmonitor = smartcard.CardMonitoring.CardMonitor()

        # Add your observer to the CardMonitor
        cardobserver = CardInsertedHook(on_insert, on_eject)
        cardmonitor.addObserver(cardobserver)

        threading.Thread.__init__(self)
        self.start()

    def run(self):
        # We stay alive as long as our overlord says to stay alive
        while test['alive']:
            time.sleep(0.25)

class YubikeySession:
    def __init__(self):
        self.session = None
        self.share_holders = {}

        # Initialize the library
        self.pkcs11 = PyKCS11Lib()
        self.pkcs11.load(pkcs11_lib)

    def sign(self, data :bytes):
        print(f"Signing {data} with {self.session}")

    def decrypt(self, data :bytes):
        print(f"Decrypt {data} with {self.session}")

    def on_insert(self, cards):
        for device in self.pkcs11.getSlotList():
            try:
                _tmp_session = self.pkcs11.openSession(device, CKF_SERIAL_SESSION | CKF_RW_SESSION)
            except PyKCS11.PyKCS11Error as error:
                if error.value == 224:
                    continue
                raise error

            try:
                _tmp_session.login(yubikey_pin)
            except PyKCS11.PyKCS11Error as error:
                if error.value == 256:
                    # Already logged in
                    pass
                else:
                    raise error

            certificates = _tmp_session.findObjects([
                (CKA_CLASS, CKO_CERTIFICATE),
            ])

            for certificate in certificates:
                _tmp_cert_der = bytes(_tmp_session.getAttributeValue(certificate, [CKA_VALUE])[0])

            _tmp_private_keys = _tmp_session.findObjects([
                (CKA_CLASS, CKO_PRIVATE_KEY),
            ])

            for _tmp_private_key in _tmp_private_keys:
                attributes = _tmp_session.getAttributeValue(_tmp_private_key, [CKA_ID])

                _tmp_cert = cryptography.x509.load_der_x509_certificate(_tmp_cert_der)
                _tmp_fingerprint = _tmp_cert.fingerprint(cryptography.hazmat.primitives.hashes.SHA512())
                break # .. todo:: make sure this belongs to the cert we found

            print(f"Has detected Yubikey for {_tmp_cert.subject.get_attributes_for_oid(cryptography.x509.NameOID.COMMON_NAME)[0].value}")

            self.session = _tmp_session
            break

        if self.session:
            self.sign(b"data", "This should break things")

    def on_eject(self, cards):
        if self.session:
            self.session.logout()
            self.session.closeSession()
            self.session = None

yubikey = YubikeySession()
sc_monitor = CardMonitor(on_insert=yubikey.on_insert, on_eject=yubikey.on_eject)

time.sleep(2)

yubikey.on_eject([])
test['alive'] = False

[LudovicRousseau]

Fixed. Thanks

If you are waiting for a PKCS#11 slot insertion or removal I would suggest to use waitForSlotEvent() https://pkcs11wrap.sourceforge.io/api/api.html#PyKCS11.PyKCS11Lib.waitForSlotEvent from PyKCS11. No need to use PySCard for that.

See Get token events sample code: https://github.com/LudovicRousseau/PyKCS11/blob/master/samples/events.py

[Torxed]

No worries, and great work but also thanks for being patient!

And waitForSlotEvent() is spot on what I need by the looks of it, will for sure give that a go. Thanks for nudging me in the right direction, looks a lot cleaner than what I'm doing currently.