Fuzzing support cont.

Earthfile

@@ -0,0 +1,20 @@
+VERSION 0.6
+FROM ubuntu:latest
+WORKDIR /workdir
+
+build:
+    ENV FLIT_ROOT_INSTALL=1
+    RUN apt update && apt install python3 python3-dev make python3-pip python3.10-venv libpcsclite-dev swig  -qy
+    RUN apt install libusb-1.0-0-dev  -qy
+    RUN python3 -m pip install -U pip
+    RUN python3 -m pip install -U flit
+    RUN mkdir pynitrokey
+    COPY pyproject.toml README.md .
+    RUN python3 -m flit install --only-deps
+    COPY . .
+    RUN make clean
+    RUN make init
+    ENTRYPOINT ["/workdir/venv/bin/nitropy"]
+    ENV ALLOW_ROOT=1
+    ENV PATH /workdir/venv/bin:$PATH
+    SAVE IMAGE pynitrokey:latest

Makefile

@@ -11,9 +11,26 @@ ISORT_FLAGS=--py 35 --extend-skip pynitrokey/nethsm/client
 # whitelist of directories for flake8
 FLAKE8_DIRS=pynitrokey/nethsm pynitrokey/cli/nk3 pynitrokey/nk3
 
+.PHONY: init-fedora37
+init-fedora37:
+	sudo dnf install -y swig pcsc-lite-devel
+	$(MAKE) init
+
 # setup development environment
 init: update-venv
 
+ARGS=
+.PHONY: run rune builde
+run:
+	./venv/bin/nitropy $(ARGS)
+
+DOCKER=docker
+rune:
+	$(DOCKER) run --privileged --rm -it --entrypoint /bin/bash pynitrokey
+
+builde:
+	earthly +build
+
 # ensure this passes before commiting
 check: lint
 	$(VENV)/bin/python3 -m black $(BLACK_FLAGS) --check $(PACKAGE_NAME)/
@@ -130,4 +147,3 @@ wine-build/pynitrokey-$(VERSION).msi wine-build/nitropy-$(VERSION).exe:
 	bash build-wine.sh
 	#cp wine-build/out/pynitrokey-$(VERSION)-win32.msi wine-build
 	cp wine-build/out/nitropy-$(VERSION).exe wine-build
-

docs/developer-guide.rst

@@ -6,6 +6,25 @@ Setting Up The Environment
 
 Use ``make init`` to install ``pynitrokey``, its dependencies and the development tools inside a virtual environment in the ``venv`` folder.  This virtual environment is also used by other targets in the Makefile like ``make check``.  ``pynitrokey`` is installed in editable mode so that all changes to the source code also apply to the ``nitropy`` executable in the virtual environment.  If dependencies are changed, run ``make update-venv`` to update the virtual environment.
 
+
+By default the development environment is isolated using venv, when set up using ``make init``. To run the ``nitropy`` from there call ``./venv/bin/nitropy``. This can be used as a shortcut ``make run ARGS="nk3 list"``, where ``ARGS`` is an optional argument to pass to ``nitropy``.
+
+
+Containerized Environment
+--------------------
+
+Setting up a proper environment, matching the exact Python version, or having a newer one than it is supported, might be problematic for the development.
+For such cases it is handy to have development/test environment containerized, so it does not affect nor need changes in the host OS internals.
+For that Earthly tool is used, but any Docker-like tool will work.
+
+The common targets are listed in the main Makefile with the ``e`` suffix:
+
+- ``make builde`` - executes Earthly build process for the pynitrokey container image;
+- ``make rune`` - runs Docker container based on the pynitrokey image for a quick tests. Use ``DOCKER=podman`` to call ``podman`` binary instead of the default Docker.
+
+See https://earthly.dev/ for the Earthly setup instructions. Mind, that their binaries currently are not signed and cannot be verified, thus installation to the root directory of the host OS is not advised (on contrary to the claims on their main page). Use Dockerfile directly instead if in doubt.
+
+
 Linters
 -------
 

pynitrokey/conftest.py

@@ -1,44 +1,65 @@
-import hashlib
 import logging
 import os
 import pathlib
+from functools import partial
 
 import pytest
