Skip to content

Commit

Permalink
Enable more type checks for old code
Browse files Browse the repository at this point in the history
This patch updates the mypy configuration to run more tests for the old
code and fixes some issues found by it.
  • Loading branch information
robin-nitrokey committed Jan 3, 2025
1 parent a55974c commit b1106ff
Show file tree
Hide file tree
Showing 12 changed files with 83 additions and 56 deletions.
11 changes: 4 additions & 7 deletions pynitrokey/cli/pro.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,10 @@ def update(firmware_path: str):
print = local_print
# TODO(szszsz): extract logic to nkdfu, leaving only end-user error handling
assert firmware_path.endswith("bin")
vendor = "20a0:42b4"
product = None
if vendor is not None:
if ":" in vendor:
vendor, product = vendor.split(":")
product = int(product, 16) # type: ignore
vendor = int(vendor, 16) # type: ignore
vendor_str = "20a0:42b4"
vendor_str, product_str = vendor_str.split(":")
product = int(product_str, 16)
vendor = int(vendor_str, 16)
dev = None
bus = None
with usb1.USBContext() as context:
Expand Down
14 changes: 7 additions & 7 deletions pynitrokey/cli/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def storage():
pass


def process_runner(c: str, args: Optional[dict] = None) -> str:
def process_runner(c: str, args: Optional[dict[str, str]] = None) -> str:
"""Wrapper for running command and returning output, both logged"""
cmd = c.split()
if args and any(f"${key}" in c for key in args.keys()):
Expand All @@ -73,7 +73,7 @@ class DfuTool:
name = "dfu-programmer"

@classmethod
def is_available(cls):
def is_available(cls) -> bool:
"""Check whether `name` is on PATH and marked as executable."""
return which(cls.name) is not None

Expand Down Expand Up @@ -112,7 +112,7 @@ class ConnectedDevices:
application_mode: int
update_mode: int

def __init__(self, application_mode, update_mode):
def __init__(self, application_mode: int, update_mode: int) -> None:
self.application_mode = application_mode
self.update_mode = update_mode

Expand All @@ -121,7 +121,7 @@ class UsbId:
vid: int
pid: int

def __init__(self, vid, pid):
def __init__(self, vid: int, pid: int) -> None:
self.vid = vid
self.pid = pid

Expand Down Expand Up @@ -214,14 +214,14 @@ def update(firmware: str, experimental):
list_cmd.callback()


def check_for_update_mode():
def check_for_update_mode() -> None:
connected = is_connected()
assert (
connected.application_mode + connected.update_mode > 0
), "No connected Nitrokey Storage devices found"
if connected.application_mode and not connected.update_mode:
# execute bootloader
storage.commands["enable-update"].callback()
storage.commands["enable-update"].callback() # type: ignore[misc]
for _ in tqdm(range(10), leave=False):
if is_connected().update_mode != 0:
break
Expand Down Expand Up @@ -465,7 +465,7 @@ def compare(fw1_path: str, fw2_path: str, region: str, max_diff: int):
else:
raise click.ClickException(f"Wrong type")

def geti(f, i):
def geti(f: IntelHex, i: int) -> int:
return f[i + offset[f]]

click.echo(f"Checking binary images in range {hex(data_start)}:{hex(data_stop)}")
Expand Down
14 changes: 7 additions & 7 deletions pynitrokey/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from _pytest.fixtures import FixtureRequest
from nitrokey.nk3.secrets_app import Instruction, SecretsApp

from pynitrokey.cli import CliException
from pynitrokey.cli.exceptions import CliException
from pynitrokey.cli.nk3 import Context

CORPUS_PATH = "/tmp/corpus"
Expand Down Expand Up @@ -107,22 +107,22 @@ def secretsAppRaw(corpus_func, dev) -> SecretsApp:
],
ids=lambda x: f"Key{str(x).split('.')[-1]}",
)
def secretsApp(request, secretsAppRaw) -> SecretsApp:
def secretsApp(request, secretsAppRaw: SecretsApp) -> SecretsApp:
"""
Create Secrets App client in two forms, w/ or w/o PIN-based encryption
"""
app = copy.deepcopy(secretsAppRaw)

credentials_type: CredEncryptionType = request.param
app.verify_pin_raw_always = app.verify_pin_raw
app.verify_pin_raw_always = app.verify_pin_raw # type: ignore[attr-defined]
if credentials_type == CredEncryptionType.PinBased:
# Make all credentials registered with the PIN-based encryption
# Leave verify_pin_raw() working
app.register = partial(app.register, pin_based_encryption=True)
app.register = partial(app.register, pin_based_encryption=True) # type: ignore[method-assign]
elif credentials_type == CredEncryptionType.HardwareBased:
# Make all verify_pin_raw() calls dormant
# All credentials should register themselves as not requiring PIN
app.verify_pin_raw = lambda x: secretsAppRaw.logfn(
app.verify_pin_raw = lambda x: secretsAppRaw.logfn( # type: ignore[method-assign]
"Skipping verify_pin_raw() call due to fixture configuration"
)
else:
Expand All @@ -134,15 +134,15 @@ def secretsApp(request, secretsAppRaw) -> SecretsApp:


@pytest.fixture(scope="function")
def secretsAppResetLogin(secretsApp) -> SecretsApp:
def secretsAppResetLogin(secretsApp: SecretsApp) -> SecretsApp:
secretsApp.reset()
secretsApp.set_pin_raw(PIN)
secretsApp.verify_pin_raw(PIN)
return secretsApp


@pytest.fixture(scope="function")
def secretsAppNoLog(secretsApp) -> SecretsApp:
def secretsAppNoLog(secretsApp: SecretsApp) -> SecretsApp:
return secretsApp


Expand Down
2 changes: 1 addition & 1 deletion pynitrokey/libnk.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ def fw_version(self):
)

