Merge branch '295-switching-id-fails'

Make switching ID more robust

Release USB handle after use
Set up proper smart card services restart

Fixes #295

pynitrokey/cli/start.py

@@ -25,6 +25,7 @@
     IS_LINUX,
     kill_smartcard_services,
     logger,
+    restart_smartcard_services,
     show_kdf_details,
     start_update,
     validate_gnuk,
@@ -94,27 +95,48 @@ def rng(count, raw, quiet):
 
 @click.command()
 @click.argument("identity")
-def set_identity(identity):
-    """set given identity (one of: 0, 1, 2)"""
+@click.option(
+    "--force-restart",
+    is_flag=True,
+    help="Force pcscd and GnuPG services restart once finished",
+)
+def set_identity(identity, force_restart):
+    """Set given identity (one of: 0, 1, 2)
+
+    Might require stopping other smart card services to connect directly to the device over CCID interface.
+    These will be restarted after operation, if it is required.
+
+    This could be replaced with:
+
+    gpg-connect-agent "SCD APDU 00 85 00 0<IDENTITY>
+    """
     if not identity.isdigit():
         local_critical("identity number must be a digit")
 
     identity = int(identity)
     if identity < 0 or identity > 2:
         local_critical("identity must be 0, 1 or 2")
 
+    kill_done = False
     local_print(f"Setting identity to {identity}")
     for x in range(3):
         try:
             gnuk = get_gnuk_device()
-            gnuk.cmd_select_openpgp()
-            try:
-                gnuk.cmd_set_identity(identity)
-            except USBError:
-                local_print(f"reset done - now active identity: {identity}")
-                break
+            with gnuk.release_on_exit() as gnuk:
+                gnuk.cmd_select_openpgp()
+                try:
+                    gnuk.cmd_set_identity(identity)
+                except USBError:
+                    local_print(f"Reset done - now active identity: {identity}")
+                    break
 
         except OnlyBusyICCError:
+            # Handle is occupied. Try to close pcscd and try again.
+            if not kill_done:
+                kill_smartcard_services("pcscd")
+                kill_done = True
+                continue
+
             local_print(
                 "Identity not changed - device is busy or the selected identity is already active"
             )
@@ -129,6 +151,9 @@ def set_identity(identity):
                 local_critical(e)
         except Exception as e:
             local_critical(e)
+    if kill_done or force_restart:
+        sleep(1)
+        restart_smartcard_services("pcscd")
 
 
 @click.command()

pynitrokey/start/gnuk_token.py

@@ -24,6 +24,7 @@
 import logging
 import time
 from array import array
+from contextlib import contextmanager
 from struct import *
 
 import usb
@@ -133,6 +134,17 @@ def reset_device(self):
     def release_gnuk(self):
         self.__devhandle.releaseInterface()
 
+    @contextmanager
+    def release_on_exit(self, *args, **kwds):
+        """
+        Release GNUK device once finished, to not block other smart card services
+        trying to claim it
+        """
+        try:
+            yield self
+        finally:
+            self.release_gnuk()
+
     def stop_gnuk(self):
         self.__devhandle.releaseInterface()
         if self.__hid_intf:
@@ -734,7 +746,7 @@ def get_gnuk_device(verbose=True, logger: logging.Logger = None):
         except Exception:
             pass
     if not icc and candidates:
-        raise OnlyBusyICCError("Found only busy ICC")
+        raise OnlyBusyICCError(f"Found only busy ICC: {candidates}")
     if not icc:
         raise ValueError("No ICC present")
     status = icc.icc_get_status()

pynitrokey/start/upgrade_by_passwd.py

@@ -340,36 +340,47 @@ def validate_regnual(ctx, param, path: str):
     return path
 
 
-def kill_smartcard_services():
-    local_print("Could not connect to the device. Attempting to close scdaemon.")
+def kill_smartcard_services(filter_word=None):
+    local_print(
+        "Could not connect to the device. Attempting to close other smart card services."
+    )
 
     commands = [
         ("gpgconf --kill all".split(), True),
         ("sudo systemctl stop pcscd pcscd.socket".split(), IS_LINUX),
     ]
+    if filter_word:
+        commands = filter_commands(commands, filter_word)
 
     try_to_run_commands(commands)
 
 
-def restart_smartcard_services():
-    local_print("*** Restarting smartcard services")
+def filter_commands(commands, filter_word):
+    commands = list(filter(lambda x: filter_word in x[0], commands))
+    return commands
+
+
+def restart_smartcard_services(filter_word=None):
+    local_print("Restarting smartcard services")
     commands = [
-        ("gpgconf --reload all".split(), True),
         ("sudo systemctl restart pcscd pcscd.socket".split(), IS_LINUX),
+        ("gpgconf --reload all".split(), True),
     ]
+    if filter_word:
+        commands = filter_commands(commands, filter_word)
     try_to_run_commands(commands)
 
 
 def try_to_run_commands(commands):
     for command, flag in commands:
         if not flag:
             continue
-        local_print(f"Running: {' '.join(command)}")
+        local_print(f"*** Running: {' '.join(command)}")
         try:
             check_output(command)
         except Exception as e:
-            local_print("Error while running command", e)
-    time.sleep(3)
+            local_print("*** Error while running command", e)
+        time.sleep(1)
 
 
 # @fixme: maybe also move to confconsts.py?