From 518b7d479e599601578dea2b211a41c7919e7501 Mon Sep 17 00:00:00 2001 From: Szczepan Zalega Date: Sat, 17 Dec 2022 15:17:06 +0100 Subject: [PATCH 1/7] NK Start: use pcscd and gpg-connect-agent for switching ids --- pynitrokey/cli/start.py | 131 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 122 insertions(+), 9 deletions(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 81183379..22976e55 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -6,13 +6,15 @@ # http://apache.org/licenses/LICENSE-2.0> or the MIT license , at your option. This file may not be # copied, modified, or distributed except according to those terms. - - +import binascii +import typing from subprocess import check_output from sys import stderr, stdout from time import sleep import click +from smartcard.Exceptions import CardConnectionException +from smartcard.pcsc.PCSCExceptions import EstablishContextException from tqdm import tqdm from usb.core import USBError @@ -93,14 +95,81 @@ def rng(count, raw, quiet): print(challenge.hex()) +class CardRemovedGPGAgentException(RuntimeWarning): + pass + + +def gpg_agent_set_identity(identity: int): + from pexpect import run + + cmd = f"gpg-connect-agent 'SCD APDU 00 85 00 0{identity}' /bye" + app = run(cmd) + print(cmd) + print(app) + if b"ERR 100663406 Card removed" in app: + raise CardRemovedGPGAgentException("Card removed") + + +def pcsc_set_identity(identity): + try: + from smartcard import System + from smartcard.CardConnection import CardConnection + from smartcard.Exceptions import NoCardException + + def find_smartcard(uuid: typing.Optional[int] = None) -> CardConnection: + for reader in System.readers(): + if "Nitrokey Start" not in str(reader): + continue + conn = reader.createConnection() + try: + conn.connect() + except NoCardException: + continue + # use this for debug + # sudo pcscd -f -a + + # if not select(conn, AID_ADMIN): + # continue + # data, sw1, sw2 = conn.transmit([0x00, 0x62, 0x00, 0x00, 16]) + print(reader) + print(conn) + return conn + # raise Exception(f"No smartcard with UUID {uuid:X} found") + + def select(conn: CardConnection, aid: bytes) -> bool: + apdu = [0x00, 0xA4, 0x04, 0x00] + apdu.append(len(aid)) + apdu.extend(aid) + _, sw1, sw2 = conn.transmit(apdu) + return (sw1, sw2) == (0x90, 0x00) + + def send_id_change(conn: CardConnection, identity: int) -> None: + # 00 85 00 02 + out = [0x00, 0x85, 0x00, identity] + for i in range(5): + data, sw1, sw2 = conn.transmit(out) + print((bytes(out).hex(), data, hex(sw1), hex(sw2))) + res = bytes([sw1, sw2]).hex() + if res == "9000": + print("success") + break + if res == "6a88": + print(f"error: {res}") + continue + + conn = find_smartcard() + aid = binascii.a2b_hex("D276:0001:2401".replace(":", "")) + select(conn, aid) + send_id_change(conn, identity) + + except ImportError: + logger.debug("pcsc feature is deactivated, skipping firmware mode test") + pass + + @click.command() @click.argument("identity") -@click.option( - "--force-restart", - is_flag=True, - help="Force pcscd and GnuPG services restart once finished", -) -def set_identity(identity, force_restart): +def set_identity(identity): """Set given identity (one of: 0, 1, 2) Might require stopping other smart card services to connect directly to the device over CCID interface. @@ -118,6 +187,50 @@ def set_identity(identity, force_restart): local_critical("identity must be 0, 1 or 2") local_print(f"Setting identity to {identity}") + + # Note: the error in communication coming after changing the identity is caused by the immediate restart of + # the device, without responding to the call. The idea was to avoid operating with a potentially inconsistent state in + # the memory. + def inner(): + """ + Call all the methods in the order of the success chance + """ + + # this works when gpg has opened connection, stops after changing identity with it + try: + gpg_agent_set_identity(identity) + return True + except CardRemovedGPGAgentException: + # this error shows up when the identity was just changed with gpg, and the new state was not reloaded + pass + + # this works when gpg has no connection, but pcsc server is working + try: + pcsc_set_identity(identity) + print(f"PCSC change works") + return True + except CardConnectionException as e: + print(f"Expected error. PCSC reports {e}") + # this error is expected after sucessfully changing the identity + return True + except EstablishContextException: + # pcscd must not work, try another method + local_print("pcscd must not work, try another method") + + # this works, when neither gnupg nor opensc is claiming the smart card interface + try: + set_identity_raw(identity) + except: + raise + + inner() + # apparently calling it 2 times reloads the gnupg, and allows for immediate use of it after changing the identity + # otherwise its reload is needed with gpgconf --reload all + inner() + local_print(f"Reset done - now active identity: {identity}") + + +def set_identity_raw(identity): for x in range(3): try: gnuk = get_gnuk_device() @@ -127,7 +240,7 @@ def set_identity(identity, force_restart): gnuk.cmd_set_identity(identity) break except USBError: - local_print(f"Reset done - now active identity: {identity}") + # local_print(f"Reset done - now active identity: {identity}") break except OnlyBusyICCError: From 99953d060e081be6b2252abeb848da1235c61b13 Mon Sep 17 00:00:00 2001 From: Szczepan Zalega Date: Sat, 17 Dec 2022 15:21:58 +0100 Subject: [PATCH 2/7] Add guard for the pcsc feature import --- pynitrokey/cli/start.py | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 22976e55..2516e4a3 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -8,13 +8,9 @@ # copied, modified, or distributed except according to those terms. import binascii import typing -from subprocess import check_output from sys import stderr, stdout -from time import sleep import click -from smartcard.Exceptions import CardConnectionException -from smartcard.pcsc.PCSCExceptions import EstablishContextException from tqdm import tqdm from usb.core import USBError @@ -204,18 +200,24 @@ def inner(): # this error shows up when the identity was just changed with gpg, and the new state was not reloaded pass - # this works when gpg has no connection, but pcsc server is working try: - pcsc_set_identity(identity) - print(f"PCSC change works") - return True - except CardConnectionException as e: - print(f"Expected error. PCSC reports {e}") - # this error is expected after sucessfully changing the identity - return True - except EstablishContextException: - # pcscd must not work, try another method - local_print("pcscd must not work, try another method") + from smartcard.Exceptions import CardConnectionException + from smartcard.pcsc.PCSCExceptions import EstablishContextException + + # this works when gpg has no connection, but pcsc server is working + try: + pcsc_set_identity(identity) + print(f"PCSC change works") + return True + except CardConnectionException as e: + print(f"Expected error. PCSC reports {e}") + # this error is expected after sucessfully changing the identity + return True + except EstablishContextException: + # pcscd must not work, try another method + local_print("pcscd must not work, try another method") + except ImportError: + local_print("pcsc feature is deactivated, skipping this switching method") # this works, when neither gnupg nor opensc is claiming the smart card interface try: From 9306b896384374cfd3a52320cb72ad8b83e077b8 Mon Sep 17 00:00:00 2001 From: Szczepan Zalega Date: Sat, 17 Dec 2022 17:41:17 +0100 Subject: [PATCH 3/7] Cleanup implementation. Use logger instead of print. --- pynitrokey/cli/start.py | 83 ++++++++++++++++++++-------------------- pynitrokey/confconsts.py | 9 +++++ 2 files changed, 51 insertions(+), 41 deletions(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 2516e4a3..80f63d41 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -14,6 +14,7 @@ from tqdm import tqdm from usb.core import USBError +from pynitrokey.confconsts import logger from pynitrokey.helpers import local_critical, local_print from pynitrokey.start.gnuk_token import OnlyBusyICCError, get_gnuk_device from pynitrokey.start.threaded_log import ThreadLog @@ -21,9 +22,6 @@ DEFAULT_PW3, DEFAULT_WAIT_FOR_REENUMERATION, IS_LINUX, - kill_smartcard_services, - logger, - restart_smartcard_services, show_kdf_details, start_update, validate_gnuk, @@ -95,14 +93,18 @@ class CardRemovedGPGAgentException(RuntimeWarning): pass +class NoCardPCSCException(RuntimeWarning): + pass + + def gpg_agent_set_identity(identity: int): - from pexpect import run + from subprocess import check_output as run - cmd = f"gpg-connect-agent 'SCD APDU 00 85 00 0{identity}' /bye" - app = run(cmd) - print(cmd) - print(app) - if b"ERR 100663406 Card removed" in app: + cmd = f"gpg-connect-agent" f"|SCD APDU 00 85 00 0{identity}" f"|/bye".split("|") + app_out = run(cmd) + logger.debug(cmd) + logger.debug(app_out) + if b"ERR 100663406 Card removed" in app_out: raise CardRemovedGPGAgentException("Card removed") @@ -112,25 +114,22 @@ def pcsc_set_identity(identity): from smartcard.CardConnection import CardConnection from smartcard.Exceptions import NoCardException - def find_smartcard(uuid: typing.Optional[int] = None) -> CardConnection: + def find_smartcard(name: typing.Optional[str] = None) -> CardConnection: + # use this for debug: sudo pcscd -f -a for reader in System.readers(): - if "Nitrokey Start" not in str(reader): + if "Nitrokey Start" not in str(reader) or ( + name and name not in str(reader) + ): continue + conn: CardConnection conn = reader.createConnection() try: conn.connect() except NoCardException: continue - # use this for debug - # sudo pcscd -f -a - - # if not select(conn, AID_ADMIN): - # continue - # data, sw1, sw2 = conn.transmit([0x00, 0x62, 0x00, 0x00, 16]) - print(reader) - print(conn) + logger.debug([reader, conn]) return conn - # raise Exception(f"No smartcard with UUID {uuid:X} found") + raise NoCardPCSCException(f"No smartcard with UUID {name} found") def select(conn: CardConnection, aid: bytes) -> bool: apdu = [0x00, 0xA4, 0x04, 0x00] @@ -140,17 +139,15 @@ def select(conn: CardConnection, aid: bytes) -> bool: return (sw1, sw2) == (0x90, 0x00) def send_id_change(conn: CardConnection, identity: int) -> None: - # 00 85 00 02 out = [0x00, 0x85, 0x00, identity] - for i in range(5): + for i in range(2): data, sw1, sw2 = conn.transmit(out) - print((bytes(out).hex(), data, hex(sw1), hex(sw2))) + logger.debug((bytes(out).hex(), data, hex(sw1), hex(sw2))) res = bytes([sw1, sw2]).hex() if res == "9000": - print("success") break if res == "6a88": - print(f"error: {res}") + logger.debug(f"error: {res}") continue conn = find_smartcard() @@ -159,8 +156,7 @@ def send_id_change(conn: CardConnection, identity: int) -> None: send_id_change(conn, identity) except ImportError: - logger.debug("pcsc feature is deactivated, skipping firmware mode test") - pass + logger.debug("pcsc feature is deactivated, skipping this method") @click.command() @@ -187,15 +183,16 @@ def set_identity(identity): # Note: the error in communication coming after changing the identity is caused by the immediate restart of # the device, without responding to the call. The idea was to avoid operating with a potentially inconsistent state in # the memory. - def inner(): + def inner() -> None: """ Call all the methods in the order of the success chance """ # this works when gpg has opened connection, stops after changing identity with it try: + logger.info("Trying changing identity through gpg-agent") gpg_agent_set_identity(identity) - return True + return except CardRemovedGPGAgentException: # this error shows up when the identity was just changed with gpg, and the new state was not reloaded pass @@ -206,21 +203,25 @@ def inner(): # this works when gpg has no connection, but pcsc server is working try: + logger.info("Trying changing identity through pcscd") pcsc_set_identity(identity) - print(f"PCSC change works") - return True + logger.info( + f"Device returns success. Identity is already set to {identity}." + ) + return except CardConnectionException as e: - print(f"Expected error. PCSC reports {e}") - # this error is expected after sucessfully changing the identity - return True - except EstablishContextException: - # pcscd must not work, try another method - local_print("pcscd must not work, try another method") + logger.debug(f"Expected error. PCSC reports {e}") + return + except (EstablishContextException, NoCardPCSCException) as e: + logger.debug( + f"pcscd must not work, try another method. Reported error: {e}" + ) except ImportError: local_print("pcsc feature is deactivated, skipping this switching method") # this works, when neither gnupg nor opensc is claiming the smart card interface try: + logger.info("Trying changing identity through direct connection") set_identity_raw(identity) except: raise @@ -233,7 +234,7 @@ def inner(): def set_identity_raw(identity): - for x in range(3): + for x in range(1): try: gnuk = get_gnuk_device() with gnuk.release_on_exit() as gnuk: @@ -242,7 +243,7 @@ def set_identity_raw(identity): gnuk.cmd_set_identity(identity) break except USBError: - # local_print(f"Reset done - now active identity: {identity}") + logger.debug(f"Reset done - now active identity: {identity}") break except OnlyBusyICCError: @@ -252,8 +253,8 @@ def set_identity_raw(identity): break except ValueError as e: if "No ICC present" in str(e): - local_print( - "Device is occupied by some other service and cannot be connected to. Identity not changed." + local_critical( + "Device is not connected or occupied by some other service and cannot be connected to. Identity not changed." ) else: local_critical(e) diff --git a/pynitrokey/confconsts.py b/pynitrokey/confconsts.py index 125fdbad..99171478 100644 --- a/pynitrokey/confconsts.py +++ b/pynitrokey/confconsts.py @@ -36,6 +36,8 @@ class Verbosity(IntEnum): f"environment variable: '{ENV_DEBUG_VAR}' invalid, " f"setting default: {VERBOSE.name} = {VERBOSE.value}" ) + print(f"Found {ENV_DEBUG_VAR}='{_env_dbg_lvl}'. Setting VERBOSE={VERBOSE}") + LOG_FN = tempfile.NamedTemporaryFile(prefix="nitropy.log.").name LOG_FORMAT_STDOUT = "%(asctime)-15s %(levelname)6s %(name)10s %(message)s" @@ -47,3 +49,10 @@ class Verbosity(IntEnum): UDEV_URL = ( "https://docs.nitrokey.com/nitrokey3/linux/firmware-update.html#troubleshooting" ) + + +logger = logging.getLogger(__name__) +stream_handler = logging.StreamHandler() +stream_handler.setLevel(VERBOSE) +stream_handler.setFormatter(logging.Formatter(LOG_FORMAT_STDOUT)) +logger.addHandler(stream_handler) From 8818cd5cbb0f97d1cef6332ba4e6c1b554998141 Mon Sep 17 00:00:00 2001 From: Szczepan Zalega Date: Sat, 17 Dec 2022 18:02:42 +0100 Subject: [PATCH 4/7] Remove mypy_cache on clean --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index dde440c8..b7e7fa17 100644 --- a/Makefile +++ b/Makefile @@ -55,6 +55,7 @@ semi-clean: clean: semi-clean rm -rf $(VENV) rm -rf dist + rm -rf ./.mypy_cache/ # Package management From ae1e375297b95723922e38d01b77ee31bb4aa4aa Mon Sep 17 00:00:00 2001 From: Szczepan Zalega Date: Sat, 17 Dec 2022 17:41:33 +0100 Subject: [PATCH 5/7] Mypy: ignore smartcard.pcsc related errors Ignore smartcard.pcsc types Ignore this particular mypy error --- pynitrokey/cli/start.py | 2 +- pyproject.toml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 80f63d41..7246591d 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -198,7 +198,7 @@ def inner() -> None: pass try: - from smartcard.Exceptions import CardConnectionException + from smartcard.Exceptions import CardConnectionException # type: ignore from smartcard.pcsc.PCSCExceptions import EstablishContextException # this works when gpg has no connection, but pcsc server is working diff --git a/pyproject.toml b/pyproject.toml index 7aa8c8ce..cb9998d1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -97,5 +97,6 @@ module = [ "usb1.*", "tlv8.*", "pytest.*", + "smartcard.pcsc.*", ] ignore_missing_imports = true From 8767cf3f6dc584065385369aaa423bdbb0984ac4 Mon Sep 17 00:00:00 2001 From: Szczepan Zalega Date: Sat, 17 Dec 2022 19:49:04 +0100 Subject: [PATCH 6/7] Support another gpg-agent error --- pynitrokey/cli/start.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 7246591d..82b099e5 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -7,6 +7,7 @@ # http://opensource.org/licenses/MIT>, at your option. This file may not be # copied, modified, or distributed except according to those terms. import binascii +import logging import typing from sys import stderr, stdout @@ -16,7 +17,7 @@ from pynitrokey.confconsts import logger from pynitrokey.helpers import local_critical, local_print -from pynitrokey.start.gnuk_token import OnlyBusyICCError, get_gnuk_device +from pynitrokey.start.gnuk_token import OnlyBusyICCError, get_gnuk_device, gnuk_token from pynitrokey.start.threaded_log import ThreadLog from pynitrokey.start.upgrade_by_passwd import ( DEFAULT_PW3, @@ -93,6 +94,10 @@ class CardRemovedGPGAgentException(RuntimeWarning): pass +class ServiceNotRunningGPGAgentException(RuntimeWarning): + pass + + class NoCardPCSCException(RuntimeWarning): pass @@ -106,6 +111,8 @@ def gpg_agent_set_identity(identity: int): logger.debug(app_out) if b"ERR 100663406 Card removed" in app_out: raise CardRemovedGPGAgentException("Card removed") + if b"ERR 100663614 Service is not running" in app_out: + raise ServiceNotRunningGPGAgentException() def pcsc_set_identity(identity): @@ -193,7 +200,7 @@ def inner() -> None: logger.info("Trying changing identity through gpg-agent") gpg_agent_set_identity(identity) return - except CardRemovedGPGAgentException: + except (CardRemovedGPGAgentException, ServiceNotRunningGPGAgentException): # this error shows up when the identity was just changed with gpg, and the new state was not reloaded pass From c1534dfa4595876169136ba16d26fd06190f4d00 Mon Sep 17 00:00:00 2001 From: Szczepan Zalega Date: Thu, 29 Dec 2022 10:11:34 +0100 Subject: [PATCH 7/7] WIP SIGN try to get serial number data by reading AID file --- pynitrokey/cli/start.py | 4 +++- pynitrokey/start/gnuk_token.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 82b099e5..1d55f653 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -243,10 +243,12 @@ def inner() -> None: def set_identity_raw(identity): for x in range(1): try: - gnuk = get_gnuk_device() + gnuk: gnuk_token = get_gnuk_device() with gnuk.release_on_exit() as gnuk: gnuk.cmd_select_openpgp() try: + aid = gnuk.cmd_read_binary(0x004F, pre=0) + logging.debug(f"{aid=}") gnuk.cmd_set_identity(identity) break except USBError: diff --git a/pynitrokey/start/gnuk_token.py b/pynitrokey/start/gnuk_token.py index 4a647f79..abe8ba0d 100644 --- a/pynitrokey/start/gnuk_token.py +++ b/pynitrokey/start/gnuk_token.py @@ -299,8 +299,8 @@ def cmd_verify(self, who, passwd): raise ValueError("%02x%02x" % (sw[0], sw[1])) return True - def cmd_read_binary(self, fileid): - cmd_data = iso7816_compose(0xB0, 0x80 + fileid, 0x00, b"") + def cmd_read_binary(self, fileid, pre=0x80): + cmd_data = iso7816_compose(0xB0, pre + fileid, 0x00, b"") sw = self.icc_send_cmd(cmd_data) if len(sw) != 2: raise ValueError(sw)