+import secrets
+from _pytest.fixtures import FixtureRequest
 
 from pynitrokey.cli.nk3 import Context
 from pynitrokey.nk3.otp_app import Instruction, OTPApp
 
+CORPUS_PATH = "/tmp/corpus"
+
 logging.basicConfig(
     encoding="utf-8", level=logging.DEBUG, handlers=[logging.StreamHandler()]
 )
 
 
-def _write_corpus(ins: Instruction, data: bytes):
-    corpus_name = f"{ins}-{hashlib.sha1(data).digest().hex()}"
-    corpus_path = f"/tmp/corpus/{corpus_name}"
-    with open(corpus_path, "bw") as f:
+def _write_corpus(
+    ins: Instruction, data: bytes, prefix: str = "", path: str = CORPUS_PATH
+):
+    corpus_name = f"{prefix}"
+    corpus_path = f"{path}/{corpus_name}"
+    if len(data) > 255:
+        # Do not write records longer than 255 bytes
+        return
+    data = bytes([len(data)]) + data
+    with open(corpus_path, "ba") as f:
+        print(f"Writing corpus data to the path {corpus_path}")
         f.write(data)
 
 
-def setup_for_making_corpus(app):
-    pathlib.Path("/tmp/corpus").mkdir(exist_ok=True)
+@pytest.fixture(scope="function")
+def corpus_func(request: FixtureRequest):
     if os.environ.get("NK_FUZZ") is not None:
-        app.write_corpus_fn = _write_corpus
+        path = os.environ.get("NK_FUZZ_PATH", CORPUS_PATH)
+        pathlib.Path(path).mkdir(exist_ok=True)
+        # Add some random suffix to have separate outputs for parametrized test cases
+        pre = secrets.token_bytes(4).hex()
+        pre = f"{request.function.__name__}-{pre}"
+        return partial(_write_corpus, prefix=pre, path=path)
+    return None
 
 
 @pytest.fixture(scope="session")
-def otpApp():
+def dev():
     ctx = Context(None)
-    app = OTPApp(ctx.connect_device(), logfn=print)
-    setup_for_making_corpus(app)
+    return ctx.connect_device()
+
+
+@pytest.fixture(scope="function")
+def otpApp(corpus_func, dev):
+    app = OTPApp(dev, logfn=print)
+    app.write_corpus_fn = corpus_func
     return app
 
 
-@pytest.fixture(scope="session")
-def otpAppNoLog():
-    ctx = Context(None)
-    app = OTPApp(ctx.connect_device())
-    setup_for_making_corpus(app)
+@pytest.fixture(scope="function")
+def otpAppNoLog(corpus_func, dev):
+    app = OTPApp(dev)
+    app.write_corpus_fn = corpus_func
     return app
 
 

pynitrokey/nk3/otp_app.py

@@ -33,6 +33,43 @@ class SelectResponse:
     algorithm: Optional[bytes]
 
 
+@dataclasses.dataclass
+class OTPAppException(Exception):
+    code: str
+    context: str
+
+    def to_string(self) -> str:
+        d = {
+            "6300": "VerificationFailed",
+            "6400": "UnspecifiedNonpersistentExecutionError",
+            "6500": "UnspecifiedPersistentExecutionError",
+            "6700": "WrongLength",
+            "6881": "LogicalChannelNotSupported",
+            "6882": "SecureMessagingNotSupported",
+            "6884": "CommandChainingNotSupported",
+            "6982": "SecurityStatusNotSatisfied",
+            "6985": "ConditionsOfUseNotSatisfied",
+            "6983": "OperationBlocked",
+            "6a80": "IncorrectDataParameter",
+            "6a81": "FunctionNotSupported",
+            "6a82": "NotFound",
+            "6a84": "NotEnoughMemory",
+            "6a86": "IncorrectP1OrP2Parameter",
+            "6a88": "KeyReferenceNotFound",
+            "6d00": "InstructionNotSupportedOrInvalid",
+            "6e00": "ClassNotSupported",
+            "6f00": "UnspecifiedCheckingError",
+            "9000": "Success",
+        }
+        return d.get(self.code, "Unknown SW code")
+
+    def __repr__(self) -> str:
+        return f"OTPAppException(code={self.code}/{self.to_string()})"
+
+    def __str__(self) -> str:
+        return self.__repr__()
+
+
 class Instruction(Enum):
     Put = 0x1
     Delete = 0x2