@property
def serial(self, as_int=False): # type: ignore
def serial(self, as_int=False):
return (
py_enc(self.api.NK_device_serial_number())
if not as_int
Expand Down
37 changes: 22 additions & 15 deletions pynitrokey/start/gnuk_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from array import array
from contextlib import contextmanager
from struct import *
from typing import Optional
from typing import Iterator, Optional, Tuple

import usb

Expand Down Expand Up @@ -55,7 +55,9 @@
HID_PROTOCOL_0 = 0x00


def icc_compose(msg_type, data_len, slot, seq, param, data):
def icc_compose(
msg_type: int, data_len: int, slot: int, seq: int, param: int, data: bytes
) -> bytes:
return pack("<BiBBBH", msg_type, data_len, slot, seq, 0, param) + data


Expand Down Expand Up @@ -94,7 +96,12 @@ def iso7816_compose(

# This class only supports Gnuk (for now)
class gnuk_token(object):
def __init__(self, device, configuration, interface):
def __init__(
self,
device: usb.Device,
configuration: usb.Configuration,
interface: usb.Interface,
) -> None:
"""
__init__(device, configuration, interface) -> None
Initialize the device.
Expand Down Expand Up @@ -142,16 +149,16 @@ def local_print(self, message: str, verbose=False):
def get_string(self, num):
return self.__devhandle.getString(num, 512)

def increment_seq(self):
def increment_seq(self) -> None:
self.__seq = (self.__seq + 1) & 0xFF

def reset_device(self):
def reset_device(self) -> None:
try:
self.__devhandle.reset()
except:
pass

def release_gnuk(self):
def release_gnuk(self) -> None:
self.__devhandle.releaseInterface()

@contextmanager
Expand Down Expand Up @@ -223,10 +230,10 @@ def execute(self, last_addr):
requestType=0x40, request=2, buffer=None, value=i, index=o, timeout=10
)

def icc_get_result(self):
def icc_get_result(self) -> Tuple[int, int, array[int]]:
usbmsg = self.__devhandle.bulkRead(self.__bulkin, 1024, self.__timeout)
if len(usbmsg) < 10:
self.local_print(usbmsg, True)
self.local_print(usbmsg, True) # type: ignore[arg-type]
raise ValueError("icc_get_result")
msg = array("B", usbmsg)
msg_type = msg[0]
Expand All @@ -240,15 +247,15 @@ def icc_get_result(self):
# XXX: check msg_type, data_len, slot, seq, error
return (status, chain, data)

def icc_get_status(self):
def icc_get_status(self) -> int:
msg = icc_compose(0x65, 0, 0, self.__seq, 0, b"")
self.__devhandle.bulkWrite(self.__bulkout, msg, self.__timeout)
self.increment_seq()
status, chain, data = self.icc_get_result()
# XXX: check chain, data
return status

def icc_power_on(self):
def icc_power_on(self) -> array[int]:
msg = icc_compose(0x62, 0, 0, self.__seq, 0, b"")
self.__devhandle.bulkWrite(self.__bulkout, msg, self.__timeout)
self.increment_seq()
Expand All @@ -257,21 +264,21 @@ def icc_power_on(self):
self.atr = data
return self.atr

def icc_power_off(self):
def icc_power_off(self) -> int:
msg = icc_compose(0x63, 0, 0, self.__seq, 0, b"")
self.__devhandle.bulkWrite(self.__bulkout, msg, self.__timeout)
self.increment_seq()
status, chain, data = self.icc_get_result()
# XXX: check chain, data
return status

def icc_send_data_block(self, data):
def icc_send_data_block(self, data) -> Tuple[int, int, array[int]]:
msg = icc_compose(0x6F, len(data), 0, self.__seq, 0, data)
self.__devhandle.bulkWrite(self.__bulkout, msg, self.__timeout)
self.increment_seq()
return self.icc_get_result()

def icc_send_cmd(self, data):
def icc_send_cmd(self, data) -> array[int]:
status, chain, data_rcv = self.icc_send_data_block(data)
if chain == 0:
while status == 0x80:
Expand Down Expand Up @@ -698,7 +705,7 @@ def compare(data_original, data_in_device):
raise ValueError("verify failed")


def gnuk_devices():
def gnuk_devices() -> Iterator[Tuple[usb.Device, usb.Configuration, usb.Interface]]:
busses = usb.busses()
for bus in busses:
devices = bus.devices
Expand All @@ -715,7 +722,7 @@ def gnuk_devices():
yield dev, config, alt


def gnuk_devices_by_vidpid():
def gnuk_devices_by_vidpid() -> Iterator[usb.Device]:
try:
busses = usb.busses()
except usb.core.NoBackendError:
Expand Down
8 changes: 4 additions & 4 deletions pynitrokey/start/threaded_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import threading
import time
from sys import stderr
from typing import List, Optional
from typing import Iterable, List, Optional


class ThreadLog(threading.Thread):
Expand All @@ -36,7 +36,7 @@ def run(self):
self.execute(self.command.split())

@staticmethod
def _contains(value, strings):
def _contains(value: str, strings: Iterable[str]) -> bool:
for s in strings:
if s in value:
return True
Expand All @@ -60,7 +60,7 @@ def execute(self, command: List[str]):
self.process.wait()
self.logger.debug("Finished")

def start_logging(self):
def start_logging(self) -> None:
self._write_to_log = True

def __enter__(self):
Expand All @@ -76,7 +76,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
self.join(10)


def test_run():
def test_run() -> None:
FORMAT = "%(relativeCreated)05d [%(process)x] - %(levelname)s - %(name)s - %(message)s [%(filename)s:%(lineno)d]"
logging.basicConfig(format=FORMAT, stream=stderr, level=logging.DEBUG)
logger = logging.getLogger("threadlog")
Expand Down
4 changes: 2 additions & 2 deletions pynitrokey/start/upgrade_by_passwd.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ class FirmwareType(Enum):
}


def hash_data_512(data):
def hash_data_512(data: bytes) -> bytes:
hash512 = hashlib.sha512(data).digest()
hash512_hex = binascii.b2a_hex(hash512)
return hash512_hex
Expand Down Expand Up @@ -440,7 +440,7 @@ def get_firmware_file(file_name: str, type: FirmwareType):

local_print(
f"- {type}: {len(firmware_data)}, "
f"hash: ...{hash_data[-8:]} {hash_valid} (from ...{url[-24:]})"
f"hash: ...{hash_data[-8:]} {hash_valid} (from ...{url[-24:]})" # type: ignore[str-bytes-safe]
)
return firmware_data

Expand Down
2 changes: 1 addition & 1 deletion pynitrokey/start/usb_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def get_dict_for_device(dev: usb.Device) -> dict[str, Optional[str]]:
return res


def get_devices() -> list:
def get_devices() -> list[dict[str, Optional[str]]]:
from pynitrokey.start.gnuk_token import gnuk_devices_by_vidpid

res = []
Expand Down
6 changes: 3 additions & 3 deletions pynitrokey/test_secrets_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ def test_load(secretsAppResetLogin, kind: Kind, long_labels: str, count):
# Iterate over credentials and check code at given challenge
nameb = name_gen(i).encode()
secretsApp.verify_pin_raw(PIN)
assert secretsApp.calculate(nameb, i) == lib_at(i)
assert secretsApp.calculate(nameb, i) == lib_at(i) # type: ignore[no-untyped-call]

secretsApp.verify_pin_raw(PIN)
l = secretsApp.list()
Expand Down Expand Up @@ -916,7 +916,7 @@ def helper_test_calculated_codes_totp(secretsApp, secret: str, PIN: str):
for i in range(10):
# Use non-modified verify_pin_raw_always call to always verify PIN, regardless of the fixture type
secretsApp.verify_pin_raw_always(PIN)
assert secretsApp.calculate(CREDID, i) == lib_at(i)
assert secretsApp.calculate(CREDID, i) == lib_at(i) # type: ignore[no-untyped-call]

# Initial setup for the TOTP slot and test
secret = "00" * 20
Expand Down Expand Up @@ -1316,7 +1316,7 @@ def test_password_safe(secretsAppResetLogin: SecretsApp, length: int) -> None:
).encode()
for i in range(10):
secretsApp.verify_pin_raw(PIN)
assert secretsApp.calculate(name, i) == lib_at(i)
assert secretsApp.calculate(name, i) == lib_at(i) # type: ignore[no-untyped-call]

secretsApp.verify_pin_raw(PIN)
p: PasswordSafeEntry = secretsApp.get_credential(name)
Expand Down
9 changes: 0 additions & 9 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,8 @@ module = [
"pynitrokey.test_secrets_app",
]
check_untyped_defs = false
disallow_any_generics = false
disallow_incomplete_defs = false
disallow_subclassing_any = false
disallow_untyped_calls = false
disallow_untyped_decorators = false
disallow_untyped_defs = false
no_implicit_reexport = false
strict_concatenate = false
strict_equality = false
warn_unused_ignores = false
warn_return_any = false

[tool.pytest.ini_options]
log_cli = false
Expand Down
Loading

0 comments on commit b1106ff

Please sign in to comment.