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