@@ -133,7 +170,14 @@ def _send_receive_inner(self, data: bytes) -> bytes:
             self.logfn(f"Got exception: {e}")
             raise
 
-        self.logfn(f"Received {result.hex() if data else data!r}")
+        status_bytes, result = result[:2], result[2:]
+        self.logfn(
+            f"Received [{status_bytes.hex()}] {result.hex() if result else result!r}"
+        )
+
+        if status_bytes != b"\x90\00":
+            raise OTPAppException(status_bytes.hex(), "Received error")
+
         return result
 
     @classmethod
@@ -257,14 +301,14 @@ def verify_code(self, cred_id: bytes, code: int) -> bool:
         Proceed with the incoming OTP code verification (aka reverse HOTP).
         :param cred_id: The name of the credential
         :param code: The HOTP code to verify. u32 representation.
-        :return: fails with CTAP1 error; returns True if code matches the value calculated internally.
+        :return: fails with OTPAppException error; returns True if code matches the value calculated internally.
         """
         structure = [
             tlv8.Entry(Tag.CredentialId.value, cred_id),
             tlv8.Entry(Tag.Response.value, pack(">L", code)),
         ]
-        res = self._send_receive(Instruction.VerifyCode, structure=structure)
-        return res.hex() == "7700"
+        self._send_receive(Instruction.VerifyCode, structure=structure)
+        return True
 
     def set_code(self, passphrase: str) -> None:
         """

pynitrokey/test_otp.py

@@ -16,7 +16,14 @@
 import tlv8
 
 from pynitrokey.conftest import CHALLENGE, CREDID, DIGITS, HOTP_WINDOW_SIZE, SECRET
-from pynitrokey.nk3.otp_app import Algorithm, Instruction, Kind, RawBytes, Tag
+from pynitrokey.nk3.otp_app import (
+    Algorithm,
+    Instruction,
+    Kind,
+    OTPAppException,
+    RawBytes,
+    Tag,
+)
 
 
 def test_reset(otpApp):
@@ -224,19 +231,19 @@ def test_reverse_hotp_failure(otpApp):
     )
     for i in range(3):
         c = codes[i]
-        with pytest.raises(fido2.ctap.CtapError):
+        with pytest.raises(OTPAppException, match="VerificationFailed"):
             assert not otpApp.verify_code(CREDID, c)
 
     # Test parsing too long code
-    with pytest.raises(fido2.ctap.CtapError):
+    with pytest.raises(OTPAppException, match="VerificationFailed"):
         assert not otpApp.verify_code(CREDID, 10**5)
 
     otpApp.register(CREDID, secretb, digits=7, kind=Kind.Hotp, algo=Algorithm.Sha1)
-    with pytest.raises(fido2.ctap.CtapError):
+    with pytest.raises(OTPAppException, match="ConditionsOfUseNotSatisfied"):
         assert not otpApp.verify_code(CREDID, 10**6)
 
     otpApp.register(CREDID, secretb, digits=8, kind=Kind.Hotp, algo=Algorithm.Sha1)
-    with pytest.raises(fido2.ctap.CtapError):
+    with pytest.raises(OTPAppException, match="ConditionsOfUseNotSatisfied"):
         assert not otpApp.verify_code(CREDID, 10**7)
 
 
@@ -280,7 +287,7 @@ def test_reverse_hotp_window(otpApp, offset, start_value):
     code_to_send = int(code_to_send)
     if offset > HOTP_WINDOW_SIZE:
         # calls with offset bigger than HOTP_WINDOW_SIZE should fail
-        with pytest.raises(fido2.ctap.CtapError):
+        with pytest.raises(OTPAppException, match="VerificationFailed"):
             otpApp.verify_code(CREDID, code_to_send)
     else:
         # check if this code will be accepted on the given offset
@@ -291,17 +298,23 @@ def test_reverse_hotp_window(otpApp, offset, start_value):
             and offset == HOTP_WINDOW_SIZE
         )
         if not is_counter_saturated:
