From 2d5f857adc08dc83a252380faf360d082b62f5f6 Mon Sep 17 00:00:00 2001 From: Mladen Milankovic Date: Wed, 7 Oct 2026 18:55:26 +0200 Subject: [PATCH] Treat the Discovery Object as optional in the PIV state Derive the mandatory object set from the PIV object registry (CHUID and CCC). Discovery is optional in PIV and no built-in profile creates it, so a complete card never reached PivPersonalized. Report finalize in `preperso status` under the rule the finalize command applies (applet selectable, not SECURED) and add `load_config_allowed` for the blank-applet case. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 65 ++++++++--------- src/cryptnox_id_cli/cli/commands/factory.py | 17 +++-- src/cryptnox_id_cli/state/detector.py | 2 +- tests/unit/test_preperso_status_gate.py | 77 +++++++++++++++++++++ tests/unit/test_state_discovery_optional.py | 47 +++++++++++++ 5 files changed, 166 insertions(+), 42 deletions(-) create mode 100644 tests/unit/test_preperso_status_gate.py create mode 100644 tests/unit/test_state_discovery_optional.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 887453b..be68d47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,50 +9,43 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `RSA4096` alongside the existing algorithms: `perso generate-key`, - `perso import-key`, CSR/self-signed-cert signing, and profile key - mechanisms. -- `perso generate-key --create-key-object` - the same dev/eval fallback +- `RSA4096` in `perso generate-key`, `perso import-key`, CSR and + self-signed-cert signing, and profile key mechanisms. +- `perso generate-key --create-key-object`, the dev/eval fallback `import-key` already had. -- Coverage measurement in CI: `pytest-cov` in the `dev` extra, a project-wide - floor from `fail_under`, and a tighter floor over the command modules. With - branch coverage enabled both floors compare the combined statement-and-branch - percentage, which sits below line coverage alone. -- `factory piv preperso set-mgmt-key` - load the PIV management key (9B) - value over the admin channel; `--replace` replaces a value that is set. +- Coverage measurement in CI (`pytest-cov` in the `dev` extra) with a + project-wide floor and a tighter one over the command modules, both on + combined statement-and-branch coverage. +- `factory piv preperso set-mgmt-key`: load the PIV management key (9B) + over the admin channel; `--replace` overwrites a set value. `factory piv preperso status` reports whether 9B holds a value. -- `PIV_MGMT_KEY` (hex) for the PIV management key, alongside the existing - `PIV_SCP03_*` admin-channel variables. +- `PIV_MGMT_KEY` (hex) for the PIV management key, next to `PIV_SCP03_*`. ### Removed -- `transport.pcsc.list_reader_names()`. Nothing called it; `reader_states()` - returns the same names along with card presence and ATR. +- `transport.pcsc.list_reader_names()`; `reader_states()` returns the same + names with card presence and ATR. ### Fixed -- The APDU transcript no longer renders the data field of a secret-bearing - command whose length byte disagrees with the bytes present. An `Lc` larger - than the remaining bytes left the data field unsplit, and the command then - took the path that masks only values registered with the redactor, so an - unregistered PIN could reach the log in the clear. Commands that can carry a - secret now withhold the whole body when the length does not parse; commands - that cannot still render in full, and the case-1 retry-counter probe is - unaffected. -- `doctor` no longer advises "use a DESFire-capable contactless reader" to - someone already on a contactless one. It now tells the detector which reader - the session is on, so the DESFire diagnosis uses the reader name as well as - the ATR. Cards that answer with their own wired-style ATR rather than the - PC/SC composed contactless one were the affected case. -- `perso generate-key` and `quickstart` complete on cards that return at most - 256 bytes of an RSA public-key template over the admin channel: the - truncated response is detected and the generation is repeated over plain - APDUs after authenticating the PIV management key (9B). ECC and cards that - return the full template are unaffected. -- A public-key template the CLI cannot parse now ends in a CLI error naming - the response length, instead of an unhandled exception. -- A `PIV_SCP03_*` value that is not valid hex now ends in a CLI error naming - the variable, instead of an unhandled exception that printed the value. +- `PivPersonalized` no longer requires the optional Discovery Object; CHUID, + CCC and a set PIN suffice. Discovery is still probed and listed. +- `factory piv preperso status` reports `finalize_allowed` under the rule + `finalize` applies (applet selectable, not SECURED) and adds + `load_config_allowed` for a blank applet. +- The APDU transcript withholds the whole body of a secret-bearing command + whose `Lc` disagrees with the bytes present, instead of rendering an + unmasked data field. Other commands still render in full. +- `doctor` uses the reader name as well as the ATR for the DESFire + diagnosis, so it no longer recommends a contactless reader to someone + already on one. +- `perso generate-key` and `quickstart` handle an RSA public-key template + truncated at 256 bytes over the admin channel by repeating the generation + over plain APDUs after 9B authentication. +- An unparseable public-key template ends in a CLI error naming the response + length, not an unhandled exception. +- A `PIV_SCP03_*` value that is not valid hex ends in a CLI error naming the + variable, not an unhandled exception that printed the value. ## [1.0.3] - 2026-08-31 diff --git a/src/cryptnox_id_cli/cli/commands/factory.py b/src/cryptnox_id_cli/cli/commands/factory.py index 715ce43..6971066 100644 --- a/src/cryptnox_id_cli/cli/commands/factory.py +++ b/src/cryptnox_id_cli/cli/commands/factory.py @@ -103,7 +103,9 @@ def status(app: AppContext) -> None: except CryptnoxError: scp_supported = False mgmt = _probe_mgmt_key(session) - finalize_allowed = st.piv == PivState.PRE_PERSONALIZED + # Same gate as the finalize command: selectable and not yet SECURED. + finalize_allowed = st.piv not in (PivState.NOT_PRESENT, PivState.UNKNOWN, PivState.SECURED) + load_config_allowed = st.piv == PivState.PRE_PERSONALIZED scp_ver_label = scp_label(scp_version) payload = { "state": st.piv.label, @@ -111,6 +113,7 @@ def status(app: AppContext) -> None: "scp_version": scp_ver_label if scp_supported else None, "secured": st.piv == PivState.SECURED, "finalize_allowed": finalize_allowed, + "load_config_allowed": load_config_allowed, "management_key": mgmt, } @@ -120,13 +123,17 @@ def human(con: Console) -> None: con.print(f" Admin secure channel available: {avail}") con.print(f" Finalized (SECURED): {'yes' if payload['secured'] else 'no/undetermined'}") con.print(f" Management key (9B): {_mgmt_key_line(mgmt)}") - if finalize_allowed: - con.print(" Pre-perso load + finalize: [green]allowed[/green]") + if load_config_allowed: + con.print(" Pre-perso load-config: [green]allowed[/green] (no structure yet)") else: con.print( - f" [yellow]finalize is NOT allowed[/yellow] in state {st.piv.label} " - "(only on a confidently PivPrePersonalized card)." + " Pre-perso load-config: structure present; it can only add elements, " + "existing ones cannot be changed or removed" ) + if finalize_allowed: + con.print(" Finalize: [green]allowed[/green]") + else: + con.print(f" Finalize: [yellow]not allowed[/yellow] in state {st.piv.label}") app.out.result(payload, human) diff --git a/src/cryptnox_id_cli/state/detector.py b/src/cryptnox_id_cli/state/detector.py index 373a64c..e46ba9b 100644 --- a/src/cryptnox_id_cli/state/detector.py +++ b/src/cryptnox_id_cli/state/detector.py @@ -29,7 +29,7 @@ FIDO_AID = bytes.fromhex("A0000006472F0001") CTAP_GETINFO = bytes([0x04]) -_MANDATORY = ("chuid", "ccc", "discovery") +_MANDATORY = tuple(o.name for o in piv_obj.PIV_OBJECTS if o.mandatory) _PROBE_OBJECTS = ( "chuid", "ccc", diff --git a/tests/unit/test_preperso_status_gate.py b/tests/unit/test_preperso_status_gate.py new file mode 100644 index 0000000..2ef4178 --- /dev/null +++ b/tests/unit/test_preperso_status_gate.py @@ -0,0 +1,77 @@ +"""`preperso status` must report finalize under the same rule `finalize` applies.""" + +import contextlib +import json + +import pytest +from click.testing import CliRunner + +from cryptnox_id_cli.cli.commands import factory as factory_cmd +from cryptnox_id_cli.cli.context import AppContext +from cryptnox_id_cli.cli.main import main as root +from cryptnox_id_cli.state.model import PivState + + +class _NoChannel: + def __init__(self, session): + pass + + def initialize_update_probe(self): + return {"supported": False} + + +def _status(monkeypatch, piv_state): + @contextlib.contextmanager + def fake_session(self): + yield object() + + class _State: + piv = piv_state + + monkeypatch.setattr(AppContext, "open_session", fake_session) + monkeypatch.setattr(factory_cmd, "PivAdmin", _NoChannel) + monkeypatch.setattr(factory_cmd, "_probe_mgmt_key", lambda session: None) + monkeypatch.setattr( + factory_cmd, "StateDetector", lambda *a, **kw: type("D", (), {"detect": lambda s: _State})() + ) + result = CliRunner().invoke(root, ["--json", "factory", "piv", "preperso", "status"]) + assert result.exit_code == 0, result.output + return json.loads(result.stdout) + + +@pytest.mark.parametrize( + "state", + [PivState.PRE_PERSONALIZED, PivState.PARTIALLY_PERSONALIZED, PivState.PERSONALIZED], +) +def test_finalize_allowed_on_any_selectable_unsecured_card(monkeypatch, state): + payload = _status(monkeypatch, state) + assert payload["finalize_allowed"] is True + assert payload["load_config_allowed"] is (state == PivState.PRE_PERSONALIZED) + + +@pytest.mark.parametrize("state", [PivState.SECURED, PivState.NOT_PRESENT, PivState.UNKNOWN]) +def test_finalize_not_allowed_when_finalize_itself_refuses(monkeypatch, state): + payload = _status(monkeypatch, state) + assert payload["finalize_allowed"] is False + assert payload["load_config_allowed"] is False + + +def test_human_output_no_longer_says_finalize_is_not_allowed_with_structure(monkeypatch): + @contextlib.contextmanager + def fake_session(self): + yield object() + + class _State: + piv = PivState.PARTIALLY_PERSONALIZED + + monkeypatch.setattr(AppContext, "open_session", fake_session) + monkeypatch.setattr(factory_cmd, "PivAdmin", _NoChannel) + monkeypatch.setattr(factory_cmd, "_probe_mgmt_key", lambda session: None) + monkeypatch.setattr( + factory_cmd, "StateDetector", lambda *a, **kw: type("D", (), {"detect": lambda s: _State})() + ) + result = CliRunner().invoke(root, ["factory", "piv", "preperso", "status"]) + assert result.exit_code == 0, result.output + assert "Finalize: allowed" in result.output + assert "finalize is NOT allowed" not in result.output + assert "structure present" in result.output diff --git a/tests/unit/test_state_discovery_optional.py b/tests/unit/test_state_discovery_optional.py new file mode 100644 index 0000000..2bf464e --- /dev/null +++ b/tests/unit/test_state_discovery_optional.py @@ -0,0 +1,47 @@ +"""The PIV state reads PivPersonalized without a Discovery Object. + +Discovery is optional in PIV and no built-in profile creates it; the detector must +not hold a complete card at PivPartiallyPersonalized because of it.""" + +from cryptnox_id_cli.applets.piv import objects as piv_obj +from cryptnox_id_cli.state import StateDetector +from cryptnox_id_cli.state.detector import _MANDATORY +from cryptnox_id_cli.state.model import PivState +from cryptnox_id_cli.transport.pcsc import CardSession + +CHUID = "00CB3FFF055C035FC10200" +CCC = "00CB3FFF055C035FC10700" +DISCOVERY = "00CB3FFF035C017E00" +PRESENT = "5303AABBCC|9000" + + +def _detect(mock_connection, transcript, extra): + exchanges = {**transcript["exchanges"], **extra} + conn = mock_connection(transcript["atr"], exchanges, []) + return StateDetector( + CardSession(conn), probe_fido=False, probe_desfire=False, probe_genuine=False + ).detect() + + +def test_mandatory_set_comes_from_the_object_registry(): + assert set(_MANDATORY) == {o.name for o in piv_obj.PIV_OBJECTS if o.mandatory} + assert "discovery" not in _MANDATORY + + +def test_chuid_ccc_and_pin_make_the_card_personalized(mock_connection, acs_transcript): + st = _detect(mock_connection, acs_transcript, {CHUID: PRESENT, CCC: PRESENT}) + assert st.piv == PivState.PERSONALIZED + assert st.piv_objects["discovery"] is False + + +def test_discovery_is_still_reported_when_present(mock_connection, acs_transcript): + st = _detect( + mock_connection, acs_transcript, {CHUID: PRESENT, CCC: PRESENT, DISCOVERY: "7E00|9000"} + ) + assert st.piv == PivState.PERSONALIZED + assert st.piv_objects["discovery"] is True + + +def test_missing_ccc_keeps_the_card_partially_personalized(mock_connection, acs_transcript): + st = _detect(mock_connection, acs_transcript, {CHUID: PRESENT}) + assert st.piv == PivState.PARTIALLY_PERSONALIZED