Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nitrokey Start ID switching without sudo #305

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ semi-clean:
clean: semi-clean
rm -rf $(VENV)
rm -rf dist
rm -rf ./.mypy_cache/


# Package management
Expand Down
163 changes: 144 additions & 19 deletions pynitrokey/cli/start.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,23 @@
# http://apache.org/licenses/LICENSE-2.0> or the MIT license <LICENSE-MIT or
# http://opensource.org/licenses/MIT>, at your option. This file may not be
# copied, modified, or distributed except according to those terms.


from subprocess import check_output
import binascii
import logging
import typing
from sys import stderr, stdout
from time import sleep

import click
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.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,
DEFAULT_WAIT_FOR_REENUMERATION,
IS_LINUX,
kill_smartcard_services,
logger,
restart_smartcard_services,
show_kdf_details,
start_update,
validate_gnuk,
Expand Down Expand Up @@ -93,14 +90,85 @@ def rng(count, raw, quiet):
print(challenge.hex())


class CardRemovedGPGAgentException(RuntimeWarning):
pass


class ServiceNotRunningGPGAgentException(RuntimeWarning):
pass


class NoCardPCSCException(RuntimeWarning):
pass


def gpg_agent_set_identity(identity: int):
from subprocess import check_output as run
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d prefer removing that rename because there also is subprocess.run.


cmd = f"gpg-connect-agent" f"|SCD APDU 00 85 00 0{identity}" f"|/bye".split("|")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need an f-string for all of those strings.
Also, it might be worth providing a string list instead.

app_out = run(cmd)
logger.debug(cmd)
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):
try:
from smartcard import System
from smartcard.CardConnection import CardConnection
from smartcard.Exceptions import NoCardException

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) or (
name and name not in str(reader)
):
continue
conn: CardConnection
conn = reader.createConnection()
try:
conn.connect()
except NoCardException:
continue
logger.debug([reader, conn])
return conn
raise NoCardPCSCException(f"No smartcard with UUID {name} 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:
out = [0x00, 0x85, 0x00, identity]
for i in range(2):
data, sw1, sw2 = conn.transmit(out)
logger.debug((bytes(out).hex(), data, hex(sw1), hex(sw2)))
res = bytes([sw1, sw2]).hex()
if res == "9000":
break
if res == "6a88":
logger.debug(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 this method")


@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.
Expand All @@ -118,16 +186,73 @@ def set_identity(identity, force_restart):
local_critical("identity must be 0, 1 or 2")

local_print(f"Setting identity to {identity}")
for x in range(3):

# 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() -> 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
except (CardRemovedGPGAgentException, ServiceNotRunningGPGAgentException):
# this error shows up when the identity was just changed with gpg, and the new state was not reloaded
pass

try:
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
try:
logger.info("Trying changing identity through pcscd")
pcsc_set_identity(identity)
logger.info(
f"Device returns success. Identity is already set to {identity}."
)
return
except CardConnectionException as e:
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

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(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:
local_print(f"Reset done - now active identity: {identity}")
logger.debug(f"Reset done - now active identity: {identity}")
break

except OnlyBusyICCError:
Expand All @@ -137,8 +262,8 @@ def set_identity(identity, force_restart):
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)
Expand Down
9 changes: 9 additions & 0 deletions pynitrokey/confconsts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Comment on lines +54 to +58
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default verbosity level should probably be higher than INFO to avoid printing to console.

4 changes: 2 additions & 2 deletions pynitrokey/start/gnuk_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,6 @@ module = [
"usb1.*",
"tlv8.*",
"pytest.*",
"smartcard.pcsc.*",
]
ignore_missing_imports = true