Merge pull request #3565 from pkilar/epassport-diagnose-failed-reads

Fix ePassport reporting of failed reads, and decode EF_CardAccess

CHANGELOG.md

@@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file.
 This project uses the changelog in accordance with [keepchangelog](http://keepachangelog.com/). Please use this to write notable changes, which is not the same as git commit log...
 
 ## [unreleased][unreleased]
+- Fixed `tools/ePassport` - a read that dumped nothing is now reported as a failure with the client log kept beside the dump, and EF_CardAccess/DG14 protocols are decoded and correctly named (@pkilar)
 - Fixed `tools/ePassport` - FILES and LOG panes no longer go blank on long content, and picture files now show the picture (@pkilar)
 - Fixed `hf xerox view` - now rejects a dump file too small to contain the info blocks instead of reading past it (@munzzyy)
 - Changed NG frame layout to align better with 64bytes frames. From 512 -> 624bytes (@iceman1001)

tools/ePassport/README.md

@@ -314,6 +314,11 @@ delete one with the bin button, open the folder in your file manager, or clear
 out the empty directories left behind by reads that failed. The path is also in
 the status bar (click it) and in the log at the start of every read.
 
+Every read also leaves `pm3.log` in its directory: the client output for that
+run, which is the only record of why a read that dumped nothing gave up. It
+holds what the LOG pane showed and not the MRZ or CAN you typed, and it does
+not count as a file when the app decides whether a read produced anything.
+
 Deleting always asks first, and says what it is about to destroy — a dump holds
 a real portrait and a real MRZ.
 

tools/ePassport/epassport/app.py

@@ -400,6 +400,17 @@ class Pm3PassportApp(App):
             self.show_error(result.error)
             self.select_tab(5)
             return
+        if record is not None and record.is_empty:
+            # Nothing the classifier recognises went wrong, but no file was
+            # written.  Calling that a success drops you on an empty page.
+            self.status_bac = "failed"
+            self.status_auth = "None"
+            self.show_error(
+                errors.NothingReadError(),
+                detail=str(result.log_path or record.source_dir or ""),
+            )
+            self.select_tab(5)
+            return
         self.status_bac = "success"
         mechanism = errors.detect_mechanism(result.lines)
         if mechanism:

tools/ePassport/epassport/config.py

@@ -22,6 +22,10 @@ APP_NAME = "ePassport"
 #: them across rather than silently start a fresh, empty directory.
 LEGACY_NAMES = ("pm3-passport",)
 
+#: The client output kept beside a dump.  It is evidence about the read, not
+#: a file off the chip, so it does not count towards a dump's contents.
+DUMP_LOG_NAME = "pm3.log"
+
 
 def _adopt_legacy(base: Path, name: str) -> Path:
     """Return ``base/name``, moving an older directory into place first."""
@@ -120,7 +124,7 @@ def list_dumps() -> list[DumpEntry]:
     for entry in root.iterdir():
         if not entry.is_dir():
             continue
-        files = [f for f in entry.iterdir() if f.is_file()]
+        files = [f for f in entry.iterdir() if f.is_file() and f.name != DUMP_LOG_NAME]
         try:
             modified = entry.stat().st_mtime
         except OSError:
@@ -145,6 +149,9 @@ def prune_empty_dumps() -> int:
     for entry in list_dumps():
         if entry.is_empty:
             try:
+                # rmdir refuses a directory that still holds the log; drop that
+                # first, and let anything unexpected keep the directory alive.
+                (entry.path / DUMP_LOG_NAME).unlink(missing_ok=True)
                 entry.path.rmdir()
                 removed += 1
             except OSError as exc:

tools/ePassport/epassport/emrtd/__main__.py

@@ -62,6 +62,11 @@ def render(record: PassportRecord) -> str:
                 add(f"  {key:<20}: {value}")
 
     s = record.security
+    if s.pace:
+        add("")
+        add("EF_CardAccess :")
+        for proto in s.pace:
+            add(f"  protocol            : {proto}")
     if s.protocols or s.aa_algorithm:
         add("")
         add("DG14/15       :")

tools/ePassport/epassport/emrtd/dg.py

@@ -9,7 +9,7 @@ from __future__ import annotations
 import logging
 from pathlib import Path
 
-from . import images, tlv
+from . import images, securityinfos, tlv
 from .model import (
     ComInfo,
     DocumentDetails,
@@ -204,57 +204,19 @@ def _clean_field(raw: bytes, *, numeric: bool = False) -> str:
 
 
 # ----------------------------------------------------------- EF_DG14/15
-_SECURITY_OIDS = {
-    "0.4.0.127.0.7.2.2.1.1": "PACE (DH, generic mapping)",
-    "0.4.0.127.0.7.2.2.1.2": "PACE (ECDH, generic mapping)",
-    "0.4.0.127.0.7.2.2.3.2": "Terminal Authentication (ECDSA)",
-    "0.4.0.127.0.7.2.2.4": "PACE info",
-    "0.4.0.127.0.7.2.2.5": "Chip Authentication (public key)",
-    "0.4.0.127.0.7.2.2.3.1": "Terminal Authentication (RSA)",
-    "2.23.136.1.1.5": "Active Authentication",
-    "0.4.0.127.0.7.2.2.1": "Chip Authentication",
-}
-
-
-def parse_security(dg14: bytes, dg15: bytes) -> SecurityInfo:
+def parse_security(dg14: bytes, dg15: bytes, card_access: bytes = b"") -> SecurityInfo:
     out = SecurityInfo()
     if dg14:
         out.dg14_hex = dg14.hex().upper()
-        out.protocols = _oid_summary(dg14)
+        out.protocols = [p.text for p in securityinfos.parse(dg14)]
     if dg15:
         out.dg15_hex = dg15.hex().upper()
         _describe_aa_key(dg15, out)
+    if card_access:
+        out.pace = [p.text for p in securityinfos.parse(card_access)]
     return out
 
 
-def _oid_summary(data: bytes) -> list[str]:
-    """Best-effort list of security protocol OIDs found in DG14."""
-    try:
-        from asn1crypto.core import ObjectIdentifier
-    except ImportError:
-        return []
-    found: list[str] = []
-    pos = 0
-    while True:
-        idx = data.find(b"\x06", pos)
-        if idx < 0 or idx + 1 >= len(data):
-            break
-        length = data[idx + 1]
-        chunk = data[idx : idx + 2 + length]
-        pos = idx + 1
-        if length == 0 or length > 32 or idx + 2 + length > len(data):
-            continue
-        try:
-            oid = ObjectIdentifier.load(chunk).dotted
-        except Exception:
-            continue
-        label = _SECURITY_OIDS.get(oid)
-        entry = f"{oid}  {label}" if label else oid
-        if entry not in found:
-            found.append(entry)
-    return found
-
-
 def _describe_aa_key(dg15: bytes, out: SecurityInfo) -> None:
     """Decode the SubjectPublicKeyInfo in DG15 (tag 6F)."""
     nodes = tlv.parse(dg15)
@@ -313,7 +275,10 @@ def load_dump(directory: Path) -> PassportRecord:
     if "EF_COM" in raw:
         record.com = parse_com(raw["EF_COM"])
 
-    if "EF_DG1" in raw:
+    if not raw:
+        # Naming DG1 here sends you after the wrong file: nothing was read.
+        record.warnings.append("No files were read - the dump directory is empty.")
+    elif "EF_DG1" in raw:
         record.mrz = parse_dg1(raw["EF_DG1"])
         if record.mrz is None:
             record.warnings.append(
@@ -338,7 +303,11 @@ def load_dump(directory: Path) -> PassportRecord:
         record.personal = parse_dg11(raw["EF_DG11"])
     if "EF_DG12" in raw:
         record.document = parse_dg12(raw["EF_DG12"])
-    record.security = parse_security(raw.get("EF_DG14", b""), raw.get("EF_DG15", b""))
+    record.security = parse_security(
+        raw.get("EF_DG14", b""),
+        raw.get("EF_DG15", b""),
+        raw.get("EF_CardAccess", b"") or raw.get("EF_CardSecurity", b""),
+    )
 
     from .sod import parse_sod  # local import: optional dependencies
 

tools/ePassport/epassport/emrtd/model.py

@@ -86,6 +86,7 @@ class SecurityInfo:
     """EF_DG14 / EF_DG15 - chip security options and AA public key."""
 
     protocols: list[str] = field(default_factory=list)
+    pace: list[str] = field(default_factory=list)
     aa_algorithm: str = ""
     aa_key_size: str = ""
     aa_public_key_hex: str = ""
@@ -235,6 +236,11 @@ class PassportRecord:
                 return f
         return None
 
+    @property
+    def is_empty(self) -> bool:
+        """True when the dump held no files at all - the read got nothing."""
+        return not any(f.state == FileState.PRESENT for f in self.files)
+
     def image_for(self, name: str) -> bytes:
         """The decoded PNG for a file that carries a picture, else empty."""
         return {

tools/ePassport/epassport/emrtd/securityinfos.py

@@ -0,0 +1,187 @@
+"""SecurityInfos: what EF_DG14, EF_CardAccess and EF_CardSecurity advertise.
+
+The OID arcs and the names come from the client's own tables in
+``client/src/emrtd/emrtd_pace.c``, so the two agree on what a chip announces.
+PACE lives under ``0.4.0.127.0.7.2.2.4``; the neighbouring arcs are the public
+key, terminal authentication and chip authentication.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+
+from . import tlv
+
+_BSI = "0.4.0.127.0.7.2.2"
+
+#: Protocol families, by OID prefix.  Used when the exact leaf is unknown.
+FAMILIES: dict[str, str] = {
+    f"{_BSI}.1": "Chip Authentication public key",
+    f"{_BSI}.2": "Terminal Authentication",
+    f"{_BSI}.3": "Chip Authentication",
+    f"{_BSI}.4": "PACE",
+    f"{_BSI}.5": "Restricted Identification",
+}
+
+_CIPHERS = ("3DES-CBC-CBC", "AES-CMAC-128", "AES-CMAC-192", "AES-CMAC-256")
+_PACE_MAPPINGS = {
+    1: "DH, Generic Mapping",
+    2: "ECDH, Generic Mapping",
+    3: "DH, Integrated Mapping",
+    4: "ECDH, Integrated Mapping",
+    6: "ECDH, CA Mapping",
+}
+
+PROTOCOLS: dict[str, str] = {
+    f"{_BSI}.1.1": "Chip Authentication public key (DH)",
+    f"{_BSI}.1.2": "Chip Authentication public key (ECDH)",
+    **{
+        f"{_BSI}.4.{mapping}.{n}": f"PACE {name}, {cipher}"
+        for mapping, name in _PACE_MAPPINGS.items()
+        for n, cipher in enumerate(_CIPHERS, start=1)
+    },
+    **{
+        f"{_BSI}.3.{arc}.{n}": f"Chip Authentication {agreement}, {cipher}"
+        for arc, agreement in ((1, "DH"), (2, "ECDH"))
+        for n, cipher in enumerate(_CIPHERS, start=1)
+    },
+}
+
+#: ICAO Doc 9303 security object OIDs, which sit outside the BSI arcs.
+PROTOCOLS.update(
+    {
+        "2.23.136.1.1.1": "LDS Security Object",
+        "2.23.136.1.1.5": "Active Authentication",
+    }
+)
+
+#: Standardised domain parameters, from the client's ``pacesdp_table``.
+DOMAIN_PARAMETERS: dict[int, str] = {
+    0: "1024-bit MODP Group with 160-bit Prime Order Subgroup",
+    1: "2048-bit MODP Group with 224-bit Prime Order Subgroup",
+    2: "2048-bit MODP Group with 256-bit Prime Order Subgroup",
+    8: "NIST P-192 (secp192r1)",
+    9: "BrainpoolP192r1",
+    10: "NIST P-224 (secp224r1)",
+    11: "BrainpoolP224r1",
+    12: "NIST P-256 (secp256r1)",
+    13: "BrainpoolP256r1",
+    14: "BrainpoolP320r1",
+    15: "NIST P-384 (secp384r1)",
+    16: "BrainpoolP384r1",
+    17: "BrainpoolP512r1",
+    18: "NIST P-521 (secp521r1)",
+}
+
+_OID_TAG = 0x06
+_INTEGER_TAG = 0x02
+_BIT_STRING_TAG = 0x03
+_SEQUENCE_TAG = 0x30
+_SET_TAG = 0x31
+
+
+def describe(oid: str) -> str:
+    """The friendliest name for ``oid``: exact, then family, then the OID."""
+    if oid in PROTOCOLS:
+        return PROTOCOLS[oid]
+    for prefix in sorted(FAMILIES, key=len, reverse=True):
+        if oid == prefix or oid.startswith(prefix + "."):
+            return FAMILIES[prefix]
+    return oid
+
+
+@dataclass
+class Protocol:
+    """One SecurityInfo entry."""
+
+    oid: str
+    name: str
+    version: int | None = None
+    parameter_id: int | None = None
+    curve: str = ""
+    key_bits: int | None = None
+
+    @property
+    def text(self) -> str:
+        """One line for the SECURITY tab."""
+        parts = [self.name]
+        if self.curve:
+            parts.append(self.curve)
+        elif self.parameter_id is not None:
+            parts.append(f"domain parameter {self.parameter_id}")
+        if self.key_bits:
+            parts.append(f"{self.key_bits}-bit key")
+        return "  ·  ".join(parts)
+
+
+def parse(data: bytes) -> list[Protocol]:
+    """Every SecurityInfo in ``data``, best effort - never raises."""
+    out: list[Protocol] = []
+    for node in _sequences(data):
+        oid = next(
+            (decode_oid(c.value) for c in node.children if c.tag == _OID_TAG), ""
+        )
+        if not oid:
+            continue
+        numbers = [
+            int.from_bytes(c.value, "big")
+            for c in node.children
+            if c.tag == _INTEGER_TAG and c.value
+        ]
+        entry = Protocol(oid=oid, name=describe(oid))
+        if numbers:
+            entry.version = numbers[0]
+        if len(numbers) > 1:
+            entry.parameter_id = numbers[1]
+            entry.curve = DOMAIN_PARAMETERS.get(entry.parameter_id, "")
+        entry.key_bits = _key_bits(node)
+        out.append(entry)
+    return out
+
+
+def _sequences(data: bytes) -> list[tlv.Tlv]:
+    """The members of the SET, which is what a SecurityInfo actually is.
+
+    Descending further would also collect the algorithm identifiers nested
+    inside a public key, which are not protocols the chip supports.
+    """
+    if not data:
+        return []
+    try:
+        nodes = tlv.parse(data)
+    except Exception:
+        return []
+    # EF_CardAccess is the bare SET; DG14 wraps it in the 0x6E data-group tag.
+    sets = [found for node in nodes for found in node.walk() if found.tag == _SET_TAG]
+    members = sets[0].children if sets else nodes
+    return [
+        m
+        for m in members
+        if m.tag == _SEQUENCE_TAG and any(c.tag == _OID_TAG for c in m.children)
+    ]
+
+
+def _key_bits(node: tlv.Tlv) -> int | None:
+    """Field size of an EC point carried in this entry's public key."""
+    for candidate in node.walk():
+        if candidate.tag != _BIT_STRING_TAG or len(candidate.value) < 4:
+            continue
+        point = candidate.value[1:]  # drop the unused-bits count
+        if point[0] == 0x04 and len(point) % 2 == 1:
+            return (len(point) - 1) // 2 * 8
+    return None
+
+
+def decode_oid(value: bytes) -> str:
+    """Dotted form of a DER OBJECT IDENTIFIER body."""
+    if not value:
+        return ""
+    first = value[0]
+    parts = [str(first // 40), str(first % 40)]
+    number = 0
+    for byte in value[1:]:
+        number = (number << 7) | (byte & 0x7F)
+        if not byte & 0x80:
+            parts.append(str(number))
+            number = 0
+    return ".".join(parts)

tools/ePassport/epassport/pm3/client.py

@@ -18,6 +18,7 @@ from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Callable, Iterable, Sequence
 
+from ..config import DUMP_LOG_NAME as LOG_NAME
 from . import errors
 
 #: Matches every ANSI CSI/OSC escape the client emits for colour.
@@ -217,6 +218,7 @@ class Pm3Result:
     error: errors.Pm3Error | None = None
     duration: float = 0.0
     dump_dir: Path | None = None
+    log_path: Path | None = None
 
     @property
     def ok(self) -> bool:
@@ -227,6 +229,44 @@ class Pm3Result:
         return "\n".join(self.lines)
 
 
+#: The arguments that carry key material: MRZ line, document number, dates, CAN.
+_KEY_MATERIAL = re.compile(r"(?<![\w-])(--can|-[mnde])(\s+)(\S+)")
+
+
+def redact(text: str) -> str:
+    """Blank the key material out of a command line.
+
+    The client echoes its own command line back, so scrubbing what we write is
+    not enough on its own - every line has to go through this.
+    """
+    return _KEY_MATERIAL.sub(lambda m: f"{m.group(1)}{m.group(2)}<redacted>", text)
+
+
+def write_log(result: "Pm3Result") -> Path | None:
+    """Write the client output into the dump directory.  Returns the path.
+
+    A failed read leaves this where it used to leave an empty directory, so the
+    MRZ, document number and CAN are redacted: they would otherwise be a
+    plaintext copy of the holder's details written out as a side effect.
+    """
+    directory = result.dump_dir
+    if directory is None:
+        return None
+    directory = Path(directory)
+    header = (
+        f"# pm3 {redact(result.command)}"
+        f", exit {result.returncode}, {result.duration:.1f}s"
+    )
+    body = "\n".join(redact(line) for line in result.lines)
+    try:
+        directory.mkdir(parents=True, exist_ok=True)
+        path = directory / LOG_NAME
+        path.write_text(f"{header}\n{body}\n", encoding="utf-8")
+    except OSError:
+        return None
+    return path
+
+
 def find_pm3_binary(configured: str | os.PathLike[str] | None = None) -> Path | None:
     """$PM3_BIN -> configured path -> PATH -> ~/pm3/pm3 -> repo-relative ./pm3."""
     candidates: list[Path] = []
@@ -394,6 +434,10 @@ class Pm3Client:
             )
         else:
             result.error = errors.classify(lines)
+        # Keyed on the directory, not the command: run() is handed the whole
+        # built command line, so matching a bare subcommand never fires.
+        if dump_dir is not None:
+            result.log_path = write_log(result)
         return result
 
     # -- cancellation -----------------------------------------------------

tools/ePassport/epassport/pm3/errors.py

@@ -55,11 +55,31 @@ class NoCardError(Pm3Error):
     )
 
 
+class NothingReadError(Pm3Error):
+    title = "The read produced no files"
+    remedy = (
+        "The client finished without reporting a failure, but wrote nothing.\n"
+        "Check the log pane for what it actually said - a mismatched MRZ or CAN,\n"
+        "or a chip that needs PACE the client could not negotiate, both land here."
+    )
+
+
 class CardLostError(Pm3Error):
     title = "Lost the card mid-transaction"
     remedy = "The chip left the field before the dump finished. Do not move the passport while it reads."
 
 
+class NoApduResponseError(Pm3Error):
+    title = "The chip stopped answering"
+    remedy = (
+        "A command got no reply, so the key was never tested - this is not an\n"
+        "MRZ or a CAN problem, and no attempt was counted against the chip.\n"
+        "Failing at the same step on every run points at the chip or the reader\n"
+        "rather than at the passport having moved.  pm3.log in the dump\n"
+        "directory has the exchange."
+    )
+
+
 class BacFailedError(Pm3Error):
     title = "BAC authentication failed"
     remedy = (
@@ -143,6 +163,10 @@ RULES: tuple[_Rule, ...] = (
         PortBusyError,
     ),
     _Rule(re.compile(r"OFFLINE.*mode", re.I), NoDeviceError),
+    # Ranked above the MRZ verdict: the client prints that line whenever
+    # external authentication fails, so it also fires when the chip never
+    # answered and the key was consequently never tested.
+    _Rule(re.compile(r"APDU: no APDU response", re.I), NoApduResponseError),
     _Rule(re.compile(r"Did you supply the correct MRZ info", re.I), BacFailedError),
     _Rule(
         re.compile(r"Challenge failed, rnd_ifd does not match", re.I), BacFailedError
@@ -152,20 +176,50 @@ RULES: tuple[_Rule, ...] = (
         MissingBacInputError,
     ),
     _Rule(re.compile(r"Couldn't get challenge", re.I), CardLostError),
+    _Rule(re.compile(r"Couldn't reconnect to the document", re.I), CardLostError),
+    # "Secure select rejected" is deliberately absent: the client selects each
+    # file in turn, so an absent optional DG answers 6A82 during a good read.
+    _Rule(re.compile(r"Secure select got no response", re.I), CardLostError),
+    _Rule(
+        re.compile(r"Secure select response failed the MAC check", re.I), CardLostError
+    ),
     _Rule(re.compile(r"Couldn't select the MRTD application", re.I), NoCardError),
     _Rule(re.compile(r"Can't select card", re.I), NoCardError),
+    _Rule(re.compile(r"No ISO14443-[AB] Card in field", re.I), NoCardError),
     _Rule(
         re.compile(r"(iso14443a card select failed|No known/supported 14a tag)", re.I),
         NoCardError,
     ),
     _Rule(re.compile(r"Failed to read EF_COM", re.I), CardLostError),
+    _Rule(
+        re.compile(r"Couldn't build the MRZ information string", re.I),
+        MissingBacInputError,
+    ),
+    _Rule(
+        re.compile(r"(Date of birth|Expiry) date format is incorrect", re.I),
+        MissingBacInputError,
+    ),
     # PACE, most specific first: a wrong password is actionable, the rest is not.
     _Rule(re.compile(r"PACE.*wrong password", re.I), CanWrongError),
     _Rule(re.compile(r"PACE: invalid CAN", re.I), CanWrongError),
+    _Rule(re.compile(r"PACE: invalid MRZ data", re.I), BacFailedError),
     _Rule(
         re.compile(r"CAN (has to be numeric|length is incorrect)", re.I), CanWrongError
     ),
     _Rule(re.compile(r"PACE.*rejected the algorithm", re.I), PaceAlgorithmError),
+    _Rule(re.compile(r"PACE.*offers no algorithm we can do", re.I), PaceAlgorithmError),
+    _Rule(
+        re.compile(r"PACE was forced.*no usable EF_CardAccess", re.I),
+        PaceAlgorithmError,
+    ),
+    _Rule(
+        re.compile(
+            r"PACE: (mapped generator|shared secret).*"
+            r"(not on the curve|point at infinity)",
+            re.I,
+        ),
+        PaceFailedError,
+    ),
     _Rule(
         re.compile(r"PACE.*(authentication token is wrong|echoed our ephemeral)", re.I),
         PaceFailedError,
@@ -199,9 +253,25 @@ def detect_mechanism(lines: list[str]) -> str:
     return ""
 
 
+#: Failures that describe one authentication attempt.  The client tries PACE
+#: and falls back to BAC, so a later success means the attempt was superseded
+#: rather than fatal - reporting it would fail a dump that has every file.
+_ATTEMPT_ERRORS = (
+    BacFailedError,
+    CanWrongError,
+    MissingBacInputError,
+    PaceAlgorithmError,
+    PaceFailedError,
+    PaceOnlyError,
+)
+
+
 def classify(lines: list[str]) -> Pm3Error | None:
     """Return the most specific failure implied by the collected log lines."""
+    authenticated = bool(detect_mechanism(lines))
     for rule in RULES:
+        if authenticated and rule.exc in _ATTEMPT_ERRORS:
+            continue
         for line in lines:
             if rule.pattern.search(line):
                 return rule.exc(detail=line.strip())

tools/ePassport/epassport/ui/tabs/pages.py

@@ -204,6 +204,7 @@ class SecurityPage(TabPage):
                 ("Signer serial", sod.signer_serial),
                 ("Certificate valid from", sod.valid_from),
                 ("Certificate valid to", sod.valid_to),
+                ("PACE  ·  EF_CardAccess", "\n".join(sec.pace)),
                 ("DG14 protocols", "\n".join(sec.protocols)),
                 ("DG15 AA public key", f"{sec.aa_algorithm} {sec.aa_key_size}".strip()),
             )

tools/ePassport/tests/test_dg_parse.py

@@ -146,9 +146,17 @@ def test_td1_dump_without_sod(td1: Path) -> None:
 
 
 def test_empty_directory_yields_a_warning(tmp_path: Path) -> None:
+    """A read that dumped nothing used to surface as a DG1 problem.
+
+    Every file is missing when the directory is empty; DG1 was just the first
+    one checked, so "EF_DG1 is missing - no MRZ to render" sent you looking at
+    the wrong file instead of at the read that produced nothing.
+    """
     r = dg.load_dump(tmp_path)
     assert r.mrz is None
-    assert any("EF_DG1 is missing" in w for w in r.warnings)
+    assert r.is_empty
+    assert any("no files" in w.lower() for w in r.warnings)
+    assert not any("EF_DG1" in w for w in r.warnings)
 
 
 def test_uppercase_bin_suffix_is_accepted(tmp_path: Path, td3: Path) -> None:
@@ -216,3 +224,16 @@ def test_image_for_is_empty_for_files_that_carry_no_picture(record) -> None:
     assert record.image_for("EF_COM") == b""
     assert record.image_for("EF_SOD") == b""
     assert record.image_for("nonsense") == b""
+
+
+def test_a_dump_with_files_is_not_empty(record) -> None:
+    assert not record.is_empty
+
+
+def test_a_dump_missing_only_dg1_still_names_dg1(tmp_path: Path, td3: Path) -> None:
+    import shutil
+
+    shutil.copy(td3 / "EF_COM.bin", tmp_path / "EF_COM.bin")
+    r = dg.load_dump(tmp_path)
+    assert not r.is_empty
+    assert any("EF_DG1 is missing" in w for w in r.warnings)

tools/ePassport/tests/test_dumps.py

@@ -155,3 +155,45 @@ def test_every_test_here_redirects_the_data_directory() -> None:
             for token in ("config.list_dumps", "config.data_dir", "prune_empty")
         ):
             assert "XDG_DATA_HOME" in source, f"{name} must redirect XDG_DATA_HOME"
+
+
+def _failed_read(root: Path, name: str = "20260101T000000Z") -> Path:
+    """A read that dumped nothing but left its client log behind."""
+    directory = root / "ePassport" / "dumps" / name
+    directory.mkdir(parents=True)
+    (directory / config.DUMP_LOG_NAME).write_text("# pm3 dump, exit 0\n[!] failed\n")
+    return directory
+
+
+def test_a_directory_holding_only_the_log_still_reads_as_empty(
+    monkeypatch, tmp_path: Path
+) -> None:
+    """The log is evidence about the read, not a file off the chip.
+
+    Counting it would hide a failed read behind "1 files" and take it out of
+    reach of the empty-dump cleanup.
+    """
+    monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
+    _failed_read(tmp_path)
+    (entry,) = config.list_dumps()
+    assert entry.is_empty
+    assert "did not complete" in entry.summary
+
+
+def test_pruning_clears_a_failed_read_along_with_its_log(
+    monkeypatch, tmp_path: Path
+) -> None:
+    monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
+    directory = _failed_read(tmp_path)
+    assert config.prune_empty_dumps() == 1
+    assert not directory.exists()
+
+
+def test_pruning_leaves_a_directory_holding_real_files(
+    monkeypatch, tmp_path: Path
+) -> None:
+    monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
+    directory = _dump(tmp_path, "20260101T000000Z", 2)
+    (directory / config.DUMP_LOG_NAME).write_text("# pm3 dump, exit 0\n")
+    assert config.prune_empty_dumps() == 0
+    assert directory.exists()

tools/ePassport/tests/test_pm3_client.py

@@ -8,7 +8,7 @@ from pathlib import Path
 
 import pytest
 
-from epassport.pm3 import errors
+from epassport.pm3 import client, errors
 from epassport.pm3.client import (
     BacInput,
     Pm3Client,
@@ -420,3 +420,223 @@ def test_a_recorded_wrong_can_session_classifies_as_a_wrong_can(logs: Path) -> N
     result = client.run("hf emrtd dump --can 111111 --pace")
     assert isinstance(result.error, errors.CanWrongError)
     assert errors.detect_mechanism(result.lines) == ""
+
+
+# ------------------------------------------------ keeping the client output
+def test_a_dump_leaves_its_log_behind(tmp_path: Path) -> None:
+    """A read that fails writes no files, so without this there is no evidence.
+
+    An empty dump directory is all a failed read used to leave, which made it
+    impossible to say afterwards why the chip gave up nothing.
+    """
+    result = Pm3Result(
+        command="dump",
+        argv=["pm3", "-c", "hf emrtd dump"],
+        returncode=0,
+        lines=["[=] Reading EF_COM", "[-] Failed"],
+        dump_dir=tmp_path,
+    )
+    path = client.write_log(result)
+    assert path == tmp_path / client.LOG_NAME
+    written = path.read_text()
+    assert "[-] Failed" in written
+    assert "[=] Reading EF_COM" in written
+
+
+MRZ_LINE2 = "FE21053688POL1003097M3312201<<<<<<<<<<<<<<04"
+
+
+@pytest.mark.parametrize(
+    "command",
+    [
+        f"hf emrtd dump -m {MRZ_LINE2} --dir /tmp/d",
+        "hf emrtd dump -n L898902C3 -d 740812 -e 120415 --dir /tmp/d",
+        "hf emrtd dump --can 123456 --dir /tmp/d",
+    ],
+)
+def test_the_log_does_not_carry_the_key_material(tmp_path: Path, command: str) -> None:
+    """A failed read leaves this behind where it used to leave nothing.
+
+    ``command`` is the whole built command line and the client echoes it back
+    in its own output, so the MRZ, the document number and the CAN all reach
+    the log unless they are scrubbed out of both.
+    """
+    result = Pm3Result(
+        command=command,
+        argv=["pm3", "-c", command],
+        returncode=0,
+        lines=[f"[+] execute command from commandline: {command}"],
+        dump_dir=tmp_path,
+    )
+    written = client.write_log(result).read_text()
+    for secret in (MRZ_LINE2, "L898902C3", "740812", "120415", "123456"):
+        assert secret not in written
+    assert "--dir" in written  # the dump path is not a secret and stays
+
+
+def test_writing_a_log_without_a_dump_dir_is_a_no_op() -> None:
+    result = Pm3Result(command="dump", argv=[], returncode=0, dump_dir=None)
+    assert client.write_log(result) is None
+
+
+@pytest.mark.parametrize(
+    "line, expected",
+    [
+        ("[!] PACE: invalid MRZ data", errors.BacFailedError),
+        (
+            "[!] PACE: this document offers no algorithm we can do",
+            errors.PaceAlgorithmError,
+        ),
+        (
+            "[!] PACE was forced but this document has no usable EF_CardAccess.",
+            errors.PaceAlgorithmError,
+        ),
+        ("[!] PACE: mapped generator is not on the curve", errors.PaceFailedError),
+        ("[!] PACE: shared secret is the point at infinity", errors.PaceFailedError),
+        ("[!] Couldn't reconnect to the document.", errors.CardLostError),
+        ("[!] Secure select got no response", errors.CardLostError),
+        ("[!] Secure select response failed the MAC check", errors.CardLostError),
+        (
+            "[!] Couldn't build the MRZ information string.",
+            errors.MissingBacInputError,
+        ),
+        (
+            "[!] Date of birth date format is incorrect, cannot continue.",
+            errors.MissingBacInputError,
+        ),
+        (
+            "[!] Expiry date format is incorrect, cannot continue.",
+            errors.MissingBacInputError,
+        ),
+    ],
+)
+def test_fatal_client_messages_classify(line: str, expected: type) -> None:
+    """Each of these ends in a return false in the client, and left no error.
+
+    Unmatched, they surfaced as the generic "read produced no files" instead
+    of saying which thing went wrong.
+    """
+    err = errors.classify([line])
+    assert isinstance(err, expected)
+    assert err.remedy
+
+
+def test_an_absent_optional_file_is_not_a_failure() -> None:
+    """The client selects each file in turn, so a missing one is answered 6A82.
+
+    That is ordinary during a good read - classifying it would turn a working
+    dump into an error.
+    """
+    lines = [
+        "[=] Reading EF_COM",
+        "[!] Secure select rejected by the document (6A82 - File not found)",
+        "[!] Failed to secure select 0103",
+        "[+] Read EF_DG1",
+    ]
+    assert errors.classify(lines) is None
+
+
+# --------------------------------- a failed attempt that was recovered from
+_FALLBACK = [
+    "[=] Read EF_CardAccess, len 22",
+    "[=] Trying PACE with the MRZ",
+    "[!!] PACE: step 4 (mutual authentication) failed (6300), the document rejected it",
+    "[=] PACE failed, falling back to BAC",
+    "[+] Basic Access Control successful",
+    "[+] Saved 93 bytes to binary file `EF_DG1.bin`",
+]
+
+
+def test_a_failed_pace_attempt_is_not_the_outcome_when_bac_gets_in() -> None:
+    """Falling back to BAC is ordinary, and the read that follows works.
+
+    Every line was matched on its own, so the superseded PACE failure was
+    reported as the result: a dump with every file in it announced itself as
+    "PACE authentication failed" and dropped the user on the log tab.
+    """
+    assert errors.detect_mechanism(_FALLBACK) == "BAC"
+    assert errors.classify(_FALLBACK) is None
+
+
+def test_a_failure_after_authentication_still_counts() -> None:
+    """Only the authentication attempt is superseded, not what follows it."""
+    lines = _FALLBACK + ["[!!] Couldn't reconnect to the document."]
+    assert isinstance(errors.classify(lines), errors.CardLostError)
+
+
+def test_both_mechanisms_failing_is_still_a_failure() -> None:
+    """Nothing got in here, so the MRZ verdict stands."""
+    lines = [
+        "[=] Trying PACE with the MRZ",
+        "[!!] PACE: step 4 (mutual authentication) failed (6300), the document rejected it",
+        "[=] PACE failed, falling back to BAC",
+        "[!!] Couldn't do external authentication. Did you supply the correct MRZ info?",
+    ]
+    assert errors.detect_mechanism(lines) == ""
+    assert isinstance(errors.classify(lines), errors.BacFailedError)
+
+
+def test_a_real_dump_run_leaves_its_log(tmp_path: Path) -> None:
+    """Exercise run(), not write_log().
+
+    run() received the whole built command - "hf emrtd dump -n ... --dir ..." -
+    and was comparing it to the bare subcommand, so the log was never written
+    for an actual read.  Calling write_log() directly in a test hid that.
+    """
+    fake = tmp_path / "pm3"
+    fake.write_text('#!/bin/sh\necho "[=] Read EF_CardAccess, len 22"\n')
+    fake.chmod(0o755)
+    dump_dir = tmp_path / "dump"
+    dump_dir.mkdir()
+
+    client_under_test = Pm3Client(fake)
+    command = Pm3Client.build_command("dump", None, dump_dir)
+    result = client_under_test.run(command, dump_dir=dump_dir)
+
+    assert result.log_path == dump_dir / client.LOG_NAME
+    assert "Read EF_CardAccess" in result.log_path.read_text()
+
+
+def test_a_card_that_stops_answering_is_not_an_mrz_problem() -> None:
+    """The client blames the MRZ whenever external authentication fails.
+
+    That line is printed regardless of why, so when the chip simply stopped
+    responding it still asked whether the MRZ was right - sending you to check
+    digits that were never tested.  No APDU came back at all here.
+    """
+    lines = [
+        "[=] Read EF_CardAccess, len 22",
+        "[=] Trying PACE with the MRZ",
+        "[!!] APDU: no APDU response",
+        "[!!] PACE: MSE:Set AT got no response",
+        "[=] PACE failed, falling back to BAC",
+        "[!!] APDU: no APDU response",
+        "[!!] Couldn't do external authentication. Did you supply the correct MRZ info?",
+    ]
+    err = errors.classify(lines)
+    assert isinstance(err, errors.NoApduResponseError)
+    assert "not an\nMRZ" in err.remedy
+
+
+def test_a_document_that_rejects_the_key_is_still_an_mrz_problem() -> None:
+    """The chip answered and said no - there the MRZ verdict is the right one."""
+    lines = [
+        "[=] Trying PACE with the MRZ",
+        "[!!] PACE: step 4 (mutual authentication) failed (6300), the document rejected it",
+        "[=] PACE failed, falling back to BAC",
+        "[!!] Couldn't do external authentication. Did you supply the correct MRZ info?",
+    ]
+    assert isinstance(errors.classify(lines), errors.BacFailedError)
+
+
+@pytest.mark.parametrize(
+    "line",
+    ["[!] No ISO14443-A Card in field", "[!] No ISO14443-B Card in field"],
+)
+def test_no_card_in_field_says_so(line: str) -> None:
+    """Both are printed when the poll finds nothing, A and B alike.
+
+    Unmatched, a read that never saw the passport was reported as one that
+    produced no files, which describes the outcome and not the cause.
+    """
+    assert isinstance(errors.classify([line]), errors.NoCardError)

tools/ePassport/tests/test_securityinfos.py

@@ -0,0 +1,104 @@
+"""SecurityInfos - the protocol list in EF_DG14, EF_CardAccess, EF_CardSecurity.
+
+The OID arcs are the ones the client uses in client/src/emrtd/emrtd_pace.c:
+PACE is 0.4.0.127.0.7.2.2.4.x.y, and 0.4.0.127.0.7.2.2.1.2 is the Chip
+Authentication public key the client calls oid_pk_ecdh. The viewer used to
+label the second one as PACE, so a PACE passport had its CA key announced as
+PACE while its actual PACE protocol went unlabelled.
+"""
+
+from __future__ import annotations
+
+from epassport.emrtd import securityinfos as si
+
+#: A real EF_CardAccess: PACE ECDH generic mapping, 3DES, NIST P-256.
+CARD_ACCESS = bytes.fromhex("31143012060A04007F0007020204020102010202010C")
+
+
+def test_the_public_key_oid_is_not_pace() -> None:
+    name = si.describe("0.4.0.127.0.7.2.2.1.2")
+    assert "PACE" not in name
+    assert "Chip Authentication" in name
+
+
+def test_the_chip_authentication_oid_is_not_terminal_authentication() -> None:
+    name = si.describe("0.4.0.127.0.7.2.2.3.2.1")
+    assert "Terminal Authentication" not in name
+    assert "Chip Authentication" in name
+
+
+def test_pace_is_the_arc_the_client_uses() -> None:
+    assert "PACE" in si.describe("0.4.0.127.0.7.2.2.4.2.1")
+    assert "ECDH" in si.describe("0.4.0.127.0.7.2.2.4.2.1")
+
+
+def test_terminal_authentication_has_its_own_arc() -> None:
+    assert "Terminal Authentication" in si.describe("0.4.0.127.0.7.2.2.2")
+
+
+def test_an_unknown_oid_comes_back_as_itself() -> None:
+    assert si.describe("1.2.840.10045.2.1") == "1.2.840.10045.2.1"
+
+
+def test_card_access_decodes_to_protocol_version_and_curve() -> None:
+    (entry,) = si.parse(CARD_ACCESS)
+    assert entry.oid == "0.4.0.127.0.7.2.2.4.2.1"
+    assert "PACE" in entry.name
+    assert entry.version == 2
+    assert entry.parameter_id == 12
+    assert entry.curve == "NIST P-256 (secp256r1)"
+
+
+def test_the_display_line_names_the_protocol_and_the_curve() -> None:
+    (entry,) = si.parse(CARD_ACCESS)
+    assert "PACE" in entry.text
+    assert "NIST P-256 (secp256r1)" in entry.text
+
+
+def test_parsing_rubbish_yields_nothing_rather_than_raising() -> None:
+    assert si.parse(b"") == []
+    assert si.parse(b"\xff\xff\xff") == []
+
+
+def test_domain_parameters_match_the_client_table() -> None:
+    assert si.DOMAIN_PARAMETERS[12] == "NIST P-256 (secp256r1)"
+    assert si.DOMAIN_PARAMETERS[13] == "BrainpoolP256r1"
+    assert 3 not in si.DOMAIN_PARAMETERS  # 3..7 are not assigned
+
+
+def test_the_icao_active_authentication_oid_is_named() -> None:
+    assert si.describe("2.23.136.1.1.5") == "Active Authentication"
+
+
+def _der(tag: int, value: bytes) -> bytes:
+    from epassport.emrtd import tlv
+
+    return tlv.encode(tag, value)
+
+
+def _public_key_info() -> bytes:
+    """A ChipAuthenticationPublicKeyInfo carrying a P-256 point."""
+    algorithm = _der(
+        0x30,
+        _der(0x06, bytes.fromhex("2A8648CE3D0201"))
+        + _der(0x06, bytes.fromhex("2A8648CE3D0101")),
+    )
+    key = _der(0x30, algorithm + _der(0x03, b"\x00\x04" + bytes(64)))
+    info = _der(0x30, _der(0x06, bytes.fromhex("04007F000702020102")) + key)
+    return _der(0x31, info)
+
+
+def test_nested_algorithm_oids_are_not_listed_as_protocols() -> None:
+    """SecurityInfos is a SET OF SecurityInfo - only its members count.
+
+    Walking every nested SEQUENCE also picked up the X9.62 identifiers inside
+    the public key, which are not protocols the chip supports.
+    """
+    entries = si.parse(_public_key_info())
+    assert [e.oid for e in entries] == ["0.4.0.127.0.7.2.2.1.2"]
+
+
+def test_a_public_key_entry_reports_its_size() -> None:
+    (entry,) = si.parse(_public_key_info())
+    assert entry.key_bits == 256
+    assert "256" in entry.text