From cd12a198b7e7df79ca115dfccd3a3a6f5d7d03e6 Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Fri, 18 Sep 2026 19:16:04 +0200 Subject: [PATCH 1/8] Add remote authenticate and inspect through the Cryptnox PIV service The remote PIV service performs lifecycle operations that need keys which never leave Cryptnox, with the card in the holder's own reader. This adds the client side for its two key-free operations. `remote authenticate` identifies the card to the service (ATR and CPLC UID); `remote inspect` probes whether the PIV applet and its security domain are present and which key versions they carry. Both open the local reader first, connect over TLS, answer the service's proof-of-work, and relay the commands the service sends. The relay is raw: it never sends a command of its own and never reassembles a chained response, so the service sees the card as a local reader would. A fail-closed relay policy decides what reaches the card. It is an allow-list per operation over the selected applet, class, instruction and SELECT target: the wallet, FIDO2, genuineness and DESFire functions cannot be selected, card-holder verifier commands are never relayed, and EXTERNAL AUTHENTICATE is refused for both operations so no card-management retry can be spent. A refusal ends the operation and names the command header. The output keeps what the service asserted apart from what was verified locally (CPLC UID and ATR read before the connection opens). Relayed commands are written to the transcript with their data masked unless the instruction is a known read; `remote --full-transcript` lifts that extra masking. `websockets` becomes a runtime dependency (synchronous client). New exit codes 12 to 16 cover connection, protocol, policy and service failures. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 6 + CONTEXT.md | 20 ++ docs/cli-basics.rst | 3 + docs/exit-codes.rst | 20 ++ docs/index.rst | 7 + docs/remote/remote-commands.rst | 51 ++++ docs/remote/remote-guide.rst | 67 +++++ pyproject.toml | 4 + src/cryptnox_id_cli/cli/commands/remote.py | 207 +++++++++++++ src/cryptnox_id_cli/cli/context.py | 12 + src/cryptnox_id_cli/cli/dryrun.py | 4 + src/cryptnox_id_cli/cli/main.py | 2 + src/cryptnox_id_cli/remote/__init__.py | 12 + src/cryptnox_id_cli/remote/channel.py | 186 ++++++++++++ src/cryptnox_id_cli/remote/policy.py | 293 ++++++++++++++++++ src/cryptnox_id_cli/remote/pow.py | 71 +++++ src/cryptnox_id_cli/remote/protocol.py | 225 ++++++++++++++ src/cryptnox_id_cli/remote/relay.py | 207 +++++++++++++ src/cryptnox_id_cli/transport/errors.py | 53 ++++ tests/unit/test_remote_channel.py | 204 +++++++++++++ tests/unit/test_remote_commands.py | 218 ++++++++++++++ tests/unit/test_remote_policy.py | 253 ++++++++++++++++ tests/unit/test_remote_pow.py | 63 ++++ tests/unit/test_remote_protocol.py | 217 ++++++++++++++ tests/unit/test_remote_relay.py | 332 +++++++++++++++++++++ 25 files changed, 2737 insertions(+) create mode 100644 docs/remote/remote-commands.rst create mode 100644 docs/remote/remote-guide.rst create mode 100644 src/cryptnox_id_cli/cli/commands/remote.py create mode 100644 src/cryptnox_id_cli/remote/__init__.py create mode 100644 src/cryptnox_id_cli/remote/channel.py create mode 100644 src/cryptnox_id_cli/remote/policy.py create mode 100644 src/cryptnox_id_cli/remote/pow.py create mode 100644 src/cryptnox_id_cli/remote/protocol.py create mode 100644 src/cryptnox_id_cli/remote/relay.py create mode 100644 tests/unit/test_remote_channel.py create mode 100644 tests/unit/test_remote_commands.py create mode 100644 tests/unit/test_remote_policy.py create mode 100644 tests/unit/test_remote_pow.py create mode 100644 tests/unit/test_remote_protocol.py create mode 100644 tests/unit/test_remote_relay.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d35f653..f1e1dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 mechanisms. - `perso generate-key --create-key-object` - the same dev/eval fallback `import-key` already had. +- `remote authenticate` and `remote inspect`: identify and probe the card + through the Cryptnox remote PIV service, with the card in the local reader. + The tool relays the commands the service sends under a fail-closed policy + that keeps every other card function out of reach, and reports what the + service asserted apart from what it verified locally. Adds the `websockets` + dependency and exit codes 12 to 16. ## [1.0.3] - 2026-08-31 diff --git a/CONTEXT.md b/CONTEXT.md index 6e8e7b0..fd5bb5d 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -105,6 +105,26 @@ The DESFire EV3 mechanism that embeds per-read authentication data (UID, counter MAC) into a file's NDEF content. EV3-only. _Avoid_: SUN (the message SDM produces, not the mechanism) +### Remote service + +**Remote service**: +The Cryptnox-operated service that performs PIV lifecycle operations needing keys +that never leave Cryptnox, with the card in the holder's own reader. It drives; the +tool relays. +_Avoid_: cloud, backend, API (it is an operation partner, not a data source) + +**Relay**: +The tool's role during a remote operation: transmit each command the service sends +to the card and return the answer verbatim. The relay never sends a command of its +own and never reassembles a chained response. +_Avoid_: proxy, tunnel (both suggest transparency; the relay refuses) + +**Relay policy**: +The fail-closed allow-list the relay applies to every command before it reaches the +card: which applet may be selected, which instructions may pass, how many +authentication attempts each security domain may see. A refusal ends the operation. +_Avoid_: firewall, filter (a filter drops; the policy stops) + ### Tool behavior **Dry run**: diff --git a/docs/cli-basics.rst b/docs/cli-basics.rst index efd5030..57335ca 100644 --- a/docs/cli-basics.rst +++ b/docs/cli-basics.rst @@ -71,6 +71,9 @@ Top-level commands * - ``report card`` / ``report piv`` / ``report mifare`` / ``report fido`` / ``report genuine`` / ``report full`` - Secret-safe JSON reports (``--out FILE``). + * - ``remote authenticate`` / ``remote inspect`` + - Operations run through the Cryptnox remote PIV service, with the card + in the local reader; see :doc:`/remote/remote-commands`. * - ``shell`` - Interactive prompt: run subcommands without the ``cryptnox-id`` prefix (type ``piv info``); global options passed when launching carry into every diff --git a/docs/exit-codes.rst b/docs/exit-codes.rst index 8a67096..662b6a9 100644 --- a/docs/exit-codes.rst +++ b/docs/exit-codes.rst @@ -51,3 +51,23 @@ machine token (see :doc:`/json-output`). * - 11 - ``ev2_error`` - DESFire secure-messaging (session/MAC) failure. + * - 12 + - ``remote_error`` + - A remote PIV service operation failed for a reason without a more + specific class. + * - 13 + - ``remote_connect`` + - The remote PIV service could not be reached, or the TLS handshake + failed. + * - 14 + - ``remote_protocol`` + - The remote PIV service sent something the client cannot interpret + safely. + * - 15 + - ``remote_policy`` + - The remote PIV service asked for a card command the relay policy + refuses to send. The message names the command header. + * - 16 + - ``remote_failed`` + - The exchange completed, but the remote PIV service reported the + operation as failed. diff --git a/docs/index.rst b/docs/index.rst index 8c3342e..8c1085e 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -62,5 +62,12 @@ driven by one tool: |cli|. Guide Commands +.. toctree:: + :maxdepth: 2 + :caption: Remote service + + Guide + Commands + .. API Reference — RESERVED: autodoc of the Python package, if/when it is publicly pip-installable (see the internal documentation plan, decision D4). diff --git a/docs/remote/remote-commands.rst b/docs/remote/remote-commands.rst new file mode 100644 index 0000000..706eb85 --- /dev/null +++ b/docs/remote/remote-commands.rst @@ -0,0 +1,51 @@ +Remote service commands +======================= + +The ``remote`` commands run operations through the Cryptnox remote PIV +service. The card stays in the local reader; |cli| relays the commands the +service sends to the card and reports the outcome. What the service asserts +and what |cli| verified locally are shown apart. The service, the relay policy +and what each operation does to the card are described in +:doc:`/remote/remote-guide`. + +Every command opens the local reader first, then connects to the service over +TLS and answers a small proof-of-work before any command is relayed. Only the +PIV function and its security domains are reachable through the relay; the +other card functions are refused by the relay policy. + +Group options +------------- + +.. code-block:: text + + remote [--full-transcript] COMMAND + + Options: + --full-transcript write relayed commands to --apdu-log and --verbose + without the extra masking the relay applies by + default; the standard secret masking still applies + +Identification and inspection +----------------------------- + +.. code-block:: text + + remote authenticate identify the card to the service: ATR and CPLC UID, + nothing else; key-free on both sides + + remote inspect probe the PIV function through the service: whether + the PIV applet and its security domain are present + and which key versions they carry; key-free, the + relay refuses any authentication attempt + +Both commands report the card as read locally (reader, ATR, CPLC UID), the +service's result as the service asserted it, and a cross-check of the fields +the two have in common. + +Endpoint +-------- + +The service endpoint is fixed to ``wss://piv.cryptnox.com``. For development +against a local stand-in, ``CRYPTNOX_REMOTE_URL`` redirects the identification +and inspection commands only; it must be a ``wss://`` URL, or ``ws://`` to +loopback. diff --git a/docs/remote/remote-guide.rst b/docs/remote/remote-guide.rst new file mode 100644 index 0000000..a1a107a --- /dev/null +++ b/docs/remote/remote-guide.rst @@ -0,0 +1,67 @@ +Remote service guide +==================== + +Some PIV lifecycle operations need keys that never leave Cryptnox: the key +that makes a card genuine, and the certificate authority that attests keys +generated on it. The remote PIV service holds those keys in a hardware +security module and performs the operation over a relayed command channel, +with the card in the holder's own reader. + +How a relayed operation works +----------------------------- + +1. |cli| opens the local reader and reads the card's ATR and CPLC serial. +2. It connects to the service over TLS and names the operation. +3. The service issues a small proof-of-work; |cli| answers it. This is an + anti-abuse throttle, not a credential. +4. The service sends card commands one at a time. |cli| checks each one + against the relay policy, transmits it, and returns the card's answer + verbatim, status word included. |cli| never sends a command of its own and + never reassembles a chained response; the service sees the card exactly as + a local reader would. +5. The service returns a result. |cli| shows it as the service's assertion and + adds what it verified locally. + +The relay policy +---------------- + +The card carries more than the PIV function. A relay that executed whatever +it was sent would expose the wallet, the FIDO2 passkeys and the genuineness +key to the service. The relay policy is an allow-list, per operation, over +what a command header reveals: the selected applet, the class byte, the +instruction, and for a SELECT the target application. Anything not listed is +refused, the operation stops, and the refusal names the command header. + +The policy also caps the number of commands and the time an operation may +take, limits secure-channel authentication attempts per security domain, and +never relays a card-holder PIN or PUK command. Inside a secure channel only +the class and instruction bytes stay in the clear, so the policy fences which +security domain the channel was opened to and which instructions pass through +it; it cannot see what an encrypted command carries. + +Operations +---------- + +``authenticate`` + Reads the ATR and CPLC serial. Key-free. Allows only card-manager reads. + +``inspect`` + Probes whether the PIV applet and its security domain are present and which + key versions they carry. Key-free: an authentication attempt from the + service is refused before it reaches the card. + +What is sent to the service +--------------------------- + +The card's ATR and CPLC serial identify the card to the service. They are the +same values any reader can obtain from the card without a key. Nothing else +about the card holder is sent. + +Transcripts +----------- + +With ``--apdu-log`` or ``--verbose``, relayed commands are written with their +data masked unless the instruction is a known-harmless read, because the +service chose the command and |cli| cannot vouch for what it carries. +``remote --full-transcript`` lifts that extra masking for troubleshooting; the +standard masking of secret-bearing commands still applies. diff --git a/pyproject.toml b/pyproject.toml index a208b70..4612a52 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,10 @@ dependencies = [ "cbor2>=5.4,<6", "asn1crypto>=1.5", "pyyaml>=6.0", + # The remote PIV service speaks WebSocket. Floor is 13.0, the first release + # whose synchronous client (websockets.sync.client) is stable; the CLI is + # synchronous throughout and does not run an event loop. + "websockets>=13,<18", ] [project.urls] diff --git a/src/cryptnox_id_cli/cli/commands/remote.py b/src/cryptnox_id_cli/cli/commands/remote.py new file mode 100644 index 0000000..55315fe --- /dev/null +++ b/src/cryptnox_id_cli/cli/commands/remote.py @@ -0,0 +1,207 @@ +"""``remote`` — operations carried out by the Cryptnox remote PIV service. + +The card stays in the local reader. The service holds the keys that make a card +genuine and drives the operation over a relayed command channel; this tool +relays what the service asks for, under a fail-closed policy, and shows the +outcome. What the service asserts and what this tool verified locally are kept +apart in the output. +""" + +from __future__ import annotations + +import json +import os +from collections.abc import Callable + +import click +from rich.console import Console +from rich.markup import escape + +from cryptnox_id_cli.cli.commands.info import _read_cplc +from cryptnox_id_cli.cli.context import AppContext +from cryptnox_id_cli.remote import channel as rchannel +from cryptnox_id_cli.remote import relay +from cryptnox_id_cli.remote.policy import RelayPolicy +from cryptnox_id_cli.transport.errors import RemoteConnectError +from cryptnox_id_cli.transport.pcsc import connect, pick_reader + +_FULL_TRANSCRIPT_KEY = "remote.full_transcript" + + +@click.group("remote") +@click.option( + "--full-transcript", + is_flag=True, + help=( + "Write relayed commands to --apdu-log and --verbose without the extra masking " + "the relay applies by default. The standard secret masking still applies." + ), +) +@click.pass_context +def command(ctx: click.Context, full_transcript: bool) -> None: + """Run operations through the Cryptnox remote PIV service. + + Every command opens the local reader, connects to the service over TLS, + answers a small proof-of-work, and relays the commands the service sends to + the card. Only the PIV function and its security domains are reachable; the + other card functions are fenced off by the relay policy. + """ + ctx.meta[_FULL_TRANSCRIPT_KEY] = full_transcript + + +def _endpoint(op: str) -> tuple[str, bool]: + """The service URL for this run, and whether it is the production endpoint.""" + override = os.environ.get(rchannel.URL_ENV) + if not override: + return rchannel.PRODUCTION_URL, True + url = rchannel.validate_url(override, allow_insecure_loopback=True) + if op not in ("authenticate", "inspect"): + raise RemoteConnectError( + f"${rchannel.URL_ENV} is set; {op} runs only against the production service" + ) + return url, False + + +def _run(app: AppContext, ctx: click.Context, op: str, params: dict[str, object]) -> None: + """Open the card, run one operation through the service, render the outcome.""" + url, production = _endpoint(op) + reader = pick_reader(app.reader) + app.resolved_reader = reader + conn = connect(reader) + try: + session = app.make_session(conn, reader_name=reader) + atr = session.atr + cplc = _read_cplc(session) # leaves the card manager selected, as the policy assumes + card: dict[str, object] = { + "reader": reader, + "atr": atr.hex().upper(), + "cplc_uid": cplc["uid"] if cplc else None, + "ic_serial": cplc["ic_serial"] if cplc else None, + } + + if not production: + app.out.warn(f"connecting to a non-production endpoint: {url}") + app.out.note(f"remote {op} via {url}") + + with rchannel.open_channel(url) as ch: + report = relay.run_operation( + ch, + conn, + op, + params, + policy=RelayPolicy(op), + redactor=app.redactor, + transcript=app.apdu_trace, + unmasked_transcript=bool(ctx.meta.get(_FULL_TRANSCRIPT_KEY)), + on_log=lambda msg: app.out.note(f"service: {escape(msg)}"), + ) + finally: + conn.disconnect() + + checks = _cross_check(card, report.result) + relay_info: dict[str, object] = { + "apdus_relayed": report.apdus_relayed, + "seconds": round(report.seconds, 3), + "pow_bits": report.pow_bits, + "service_log": report.log_lines, + } + payload: dict[str, object] = { + "op": op, + "endpoint": url, + "card": card, + "result": report.result, + "verified_locally": checks, + "relay": relay_info, + } + app.out.result(payload, _human(op, url, card, report.result, checks, relay_info)) + + +def _cross_check(card: dict[str, object], result: dict[str, object]) -> dict[str, object]: + """Compare what the service reports about the card with what was read locally. + + The service's field names are not fixed; each candidate is tried and the + comparison reported as ``None`` when the service said nothing usable. + """ + + def pick(*keys: str) -> str | None: + for key in keys: + value = result.get(key) + if isinstance(value, str) and value.strip(): + return value.strip().upper() + return None + + checks: dict[str, object] = {} + reported_uid = pick("cplc_uid", "uid", "card_uid") + local_uid = card.get("cplc_uid") + checks["cplc_uid_matches"] = ( + None if reported_uid is None or not local_uid else reported_uid == local_uid + ) + reported_atr = pick("atr") + checks["atr_matches"] = None if reported_atr is None else reported_atr == card.get("atr") + return checks + + +def _human( + op: str, + url: str, + card: dict[str, object], + result: dict[str, object], + checks: dict[str, object], + relay_info: dict[str, object], +) -> Callable[[Console], None]: + def render(c: Console) -> None: + c.print(f"[bold]remote {op}[/bold] {escape(url)}") + c.print(f" Reader: {escape(str(card['reader']))}") + c.print(f" ATR: {card['atr']}") + if card.get("cplc_uid"): + c.print(f" UID: {card['cplc_uid']} (CPLC, read locally)") + c.print( + f" Relay: {relay_info['apdus_relayed']} command(s) in " + f"{relay_info['seconds']}s, proof-of-work {relay_info['pow_bits']} bits" + ) + + c.print("\n[bold]Service result[/bold] (asserted by the service)") + for key, value in result.items(): + c.print(f" {escape(str(key))}: {escape(_fmt(value))}") + + c.print("\n[bold]Verified locally[/bold]") + for key, value in checks.items(): + if value is True: + label = "[green]match[/green]" + elif value is False: + label = "[red]MISMATCH[/red]" + else: + label = "[dim]not reported by the service[/dim]" + c.print(f" {key}: {label}") + + return render + + +def _fmt(value: object) -> str: + if isinstance(value, bool): + return "yes" if value else "no" + if isinstance(value, dict | list): + return json.dumps(value, separators=(", ", ": ")) + return str(value) + + +@command.command("authenticate") +@click.pass_context +def authenticate(ctx: click.Context) -> None: + """Identify the card to the service: ATR and CPLC UID, nothing else. + + Key-free on both sides. The relay policy allows only card-manager reads. + """ + _run(ctx.obj, ctx, "authenticate", {}) + + +@command.command("inspect") +@click.pass_context +def inspect(ctx: click.Context) -> None: + """Probe the PIV function through the service without changing anything. + + Reports whether the PIV applet and its security domain are present and + which key versions they carry. Key-free: the relay policy refuses any + authentication attempt. + """ + _run(ctx.obj, ctx, "inspect", {}) diff --git a/src/cryptnox_id_cli/cli/context.py b/src/cryptnox_id_cli/cli/context.py index 558c913..b4af7ed 100644 --- a/src/cryptnox_id_cli/cli/context.py +++ b/src/cryptnox_id_cli/cli/context.py @@ -47,6 +47,18 @@ def _ensure_log(self) -> TextIO | None: self._log_fh.write(f"# {CLI_NAME} APDU transcript (secrets redacted)\n") return self._log_fh + def apdu_trace(self, line: str) -> None: + """Record one already-redacted transcript line (log file + verbose trace). + + The same sink :class:`CardSession` writes to, for code that transmits + outside a session. + """ + fh = self._ensure_log() + if fh is not None: + fh.write(line + "\n") + fh.flush() + self._trace(line) + def make_session(self, conn: RawConnection, *, reader_name: str | None = None) -> CardSession: """Wrap an already-open connection in a CardSession with our redactor/log.""" return CardSession( diff --git a/src/cryptnox_id_cli/cli/dryrun.py b/src/cryptnox_id_cli/cli/dryrun.py index 9e6628d..fccc3e4 100644 --- a/src/cryptnox_id_cli/cli/dryrun.py +++ b/src/cryptnox_id_cli/cli/dryrun.py @@ -144,6 +144,10 @@ "piv pin unblock", "piv pin verify", "piv puk change", + # Relayed operations: the service chooses the commands, so nothing can be + # planned locally, and every run opens a network connection. + "remote authenticate", + "remote inspect", } ) diff --git a/src/cryptnox_id_cli/cli/main.py b/src/cryptnox_id_cli/cli/main.py index 4c0f7a4..5533631 100644 --- a/src/cryptnox_id_cli/cli/main.py +++ b/src/cryptnox_id_cli/cli/main.py @@ -18,6 +18,7 @@ from cryptnox_id_cli.cli.commands import mifare as mifare_cmd from cryptnox_id_cli.cli.commands import piv as piv_cmd from cryptnox_id_cli.cli.commands import readers as readers_cmd +from cryptnox_id_cli.cli.commands import remote as remote_cmd from cryptnox_id_cli.cli.commands import report as report_cmd from cryptnox_id_cli.cli.commands import shell as shell_cmd from cryptnox_id_cli.cli.context import AppContext @@ -104,6 +105,7 @@ def main( main.add_command(mifare_cmd.command) main.add_command(fido_cmd.command) main.add_command(genuine_cmd.command) +main.add_command(remote_cmd.command) main.add_command(report_cmd.command) main.add_command(factory_cmd.command) main.add_command(shell_cmd.command) diff --git a/src/cryptnox_id_cli/remote/__init__.py b/src/cryptnox_id_cli/remote/__init__.py new file mode 100644 index 0000000..da02c0f --- /dev/null +++ b/src/cryptnox_id_cli/remote/__init__.py @@ -0,0 +1,12 @@ +"""Client for the Cryptnox remote PIV service. + +The card stays in the holder's own reader; the service drives. This package +speaks the service's JSON frame protocol over a TLS WebSocket, relays the +commands it asks for to the local card, and returns each card answer verbatim. + +The relay never interprets an operation on the card's behalf: it enforces a +fail-closed policy over what may be sent, and passes everything the policy +allows through untouched. +""" + +from __future__ import annotations diff --git a/src/cryptnox_id_cli/remote/channel.py b/src/cryptnox_id_cli/remote/channel.py new file mode 100644 index 0000000..0765a62 --- /dev/null +++ b/src/cryptnox_id_cli/remote/channel.py @@ -0,0 +1,186 @@ +"""The message channel to the remote PIV service. + +The relay is written against :class:`FrameChannel`, a two-method view of a +message stream, so the operation loop can be exercised against a scripted +channel with no socket and no card. + +The production implementation is a synchronous WebSocket over TLS. Timeouts are +explicit everywhere: an operation that generates a key on the card can leave the +client waiting tens of seconds for the next frame, and a stall must end in a +clear message rather than a hung command. +""" + +from __future__ import annotations + +import contextlib +import ssl +from collections.abc import Iterator +from contextlib import contextmanager +from typing import Protocol, runtime_checkable +from urllib.parse import urlparse + +from ..transport.errors import RemoteConnectError + +#: The service. Shipped as a constant rather than an option so an operation +#: cannot be pointed somewhere else by accident. +PRODUCTION_URL = "wss://piv.cryptnox.com" + +#: Environment variable that redirects the client, for development against a +#: local stand-in. Callers decide which operations tolerate a redirect. +URL_ENV = "CRYPTNOX_REMOTE_URL" + +#: Seconds to wait for the connection and the TLS handshake. +CONNECT_TIMEOUT = 20.0 + +#: Seconds to wait for the next frame. Generous: RSA key generation on the card +#: is measured in tens of seconds, and the service stays silent while it runs. +RECV_TIMEOUT = 120.0 + +#: Seconds to wait for the closing handshake before dropping the connection. +CLOSE_TIMEOUT = 5.0 + +#: Largest message accepted at the WebSocket layer. The codec caps again, in +#: its own terms; this stops an oversize message from being buffered at all. +MAX_MESSAGE_BYTES = 1 << 20 + + +@runtime_checkable +class FrameChannel(Protocol): + """A bidirectional stream of text messages.""" + + def send(self, message: str) -> None: ... + + def recv(self, timeout: float | None = None) -> str: ... + + def close(self) -> None: ... + + +def validate_url(url: str, *, allow_insecure_loopback: bool = False) -> str: + """Check a service URL before anything is sent to it. + + Only ``wss://`` is accepted. Plain ``ws://`` is allowed solely against + loopback, and only when the caller opts in, so a development stand-in can + run without a certificate. + """ + parsed = urlparse(url) + if parsed.scheme == "wss": + return url + if parsed.scheme == "ws": + host = (parsed.hostname or "").lower() + if allow_insecure_loopback and host in ("localhost", "127.0.0.1", "::1"): + return url + raise RemoteConnectError( + f"refusing an unencrypted connection to {url!r}; the service must be reached " + "over wss://" + ) + raise RemoteConnectError(f"not a WebSocket URL: {url!r}") + + +def default_ssl_context() -> ssl.SSLContext: + """A TLS context with verification and hostname checking on. + + Certificate pinning is deliberately not applied. The endpoint presents a + short-lived certificate on a wildcard shared with other Cryptnox services + and fronted by a CDN, so a pin held in a released CLI would fail on routine + rotation, and a pin that fails open would prove nothing. + """ + context = ssl.create_default_context() + context.check_hostname = True + context.verify_mode = ssl.CERT_REQUIRED + context.minimum_version = ssl.TLSVersion.TLSv1_2 + return context + + +class WebSocketChannel: + """A :class:`FrameChannel` over a synchronous WebSocket connection.""" + + def __init__(self, connection: object) -> None: + self._conn = connection + + def send(self, message: str) -> None: + from websockets.exceptions import WebSocketException + + try: + self._conn.send(message) # type: ignore[attr-defined] + except WebSocketException as exc: + raise RemoteConnectError(f"the connection to the service failed: {exc}") from exc + except OSError as exc: + raise RemoteConnectError(f"the connection to the service failed: {exc}") from exc + + def recv(self, timeout: float | None = None) -> str: + from websockets.exceptions import ConnectionClosed, WebSocketException + + try: + message = self._conn.recv(timeout=timeout) # type: ignore[attr-defined] + except TimeoutError as exc: + raise RemoteConnectError( + f"the service sent nothing for {timeout:g} seconds" + if timeout is not None + else "the service sent nothing" + ) from exc + except ConnectionClosed as exc: + raise RemoteConnectError( + f"the service closed the connection before the operation finished: {exc}" + ) from exc + except WebSocketException as exc: + raise RemoteConnectError(f"the connection to the service failed: {exc}") from exc + except OSError as exc: + raise RemoteConnectError(f"the connection to the service failed: {exc}") from exc + + if isinstance(message, bytes | bytearray): + try: + return bytes(message).decode("utf-8") + except UnicodeDecodeError as exc: + raise RemoteConnectError("the service sent a message that is not UTF-8") from exc + return message + + def close(self) -> None: + # Closing must not mask whatever error brought us here. + with contextlib.suppress(Exception): + self._conn.close() # type: ignore[attr-defined] + + +@contextmanager +def open_channel( + url: str, + *, + ssl_context: ssl.SSLContext | None = None, + connect_timeout: float = CONNECT_TIMEOUT, +) -> Iterator[FrameChannel]: + """Open a channel to the service and close it on the way out.""" + from websockets.exceptions import InvalidURI, WebSocketException + from websockets.sync.client import connect + + parsed = urlparse(url) + context = ssl_context + if parsed.scheme == "wss" and context is None: + context = default_ssl_context() + + try: + connection = connect( + url, + ssl=context, + open_timeout=connect_timeout, + close_timeout=CLOSE_TIMEOUT, + max_size=MAX_MESSAGE_BYTES, + ) + except ssl.SSLCertVerificationError as exc: + raise RemoteConnectError(f"the service's certificate could not be verified: {exc}") from exc + except ssl.SSLError as exc: + raise RemoteConnectError(f"the TLS handshake with the service failed: {exc}") from exc + except InvalidURI as exc: + raise RemoteConnectError(f"not a usable service URL: {url!r}") from exc + except TimeoutError as exc: + raise RemoteConnectError( + f"the service did not answer within {connect_timeout:g} seconds" + ) from exc + except WebSocketException as exc: + raise RemoteConnectError(f"could not open a connection to the service: {exc}") from exc + except OSError as exc: + raise RemoteConnectError(f"could not reach the service: {exc}") from exc + + channel = WebSocketChannel(connection) + try: + yield channel + finally: + channel.close() diff --git a/src/cryptnox_id_cli/remote/policy.py b/src/cryptnox_id_cli/remote/policy.py new file mode 100644 index 0000000..784ebb0 --- /dev/null +++ b/src/cryptnox_id_cli/remote/policy.py @@ -0,0 +1,293 @@ +"""What the relay will and will not send to the card. + +The service chooses every command; this module decides whether it reaches the +card. The card carries more than the PIV function (a wallet, FIDO2 passkeys, a +genuineness key, DESFire), and none of those has any business in a PIV +operation. So the policy is an allow-list, per operation, over what a command +header reveals: which applet is selected, the class byte, the instruction, and +for a SELECT the target AID. Anything not listed is refused, the operation is +aborted, and the refusal names the header so the list can be reviewed against +what the service actually needs. + +Where the service works inside a secure channel, only the class and instruction +bytes stay in the clear. The policy is honest about that: it fences which +security domain the channel was opened to and which instructions pass through +it, and it cannot see what an encrypted command carries. + +Counters guard the other way a command can do harm without being forbidden: +a failed GlobalPlatform authentication decrements a bounded retry counter, so +the number of authentication attempts per security domain is capped, and a +refused authentication ends the operation on the spot. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +from ..transport.errors import RemotePolicyError + +# --------------------------------------------------------------------------- # +# Applets the relay knows about # +# --------------------------------------------------------------------------- # +ISD_AID = bytes.fromhex("A000000151000000") +PIV_SSD_AID = bytes.fromhex("A00000015153504101") +PIV_AID = bytes.fromhex("A000000308000010000100") + +#: Contexts a command can be evaluated in, keyed by the selected AID. +ISD = "isd" +PIV_SSD = "piv-ssd" +PIV = "piv" + +_CONTEXT_BY_AID: dict[bytes, str] = {ISD_AID: ISD, PIV_SSD_AID: PIV_SSD, PIV_AID: PIV} + +#: The operations this module has rules for. +READ_ONLY_OPS = ("authenticate", "inspect") + +# Instructions, by name, so the tables read as prose. +INS_SELECT = 0xA4 +INS_GET_DATA_GP = 0xCA +INS_GET_DATA_PIV = 0xCB +INS_GET_STATUS = 0xF2 +INS_INITIALIZE_UPDATE = 0x50 +INS_EXTERNAL_AUTHENTICATE = 0x82 +INS_GET_RESPONSE = 0xC0 +INS_VERIFY = 0x20 +INS_CHANGE_REFERENCE_DATA = 0x24 +INS_RESET_RETRY_COUNTER = 0x2C +INS_GENERAL_AUTHENTICATE = 0x87 +INS_GENERATE_ASYMMETRIC = 0x47 +INS_PUT_DATA = 0xDB +INS_PUT_KEY = 0xD8 +INS_DELETE = 0xE4 +INS_INSTALL = 0xE6 +INS_LOAD = 0xE8 +INS_STORE_DATA = 0xE2 +INS_CTAP_MSG = 0x10 + +#: Never, in any operation or context: card-holder verifiers and anything that +#: decrements a retry counter the service has no business touching. +_ALWAYS_DENIED_INS: frozenset[int] = frozenset( + {INS_VERIFY, INS_CHANGE_REFERENCE_DATA, INS_RESET_RETRY_COUNTER} +) + + +@dataclass(frozen=True) +class OpRules: + """The allow-list for one operation.""" + + #: AIDs a SELECT may target. The empty AID (select the default) counts as ISD. + selectable: frozenset[bytes] + #: Instructions allowed per context, over plain and secure-messaging classes. + allowed_ins: dict[str, frozenset[int]] + #: Largest number of commands relayed in one operation. + max_apdus: int + #: Longest an operation may run, in seconds, measured from the first command. + max_seconds: float + #: INITIALIZE UPDATE attempts allowed per security domain. + max_initialize_updates: int + #: Whether EXTERNAL AUTHENTICATE may be relayed at all. + allow_external_authenticate: bool + + +_GP_READS: frozenset[int] = frozenset( + {INS_SELECT, INS_GET_DATA_GP, INS_GET_STATUS, INS_GET_RESPONSE} +) + +RULES: dict[str, OpRules] = { + "authenticate": OpRules( + selectable=frozenset({ISD_AID}), + allowed_ins={ + ISD: _GP_READS, + PIV_SSD: frozenset(), + PIV: frozenset(), + }, + max_apdus=20, + max_seconds=60.0, + max_initialize_updates=0, + allow_external_authenticate=False, + ), + "inspect": OpRules( + selectable=frozenset({ISD_AID, PIV_SSD_AID, PIV_AID}), + allowed_ins={ + ISD: _GP_READS | {INS_INITIALIZE_UPDATE}, + PIV_SSD: _GP_READS | {INS_INITIALIZE_UPDATE}, + PIV: frozenset({INS_SELECT, INS_GET_DATA_PIV, INS_GET_RESPONSE}), + }, + max_apdus=60, + max_seconds=120.0, + # The service enumerates key versions by repeated INITIALIZE UPDATE with + # different P1 values (observed: KVN 00, then KVN 01). None of them costs + # a retry as long as EXTERNAL AUTHENTICATE stays refused. + max_initialize_updates=4, + allow_external_authenticate=False, + ), +} + + +def rules_for(op: str) -> OpRules: + """The rule set for an operation; refuses operations with no rules yet.""" + try: + return RULES[op] + except KeyError: + raise RemotePolicyError(f"no relay policy exists for operation {op!r}") from None + + +@dataclass(frozen=True) +class Header: + cla: int + ins: int + p1: int + p2: int + + @property + def hex(self) -> str: + return f"{self.cla:02X}{self.ins:02X}{self.p1:02X}{self.p2:02X}" + + @property + def channel(self) -> int: + """Logical channel number encoded in the class byte.""" + if self.cla & 0x40: + return 4 + (self.cla & 0x0F) + return self.cla & 0x03 + + @property + def secure_messaging(self) -> bool: + """GlobalPlatform secure messaging (class bit 3) is in use.""" + return bool(self.cla & 0x04) and not (self.cla & 0x40) + + +def parse_header(apdu: bytes) -> Header: + if len(apdu) < 4: + raise RemotePolicyError("command shorter than an APDU header", header=apdu.hex().upper()) + return Header(apdu[0], apdu[1], apdu[2], apdu[3]) + + +def select_target(apdu: bytes) -> bytes | None: + """The AID a SELECT-by-name command targets, or ``None`` if it is not one. + + Only the short form with P1 = 04 (select by DF name) is recognised. Anything + else that looks like a SELECT is refused by the caller, because a SELECT the + relay cannot read is a SELECT it cannot vouch for. + """ + header = parse_header(apdu) + if header.ins != INS_SELECT: + return None + if header.p1 != 0x04: + raise RemotePolicyError( + f"SELECT with P1={header.p1:02X} is not select-by-name; refused", header=header.hex + ) + body = apdu[4:] + if not body: + return b"" # SELECT with no AID: the default applet + lc = body[0] + if lc == 0 and len(body) > 1: + raise RemotePolicyError("extended-length SELECT refused", header=header.hex) + aid = body[1 : 1 + lc] + if len(aid) != lc: + raise RemotePolicyError("SELECT data shorter than its length byte", header=header.hex) + return aid + + +@dataclass +class RelayPolicy: + """Stateful gate over one operation's command stream. + + Call :meth:`check` before transmitting and :meth:`observe` with the card's + answer afterwards, so the selected-applet tracking follows what the card + actually did rather than what the service asked for. + """ + + op: str + rules: OpRules = field(init=False) + #: Context after a fresh connection: the card manager answers by default. + context: str = ISD + apdus: int = 0 + initialize_updates: dict[str, int] = field(default_factory=dict) + _pending_select: bytes | None = field(default=None, init=False, repr=False) + + def __post_init__(self) -> None: + self.rules = rules_for(self.op) + + # -- evaluation -------------------------------------------------------- # + def check(self, apdu: bytes) -> Header: + """Refuse or allow one command. Returns the parsed header on allow.""" + header = parse_header(apdu) + self._pending_select = None + + if self.apdus >= self.rules.max_apdus: + raise RemotePolicyError( + f"the service sent more than {self.rules.max_apdus} commands in one " + f"{self.op} operation", + header=header.hex, + ) + if header.channel != 0: + raise RemotePolicyError( + f"logical channel {header.channel} refused; only the basic channel is relayed", + header=header.hex, + ) + if (header.cla & 0xF0) == 0x90: + raise RemotePolicyError("DESFire-wrapped command refused", header=header.hex) + if (header.cla & 0x0C) == 0x0C: + # Both secure-messaging bits: ISO 7816-4 SM, which nothing here speaks. + # GlobalPlatform SM sets only bit 2 (CLA 84) and passes. + raise RemotePolicyError("ISO secure messaging refused", header=header.hex) + if (header.cla & 0xF0) == 0x80 and header.ins == INS_CTAP_MSG: + raise RemotePolicyError("FIDO2 CTAP command refused", header=header.hex) + if header.ins in _ALWAYS_DENIED_INS: + raise RemotePolicyError( + f"INS {header.ins:02X} is never relayed (card-holder verifier)", header=header.hex + ) + if header.ins == INS_EXTERNAL_AUTHENTICATE and not self.rules.allow_external_authenticate: + raise RemotePolicyError( + f"EXTERNAL AUTHENTICATE is not allowed during {self.op}; a failed attempt " + "burns a card-management retry", + header=header.hex, + ) + + aid = select_target(apdu) + if aid is not None: + target = ISD_AID if aid == b"" else aid + if target not in self.rules.selectable: + raise RemotePolicyError( + f"SELECT of AID {target.hex().upper()} is not allowed during {self.op}", + header=header.hex, + ) + self._pending_select = target + return header + + allowed = self.rules.allowed_ins.get(self.context, frozenset()) + if header.ins not in allowed: + raise RemotePolicyError( + f"INS {header.ins:02X} is not allowed while the {self.context} is selected " + f"during {self.op}", + header=header.hex, + ) + + if header.ins == INS_INITIALIZE_UPDATE: + count = self.initialize_updates.get(self.context, 0) + if count >= self.rules.max_initialize_updates: + raise RemotePolicyError( + f"INITIALIZE UPDATE to the {self.context} more than " + f"{self.rules.max_initialize_updates} time(s) during {self.op}", + header=header.hex, + ) + return header + + def observe(self, apdu: bytes, sw1: int, sw2: int) -> None: + """Record what the card did with an allowed command.""" + self.apdus += 1 + header = parse_header(apdu) + ok = sw1 == 0x90 and sw2 == 0x00 + if self._pending_select is not None: + if ok: + self.context = _CONTEXT_BY_AID[self._pending_select] + self._pending_select = None + return + if header.ins == INS_INITIALIZE_UPDATE: + self.initialize_updates[self.context] = self.initialize_updates.get(self.context, 0) + 1 + if header.ins == INS_EXTERNAL_AUTHENTICATE and not ok: + raise RemotePolicyError( + f"EXTERNAL AUTHENTICATE failed (SW={sw1:02X}{sw2:02X}); stopping before " + "another attempt can burn a retry", + header=header.hex, + ) diff --git a/src/cryptnox_id_cli/remote/pow.py b/src/cryptnox_id_cli/remote/pow.py new file mode 100644 index 0000000..6b48b66 --- /dev/null +++ b/src/cryptnox_id_cli/remote/pow.py @@ -0,0 +1,71 @@ +"""Proof-of-work for the remote PIV service. + +The service gates each operation behind a small hash puzzle: find a nonce whose +SHA-256 digest of ``challenge || decimal(nonce)`` starts with at least the +requested number of zero bits. It is an anti-abuse throttle, not a credential, +and it is solved before the service knows which card is present. + +The work is bounded twice over: the client refuses a difficulty it would not +finish in reasonable time, and the search itself stops rather than spinning +forever on a challenge that has no cheap answer. +""" + +from __future__ import annotations + +import hashlib + +from ..transport.errors import RemoteProtocolError + +#: Highest difficulty this client will attempt. At 24 bits the expected work is +#: ~17 million hashes, already tens of seconds of pure Python. Anything beyond +#: that is refused rather than silently hanging the command. +MAX_BITS = 24 + +#: Safety factor on the expected number of attempts, so a pathological challenge +#: aborts instead of running unbounded. The expected count is 2**bits. +_ATTEMPT_MARGIN = 16 + + +def leading_zero_bits(digest: bytes) -> int: + """Count the zero bits at the start of ``digest``.""" + count = 0 + for byte in digest: + if byte == 0: + count += 8 + continue + # 8 bits wide, so the first set bit sits at 7 - floor(log2(byte)). + count += 8 - byte.bit_length() + break + return count + + +def digest_for(challenge: bytes, nonce: int) -> bytes: + """The digest the service checks: SHA-256 over the challenge and the nonce. + + The nonce contributes its decimal representation, not its bytes. + """ + return hashlib.sha256(bytes(challenge) + str(nonce).encode("ascii")).digest() + + +def solve(challenge: bytes, bits: int, *, max_attempts: int | None = None) -> int: + """Find a nonce whose digest has at least ``bits`` leading zero bits. + + Raises :class:`RemoteProtocolError` if the difficulty is beyond what this + client attempts, or if the bounded search finds no answer. + """ + if bits < 0: + raise RemoteProtocolError("the service asked for a negative proof-of-work difficulty") + if bits > MAX_BITS: + raise RemoteProtocolError( + f"the service asked for {bits} bits of proof-of-work; this client attempts " + f"at most {MAX_BITS}" + ) + if bits == 0: + return 0 + + limit = max_attempts if max_attempts is not None else (1 << bits) * _ATTEMPT_MARGIN + challenge = bytes(challenge) + for nonce in range(limit): + if leading_zero_bits(digest_for(challenge, nonce)) >= bits: + return nonce + raise RemoteProtocolError(f"no proof-of-work answer for {bits} bits within {limit} attempts") diff --git a/src/cryptnox_id_cli/remote/protocol.py b/src/cryptnox_id_cli/remote/protocol.py new file mode 100644 index 0000000..67bf6ca --- /dev/null +++ b/src/cryptnox_id_cli/remote/protocol.py @@ -0,0 +1,225 @@ +"""Frame codec for the remote PIV service. + +The service exchanges JSON objects over a WebSocket. Frames are sent one +compact object per text message. On receive, a message is split on newlines and +every non-empty line is parsed as its own frame, so both framings are accepted. + +Everything arriving from the service is untrusted input: sizes are capped, types +are checked, and unknown fields are ignored rather than trusted. Unknown frame +types are reported rather than skipped, because a frame this client does not +understand may be one it was meant to answer. +""" + +from __future__ import annotations + +import json +import re +from collections.abc import Mapping +from dataclasses import dataclass, field + +from ..transport.errors import RemoteProtocolError + +#: Operations the service exposes. +OPS: tuple[str, ...] = ("authenticate", "inspect", "attest", "reset", "dev-reset") + +#: Largest WebSocket message accepted, in bytes. A CAP load block is ~255 bytes +#: and the largest result carries a certificate, so this is generous. +MAX_MESSAGE_BYTES = 1 << 20 + +#: Largest number of frames accepted in a single message. +MAX_FRAMES_PER_MESSAGE = 16 + +#: Largest command APDU accepted from the service, in bytes. +MAX_APDU_BYTES = 65544 + +#: Longest untrusted string rendered from a service frame. +MAX_TEXT_CHARS = 2000 + +_ANSI_CSI = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") +_ANSI_OTHER = re.compile(r"\x1b[@-Z\\-_]") + + +@dataclass(frozen=True) +class PowChallenge: + """A proof-of-work challenge the client must answer before the operation runs.""" + + challenge: bytes + bits: int + + +@dataclass(frozen=True) +class ApduCommand: + """A command the service wants transmitted to the card, verbatim.""" + + apdu: bytes + + @property + def header_hex(self) -> str: + return self.apdu[:4].hex().upper() + + +@dataclass(frozen=True) +class AtrRequest: + """A request for the card's answer-to-reset.""" + + +@dataclass(frozen=True) +class LogMessage: + """An informational line from the service. Untrusted text.""" + + message: str + + +@dataclass(frozen=True) +class OpResult: + """The final frame: the operation's outcome.""" + + payload: dict[str, object] = field(default_factory=dict) + + @property + def ok(self) -> bool: + return bool(self.payload.get("ok", False)) + + +ServerFrame = PowChallenge | ApduCommand | AtrRequest | LogMessage | OpResult + + +def sanitize_server_text(text: object, *, limit: int = MAX_TEXT_CHARS) -> str: + """Make a string from the service safe to display. + + Strips ANSI escape sequences and control characters, and caps the length. + Callers rendering through rich must still escape markup. + """ + if not isinstance(text, str): + text = str(text) + cleaned = _ANSI_CSI.sub("", text) + cleaned = _ANSI_OTHER.sub("", cleaned) + cleaned = "".join(ch for ch in cleaned if ch == " " or ch.isprintable()) + if len(cleaned) > limit: + cleaned = cleaned[:limit] + "… (truncated)" + return cleaned + + +def encode_frame(frame: Mapping[str, object]) -> str: + """Render one frame as a compact JSON text message.""" + return json.dumps(frame, separators=(",", ":")) + + +def decode_frames(message: str | bytes) -> list[dict[str, object]]: + """Parse a WebSocket message into frames. + + Accepts either a single JSON object or several separated by newlines. + """ + if isinstance(message, bytes | bytearray): + try: + message = bytes(message).decode("utf-8") + except UnicodeDecodeError as exc: + raise RemoteProtocolError("the service sent a message that is not UTF-8") from exc + if len(message.encode("utf-8", "replace")) > MAX_MESSAGE_BYTES: + raise RemoteProtocolError( + f"the service sent a message larger than {MAX_MESSAGE_BYTES} bytes" + ) + + frames: list[dict[str, object]] = [] + for line in message.split("\n"): + if not line.strip(): + continue + if len(frames) >= MAX_FRAMES_PER_MESSAGE: + raise RemoteProtocolError( + f"the service sent more than {MAX_FRAMES_PER_MESSAGE} frames in one message" + ) + try: + obj = json.loads(line) + except ValueError as exc: + raise RemoteProtocolError(f"the service sent malformed JSON: {exc}") from exc + if not isinstance(obj, dict): + raise RemoteProtocolError("the service sent a frame that is not a JSON object") + frames.append(obj) + if not frames: + raise RemoteProtocolError("the service sent an empty message") + return frames + + +def _require_str(obj: Mapping[str, object], key: str, kind: str) -> str: + value = obj.get(key) + if not isinstance(value, str): + raise RemoteProtocolError(f"the service sent a {kind} frame without a usable {key!r}") + return value + + +def _require_hex(obj: Mapping[str, object], key: str, kind: str, *, limit: int) -> bytes: + text = _require_str(obj, key, kind).strip() + if len(text) % 2 != 0: + raise RemoteProtocolError(f"the service sent a {kind} frame with odd-length hex {key!r}") + if len(text) // 2 > limit: + raise RemoteProtocolError( + f"the service sent a {kind} frame whose {key!r} exceeds {limit} bytes" + ) + try: + return bytes.fromhex(text) + except ValueError as exc: + raise RemoteProtocolError( + f"the service sent a {kind} frame with invalid hex in {key!r}" + ) from exc + + +def parse_server_frame(obj: Mapping[str, object]) -> ServerFrame: + """Turn a decoded frame into its typed form.""" + kind = obj.get("type") + if not isinstance(kind, str): + raise RemoteProtocolError("the service sent a frame without a type") + + if kind == "pow_challenge": + challenge = _require_hex(obj, "challenge", kind, limit=64) + bits = obj.get("bits") + if not isinstance(bits, int) or isinstance(bits, bool) or bits < 0: + raise RemoteProtocolError( + "the service sent a proof-of-work challenge without a usable bit count" + ) + return PowChallenge(challenge=challenge, bits=bits) + + if kind == "apdu": + apdu = _require_hex(obj, "hex", kind, limit=MAX_APDU_BYTES) + if len(apdu) < 4: + raise RemoteProtocolError("the service sent a command shorter than an APDU header") + return ApduCommand(apdu=apdu) + + if kind == "atr": + return AtrRequest() + + if kind == "log": + return LogMessage(message=sanitize_server_text(obj.get("msg", ""))) + + if kind == "result": + payload = {k: v for k, v in obj.items() if k != "type"} + return OpResult(payload=payload) + + raise RemoteProtocolError(f"the service sent an unsupported frame type: {kind!r}") + + +def hello_frame(op: str, params: Mapping[str, object] | None = None) -> dict[str, object]: + """Build the opening frame that names the operation.""" + if op not in OPS: + raise ValueError(f"unknown remote operation: {op!r}") + return {"type": "hello", "op": op, "params": dict(params or {})} + + +def pow_frame(nonce: int) -> dict[str, object]: + return {"type": "pow", "nonce": nonce} + + +def apdu_response_frame(data: bytes, sw1: int, sw2: int) -> dict[str, object]: + return { + "type": "apdu_resp", + "data": bytes(data).hex().upper(), + "sw1": sw1 & 0xFF, + "sw2": sw2 & 0xFF, + } + + +def apdu_error_frame(message: str) -> dict[str, object]: + return {"type": "apdu_resp", "error": message} + + +def atr_response_frame(atr: bytes) -> dict[str, object]: + return {"type": "atr_resp", "atr": bytes(atr).hex().upper()} diff --git a/src/cryptnox_id_cli/remote/relay.py b/src/cryptnox_id_cli/remote/relay.py new file mode 100644 index 0000000..c2737cf --- /dev/null +++ b/src/cryptnox_id_cli/remote/relay.py @@ -0,0 +1,207 @@ +"""The operation loop: announce the operation, answer the proof-of-work, relay +commands under the policy, return the result. + +The service drives. This loop only ever answers: it never sends a command of its +own to the card, never reassembles a chained response, and never retries. What +the card says goes back verbatim, status word included, so the service sees the +card exactly as a local reader would. The one thing the loop adds is refusal, +through :class:`~cryptnox_id_cli.remote.policy.RelayPolicy`, and every refusal +ends the operation. + +Relayed commands are written to the transcript with their data masked unless +the instruction is on a short list of known-harmless reads. That is the reverse +of the CLI's own transcript rule, because here the client did not choose the +command and cannot vouch for what it carries. +""" + +from __future__ import annotations + +import time +from collections.abc import Callable, Mapping +from dataclasses import dataclass, field + +from ..secrets.redaction import Redactor +from ..transport.errors import ( + RemoteError, + RemoteOperationError, + RemoteProtocolError, + TransportError, +) +from ..transport.pcsc import RawConnection +from . import pow as rpow +from . import protocol as proto +from .channel import RECV_TIMEOUT, FrameChannel +from .policy import ( + INS_GET_DATA_GP, + INS_GET_DATA_PIV, + INS_GET_RESPONSE, + INS_GET_STATUS, + INS_INITIALIZE_UPDATE, + INS_SELECT, + RelayPolicy, +) + +#: Instructions whose command data is shown in the transcript. Everything else +#: is masked: the service chose it, and the relay cannot tell what it carries. +TRANSCRIPT_CLEAR_INS: frozenset[int] = frozenset( + { + INS_SELECT, + INS_GET_DATA_GP, + INS_GET_DATA_PIV, + INS_GET_STATUS, + INS_GET_RESPONSE, + INS_INITIALIZE_UPDATE, + } +) + +#: Proof-of-work challenges answered per operation. The service issues one. +MAX_POW_CHALLENGES = 1 + + +@dataclass +class RelayReport: + """What happened during one operation.""" + + op: str + result: dict[str, object] + apdus_relayed: int = 0 + seconds: float = 0.0 + atr: bytes | None = None + log_lines: list[str] = field(default_factory=list) + pow_bits: int | None = None + + @property + def ok(self) -> bool: + return bool(self.result.get("ok", False)) + + +def redact_relayed_command(apdu: bytes, redactor: Redactor, *, unmasked: bool = False) -> str: + """Transcript rendering of a relayed command: masked unless known-harmless.""" + if unmasked or (len(apdu) >= 4 and apdu[1] in TRANSCRIPT_CLEAR_INS): + return redactor.redact_command(apdu) + header = apdu[:4].hex().upper() + body = apdu[4:] + if not body: + return header + return f"{header}" + + +def redact_relayed_response( + data: bytes, sw1: int, sw2: int, ins: int, redactor: Redactor, *, unmasked: bool = False +) -> str: + """Transcript rendering of a relayed response: masked outside known reads.""" + if unmasked or ins in TRANSCRIPT_CLEAR_INS: + return redactor.redact_response(data, sw1, sw2, ins=ins) + if not data: + return f"{sw1:02X}{sw2:02X}" + return f"{sw1:02X}{sw2:02X}" + + +def run_operation( + channel: FrameChannel, + card: RawConnection, + op: str, + params: Mapping[str, object] | None = None, + *, + policy: RelayPolicy | None = None, + redactor: Redactor | None = None, + transcript: Callable[[str], None] | None = None, + unmasked_transcript: bool = False, + on_log: Callable[[str], None] | None = None, + recv_timeout: float = RECV_TIMEOUT, + clock: Callable[[], float] = time.monotonic, +) -> RelayReport: + """Run one operation against the service and return its report. + + Raises a :class:`~cryptnox_id_cli.transport.errors.RemoteError` subclass on + any failure. A result frame with ``ok`` false raises + :class:`RemoteOperationError` carrying the service's payload. + """ + policy = policy or RelayPolicy(op) + redactor = redactor or Redactor() + report = RelayReport(op=op, result={}) + started: float | None = None + pow_answered = 0 + + def emit(line: str) -> None: + if transcript is not None: + transcript(line) + + channel.send(proto.encode_frame(proto.hello_frame(op, params))) + + while True: + raw = channel.recv(timeout=recv_timeout) + for obj in proto.decode_frames(raw): + frame = proto.parse_server_frame(obj) + + if isinstance(frame, proto.PowChallenge): + if pow_answered >= MAX_POW_CHALLENGES: + raise RemoteProtocolError( + "the service issued a second proof-of-work challenge in one operation" + ) + nonce = rpow.solve(frame.challenge, frame.bits) + pow_answered += 1 + report.pow_bits = frame.bits + channel.send(proto.encode_frame(proto.pow_frame(nonce))) + continue + + if isinstance(frame, proto.LogMessage): + report.log_lines.append(frame.message) + if on_log is not None: + on_log(frame.message) + continue + + if isinstance(frame, proto.AtrRequest): + atr = bytes(card.get_atr()) + report.atr = atr + channel.send(proto.encode_frame(proto.atr_response_frame(atr))) + continue + + if isinstance(frame, proto.ApduCommand): + if started is None: + started = clock() + elapsed = clock() - started + if elapsed > policy.rules.max_seconds: + raise RemoteError( + f"the {op} operation exceeded {policy.rules.max_seconds:g} seconds " + "of card activity" + ) + apdu = frame.apdu + policy.check(apdu) # raises RemotePolicyError; nothing was sent + emit("> " + redact_relayed_command(apdu, redactor, unmasked=unmasked_transcript)) + try: + data, sw1, sw2 = card.transmit(list(apdu)) + except TransportError as exc: + channel.send(proto.encode_frame(proto.apdu_error_frame(str(exc)))) + raise RemoteError(f"the card stopped answering: {exc}") from exc + body = bytes(data) + emit( + "< " + + redact_relayed_response( + body, sw1, sw2, apdu[1], redactor, unmasked=unmasked_transcript + ) + ) + report.apdus_relayed += 1 + policy.observe(apdu, sw1, sw2) # may raise after a failed authentication + channel.send(proto.encode_frame(proto.apdu_response_frame(body, sw1, sw2))) + continue + + if isinstance(frame, proto.OpResult): + report.result = frame.payload + if started is not None: + report.seconds = clock() - started + if not frame.ok: + raise RemoteOperationError( + _failure_message(op, frame.payload), result=frame.payload + ) + return report + + raise RemoteProtocolError(f"unhandled frame {type(frame).__name__}") + + +def _failure_message(op: str, payload: Mapping[str, object]) -> str: + for key in ("error", "message", "reason", "detail"): + value = payload.get(key) + if isinstance(value, str) and value.strip(): + return f"the service reported {op} as failed: {proto.sanitize_server_text(value)}" + return f"the service reported {op} as failed" diff --git a/src/cryptnox_id_cli/transport/errors.py b/src/cryptnox_id_cli/transport/errors.py index f1f8228..dbdd390 100644 --- a/src/cryptnox_id_cli/transport/errors.py +++ b/src/cryptnox_id_cli/transport/errors.py @@ -69,6 +69,59 @@ class Scp02Error(CryptnoxError): exit_code = 7 +class RemoteError(CryptnoxError): + """A remote PIV service operation could not be carried out.""" + + code = "remote_error" + exit_code = 12 + + +class RemoteConnectError(RemoteError): + """The service could not be reached, or the TLS handshake failed.""" + + code = "remote_connect" + exit_code = 13 + + +class RemoteProtocolError(RemoteError): + """The service sent something this client cannot interpret safely.""" + + code = "remote_protocol" + exit_code = 14 + + +class RemotePolicyError(RemoteError): + """The service asked for a command the relay policy refuses to send. + + Carries the offending APDU header so the rule set can be reviewed against + what the service actually needs. + """ + + code = "remote_policy" + exit_code = 15 + + def __init__(self, message: str, *, header: str | None = None) -> None: + super().__init__(message) + self.header = header + + def to_dict(self) -> dict[str, object]: + return {"error": self.code, "message": str(self), "apdu_header": self.header} + + +class RemoteOperationError(RemoteError): + """The service completed the exchange but reported the operation as failed.""" + + code = "remote_failed" + exit_code = 16 + + def __init__(self, message: str, *, result: dict[str, object] | None = None) -> None: + super().__init__(message) + self.result = result + + def to_dict(self) -> dict[str, object]: + return {"error": self.code, "message": str(self), "result": self.result} + + @dataclass(frozen=True) class SWInfo: sw: int diff --git a/tests/unit/test_remote_channel.py b/tests/unit/test_remote_channel.py new file mode 100644 index 0000000..f8f1511 --- /dev/null +++ b/tests/unit/test_remote_channel.py @@ -0,0 +1,204 @@ +"""The channel to the service: URL policy, TLS defaults, and error mapping.""" + +import ssl + +import pytest +from websockets.exceptions import ConnectionClosed, InvalidURI, WebSocketException + +from cryptnox_id_cli.remote import channel as ch +from cryptnox_id_cli.transport.errors import RemoteConnectError + + +# --------------------------------------------------------------------------- # +# URL policy # +# --------------------------------------------------------------------------- # +def test_production_url_is_wss(): + assert ch.PRODUCTION_URL.startswith("wss://") + assert ch.validate_url(ch.PRODUCTION_URL) == ch.PRODUCTION_URL + + +def test_plain_ws_to_a_remote_host_is_refused(): + with pytest.raises(RemoteConnectError, match="unencrypted"): + ch.validate_url("ws://piv.cryptnox.com") + + +def test_plain_ws_to_loopback_is_refused_unless_opted_in(): + with pytest.raises(RemoteConnectError, match="unencrypted"): + ch.validate_url("ws://localhost:8765") + assert ch.validate_url("ws://localhost:8765", allow_insecure_loopback=True) + assert ch.validate_url("ws://127.0.0.1:8765", allow_insecure_loopback=True) + + +def test_loopback_opt_in_does_not_extend_to_other_hosts(): + with pytest.raises(RemoteConnectError, match="unencrypted"): + ch.validate_url("ws://localhost.evil.example", allow_insecure_loopback=True) + + +def test_non_websocket_schemes_are_refused(): + for url in ("https://piv.cryptnox.com", "piv.cryptnox.com", "file:///etc/passwd"): + with pytest.raises(RemoteConnectError, match="not a WebSocket URL"): + ch.validate_url(url) + + +# --------------------------------------------------------------------------- # +# TLS defaults # +# --------------------------------------------------------------------------- # +def test_default_tls_context_verifies_peer_and_hostname(): + context = ch.default_ssl_context() + assert context.check_hostname is True + assert context.verify_mode == ssl.CERT_REQUIRED + assert context.minimum_version >= ssl.TLSVersion.TLSv1_2 + + +# --------------------------------------------------------------------------- # +# The WebSocket-backed channel # +# --------------------------------------------------------------------------- # +class _FakeConnection: + def __init__(self, *, recv_result=None, recv_raises=None, send_raises=None): + self.sent: list[str] = [] + self.closed = False + self._recv_result = recv_result + self._recv_raises = recv_raises + self._send_raises = send_raises + self.last_timeout = None + + def send(self, message): + if self._send_raises: + raise self._send_raises + self.sent.append(message) + + def recv(self, timeout=None): + self.last_timeout = timeout + if self._recv_raises: + raise self._recv_raises + return self._recv_result + + def close(self): + self.closed = True + + +def test_channel_satisfies_the_protocol(): + assert isinstance(ch.WebSocketChannel(_FakeConnection()), ch.FrameChannel) + + +def test_send_passes_the_message_through(): + conn = _FakeConnection() + ch.WebSocketChannel(conn).send('{"type":"hello"}') + assert conn.sent == ['{"type":"hello"}'] + + +def test_recv_passes_the_timeout_through_and_returns_text(): + conn = _FakeConnection(recv_result='{"type":"atr"}') + assert ch.WebSocketChannel(conn).recv(timeout=7.5) == '{"type":"atr"}' + assert conn.last_timeout == 7.5 + + +def test_recv_decodes_binary_frames_as_utf8(): + conn = _FakeConnection(recv_result=b'{"type":"atr"}') + assert ch.WebSocketChannel(conn).recv() == '{"type":"atr"}' + + +def test_recv_refuses_binary_that_is_not_utf8(): + conn = _FakeConnection(recv_result=b"\xff\xfe") + with pytest.raises(RemoteConnectError, match="UTF-8"): + ch.WebSocketChannel(conn).recv() + + +def test_recv_timeout_names_the_wait(): + conn = _FakeConnection(recv_raises=TimeoutError()) + with pytest.raises(RemoteConnectError, match="nothing for 30 seconds"): + ch.WebSocketChannel(conn).recv(timeout=30) + + +def test_recv_on_a_closed_connection_says_so(): + conn = _FakeConnection(recv_raises=ConnectionClosed(None, None)) + with pytest.raises(RemoteConnectError, match="closed the connection"): + ch.WebSocketChannel(conn).recv() + + +def test_recv_wraps_other_websocket_failures(): + conn = _FakeConnection(recv_raises=WebSocketException("boom")) + with pytest.raises(RemoteConnectError, match="connection to the service failed"): + ch.WebSocketChannel(conn).recv() + + +def test_recv_wraps_os_errors(): + conn = _FakeConnection(recv_raises=ConnectionResetError("reset by peer")) + with pytest.raises(RemoteConnectError, match="reset by peer"): + ch.WebSocketChannel(conn).recv() + + +def test_send_wraps_failures(): + conn = _FakeConnection(send_raises=WebSocketException("gone")) + with pytest.raises(RemoteConnectError): + ch.WebSocketChannel(conn).send("x") + + +def test_close_swallows_errors_so_the_real_failure_survives(): + class _Explodes(_FakeConnection): + def close(self): + raise RuntimeError("already dead") + + ch.WebSocketChannel(_Explodes()).close() # must not raise + + +# --------------------------------------------------------------------------- # +# Opening the channel # +# --------------------------------------------------------------------------- # +def test_open_channel_uses_a_verifying_tls_context_for_wss(monkeypatch): + seen = {} + + def fake_connect(url, **kwargs): + seen["url"] = url + seen.update(kwargs) + return _FakeConnection() + + monkeypatch.setattr("websockets.sync.client.connect", fake_connect) + with ch.open_channel("wss://piv.cryptnox.com") as channel: + assert isinstance(channel, ch.WebSocketChannel) + assert seen["url"] == "wss://piv.cryptnox.com" + assert isinstance(seen["ssl"], ssl.SSLContext) + assert seen["ssl"].check_hostname is True + assert seen["open_timeout"] == ch.CONNECT_TIMEOUT + assert seen["max_size"] == ch.MAX_MESSAGE_BYTES + + +def test_open_channel_leaves_tls_off_for_plain_ws(monkeypatch): + seen = {} + + def fake_connect(url, **kwargs): + seen.update(kwargs) + return _FakeConnection() + + monkeypatch.setattr("websockets.sync.client.connect", fake_connect) + with ch.open_channel("ws://localhost:1"): + pass + assert seen["ssl"] is None + + +def test_open_channel_closes_on_exit_even_after_an_error(monkeypatch): + conn = _FakeConnection() + monkeypatch.setattr("websockets.sync.client.connect", lambda url, **kw: conn) + with pytest.raises(RuntimeError), ch.open_channel("wss://x"): + raise RuntimeError("inside") + assert conn.closed is True + + +@pytest.mark.parametrize( + ("raised", "expected"), + [ + (ssl.SSLCertVerificationError(1, "self-signed certificate"), "could not be verified"), + (ssl.SSLError("handshake failure"), "TLS handshake"), + (InvalidURI("wss://x", "bad"), "not a usable service URL"), + (TimeoutError(), "did not answer within"), + (WebSocketException("426"), "could not open a connection"), + (ConnectionRefusedError("refused"), "could not reach the service"), + ], +) +def test_open_channel_maps_every_failure_to_a_connect_error(monkeypatch, raised, expected): + def fake_connect(url, **kwargs): + raise raised + + monkeypatch.setattr("websockets.sync.client.connect", fake_connect) + with pytest.raises(RemoteConnectError, match=expected), ch.open_channel("wss://x"): + pass diff --git a/tests/unit/test_remote_commands.py b/tests/unit/test_remote_commands.py new file mode 100644 index 0000000..4c50fd8 --- /dev/null +++ b/tests/unit/test_remote_commands.py @@ -0,0 +1,218 @@ +"""The ``remote`` command group: wiring, output shape, gates. No socket, no card.""" + +import contextlib +import json + +import pytest +from click.testing import CliRunner + +from cryptnox_id_cli.cli.commands import remote as remote_cmd +from cryptnox_id_cli.cli.context import AppContext +from cryptnox_id_cli.cli.main import main as root +from cryptnox_id_cli.remote import channel as rchannel + +ATR = "3BFA1300FF910131FE000031C173C84000009000D2" +CPLC_CMD = "80CA9F7F00" +CPLC_RESP = ( + "9F7F2A4790D6004700000000004301012345670042000000000000000002507231323334350000000000000000" +) +SELECT_ISD = "00A4040008A00000015100000000" +SELECT_WALLET = "00A4040007A000001000011200" + + +class ScriptedChannel: + def __init__(self, script): + self.script = list(script) + self.sent: list[dict] = [] + self.closed = False + + def send(self, message: str) -> None: + self.sent.append(json.loads(message)) + + def recv(self, timeout=None) -> str: + return json.dumps(self.script.pop(0)) + + def close(self) -> None: + self.closed = True + + +@pytest.fixture +def wired(monkeypatch, mock_connection): + """Patch the reader, the card and the channel; return the scripted channel.""" + conn = mock_connection( + ATR, + {SELECT_ISD: "6F108408A000000151000000A5049F6501FF|9000", CPLC_CMD: f"{CPLC_RESP}|9000"}, + [], + ) + monkeypatch.setattr(remote_cmd, "pick_reader", lambda pref: "Fake Reader 00 00") + monkeypatch.setattr(remote_cmd, "connect", lambda name: conn) + holder: dict = {} + + @contextlib.contextmanager + def fake_open(url, **kwargs): + holder["url"] = url + yield holder["channel"] + + monkeypatch.setattr(rchannel, "open_channel", fake_open) + monkeypatch.delenv(rchannel.URL_ENV, raising=False) + + def arm(script): + holder["channel"] = ScriptedChannel(script) + return holder["channel"] + + holder["arm"] = arm + return holder + + +def run(args, **kwargs): + return CliRunner().invoke(root, args, **kwargs) + + +# --------------------------------------------------------------------------- # +# Wiring # +# --------------------------------------------------------------------------- # +def test_group_is_registered_with_both_read_only_commands(): + result = run(["remote", "--help"]) + assert result.exit_code == 0 + assert "authenticate" in result.output + assert "inspect" in result.output + + +def test_dry_run_refuses_before_anything_opens(monkeypatch): + def boom(*a, **k): + raise AssertionError("reader must not be opened under --dry-run") + + monkeypatch.setattr(remote_cmd, "pick_reader", boom) + result = run(["--dry-run", "remote", "inspect"]) + assert result.exit_code != 0 + assert "nothing was sent to the card" in result.output + + +# --------------------------------------------------------------------------- # +# A whole operation through the command # +# --------------------------------------------------------------------------- # +def test_authenticate_json_payload(wired): + channel = wired["arm"]( + [ + {"type": "pow_challenge", "challenge": "0011223344556677", "bits": 4}, + {"type": "atr"}, + {"type": "log", "msg": "hello from the service"}, + {"type": "apdu", "hex": CPLC_CMD}, + {"type": "result", "ok": True, "cplc_uid": "4301012345670042", "atr": ATR}, + ] + ) + result = run(["--json", "remote", "authenticate"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + + assert payload["op"] == "authenticate" + assert payload["endpoint"] == rchannel.PRODUCTION_URL + assert wired["url"] == rchannel.PRODUCTION_URL + assert payload["card"]["atr"] == ATR + assert payload["card"]["cplc_uid"] == "4301012345670042" + assert payload["card"]["ic_serial"] == "01234567" + assert payload["result"]["ok"] is True + assert payload["verified_locally"] == {"cplc_uid_matches": True, "atr_matches": True} + assert payload["relay"]["apdus_relayed"] == 1 + assert payload["relay"]["pow_bits"] == 4 + assert payload["relay"]["service_log"] == ["hello from the service"] + assert [f["type"] for f in channel.sent] == ["hello", "pow", "atr_resp", "apdu_resp"] + assert channel.sent[0]["op"] == "authenticate" + + +def test_mismatch_between_service_and_card_is_reported_not_hidden(wired): + wired["arm"]([{"type": "result", "ok": True, "cplc_uid": "0000000000000000"}]) + result = run(["--json", "remote", "authenticate"]) + assert result.exit_code == 0 + assert json.loads(result.stdout)["verified_locally"]["cplc_uid_matches"] is False + + +def test_human_output_separates_asserted_from_verified(wired): + wired["arm"]([{"type": "result", "ok": True, "piv_ssd": {"present": False}}]) + result = run(["remote", "inspect"]) + assert result.exit_code == 0, result.output + assert "asserted by the service" in result.output + assert "Verified locally" in result.output + assert "piv_ssd" in result.output + + +# --------------------------------------------------------------------------- # +# Failures map to their exit codes and JSON shapes # +# --------------------------------------------------------------------------- # +def test_policy_refusal_exits_15_with_the_header(wired): + wired["arm"]([{"type": "apdu", "hex": SELECT_WALLET}]) + result = run(["--json", "remote", "inspect"]) + assert result.exit_code == 15 + error = json.loads(result.stdout) + assert error["error"] == "remote_policy" + assert error["apdu_header"] == "00A40400" + + +def test_service_failure_exits_16_with_the_result(wired): + wired["arm"]([{"type": "result", "ok": False, "error": "rate limit"}]) + result = run(["--json", "remote", "inspect"]) + assert result.exit_code == 16 + error = json.loads(result.stdout) + assert error["error"] == "remote_failed" + assert error["result"]["error"] == "rate limit" + + +def test_protocol_violation_exits_14(wired): + wired["arm"]([{"type": "nonsense"}]) + result = run(["--json", "remote", "inspect"]) + assert result.exit_code == 14 + assert json.loads(result.stdout)["error"] == "remote_protocol" + + +def test_card_is_released_even_when_the_operation_fails(wired, monkeypatch): + released = [] + conn = remote_cmd.connect("x") + monkeypatch.setattr(conn, "disconnect", lambda: released.append(True)) + wired["arm"]([{"type": "nonsense"}]) + run(["--json", "remote", "inspect"]) + assert released == [True] + + +# --------------------------------------------------------------------------- # +# Endpoint override # +# --------------------------------------------------------------------------- # +def test_env_override_redirects_read_only_ops_and_warns(wired, monkeypatch): + monkeypatch.setenv(rchannel.URL_ENV, "ws://localhost:9999") + wired["arm"]([{"type": "result", "ok": True}]) + result = run(["remote", "inspect"]) + assert result.exit_code == 0, result.output + assert wired["url"] == "ws://localhost:9999" + assert "non-production endpoint" in result.output + + +def test_env_override_to_a_remote_plain_ws_is_refused(wired, monkeypatch): + monkeypatch.setenv(rchannel.URL_ENV, "ws://piv.cryptnox.com") + result = run(["--json", "remote", "inspect"]) + assert result.exit_code == 13 + assert json.loads(result.stdout)["error"] == "remote_connect" + + +def test_endpoint_helper_refuses_override_for_operations_that_change_the_card(monkeypatch): + monkeypatch.setenv(rchannel.URL_ENV, "ws://localhost:9999") + with pytest.raises(Exception, match="runs only against the production service"): + remote_cmd._endpoint("reset") + + +# --------------------------------------------------------------------------- # +# Transcript # +# --------------------------------------------------------------------------- # +def test_relayed_commands_reach_the_apdu_log(wired, tmp_path): + log = tmp_path / "apdu.log" + wired["arm"]([{"type": "apdu", "hex": CPLC_CMD}, {"type": "result", "ok": True}]) + result = run(["--apdu-log", str(log), "remote", "inspect"]) + assert result.exit_code == 0, result.output + text = log.read_text() + assert "> " + CPLC_CMD in text + assert "< " + CPLC_RESP + "9000" in text + + +def test_app_context_apdu_trace_writes_and_traces(tmp_path): + app = AppContext(apdu_log_path=str(tmp_path / "t.log"), verbose=False) + app.apdu_trace("> 00A4040000") + app.close() + assert "> 00A4040000" in (tmp_path / "t.log").read_text() diff --git a/tests/unit/test_remote_policy.py b/tests/unit/test_remote_policy.py new file mode 100644 index 0000000..4e1a28a --- /dev/null +++ b/tests/unit/test_remote_policy.py @@ -0,0 +1,253 @@ +"""The relay policy: what a hostile or confused service cannot make the card do.""" + +import pytest + +from cryptnox_id_cli.remote import policy as pol +from cryptnox_id_cli.transport.errors import RemotePolicyError + +WALLET_AID = bytes.fromhex("A0000010000112") +FIDO2_AID = bytes.fromhex("A0000006472F0001") +GENUINE_AID = bytes.fromhex("A000001000024701") +DESFIRE_AID = bytes.fromhex("D2760000850100") + + +def select(aid: bytes) -> bytes: + return bytes([0x00, 0xA4, 0x04, 0x00, len(aid)]) + aid + b"\x00" + + +def apdu(cla: int, ins: int, p1: int = 0, p2: int = 0, data: bytes = b"", le: bool = True) -> bytes: + out = bytes([cla, ins, p1, p2]) + if data: + out += bytes([len(data)]) + data + if le: + out += b"\x00" + return out + + +CPLC = apdu(0x80, 0xCA, 0x9F, 0x7F) +KEY_INFO = apdu(0x80, 0xCA, 0x00, 0xE0) +INIT_UPDATE = apdu(0x80, 0x50, 0x00, 0x00, bytes(8)) +EXT_AUTH = apdu(0x84, 0x82, 0x00, 0x00, bytes(16), le=False) +PIV_GET_DATA = apdu(0x00, 0xCB, 0x3F, 0xFF, bytes.fromhex("5C035FC102")) + + +def allow(policy: pol.RelayPolicy, command: bytes, sw=(0x90, 0x00)) -> None: + policy.check(command) + policy.observe(command, *sw) + + +# --------------------------------------------------------------------------- # +# Header parsing # +# --------------------------------------------------------------------------- # +def test_channel_bits_are_decoded(): + assert pol.Header(0x00, 0, 0, 0).channel == 0 + assert pol.Header(0x01, 0, 0, 0).channel == 1 + assert pol.Header(0x83, 0, 0, 0).channel == 3 + assert pol.Header(0x40, 0, 0, 0).channel == 4 + assert pol.Header(0x4F, 0, 0, 0).channel == 19 + + +def test_gp_secure_messaging_is_recognised(): + assert pol.Header(0x84, 0, 0, 0).secure_messaging is True + assert pol.Header(0x80, 0, 0, 0).secure_messaging is False + assert pol.Header(0x00, 0, 0, 0).secure_messaging is False + + +def test_select_target_extracts_the_aid(): + assert pol.select_target(select(pol.PIV_AID)) == pol.PIV_AID + assert pol.select_target(bytes.fromhex("00A4040000")) == b"" + assert pol.select_target(CPLC) is None + + +def test_select_by_file_id_is_refused_not_ignored(): + with pytest.raises(RemotePolicyError, match="not select-by-name"): + pol.select_target(bytes.fromhex("00A40000023F00")) + + +def test_extended_length_select_is_refused(): + with pytest.raises(RemotePolicyError, match="extended-length"): + pol.select_target(bytes.fromhex("00A40400000009") + pol.PIV_SSD_AID) + + +def test_select_with_short_data_is_refused(): + with pytest.raises(RemotePolicyError, match="shorter than its length"): + pol.select_target(bytes.fromhex("00A404000BA000")) + + +# --------------------------------------------------------------------------- # +# Operations without rules # +# --------------------------------------------------------------------------- # +def test_operations_without_rules_are_refused_up_front(): + with pytest.raises(RemotePolicyError, match="no relay policy"): + pol.RelayPolicy("reset") + + +# --------------------------------------------------------------------------- # +# authenticate: ATR + CPLC, nothing else # +# --------------------------------------------------------------------------- # +def test_authenticate_allows_cplc_from_the_card_manager(): + policy = pol.RelayPolicy("authenticate") + allow(policy, CPLC) + allow(policy, KEY_INFO) + allow(policy, select(pol.ISD_AID)) + allow(policy, bytes.fromhex("00A4040000")) # select the default applet + assert policy.context == pol.ISD + + +def test_authenticate_refuses_selecting_the_piv_domains(): + policy = pol.RelayPolicy("authenticate") + for aid in (pol.PIV_SSD_AID, pol.PIV_AID): + with pytest.raises(RemotePolicyError, match="not allowed during authenticate"): + policy.check(select(aid)) + + +def test_authenticate_refuses_initialize_update(): + policy = pol.RelayPolicy("authenticate") + with pytest.raises(RemotePolicyError, match="not allowed while the isd"): + policy.check(INIT_UPDATE) + + +# --------------------------------------------------------------------------- # +# Every operation: the other card functions are unreachable # +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +@pytest.mark.parametrize("aid", [WALLET_AID, FIDO2_AID, GENUINE_AID, DESFIRE_AID]) +def test_other_card_functions_cannot_be_selected(op, aid): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="SELECT of AID") as info: + policy.check(select(aid)) + assert info.value.header == "00A40400" + + +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +def test_partial_aid_select_cannot_sneak_past_the_allow_list(op): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="SELECT of AID"): + policy.check(select(pol.PIV_AID[:5])) + + +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +def test_ctap_framing_is_refused(op): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="CTAP"): + policy.check(apdu(0x80, 0x10, 0x00, 0x00, b"\x04")) + + +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +def test_desfire_wrapping_is_refused(op): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="DESFire"): + policy.check(apdu(0x90, 0x60)) + + +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +def test_logical_channels_are_refused(op): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="logical channel 1"): + policy.check(apdu(0x01, 0xCA, 0x9F, 0x7F)) + with pytest.raises(RemotePolicyError, match="logical channel 4"): + policy.check(apdu(0x40, 0xCA, 0x9F, 0x7F)) + + +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +@pytest.mark.parametrize("ins", [0x20, 0x24, 0x2C]) +def test_card_holder_verifier_commands_are_never_relayed(op, ins): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="never relayed"): + policy.check(apdu(0x00, ins, 0x00, 0x80, b"\xff" * 8, le=False)) + + +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +def test_iso_secure_messaging_class_is_refused(op): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="ISO secure messaging"): + policy.check(apdu(0x0C, 0xCA, 0x9F, 0x7F)) + + +# --------------------------------------------------------------------------- # +# inspect: probe, but never authenticate # +# --------------------------------------------------------------------------- # +def test_inspect_follows_the_selected_applet(): + policy = pol.RelayPolicy("inspect") + allow(policy, CPLC) + allow(policy, select(pol.PIV_SSD_AID)) + assert policy.context == pol.PIV_SSD + allow(policy, KEY_INFO) + allow(policy, select(pol.PIV_AID)) + assert policy.context == pol.PIV + allow(policy, PIV_GET_DATA) + + +def test_a_failed_select_does_not_move_the_context(): + policy = pol.RelayPolicy("inspect") + allow(policy, select(pol.PIV_SSD_AID), sw=(0x6A, 0x82)) + assert policy.context == pol.ISD + allow(policy, KEY_INFO) # still the card manager's key table + + +def test_piv_context_allows_only_piv_reads(): + policy = pol.RelayPolicy("inspect") + allow(policy, select(pol.PIV_AID)) + for forbidden in ( + apdu(0x00, 0x47, 0x00, 0x9C, b"\xac\x03\x80\x01\x11", le=False), # GENERATE + apdu(0x00, 0xDB, 0x3F, 0xFF, b"\x5c\x03\x5f\xc1\x02", le=False), # PUT DATA + apdu(0x00, 0x87, 0x11, 0x9B, b"\x7c\x02\x81\x00", le=False), # GENERAL AUTH + apdu(0x80, 0xCA, 0x9F, 0x7F), # GP GET DATA is not a PIV command + INIT_UPDATE, + ): + with pytest.raises(RemotePolicyError, match="not allowed while the piv"): + policy.check(forbidden) + + +def test_inspect_caps_initialize_updates_per_domain(): + policy = pol.RelayPolicy("inspect") + budget = policy.rules.max_initialize_updates + for _ in range(budget): + allow(policy, INIT_UPDATE) # to the card manager, probing key versions + with pytest.raises(RemotePolicyError, match=f"more than {budget} time"): + policy.check(INIT_UPDATE) + allow(policy, select(pol.PIV_SSD_AID)) + for _ in range(budget): + allow(policy, INIT_UPDATE) # a fresh budget for the PIV security domain + with pytest.raises(RemotePolicyError, match=f"more than {budget} time"): + policy.check(INIT_UPDATE) + + +def test_inspect_never_relays_external_authenticate(): + policy = pol.RelayPolicy("inspect") + allow(policy, INIT_UPDATE) + with pytest.raises(RemotePolicyError, match="EXTERNAL AUTHENTICATE"): + policy.check(EXT_AUTH) + + +def test_a_failed_select_leaves_the_initialize_update_budget_alone(): + policy = pol.RelayPolicy("inspect") + allow(policy, select(pol.PIV_SSD_AID), sw=(0x6A, 0x82)) + allow(policy, INIT_UPDATE) # still counts against the card manager + assert policy.initialize_updates == {pol.ISD: 1} + + +# --------------------------------------------------------------------------- # +# Volume # +# --------------------------------------------------------------------------- # +def test_command_budget_is_enforced(): + policy = pol.RelayPolicy("authenticate") + for _ in range(policy.rules.max_apdus): + allow(policy, CPLC) + with pytest.raises(RemotePolicyError, match="more than 20 commands"): + policy.check(CPLC) + + +def test_a_refused_command_is_not_counted(): + policy = pol.RelayPolicy("authenticate") + with pytest.raises(RemotePolicyError): + policy.check(INIT_UPDATE) + assert policy.apdus == 0 + + +def test_refusal_carries_the_header_for_review(): + policy = pol.RelayPolicy("authenticate") + with pytest.raises(RemotePolicyError) as info: + policy.check(apdu(0x00, 0x47, 0x00, 0x9C)) + assert info.value.header == "0047009C" + assert info.value.to_dict()["apdu_header"] == "0047009C" diff --git a/tests/unit/test_remote_pow.py b/tests/unit/test_remote_pow.py new file mode 100644 index 0000000..b032551 --- /dev/null +++ b/tests/unit/test_remote_pow.py @@ -0,0 +1,63 @@ +"""The proof-of-work solver: correct answers, and bounded work.""" + +import hashlib + +import pytest + +from cryptnox_id_cli.remote import pow as rpow +from cryptnox_id_cli.transport.errors import RemoteProtocolError + + +def test_leading_zero_bits_counts_bits_not_nibbles(): + assert rpow.leading_zero_bits(b"\xff") == 0 + assert rpow.leading_zero_bits(b"\x80") == 0 + assert rpow.leading_zero_bits(b"\x7f") == 1 + assert rpow.leading_zero_bits(b"\x01") == 7 + assert rpow.leading_zero_bits(b"\x00\xff") == 8 + assert rpow.leading_zero_bits(b"\x00\x7f") == 9 + assert rpow.leading_zero_bits(b"\x00\x00\x01") == 23 + assert rpow.leading_zero_bits(b"\x00\x00\x00\x00") == 32 + + +def test_digest_uses_decimal_nonce_not_bytes(): + challenge = bytes.fromhex("00112233") + # The service checks sha256(challenge || str(nonce)), so nonce 10 must hash + # the two ASCII characters "10", never the single byte 0x0A. + assert rpow.digest_for(challenge, 10) == hashlib.sha256(challenge + b"10").digest() + assert rpow.digest_for(challenge, 10) != hashlib.sha256(challenge + b"\x0a").digest() + + +def test_solve_returns_a_nonce_the_service_would_accept(): + challenge = bytes.fromhex("a1b2c3d4") + bits = 12 + nonce = rpow.solve(challenge, bits) + assert rpow.leading_zero_bits(rpow.digest_for(challenge, nonce)) >= bits + + +def test_solve_returns_the_first_valid_nonce(): + challenge = bytes.fromhex("deadbeef") + bits = 10 + nonce = rpow.solve(challenge, bits) + for earlier in range(nonce): + assert rpow.leading_zero_bits(rpow.digest_for(challenge, earlier)) < bits + + +def test_zero_bits_needs_no_work(): + assert rpow.solve(b"\x01\x02", 0) == 0 + + +def test_difficulty_beyond_the_ceiling_is_refused_without_hashing(): + with pytest.raises(RemoteProtocolError, match="at most"): + rpow.solve(b"\x00", rpow.MAX_BITS + 1) + + +def test_negative_difficulty_is_refused(): + with pytest.raises(RemoteProtocolError): + rpow.solve(b"\x00", -1) + + +def test_search_is_bounded_rather_than_endless(): + # A tight attempt budget cannot satisfy a difficulty this high, and the + # solver must give up instead of spinning. + with pytest.raises(RemoteProtocolError, match="attempts"): + rpow.solve(b"\x00", 24, max_attempts=50) diff --git a/tests/unit/test_remote_protocol.py b/tests/unit/test_remote_protocol.py new file mode 100644 index 0000000..9cbc4f9 --- /dev/null +++ b/tests/unit/test_remote_protocol.py @@ -0,0 +1,217 @@ +"""The frame codec: everything from the service is untrusted input.""" + +import json + +import pytest + +from cryptnox_id_cli.remote import protocol as proto +from cryptnox_id_cli.transport.errors import RemoteProtocolError + + +# --------------------------------------------------------------------------- # +# Decoding messages into frames # +# --------------------------------------------------------------------------- # +def test_single_object_message(): + frames = proto.decode_frames('{"type":"atr"}') + assert frames == [{"type": "atr"}] + + +def test_newline_separated_objects_in_one_message(): + frames = proto.decode_frames('{"type":"log","msg":"a"}\n{"type":"atr"}\n') + assert [f["type"] for f in frames] == ["log", "atr"] + + +def test_bytes_message_is_decoded_as_utf8(): + assert proto.decode_frames(b'{"type":"atr"}') == [{"type": "atr"}] + + +def test_non_utf8_message_is_refused(): + with pytest.raises(RemoteProtocolError, match="UTF-8"): + proto.decode_frames(b"\xff\xfe{") + + +def test_malformed_json_is_refused(): + with pytest.raises(RemoteProtocolError, match="malformed JSON"): + proto.decode_frames("{not json}") + + +def test_json_that_is_not_an_object_is_refused(): + with pytest.raises(RemoteProtocolError, match="not a JSON object"): + proto.decode_frames("[1, 2, 3]") + + +def test_empty_message_is_refused(): + with pytest.raises(RemoteProtocolError, match="empty message"): + proto.decode_frames(" \n \n") + + +def test_oversize_message_is_refused_before_parsing(): + huge = '{"type":"log","msg":"' + "x" * (proto.MAX_MESSAGE_BYTES + 10) + '"}' + with pytest.raises(RemoteProtocolError, match="larger than"): + proto.decode_frames(huge) + + +def test_too_many_frames_in_one_message_is_refused(): + message = "\n".join(['{"type":"atr"}'] * (proto.MAX_FRAMES_PER_MESSAGE + 1)) + with pytest.raises(RemoteProtocolError, match="more than"): + proto.decode_frames(message) + + +# --------------------------------------------------------------------------- # +# Typing the frames # +# --------------------------------------------------------------------------- # +def test_pow_challenge_parsed(): + frame = proto.parse_server_frame({"type": "pow_challenge", "challenge": "00ff", "bits": 20}) + assert isinstance(frame, proto.PowChallenge) + assert frame.challenge == b"\x00\xff" + assert frame.bits == 20 + + +def test_pow_challenge_without_bits_is_refused(): + with pytest.raises(RemoteProtocolError, match="bit count"): + proto.parse_server_frame({"type": "pow_challenge", "challenge": "00ff"}) + + +def test_pow_challenge_bits_must_not_be_a_bool(): + # json.loads turns `true` into a bool, which is an int subclass; a bool is + # not a difficulty. + with pytest.raises(RemoteProtocolError, match="bit count"): + proto.parse_server_frame({"type": "pow_challenge", "challenge": "00ff", "bits": True}) + + +def test_apdu_frame_parsed(): + frame = proto.parse_server_frame({"type": "apdu", "hex": "00A4040000"}) + assert isinstance(frame, proto.ApduCommand) + assert frame.apdu == bytes.fromhex("00A4040000") + assert frame.header_hex == "00A40400" + + +def test_apdu_shorter_than_a_header_is_refused(): + with pytest.raises(RemoteProtocolError, match="shorter than an APDU header"): + proto.parse_server_frame({"type": "apdu", "hex": "00A4"}) + + +def test_apdu_with_invalid_hex_is_refused(): + with pytest.raises(RemoteProtocolError, match="invalid hex"): + proto.parse_server_frame({"type": "apdu", "hex": "00zz040000"}) + + +def test_apdu_with_odd_length_hex_is_refused(): + with pytest.raises(RemoteProtocolError, match="odd-length"): + proto.parse_server_frame({"type": "apdu", "hex": "00A40400001"}) + + +def test_oversize_apdu_is_refused(): + with pytest.raises(RemoteProtocolError, match="exceeds"): + proto.parse_server_frame({"type": "apdu", "hex": "AA" * (proto.MAX_APDU_BYTES + 1)}) + + +def test_atr_request_parsed(): + assert isinstance(proto.parse_server_frame({"type": "atr"}), proto.AtrRequest) + + +def test_result_keeps_every_field_except_the_type(): + frame = proto.parse_server_frame( + {"type": "result", "ok": True, "wiped_and_reprovisioned": True, "steps": []} + ) + assert isinstance(frame, proto.OpResult) + assert frame.ok is True + assert frame.payload == {"ok": True, "wiped_and_reprovisioned": True, "steps": []} + assert "type" not in frame.payload + + +def test_result_without_ok_is_not_treated_as_success(): + frame = proto.parse_server_frame({"type": "result", "error": "rate limit"}) + assert isinstance(frame, proto.OpResult) + assert frame.ok is False + + +def test_unknown_frame_type_is_reported_not_skipped(): + with pytest.raises(RemoteProtocolError, match="unsupported frame type"): + proto.parse_server_frame({"type": "reboot_everything"}) + + +def test_frame_without_a_type_is_refused(): + with pytest.raises(RemoteProtocolError, match="without a type"): + proto.parse_server_frame({"msg": "hello?"}) + + +def test_unknown_fields_on_a_known_frame_are_ignored(): + frame = proto.parse_server_frame({"type": "atr", "future_field": 1}) + assert isinstance(frame, proto.AtrRequest) + + +# --------------------------------------------------------------------------- # +# Untrusted text # +# --------------------------------------------------------------------------- # +def test_log_message_is_stripped_of_ansi_and_control_characters(): + frame = proto.parse_server_frame({"type": "log", "msg": "\x1b[31mred\x1b[0m\x07 and \x00 gone"}) + assert isinstance(frame, proto.LogMessage) + assert "\x1b" not in frame.message + assert "\x07" not in frame.message + assert "\x00" not in frame.message + assert "red" in frame.message + + +def test_log_message_is_capped(): + frame = proto.parse_server_frame({"type": "log", "msg": "x" * 10_000}) + assert isinstance(frame, proto.LogMessage) + assert len(frame.message) <= proto.MAX_TEXT_CHARS + 20 + assert "truncated" in frame.message + + +def test_log_message_that_is_not_a_string_is_coerced(): + frame = proto.parse_server_frame({"type": "log", "msg": {"nested": "object"}}) + assert isinstance(frame, proto.LogMessage) + assert isinstance(frame.message, str) + + +# --------------------------------------------------------------------------- # +# Frames this client sends # +# --------------------------------------------------------------------------- # +def test_hello_names_the_operation_and_carries_params(): + frame = proto.hello_frame("attest", {"slot": "9C", "algorithm": "ECCP256"}) + assert frame["type"] == "hello" + assert frame["op"] == "attest" + assert frame["params"] == {"slot": "9C", "algorithm": "ECCP256"} + + +def test_hello_refuses_an_operation_the_service_does_not_expose(): + with pytest.raises(ValueError, match="unknown remote operation"): + proto.hello_frame("format-everything") + + +def test_hello_without_params_sends_an_empty_object(): + assert proto.hello_frame("inspect")["params"] == {} + + +def test_apdu_response_carries_uppercase_hex_and_integer_status_bytes(): + frame = proto.apdu_response_frame(b"\xde\xad", 0x90, 0x00) + assert frame == {"type": "apdu_resp", "data": "DEAD", "sw1": 0x90, "sw2": 0x00} + + +def test_apdu_response_with_no_body_sends_empty_hex(): + assert proto.apdu_response_frame(b"", 0x6A, 0x82)["data"] == "" + + +def test_apdu_error_frame_shape(): + assert proto.apdu_error_frame("card removed") == { + "type": "apdu_resp", + "error": "card removed", + } + + +def test_atr_response_shape(): + assert proto.atr_response_frame(b"\x3b\xfa")["atr"] == "3BFA" + + +def test_encode_is_compact_single_line_json(): + text = proto.encode_frame(proto.hello_frame("inspect")) + assert "\n" not in text + assert " " not in text + assert json.loads(text)["op"] == "inspect" + + +def test_round_trip_through_the_codec(): + text = proto.encode_frame({"type": "atr"}) + assert proto.decode_frames(text) == [{"type": "atr"}] diff --git a/tests/unit/test_remote_relay.py b/tests/unit/test_remote_relay.py new file mode 100644 index 0000000..aed5737 --- /dev/null +++ b/tests/unit/test_remote_relay.py @@ -0,0 +1,332 @@ +"""The operation loop, driven by a scripted service and a mock card.""" + +import json + +import pytest + +from cryptnox_id_cli.remote import pow as rpow +from cryptnox_id_cli.remote import relay +from cryptnox_id_cli.remote.channel import FrameChannel +from cryptnox_id_cli.secrets.redaction import Redactor +from cryptnox_id_cli.transport.errors import ( + RemoteConnectError, + RemoteError, + RemoteOperationError, + RemotePolicyError, + RemoteProtocolError, +) + +ATR = "3BFA1300FF910131FE000031C173C84000009000D2" +CPLC_CMD = "80CA9F7F00" +CPLC_RESP = ( + "9F7F2A4790D6004700000000004301012345670042000000000000000002507231323334350000000000000000" +) +SELECT_SSD = "00A4040009A0000001515350410100" +SELECT_PIV = "00A404000BA00000030800001000010000" +SELECT_WALLET = "00A4040007A000001000011200" +KEY_INFO = "80CA00E000" +KEY_INFO_RESP = "E012C00401018010C00402018010C00403018010" + + +class ScriptedChannel: + """A FrameChannel that plays server frames in order and records client frames. + + A script entry is either a frame dict (sent to the client as one message), + a string (sent raw, for framing tests), or an exception (raised on recv). + """ + + def __init__(self, script): + self.script = list(script) + self.sent: list[dict] = [] + self.closed = False + self.timeouts: list[float | None] = [] + + def send(self, message: str) -> None: + self.sent.append(json.loads(message)) + + def recv(self, timeout=None) -> str: + self.timeouts.append(timeout) + if not self.script: + raise RemoteConnectError("script exhausted: the service went silent") + item = self.script.pop(0) + if isinstance(item, BaseException): + raise item + if isinstance(item, str): + return item + return json.dumps(item) + + def close(self) -> None: + self.closed = True + + # Convenience for assertions. + def sent_types(self) -> list[str]: + return [f["type"] for f in self.sent] + + +def pow_challenge(bits: int = 8, challenge: str = "00112233445566778899aabbccddeeff") -> dict: + return {"type": "pow_challenge", "challenge": challenge, "bits": bits} + + +def card(mock_connection, exchanges=None, deny=None): + return mock_connection(ATR, exchanges or {}, deny or []) + + +def test_scripted_channel_satisfies_the_protocol(): + assert isinstance(ScriptedChannel([]), FrameChannel) + + +# --------------------------------------------------------------------------- # +# The happy path # +# --------------------------------------------------------------------------- # +def test_authenticate_end_to_end(mock_connection): + channel = ScriptedChannel( + [ + pow_challenge(bits=8), + {"type": "atr"}, + {"type": "log", "msg": "reading CPLC"}, + {"type": "apdu", "hex": CPLC_CMD}, + {"type": "result", "ok": True, "cplc_uid": "4301012345670042"}, + ] + ) + conn = card(mock_connection, {CPLC_CMD: f"{CPLC_RESP}|9000"}) + logs: list[str] = [] + lines: list[str] = [] + + report = relay.run_operation( + channel, conn, "authenticate", transcript=lines.append, on_log=logs.append + ) + + assert report.ok is True + assert report.result == {"ok": True, "cplc_uid": "4301012345670042"} + assert report.apdus_relayed == 1 + assert report.atr == bytes.fromhex(ATR) + assert report.pow_bits == 8 + assert logs == ["reading CPLC"] + + assert channel.sent_types() == ["hello", "pow", "atr_resp", "apdu_resp"] + assert channel.sent[0] == {"type": "hello", "op": "authenticate", "params": {}} + nonce = channel.sent[1]["nonce"] + assert isinstance(nonce, int) + assert ( + rpow.leading_zero_bits( + rpow.digest_for(bytes.fromhex("00112233445566778899aabbccddeeff"), nonce) + ) + >= 8 + ) + assert channel.sent[2] == {"type": "atr_resp", "atr": ATR} + assert channel.sent[3] == {"type": "apdu_resp", "data": CPLC_RESP, "sw1": 0x90, "sw2": 0x00} + + assert lines == ["> " + CPLC_CMD, "< " + CPLC_RESP + "9000"] + + +def test_hello_carries_params(mock_connection): + channel = ScriptedChannel([{"type": "result", "ok": True}]) + relay.run_operation(channel, card(mock_connection), "inspect", {"fused": False}) + assert channel.sent[0]["params"] == {"fused": False} + + +def test_a_failed_select_is_returned_verbatim(mock_connection): + channel = ScriptedChannel( + [ + {"type": "apdu", "hex": SELECT_SSD}, + {"type": "result", "ok": True, "piv_ssd": {"present": False}}, + ] + ) + report = relay.run_operation(channel, card(mock_connection), "inspect") + assert channel.sent[-1] == {"type": "apdu_resp", "data": "", "sw1": 0x6A, "sw2": 0x82} + assert report.apdus_relayed == 1 + + +def test_several_frames_in_one_message_are_all_handled(mock_connection): + two = ( + json.dumps({"type": "log", "msg": "a"}) + "\n" + json.dumps({"type": "result", "ok": True}) + ) + channel = ScriptedChannel([two]) + report = relay.run_operation(channel, card(mock_connection), "inspect") + assert report.log_lines == ["a"] + assert report.ok + + +def test_recv_uses_the_configured_timeout(mock_connection): + channel = ScriptedChannel([{"type": "result", "ok": True}]) + relay.run_operation(channel, card(mock_connection), "inspect", recv_timeout=42.0) + assert channel.timeouts == [42.0] + + +# --------------------------------------------------------------------------- # +# The relay is raw: no chaining, no retries, no commands of its own # +# --------------------------------------------------------------------------- # +def test_61xx_goes_back_to_the_service_untouched(mock_connection): + """The service owns GET RESPONSE. The client must never issue one itself.""" + channel = ScriptedChannel( + [ + {"type": "apdu", "hex": CPLC_CMD}, + {"type": "result", "ok": True}, + ] + ) + sent_to_card: list[str] = [] + conn = card(mock_connection, {CPLC_CMD: "AABB|6110"}) + original = conn.transmit + + def spy(apdu): + sent_to_card.append(bytes(apdu).hex().upper()) + return original(apdu) + + conn.transmit = spy + relay.run_operation(channel, conn, "inspect") + assert sent_to_card == [CPLC_CMD] # no 00C0 GET RESPONSE from the client + assert channel.sent[-1] == {"type": "apdu_resp", "data": "AABB", "sw1": 0x61, "sw2": 0x10} + + +def test_6cxx_goes_back_to_the_service_untouched(mock_connection): + channel = ScriptedChannel([{"type": "apdu", "hex": CPLC_CMD}, {"type": "result", "ok": True}]) + conn = card(mock_connection, {CPLC_CMD: "|6C2A"}) + relay.run_operation(channel, conn, "inspect") + assert channel.sent[-1] == {"type": "apdu_resp", "data": "", "sw1": 0x6C, "sw2": 0x2A} + + +# --------------------------------------------------------------------------- # +# Policy is enforced before anything reaches the card # +# --------------------------------------------------------------------------- # +def test_a_refused_command_never_reaches_the_card_and_ends_the_operation(mock_connection): + channel = ScriptedChannel( + [ + {"type": "apdu", "hex": SELECT_WALLET}, + {"type": "result", "ok": True}, # never reached + ] + ) + conn = card(mock_connection, {SELECT_WALLET: "|9000"}) + touched: list[str] = [] + original = conn.transmit + conn.transmit = lambda apdu: (touched.append(bytes(apdu).hex()), original(apdu))[1] + + with pytest.raises(RemotePolicyError, match="SELECT of AID A0000010000112"): + relay.run_operation(channel, conn, "inspect") + assert touched == [] + assert channel.sent_types() == ["hello"] # no apdu_resp for the refused command + + +def test_policy_follows_the_card_not_the_service(mock_connection): + # The service selects the PIV applet, the card says 6A82; a PIV read that + # follows must be judged in the card-manager context, and refused there. + channel = ScriptedChannel( + [ + {"type": "apdu", "hex": SELECT_PIV}, + {"type": "apdu", "hex": "00CB3FFF055C035FC10200"}, + ] + ) + with pytest.raises(RemotePolicyError, match="while the isd is selected"): + relay.run_operation(channel, card(mock_connection), "inspect") + + +def test_wall_clock_budget_is_enforced(mock_connection): + # started, elapsed for the first command, elapsed for the second. + ticks = iter([0.0, 0.0, 1000.0]) + channel = ScriptedChannel( + [{"type": "apdu", "hex": CPLC_CMD}, {"type": "apdu", "hex": CPLC_CMD}] + ) + conn = card(mock_connection, {CPLC_CMD: f"{CPLC_RESP}|9000"}) + with pytest.raises(RemoteError, match="exceeded 120 seconds"): + relay.run_operation(channel, conn, "inspect", clock=lambda: next(ticks)) + + +# --------------------------------------------------------------------------- # +# Failures # +# --------------------------------------------------------------------------- # +def test_result_not_ok_raises_with_the_payload(mock_connection): + channel = ScriptedChannel([{"type": "result", "ok": False, "error": "rate limit exceeded"}]) + with pytest.raises(RemoteOperationError, match="rate limit exceeded") as info: + relay.run_operation(channel, card(mock_connection), "inspect") + assert info.value.result == {"ok": False, "error": "rate limit exceeded"} + assert info.value.to_dict()["result"]["error"] == "rate limit exceeded" + + +def test_result_without_ok_is_a_failure(mock_connection): + channel = ScriptedChannel([{"type": "result"}]) + with pytest.raises(RemoteOperationError, match="reported inspect as failed"): + relay.run_operation(channel, card(mock_connection), "inspect") + + +def test_failure_text_from_the_service_is_sanitised(mock_connection): + channel = ScriptedChannel([{"type": "result", "ok": False, "error": "\x1b[31mnope\x1b[0m"}]) + with pytest.raises(RemoteOperationError) as info: + relay.run_operation(channel, card(mock_connection), "inspect") + assert "\x1b" not in str(info.value) + + +def test_second_pow_challenge_is_refused(mock_connection): + channel = ScriptedChannel([pow_challenge(4), pow_challenge(4)]) + with pytest.raises(RemoteProtocolError, match="second proof-of-work"): + relay.run_operation(channel, card(mock_connection), "inspect") + + +def test_card_error_is_reported_to_the_service_then_aborts(mock_connection): + channel = ScriptedChannel([{"type": "apdu", "hex": "00A4040000"}]) + conn = card(mock_connection, deny=["00A4040000"]) + with pytest.raises(RemoteError, match="card stopped answering"): + relay.run_operation(channel, conn, "inspect") + assert channel.sent[-1]["type"] == "apdu_resp" + assert "error" in channel.sent[-1] + assert "data" not in channel.sent[-1] + + +def test_connection_loss_propagates(mock_connection): + channel = ScriptedChannel([RemoteConnectError("closed")]) + with pytest.raises(RemoteConnectError, match="closed"): + relay.run_operation(channel, card(mock_connection), "inspect") + + +def test_malformed_frame_ends_the_operation(mock_connection): + channel = ScriptedChannel(["not json at all"]) + with pytest.raises(RemoteProtocolError, match="malformed"): + relay.run_operation(channel, card(mock_connection), "inspect") + + +def test_unknown_frame_type_ends_the_operation(mock_connection): + channel = ScriptedChannel([{"type": "surprise"}]) + with pytest.raises(RemoteProtocolError, match="unsupported frame type"): + relay.run_operation(channel, card(mock_connection), "inspect") + + +# --------------------------------------------------------------------------- # +# Transcript masking # +# --------------------------------------------------------------------------- # +def test_known_reads_are_shown_and_everything_else_is_masked(): + r = Redactor() + assert relay.redact_relayed_command(bytes.fromhex(CPLC_CMD), r) == CPLC_CMD + assert relay.redact_relayed_command(bytes.fromhex(SELECT_PIV), r) == SELECT_PIV + put_data = bytes.fromhex("00DB3FFF0A5C035FC1025303AABBCC") + assert relay.redact_relayed_command(put_data, r) == "00DB3FFF" + assert relay.redact_relayed_command(bytes.fromhex("0047009C"), r) == "0047009C" + + +def test_unmasked_transcript_falls_back_to_the_standard_redactor(): + r = Redactor() + put_data = bytes.fromhex("00DB3FFF0A5C035FC1025303AABBCC") + assert relay.redact_relayed_command(put_data, r, unmasked=True) == put_data.hex().upper() + # Even unmasked, the standard redactor keeps masking verifier data. + verify = bytes.fromhex("0020008008313233343536FFFF") + assert "313233343536" not in relay.redact_relayed_command(verify, r, unmasked=True) + + +def test_responses_are_masked_outside_known_reads(): + r = Redactor() + assert relay.redact_relayed_response(b"\xaa\xbb", 0x90, 0x00, 0xCA, r) == "AABB9000" + assert relay.redact_relayed_response(b"\xaa\xbb", 0x90, 0x00, 0x47, r) == "9000" + assert relay.redact_relayed_response(b"", 0x6A, 0x82, 0x47, r) == "6A82" + assert relay.redact_relayed_response(b"\xaa", 0x90, 0x00, 0x47, r, unmasked=True) == "AA9000" + + +def test_relayed_transcript_masks_by_default(mock_connection): + generate = "0047009C05AC03800111" + channel = ScriptedChannel([{"type": "apdu", "hex": generate}, {"type": "result", "ok": True}]) + # Give inspect a policy that lets the command through, so the masking alone is tested. + from cryptnox_id_cli.remote.policy import PIV, RelayPolicy + + policy = RelayPolicy("inspect") + policy.rules.allowed_ins[PIV] |= {0x47} # type: ignore[index] + policy.context = PIV + conn = card(mock_connection, {generate: "7F49AABB|9000"}) + lines: list[str] = [] + relay.run_operation(channel, conn, "inspect", policy=policy, transcript=lines.append) + assert lines == ["> 0047009C", "< 9000"] From 66ee0677f3db0f8b85ec556fe292fdd865c5c87e Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Tue, 22 Sep 2026 13:08:26 +0200 Subject: [PATCH 2/8] Add remote reset, dev-reset and attest through the Cryptnox PIV service The remaining three operations of the remote PIV service, on the relay and policy the read-only operations introduced. `remote reset` wipes and reinstalls the PIV function: the service authenticates to the card manager, deletes the PIV applet, its package and its security domain, loads the applet again, recreates the security domain and loads the card's keys. `remote dev-reset` does the same for development cards and leaves the card manager and the security domain on the public default key; its access credential comes from the environment only. Both sit behind the irreversibility gate the codebase already uses: a typed phrase interactively, an explicit flag otherwise, and no `--yes` bypass. What the card holds is read locally and shown before consent, and read again afterwards next to the service's result. The relay policy for these operations admits card content management through the card manager only, lets a DELETE whose data travels in the clear name nothing but the PIV instance, package and security domain, caps INITIALIZE UPDATE, EXTERNAL AUTHENTICATE and LOAD per operation, and stops at the first failed authentication so no further attempt can spend one of the card's bounded card-management retries. Inside an encrypted channel the policy sees instructions, not targets; the docs say so. `remote attest` generates a key in one slot through the service and receives its key-attestation certificate. The policy binds key generation to the requested slot and the management-key handshake to key 9B, so no slot key can be exercised. The certificate is then verified locally: chain to the pinned anchors, subject serial number against the card's CPLC UID, attested slot against the request, trust model, and the certified key against the certificate read back from the card. An unchecked binding never counts as a passed one, and the output states that on-card generation is the service's claim. `inspect` also learned that the service probes the applet's own admin channel with INITIALIZE UPDATE, and admits that probe. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 14 + README.md | 12 + .../0003-relay-for-the-remote-piv-service.md | 86 +++ docs/json-output.rst | 14 + docs/remote/remote-commands.rst | 67 +- docs/remote/remote-guide.rst | 47 ++ docs/troubleshooting.rst | 28 + src/cryptnox_id_cli/cli/commands/remote.py | 611 +++++++++++++++--- src/cryptnox_id_cli/cli/dryrun.py | 3 + src/cryptnox_id_cli/remote/attest_verify.py | 196 ++++++ src/cryptnox_id_cli/remote/policy.py | 182 +++++- tests/unit/test_remote_attest_command.py | 333 ++++++++++ tests/unit/test_remote_attest_verify.py | 226 +++++++ tests/unit/test_remote_gates.py | 251 +++++++ tests/unit/test_remote_policy.py | 182 +++++- 15 files changed, 2139 insertions(+), 113 deletions(-) create mode 100644 docs/adr/0003-relay-for-the-remote-piv-service.md create mode 100644 src/cryptnox_id_cli/remote/attest_verify.py create mode 100644 tests/unit/test_remote_attest_command.py create mode 100644 tests/unit/test_remote_attest_verify.py create mode 100644 tests/unit/test_remote_gates.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f1e1dbf..58e3d22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 that keeps every other card function out of reach, and reports what the service asserted apart from what it verified locally. Adds the `websockets` dependency and exit codes 12 to 16. +- `remote reset` and `remote dev-reset`: wipe and reinstall the PIV function + through the service, behind the same irreversibility gate as `factory piv + preperso finalize` (typed phrase or explicit flag, `--yes` not accepted), + with the card's PIV state, data objects, security-domain key versions and + neighbouring functions read locally before consent and again afterwards. + The relay policy for these operations admits card content management + through the card manager only, restricts a readable DELETE to the PIV + instance, package and security domain, and stops at the first failed + authentication. +- `remote attest`: generate a key in a slot through the service and receive + its key-attestation certificate. The certificate is verified locally against + the pinned anchors and bound to the card's CPLC UID, the requested slot, the + trust model and the certificate read back from the card before success is + reported. ## [1.0.3] - 2026-08-31 diff --git a/README.md b/README.md index 9c4db9b..2a72082 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,18 @@ cryptnox-id doctor # PC/SC service, reader, per-function reachability cryptnox-id shell # run subcommands without re-typing the prefix ``` +### 6. Use the Cryptnox remote PIV service + +```bash +cryptnox-id remote inspect # what the service sees: applet, security domain, key versions +cryptnox-id remote attest --slot 9C # a key generated through the service, with its attestation +cryptnox-id remote reset # wipe and reinstall the PIV function (irreversible, gated) +``` + +The card stays in your reader; the tool relays the service's commands under a +policy that keeps the other card functions out of reach, and shows what the +service asserted apart from what it verified locally. + --- ## Documentation diff --git a/docs/adr/0003-relay-for-the-remote-piv-service.md b/docs/adr/0003-relay-for-the-remote-piv-service.md new file mode 100644 index 0000000..a3e6b7e --- /dev/null +++ b/docs/adr/0003-relay-for-the-remote-piv-service.md @@ -0,0 +1,86 @@ +# 3. A policed relay for the remote PIV service + +## Context + +Some PIV lifecycle operations need keys that never leave Cryptnox: the key that +makes a card genuine, and the certificate authority that attests keys generated +on it. Cryptnox operates a service that performs those operations with the card +in the holder's own reader. The service chooses every card command; the tool's +part is to carry commands to the card and answers back over a TLS WebSocket. + +The card carries more than the PIV function. A wallet, FIDO2 passkeys, a +genuineness key and DESFire share the plastic, and each has retry counters or +content that a stray command can destroy. A card-management authentication that +fails spends one of a bounded number of retries, after which the card manager +locks for good. The service is one party; anything that can speak its protocol +to the tool is another. + +The tool had no network code, and every command it sent to a card was one it had +composed itself. + +## Options + +1. **Present the service as a card connection**, so the existing PIV code runs + over the wire. Backwards: the card is local and the service drives, so the + tool would be relaying the service's commands whatever the seam is called, + and the CLI's transport reassembles chained responses the service expects to + drive itself. +2. **A transparent relay**: carry every command the service sends. Simplest, and + it hands the other card functions to whoever holds the connection. +3. **A relay behind a fail-closed policy**, per operation, over what a command + header reveals. + +## Decision + +Option 3. + +The relay transmits each command the service sends through the raw card +connection and returns the answer verbatim, status word included. It never sends +a command of its own and never reassembles a chained response; the service sees +the card as a local reader would. + +Before a command reaches the card, a policy decides. The policy is an allow-list +per operation: which applet may be selected (the PIV applet and its security +domain; the card manager only where card content management is the point), +which instructions may pass in each of those contexts, how many INITIALIZE +UPDATE and EXTERNAL AUTHENTICATE commands each security domain may see, how many +commands and seconds an operation may take. A DELETE whose data travels in the +clear may name the PIV instance, its package or its security domain and nothing +else. Key generation is bound to the slot the holder asked for, and the +management-key handshake to key 9B, so no slot key can be exercised. Card-holder +verifier commands are never relayed. A refusal ends the operation and names the +command header, so the rules can be reviewed against what the service actually +needs rather than loosened in advance. + +Inside an encrypted channel only the class and instruction bytes stay in the +clear. The policy fences which security domain the channel was opened to and +which instructions pass through it; it cannot see what an encrypted command +carries, and the documentation says so. + +The service's result is shown as the service's assertion. What the tool can +verify itself, it reads from the card before and after: identity, personalization +state, security-domain key versions, the neighbouring functions, and for key +attestation the returned certificate bound to the card, the slot and the +certificate stored on it. An unchecked binding never counts as a passed one. + +Irreversible operations sit behind the gate the codebase already uses for +`finalize` and `fido reset`: a typed phrase interactively, an explicit flag +otherwise, and no `--yes` bypass. The development access credential comes from +the environment, never from an option. The production endpoint is a constant; +an override reaches the read-only operations only. + +## Consequences + +* `websockets` is the tool's first network dependency. The synchronous client + keeps every command synchronous, as the rest of the tool is. +* A service change that needs a command the policy does not admit fails loudly + with the header, and needs a tool release to admit it. That is the intended + trade: the policy is written from what the service was observed to send, not + from what it might. +* Relayed commands are written to the transcript with their data masked unless + the instruction is a known read, the reverse of the tool's own transcript rule, + because the tool did not compose the command. `remote --full-transcript` + lifts that masking for troubleshooting. +* The tool cannot witness that a key was generated on the card. The output says + so next to every key attestation. +* The glossary gains the remote service, the relay and the relay policy. diff --git a/docs/json-output.rst b/docs/json-output.rst index 7415a0b..832fc1d 100644 --- a/docs/json-output.rst +++ b/docs/json-output.rst @@ -159,3 +159,17 @@ re-present the card, and check the reader passes native DESFire APDUs. A section may also report ``{"state": "Unknown", "error": ""}`` when a probe fails. Nothing in any report contains a PIN, key, or private material — ``safe_to_share`` is always ``true``. + +Remote service operations +------------------------- + +``remote`` commands return one object with a fixed top level: ``op``, +``endpoint``, ``card`` (reader, ATR, CPLC UID and serial, all read locally), +``result`` (the service's own result object, passed through unchanged), +``verified_locally`` (each cross-check as ``true``, ``false`` or ``null`` when +the service reported nothing to compare) and ``relay`` (commands relayed, +seconds, proof-of-work bits, the service's log lines). ``reset`` and +``dev-reset`` add ``before`` and ``after`` inventories; ``attest`` adds +``verification`` with every binding checked on the returned certificate. +Fields under ``result`` are the service's assertions; only ``verified_locally``, +``before``, ``after`` and ``verification`` are the tool's own findings. diff --git a/docs/remote/remote-commands.rst b/docs/remote/remote-commands.rst index 706eb85..05cc411 100644 --- a/docs/remote/remote-commands.rst +++ b/docs/remote/remote-commands.rst @@ -42,10 +42,75 @@ Both commands report the card as read locally (reader, ATR, CPLC UID), the service's result as the service asserted it, and a cross-check of the fields the two have in common. +Key attestation +--------------- + +.. code-block:: text + + remote attest [--slot 9C] [--algorithm ECCP256] [--ca-obj HEX] [--out FILE] + + generate a key in the slot through the service and + receive its key-attestation certificate; the service + also stores the certificate in the card's attestation + container. The slot's previous key and the container's + previous certificate are replaced: when either holds + something, the command asks, or needs --yes. + + Options: + --slot HEX 9A, 9C, 9D, 9E or a retired slot 82 to 95 (default 9C) + --algorithm NAME ECCP256 (default), ECCP384, RSA2048, RSA3072, RSA4096 + --ca-obj HEX signing sub-CA in the service; omit for the + key-attestation CA + --out FILE write the returned certificate (PEM) + +The returned certificate is verified locally before success is reported: the +chain must end at a pinned trust anchor, the subject serial number must be the +card's CPLC UID, the ``cryptnoxPivSlot`` extension must name the requested +slot, the trust model must be the server-driven value, and the certified +public key must equal the certificate read back from the card's attestation +container. That the key was generated on the card is the service's claim; the +tool cannot witness it and says so. + +Reset +----- + +.. code-block:: text + + remote reset [--fused] [--i-understand-this-is-irreversible] + + IRREVERSIBLY wipe and reinstall the PIV function: the + service deletes the PIV applet, its package and its + security domain, reinstalls the applet, recreates the + security domain and loads the card's keys. Every PIV + key and certificate is destroyed. Interactively the + phrase RESET-PIV must be typed; non-interactively the + flag is required. --yes is not accepted. + + remote dev-reset [--fused] [--i-understand-this-is-irreversible] + + Development cards only. As reset, but the card manager + and the PIV security domain are left on the public + default key, so the PIV function can be worked on + locally. Needs the development access credential in + $CRYPTNOX_REMOTE_DEV_TOKEN (never an option). The + phrase is DEV-RESET-PIV. + + Options: + --fused the card is a fused production card: the service + derives its per-card card-manager key. On a card that + is not fused the derived key is wrong and the failed + authentication costs a card-management retry. + +Before asking for consent, both commands read and show what the card holds: +the PIV personalization state and data objects, the PIV security domain and +its key versions, and the state of the FIDO2 and genuineness functions, which +stay outside the operation. The same inventory is read again afterwards and +shown next to the service's result. + Endpoint -------- The service endpoint is fixed to ``wss://piv.cryptnox.com``. For development against a local stand-in, ``CRYPTNOX_REMOTE_URL`` redirects the identification and inspection commands only; it must be a ``wss://`` URL, or ``ws://`` to -loopback. +loopback. ``reset`` and ``dev-reset`` refuse to run while it is set. diff --git a/docs/remote/remote-guide.rst b/docs/remote/remote-guide.rst index a1a107a..056e33f 100644 --- a/docs/remote/remote-guide.rst +++ b/docs/remote/remote-guide.rst @@ -50,6 +50,53 @@ Operations key versions they carry. Key-free: an authentication attempt from the service is refused before it reaches the card. +``attest`` + Generates a key in one slot and returns a key-attestation certificate for + it, signed by the Cryptnox key-attestation CA and stored in the card's + attestation container as well. The relay policy admits the applet's admin + channel for the key-object setup, the management-key handshake (never a + slot key), key generation for the requested slot only, and the certificate + write. The tool then verifies the certificate against the pinned anchors and + binds it to the card, the slot and the certificate stored on the card. The + generation itself happening on the card is the service's claim; nothing the + relay sees authenticates it. + +``reset`` + Wipes and reinstalls the PIV function. The service authenticates to the + card manager, deletes the PIV applet, its package and its security domain, + loads the applet again, recreates the security domain and loads the card's + keys, then verifies the result. Every PIV key and certificate is destroyed; + the keys were never exportable, so nothing can bring them back. Interactive + use requires typing ``RESET-PIV``; non-interactive use requires + ``--i-understand-this-is-irreversible``. ``--yes`` is not accepted. + + The relay policy for this operation admits card content management through + the card manager only, lets a DELETE name nothing but the PIV instance, its + package and its security domain where the command travels in the clear, + caps authentication attempts per security domain, and stops at the first + failed authentication so no further attempt can spend a card-management + retry. Inside an encrypted channel the policy sees instructions, not + targets; that limit is real and is why the FIDO2 and genuineness state is + read before and after. + +``dev-reset`` + As ``reset``, for development cards, leaving the card manager and the PIV + security domain on the publicly documented GlobalPlatform key so the PIV + function can be pre-personalized and personalized locally. Anyone with a + reader can then install or delete applets on the card, and the card is not + genuine until it is reset through the service again. The development access + credential comes from ``CRYPTNOX_REMOTE_DEV_TOKEN`` only. + +The ``--fused`` flag +-------------------- + +A fused production card has its card manager locked to a per-card key. The +service derives that key when ``--fused`` is passed and uses the shared +default otherwise. The tool cannot tell the two apart without a key, so the +flag is the holder's statement. A wrong statement makes the service present +the wrong key; the failed authentication costs one of the card's bounded +card-management retries, and the relay stops there. + What is sent to the service --------------------------- diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index ad913e3..a8b7d5f 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -117,6 +117,34 @@ throwaway dev root: ``genuine verify --anchors ``. chain. It does **not** verify the per-card ISD or PIV-SSD keys (KDF/HSM-derived) — this tool has no access to those secrets, and the output says so. +Remote service +-------------- + +``remote`` commands exit with a code that names the failure class (see +:doc:`/exit-codes`): + +- **13** ``remote_connect``: the service could not be reached or the TLS + handshake failed. Check the network; the endpoint is fixed to + ``wss://piv.cryptnox.com``. With ``CRYPTNOX_REMOTE_URL`` set, only + ``authenticate`` and ``inspect`` run, and only to ``wss://`` or loopback. +- **14** ``remote_protocol``: the service sent a frame the tool cannot + interpret. Update the tool; if the newest version fails the same way, + report it with the message. +- **15** ``remote_policy``: the service asked for a card command the relay + policy refuses; the message names the command header. Nothing was sent to + the card for that command and the operation stopped. This is the tool + protecting the card; report the header so the policy can be reviewed. +- **16** ``remote_failed``: the exchange completed and the service reported + the operation as failed; the error object carries the service's result. + A per-card rate limit exists; wait before retrying. + +``remote reset`` and ``remote dev-reset`` refuse to run non-interactively +without ``--i-understand-this-is-irreversible``, and ``--yes`` is not +accepted for them. ``remote attest`` refuses non-interactively without +``--yes`` when the slot or the attestation container already holds a +certificate. + + yubico-piv-tool interop ------------------------- diff --git a/src/cryptnox_id_cli/cli/commands/remote.py b/src/cryptnox_id_cli/cli/commands/remote.py index 55315fe..057d7c9 100644 --- a/src/cryptnox_id_cli/cli/commands/remote.py +++ b/src/cryptnox_id_cli/cli/commands/remote.py @@ -11,22 +11,41 @@ import json import os -from collections.abc import Callable +import sys +from dataclasses import dataclass +from pathlib import Path import click from rich.console import Console from rich.markup import escape +from cryptnox_id_cli.applets.piv.piv import PivApplet from cryptnox_id_cli.cli.commands.info import _read_cplc +from cryptnox_id_cli.cli.commands.piv import SLOT_CERT_OBJECT, _read_cert_der from cryptnox_id_cli.cli.context import AppContext +from cryptnox_id_cli.remote import attest_verify as av from cryptnox_id_cli.remote import channel as rchannel from cryptnox_id_cli.remote import relay -from cryptnox_id_cli.remote.policy import RelayPolicy -from cryptnox_id_cli.transport.errors import RemoteConnectError -from cryptnox_id_cli.transport.pcsc import connect, pick_reader +from cryptnox_id_cli.remote.policy import PIV_SSD_AID, RelayPolicy +from cryptnox_id_cli.secrets.resolver import resolve_secret +from cryptnox_id_cli.state.detector import StateDetector +from cryptnox_id_cli.transport.apdu import APDU +from cryptnox_id_cli.transport.errors import CryptnoxError, RemoteConnectError +from cryptnox_id_cli.transport.pcsc import CardSession, PCSCConnection, connect, pick_reader +from cryptnox_id_cli.util import tlv _FULL_TRANSCRIPT_KEY = "remote.full_transcript" +#: Environment variable carrying the development access credential for +#: ``dev-reset``. Never an option: it would land in shell history. +DEV_ACCESS_ENV = "CRYPTNOX_REMOTE_DEV_TOKEN" + +#: Phrases typed to confirm the irreversible operations interactively. +RESET_PHRASE = "RESET-PIV" +DEV_RESET_PHRASE = "DEV-RESET-PIV" + +_READ_ONLY = ("authenticate", "inspect") + @click.group("remote") @click.option( @@ -49,71 +68,116 @@ def command(ctx: click.Context, full_transcript: bool) -> None: ctx.meta[_FULL_TRANSCRIPT_KEY] = full_transcript +# --------------------------------------------------------------------------- # +# Shared plumbing # +# --------------------------------------------------------------------------- # def _endpoint(op: str) -> tuple[str, bool]: """The service URL for this run, and whether it is the production endpoint.""" override = os.environ.get(rchannel.URL_ENV) if not override: return rchannel.PRODUCTION_URL, True url = rchannel.validate_url(override, allow_insecure_loopback=True) - if op not in ("authenticate", "inspect"): + if op not in _READ_ONLY: raise RemoteConnectError( f"${rchannel.URL_ENV} is set; {op} runs only against the production service" ) return url, False -def _run(app: AppContext, ctx: click.Context, op: str, params: dict[str, object]) -> None: - """Open the card, run one operation through the service, render the outcome.""" - url, production = _endpoint(op) +@dataclass +class _Card: + reader: str + conn: PCSCConnection + session: CardSession + atr: str + cplc_uid: str | None + ic_serial: str | None + + def to_dict(self) -> dict[str, object]: + return { + "reader": self.reader, + "atr": self.atr, + "cplc_uid": self.cplc_uid, + "ic_serial": self.ic_serial, + } + + +def _open_card(app: AppContext) -> _Card: + """Open the reader and read the card's identity. Leaves the card manager selected.""" reader = pick_reader(app.reader) app.resolved_reader = reader conn = connect(reader) - try: - session = app.make_session(conn, reader_name=reader) - atr = session.atr - cplc = _read_cplc(session) # leaves the card manager selected, as the policy assumes - card: dict[str, object] = { - "reader": reader, - "atr": atr.hex().upper(), - "cplc_uid": cplc["uid"] if cplc else None, - "ic_serial": cplc["ic_serial"] if cplc else None, - } + session = app.make_session(conn, reader_name=reader) + cplc = _read_cplc(session) + return _Card( + reader=reader, + conn=conn, + session=session, + atr=session.atr.hex().upper(), + cplc_uid=cplc["uid"] if cplc else None, + ic_serial=cplc["ic_serial"] if cplc else None, + ) - if not production: - app.out.warn(f"connecting to a non-production endpoint: {url}") - app.out.note(f"remote {op} via {url}") - with rchannel.open_channel(url) as ch: - report = relay.run_operation( - ch, - conn, - op, - params, - policy=RelayPolicy(op), - redactor=app.redactor, - transcript=app.apdu_trace, - unmasked_transcript=bool(ctx.meta.get(_FULL_TRANSCRIPT_KEY)), - on_log=lambda msg: app.out.note(f"service: {escape(msg)}"), - ) - finally: - conn.disconnect() +def _ssd_key_versions(session: CardSession) -> list[int] | None: + """Key versions the PIV security domain holds, read key-free. - checks = _cross_check(card, report.result) - relay_info: dict[str, object] = { - "apdus_relayed": report.apdus_relayed, - "seconds": round(report.seconds, 3), - "pow_bits": report.pow_bits, - "service_log": report.log_lines, - } - payload: dict[str, object] = { - "op": op, - "endpoint": url, - "card": card, - "result": report.result, - "verified_locally": checks, - "relay": relay_info, + ``None`` when the security domain is absent. The key information template + lists one ``C0`` entry per key: id, version, type, length. + """ + selected = session.transmit(APDU(0x00, 0xA4, 0x04, 0x00, data=PIV_SSD_AID, le=256)) + if not selected.ok: + return None + info = session.transmit(APDU(0x80, 0xCA, 0x00, 0xE0, le=256)) + if not info.ok: + return [] + versions: set[int] = set() + for node in tlv.parse(info.data): + if node.tag != 0xE0: + continue + for entry in tlv.parse(node.value, recurse=False): + if entry.tag == 0xC0 and len(entry.value) >= 2: + versions.add(entry.value[1]) + return sorted(versions) + + +def _inventory(card: _Card) -> dict[str, object]: + """What the PIV function and its neighbours hold right now, read locally. + + Ends by re-reading the CPLC so the card manager is the selected applet + again, which is what the relay policy assumes at the start of an operation. + """ + state = StateDetector( + card.session, probe_fido=True, probe_desfire=False, probe_genuine=True + ).detect() + ssd = _ssd_key_versions(card.session) + _read_cplc(card.session) + return { + "piv_state": state.piv.label, + "piv_objects": sorted(name for name, present in state.piv_objects.items() if present), + "piv_ssd_key_versions": ssd, + "fido_state": state.fido.label, + "genuineness_state": state.genuine.label, } - app.out.result(payload, _human(op, url, card, report.result, checks, relay_info)) + + +def _relay( + app: AppContext, ctx: click.Context, card: _Card, op: str, params: dict[str, object], url: str +) -> relay.RelayReport: + slot = params.get("slot") + policy = RelayPolicy(op, slot=int(str(slot), 16) if isinstance(slot, str) else None) + with rchannel.open_channel(url) as ch: + return relay.run_operation( + ch, + card.conn, + op, + params, + policy=policy, + redactor=app.redactor, + transcript=app.apdu_trace, + unmasked_transcript=bool(ctx.meta.get(_FULL_TRANSCRIPT_KEY)), + on_log=lambda msg: app.out.note(f"service: {escape(msg)}"), + ) def _cross_check(card: dict[str, object], result: dict[str, object]) -> dict[str, object]: @@ -141,40 +205,13 @@ def pick(*keys: str) -> str | None: return checks -def _human( - op: str, - url: str, - card: dict[str, object], - result: dict[str, object], - checks: dict[str, object], - relay_info: dict[str, object], -) -> Callable[[Console], None]: - def render(c: Console) -> None: - c.print(f"[bold]remote {op}[/bold] {escape(url)}") - c.print(f" Reader: {escape(str(card['reader']))}") - c.print(f" ATR: {card['atr']}") - if card.get("cplc_uid"): - c.print(f" UID: {card['cplc_uid']} (CPLC, read locally)") - c.print( - f" Relay: {relay_info['apdus_relayed']} command(s) in " - f"{relay_info['seconds']}s, proof-of-work {relay_info['pow_bits']} bits" - ) - - c.print("\n[bold]Service result[/bold] (asserted by the service)") - for key, value in result.items(): - c.print(f" {escape(str(key))}: {escape(_fmt(value))}") - - c.print("\n[bold]Verified locally[/bold]") - for key, value in checks.items(): - if value is True: - label = "[green]match[/green]" - elif value is False: - label = "[red]MISMATCH[/red]" - else: - label = "[dim]not reported by the service[/dim]" - c.print(f" {key}: {label}") - - return render +def _relay_info(report: relay.RelayReport) -> dict[str, object]: + return { + "apdus_relayed": report.apdus_relayed, + "seconds": round(report.seconds, 3), + "pow_bits": report.pow_bits, + "service_log": report.log_lines, + } def _fmt(value: object) -> str: @@ -185,6 +222,98 @@ def _fmt(value: object) -> str: return str(value) +def _print_result(c: Console, result: dict[str, object]) -> None: + c.print("\n[bold]Service result[/bold] (asserted by the service)") + for key, value in result.items(): + if key == "steps" and isinstance(value, list): + c.print(" steps:") + for step in value: + c.print(f" {escape(_fmt(step))}") + continue + c.print(f" {escape(str(key))}: {escape(_fmt(value))}") + + +def _print_checks(c: Console, checks: dict[str, object]) -> None: + c.print("\n[bold]Verified locally[/bold]") + for key, value in checks.items(): + if value is True: + label = "[green]match[/green]" + elif value is False: + label = "[red]MISMATCH[/red]" + else: + label = "[dim]not reported by the service[/dim]" + c.print(f" {key}: {label}") + + +def _print_header( + c: Console, op: str, url: str, card: dict[str, object], relay_info: dict[str, object] +) -> None: + c.print(f"[bold]remote {op}[/bold] {escape(url)}") + c.print(f" Reader: {escape(str(card['reader']))}") + c.print(f" ATR: {card['atr']}") + if card.get("cplc_uid"): + c.print(f" UID: {card['cplc_uid']} (CPLC, read locally)") + c.print( + f" Relay: {relay_info['apdus_relayed']} command(s) in " + f"{relay_info['seconds']}s, proof-of-work {relay_info['pow_bits']} bits" + ) + + +def _print_inventory(c: Console, title: str, inv: dict[str, object]) -> None: + ssd = inv.get("piv_ssd_key_versions") + if not isinstance(ssd, list): + ssd_text = "absent" + elif not ssd: + ssd_text = "present, key versions unreadable" + else: + ssd_text = "present, key version(s) " + ", ".join(str(v) for v in ssd) + objects = inv.get("piv_objects") + objects_text = ( + ", ".join(str(o) for o in objects) if isinstance(objects, list) and objects else "none" + ) + c.print(f"[bold]{title}[/bold] (read locally)") + c.print(f" PIV: {inv['piv_state']}") + c.print(f" PIV data objects: {objects_text}") + c.print(f" PIV security domain: {ssd_text}") + c.print(f" FIDO2: {inv['fido_state']}") + c.print(f" Genuineness: {inv['genuineness_state']}") + + +# --------------------------------------------------------------------------- # +# Read-only operations # +# --------------------------------------------------------------------------- # +def _run_read_only(ctx: click.Context, op: str) -> None: + app: AppContext = ctx.obj + url, production = _endpoint(op) + card = _open_card(app) + try: + if not production: + app.out.warn(f"connecting to a non-production endpoint: {url}") + app.out.note(f"remote {op} via {url}") + report = _relay(app, ctx, card, op, {}, url) + finally: + card.conn.disconnect() + + card_info = card.to_dict() + checks = _cross_check(card_info, report.result) + relay_info = _relay_info(report) + payload: dict[str, object] = { + "op": op, + "endpoint": url, + "card": card_info, + "result": report.result, + "verified_locally": checks, + "relay": relay_info, + } + + def human(c: Console) -> None: + _print_header(c, op, url, card_info, relay_info) + _print_result(c, report.result) + _print_checks(c, checks) + + app.out.result(payload, human) + + @command.command("authenticate") @click.pass_context def authenticate(ctx: click.Context) -> None: @@ -192,7 +321,7 @@ def authenticate(ctx: click.Context) -> None: Key-free on both sides. The relay policy allows only card-manager reads. """ - _run(ctx.obj, ctx, "authenticate", {}) + _run_read_only(ctx, "authenticate") @command.command("inspect") @@ -204,4 +333,314 @@ def inspect(ctx: click.Context) -> None: which key versions they carry. Key-free: the relay policy refuses any authentication attempt. """ - _run(ctx.obj, ctx, "inspect", {}) + _run_read_only(ctx, "inspect") + + +# --------------------------------------------------------------------------- # +# Destructive operations # +# --------------------------------------------------------------------------- # +_FUSED_HELP = ( + "The card is a fused production card: the service derives its per-card " + "card-manager key. On a card that is not fused the derived key is wrong and the " + "failed authentication costs a card-management retry. Omit unless certain." +) + + +def _confirm_destructive( + app: AppContext, op: str, phrase: str, understood: bool, card: _Card, fused: bool +) -> dict[str, object]: + """Show what is about to be destroyed and obtain consent. Returns the inventory.""" + before = _inventory(card) + app.out.warn( + f"remote {op} wipes the PIV function on card {card.cplc_uid or card.atr}: the " + "applet, every key and certificate in it, and its security domain are deleted and " + "reinstalled. This cannot be undone." + ) + if op == "dev-reset": + app.out.warn( + "dev-reset leaves the card manager and the PIV security domain on the PUBLIC " + "default key: anyone with a reader can then install or delete applets, " + "including the wallet. The card is not genuine again until it is reset through " + "the service." + ) + if fused: + app.out.warn( + "--fused: the service will derive this card's per-card card-manager key. If " + "the card is not fused, that key is wrong and the failed authentication costs " + "a card-management retry." + ) + if not app.json: + _print_inventory(app.out.console, "Before", before) + if not understood: + if app.json or not sys.stdin.isatty(): + raise CryptnoxError( + f"remote {op} needs --i-understand-this-is-irreversible non-interactively." + ) + if click.prompt(f"Type {phrase} to continue") != phrase: + raise click.Abort() + return before + + +def _run_destructive( + ctx: click.Context, op: str, *, fused: bool, understood: bool, phrase: str +) -> None: + app: AppContext = ctx.obj + url, _production = _endpoint(op) # refuses any override for these operations + params: dict[str, object] = {"fused": fused} + if op == "dev-reset": + credential = resolve_secret( + redactor=app.redactor, + env_var=DEV_ACCESS_ENV, + prompt_label="Cryptnox remote development access", + ) + params["dev_token"] = credential.decode("utf-8") + + card = _open_card(app) + try: + before = _confirm_destructive(app, op, phrase, understood, card, fused) + app.out.note(f"remote {op} via {url}") + report = _relay(app, ctx, card, op, params, url) + after = _inventory(card) + finally: + card.conn.disconnect() + + card_info = card.to_dict() + checks = _cross_check(card_info, report.result) + checks["piv_present_after"] = after["piv_state"] not in ("PivNotPresent", "PivUnknown") + ssd_after = after.get("piv_ssd_key_versions") + checks["piv_ssd_present_after"] = ssd_after is not None + relay_info = _relay_info(report) + payload: dict[str, object] = { + "op": op, + "endpoint": url, + "fused": fused, + "card": card_info, + "before": before, + "result": report.result, + "after": after, + "verified_locally": checks, + "relay": relay_info, + } + + def human(c: Console) -> None: + _print_header(c, op, url, card_info, relay_info) + _print_result(c, report.result) + c.print() + _print_inventory(c, "After", after) + _print_checks(c, checks) + + app.out.result(payload, human) + + +@command.command("reset") +@click.option("--fused", is_flag=True, help=_FUSED_HELP) +@click.option( + "--i-understand-this-is-irreversible", + "understood", + is_flag=True, + help="Required for non-interactive use.", +) +@click.pass_context +def reset(ctx: click.Context, fused: bool, understood: bool) -> None: + """IRREVERSIBLY wipe and reinstall the PIV function through the service. + + The service deletes the PIV applet, its package and its security domain, + reinstalls the applet, recreates the security domain and loads the card's + keys, so the card is genuine and remotely manageable afterwards. Every PIV + key and certificate is destroyed. The other card functions are outside the + operation. + """ + _run_destructive(ctx, "reset", fused=fused, understood=understood, phrase=RESET_PHRASE) + + +@command.command("dev-reset") +@click.option("--fused", is_flag=True, help=_FUSED_HELP) +@click.option( + "--i-understand-this-is-irreversible", + "understood", + is_flag=True, + help="Required for non-interactive use.", +) +@click.pass_context +def dev_reset(ctx: click.Context, fused: bool, understood: bool) -> None: + """IRREVERSIBLY wipe the PIV function and leave the card on the public default keys. + + For development cards only. Needs the development access credential in + $CRYPTNOX_REMOTE_DEV_TOKEN. Afterwards the card manager and the PIV security + domain are on the publicly documented GlobalPlatform key, so the PIV function + can be pre-personalized and personalized locally; the card is not genuine + until reset through the service again. + """ + _run_destructive(ctx, "dev-reset", fused=fused, understood=understood, phrase=DEV_RESET_PHRASE) + + +# --------------------------------------------------------------------------- # +# Key attestation # +# --------------------------------------------------------------------------- # +ATTEST_ALGORITHMS = ("ECCP256", "ECCP384", "RSA2048", "RSA3072", "RSA4096") +ATTESTATION_CONTAINER = "attestation-cert" # 5FC120, the one container the service writes + + +def _parse_slot(value: str) -> int: + try: + slot = int(value, 16) + except ValueError: + raise click.BadParameter(f"not a hex slot: {value!r}", param_hint="--slot") from None + if slot not in (0x9A, 0x9C, 0x9D, 0x9E) and not 0x82 <= slot <= 0x95: + raise click.BadParameter( + "slot must be 9A, 9C, 9D, 9E or a retired slot 82 to 95", param_hint="--slot" + ) + return slot + + +def _read_certificates(card: _Card, slot: int) -> tuple[bytes | None, bytes | None]: + """The slot's certificate and the attestation container, read locally. + + Leaves the card manager selected afterwards. + """ + piv = PivApplet(card.session) + slot_cert = attestation = None + try: + piv.select() + slot_object = SLOT_CERT_OBJECT.get(slot) + slot_cert = _read_cert_der(piv, slot_object) if slot_object else None + attestation = _read_cert_der(piv, ATTESTATION_CONTAINER) + except CryptnoxError: + pass + _read_cplc(card.session) + return slot_cert, attestation + + +@command.command("attest") +@click.option("--slot", default="9C", show_default=True, help="Slot to generate the key in.") +@click.option( + "--algorithm", + default="ECCP256", + show_default=True, + type=click.Choice(ATTEST_ALGORITHMS, case_sensitive=False), + help="Key algorithm.", +) +@click.option( + "--ca-obj", + default=None, + help="Signing sub-CA object in the service (hex). Omit for the key-attestation CA.", +) +@click.option("--out", "out_", type=click.Path(dir_okay=False), help="Write the leaf PEM here.") +@click.pass_context +def attest( + ctx: click.Context, slot: str, algorithm: str, ca_obj: str | None, out_: str | None +) -> None: + """Generate a key on the card through the service and receive its attestation. + + The service generates the key in the slot, signs a key-attestation certificate + for it and stores that certificate in the card's attestation container. The + slot's previous key and the container's previous certificate are replaced. + Afterwards the certificate is verified locally: chain to the pinned anchors, + subject serial number against the card's CPLC UID, attested slot against the + request, trust model, and the certified key against the certificate read + back from the card. That the key was generated on the card is the service's + claim; the relay cannot witness it. + """ + app: AppContext = ctx.obj + op = "attest" + slot_ref = _parse_slot(slot) + slot_hex = f"{slot_ref:02X}" + url, _production = _endpoint(op) + params: dict[str, object] = {"slot": slot_hex, "algorithm": algorithm.upper()} + if ca_obj: + params["ca_obj"] = ca_obj + + card = _open_card(app) + try: + slot_cert, container_before = _read_certificates(card, slot_ref) + occupied = [] + if slot_cert is not None: + occupied.append(f"slot {slot_hex} holds a certificate") + if container_before is not None: + occupied.append("the attestation container holds a certificate") + if occupied: + app.out.warn( + "remote attest replaces the key in the slot and the certificate in the " + "attestation container: " + "; ".join(occupied) + "." + ) + if not app.yes: + if app.json or not sys.stdin.isatty(): + raise CryptnoxError( + "remote attest would overwrite existing material; pass --yes to confirm." + ) + if not click.confirm("Continue and overwrite?", default=False): + raise click.Abort() + + app.out.note(f"remote {op} via {url}") + report = _relay(app, ctx, card, op, params, url) + _slot_after, container_after = _read_certificates(card, slot_ref) + finally: + card.conn.disconnect() + + pem = report.result.get("attestation_cert_pem") + verification: av.AttestVerification | None = None + if isinstance(pem, str) and pem.strip(): + try: + leaf_der = av.leaf_from_pem(pem) + except ValueError as exc: + raise CryptnoxError(f"the service returned an unreadable certificate: {exc}") from exc + verification = av.verify_returned_attestation( + leaf_der, slot=slot_ref, cplc_uid=card.cplc_uid, on_card_der=container_after + ) + if out_: + Path(out_).write_bytes(pem.encode("utf-8")) + + card_info = card.to_dict() + checks = _cross_check(card_info, report.result) + checks["attestation_returned"] = verification is not None + checks["attestation_verified"] = verification.verified if verification else False + checks["attestation_container_written"] = container_after is not None + relay_info = _relay_info(report) + payload: dict[str, object] = { + "op": op, + "endpoint": url, + "slot": slot_hex, + "algorithm": algorithm.upper(), + "card": card_info, + "result": report.result, + "verification": verification.to_dict() if verification else None, + "verified_locally": checks, + "out": out_, + "relay": relay_info, + } + + def human(c: Console) -> None: + _print_header(c, op, url, card_info, relay_info) + _print_result(c, {k: v for k, v in report.result.items() if k != "attestation_cert_pem"}) + c.print("\n[bold]Key attestation[/bold] (checked locally)") + if verification is None: + c.print(" [red]the service returned no certificate[/red]") + else: + verdict = ( + "[green]verified[/green]" if verification.verified else "[red]NOT verified[/red]" + ) + c.print(f" {verdict} {escape(verification.subject)}") + c.print(f" chain: {' -> '.join(escape(n) for n in verification.chain)}") + for label, value in ( + ("serial number is this card", verification.serial_number_matches_card), + (f"attests slot {slot_hex}", verification.slot_matches), + ("trust model is server-driven", verification.trust_model_matches), + ("key equals the card's stored certificate", verification.public_key_matches_card), + ): + if value is True: + mark = "[green]yes[/green]" + elif value is False: + mark = "[red]NO[/red]" + else: + mark = "[dim]unchecked[/dim]" + c.print(f" {label}: {mark}") + for reason in verification.reasons: + c.print(f" [yellow]-[/yellow] {escape(reason)}") + if out_: + c.print(f" PEM written to {escape(out_)}") + c.print( + " [dim]On-card generation is the service's claim; the relay cannot witness it.[/dim]" + ) + _print_checks(c, checks) + + app.out.result(payload, human) diff --git a/src/cryptnox_id_cli/cli/dryrun.py b/src/cryptnox_id_cli/cli/dryrun.py index fccc3e4..953e9a1 100644 --- a/src/cryptnox_id_cli/cli/dryrun.py +++ b/src/cryptnox_id_cli/cli/dryrun.py @@ -146,8 +146,11 @@ "piv puk change", # Relayed operations: the service chooses the commands, so nothing can be # planned locally, and every run opens a network connection. + "remote attest", "remote authenticate", + "remote dev-reset", "remote inspect", + "remote reset", } ) diff --git a/src/cryptnox_id_cli/remote/attest_verify.py b/src/cryptnox_id_cli/remote/attest_verify.py new file mode 100644 index 0000000..926bf99 --- /dev/null +++ b/src/cryptnox_id_cli/remote/attest_verify.py @@ -0,0 +1,196 @@ +"""Local verification of a key-attestation certificate returned by the service. + +The service says it generated a key on the card and returns a certificate for +it. This module decides how much of that to believe, from evidence this tool +can check itself: + +* the chain terminates at a pinned trust anchor (never at anything that came + with the result); +* the subject's serial number is the card's CPLC UID, read locally; +* the ``cryptnoxPivSlot`` extension names the slot that was asked for; +* the ``cryptnoxAttestTrustModel`` extension carries the server-driven value; +* the certified public key equals the one stored in the card's attestation + container, read locally after the operation. + +What it cannot check is stated as plainly: the key's generation on the card is +the service's claim, witnessed by nothing the relay can authenticate. +""" + +from __future__ import annotations + +import datetime +import hashlib +from dataclasses import dataclass, field + +from cryptography import x509 +from cryptography.hazmat.primitives import serialization +from cryptography.x509.oid import NameOID + +from .. import trust +from ..crypto.attestation import verify_attestation_chain + +#: Cryptnox private enterprise arc and the two key-attestation extensions. +OID_PIV_SLOT = x509.ObjectIdentifier("1.3.6.1.4.1.55934.1.1.1") +OID_TRUST_MODEL = x509.ObjectIdentifier("1.3.6.1.4.1.55934.1.1.2") + +#: Trust model value for a key generated on the card under the service's direction. +TRUST_MODEL_SERVER_DRIVEN = 2 + + +@dataclass +class AttestVerification: + """Everything checked locally about a returned attestation certificate.""" + + chain_verified: bool + chain: list[str] = field(default_factory=list) + reasons: list[str] = field(default_factory=list) + subject: str = "" + issuer: str = "" + leaf_sha256: str = "" + serial_number: str | None = None + serial_number_matches_card: bool | None = None + slot: int | None = None + slot_matches: bool | None = None + trust_model: int | None = None + trust_model_matches: bool | None = None + public_key_matches_card: bool | None = None + + @property + def verified(self) -> bool: + """True only when the chain holds and every binding that could be checked holds.""" + bindings = ( + self.serial_number_matches_card, + self.slot_matches, + self.trust_model_matches, + self.public_key_matches_card, + ) + return self.chain_verified and all(b is True for b in bindings) + + def to_dict(self) -> dict[str, object]: + return { + "verified": self.verified, + "chain_verified": self.chain_verified, + "chain": self.chain, + "reasons": self.reasons, + "subject": self.subject, + "issuer": self.issuer, + "leaf_sha256": self.leaf_sha256, + "serial_number": self.serial_number, + "serial_number_matches_card": self.serial_number_matches_card, + "slot": f"{self.slot:02X}" if self.slot is not None else None, + "slot_matches": self.slot_matches, + "trust_model": self.trust_model, + "trust_model_matches": self.trust_model_matches, + "public_key_matches_card": self.public_key_matches_card, + } + + +def leaf_from_pem(pem: str | bytes) -> bytes: + """DER of the first certificate in a PEM blob.""" + raw = pem.encode("utf-8") if isinstance(pem, str) else bytes(pem) + certs = x509.load_pem_x509_certificates(raw) + if not certs: + raise ValueError("no certificate in the PEM data") + return certs[0].public_bytes(serialization.Encoding.DER) + + +def der_integer(value: bytes) -> int | None: + """Decode a DER-encoded INTEGER, or ``None`` if the bytes are not one.""" + if len(value) < 3 or value[0] != 0x02: + return None + length = value[1] + if length == 0 or length > 4 or len(value) != 2 + length: + return None + return int.from_bytes(value[2 : 2 + length], "big", signed=True) + + +def extension_integer(cert: x509.Certificate, oid: x509.ObjectIdentifier) -> int | None: + """The INTEGER value of a private extension, or ``None`` when absent or malformed.""" + try: + ext = cert.extensions.get_extension_for_oid(oid) + except x509.ExtensionNotFound: + return None + value = ext.value + if isinstance(value, x509.UnrecognizedExtension): + return der_integer(value.value) + return None + + +def _spki(der: bytes) -> bytes: + return ( + x509.load_der_x509_certificate(der) + .public_key() + .public_bytes(serialization.Encoding.DER, serialization.PublicFormat.SubjectPublicKeyInfo) + ) + + +def verify_returned_attestation( + leaf_der: bytes, + *, + slot: int, + cplc_uid: str | None, + on_card_der: bytes | None, + anchors: tuple[list[bytes], list[bytes]] | None = None, + at_time: datetime.datetime | None = None, +) -> AttestVerification: + """Check a returned attestation leaf against the pinned anchors and the card. + + ``anchors`` is ``(roots, intermediates)`` in DER; the bundled store is used + when omitted. ``on_card_der`` is the certificate read from the card's + attestation container after the operation, or ``None`` if it could not be + read; the public-key binding is then reported as unchecked. + """ + roots, intermediates = anchors if anchors is not None else trust.load_anchors() + chain = verify_attestation_chain(leaf_der, intermediates, roots, at_time=at_time) + cert = x509.load_der_x509_certificate(leaf_der) + out = AttestVerification( + chain_verified=chain.verified, + chain=chain.chain, + reasons=list(chain.reasons), + subject=cert.subject.rfc4514_string(), + issuer=cert.issuer.rfc4514_string(), + leaf_sha256=hashlib.sha256(leaf_der).hexdigest().upper(), + ) + + serials = cert.subject.get_attributes_for_oid(NameOID.SERIAL_NUMBER) + serial = str(serials[0].value) if serials else None + out.serial_number = serial + if serial is None: + out.serial_number_matches_card = False + out.reasons.append("certificate subject carries no serialNumber") + elif cplc_uid: + out.serial_number_matches_card = serial.strip().upper() == cplc_uid.strip().upper() + if not out.serial_number_matches_card: + out.reasons.append("certificate serialNumber is not this card's CPLC UID") + + out.slot = extension_integer(cert, OID_PIV_SLOT) + if out.slot is None: + out.slot_matches = False + out.reasons.append("certificate carries no cryptnoxPivSlot extension") + else: + out.slot_matches = out.slot == slot + if not out.slot_matches: + out.reasons.append( + f"certificate attests slot {out.slot:02X}, the operation asked for {slot:02X}" + ) + + out.trust_model = extension_integer(cert, OID_TRUST_MODEL) + if out.trust_model is None: + out.trust_model_matches = False + out.reasons.append("certificate carries no cryptnoxAttestTrustModel extension") + else: + out.trust_model_matches = out.trust_model == TRUST_MODEL_SERVER_DRIVEN + if not out.trust_model_matches: + out.reasons.append( + f"certificate trust model is {out.trust_model}, expected " + f"{TRUST_MODEL_SERVER_DRIVEN} (server-driven on-card generation)" + ) + + if on_card_der is not None: + out.public_key_matches_card = _spki(leaf_der) == _spki(on_card_der) + if not out.public_key_matches_card: + out.reasons.append( + "the certified public key differs from the certificate stored on the card" + ) + + return out diff --git a/src/cryptnox_id_cli/remote/policy.py b/src/cryptnox_id_cli/remote/policy.py index 784ebb0..30ce020 100644 --- a/src/cryptnox_id_cli/remote/policy.py +++ b/src/cryptnox_id_cli/remote/policy.py @@ -12,12 +12,14 @@ Where the service works inside a secure channel, only the class and instruction bytes stay in the clear. The policy is honest about that: it fences which security domain the channel was opened to and which instructions pass through -it, and it cannot see what an encrypted command carries. +it, and it cannot see what an encrypted command carries. Where a command's data +does travel in the clear, the policy reads it: a DELETE may only name the PIV +instance, the PIV package or the PIV security domain. Counters guard the other way a command can do harm without being forbidden: a failed GlobalPlatform authentication decrements a bounded retry counter, so the number of authentication attempts per security domain is capped, and a -refused authentication ends the operation on the spot. +failed authentication ends the operation on the spot. """ from __future__ import annotations @@ -32,6 +34,7 @@ ISD_AID = bytes.fromhex("A000000151000000") PIV_SSD_AID = bytes.fromhex("A00000015153504101") PIV_AID = bytes.fromhex("A000000308000010000100") +PIV_PACKAGE_AID = bytes.fromhex("A0000003084F46323031") #: Contexts a command can be evaluated in, keyed by the selected AID. ISD = "isd" @@ -40,8 +43,14 @@ _CONTEXT_BY_AID: dict[bytes, str] = {ISD_AID: ISD, PIV_SSD_AID: PIV_SSD, PIV_AID: PIV} -#: The operations this module has rules for. +#: Operations grouped by what they may do to the card. READ_ONLY_OPS = ("authenticate", "inspect") +DESTRUCTIVE_OPS = ("reset", "dev-reset") +KEY_OPS = ("attest",) + +#: The PIV management key reference: the only key GENERAL AUTHENTICATE may +#: address during a remote operation. +MANAGEMENT_KEY_REF = 0x9B # Instructions, by name, so the tables read as prose. INS_SELECT = 0xA4 @@ -85,13 +94,49 @@ class OpRules: max_seconds: float #: INITIALIZE UPDATE attempts allowed per security domain. max_initialize_updates: int - #: Whether EXTERNAL AUTHENTICATE may be relayed at all. - allow_external_authenticate: bool + #: EXTERNAL AUTHENTICATE attempts allowed per security domain (0 = never). + max_external_authenticates: int = 0 + #: AIDs a DELETE may name when its data is readable. ``None`` = no DELETE. + delete_targets: frozenset[bytes] | None = None + #: LOAD blocks allowed in one operation (0 = no LOAD). + max_load_blocks: int = 0 + #: Whether GENERATE and GENERAL AUTHENTICATE are bound to a slot and the + #: management key. Only meaningful for key operations. + slot_bound: bool = False _GP_READS: frozenset[int] = frozenset( {INS_SELECT, INS_GET_DATA_GP, INS_GET_STATUS, INS_GET_RESPONSE} ) +_GP_AUTH: frozenset[int] = frozenset({INS_INITIALIZE_UPDATE, INS_EXTERNAL_AUTHENTICATE}) +_GP_CONTENT: frozenset[int] = frozenset({INS_DELETE, INS_INSTALL, INS_LOAD}) +_GP_KEYS: frozenset[int] = frozenset({INS_PUT_KEY, INS_STORE_DATA}) +_PIV_READS: frozenset[int] = frozenset({INS_SELECT, INS_GET_DATA_PIV, INS_GET_RESPONSE}) + +#: What the service may delete: the PIV instance, its package, its security +#: domain. Never a package or instance of any other card function. +_PIV_DELETE_TARGETS: frozenset[bytes] = frozenset({PIV_AID, PIV_PACKAGE_AID, PIV_SSD_AID}) + +_RESET_RULES = OpRules( + selectable=frozenset({ISD_AID, PIV_SSD_AID, PIV_AID}), + allowed_ins={ + # Card content management happens through the card manager only. + ISD: _GP_READS | _GP_AUTH | _GP_CONTENT | _GP_KEYS, + # The PIV security domain is created, keyed and verified; it never + # loads or deletes anything itself. + PIV_SSD: _GP_READS | _GP_AUTH | _GP_KEYS, + # The fresh applet is checked and may receive its baseline structure + # through its own admin channel; no key use, no key generation. + PIV: _PIV_READS | _GP_AUTH | {INS_PUT_DATA}, + }, + # A CAP of ~190 KB loads in ~800 blocks; the rest is bookkeeping. + max_apdus=1500, + max_seconds=1800.0, + max_initialize_updates=8, + max_external_authenticates=3, + delete_targets=_PIV_DELETE_TARGETS, + max_load_blocks=1500, +) RULES: dict[str, OpRules] = { "authenticate": OpRules( @@ -104,14 +149,15 @@ class OpRules: max_apdus=20, max_seconds=60.0, max_initialize_updates=0, - allow_external_authenticate=False, ), "inspect": OpRules( selectable=frozenset({ISD_AID, PIV_SSD_AID, PIV_AID}), allowed_ins={ ISD: _GP_READS | {INS_INITIALIZE_UPDATE}, PIV_SSD: _GP_READS | {INS_INITIALIZE_UPDATE}, - PIV: frozenset({INS_SELECT, INS_GET_DATA_PIV, INS_GET_RESPONSE}), + # The applet hosts its own admin channel; the service probes its + # secure-channel version the same way, without authenticating. + PIV: _PIV_READS | {INS_INITIALIZE_UPDATE}, }, max_apdus=60, max_seconds=120.0, @@ -119,13 +165,31 @@ class OpRules: # different P1 values (observed: KVN 00, then KVN 01). None of them costs # a retry as long as EXTERNAL AUTHENTICATE stays refused. max_initialize_updates=4, - allow_external_authenticate=False, + ), + "reset": _RESET_RULES, + "dev-reset": _RESET_RULES, + "attest": OpRules( + selectable=frozenset({ISD_AID, PIV_SSD_AID, PIV_AID}), + allowed_ins={ + ISD: _GP_READS | {INS_INITIALIZE_UPDATE}, + PIV_SSD: _GP_READS | _GP_AUTH, + # The applet's admin channel for the key-object setup, then the + # management-key handshake, key generation and the certificate write. + PIV: _PIV_READS + | _GP_AUTH + | {INS_GENERAL_AUTHENTICATE, INS_GENERATE_ASYMMETRIC, INS_PUT_DATA}, + }, + max_apdus=200, + max_seconds=600.0, + max_initialize_updates=6, + max_external_authenticates=2, + slot_bound=True, ), } def rules_for(op: str) -> OpRules: - """The rule set for an operation; refuses operations with no rules yet.""" + """The rule set for an operation; refuses operations with no rules.""" try: return RULES[op] except KeyError: @@ -188,6 +252,22 @@ def select_target(apdu: bytes) -> bytes | None: return aid +def delete_target(apdu: bytes) -> bytes | None: + """The AID a DELETE names, when its data travels in the clear. + + Under a secure channel with command encryption the data is opaque and this + returns ``None``; with a MAC alone the ``4F`` AID template is readable and + the MAC simply trails it. + """ + body = apdu[5:] # past CLA INS P1 P2 Lc + if len(body) < 3 or body[0] != 0x4F: + return None + length = body[1] + if not 5 <= length <= 16 or len(body) < 2 + length: + return None + return body[2 : 2 + length] + + @dataclass class RelayPolicy: """Stateful gate over one operation's command stream. @@ -198,26 +278,32 @@ class RelayPolicy: """ op: str + #: For key operations: the slot the operation was asked to generate into. + slot: int | None = None rules: OpRules = field(init=False) #: Context after a fresh connection: the card manager answers by default. context: str = ISD apdus: int = 0 + load_blocks: int = 0 initialize_updates: dict[str, int] = field(default_factory=dict) + external_authenticates: dict[str, int] = field(default_factory=dict) _pending_select: bytes | None = field(default=None, init=False, repr=False) def __post_init__(self) -> None: self.rules = rules_for(self.op) + if self.rules.slot_bound and self.slot is None: + raise RemotePolicyError(f"the {self.op} policy needs the target slot") # -- evaluation -------------------------------------------------------- # def check(self, apdu: bytes) -> Header: """Refuse or allow one command. Returns the parsed header on allow.""" header = parse_header(apdu) self._pending_select = None + rules = self.rules - if self.apdus >= self.rules.max_apdus: + if self.apdus >= rules.max_apdus: raise RemotePolicyError( - f"the service sent more than {self.rules.max_apdus} commands in one " - f"{self.op} operation", + f"the service sent more than {rules.max_apdus} commands in one {self.op} operation", header=header.hex, ) if header.channel != 0: @@ -237,7 +323,7 @@ def check(self, apdu: bytes) -> Header: raise RemotePolicyError( f"INS {header.ins:02X} is never relayed (card-holder verifier)", header=header.hex ) - if header.ins == INS_EXTERNAL_AUTHENTICATE and not self.rules.allow_external_authenticate: + if header.ins == INS_EXTERNAL_AUTHENTICATE and rules.max_external_authenticates == 0: raise RemotePolicyError( f"EXTERNAL AUTHENTICATE is not allowed during {self.op}; a failed attempt " "burns a card-management retry", @@ -247,7 +333,7 @@ def check(self, apdu: bytes) -> Header: aid = select_target(apdu) if aid is not None: target = ISD_AID if aid == b"" else aid - if target not in self.rules.selectable: + if target not in rules.selectable: raise RemotePolicyError( f"SELECT of AID {target.hex().upper()} is not allowed during {self.op}", header=header.hex, @@ -255,7 +341,7 @@ def check(self, apdu: bytes) -> Header: self._pending_select = target return header - allowed = self.rules.allowed_ins.get(self.context, frozenset()) + allowed = rules.allowed_ins.get(self.context, frozenset()) if header.ins not in allowed: raise RemotePolicyError( f"INS {header.ins:02X} is not allowed while the {self.context} is selected " @@ -264,15 +350,57 @@ def check(self, apdu: bytes) -> Header: ) if header.ins == INS_INITIALIZE_UPDATE: - count = self.initialize_updates.get(self.context, 0) - if count >= self.rules.max_initialize_updates: + self._enforce_cap( + self.initialize_updates, rules.max_initialize_updates, "INITIALIZE UPDATE", header + ) + elif header.ins == INS_EXTERNAL_AUTHENTICATE: + self._enforce_cap( + self.external_authenticates, + rules.max_external_authenticates, + "EXTERNAL AUTHENTICATE", + header, + ) + elif header.ins == INS_DELETE: + deleted = delete_target(apdu) + if deleted is not None and ( + rules.delete_targets is None or deleted not in rules.delete_targets + ): raise RemotePolicyError( - f"INITIALIZE UPDATE to the {self.context} more than " - f"{self.rules.max_initialize_updates} time(s) during {self.op}", + f"DELETE of {deleted.hex().upper()} refused; only the PIV instance, " + "package and security domain may be deleted", header=header.hex, ) + elif header.ins == INS_LOAD: + if self.load_blocks >= rules.max_load_blocks: + raise RemotePolicyError( + f"more than {rules.max_load_blocks} LOAD blocks in one {self.op} operation", + header=header.hex, + ) + elif rules.slot_bound and header.ins == INS_GENERATE_ASYMMETRIC and header.p2 != self.slot: + raise RemotePolicyError( + f"GENERATE for slot {header.p2:02X} refused; this operation was asked " + f"for slot {self.slot:02X}", + header=header.hex, + ) + elif ( + rules.slot_bound + and header.ins == INS_GENERAL_AUTHENTICATE + and header.p2 != MANAGEMENT_KEY_REF + ): + raise RemotePolicyError( + f"GENERAL AUTHENTICATE with key {header.p2:02X} refused; only the " + "management key may be exercised, never a slot key", + header=header.hex, + ) return header + def _enforce_cap(self, counter: dict[str, int], cap: int, what: str, header: Header) -> None: + if counter.get(self.context, 0) >= cap: + raise RemotePolicyError( + f"{what} to the {self.context} more than {cap} time(s) during {self.op}", + header=header.hex, + ) + def observe(self, apdu: bytes, sw1: int, sw2: int) -> None: """Record what the card did with an allowed command.""" self.apdus += 1 @@ -285,9 +413,15 @@ def observe(self, apdu: bytes, sw1: int, sw2: int) -> None: return if header.ins == INS_INITIALIZE_UPDATE: self.initialize_updates[self.context] = self.initialize_updates.get(self.context, 0) + 1 - if header.ins == INS_EXTERNAL_AUTHENTICATE and not ok: - raise RemotePolicyError( - f"EXTERNAL AUTHENTICATE failed (SW={sw1:02X}{sw2:02X}); stopping before " - "another attempt can burn a retry", - header=header.hex, + elif header.ins == INS_EXTERNAL_AUTHENTICATE: + self.external_authenticates[self.context] = ( + self.external_authenticates.get(self.context, 0) + 1 ) + if not ok: + raise RemotePolicyError( + f"EXTERNAL AUTHENTICATE failed (SW={sw1:02X}{sw2:02X}); stopping before " + "another attempt can burn a retry", + header=header.hex, + ) + elif header.ins == INS_LOAD: + self.load_blocks += 1 diff --git a/tests/unit/test_remote_attest_command.py b/tests/unit/test_remote_attest_command.py new file mode 100644 index 0000000..d46dba2 --- /dev/null +++ b/tests/unit/test_remote_attest_command.py @@ -0,0 +1,333 @@ +"""The ``remote attest`` command: wiring, overwrite gate, post-verification. No socket, no card.""" + +import contextlib +import datetime +import json +import sys + +import pytest +from click.testing import CliRunner +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509.oid import NameOID + +from cryptnox_id_cli.applets.piv import objects as piv_obj +from cryptnox_id_cli.cli.commands import remote as remote_cmd +from cryptnox_id_cli.cli.main import main as root +from cryptnox_id_cli.remote import attest_verify as av +from cryptnox_id_cli.remote import channel as rchannel + +ATR = "3BFA1300FF910131FE000031C173C84000009000D2" +UID = "4301012345670042" +SELECT_ISD = "00A4040008A00000015100000000" +CPLC_CMD = "80CA9F7F00" +CPLC_RESP = ( + "9F7F2A4790D6004700000000004301012345670042000000000000000002507231323334350000000000000000" +) +NOW = datetime.datetime(2026, 9, 22, tzinfo=datetime.timezone.utc) + + +# --------------------------------------------------------------------------- # +# A tiny PKI and a card whose attestation container we control # +# --------------------------------------------------------------------------- # +def _name(cn, serial=None): + attrs = [x509.NameAttribute(NameOID.COMMON_NAME, cn)] + if serial: + attrs.append(x509.NameAttribute(NameOID.SERIAL_NUMBER, serial)) + return x509.Name(attrs) + + +def _der_int(value): + body = value.to_bytes((value.bit_length() + 8) // 8, "big", signed=True) + return bytes([0x02, len(body)]) + body + + +def _cert(subject, issuer_name, issuer_key, key, *, ca=False, extensions=()): + b = ( + x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(issuer_name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(NOW - datetime.timedelta(days=1)) + .not_valid_after(NOW + datetime.timedelta(days=365)) + ) + if ca: + b = b.add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + for ext in extensions: + b = b.add_extension(ext, critical=False) + return b.sign(issuer_key, hashes.SHA256()) + + +@pytest.fixture(scope="module") +def pki(): + root_key = ec.generate_private_key(ec.SECP256R1()) + root = _cert(_name("TEST ROOT"), _name("TEST ROOT"), root_key, root_key, ca=True) + slot_key = ec.generate_private_key(ec.SECP256R1()) + leaf = _cert( + _name("CRYPTNOX PIV ATTESTATION", UID), + root.subject, + root_key, + slot_key, + extensions=[ + x509.UnrecognizedExtension(av.OID_PIV_SLOT, _der_int(0x9C)), + x509.UnrecognizedExtension(av.OID_TRUST_MODEL, _der_int(2)), + ], + ) + return { + "root_der": root.public_bytes(serialization.Encoding.DER), + "leaf_der": leaf.public_bytes(serialization.Encoding.DER), + "leaf_pem": leaf.public_bytes(serialization.Encoding.PEM).decode(), + } + + +def _piv_exchanges(container_der: bytes | None) -> dict[str, str]: + """A PIV applet whose attestation container holds ``container_der`` (or nothing).""" + select_piv = "00A404000BA00000030800001000010000" + apt = ( + "616F4F0BA00000030800001000010079074F05A000000308500B4F70656E464950533230315F5049" + "687474703A2F2F6E766C707562732E6E6973742E676F762F6E697374707562732F5370656369616C" + "5075626C69636174696F6E732F4E4953542E53502E3830302D37332D342E706466" + ) + ex = {select_piv: f"{apt}|9000"} + obj = piv_obj.object_by_name("attestation-cert") + assert obj is not None + get_att = piv_obj.get_data_apdu(obj.oid).to_bytes().hex().upper() + if container_der is not None: + ex[get_att] = _wrapped(piv_obj.wrap_certificate(container_der)) + else: + ex[get_att] = "|6A82" + sign_obj = piv_obj.object_by_name("sign-cert") + assert sign_obj is not None + ex[piv_obj.get_data_apdu(sign_obj.oid).to_bytes().hex().upper()] = "|6A82" + return ex + + +def _len(value: bytes) -> bytes: + from cryptnox_id_cli.util import tlv + + return tlv.encode_length(len(value)) + + +def _wrapped(value: bytes) -> str: + return (bytes([0x53]) + _len(value) + value).hex().upper() + "|9000" + + +class ScriptedChannel: + def __init__(self, script): + self.script = list(script) + self.sent: list[dict] = [] + + def send(self, message: str) -> None: + self.sent.append(json.loads(message)) + + def recv(self, timeout=None) -> str: + return json.dumps(self.script.pop(0)) + + def close(self) -> None: + pass + + +class _FakeSys: + class _Stdin: + def isatty(self): + return True + + def __getattr__(self, name): + return getattr(sys.stdin, name) + + stdin = _Stdin() + + def __getattr__(self, name): + return getattr(sys, name) + + +@pytest.fixture +def wired(monkeypatch, mock_connection, pki): + """Card with an EMPTY attestation container before, the leaf in it after.""" + base = {SELECT_ISD: "6F108408A000000151000000A5049F6501FF|9000", CPLC_CMD: f"{CPLC_RESP}|9000"} + before = mock_connection(ATR, {**base, **_piv_exchanges(None)}, []) + after_ex = {**base, **_piv_exchanges(pki["leaf_der"])} + holder: dict = {"conn": before, "opened": 0, "after": after_ex} + + original = before.transmit + + def transmit(apdu): + # Once the service has run (the channel was opened), the card holds the leaf. + if holder["opened"]: + key = bytes(apdu).hex().upper() + spec = holder["after"].get(key) + if spec is not None: + data_hex, sw_hex = spec.split("|") + sw = bytes.fromhex(sw_hex) + return (list(bytes.fromhex(data_hex)) if data_hex else []), sw[0], sw[1] + return original(apdu) + + before.transmit = transmit + monkeypatch.setattr(remote_cmd, "pick_reader", lambda pref: "Fake Reader 00 00") + monkeypatch.setattr(remote_cmd, "connect", lambda name: before) + monkeypatch.setattr(av.trust, "load_anchors", lambda extra_dir=None: ([pki["root_der"]], [])) + monkeypatch.delenv(rchannel.URL_ENV, raising=False) + + @contextlib.contextmanager + def fake_open(url, **kwargs): + holder["opened"] += 1 + yield holder["channel"] + + monkeypatch.setattr(rchannel, "open_channel", fake_open) + + def arm(script): + holder["channel"] = ScriptedChannel(script) + return holder["channel"] + + holder["arm"] = arm + return holder + + +def run(args, **kwargs): + return CliRunner().invoke(root, args, **kwargs) + + +def ok_result(pki, **extra): + return { + "type": "result", + "ok": True, + "slot": "9C", + "algorithm": "ECCP256", + "chain_verified": True, + "written_to_card": True, + "readback_matches": True, + "attestation_cert_pem": pki["leaf_pem"], + **extra, + } + + +# --------------------------------------------------------------------------- # +# Wiring and parameters # +# --------------------------------------------------------------------------- # +def test_attest_sends_slot_and_algorithm_and_omits_ca_obj_by_default(wired, pki): + channel = wired["arm"]([ok_result(pki)]) + result = run(["--json", "remote", "attest"]) + assert result.exit_code == 0, result.output + assert channel.sent[0] == { + "type": "hello", + "op": "attest", + "params": {"slot": "9C", "algorithm": "ECCP256"}, + } + + +def test_ca_obj_is_sent_only_when_given(wired, pki): + channel = wired["arm"]([ok_result(pki)]) + run(["--json", "remote", "attest", "--ca-obj", "0301"]) + assert channel.sent[0]["params"]["ca_obj"] == "0301" + + +def test_slot_is_validated(): + result = run(["remote", "attest", "--slot", "9B"]) + assert result.exit_code == 2 + result = run(["remote", "attest", "--slot", "zz"]) + assert result.exit_code == 2 + + +def test_dry_run_refuses(monkeypatch): + monkeypatch.setattr(remote_cmd, "pick_reader", lambda *a: (_ for _ in ()).throw(AssertionError)) + result = run(["--dry-run", "remote", "attest"]) + assert result.exit_code != 0 + assert "nothing was sent to the card" in result.output + + +# --------------------------------------------------------------------------- # +# Post-verification # +# --------------------------------------------------------------------------- # +def test_genuine_result_verifies_and_binds(wired, pki): + wired["arm"]([ok_result(pki)]) + result = run(["--json", "remote", "attest"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + v = payload["verification"] + assert v["verified"] is True + assert v["chain_verified"] is True + assert v["serial_number_matches_card"] is True + assert v["slot_matches"] is True + assert v["trust_model_matches"] is True + assert v["public_key_matches_card"] is True + assert payload["verified_locally"]["attestation_verified"] is True + assert payload["verified_locally"]["attestation_container_written"] is True + + +def test_wrong_slot_in_the_certificate_is_reported(wired, pki): + wired["arm"]([ok_result(pki)]) + result = run(["--json", "remote", "attest", "--slot", "9A"]) + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["verification"]["slot_matches"] is False + assert payload["verified_locally"]["attestation_verified"] is False + + +def test_missing_certificate_is_reported_not_faked(wired, pki): + wired["arm"]([ok_result(pki, attestation_cert_pem="")]) + result = run(["--json", "remote", "attest"]) + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["verification"] is None + assert payload["verified_locally"]["attestation_returned"] is False + + +def test_unreadable_certificate_is_an_error(wired, pki): + wired["arm"]([ok_result(pki, attestation_cert_pem="-----BEGIN CERTIFICATE-----\nnope\n")]) + result = run(["--json", "remote", "attest"]) + assert result.exit_code == 1 + assert "unreadable certificate" in json.loads(result.stdout)["message"] + + +def test_out_writes_the_pem(wired, pki, tmp_path): + wired["arm"]([ok_result(pki)]) + out = tmp_path / "att.pem" + result = run(["--json", "remote", "attest", "--out", str(out)]) + assert result.exit_code == 0 + assert out.read_text() == pki["leaf_pem"] + + +def test_human_output_names_the_claim_it_cannot_witness(wired, pki): + wired["arm"]([ok_result(pki)]) + result = run(["remote", "attest"]) + assert result.exit_code == 0, result.output + assert "cannot witness" in result.output + assert "verified" in result.output + + +# --------------------------------------------------------------------------- # +# Overwrite gate # +# --------------------------------------------------------------------------- # +@pytest.fixture +def occupied(wired, pki): + """The attestation container already holds a certificate before the run.""" + wired["conn"]._exchanges.update(_piv_exchanges(pki["leaf_der"])) + return wired + + +def test_occupied_container_fails_closed_non_interactively(occupied, pki): + occupied["arm"]([ok_result(pki)]) + result = run(["--json", "remote", "attest"]) + assert result.exit_code == 1 + assert "pass --yes" in json.loads(result.stdout)["message"] + assert occupied["opened"] == 0 + + +def test_occupied_container_proceeds_with_yes(occupied, pki): + occupied["arm"]([ok_result(pki)]) + result = run(["--yes", "--json", "remote", "attest"]) + assert result.exit_code == 0, result.output + assert occupied["opened"] == 1 + + +def test_occupied_container_asks_interactively(occupied, pki, monkeypatch): + monkeypatch.setattr(remote_cmd, "sys", _FakeSys()) + occupied["arm"]([ok_result(pki)]) + result = run(["remote", "attest"], input="n\n") + assert result.exit_code != 0 + assert occupied["opened"] == 0 + occupied["arm"]([ok_result(pki)]) + result = run(["remote", "attest"], input="y\n") + assert result.exit_code == 0, result.output diff --git a/tests/unit/test_remote_attest_verify.py b/tests/unit/test_remote_attest_verify.py new file mode 100644 index 0000000..67831a5 --- /dev/null +++ b/tests/unit/test_remote_attest_verify.py @@ -0,0 +1,226 @@ +"""Local verification of a returned attestation certificate, against a test PKI.""" + +import datetime + +import pytest +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509.oid import NameOID + +from cryptnox_id_cli.remote import attest_verify as av + +UID = "4301012345670042" +NOW = datetime.datetime(2026, 9, 22, tzinfo=datetime.timezone.utc) + + +def _name(cn: str, serial: str | None = None) -> x509.Name: + attrs = [ + x509.NameAttribute(NameOID.COUNTRY_NAME, "CH"), + x509.NameAttribute(NameOID.ORGANIZATION_NAME, "TEST PKI"), + x509.NameAttribute(NameOID.COMMON_NAME, cn), + ] + if serial is not None: + attrs.append(x509.NameAttribute(NameOID.SERIAL_NUMBER, serial)) + return x509.Name(attrs) + + +def _der_int(value: int) -> bytes: + # Minimal two's-complement encoding, so 0x9C becomes 00 9C, not a negative byte. + body = value.to_bytes((value.bit_length() + 8) // 8, "big", signed=True) + return bytes([0x02, len(body)]) + body + + +def _ca(subject: str, issuer_key=None, issuer_name=None): + key = ec.generate_private_key(ec.SECP256R1()) + name = _name(subject) + builder = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(issuer_name or name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(NOW - datetime.timedelta(days=1)) + .not_valid_after(NOW + datetime.timedelta(days=3650)) + .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + ) + cert = builder.sign(issuer_key or key, hashes.SHA256()) + return key, cert + + +def _leaf( + issuer_key, + issuer_cert, + *, + uid: str | None = UID, + slot: int | None = 0x9C, + trust_model: int | None = 2, + key=None, +): + key = key or ec.generate_private_key(ec.SECP256R1()) + builder = ( + x509.CertificateBuilder() + .subject_name(_name("CRYPTNOX PIV ATTESTATION", uid)) + .issuer_name(issuer_cert.subject) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(NOW - datetime.timedelta(days=1)) + .not_valid_after(NOW + datetime.timedelta(days=365)) + ) + if slot is not None: + builder = builder.add_extension( + x509.UnrecognizedExtension(av.OID_PIV_SLOT, _der_int(slot)), critical=False + ) + if trust_model is not None: + builder = builder.add_extension( + x509.UnrecognizedExtension(av.OID_TRUST_MODEL, _der_int(trust_model)), + critical=False, + ) + return key, builder.sign(issuer_key, hashes.SHA256()) + + +def _der(cert) -> bytes: + return cert.public_bytes(serialization.Encoding.DER) + + +def _pem(cert) -> str: + return cert.public_bytes(serialization.Encoding.PEM).decode() + + +@pytest.fixture(scope="module") +def pki(): + root_key, root = _ca("TEST ROOT CA") + att_key, att = _ca("TEST ATTESTATION CA", issuer_key=root_key, issuer_name=root.subject) + return {"root": root, "att_key": att_key, "att": att, "anchors": ([_der(root)], [_der(att)])} + + +def verify(pki, leaf, *, slot=0x9C, uid=UID, on_card=None): + return av.verify_returned_attestation( + _der(leaf), + slot=slot, + cplc_uid=uid, + on_card_der=on_card, + anchors=pki["anchors"], + at_time=NOW, + ) + + +# --------------------------------------------------------------------------- # +# Helpers # +# --------------------------------------------------------------------------- # +def test_leaf_from_pem_returns_the_first_certificate(pki): + _, leaf = _leaf(pki["att_key"], pki["att"]) + assert av.leaf_from_pem(_pem(leaf) + _pem(pki["att"])) == _der(leaf) + + +def test_leaf_from_pem_rejects_empty(): + with pytest.raises(ValueError): + av.leaf_from_pem("not a certificate") + + +def test_der_integer(): + assert av.der_integer(bytes.fromhex("020102")) == 2 + assert av.der_integer(bytes.fromhex("02019C")) == -100 # signed: 0x9C alone is negative + assert av.der_integer(bytes.fromhex("0202009C")) == 0x9C + assert av.der_integer(bytes.fromhex("0401AA")) is None # not an INTEGER + assert av.der_integer(b"") is None + assert av.der_integer(bytes.fromhex("020200")) is None # truncated + + +# --------------------------------------------------------------------------- # +# The whole check # +# --------------------------------------------------------------------------- # +def test_genuine_leaf_verifies_and_binds(pki): + _, leaf = _leaf(pki["att_key"], pki["att"]) + result = verify(pki, leaf, on_card=_der(leaf)) + assert result.verified is True + assert result.chain_verified is True + assert "CN=CRYPTNOX PIV ATTESTATION" in result.chain[0] + assert result.serial_number == UID + assert result.serial_number_matches_card is True + assert result.slot == 0x9C + assert result.slot_matches is True + assert result.trust_model == 2 + assert result.trust_model_matches is True + assert result.public_key_matches_card is True + assert result.reasons == [] + assert result.to_dict()["slot"] == "9C" + + +def test_without_the_card_copy_the_key_binding_is_unchecked_and_not_verified(pki): + _, leaf = _leaf(pki["att_key"], pki["att"]) + result = verify(pki, leaf, on_card=None) + assert result.public_key_matches_card is None + assert result.verified is False # an unchecked binding is not a passed one + assert result.chain_verified is True + + +def test_wrong_slot_is_a_failure(pki): + _, leaf = _leaf(pki["att_key"], pki["att"], slot=0x9A) + result = verify(pki, leaf, slot=0x9C, on_card=_der(leaf)) + assert result.slot == 0x9A + assert result.slot_matches is False + assert result.verified is False + assert any("attests slot 9A" in r for r in result.reasons) + + +def test_wrong_card_is_a_failure(pki): + _, leaf = _leaf(pki["att_key"], pki["att"], uid="4301999999990042") + result = verify(pki, leaf, on_card=_der(leaf)) + assert result.serial_number_matches_card is False + assert result.verified is False + + +def test_serial_comparison_ignores_case(pki): + _, leaf = _leaf(pki["att_key"], pki["att"], uid=UID.lower()) + assert verify(pki, leaf, on_card=_der(leaf)).serial_number_matches_card is True + + +def test_wrong_trust_model_is_a_failure(pki): + _, leaf = _leaf(pki["att_key"], pki["att"], trust_model=1) + result = verify(pki, leaf, on_card=_der(leaf)) + assert result.trust_model == 1 + assert result.trust_model_matches is False + assert result.verified is False + + +def test_missing_extensions_are_failures_not_passes(pki): + _, leaf = _leaf(pki["att_key"], pki["att"], slot=None, trust_model=None) + result = verify(pki, leaf, on_card=_der(leaf)) + assert result.slot_matches is False + assert result.trust_model_matches is False + assert result.verified is False + assert any("no cryptnoxPivSlot" in r for r in result.reasons) + + +def test_missing_serial_number_is_a_failure(pki): + _, leaf = _leaf(pki["att_key"], pki["att"], uid=None) + result = verify(pki, leaf, on_card=_der(leaf)) + assert result.serial_number is None + assert result.serial_number_matches_card is False + assert result.verified is False + + +def test_untrusted_chain_is_a_failure(pki): + other_key, other_root = _ca("SOME OTHER ROOT") + _, leaf = _leaf(other_key, other_root) + result = verify(pki, leaf, on_card=_der(leaf)) + assert result.chain_verified is False + assert result.verified is False + assert any("not anchored" in r for r in result.reasons) + + +def test_public_key_mismatch_with_the_card_is_a_failure(pki): + _, leaf = _leaf(pki["att_key"], pki["att"]) + _, other = _leaf(pki["att_key"], pki["att"]) # a different key pair + result = verify(pki, leaf, on_card=_der(other)) + assert result.public_key_matches_card is False + assert result.verified is False + assert any("differs from the certificate stored on the card" in r for r in result.reasons) + + +def test_uid_unknown_leaves_the_serial_binding_unchecked(pki): + _, leaf = _leaf(pki["att_key"], pki["att"]) + result = verify(pki, leaf, uid=None, on_card=_der(leaf)) + assert result.serial_number_matches_card is None + assert result.verified is False diff --git a/tests/unit/test_remote_gates.py b/tests/unit/test_remote_gates.py new file mode 100644 index 0000000..1a23b07 --- /dev/null +++ b/tests/unit/test_remote_gates.py @@ -0,0 +1,251 @@ +"""Consent gates for ``remote reset`` and ``remote dev-reset``. No socket, no card.""" + +import contextlib +import json +import sys + +import pytest +from click.testing import CliRunner + +from cryptnox_id_cli.cli.commands import remote as remote_cmd +from cryptnox_id_cli.cli.main import main as root +from cryptnox_id_cli.remote import channel as rchannel + +ATR = "3BFA1300FF910131FE000031C173C84000009000D2" +SELECT_ISD = "00A4040008A00000015100000000" +SELECT_SSD = "00A4040009A0000001515350410100" +SELECT_PIV = "00A404000BA00000030800001000010000" +CPLC_CMD = "80CA9F7F00" +CPLC_RESP = ( + "9F7F2A4790D6004700000000004301012345670042000000000000000002507231323334350000000000000000" +) +KEY_INFO = "80CA00E000" +KEY_INFO_ONE = "E012C00401018010C00402018010C00403018010" +KEY_INFO_TWO = "E018C00401018010C00402018010C00403018010C00401028810" + + +class _FakeSys: + """The real ``sys``, but ``stdin.isatty()`` reports a terminal.""" + + class _Stdin: + def isatty(self): + return True + + def __getattr__(self, name): + return getattr(sys.stdin, name) + + stdin = _Stdin() + + def __getattr__(self, name): + return getattr(sys, name) + + +class ScriptedChannel: + def __init__(self, script): + self.script = list(script) + self.sent: list[dict] = [] + + def send(self, message: str) -> None: + self.sent.append(json.loads(message)) + + def recv(self, timeout=None) -> str: + return json.dumps(self.script.pop(0)) + + def close(self) -> None: + pass + + +@pytest.fixture +def wired(monkeypatch, mock_connection): + """A blank-PIV card with a security domain on key version 1, and a scripted service.""" + conn = mock_connection( + ATR, + { + SELECT_ISD: "6F108408A000000151000000A5049F6501FF|9000", + SELECT_SSD: "6F118409A00000015153504101A5049F6501FF|9000", + SELECT_PIV: "|6A82", + "00A4040000": "|9000", + CPLC_CMD: f"{CPLC_RESP}|9000", + KEY_INFO: f"{KEY_INFO_ONE}|9000", + }, + [], + ) + monkeypatch.setattr(remote_cmd, "pick_reader", lambda pref: "Fake Reader 00 00") + monkeypatch.setattr(remote_cmd, "connect", lambda name: conn) + monkeypatch.delenv(rchannel.URL_ENV, raising=False) + monkeypatch.delenv(remote_cmd.DEV_ACCESS_ENV, raising=False) + holder: dict = {"conn": conn, "opened": 0} + + @contextlib.contextmanager + def fake_open(url, **kwargs): + holder["opened"] += 1 + yield holder["channel"] + + monkeypatch.setattr(rchannel, "open_channel", fake_open) + + def arm(script): + holder["channel"] = ScriptedChannel(script) + return holder["channel"] + + holder["arm"] = arm + return holder + + +def run(args, **kwargs): + return CliRunner().invoke(root, args, **kwargs) + + +OK_RESULT = {"type": "result", "ok": True, "wiped_and_reprovisioned": True, "steps": []} + + +# --------------------------------------------------------------------------- # +# Fail closed # +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize("op", ["reset", "dev-reset"]) +def test_non_interactive_without_the_flag_refuses_before_connecting(wired, op, monkeypatch): + monkeypatch.setenv(remote_cmd.DEV_ACCESS_ENV, "dev-credential") + wired["arm"]([OK_RESULT]) + result = run(["--json", "remote", op]) + assert result.exit_code == 1 + assert "needs --i-understand-this-is-irreversible" in json.loads(result.stdout)["message"] + assert wired["opened"] == 0 + + +@pytest.mark.parametrize("op", ["reset", "dev-reset"]) +def test_yes_is_not_a_bypass(wired, op, monkeypatch): + monkeypatch.setenv(remote_cmd.DEV_ACCESS_ENV, "dev-credential") + wired["arm"]([OK_RESULT]) + result = run(["--yes", "--json", "remote", op]) + assert result.exit_code == 1 + assert wired["opened"] == 0 + + +def test_wrong_phrase_aborts(wired, monkeypatch): + monkeypatch.setattr(remote_cmd, "sys", _FakeSys()) + wired["arm"]([OK_RESULT]) + result = run(["remote", "reset"], input="reset-piv\n") + assert result.exit_code != 0 + assert wired["opened"] == 0 + assert "Before" in result.output # the inventory was shown before asking + + +def test_right_phrase_proceeds(wired, monkeypatch): + monkeypatch.setattr(remote_cmd, "sys", _FakeSys()) + wired["arm"]([OK_RESULT]) + result = run(["remote", "reset"], input="RESET-PIV\n") + assert result.exit_code == 0, result.output + assert wired["opened"] == 1 + assert "After" in result.output + + +def test_dry_run_refuses_before_anything_opens(monkeypatch): + def boom(*a, **k): + raise AssertionError("reader must not be opened under --dry-run") + + monkeypatch.setattr(remote_cmd, "pick_reader", boom) + for op in ("reset", "dev-reset"): + result = run(["--dry-run", "remote", op, "--i-understand-this-is-irreversible"]) + assert result.exit_code != 0 + assert "nothing was sent to the card" in result.output + + +@pytest.mark.parametrize("op", ["reset", "dev-reset"]) +def test_endpoint_override_is_refused(wired, op, monkeypatch): + monkeypatch.setenv(rchannel.URL_ENV, "ws://localhost:9999") + monkeypatch.setenv(remote_cmd.DEV_ACCESS_ENV, "dev-credential") + result = run(["--json", "remote", op, "--i-understand-this-is-irreversible"]) + assert result.exit_code == 13 + assert "production service" in json.loads(result.stdout)["message"] + + +# --------------------------------------------------------------------------- # +# The flagged path # +# --------------------------------------------------------------------------- # +def test_reset_payload_carries_before_after_and_params(wired): + channel = wired["arm"]([OK_RESULT]) + result = run(["--json", "remote", "reset", "--i-understand-this-is-irreversible"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert channel.sent[0] == {"type": "hello", "op": "reset", "params": {"fused": False}} + assert payload["before"]["piv_ssd_key_versions"] == [1] + assert payload["before"]["piv_state"] == "PivNotPresent" + assert payload["after"]["piv_ssd_key_versions"] == [1] + assert payload["verified_locally"]["piv_ssd_present_after"] is True + assert payload["verified_locally"]["piv_present_after"] is False + assert payload["result"]["wiped_and_reprovisioned"] is True + + +def test_fused_is_passed_through_and_warned(wired): + channel = wired["arm"]([OK_RESULT]) + result = run(["remote", "reset", "--fused", "--i-understand-this-is-irreversible"]) + assert result.exit_code == 0, result.output + assert channel.sent[0]["params"] == {"fused": True} + assert "--fused" in result.output + + +def test_dev_reset_sends_the_credential_from_the_environment_only(wired, monkeypatch): + monkeypatch.setenv(remote_cmd.DEV_ACCESS_ENV, "dev-credential-value") + channel = wired["arm"]([OK_RESULT]) + result = run( + ["--json", "remote", "dev-reset", "--i-understand-this-is-irreversible"], + ) + assert result.exit_code == 0, result.output + assert channel.sent[0]["params"] == {"fused": False, "dev_token": "dev-credential-value"} + assert "dev-credential-value" not in result.output + + +def test_dev_reset_without_the_credential_fails_closed(wired): + wired["arm"]([OK_RESULT]) + result = run(["--json", "remote", "dev-reset", "--i-understand-this-is-irreversible"]) + assert result.exit_code == 3 # secret_input + assert wired["opened"] == 0 + + +def test_dev_reset_has_no_credential_option(): + result = run(["remote", "dev-reset", "--help"]) + assert "--dev-token" not in result.output + assert "CRYPTNOX_REMOTE_DEV_TOKEN" in result.output + + +def test_after_inventory_reflects_a_new_key_version(wired): + # The service loads key version 2 into the security domain; the local + # read afterwards must show it. + conn = wired["conn"] + channel = wired["arm"]([OK_RESULT]) + original = conn.transmit + + def evolving(apdu): + data, sw1, sw2 = original(apdu) + if bytes(apdu).hex().upper() == KEY_INFO and channel.sent: + return list(bytes.fromhex(KEY_INFO_TWO)), 0x90, 0x00 + return data, sw1, sw2 + + conn.transmit = evolving + result = run(["--json", "remote", "reset", "--i-understand-this-is-irreversible"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert payload["before"]["piv_ssd_key_versions"] == [1] + assert payload["after"]["piv_ssd_key_versions"] == [1, 2] + + +def test_service_failure_still_releases_the_card(wired, monkeypatch): + released = [] + conn = wired["conn"] + monkeypatch.setattr(conn, "disconnect", lambda: released.append(True)) + wired["arm"]([{"type": "result", "ok": False, "error": "isd auth failed"}]) + result = run(["--json", "remote", "reset", "--i-understand-this-is-irreversible"]) + assert result.exit_code == 16 + assert released == [True] + + +# --------------------------------------------------------------------------- # +# The key-free security domain probe # +# --------------------------------------------------------------------------- # +def test_ssd_key_versions_absent_and_present(wired, mock_connection): + from cryptnox_id_cli.transport.pcsc import CardSession + + absent = mock_connection(ATR, {SELECT_SSD: "|6A82"}, []) + assert remote_cmd._ssd_key_versions(CardSession(absent)) is None + assert remote_cmd._ssd_key_versions(CardSession(wired["conn"])) == [1] + two = mock_connection(ATR, {SELECT_SSD: "6F11|9000", KEY_INFO: f"{KEY_INFO_TWO}|9000"}, []) + assert remote_cmd._ssd_key_versions(CardSession(two)) == [1, 2] diff --git a/tests/unit/test_remote_policy.py b/tests/unit/test_remote_policy.py index 4e1a28a..1af9748 100644 --- a/tests/unit/test_remote_policy.py +++ b/tests/unit/test_remote_policy.py @@ -79,7 +79,7 @@ def test_select_with_short_data_is_refused(): # --------------------------------------------------------------------------- # def test_operations_without_rules_are_refused_up_front(): with pytest.raises(RemotePolicyError, match="no relay policy"): - pol.RelayPolicy("reset") + pol.RelayPolicy("format-everything") # --------------------------------------------------------------------------- # @@ -193,10 +193,13 @@ def test_piv_context_allows_only_piv_reads(): apdu(0x00, 0xDB, 0x3F, 0xFF, b"\x5c\x03\x5f\xc1\x02", le=False), # PUT DATA apdu(0x00, 0x87, 0x11, 0x9B, b"\x7c\x02\x81\x00", le=False), # GENERAL AUTH apdu(0x80, 0xCA, 0x9F, 0x7F), # GP GET DATA is not a PIV command - INIT_UPDATE, ): with pytest.raises(RemotePolicyError, match="not allowed while the piv"): policy.check(forbidden) + # The applet's admin channel may be probed, never authenticated. + allow(policy, INIT_UPDATE) + with pytest.raises(RemotePolicyError, match="EXTERNAL AUTHENTICATE"): + policy.check(EXT_AUTH) def test_inspect_caps_initialize_updates_per_domain(): @@ -251,3 +254,178 @@ def test_refusal_carries_the_header_for_review(): policy.check(apdu(0x00, 0x47, 0x00, 0x9C)) assert info.value.header == "0047009C" assert info.value.to_dict()["apdu_header"] == "0047009C" + + +# --------------------------------------------------------------------------- # +# reset / dev-reset: card content management, PIV only # +# --------------------------------------------------------------------------- # +PIV_PACKAGE_AID = pol.PIV_PACKAGE_AID +FIDO2_PACKAGE_AID = bytes.fromhex("A000000647") +SSD_PACKAGE_AID = bytes.fromhex("A0000001515350") + + +def delete(aid: bytes, cla: int = 0x84, mac: bytes = b"\x00" * 8) -> bytes: + data = bytes([0x4F, len(aid)]) + aid + mac + return bytes([cla, 0xE4, 0x00, 0x00, len(data)]) + data + b"\x00" + + +def wrapped(ins: int, p1: int = 0, p2: int = 0, n: int = 24) -> bytes: + return apdu(0x84, ins, p1, p2, b"\x00" * n, le=False) + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +def test_reset_follows_the_documented_flow(op): + policy = pol.RelayPolicy(op) + allow(policy, CPLC) + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + allow(policy, wrapped(0xF2, 0x40, 0x00)) # GET STATUS + allow(policy, delete(pol.PIV_AID)) + allow(policy, delete(PIV_PACKAGE_AID)) + allow(policy, delete(pol.PIV_SSD_AID)) + allow(policy, wrapped(0xE6, 0x02, 0x00)) # INSTALL for load + for _ in range(10): + allow(policy, wrapped(0xE8, 0x00, 0x00, n=200)) # LOAD blocks + allow(policy, wrapped(0xE6, 0x0C, 0x00)) # INSTALL for install + allow(policy, wrapped(0xE6, 0x10, 0x00)) # extradition + allow(policy, select(pol.PIV_SSD_AID)) + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + allow(policy, wrapped(0xD8, 0x00, 0x81)) # PUT KEY + allow(policy, select(pol.PIV_AID)) + allow(policy, PIV_GET_DATA) + assert policy.load_blocks == 10 + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +@pytest.mark.parametrize( + "aid", [WALLET_AID, FIDO2_AID, FIDO2_PACKAGE_AID, GENUINE_AID, SSD_PACKAGE_AID] +) +def test_reset_may_only_delete_piv_things(op, aid): + policy = pol.RelayPolicy(op) + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + with pytest.raises(RemotePolicyError, match="DELETE of") as info: + policy.check(delete(aid)) + assert info.value.header == "84E40000" + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +def test_an_encrypted_delete_passes_on_instruction_and_context_alone(op): + # Under command encryption the AID is opaque; the header-level rules still + # hold (ISD context only) and the limitation is documented. + policy = pol.RelayPolicy(op) + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + allow(policy, wrapped(0xE4, 0x00, 0x00)) + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +def test_content_management_never_leaves_the_card_manager(op): + policy = pol.RelayPolicy(op) + allow(policy, select(pol.PIV_SSD_AID)) + for ins in (0xE4, 0xE6, 0xE8): + with pytest.raises(RemotePolicyError, match="not allowed while the piv-ssd"): + policy.check(wrapped(ins)) + allow(policy, select(pol.PIV_AID)) + for ins in (0xE4, 0xE6, 0xE8, 0xD8, 0x47, 0x87): + with pytest.raises(RemotePolicyError, match="not allowed while the piv"): + policy.check(wrapped(ins)) + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +def test_a_failed_external_authenticate_ends_the_operation(op): + policy = pol.RelayPolicy(op) + allow(policy, INIT_UPDATE) + policy.check(EXT_AUTH) + with pytest.raises(RemotePolicyError, match="EXTERNAL AUTHENTICATE failed"): + policy.observe(EXT_AUTH, 0x63, 0x00) + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +def test_external_authenticate_is_capped_per_domain(op): + policy = pol.RelayPolicy(op) + cap = policy.rules.max_external_authenticates + for _ in range(cap): + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + with pytest.raises( + RemotePolicyError, match=f"EXTERNAL AUTHENTICATE to the isd more than {cap}" + ): + policy.check(EXT_AUTH) + + +def test_load_blocks_are_capped(): + policy = pol.RelayPolicy("reset") + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + policy.load_blocks = policy.rules.max_load_blocks + with pytest.raises(RemotePolicyError, match="LOAD blocks"): + policy.check(wrapped(0xE8)) + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +@pytest.mark.parametrize("aid", [WALLET_AID, FIDO2_AID, GENUINE_AID, DESFIRE_AID]) +def test_reset_cannot_select_other_card_functions(op, aid): + with pytest.raises(RemotePolicyError, match="SELECT of AID"): + pol.RelayPolicy(op).check(select(aid)) + + +def test_delete_target_parsing(): + assert pol.delete_target(delete(pol.PIV_AID)) == pol.PIV_AID + assert pol.delete_target(wrapped(0xE4)) is None # opaque + assert pol.delete_target(bytes.fromhex("84E40000034F01AA")) is None # implausible length + + +# --------------------------------------------------------------------------- # +# attest: one slot, the management key, nothing else # +# --------------------------------------------------------------------------- # +def test_attest_needs_a_slot(): + with pytest.raises(RemotePolicyError, match="needs the target slot"): + pol.RelayPolicy("attest") + + +def test_attest_follows_the_documented_flow(): + policy = pol.RelayPolicy("attest", slot=0x9C) + allow(policy, CPLC) + allow(policy, select(pol.PIV_AID)) + allow(policy, INIT_UPDATE) # the applet's admin channel, keyed by the SSD + allow(policy, EXT_AUTH) + allow(policy, wrapped(0xDB, 0x3F, 0xFF)) # key-object setup + allow(policy, apdu(0x00, 0x87, 0x03, 0x9B, b"\x7c\x02\x81\x00", le=False)) + allow(policy, apdu(0x00, 0x47, 0x00, 0x9C, b"\xac\x03\x80\x01\x11", le=False)) + allow(policy, apdu(0x00, 0xC0, 0x00, 0x00)) + allow(policy, apdu(0x00, 0xDB, 0x3F, 0xFF, b"\x5c\x03\x5f\xc1\x20", le=False)) + allow(policy, PIV_GET_DATA) + + +def test_attest_generate_is_bound_to_the_requested_slot(): + policy = pol.RelayPolicy("attest", slot=0x9C) + allow(policy, select(pol.PIV_AID)) + for other in (0x9A, 0x9D, 0x9E, 0x82): + with pytest.raises(RemotePolicyError, match=f"GENERATE for slot {other:02X}"): + policy.check(apdu(0x00, 0x47, 0x00, other, b"\xac\x03\x80\x01\x11", le=False)) + + +def test_attest_never_exercises_a_slot_key(): + policy = pol.RelayPolicy("attest", slot=0x9C) + allow(policy, select(pol.PIV_AID)) + for key in (0x9A, 0x9C, 0x9D, 0x9E): + with pytest.raises(RemotePolicyError, match="only the management key"): + policy.check(apdu(0x00, 0x87, 0x11, key, b"\x7c\x02\x82\x00", le=False)) + + +def test_attest_cannot_manage_card_content(): + policy = pol.RelayPolicy("attest", slot=0x9C) + allow(policy, INIT_UPDATE) + with pytest.raises(RemotePolicyError, match="EXTERNAL AUTHENTICATE to the isd|not allowed"): + policy.check(wrapped(0xE4)) + for ins in (0xE4, 0xE6, 0xE8, 0xD8): + with pytest.raises(RemotePolicyError, match="not allowed while the isd"): + policy.check(wrapped(ins)) + + +def test_attest_cannot_select_other_card_functions(): + for aid in (WALLET_AID, FIDO2_AID, GENUINE_AID, DESFIRE_AID): + with pytest.raises(RemotePolicyError, match="SELECT of AID"): + pol.RelayPolicy("attest", slot=0x9C).check(select(aid)) From 890ddf135b2f3fa995d2e1706e31f65cf3f5099c Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Tue, 22 Sep 2026 17:57:16 +0200 Subject: [PATCH 3/8] Correct two relay policy rules against the service's real traffic Both rules were written from the protocol description before the operations had been run against the service. Running them showed each rule refusing a command the service legitimately needs, so each refusal is now narrower while the protection it exists for is unchanged. The secure-channel authentication caps were counted per operation, but the service opens a fresh card-manager channel for each step of a reset: delete, load, install, create the security domain, key it, extradite, load the second key version, verify. A reset was refused after the security domain had been keyed, with the applet installed but not yet extradited. The caps are raised to sixteen for both INITIALIZE UPDATE and EXTERNAL AUTHENTICATE. They were never the real protection against a spent retry: that is the abort on the first authentication the card rejects, which is untouched. The service sets the PIV management key with CHANGE REFERENCE DATA, key reference 9B, inside the applet's secure channel. The policy refused the whole instruction as a card-holder verifier, which is true of the PIN and the PUK but not of the management key: 9B carries no card-holder retry counter, and the service loads it before generating a key. CHANGE REFERENCE DATA is now allowed for key reference 9B alone, and only in operations whose rules opt in, which are reset, dev-reset and attest. The PIN and the PUK stay refused everywhere, as do VERIFY and RESET RETRY COUNTER whatever they address. Attest's EXTERNAL AUTHENTICATE cap goes from two to four for the same reason as the reset caps. Co-Authored-By: Claude Fable 5.1 --- src/cryptnox_id_cli/remote/policy.py | 52 +++++++++++++++++++++++----- tests/unit/test_remote_policy.py | 29 ++++++++++++++++ 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/src/cryptnox_id_cli/remote/policy.py b/src/cryptnox_id_cli/remote/policy.py index 30ce020..2aaf656 100644 --- a/src/cryptnox_id_cli/remote/policy.py +++ b/src/cryptnox_id_cli/remote/policy.py @@ -49,9 +49,14 @@ KEY_OPS = ("attest",) #: The PIV management key reference: the only key GENERAL AUTHENTICATE may -#: address during a remote operation. +#: address during a remote operation, and the only reference whose value +#: CHANGE REFERENCE DATA may set. MANAGEMENT_KEY_REF = 0x9B +#: Card-holder references. Their retry counters are the card holder's to spend, +#: so no remote operation may address them, ever. +CARDHOLDER_REFS: frozenset[int] = frozenset({0x80, 0x81}) + # Instructions, by name, so the tables read as prose. INS_SELECT = 0xA4 INS_GET_DATA_GP = 0xCA @@ -103,6 +108,10 @@ class OpRules: #: Whether GENERATE and GENERAL AUTHENTICATE are bound to a slot and the #: management key. Only meaningful for key operations. slot_bound: bool = False + #: Whether the operation may set the PIV management key's value. The + #: service does this over the secure channel before generating a key. + #: Card-holder references stay denied regardless. + allow_management_key_change: bool = False _GP_READS: frozenset[int] = frozenset( @@ -126,16 +135,24 @@ class OpRules: # loads or deletes anything itself. PIV_SSD: _GP_READS | _GP_AUTH | _GP_KEYS, # The fresh applet is checked and may receive its baseline structure - # through its own admin channel; no key use, no key generation. - PIV: _PIV_READS | _GP_AUTH | {INS_PUT_DATA}, + # and its management key through its own admin channel; no key use, no + # key generation. + PIV: _PIV_READS | _GP_AUTH | {INS_PUT_DATA, INS_CHANGE_REFERENCE_DATA}, }, # A CAP of ~190 KB loads in ~800 blocks; the rest is bookkeeping. max_apdus=1500, max_seconds=1800.0, - max_initialize_updates=8, - max_external_authenticates=3, + # The service re-opens the card manager's channel for each step of the + # operation (delete, load, install, create the security domain, key it, + # extradite, verify), so the count is per step, not per operation. Observed: + # four before the security domain was keyed. The real protection against a + # spent retry is not this ceiling but the abort on the first FAILED + # authentication in observe(); this only bounds a runaway. + max_initialize_updates=16, + max_external_authenticates=16, delete_targets=_PIV_DELETE_TARGETS, max_load_blocks=1500, + allow_management_key_change=True, ) RULES: dict[str, OpRules] = { @@ -177,13 +194,19 @@ class OpRules: # management-key handshake, key generation and the certificate write. PIV: _PIV_READS | _GP_AUTH - | {INS_GENERAL_AUTHENTICATE, INS_GENERATE_ASYMMETRIC, INS_PUT_DATA}, + | { + INS_GENERAL_AUTHENTICATE, + INS_GENERATE_ASYMMETRIC, + INS_PUT_DATA, + INS_CHANGE_REFERENCE_DATA, + }, }, max_apdus=200, max_seconds=600.0, max_initialize_updates=6, - max_external_authenticates=2, + max_external_authenticates=4, slot_bound=True, + allow_management_key_change=True, ), } @@ -320,9 +343,20 @@ def check(self, apdu: bytes) -> Header: if (header.cla & 0xF0) == 0x80 and header.ins == INS_CTAP_MSG: raise RemotePolicyError("FIDO2 CTAP command refused", header=header.hex) if header.ins in _ALWAYS_DENIED_INS: - raise RemotePolicyError( - f"INS {header.ins:02X} is never relayed (card-holder verifier)", header=header.hex + # Setting the management key's value is administration, not a + # card-holder verifier: 9B has no card-holder retry counter, and the + # service loads it before generating a key. Every other reference, + # and every other instruction in this set, stays denied. + managing_9b = ( + header.ins == INS_CHANGE_REFERENCE_DATA + and header.p2 == MANAGEMENT_KEY_REF + and rules.allow_management_key_change ) + if not managing_9b: + raise RemotePolicyError( + f"INS {header.ins:02X} is never relayed (card-holder verifier)", + header=header.hex, + ) if header.ins == INS_EXTERNAL_AUTHENTICATE and rules.max_external_authenticates == 0: raise RemotePolicyError( f"EXTERNAL AUTHENTICATE is not allowed during {self.op}; a failed attempt " diff --git a/tests/unit/test_remote_policy.py b/tests/unit/test_remote_policy.py index 1af9748..bd6f5f5 100644 --- a/tests/unit/test_remote_policy.py +++ b/tests/unit/test_remote_policy.py @@ -157,6 +157,35 @@ def test_card_holder_verifier_commands_are_never_relayed(op, ins): policy.check(apdu(0x00, ins, 0x00, 0x80, b"\xff" * 8, le=False)) +@pytest.mark.parametrize("op", ["reset", "dev-reset", "attest"]) +@pytest.mark.parametrize("ref", sorted(pol.CARDHOLDER_REFS)) +def test_card_holder_references_are_denied_even_where_9b_is_allowed(op, ref): + policy = pol.RelayPolicy(op, slot=0x9C) if op == "attest" else pol.RelayPolicy(op) + for ins in (0x20, 0x24, 0x2C): + with pytest.raises(RemotePolicyError, match="never relayed"): + policy.check(apdu(0x04, ins, 0x00, ref, b"\xff" * 8, le=False)) + + +@pytest.mark.parametrize("op", ["reset", "dev-reset"]) +def test_setting_the_management_key_is_allowed_where_the_rules_say_so(op): + policy = pol.RelayPolicy(op) + allow(policy, select(pol.PIV_AID)) + policy.check(apdu(0x04, 0x24, 0x08, pol.MANAGEMENT_KEY_REF, b"\x00" * 24, le=False)) + + +def test_setting_the_management_key_is_allowed_during_attest(): + policy = pol.RelayPolicy("attest", slot=0x9C) + allow(policy, select(pol.PIV_AID)) + policy.check(apdu(0x04, 0x24, 0x08, pol.MANAGEMENT_KEY_REF, b"\x00" * 24, le=False)) + + +@pytest.mark.parametrize("op", pol.READ_ONLY_OPS) +def test_read_only_operations_may_not_touch_the_management_key(op): + policy = pol.RelayPolicy(op) + with pytest.raises(RemotePolicyError, match="never relayed"): + policy.check(apdu(0x04, 0x24, 0x08, pol.MANAGEMENT_KEY_REF, b"\x00" * 24, le=False)) + + @pytest.mark.parametrize("op", pol.READ_ONLY_OPS) def test_iso_secure_messaging_class_is_refused(op): policy = pol.RelayPolicy(op) From 397e0ca5e492229f0dc226885584309a68e0c9c3 Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Fri, 25 Sep 2026 15:19:37 +0200 Subject: [PATCH 4/8] Make the derived key the default for remote reset and dev-reset The service authenticates to the card manager with one of two key tiers: the card's own key, derived per card from its identity, or the publicly known GlobalPlatform default. Until now the command took `--fused` to select the derived key and sent the default otherwise, so the safe production case was the one the caller had to remember to ask for. The two mistakes are not symmetric. Forgetting the flag on a development card costs a failed authentication and nothing else; forgetting it on a production card presents a published key to a card that should only ever answer to its own. Defaults should fail safe, so the flag is inverted: the derived key is the default, and `--default-keys` opts into the development path. The name matches the flag the rest of the tool already uses for the publicly known GlobalPlatform test keys. The parameter is sent to the service explicitly either way, so the command's behaviour does not depend on which default the service applies when it is absent. Co-Authored-By: Claude Fable 5.1 --- docs/remote/remote-commands.rst | 13 +++--- docs/remote/remote-guide.rst | 22 +++++----- src/cryptnox_id_cli/cli/commands/remote.py | 47 +++++++++++++--------- tests/unit/test_remote_gates.py | 19 ++++++--- 4 files changed, 61 insertions(+), 40 deletions(-) diff --git a/docs/remote/remote-commands.rst b/docs/remote/remote-commands.rst index 05cc411..ebd8229 100644 --- a/docs/remote/remote-commands.rst +++ b/docs/remote/remote-commands.rst @@ -76,7 +76,7 @@ Reset .. code-block:: text - remote reset [--fused] [--i-understand-this-is-irreversible] + remote reset [--default-keys] [--i-understand-this-is-irreversible] IRREVERSIBLY wipe and reinstall the PIV function: the service deletes the PIV applet, its package and its @@ -86,7 +86,7 @@ Reset phrase RESET-PIV must be typed; non-interactively the flag is required. --yes is not accepted. - remote dev-reset [--fused] [--i-understand-this-is-irreversible] + remote dev-reset [--default-keys] [--i-understand-this-is-irreversible] Development cards only. As reset, but the card manager and the PIV security domain are left on the public @@ -96,10 +96,11 @@ Reset phrase is DEV-RESET-PIV. Options: - --fused the card is a fused production card: the service - derives its per-card card-manager key. On a card that - is not fused the derived key is wrong and the failed - authentication costs a card-management retry. + --default-keys development cards only: the service authenticates + with the publicly known GlobalPlatform default key + instead of deriving this card's own. A production + card refuses it, and the failed authentication costs + a card-management retry. Before asking for consent, both commands read and show what the card holds: the PIV personalization state and data objects, the PIV security domain and diff --git a/docs/remote/remote-guide.rst b/docs/remote/remote-guide.rst index 056e33f..ffacde7 100644 --- a/docs/remote/remote-guide.rst +++ b/docs/remote/remote-guide.rst @@ -87,15 +87,19 @@ Operations genuine until it is reset through the service again. The development access credential comes from ``CRYPTNOX_REMOTE_DEV_TOKEN`` only. -The ``--fused`` flag --------------------- - -A fused production card has its card manager locked to a per-card key. The -service derives that key when ``--fused`` is passed and uses the shared -default otherwise. The tool cannot tell the two apart without a key, so the -flag is the holder's statement. A wrong statement makes the service present -the wrong key; the failed authentication costs one of the card's bounded -card-management retries, and the relay stops there. +The ``--default-keys`` flag +--------------------------- + +By default the service authenticates to the card manager with the card's own +key, derived per card from its identity. That is the state a card leaves the +factory in. A development card, or one deliberately put on the published +GlobalPlatform default key, needs ``--default-keys`` instead; without it the +service presents a derived key the card does not hold, the authentication +fails, and one of the card's bounded card-management retries is spent. The +flag is the caller's statement about the card, so the default is the safe one: +forgetting it on a development card costs a failed authentication and nothing +else, while the reverse mistake on a production card would present a published +key to a card that should only ever answer to its own. What is sent to the service --------------------------- diff --git a/src/cryptnox_id_cli/cli/commands/remote.py b/src/cryptnox_id_cli/cli/commands/remote.py index 057d7c9..05d4099 100644 --- a/src/cryptnox_id_cli/cli/commands/remote.py +++ b/src/cryptnox_id_cli/cli/commands/remote.py @@ -339,15 +339,15 @@ def inspect(ctx: click.Context) -> None: # --------------------------------------------------------------------------- # # Destructive operations # # --------------------------------------------------------------------------- # -_FUSED_HELP = ( - "The card is a fused production card: the service derives its per-card " - "card-manager key. On a card that is not fused the derived key is wrong and the " - "failed authentication costs a card-management retry. Omit unless certain." +_DEFAULT_KEYS_HELP = ( + "Development cards only: have the service authenticate with the publicly known " + "GlobalPlatform default key instead of deriving this card's own. A production card " + "will refuse it, and the failed authentication costs a card-management retry." ) def _confirm_destructive( - app: AppContext, op: str, phrase: str, understood: bool, card: _Card, fused: bool + app: AppContext, op: str, phrase: str, understood: bool, card: _Card, default_keys: bool ) -> dict[str, object]: """Show what is about to be destroyed and obtain consent. Returns the inventory.""" before = _inventory(card) @@ -363,11 +363,11 @@ def _confirm_destructive( "including the wallet. The card is not genuine again until it is reset through " "the service." ) - if fused: + if default_keys: app.out.warn( - "--fused: the service will derive this card's per-card card-manager key. If " - "the card is not fused, that key is wrong and the failed authentication costs " - "a card-management retry." + "--default-keys: the service will authenticate with the publicly known " + "GlobalPlatform default key. A production card holds its own derived key and " + "will refuse this, and the failed authentication costs a card-management retry." ) if not app.json: _print_inventory(app.out.console, "Before", before) @@ -382,11 +382,12 @@ def _confirm_destructive( def _run_destructive( - ctx: click.Context, op: str, *, fused: bool, understood: bool, phrase: str + ctx: click.Context, op: str, *, default_keys: bool, understood: bool, phrase: str ) -> None: app: AppContext = ctx.obj url, _production = _endpoint(op) # refuses any override for these operations - params: dict[str, object] = {"fused": fused} + # Sent explicitly either way, so behaviour never depends on the service's own default. + params: dict[str, object] = {"fused": not default_keys} if op == "dev-reset": credential = resolve_secret( redactor=app.redactor, @@ -397,7 +398,7 @@ def _run_destructive( card = _open_card(app) try: - before = _confirm_destructive(app, op, phrase, understood, card, fused) + before = _confirm_destructive(app, op, phrase, understood, card, default_keys) app.out.note(f"remote {op} via {url}") report = _relay(app, ctx, card, op, params, url) after = _inventory(card) @@ -413,7 +414,7 @@ def _run_destructive( payload: dict[str, object] = { "op": op, "endpoint": url, - "fused": fused, + "fused": not default_keys, "card": card_info, "before": before, "result": report.result, @@ -433,7 +434,7 @@ def human(c: Console) -> None: @command.command("reset") -@click.option("--fused", is_flag=True, help=_FUSED_HELP) +@click.option("--default-keys", "default_keys", is_flag=True, help=_DEFAULT_KEYS_HELP) @click.option( "--i-understand-this-is-irreversible", "understood", @@ -441,7 +442,7 @@ def human(c: Console) -> None: help="Required for non-interactive use.", ) @click.pass_context -def reset(ctx: click.Context, fused: bool, understood: bool) -> None: +def reset(ctx: click.Context, default_keys: bool, understood: bool) -> None: """IRREVERSIBLY wipe and reinstall the PIV function through the service. The service deletes the PIV applet, its package and its security domain, @@ -450,11 +451,13 @@ def reset(ctx: click.Context, fused: bool, understood: bool) -> None: key and certificate is destroyed. The other card functions are outside the operation. """ - _run_destructive(ctx, "reset", fused=fused, understood=understood, phrase=RESET_PHRASE) + _run_destructive( + ctx, "reset", default_keys=default_keys, understood=understood, phrase=RESET_PHRASE + ) @command.command("dev-reset") -@click.option("--fused", is_flag=True, help=_FUSED_HELP) +@click.option("--default-keys", "default_keys", is_flag=True, help=_DEFAULT_KEYS_HELP) @click.option( "--i-understand-this-is-irreversible", "understood", @@ -462,7 +465,7 @@ def reset(ctx: click.Context, fused: bool, understood: bool) -> None: help="Required for non-interactive use.", ) @click.pass_context -def dev_reset(ctx: click.Context, fused: bool, understood: bool) -> None: +def dev_reset(ctx: click.Context, default_keys: bool, understood: bool) -> None: """IRREVERSIBLY wipe the PIV function and leave the card on the public default keys. For development cards only. Needs the development access credential in @@ -471,7 +474,13 @@ def dev_reset(ctx: click.Context, fused: bool, understood: bool) -> None: can be pre-personalized and personalized locally; the card is not genuine until reset through the service again. """ - _run_destructive(ctx, "dev-reset", fused=fused, understood=understood, phrase=DEV_RESET_PHRASE) + _run_destructive( + ctx, + "dev-reset", + default_keys=default_keys, + understood=understood, + phrase=DEV_RESET_PHRASE, + ) # --------------------------------------------------------------------------- # diff --git a/tests/unit/test_remote_gates.py b/tests/unit/test_remote_gates.py index 1a23b07..5564612 100644 --- a/tests/unit/test_remote_gates.py +++ b/tests/unit/test_remote_gates.py @@ -166,7 +166,8 @@ def test_reset_payload_carries_before_after_and_params(wired): result = run(["--json", "remote", "reset", "--i-understand-this-is-irreversible"]) assert result.exit_code == 0, result.output payload = json.loads(result.stdout) - assert channel.sent[0] == {"type": "hello", "op": "reset", "params": {"fused": False}} + # The derived key is the default; only --default-keys turns it off. + assert channel.sent[0] == {"type": "hello", "op": "reset", "params": {"fused": True}} assert payload["before"]["piv_ssd_key_versions"] == [1] assert payload["before"]["piv_state"] == "PivNotPresent" assert payload["after"]["piv_ssd_key_versions"] == [1] @@ -175,12 +176,18 @@ def test_reset_payload_carries_before_after_and_params(wired): assert payload["result"]["wiped_and_reprovisioned"] is True -def test_fused_is_passed_through_and_warned(wired): +def test_default_keys_selects_the_dev_path_and_warns(wired): channel = wired["arm"]([OK_RESULT]) - result = run(["remote", "reset", "--fused", "--i-understand-this-is-irreversible"]) + result = run(["remote", "reset", "--default-keys", "--i-understand-this-is-irreversible"]) assert result.exit_code == 0, result.output - assert channel.sent[0]["params"] == {"fused": True} - assert "--fused" in result.output + assert channel.sent[0]["params"] == {"fused": False} + assert "--default-keys" in result.output + + +def test_fused_flag_no_longer_exists(wired): + wired["arm"]([OK_RESULT]) + result = run(["remote", "reset", "--fused", "--i-understand-this-is-irreversible"]) + assert result.exit_code == 2 # click usage error def test_dev_reset_sends_the_credential_from_the_environment_only(wired, monkeypatch): @@ -190,7 +197,7 @@ def test_dev_reset_sends_the_credential_from_the_environment_only(wired, monkeyp ["--json", "remote", "dev-reset", "--i-understand-this-is-irreversible"], ) assert result.exit_code == 0, result.output - assert channel.sent[0]["params"] == {"fused": False, "dev_token": "dev-credential-value"} + assert channel.sent[0]["params"] == {"fused": True, "dev_token": "dev-credential-value"} assert "dev-credential-value" not in result.output From 151cb23649832df1ae160f9d818f1def9a6d5d98 Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Fri, 25 Sep 2026 17:04:48 +0200 Subject: [PATCH 5/8] Clear the CodeQL findings on the remote package Six findings on the pull request, none of them a defect, each made unambiguous rather than suppressed. The FrameChannel protocol methods carried `...` bodies, which the analyser reads as statements without effect; they now carry docstrings, which also document the contract. The empty `except` in the certificate pre-read has the comment explaining why an unreadable applet is not an error there. In the tests, a helper named `delete` built a DELETE APDU and was called inside an `assert`, which the analyser flags as a possible side effect; it is `delete_apdu` now. A test that raised inside a `with pytest.raises` block looked to the analyser as if the assertion after it could never run; the raise moved into a nested function so the flow is plain. Co-Authored-By: Claude Fable 5.1 --- src/cryptnox_id_cli/cli/commands/remote.py | 2 ++ src/cryptnox_id_cli/remote/channel.py | 9 ++++++--- tests/unit/test_remote_channel.py | 9 +++++++-- tests/unit/test_remote_policy.py | 12 ++++++------ 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/cryptnox_id_cli/cli/commands/remote.py b/src/cryptnox_id_cli/cli/commands/remote.py index 05d4099..65717de 100644 --- a/src/cryptnox_id_cli/cli/commands/remote.py +++ b/src/cryptnox_id_cli/cli/commands/remote.py @@ -515,6 +515,8 @@ def _read_certificates(card: _Card, slot: int) -> tuple[bytes | None, bytes | No slot_cert = _read_cert_der(piv, slot_object) if slot_object else None attestation = _read_cert_der(piv, ATTESTATION_CONTAINER) except CryptnoxError: + # An applet that is absent, unselectable or refusing the read simply has + # no certificate to report; the operation goes ahead either way. pass _read_cplc(card.session) return slot_cert, attestation diff --git a/src/cryptnox_id_cli/remote/channel.py b/src/cryptnox_id_cli/remote/channel.py index 0765a62..adf02b4 100644 --- a/src/cryptnox_id_cli/remote/channel.py +++ b/src/cryptnox_id_cli/remote/channel.py @@ -48,11 +48,14 @@ class FrameChannel(Protocol): """A bidirectional stream of text messages.""" - def send(self, message: str) -> None: ... + def send(self, message: str) -> None: + """Send one text message.""" - def recv(self, timeout: float | None = None) -> str: ... + def recv(self, timeout: float | None = None) -> str: + """Receive one text message, waiting at most ``timeout`` seconds.""" - def close(self) -> None: ... + def close(self) -> None: + """Close the channel; safe to call more than once.""" def validate_url(url: str, *, allow_insecure_loopback: bool = False) -> str: diff --git a/tests/unit/test_remote_channel.py b/tests/unit/test_remote_channel.py index f8f1511..80d0c5d 100644 --- a/tests/unit/test_remote_channel.py +++ b/tests/unit/test_remote_channel.py @@ -179,8 +179,13 @@ def fake_connect(url, **kwargs): def test_open_channel_closes_on_exit_even_after_an_error(monkeypatch): conn = _FakeConnection() monkeypatch.setattr("websockets.sync.client.connect", lambda url, **kw: conn) - with pytest.raises(RuntimeError), ch.open_channel("wss://x"): - raise RuntimeError("inside") + + def use_and_fail() -> None: + with ch.open_channel("wss://x"): + raise RuntimeError("inside") + + with pytest.raises(RuntimeError, match="inside"): + use_and_fail() assert conn.closed is True diff --git a/tests/unit/test_remote_policy.py b/tests/unit/test_remote_policy.py index bd6f5f5..0381886 100644 --- a/tests/unit/test_remote_policy.py +++ b/tests/unit/test_remote_policy.py @@ -293,7 +293,7 @@ def test_refusal_carries_the_header_for_review(): SSD_PACKAGE_AID = bytes.fromhex("A0000001515350") -def delete(aid: bytes, cla: int = 0x84, mac: bytes = b"\x00" * 8) -> bytes: +def delete_apdu(aid: bytes, cla: int = 0x84, mac: bytes = b"\x00" * 8) -> bytes: data = bytes([0x4F, len(aid)]) + aid + mac return bytes([cla, 0xE4, 0x00, 0x00, len(data)]) + data + b"\x00" @@ -309,9 +309,9 @@ def test_reset_follows_the_documented_flow(op): allow(policy, INIT_UPDATE) allow(policy, EXT_AUTH) allow(policy, wrapped(0xF2, 0x40, 0x00)) # GET STATUS - allow(policy, delete(pol.PIV_AID)) - allow(policy, delete(PIV_PACKAGE_AID)) - allow(policy, delete(pol.PIV_SSD_AID)) + allow(policy, delete_apdu(pol.PIV_AID)) + allow(policy, delete_apdu(PIV_PACKAGE_AID)) + allow(policy, delete_apdu(pol.PIV_SSD_AID)) allow(policy, wrapped(0xE6, 0x02, 0x00)) # INSTALL for load for _ in range(10): allow(policy, wrapped(0xE8, 0x00, 0x00, n=200)) # LOAD blocks @@ -335,7 +335,7 @@ def test_reset_may_only_delete_piv_things(op, aid): allow(policy, INIT_UPDATE) allow(policy, EXT_AUTH) with pytest.raises(RemotePolicyError, match="DELETE of") as info: - policy.check(delete(aid)) + policy.check(delete_apdu(aid)) assert info.value.header == "84E40000" @@ -401,7 +401,7 @@ def test_reset_cannot_select_other_card_functions(op, aid): def test_delete_target_parsing(): - assert pol.delete_target(delete(pol.PIV_AID)) == pol.PIV_AID + assert pol.delete_target(delete_apdu(pol.PIV_AID)) == pol.PIV_AID assert pol.delete_target(wrapped(0xE4)) is None # opaque assert pol.delete_target(bytes.fromhex("84E40000034F01AA")) is None # implausible length From cfc3746e80e6bb6310356ddebdf351152c4f0d1b Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Wed, 7 Oct 2026 12:53:57 +0200 Subject: [PATCH 6/8] Move the remote documentation to its own change The guide, the command reference and the sections added to the README and to the exit-codes, JSON-output and troubleshooting pages leave this branch. They follow in a separate change, reviewed on its own, based on this one. The command map in cli-basics keeps a row naming the five commands, which is what the command-reference drift guard requires of any change that adds a command; the cross-reference to the command reference page goes with the page. Co-Authored-By: Claude Fable 5.1 --- README.md | 12 ---- docs/cli-basics.rst | 5 +- docs/exit-codes.rst | 20 ------ docs/index.rst | 7 -- docs/json-output.rst | 14 ---- docs/remote/remote-commands.rst | 117 ------------------------------- docs/remote/remote-guide.rst | 118 -------------------------------- docs/troubleshooting.rst | 28 -------- 8 files changed, 3 insertions(+), 318 deletions(-) delete mode 100644 docs/remote/remote-commands.rst delete mode 100644 docs/remote/remote-guide.rst diff --git a/README.md b/README.md index b151d8a..5032cfe 100644 --- a/README.md +++ b/README.md @@ -139,18 +139,6 @@ cryptnox-id doctor # PC/SC service, reader, per-function reachability cryptnox-id shell # run subcommands without re-typing the prefix ``` -### 6. Use the Cryptnox remote PIV service - -```bash -cryptnox-id remote inspect # what the service sees: applet, security domain, key versions -cryptnox-id remote attest --slot 9C # a key generated through the service, with its attestation -cryptnox-id remote reset # wipe and reinstall the PIV function (irreversible, gated) -``` - -The card stays in your reader; the tool relays the service's commands under a -policy that keeps the other card functions out of reach, and shows what the -service asserted apart from what it verified locally. - --- ## Documentation diff --git a/docs/cli-basics.rst b/docs/cli-basics.rst index 57335ca..e906b80 100644 --- a/docs/cli-basics.rst +++ b/docs/cli-basics.rst @@ -71,9 +71,10 @@ Top-level commands * - ``report card`` / ``report piv`` / ``report mifare`` / ``report fido`` / ``report genuine`` / ``report full`` - Secret-safe JSON reports (``--out FILE``). - * - ``remote authenticate`` / ``remote inspect`` + * - ``remote authenticate`` / ``remote inspect`` / ``remote attest`` / + ``remote reset`` / ``remote dev-reset`` - Operations run through the Cryptnox remote PIV service, with the card - in the local reader; see :doc:`/remote/remote-commands`. + in the local reader. * - ``shell`` - Interactive prompt: run subcommands without the ``cryptnox-id`` prefix (type ``piv info``); global options passed when launching carry into every diff --git a/docs/exit-codes.rst b/docs/exit-codes.rst index 662b6a9..8a67096 100644 --- a/docs/exit-codes.rst +++ b/docs/exit-codes.rst @@ -51,23 +51,3 @@ machine token (see :doc:`/json-output`). * - 11 - ``ev2_error`` - DESFire secure-messaging (session/MAC) failure. - * - 12 - - ``remote_error`` - - A remote PIV service operation failed for a reason without a more - specific class. - * - 13 - - ``remote_connect`` - - The remote PIV service could not be reached, or the TLS handshake - failed. - * - 14 - - ``remote_protocol`` - - The remote PIV service sent something the client cannot interpret - safely. - * - 15 - - ``remote_policy`` - - The remote PIV service asked for a card command the relay policy - refuses to send. The message names the command header. - * - 16 - - ``remote_failed`` - - The exchange completed, but the remote PIV service reported the - operation as failed. diff --git a/docs/index.rst b/docs/index.rst index 8c1085e..8c3342e 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -62,12 +62,5 @@ driven by one tool: |cli|. Guide Commands -.. toctree:: - :maxdepth: 2 - :caption: Remote service - - Guide - Commands - .. API Reference — RESERVED: autodoc of the Python package, if/when it is publicly pip-installable (see the internal documentation plan, decision D4). diff --git a/docs/json-output.rst b/docs/json-output.rst index 832fc1d..7415a0b 100644 --- a/docs/json-output.rst +++ b/docs/json-output.rst @@ -159,17 +159,3 @@ re-present the card, and check the reader passes native DESFire APDUs. A section may also report ``{"state": "Unknown", "error": ""}`` when a probe fails. Nothing in any report contains a PIN, key, or private material — ``safe_to_share`` is always ``true``. - -Remote service operations -------------------------- - -``remote`` commands return one object with a fixed top level: ``op``, -``endpoint``, ``card`` (reader, ATR, CPLC UID and serial, all read locally), -``result`` (the service's own result object, passed through unchanged), -``verified_locally`` (each cross-check as ``true``, ``false`` or ``null`` when -the service reported nothing to compare) and ``relay`` (commands relayed, -seconds, proof-of-work bits, the service's log lines). ``reset`` and -``dev-reset`` add ``before`` and ``after`` inventories; ``attest`` adds -``verification`` with every binding checked on the returned certificate. -Fields under ``result`` are the service's assertions; only ``verified_locally``, -``before``, ``after`` and ``verification`` are the tool's own findings. diff --git a/docs/remote/remote-commands.rst b/docs/remote/remote-commands.rst deleted file mode 100644 index ebd8229..0000000 --- a/docs/remote/remote-commands.rst +++ /dev/null @@ -1,117 +0,0 @@ -Remote service commands -======================= - -The ``remote`` commands run operations through the Cryptnox remote PIV -service. The card stays in the local reader; |cli| relays the commands the -service sends to the card and reports the outcome. What the service asserts -and what |cli| verified locally are shown apart. The service, the relay policy -and what each operation does to the card are described in -:doc:`/remote/remote-guide`. - -Every command opens the local reader first, then connects to the service over -TLS and answers a small proof-of-work before any command is relayed. Only the -PIV function and its security domains are reachable through the relay; the -other card functions are refused by the relay policy. - -Group options -------------- - -.. code-block:: text - - remote [--full-transcript] COMMAND - - Options: - --full-transcript write relayed commands to --apdu-log and --verbose - without the extra masking the relay applies by - default; the standard secret masking still applies - -Identification and inspection ------------------------------ - -.. code-block:: text - - remote authenticate identify the card to the service: ATR and CPLC UID, - nothing else; key-free on both sides - - remote inspect probe the PIV function through the service: whether - the PIV applet and its security domain are present - and which key versions they carry; key-free, the - relay refuses any authentication attempt - -Both commands report the card as read locally (reader, ATR, CPLC UID), the -service's result as the service asserted it, and a cross-check of the fields -the two have in common. - -Key attestation ---------------- - -.. code-block:: text - - remote attest [--slot 9C] [--algorithm ECCP256] [--ca-obj HEX] [--out FILE] - - generate a key in the slot through the service and - receive its key-attestation certificate; the service - also stores the certificate in the card's attestation - container. The slot's previous key and the container's - previous certificate are replaced: when either holds - something, the command asks, or needs --yes. - - Options: - --slot HEX 9A, 9C, 9D, 9E or a retired slot 82 to 95 (default 9C) - --algorithm NAME ECCP256 (default), ECCP384, RSA2048, RSA3072, RSA4096 - --ca-obj HEX signing sub-CA in the service; omit for the - key-attestation CA - --out FILE write the returned certificate (PEM) - -The returned certificate is verified locally before success is reported: the -chain must end at a pinned trust anchor, the subject serial number must be the -card's CPLC UID, the ``cryptnoxPivSlot`` extension must name the requested -slot, the trust model must be the server-driven value, and the certified -public key must equal the certificate read back from the card's attestation -container. That the key was generated on the card is the service's claim; the -tool cannot witness it and says so. - -Reset ------ - -.. code-block:: text - - remote reset [--default-keys] [--i-understand-this-is-irreversible] - - IRREVERSIBLY wipe and reinstall the PIV function: the - service deletes the PIV applet, its package and its - security domain, reinstalls the applet, recreates the - security domain and loads the card's keys. Every PIV - key and certificate is destroyed. Interactively the - phrase RESET-PIV must be typed; non-interactively the - flag is required. --yes is not accepted. - - remote dev-reset [--default-keys] [--i-understand-this-is-irreversible] - - Development cards only. As reset, but the card manager - and the PIV security domain are left on the public - default key, so the PIV function can be worked on - locally. Needs the development access credential in - $CRYPTNOX_REMOTE_DEV_TOKEN (never an option). The - phrase is DEV-RESET-PIV. - - Options: - --default-keys development cards only: the service authenticates - with the publicly known GlobalPlatform default key - instead of deriving this card's own. A production - card refuses it, and the failed authentication costs - a card-management retry. - -Before asking for consent, both commands read and show what the card holds: -the PIV personalization state and data objects, the PIV security domain and -its key versions, and the state of the FIDO2 and genuineness functions, which -stay outside the operation. The same inventory is read again afterwards and -shown next to the service's result. - -Endpoint --------- - -The service endpoint is fixed to ``wss://piv.cryptnox.com``. For development -against a local stand-in, ``CRYPTNOX_REMOTE_URL`` redirects the identification -and inspection commands only; it must be a ``wss://`` URL, or ``ws://`` to -loopback. ``reset`` and ``dev-reset`` refuse to run while it is set. diff --git a/docs/remote/remote-guide.rst b/docs/remote/remote-guide.rst deleted file mode 100644 index ffacde7..0000000 --- a/docs/remote/remote-guide.rst +++ /dev/null @@ -1,118 +0,0 @@ -Remote service guide -==================== - -Some PIV lifecycle operations need keys that never leave Cryptnox: the key -that makes a card genuine, and the certificate authority that attests keys -generated on it. The remote PIV service holds those keys in a hardware -security module and performs the operation over a relayed command channel, -with the card in the holder's own reader. - -How a relayed operation works ------------------------------ - -1. |cli| opens the local reader and reads the card's ATR and CPLC serial. -2. It connects to the service over TLS and names the operation. -3. The service issues a small proof-of-work; |cli| answers it. This is an - anti-abuse throttle, not a credential. -4. The service sends card commands one at a time. |cli| checks each one - against the relay policy, transmits it, and returns the card's answer - verbatim, status word included. |cli| never sends a command of its own and - never reassembles a chained response; the service sees the card exactly as - a local reader would. -5. The service returns a result. |cli| shows it as the service's assertion and - adds what it verified locally. - -The relay policy ----------------- - -The card carries more than the PIV function. A relay that executed whatever -it was sent would expose the wallet, the FIDO2 passkeys and the genuineness -key to the service. The relay policy is an allow-list, per operation, over -what a command header reveals: the selected applet, the class byte, the -instruction, and for a SELECT the target application. Anything not listed is -refused, the operation stops, and the refusal names the command header. - -The policy also caps the number of commands and the time an operation may -take, limits secure-channel authentication attempts per security domain, and -never relays a card-holder PIN or PUK command. Inside a secure channel only -the class and instruction bytes stay in the clear, so the policy fences which -security domain the channel was opened to and which instructions pass through -it; it cannot see what an encrypted command carries. - -Operations ----------- - -``authenticate`` - Reads the ATR and CPLC serial. Key-free. Allows only card-manager reads. - -``inspect`` - Probes whether the PIV applet and its security domain are present and which - key versions they carry. Key-free: an authentication attempt from the - service is refused before it reaches the card. - -``attest`` - Generates a key in one slot and returns a key-attestation certificate for - it, signed by the Cryptnox key-attestation CA and stored in the card's - attestation container as well. The relay policy admits the applet's admin - channel for the key-object setup, the management-key handshake (never a - slot key), key generation for the requested slot only, and the certificate - write. The tool then verifies the certificate against the pinned anchors and - binds it to the card, the slot and the certificate stored on the card. The - generation itself happening on the card is the service's claim; nothing the - relay sees authenticates it. - -``reset`` - Wipes and reinstalls the PIV function. The service authenticates to the - card manager, deletes the PIV applet, its package and its security domain, - loads the applet again, recreates the security domain and loads the card's - keys, then verifies the result. Every PIV key and certificate is destroyed; - the keys were never exportable, so nothing can bring them back. Interactive - use requires typing ``RESET-PIV``; non-interactive use requires - ``--i-understand-this-is-irreversible``. ``--yes`` is not accepted. - - The relay policy for this operation admits card content management through - the card manager only, lets a DELETE name nothing but the PIV instance, its - package and its security domain where the command travels in the clear, - caps authentication attempts per security domain, and stops at the first - failed authentication so no further attempt can spend a card-management - retry. Inside an encrypted channel the policy sees instructions, not - targets; that limit is real and is why the FIDO2 and genuineness state is - read before and after. - -``dev-reset`` - As ``reset``, for development cards, leaving the card manager and the PIV - security domain on the publicly documented GlobalPlatform key so the PIV - function can be pre-personalized and personalized locally. Anyone with a - reader can then install or delete applets on the card, and the card is not - genuine until it is reset through the service again. The development access - credential comes from ``CRYPTNOX_REMOTE_DEV_TOKEN`` only. - -The ``--default-keys`` flag ---------------------------- - -By default the service authenticates to the card manager with the card's own -key, derived per card from its identity. That is the state a card leaves the -factory in. A development card, or one deliberately put on the published -GlobalPlatform default key, needs ``--default-keys`` instead; without it the -service presents a derived key the card does not hold, the authentication -fails, and one of the card's bounded card-management retries is spent. The -flag is the caller's statement about the card, so the default is the safe one: -forgetting it on a development card costs a failed authentication and nothing -else, while the reverse mistake on a production card would present a published -key to a card that should only ever answer to its own. - -What is sent to the service ---------------------------- - -The card's ATR and CPLC serial identify the card to the service. They are the -same values any reader can obtain from the card without a key. Nothing else -about the card holder is sent. - -Transcripts ------------ - -With ``--apdu-log`` or ``--verbose``, relayed commands are written with their -data masked unless the instruction is a known-harmless read, because the -service chose the command and |cli| cannot vouch for what it carries. -``remote --full-transcript`` lifts that extra masking for troubleshooting; the -standard masking of secret-bearing commands still applies. diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index a8b7d5f..ad913e3 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -117,34 +117,6 @@ throwaway dev root: ``genuine verify --anchors ``. chain. It does **not** verify the per-card ISD or PIV-SSD keys (KDF/HSM-derived) — this tool has no access to those secrets, and the output says so. -Remote service --------------- - -``remote`` commands exit with a code that names the failure class (see -:doc:`/exit-codes`): - -- **13** ``remote_connect``: the service could not be reached or the TLS - handshake failed. Check the network; the endpoint is fixed to - ``wss://piv.cryptnox.com``. With ``CRYPTNOX_REMOTE_URL`` set, only - ``authenticate`` and ``inspect`` run, and only to ``wss://`` or loopback. -- **14** ``remote_protocol``: the service sent a frame the tool cannot - interpret. Update the tool; if the newest version fails the same way, - report it with the message. -- **15** ``remote_policy``: the service asked for a card command the relay - policy refuses; the message names the command header. Nothing was sent to - the card for that command and the operation stopped. This is the tool - protecting the card; report the header so the policy can be reviewed. -- **16** ``remote_failed``: the exchange completed and the service reported - the operation as failed; the error object carries the service's result. - A per-card rate limit exists; wait before retrying. - -``remote reset`` and ``remote dev-reset`` refuse to run non-interactively -without ``--i-understand-this-is-irreversible``, and ``--yes`` is not -accepted for them. ``remote attest`` refuses non-interactively without -``--yes`` when the slot or the attestation container already holds a -certificate. - - yubico-piv-tool interop ------------------------- From 40fd71e45bbc0cb1ffcbe0d2a942885bfa9d09c2 Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Mon, 12 Oct 2026 16:43:51 +0200 Subject: [PATCH 7/8] Close two relay policy gaps found in review A DELETE is read in both the short and the extended length encoding, and one whose target cannot be read is refused unless the channel is known to encrypt command data. The policy learns that from the security level a successful EXTERNAL AUTHENTICATE asked for and forgets it when the channel ends. Before, an unreadable DELETE passed, so the same AID was refused in one encoding and admitted in the other. A SELECT answered 61XX moves the selected-applet tracking as 9000 does. An answer that leaves the selection uncertain, a warning or a wrong Le to re-send, puts the policy in a state that admits nothing but another SELECT. A selectable AID with no context is refused before it is sent. The ADR states that the certificate write is not slot-bound, since PUT DATA names its object in the data, and the docstring of the attestation verdict says all four bindings are required. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 6 +- .../0003-relay-for-the-remote-piv-service.md | 17 ++- src/cryptnox_id_cli/remote/attest_verify.py | 2 +- src/cryptnox_id_cli/remote/policy.py | 124 ++++++++++++---- tests/unit/test_remote_policy.py | 133 ++++++++++++++++-- 5 files changed, 233 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa58c86..0be924c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,9 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 with the card's PIV state, data objects, security-domain key versions and neighbouring functions read locally before consent and again afterwards. The relay policy for these operations admits card content management - through the card manager only, restricts a readable DELETE to the PIV - instance, package and security domain, and stops at the first failed - authentication. + through the card manager only, restricts a DELETE to the PIV instance, + package and security domain (refusing one it cannot read unless the + channel encrypts commands), and stops at the first failed authentication. - `remote attest`: generate a key in a slot through the service and receive its key-attestation certificate. The certificate is verified locally against the pinned anchors and bound to the card's CPLC UID, the requested slot, the diff --git a/docs/adr/0003-relay-for-the-remote-piv-service.md b/docs/adr/0003-relay-for-the-remote-piv-service.md index a3e6b7e..b0140c0 100644 --- a/docs/adr/0003-relay-for-the-remote-piv-service.md +++ b/docs/adr/0003-relay-for-the-remote-piv-service.md @@ -44,18 +44,23 @@ per operation: which applet may be selected (the PIV applet and its security domain; the card manager only where card content management is the point), which instructions may pass in each of those contexts, how many INITIALIZE UPDATE and EXTERNAL AUTHENTICATE commands each security domain may see, how many -commands and seconds an operation may take. A DELETE whose data travels in the -clear may name the PIV instance, its package or its security domain and nothing -else. Key generation is bound to the slot the holder asked for, and the -management-key handshake to key 9B, so no slot key can be exercised. Card-holder -verifier commands are never relayed. A refusal ends the operation and names the +commands and seconds an operation may take. A DELETE may name the PIV instance, +its package or its security domain and nothing else; one whose target cannot be +read is refused unless the channel is known to encrypt command data. Key +generation is bound to the slot the holder asked for, and the management-key +handshake to key 9B, so no slot key can be exercised. The certificate write is +not bound: PUT DATA names its object in the data, not the header, so during an +attestation the service can write any PIV data object, and only the certificate +the tool reads back is checked. Card-holder verifier commands are never relayed. A refusal ends the operation and names the command header, so the rules can be reviewed against what the service actually needs rather than loosened in advance. Inside an encrypted channel only the class and instruction bytes stay in the clear. The policy fences which security domain the channel was opened to and which instructions pass through it; it cannot see what an encrypted command -carries, and the documentation says so. +carries, and the documentation says so. The selected applet is tracked from the +card's answers, not the service's requests; an answer that leaves the selection +uncertain admits nothing but another SELECT. The service's result is shown as the service's assertion. What the tool can verify itself, it reads from the card before and after: identity, personalization diff --git a/src/cryptnox_id_cli/remote/attest_verify.py b/src/cryptnox_id_cli/remote/attest_verify.py index 926bf99..3d6e76b 100644 --- a/src/cryptnox_id_cli/remote/attest_verify.py +++ b/src/cryptnox_id_cli/remote/attest_verify.py @@ -57,7 +57,7 @@ class AttestVerification: @property def verified(self) -> bool: - """True only when the chain holds and every binding that could be checked holds.""" + """True only when the chain holds and all four bindings were checked and hold.""" bindings = ( self.serial_number_matches_card, self.slot_matches, diff --git a/src/cryptnox_id_cli/remote/policy.py b/src/cryptnox_id_cli/remote/policy.py index 2aaf656..2a09135 100644 --- a/src/cryptnox_id_cli/remote/policy.py +++ b/src/cryptnox_id_cli/remote/policy.py @@ -14,7 +14,11 @@ security domain the channel was opened to and which instructions pass through it, and it cannot see what an encrypted command carries. Where a command's data does travel in the clear, the policy reads it: a DELETE may only name the PIV -instance, the PIV package or the PIV security domain. +instance, the PIV package or the PIV security domain, and a DELETE whose target +cannot be read is refused unless the channel is known to encrypt command data. +The selected applet is tracked from the card's answers, not the service's +requests; an answer that leaves the selection uncertain admits nothing but +another SELECT. Counters guard the other way a command can do harm without being forbidden: a failed GlobalPlatform authentication decrements a bounded retry counter, so @@ -40,9 +44,19 @@ ISD = "isd" PIV_SSD = "piv-ssd" PIV = "piv" +#: After a SELECT the card answered with neither success nor a plain failure. +UNKNOWN = "unknown" _CONTEXT_BY_AID: dict[bytes, str] = {ISD_AID: ISD, PIV_SSD_AID: PIV_SSD, PIV_AID: PIV} +#: SW1 values after which the selection may or may not have taken effect: +#: warnings (the command ran, with a caveat) and a wrong Le the card wants +#: corrected and re-sent. +_UNCERTAIN_SW1: frozenset[int] = frozenset({0x62, 0x63, 0x6C}) + +#: EXTERNAL AUTHENTICATE P1 bit requesting command encryption (C-DECRYPTION). +_SECURITY_LEVEL_C_DEC = 0x02 + #: Operations grouped by what they may do to the card. READ_ONLY_OPS = ("authenticate", "inspect") DESTRUCTIVE_OPS = ("reset", "dev-reset") @@ -249,12 +263,42 @@ def parse_header(apdu: bytes) -> Header: return Header(apdu[0], apdu[1], apdu[2], apdu[3]) +def command_data(apdu: bytes) -> bytes: + """The data field of a command, in the short or the extended encoding. + + A command whose length fields do not match its size is refused: a command + the relay cannot parse is one it cannot vouch for. + """ + header = parse_header(apdu) + body = apdu[4:] + if not body: + return b"" + if body[0] != 0 or len(body) == 1: + # Short form: Lc, data, optional Le. A lone 00 is a short Le with no data. + lc = body[0] + data = body[1 : 1 + lc] + trailer = len(body) - 1 - lc + if len(data) != lc or trailer > 1: + raise RemotePolicyError( + "command data does not match its length fields", header=header.hex + ) + return data + if len(body) == 3: + return b"" # extended Le, no data + lc = int.from_bytes(body[1:3], "big") + data = body[3 : 3 + lc] + trailer = len(body) - 3 - lc + if len(body) < 4 or lc == 0 or len(data) != lc or trailer not in (0, 2): + raise RemotePolicyError("command data does not match its length fields", header=header.hex) + return data + + def select_target(apdu: bytes) -> bytes | None: """The AID a SELECT-by-name command targets, or ``None`` if it is not one. - Only the short form with P1 = 04 (select by DF name) is recognised. Anything - else that looks like a SELECT is refused by the caller, because a SELECT the - relay cannot read is a SELECT it cannot vouch for. + Only P1 = 04 (select by DF name) is recognised. Anything else that looks like + a SELECT is refused by the caller, because a SELECT the relay cannot read is + a SELECT it cannot vouch for. An empty AID selects the default applet. """ header = parse_header(apdu) if header.ins != INS_SELECT: @@ -263,32 +307,22 @@ def select_target(apdu: bytes) -> bytes | None: raise RemotePolicyError( f"SELECT with P1={header.p1:02X} is not select-by-name; refused", header=header.hex ) - body = apdu[4:] - if not body: - return b"" # SELECT with no AID: the default applet - lc = body[0] - if lc == 0 and len(body) > 1: - raise RemotePolicyError("extended-length SELECT refused", header=header.hex) - aid = body[1 : 1 + lc] - if len(aid) != lc: - raise RemotePolicyError("SELECT data shorter than its length byte", header=header.hex) - return aid + return command_data(apdu) def delete_target(apdu: bytes) -> bytes | None: - """The AID a DELETE names, when its data travels in the clear. + """The AID a DELETE names, when its data is readable; ``None`` when it is not. - Under a secure channel with command encryption the data is opaque and this - returns ``None``; with a MAC alone the ``4F`` AID template is readable and - the MAC simply trails it. + With a MAC alone the ``4F`` AID template is in the clear and the MAC trails + it. Under command encryption the data is opaque. """ - body = apdu[5:] # past CLA INS P1 P2 Lc - if len(body) < 3 or body[0] != 0x4F: + data = command_data(apdu) + if len(data) < 3 or data[0] != 0x4F: return None - length = body[1] - if not 5 <= length <= 16 or len(body) < 2 + length: + length = data[1] + if not 5 <= length <= 16 or len(data) < 2 + length: return None - return body[2 : 2 + length] + return data[2 : 2 + length] @dataclass @@ -311,6 +345,9 @@ class RelayPolicy: initialize_updates: dict[str, int] = field(default_factory=dict) external_authenticates: dict[str, int] = field(default_factory=dict) _pending_select: bytes | None = field(default=None, init=False, repr=False) + #: Whether the open secure channel encrypts command data, from the security + #: level the last successful EXTERNAL AUTHENTICATE asked for. + _command_encryption: bool = field(default=False, init=False, repr=False) def __post_init__(self) -> None: self.rules = rules_for(self.op) @@ -372,14 +409,23 @@ def check(self, apdu: bytes) -> Header: f"SELECT of AID {target.hex().upper()} is not allowed during {self.op}", header=header.hex, ) + if target not in _CONTEXT_BY_AID: + raise RemotePolicyError( + f"SELECT of AID {target.hex().upper()} has no relay context; refused", + header=header.hex, + ) self._pending_select = target return header allowed = rules.allowed_ins.get(self.context, frozenset()) if header.ins not in allowed: + where = ( + "the selected applet is unknown" + if self.context == UNKNOWN + else f"the {self.context} is selected" + ) raise RemotePolicyError( - f"INS {header.ins:02X} is not allowed while the {self.context} is selected " - f"during {self.op}", + f"INS {header.ins:02X} is not allowed while {where} during {self.op}", header=header.hex, ) @@ -395,10 +441,19 @@ def check(self, apdu: bytes) -> Header: header, ) elif header.ins == INS_DELETE: + if rules.delete_targets is None: + raise RemotePolicyError( + f"DELETE is not allowed during {self.op}", header=header.hex + ) deleted = delete_target(apdu) - if deleted is not None and ( - rules.delete_targets is None or deleted not in rules.delete_targets - ): + if deleted is None: + if not self._command_encryption: + raise RemotePolicyError( + "DELETE names a target the relay cannot read, on a channel that does " + "not encrypt commands; refused", + header=header.hex, + ) + elif deleted not in rules.delete_targets: raise RemotePolicyError( f"DELETE of {deleted.hex().upper()} refused; only the PIV instance, " "package and security domain may be deleted", @@ -439,14 +494,20 @@ def observe(self, apdu: bytes, sw1: int, sw2: int) -> None: """Record what the card did with an allowed command.""" self.apdus += 1 header = parse_header(apdu) - ok = sw1 == 0x90 and sw2 == 0x00 + # 61XX is success with response bytes waiting for GET RESPONSE. + ok = (sw1 == 0x90 and sw2 == 0x00) or sw1 == 0x61 if self._pending_select is not None: + target, self._pending_select = self._pending_select, None if ok: - self.context = _CONTEXT_BY_AID[self._pending_select] - self._pending_select = None + self.context = _CONTEXT_BY_AID[target] + self._command_encryption = False # selecting closes any secure channel + elif sw1 in _UNCERTAIN_SW1: + self.context = UNKNOWN + self._command_encryption = False return if header.ins == INS_INITIALIZE_UPDATE: self.initialize_updates[self.context] = self.initialize_updates.get(self.context, 0) + 1 + self._command_encryption = False elif header.ins == INS_EXTERNAL_AUTHENTICATE: self.external_authenticates[self.context] = ( self.external_authenticates.get(self.context, 0) + 1 @@ -457,5 +518,6 @@ def observe(self, apdu: bytes, sw1: int, sw2: int) -> None: "another attempt can burn a retry", header=header.hex, ) + self._command_encryption = bool(header.p1 & _SECURITY_LEVEL_C_DEC) elif header.ins == INS_LOAD: self.load_blocks += 1 diff --git a/tests/unit/test_remote_policy.py b/tests/unit/test_remote_policy.py index 0381886..1042f3a 100644 --- a/tests/unit/test_remote_policy.py +++ b/tests/unit/test_remote_policy.py @@ -1,5 +1,7 @@ """The relay policy: what a hostile or confused service cannot make the card do.""" +import dataclasses + import pytest from cryptnox_id_cli.remote import policy as pol @@ -27,7 +29,10 @@ def apdu(cla: int, ins: int, p1: int = 0, p2: int = 0, data: bytes = b"", le: bo CPLC = apdu(0x80, 0xCA, 0x9F, 0x7F) KEY_INFO = apdu(0x80, 0xCA, 0x00, 0xE0) INIT_UPDATE = apdu(0x80, 0x50, 0x00, 0x00, bytes(8)) -EXT_AUTH = apdu(0x84, 0x82, 0x00, 0x00, bytes(16), le=False) +# Security level in P1: 01 = C-MAC only (what the service uses on the card +# manager), 03 = C-MAC + command encryption (what it uses on the applet). +EXT_AUTH = apdu(0x84, 0x82, 0x01, 0x00, bytes(16), le=False) +EXT_AUTH_ENC = apdu(0x84, 0x82, 0x03, 0x00, bytes(16), le=False) PIV_GET_DATA = apdu(0x00, 0xCB, 0x3F, 0xFF, bytes.fromhex("5C035FC102")) @@ -64,14 +69,36 @@ def test_select_by_file_id_is_refused_not_ignored(): pol.select_target(bytes.fromhex("00A40000023F00")) -def test_extended_length_select_is_refused(): - with pytest.raises(RemotePolicyError, match="extended-length"): - pol.select_target(bytes.fromhex("00A40400000009") + pol.PIV_SSD_AID) +def test_extended_length_select_is_read_like_the_short_form(): + extended = bytes.fromhex("00A40400000009") + pol.PIV_SSD_AID + assert pol.select_target(extended) == pol.PIV_SSD_AID + assert pol.select_target(extended + b"\x00\x00") == pol.PIV_SSD_AID + +def test_command_data_reads_both_encodings(): + assert pol.command_data(bytes.fromhex("80CA9F7F")) == b"" + assert pol.command_data(bytes.fromhex("80CA9F7F00")) == b"" # short Le, no data + assert pol.command_data(bytes.fromhex("80CA9F7F000000")) == b"" # extended Le, no data + assert pol.command_data(bytes.fromhex("00A404000102")) == b"\x02" + assert pol.command_data(bytes.fromhex("00A40400010200")) == b"\x02" + assert pol.command_data(bytes.fromhex("00A4040000000102")) == b"\x02" + assert pol.command_data(bytes.fromhex("00A40400000001020000")) == b"\x02" -def test_select_with_short_data_is_refused(): - with pytest.raises(RemotePolicyError, match="shorter than its length"): - pol.select_target(bytes.fromhex("00A404000BA000")) + +@pytest.mark.parametrize( + "command", + [ + "00A404000BA000", # short Lc longer than the data + "00A40400020102FFFF", # short form with two trailing bytes + "00A404000002", # extended form cut after the first length byte + "00A40400000000AABB", # extended Lc of zero + "00A40400000009A000", # extended Lc longer than the data + "00A4040000000102FF", # extended form with one trailing byte + ], +) +def test_malformed_command_lengths_are_refused(command): + with pytest.raises(RemotePolicyError, match="length fields"): + pol.command_data(bytes.fromhex(command)) # --------------------------------------------------------------------------- # @@ -214,6 +241,39 @@ def test_a_failed_select_does_not_move_the_context(): allow(policy, KEY_INFO) # still the card manager's key table +def test_a_select_answered_61xx_moves_the_context(): + # 61XX is success with the response waiting for GET RESPONSE, the normal + # answer to a SELECT that returns data on a T=0 reader. + policy = pol.RelayPolicy("reset") + allow(policy, select(pol.PIV_AID), sw=(0x61, 0x1A)) + assert policy.context == pol.PIV + allow(policy, apdu(0x00, 0xC0, 0x00, 0x00)) + with pytest.raises(RemotePolicyError, match="not allowed while the piv is selected"): + policy.check(wrapped(0xE4)) + + +@pytest.mark.parametrize("sw", [(0x6C, 0x1A), (0x62, 0x83), (0x63, 0x00)]) +def test_a_select_with_an_uncertain_answer_admits_only_another_select(sw): + policy = pol.RelayPolicy("inspect") + allow(policy, select(pol.PIV_AID), sw=sw) + assert policy.context == pol.UNKNOWN + for command in (KEY_INFO, PIV_GET_DATA, INIT_UPDATE): + with pytest.raises(RemotePolicyError, match="selected applet is unknown"): + policy.check(command) + allow(policy, select(pol.ISD_AID)) + assert policy.context == pol.ISD + allow(policy, KEY_INFO) + + +def test_a_selectable_aid_without_a_context_is_refused(): + policy = pol.RelayPolicy("inspect") + policy.rules = dataclasses.replace( + policy.rules, selectable=policy.rules.selectable | {WALLET_AID} + ) + with pytest.raises(RemotePolicyError, match="no relay context"): + policy.check(select(WALLET_AID)) + + def test_piv_context_allows_only_piv_reads(): policy = pol.RelayPolicy("inspect") allow(policy, select(pol.PIV_AID)) @@ -298,6 +358,11 @@ def delete_apdu(aid: bytes, cla: int = 0x84, mac: bytes = b"\x00" * 8) -> bytes: return bytes([cla, 0xE4, 0x00, 0x00, len(data)]) + data + b"\x00" +def delete_apdu_extended(aid: bytes, cla: int = 0x84, mac: bytes = b"\x00" * 8) -> bytes: + data = bytes([0x4F, len(aid)]) + aid + mac + return bytes([cla, 0xE4, 0x00, 0x00, 0x00]) + len(data).to_bytes(2, "big") + data + + def wrapped(ins: int, p1: int = 0, p2: int = 0, n: int = 24) -> bytes: return apdu(0x84, ins, p1, p2, b"\x00" * n, le=False) @@ -339,16 +404,67 @@ def test_reset_may_only_delete_piv_things(op, aid): assert info.value.header == "84E40000" +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +@pytest.mark.parametrize("aid", [WALLET_AID, FIDO2_AID, FIDO2_PACKAGE_AID]) +def test_the_delete_allow_list_holds_in_the_extended_encoding_too(op, aid): + policy = pol.RelayPolicy(op) + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + with pytest.raises(RemotePolicyError, match=f"DELETE of {aid.hex().upper()}"): + policy.check(delete_apdu_extended(aid)) + allow(policy, delete_apdu_extended(pol.PIV_AID)) + + @pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) def test_an_encrypted_delete_passes_on_instruction_and_context_alone(op): # Under command encryption the AID is opaque; the header-level rules still # hold (ISD context only) and the limitation is documented. policy = pol.RelayPolicy(op) allow(policy, INIT_UPDATE) - allow(policy, EXT_AUTH) + allow(policy, EXT_AUTH_ENC) allow(policy, wrapped(0xE4, 0x00, 0x00)) +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +def test_an_unreadable_delete_is_refused_unless_the_channel_encrypts(op): + # With a MAC alone the AID is in the clear, so a DELETE that does not parse + # is not a legitimate command the relay happens not to understand. + policy = pol.RelayPolicy(op) + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH) + with pytest.raises(RemotePolicyError, match="cannot read") as info: + policy.check(wrapped(0xE4, 0x00, 0x00)) + assert info.value.header == "84E40000" + with pytest.raises(RemotePolicyError, match="cannot read"): + policy.check(delete_apdu(pol.PIV_AID)[:5] + b"\x00" * 21) # template byte lost + + +@pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) +def test_command_encryption_is_forgotten_when_the_channel_ends(op): + policy = pol.RelayPolicy(op) + allow(policy, INIT_UPDATE) + allow(policy, EXT_AUTH_ENC) + allow(policy, wrapped(0xE4)) + allow(policy, INIT_UPDATE) # a new channel negotiation + with pytest.raises(RemotePolicyError, match="cannot read"): + policy.check(wrapped(0xE4)) + allow(policy, EXT_AUTH_ENC) + allow(policy, wrapped(0xE4)) + allow(policy, select(pol.ISD_AID)) # selecting closes the channel + with pytest.raises(RemotePolicyError, match="cannot read"): + policy.check(wrapped(0xE4)) + + +def test_a_failed_authentication_does_not_mark_the_channel_encrypted(): + policy = pol.RelayPolicy("reset") + allow(policy, INIT_UPDATE) + policy.check(EXT_AUTH_ENC) + with pytest.raises(RemotePolicyError, match="EXTERNAL AUTHENTICATE failed"): + policy.observe(EXT_AUTH_ENC, 0x63, 0x00) + with pytest.raises(RemotePolicyError, match="cannot read"): + policy.check(wrapped(0xE4)) + + @pytest.mark.parametrize("op", pol.DESTRUCTIVE_OPS) def test_content_management_never_leaves_the_card_manager(op): policy = pol.RelayPolicy(op) @@ -402,6 +518,7 @@ def test_reset_cannot_select_other_card_functions(op, aid): def test_delete_target_parsing(): assert pol.delete_target(delete_apdu(pol.PIV_AID)) == pol.PIV_AID + assert pol.delete_target(delete_apdu_extended(pol.PIV_AID)) == pol.PIV_AID assert pol.delete_target(wrapped(0xE4)) is None # opaque assert pol.delete_target(bytes.fromhex("84E40000034F01AA")) is None # implausible length From 127c3968181f5f4831fc52fe8c3f3241221767eb Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Mon, 12 Oct 2026 17:14:37 +0200 Subject: [PATCH 8/8] Open the remote operations' card through the application context The remote commands opened the reader with the PC/SC entry points imported into their own module, which the card-double seam guard refuses: a fixture patched at AppContext.open_session cannot substitute such a card, and its tests reach a real reader. They go through open_session now. The relay still transmits on the connection beneath the session, so no response is reassembled on its behalf; CardSession exposes that connection read-only. The remote test fixtures use the shared wire_session helper. Co-Authored-By: Claude Fable 5.1 --- src/cryptnox_id_cli/cli/commands/remote.py | 15 ++++++--------- src/cryptnox_id_cli/transport/pcsc.py | 5 +++++ tests/unit/test_remote_attest_command.py | 10 +++++++--- tests/unit/test_remote_commands.py | 11 +++++------ tests/unit/test_remote_gates.py | 7 ++++--- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/cryptnox_id_cli/cli/commands/remote.py b/src/cryptnox_id_cli/cli/commands/remote.py index 65717de..a747acb 100644 --- a/src/cryptnox_id_cli/cli/commands/remote.py +++ b/src/cryptnox_id_cli/cli/commands/remote.py @@ -31,7 +31,7 @@ from cryptnox_id_cli.state.detector import StateDetector from cryptnox_id_cli.transport.apdu import APDU from cryptnox_id_cli.transport.errors import CryptnoxError, RemoteConnectError -from cryptnox_id_cli.transport.pcsc import CardSession, PCSCConnection, connect, pick_reader +from cryptnox_id_cli.transport.pcsc import CardSession, RawConnection from cryptnox_id_cli.util import tlv _FULL_TRANSCRIPT_KEY = "remote.full_transcript" @@ -86,8 +86,8 @@ def _endpoint(op: str) -> tuple[str, bool]: @dataclass class _Card: - reader: str - conn: PCSCConnection + reader: str | None + conn: RawConnection session: CardSession atr: str cplc_uid: str | None @@ -104,14 +104,11 @@ def to_dict(self) -> dict[str, object]: def _open_card(app: AppContext) -> _Card: """Open the reader and read the card's identity. Leaves the card manager selected.""" - reader = pick_reader(app.reader) - app.resolved_reader = reader - conn = connect(reader) - session = app.make_session(conn, reader_name=reader) + session = app.open_session() cplc = _read_cplc(session) return _Card( - reader=reader, - conn=conn, + reader=session.reader_name or app.resolved_reader, + conn=session.raw_connection, session=session, atr=session.atr.hex().upper(), cplc_uid=cplc["uid"] if cplc else None, diff --git a/src/cryptnox_id_cli/transport/pcsc.py b/src/cryptnox_id_cli/transport/pcsc.py index 080bc41..6b8036e 100644 --- a/src/cryptnox_id_cli/transport/pcsc.py +++ b/src/cryptnox_id_cli/transport/pcsc.py @@ -275,6 +275,11 @@ def redactor(self) -> Redactor: def atr(self) -> bytes: return self._conn.get_atr() + @property + def raw_connection(self) -> RawConnection: + """The connection beneath the session, for code that must not chain responses.""" + return self._conn + def _emit(self, line: str) -> None: if self._log is not None: self._log.write(line + "\n") diff --git a/tests/unit/test_remote_attest_command.py b/tests/unit/test_remote_attest_command.py index d46dba2..b299c44 100644 --- a/tests/unit/test_remote_attest_command.py +++ b/tests/unit/test_remote_attest_command.py @@ -6,6 +6,7 @@ import sys import pytest +from _cardfakes import wire_session from click.testing import CliRunner from cryptography import x509 from cryptography.hazmat.primitives import hashes, serialization @@ -14,6 +15,7 @@ from cryptnox_id_cli.applets.piv import objects as piv_obj from cryptnox_id_cli.cli.commands import remote as remote_cmd +from cryptnox_id_cli.cli.context import AppContext from cryptnox_id_cli.cli.main import main as root from cryptnox_id_cli.remote import attest_verify as av from cryptnox_id_cli.remote import channel as rchannel @@ -165,8 +167,7 @@ def transmit(apdu): return original(apdu) before.transmit = transmit - monkeypatch.setattr(remote_cmd, "pick_reader", lambda pref: "Fake Reader 00 00") - monkeypatch.setattr(remote_cmd, "connect", lambda name: before) + wire_session(monkeypatch, before, reader_name="Fake Reader 00 00") monkeypatch.setattr(av.trust, "load_anchors", lambda extra_dir=None: ([pki["root_der"]], [])) monkeypatch.delenv(rchannel.URL_ENV, raising=False) @@ -231,7 +232,10 @@ def test_slot_is_validated(): def test_dry_run_refuses(monkeypatch): - monkeypatch.setattr(remote_cmd, "pick_reader", lambda *a: (_ for _ in ()).throw(AssertionError)) + def boom(*a, **k): + raise AssertionError("reader must not be opened under --dry-run") + + monkeypatch.setattr(AppContext, "open_session", boom) result = run(["--dry-run", "remote", "attest"]) assert result.exit_code != 0 assert "nothing was sent to the card" in result.output diff --git a/tests/unit/test_remote_commands.py b/tests/unit/test_remote_commands.py index 4c50fd8..8aa19e9 100644 --- a/tests/unit/test_remote_commands.py +++ b/tests/unit/test_remote_commands.py @@ -4,6 +4,7 @@ import json import pytest +from _cardfakes import wire_session from click.testing import CliRunner from cryptnox_id_cli.cli.commands import remote as remote_cmd @@ -44,9 +45,8 @@ def wired(monkeypatch, mock_connection): {SELECT_ISD: "6F108408A000000151000000A5049F6501FF|9000", CPLC_CMD: f"{CPLC_RESP}|9000"}, [], ) - monkeypatch.setattr(remote_cmd, "pick_reader", lambda pref: "Fake Reader 00 00") - monkeypatch.setattr(remote_cmd, "connect", lambda name: conn) - holder: dict = {} + wire_session(monkeypatch, conn, reader_name="Fake Reader 00 00") + holder: dict = {"conn": conn} @contextlib.contextmanager def fake_open(url, **kwargs): @@ -82,7 +82,7 @@ def test_dry_run_refuses_before_anything_opens(monkeypatch): def boom(*a, **k): raise AssertionError("reader must not be opened under --dry-run") - monkeypatch.setattr(remote_cmd, "pick_reader", boom) + monkeypatch.setattr(AppContext, "open_session", boom) result = run(["--dry-run", "remote", "inspect"]) assert result.exit_code != 0 assert "nothing was sent to the card" in result.output @@ -166,8 +166,7 @@ def test_protocol_violation_exits_14(wired): def test_card_is_released_even_when_the_operation_fails(wired, monkeypatch): released = [] - conn = remote_cmd.connect("x") - monkeypatch.setattr(conn, "disconnect", lambda: released.append(True)) + monkeypatch.setattr(wired["conn"], "disconnect", lambda: released.append(True)) wired["arm"]([{"type": "nonsense"}]) run(["--json", "remote", "inspect"]) assert released == [True] diff --git a/tests/unit/test_remote_gates.py b/tests/unit/test_remote_gates.py index 5564612..1b67010 100644 --- a/tests/unit/test_remote_gates.py +++ b/tests/unit/test_remote_gates.py @@ -5,9 +5,11 @@ import sys import pytest +from _cardfakes import wire_session from click.testing import CliRunner from cryptnox_id_cli.cli.commands import remote as remote_cmd +from cryptnox_id_cli.cli.context import AppContext from cryptnox_id_cli.cli.main import main as root from cryptnox_id_cli.remote import channel as rchannel @@ -70,8 +72,7 @@ def wired(monkeypatch, mock_connection): }, [], ) - monkeypatch.setattr(remote_cmd, "pick_reader", lambda pref: "Fake Reader 00 00") - monkeypatch.setattr(remote_cmd, "connect", lambda name: conn) + wire_session(monkeypatch, conn, reader_name="Fake Reader 00 00") monkeypatch.delenv(rchannel.URL_ENV, raising=False) monkeypatch.delenv(remote_cmd.DEV_ACCESS_ENV, raising=False) holder: dict = {"conn": conn, "opened": 0} @@ -142,7 +143,7 @@ def test_dry_run_refuses_before_anything_opens(monkeypatch): def boom(*a, **k): raise AssertionError("reader must not be opened under --dry-run") - monkeypatch.setattr(remote_cmd, "pick_reader", boom) + monkeypatch.setattr(AppContext, "open_session", boom) for op in ("reset", "dev-reset"): result = run(["--dry-run", "remote", op, "--i-understand-this-is-irreversible"]) assert result.exit_code != 0