From dab337aff8acb39436311b76cca3859b73a9d653 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 19 Jul 2022 17:26:33 +0200 Subject: [PATCH 1/4] start update: Require confirmation for SIGINT With this patch, we register a custom signal handler for the start update subcommand that asks the user for confirmation if they trigger a SIGINT signal, for example by pressing Ctrl+C. --- pynitrokey/cli/start.py | 11 ++++++----- pynitrokey/helpers.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 7418d039..833cdc83 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -16,7 +16,7 @@ from tqdm import tqdm from usb.core import USBError -from pynitrokey.helpers import local_critical, local_print +from pynitrokey.helpers import confirm_keyboard_interrupt, local_critical, local_print from pynitrokey.start.gnuk_token import get_gnuk_device from pynitrokey.start.threaded_log import ThreadLog from pynitrokey.start.upgrade_by_passwd import ( @@ -198,11 +198,12 @@ def update( "use one from: https://github.com/Nitrokey/nitrokey-start-firmware)", ) - if IS_LINUX: - with ThreadLog(logger.getChild("dmesg"), "dmesg -w"): + with confirm_keyboard_interrupt("Cancelling the update may brick your device."): + if IS_LINUX: + with ThreadLog(logger.getChild("dmesg"), "dmesg -w"): + start_update(*args) + else: start_update(*args) - else: - start_update(*args) @click.command() diff --git a/pynitrokey/helpers.py b/pynitrokey/helpers.py index 79f2badd..724942e5 100644 --- a/pynitrokey/helpers.py +++ b/pynitrokey/helpers.py @@ -11,12 +11,14 @@ import logging import os import platform +import signal import sys import time +from contextlib import contextmanager from getpass import getpass from numbers import Number from threading import Event, Timer -from typing import List, Optional +from typing import Any, Iterator, List, Optional import click from tqdm import tqdm @@ -348,3 +350,30 @@ def ask(self): confirm = functools.partial(click.confirm, err=True) prompt = functools.partial(click.prompt, err=True) + + +@contextmanager +def confirm_keyboard_interrupt(msg: Optional[str] = None) -> Iterator[None]: + """ + Registers a signal handler for SIGINT (i. e. Ctrl+C) that asks the user to confirm before + raising a KeyboardInterrupt. + + If used as a context manager, it resets the signal handler after the execution. The given + message is appended to the confirmation prompt. + """ + + def handle_sigint(signum: int, frame: Any) -> None: + text = "Do you really want to stop nitropy?" + if msg: + text += " " + text += msg + if confirm(text): + raise KeyboardInterrupt + + handler = signal.signal(signal.SIGINT, handle_sigint) + try: + yield + except Exception: + raise + finally: + signal.signal(signal.SIGINT, handler) From a0aa8dec7b7bfd313a9153bfa4f77f9c05003572 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 19 Jul 2022 19:10:31 +0200 Subject: [PATCH 2/4] start update: Improve error messages This patch improves the error messages for the Nitrokey Start update process so that it no longer recommends device reinsertion. Instead, users should try to repeat the update if it fails. If they remove the device after an unsuccessful update, they risk bricking the device. --- pynitrokey/start/upgrade_by_passwd.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pynitrokey/start/upgrade_by_passwd.py b/pynitrokey/start/upgrade_by_passwd.py index d0188714..73436230 100755 --- a/pynitrokey/start/upgrade_by_passwd.py +++ b/pynitrokey/start/upgrade_by_passwd.py @@ -590,6 +590,7 @@ def start_update( "- all data will be removed from the device!", "- do not interrupt update process - the device may not run properly!", "- the process should not take more than 1 minute", + "- if the update fails, do not remove the device! Repeat the update instead.", ) if yes: local_print("Accepted automatically") @@ -651,9 +652,9 @@ def start_update( "", "Could not proceed with the update", "Please execute one or all of the following and try again:", - "- re-insert device to the USB slot", "- run factory-reset on the device", "- close other applications, which could use it (e.g., scdaemon, pcscd)", + "- repeat the update", ) dev_strings_upgraded = None @@ -662,7 +663,10 @@ def start_update( for i in range(TIME_DETECT_DEVICE_AFTER_UPDATE_S): if i > TIME_DETECT_DEVICE_AFTER_UPDATE_LONG_S: if not takes_long_time: - local_print("", "Please reinsert device to the USB slot") + local_print( + "", + "If you have removed the device, please reinsert it to the USB slot", + ) takes_long_time = True time.sleep(1) dev_strings_upgraded = get_devices() @@ -673,11 +677,12 @@ def start_update( local_print(".", end="", flush=True) if not dev_strings_upgraded: - local_print( + local_critical( "", "could not connect to the device - might be due to a failed update", - "please re-insert the device, check version using:", + "please check the device version with:", "$ nitropy start list", + "and repeat the update if necessary", ) local_print( From e4ac7a32105d8f418d47ce2bdf085ba9d1f5223e Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 21 Jul 2022 19:39:00 +0200 Subject: [PATCH 3/4] =?UTF-8?q?start:=20Don=E2=80=99t=20use=20pre-releases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this patch, we replace the /releases API endpoint with /releases/latest. This endpoint only returns proper releases, not pre-releases, so users are no longer offered firmware release candidates. --- pynitrokey/start/upgrade_by_passwd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pynitrokey/start/upgrade_by_passwd.py b/pynitrokey/start/upgrade_by_passwd.py index 73436230..0d8aa66c 100755 --- a/pynitrokey/start/upgrade_by_passwd.py +++ b/pynitrokey/start/upgrade_by_passwd.py @@ -279,7 +279,7 @@ def get_latest_release_data(): try: # @todo: move to confconsts.py r = requests.get( - "https://api.github.com/repos/Nitrokey/nitrokey-start-firmware/releases" + "https://api.github.com/repos/Nitrokey/nitrokey-start-firmware/releases/latest" ) json = r.json() if r.status_code == 403: @@ -287,7 +287,7 @@ def get_latest_release_data(): f"JSON raw data: {json}", f"No Github API access, status code: {r.status_code}", ) - latest_tag = json[0] + latest_tag = json except Exception as e: local_critical("Failed getting release data", e) From a489cb4e8cbc2d642c6db9b168adeb4fcecda0bb Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 21 Jul 2022 19:57:05 +0200 Subject: [PATCH 4/4] start: Check for udev rules before update --- pynitrokey/cli/start.py | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/pynitrokey/cli/start.py b/pynitrokey/cli/start.py index 833cdc83..cec0e0ee 100644 --- a/pynitrokey/cli/start.py +++ b/pynitrokey/cli/start.py @@ -7,7 +7,9 @@ # http://opensource.org/licenses/MIT>, at your option. This file may not be # copied, modified, or distributed except according to those terms. - +import fnmatch +import os +import os.path from subprocess import check_output from sys import stderr, stdout from time import sleep @@ -164,6 +166,12 @@ def set_identity(identity): default=False, help="Use firmware for early 'Nitrokey Start' key hardware revisions", ) +@click.option( + "--force", + is_flag=True, + default=False, + help="Execute the firmware update even if environment sanity checks fail", +) def update( regnual, gnuk, @@ -175,9 +183,24 @@ def update( yes, skip_bootloader, green_led, + force, ): """update device's firmware""" + if not find_udev_rules(): + if force: + local_print( + "Warning: Could not find Nitrokey udev rules but will continue anyway as --force is set." + ) + else: + local_critical( + "Failed to find Nitrokey udev rules. These udev rules are required for the update.", + "Please see the nitropy documentation for information on installing these rules:", + " https://docs.nitrokey.com/software/nitropy/linux/udev.html", + "If you want to continue anyway, you can use the --force option.", + support_hint=False, + ) + args = ( regnual, gnuk, @@ -206,6 +229,22 @@ def update( start_update(*args) +def find_udev_rules() -> bool: + dirs = [ + "/usr/lib/udev/rules.d", + "/usr/local/lib/udev/rules.d", + "/run/udev/rules.d", + "/etc/udev/rules.d", + ] + for d in dirs: + if os.path.isdir(d): + for name in os.listdir(d): + if fnmatch.fnmatch(name, "??-nitrokey.rules"): + logger.info(f"Found matching udev file at {os.path.join(d, name)}") + return True + return False + + @click.command() @click.option("--passwd", default="", help="password") def kdf_details(passwd):