-            with pytest.raises(fido2.ctap.CtapError):
+            with pytest.raises(
+                OTPAppException,
+                match="UnspecifiedPersistentExecutionError|VerificationFailed",
+            ):
+                # send the same code once again
                 otpApp.verify_code(CREDID, code_to_send)
             # test the very next value - should be accepted
             code_to_send = lib_at(start_value + offset + 1)
             code_to_send = int(code_to_send)
             assert otpApp.verify_code(CREDID, code_to_send)
         else:
             # counter got saturated, error code will be returned
-            assert otpApp.verify_code(CREDID, code_to_send)
-            assert otpApp.verify_code(CREDID, code_to_send)
-            assert otpApp.verify_code(CREDID, code_to_send)
+            for _ in range(3):
+                with pytest.raises(
+                    OTPAppException, match="UnspecifiedPersistentExecutionError"
+                ):
+                    otpApp.verify_code(CREDID, code_to_send)
 
 
 @pytest.mark.parametrize(
@@ -419,7 +432,7 @@ def test_too_long_message(otpApp):
     otpApp.list()
 
     too_long_name = b"a" * 253
-    with pytest.raises(fido2.ctap.CtapError):
+    with pytest.raises(OTPAppException, match="IncorrectDataParameter"):
         structure = [
             tlv8.Entry(Tag.CredentialId.value, too_long_name),
         ]
@@ -475,7 +488,14 @@ def test_set_code(otpApp):
     assert state.algorithm is not None
 
 
-def test_set_code_and_validate(otpApp):
+@pytest.mark.parametrize(
+    "remove_password_with",
+    [
+        Instruction.Reset,
+        Instruction.SetCode,
+    ],
+)
+def test_set_code_and_validate(otpApp, remove_password_with: Instruction):
     """
     Test device's behavior when the validation code is set.
     Non-authorized calls should be rejected, except for the selected.
@@ -508,13 +528,14 @@ def test_set_code_and_validate(otpApp):
     otpApp.set_code_raw(SECRET, CHALLENGE, response)
 
     # Make sure all the expected commands are failing, as in specification
-    with pytest.raises(fido2.ctap.CtapError):
-        # TODO check for the exact error code
+    with pytest.raises(OTPAppException, match="ConditionsOfUseNotSatisfied"):
         otpApp.list()
 
     for ins in set(Instruction) - {Instruction.Reset, Instruction.Validate}:
-        # TODO check for the exact error code
-        with pytest.raises(fido2.ctap.CtapError):
+        with pytest.raises(
+            OTPAppException,
+            match="IncorrectDataParameter|InstructionNotSupportedOrInvalid|NotFound|ConditionsOfUseNotSatisfied",
+        ):
             structure = [RawBytes([0x02] * 10)]
             otpApp._send_receive(ins, structure)
 
@@ -528,7 +549,7 @@ def test_set_code_and_validate(otpApp):
     otpApp.list()
 
     # Make sure another command call is not allowed
-    with pytest.raises(fido2.ctap.CtapError):
+    with pytest.raises(OTPAppException, match="ConditionsOfUseNotSatisfied"):
         otpApp.list()
 
     # Test running "list" command again
@@ -539,8 +560,23 @@ def test_set_code_and_validate(otpApp):
     otpApp.validate_raw(challenge=state.challenge, response=response_validate)
     otpApp.list()
 
-    # Reset should be allowed
-    otpApp.reset()
+    if remove_password_with == Instruction.Reset:
+        # Reset should be allowed
+        otpApp.reset()
+    elif remove_password_with == Instruction.SetCode:
+        # Clearing passphrase should be allowed after authentication
+        with pytest.raises(OTPAppException, match="ConditionsOfUseNotSatisfied"):
+            otpApp.clear_code()
+
+        state = otpApp.select()
+        response_validate = hmac.HMAC(
+            key=SECRET, msg=state.challenge, digestmod="sha1"
+        ).digest()
+        otpApp.validate_raw(challenge=state.challenge, response=response_validate)
+        otpApp.clear_code()
+    else:
+        raise ValueError()
+
     state = otpApp.select()
     assert state.challenge is None