From f37b2aba0eafffb151102b5474aacabc95f725fc Mon Sep 17 00:00:00 2001 From: Jesse van Muijden Date: Tue, 13 Oct 2026 10:33:48 -0400 Subject: [PATCH 1/4] fix: take the identity claims from the chip, not from MRZ OCR The credential's identity fields came from FaceTec's OCR of the MRZ in the camera image (mrzValues overriding scannedValues). An OCR misread, typically a '<' filler read as a letter, put an extra letter in a tester's name. The chip was read and verified, but only used to cross-check the OCR; a mismatch made the chip untrusted, which refused a passport under trust.required but let an ID card (or any document without trust enforcement) through with the misread. Once local passive authentication has verified the SOD signature and the data-group hashes, emrtd.Verify now returns the holder's identity as signed on the chip: names, document number, dates of birth and expiry, nationality, issuing state and sex from DG1, refined by DG11 (full, untruncated name; date of birth with century) when the chip presents it. assessChip overwrites the OCR'd fields with it, whether or not the PDP trusts the signer: signed chip data is never worse than OCR of the page printed next to it. Country codes are normalised to alpha-3, so ICAO's "D" (Germany) now maps to DE in the credential. The OCR->DG1 comparison becomes a diagnostic: differing fields are reported by name (never values) and logged. Only the document type must still agree with the signed document code, since it selects the policy rule. The SOD signing time is bounded by the chip's expiry date instead of the OCR'd one. Documents without an ICAO chip keep the OCR values. Fixes #83 --- README.md | 12 +- docs/adr/002-emrtd-document-signer-trust.md | 2 +- internal/apiv1/chip.go | 43 ++++ internal/apiv1/chip_test.go | 76 ++++++- internal/emrtd/emrtdtest/emrtdtest.go | 46 ++++- internal/emrtd/verify.go | 217 +++++++++++++++----- internal/emrtd/verify_test.go | 144 ++++++++++--- internal/facetec/types.go | 4 + 8 files changed, 460 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index a39713c..94fe338 100644 --- a/README.md +++ b/README.md @@ -342,13 +342,21 @@ constrains `chip-trusted` must list the fields before it (`(* set true false)` m `nfc-verified` is FaceTec's own chip authentication (status 4: clone detection plus signature verification against the DSC embedded in the same chip); it proves the chip is genuine hardware holding intact data, not that the data was signed by a state we trust. `chip-trusted` is `true` -only when facetec-api itself verified the SOD signature, every data-group hash and the DG1/MRZ -cross-check, **and** the go-trust PDP trusts the DSC for the issuing state +only when facetec-api itself verified the SOD signature and every data-group hash, the signed +document code matches the reported document type, **and** the go-trust PDP trusts the DSC for the issuing state (see [ADR-002](docs/adr/002-emrtd-document-signer-trust.md)). Passports need both. Passports whose chips lack Active/Chip Authentication (status 1) are therefore refused by design; this is a known coverage trade-off. With `trust.required` (passports only), a scan that passes the NFC gate (status 4) but whose raw chip data is missing or untrusted is refused with `chip_untrusted`. +**Identity from the chip.** Once the SOD signature and the data-group hashes are verified, the +credential's identity fields come from the chip, not from FaceTec's OCR of the MRZ in the photo: +names, document number, dates of birth and expiry, nationality, issuing state and sex from DG1 +(the signed MRZ), refined by DG11 (full name, date of birth with century) when the chip carries +it. This holds whether or not the PDP trusts the signer, since signed chip data is never worse +than OCR. Where the OCR differed, the fields are logged by name (never values). Documents without +an ICAO chip (e.g. an ISO 18013 driving licence) keep the OCR values. + ## ETSI 119 461 §4.5 Compliance This release implements the traceability and quality controls required by diff --git a/docs/adr/002-emrtd-document-signer-trust.md b/docs/adr/002-emrtd-document-signer-trust.md index f6a3b9d..35b6185 100644 --- a/docs/adr/002-emrtd-document-signer-trust.md +++ b/docs/adr/002-emrtd-document-signer-trust.md @@ -20,7 +20,7 @@ Split the work across three roles (shared contract for the emrtd effort): | Role | Component | Responsibility | |------|-----------|----------------| -| PEP | facetec-api (`internal/emrtd`) | Parse the SOD from `documentData.nfcValues.rawData`, verify the CMS signature with the embedded DSC, verify every presented data group against the SOD, cross-check DG1 against the scanned MRZ fields and issuing state, normalise the issuing state to ISO alpha-3, then ask the PDP. Never uses FaceTec's `signingCertificate` / `wasSignedDataValidated`. gmrtd's embedded master lists are not used as trust. | +| PEP | facetec-api (`internal/emrtd`) | Parse the SOD from `documentData.nfcValues.rawData`, verify the CMS signature with the embedded DSC, verify every presented data group against the SOD, check the signed document code against the reported document type, take the holder's identity from DG1 (and DG11 when present) instead of the scanned MRZ (#83; OCR differences are logged by field name only), normalise the issuing state to ISO alpha-3, then ask the PDP. Never uses FaceTec's `signingCertificate` / `wasSignedDataValidated`. gmrtd's embedded master lists are not used as trust. | | PDP | go-trust `emrtd` registry | Decide whether the DSC chains to an anchor in the reviewed CSCA list for the claimed state, evaluated at the SOD signing time. Does not see the SOD. | | Anchor list | emrtd-trust-anchors repo | The reviewed CSCA list and its maintenance tooling. | diff --git a/internal/apiv1/chip.go b/internal/apiv1/chip.go index 8566281..8413091 100644 --- a/internal/apiv1/chip.go +++ b/internal/apiv1/chip.go @@ -47,6 +47,21 @@ func (c *Client) assessChip(ctx context.Context, scan *facetec.ScanResult) *idve Sex: dd.Sex, DocumentType: dd.DocumentType, }) + // Once the SOD signature and the data-group hashes are verified, the + // identity the chip signed replaces FaceTec's OCR of the MRZ (#83): an OCR + // misread, e.g. a '<' filler read as a letter, must not reach the + // credential. This does not depend on the PDP's trust decision: data the + // chip signed is never worse than OCR of the page printed next to it. + if local := out.Local; local != nil && local.Identity != nil { + applyChipIdentity(&id.DocumentData, local.Identity) + id.ChipIdentityApplied = true + if len(local.Mismatches) > 0 && c.log != nil { + c.log.Info("chip identity differs from OCR; issuing from the chip", + zap.Strings("fields", local.Mismatches), + zap.String("doc_type", dd.DocumentType), + ) + } + } id.ChipTrusted = out.Trusted id.ChipTrustReason = out.Reason id.ChipDSCSHA256 = out.Local.DSCFingerprint() @@ -82,6 +97,7 @@ func chipAuditFields(id facetec.IDScanResult) []zap.Field { zap.Bool("chip_trusted", id.ChipTrusted), zap.String("chip_trust_reason", id.ChipTrustReason), zap.Int("chip_auth_status", id.ChipAuthStatus), + zap.Bool("chip_identity_applied", id.ChipIdentityApplied), zap.String("chip_dsc_sha256", id.ChipDSCSHA256), zap.String("chip_csca_sha256", id.ChipCSCASHA256), } @@ -98,3 +114,30 @@ func bindPortraitToChip(id *facetec.IDScanResult, verifiedDGs []int) { } id.DocumentData.Portrait = "" } + +// applyChipIdentity overwrites the OCR'd identity fields with the chip's +// values. A field the chip did not carry in usable form keeps its OCR value. +// Country codes are normalised to ISO 3166-1 alpha-3, so ICAO's "D" (Germany) +// becomes "DEU" and maps to an alpha-2 code in the credential. +func applyChipIdentity(dd *facetec.DocumentData, chip *emrtd.Identity) { + set := func(dst *string, v string) { + if v != "" { + *dst = v + } + } + set(&dd.FamilyName, chip.FamilyName) + set(&dd.GivenName, chip.GivenName) + set(&dd.DocumentNumber, chip.DocumentNumber) + set(&dd.DateOfBirth, chip.DateOfBirth) + set(&dd.DateOfExpiry, chip.DateOfExpiry) + set(&dd.Sex, chip.Sex) + set(&dd.Nationality, normalizedCountry(chip.Nationality)) + set(&dd.IssuingCountry, normalizedCountry(chip.IssuingState)) +} + +func normalizedCountry(code string) string { + if a3, err := emrtd.NormalizeCountry(code); err == nil { + return a3 + } + return code +} diff --git a/internal/apiv1/chip_test.go b/internal/apiv1/chip_test.go index fa91c00..7f80f2b 100644 --- a/internal/apiv1/chip_test.go +++ b/internal/apiv1/chip_test.go @@ -32,6 +32,15 @@ func (f *fakeEvaluator) EvaluateDSC(context.Context, emrtd.TrustRequest) (emrtd. // chipPayload is a completed process-request response whose documentData // carries real (synthetic) chip data and the matching MRZ-derived fields. +// chipPayloadFor is a completed, chip-authenticated passport scan whose OCR'd +// fields come from ocr and whose chip data comes from chip. +func chipPayloadFor(t *testing.T, chip, ocr *emrtdtest.Chip) string { + t.Helper() + ocrChip := *ocr + ocrChip.Raw = chip.Raw + return chipPayload(t, &ocrChip, 4, true) +} + func chipPayload(t *testing.T, chip *emrtdtest.Chip, authStatus int, withChip bool) string { t.Helper() field := func(k, v string) map[string]any { return map[string]any{"fieldKey": k, "value": v} } @@ -166,21 +175,80 @@ func TestAssessChip_FillsScanResult(t *testing.T) { fields := chipAuditFields(scan.IDScan) enc := zap.NewExample() enc.Info("x", fields...) // must not panic - assert.Len(t, fields, 5) + assert.Len(t, fields, 6) } -func TestAssessChip_MRZMismatchUntrusted(t *testing.T) { +// An OCR'd field that differs from the chip no longer makes the chip +// untrusted: the identity is taken from the chip instead (#83). +func TestAssessChip_OCRMismatchIssuesFromChip(t *testing.T) { chip := emrtdtest.New(emrtdtest.Options{}) c := &Client{chip: emrtd.NewChecker(&fakeEvaluator{dec: emrtd.TrustDecision{Trusted: true, CSCASHA256: "cafe"}}, false)} scan := &facetec.ScanResult{IDScan: facetec.IDScanResult{ ChipRaw: chip.Raw, DocumentData: facetec.DocumentData{ DocumentNumber: "SOMETHINGELSE", DateOfBirth: chip.DateOfBirth, DateOfExpiry: chip.DateOfExpiry, + FamilyName: chip.FamilyName + "K", GivenName: chip.GivenName, Nationality: "SWE", }, }} - require.Nil(t, c.assessChip(t.Context(), scan), "not required: left to policy") + require.Nil(t, c.assessChip(t.Context(), scan)) + assert.True(t, scan.IDScan.ChipTrusted) + assert.True(t, scan.IDScan.ChipIdentityApplied) + dd := scan.IDScan.DocumentData + assert.Equal(t, chip.DocumentNumber, dd.DocumentNumber) + assert.Equal(t, chip.FamilyName, dd.FamilyName) + assert.Equal(t, chip.GivenName, dd.GivenName) + assert.Equal(t, "SWE", dd.Nationality) + assert.Equal(t, "SWE", dd.IssuingCountry) + assert.Equal(t, "F", dd.Sex) +} + +// The chip's identity is used even when the PDP does not trust the document +// signer (e.g. an ID card, where trust is not required): its signature and +// hashes were verified locally, so it is never worse than OCR. +func TestAssessChip_IdentityAppliedWithoutPDPTrust(t *testing.T) { + chip := emrtdtest.New(emrtdtest.Options{DocumentCode: "ID"}) + c := &Client{chip: emrtd.NewChecker(&fakeEvaluator{dec: emrtd.TrustDecision{Trusted: false, Code: "unknown_csca"}}, true)} + scan := &facetec.ScanResult{IDScan: facetec.IDScanResult{ + ChipRaw: chip.Raw, + DocumentData: facetec.DocumentData{DocumentType: "id_card", FamilyName: "WRONG"}, + }} + require.Nil(t, c.assessChip(t.Context(), scan), "trust.required covers passports only") assert.False(t, scan.IDScan.ChipTrusted) - assert.Equal(t, emrtd.ReasonMRZMismatch, scan.IDScan.ChipTrustReason) + assert.True(t, scan.IDScan.ChipIdentityApplied) + assert.Equal(t, chip.FamilyName, scan.IDScan.DocumentData.FamilyName) +} + +// Without verified chip data the OCR values stay. +func TestAssessChip_NoChipDataKeepsOCR(t *testing.T) { + c := &Client{chip: emrtd.NewChecker(&fakeEvaluator{}, false)} + scan := &facetec.ScanResult{IDScan: facetec.IDScanResult{ + DocumentData: facetec.DocumentData{DocumentType: "dl", FamilyName: "OCR NAME"}, + }} + require.Nil(t, c.assessChip(t.Context(), scan)) + assert.False(t, scan.IDScan.ChipIdentityApplied) + assert.Equal(t, "OCR NAME", scan.IDScan.DocumentData.FamilyName) +} + +// A passport whose OCR misread the name used to be refused as chip_untrusted +// under trust.required; it now passes the chip gate (and reaches the policy). +func TestProcessRequest_PassportWithMisreadNameNotRefused(t *testing.T) { + chip := emrtdtest.New(emrtdtest.Options{}) + misread := *chip + misread.FamilyName = chip.FamilyName + "K" + pdp := &fakeEvaluator{dec: emrtd.TrustDecision{Trusted: true, CSCASHA256: "aa"}} + c, ctx := chipClient(t, chipPayloadFor(t, chip, &misread), pdp, true) + resp := process(t, c, ctx) + assert.Equal(t, string(idverrors.CodePolicyRejected), resp.CredentialIssueErrCode) +} + +func TestApplyChipIdentity_GermanyD(t *testing.T) { + dd := facetec.DocumentData{FamilyName: "OCR", Nationality: "D", IssuingCountry: "Germany", DateOfBirth: "1990-01-01"} + applyChipIdentity(&dd, &emrtd.Identity{FamilyName: "MUSTERMANN", Nationality: "D", IssuingState: "D"}) + assert.Equal(t, "MUSTERMANN", dd.FamilyName) + assert.Equal(t, "DEU", dd.Nationality) + assert.Equal(t, "DEU", dd.IssuingCountry) + assert.Equal(t, "1990-01-01", dd.DateOfBirth, "a field the chip did not carry keeps its OCR value") + assert.Equal(t, "DE", toISO3166Alpha2(dd.Nationality)) } func TestSubmitIDScan_LegacyPathNeverChipTrusted(t *testing.T) { diff --git a/internal/emrtd/emrtdtest/emrtdtest.go b/internal/emrtd/emrtdtest/emrtdtest.go index ea9ce5f..1c659d6 100644 --- a/internal/emrtd/emrtdtest/emrtdtest.go +++ b/internal/emrtd/emrtdtest/emrtdtest.go @@ -50,6 +50,13 @@ type Options struct { DocumentCode string // TwoSigners emits two SignerInfos. TwoSigners bool + // DG11Name, when set, adds a DG11 (additional personal details) carrying + // this full name in MRZ form, e.g. "ERIKSSON Date: Tue, 13 Oct 2026 10:55:37 -0400 Subject: [PATCH 2/4] test: cover the chip-identity paths and the test chip builder's new options Codecov reported 75% patch coverage: the new emrtdtest options (DG11, MRZ sex) were only exercised from other packages, which per-package coverage does not count, and a few branches had no test. emrtdtest now has tests of its own that round-trip DG1 and DG11 through gmrtd, and there are tests for the M/F/X sex mapping, an impossible DG1 expiry date (fails closed, dg_malformed) and an ICAO country code with no ISO 3166 equivalent (kept as signed). emrtdtest gains a DG1ExpiryYYMMDD option for the expiry case. --- internal/apiv1/chip_test.go | 7 +++ internal/emrtd/emrtdtest/emrtdtest.go | 7 +++ internal/emrtd/emrtdtest/emrtdtest_test.go | 61 ++++++++++++++++++++++ internal/emrtd/verify_test.go | 19 +++++++ 4 files changed, 94 insertions(+) create mode 100644 internal/emrtd/emrtdtest/emrtdtest_test.go diff --git a/internal/apiv1/chip_test.go b/internal/apiv1/chip_test.go index 7f80f2b..1f9f3ad 100644 --- a/internal/apiv1/chip_test.go +++ b/internal/apiv1/chip_test.go @@ -241,6 +241,13 @@ func TestProcessRequest_PassportWithMisreadNameNotRefused(t *testing.T) { assert.Equal(t, string(idverrors.CodePolicyRejected), resp.CredentialIssueErrCode) } +func TestApplyChipIdentity_UnknownCountryCodeKeptAsSigned(t *testing.T) { + dd := facetec.DocumentData{Nationality: "SWE"} + applyChipIdentity(&dd, &emrtd.Identity{Nationality: "XXA", IssuingState: "UNO"}) + assert.Equal(t, "XXA", dd.Nationality, "an ICAO code with no ISO 3166 equivalent stays as signed") + assert.Equal(t, "UNO", dd.IssuingCountry) +} + func TestApplyChipIdentity_GermanyD(t *testing.T) { dd := facetec.DocumentData{FamilyName: "OCR", Nationality: "D", IssuingCountry: "Germany", DateOfBirth: "1990-01-01"} applyChipIdentity(&dd, &emrtd.Identity{FamilyName: "MUSTERMANN", Nationality: "D", IssuingState: "D"}) diff --git a/internal/emrtd/emrtdtest/emrtdtest.go b/internal/emrtd/emrtdtest/emrtdtest.go index 1c659d6..f26c7cd 100644 --- a/internal/emrtd/emrtdtest/emrtdtest.go +++ b/internal/emrtd/emrtdtest/emrtdtest.go @@ -57,6 +57,10 @@ type Options struct { DG11FullDateOfBirth string // DG1Sex is the MRZ sex character. Default "F". DG1Sex string + // DG1ExpiryYYMMDD, when set, replaces the MRZ expiry date field verbatim + // (its check digit is computed over it), e.g. "311399" for an impossible + // month. + DG1ExpiryYYMMDD string } // Chip is generated chip data plus the certificates involved. @@ -273,6 +277,9 @@ func buildTD3(o Options, c *Chip) string { l1 := code + pad(issuing, 3) + pad(name, 39) doc := pad(c.DocumentNumber, 9) dob, exp := yymmdd(c.DateOfBirth), yymmdd(c.DateOfExpiry) + if o.DG1ExpiryYYMMDD != "" { + exp = o.DG1ExpiryYYMMDD + } opt := pad("", 14) sex := o.DG1Sex if sex == "" { diff --git a/internal/emrtd/emrtdtest/emrtdtest_test.go b/internal/emrtd/emrtdtest/emrtdtest_test.go new file mode 100644 index 0000000..ba42600 --- /dev/null +++ b/internal/emrtd/emrtdtest/emrtdtest_test.go @@ -0,0 +1,61 @@ +package emrtdtest + +import ( + "encoding/base64" + "testing" + + "github.com/gmrtd/gmrtd/document" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func dataGroup(t *testing.T, c *Chip, name string) []byte { + t.Helper() + b64, ok := c.Raw[name] + require.True(t, ok, "%s present", name) + b, err := base64.StdEncoding.DecodeString(b64) + require.NoError(t, err) + return b +} + +func TestNew_DefaultsHaveNoDG11(t *testing.T) { + c := New(Options{}) + assert.Contains(t, c.Raw, "SOD") + assert.Contains(t, c.Raw, "DG1") + assert.Contains(t, c.Raw, "DG2") + assert.NotContains(t, c.Raw, "DG11") + + dg1, err := document.NewDG1(dataGroup(t, c, "DG1")) + require.NoError(t, err) + assert.Equal(t, "F", dg1.Mrz.Sex) + assert.Equal(t, "ERIKSSON", dg1.Mrz.NameOfHolder.Primary) +} + +func TestNew_DG11RoundTrips(t *testing.T) { + c := New(Options{DG11Name: "ERIKSSON Date: Fri, 16 Oct 2026 08:02:10 -0400 Subject: [PATCH 3/4] Make the chip authoritative for empty identity fields too applyChipIdentity no longer keeps an OCR'd value where the chip leaves a field empty: a single-name holder has no given name, an MRZ date of birth with unknown parts gives no birth_date or age claims, and an unspecified sex is ISO/IEC 5218 0. None of these is filled in from OCR. Refs #83 --- README.md | 8 +++++-- internal/apiv1/chip.go | 32 +++++++++++++-------------- internal/apiv1/chip_test.go | 32 ++++++++++++++++++++++++++- internal/emrtd/emrtdtest/emrtdtest.go | 12 ++++++++++ 4 files changed, 65 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 94fe338..6a735f8 100644 --- a/README.md +++ b/README.md @@ -354,8 +354,12 @@ credential's identity fields come from the chip, not from FaceTec's OCR of the M names, document number, dates of birth and expiry, nationality, issuing state and sex from DG1 (the signed MRZ), refined by DG11 (full name, date of birth with century) when the chip carries it. This holds whether or not the PDP trusts the signer, since signed chip data is never worse -than OCR. Where the OCR differed, the fields are logged by name (never values). Documents without -an ICAO chip (e.g. an ISO 18013 driving licence) keep the OCR values. +than OCR. The chip is authoritative for every identity field, including the ones it leaves empty: +a holder with a single name gets no given name, an MRZ date of birth with unknown parts (e.g. +`85<<<<`) gives no `birth_date` or age claims, and an unspecified sex is issued as ISO/IEC 5218 +`0` (not known); none of these is filled in from OCR. Where the OCR differed, the fields are +logged by name (never values). Documents without an ICAO chip (e.g. an ISO 18013 driving +licence) keep the OCR values for now (#85). ## ETSI 119 461 §4.5 Compliance diff --git a/internal/apiv1/chip.go b/internal/apiv1/chip.go index 8413091..897ba85 100644 --- a/internal/apiv1/chip.go +++ b/internal/apiv1/chip.go @@ -115,24 +115,24 @@ func bindPortraitToChip(id *facetec.IDScanResult, verifiedDGs []int) { id.DocumentData.Portrait = "" } -// applyChipIdentity overwrites the OCR'd identity fields with the chip's -// values. A field the chip did not carry in usable form keeps its OCR value. +// applyChipIdentity replaces the OCR'd identity fields with the chip's values. +// The chip is authoritative for every one of them, including the ones it +// leaves empty: a holder with a single name has no given name, an MRZ date of +// birth with unknown parts (e.g. "85<<<<") yields none, and an unspecified sex +// ('<') is unknown. An empty field is left out of the credential rather than +// filled from OCR, which is trivially falsifiable (#83, #85). // Country codes are normalised to ISO 3166-1 alpha-3, so ICAO's "D" (Germany) -// becomes "DEU" and maps to an alpha-2 code in the credential. +// becomes "DEU" and maps to an alpha-2 code in the credential; a code with no +// ISO 3166 equivalent stays as signed. func applyChipIdentity(dd *facetec.DocumentData, chip *emrtd.Identity) { - set := func(dst *string, v string) { - if v != "" { - *dst = v - } - } - set(&dd.FamilyName, chip.FamilyName) - set(&dd.GivenName, chip.GivenName) - set(&dd.DocumentNumber, chip.DocumentNumber) - set(&dd.DateOfBirth, chip.DateOfBirth) - set(&dd.DateOfExpiry, chip.DateOfExpiry) - set(&dd.Sex, chip.Sex) - set(&dd.Nationality, normalizedCountry(chip.Nationality)) - set(&dd.IssuingCountry, normalizedCountry(chip.IssuingState)) + dd.FamilyName = chip.FamilyName + dd.GivenName = chip.GivenName + dd.DocumentNumber = chip.DocumentNumber + dd.DateOfBirth = chip.DateOfBirth + dd.DateOfExpiry = chip.DateOfExpiry + dd.Sex = chip.Sex + dd.Nationality = normalizedCountry(chip.Nationality) + dd.IssuingCountry = normalizedCountry(chip.IssuingState) } func normalizedCountry(code string) string { diff --git a/internal/apiv1/chip_test.go b/internal/apiv1/chip_test.go index 1f9f3ad..b5d4a15 100644 --- a/internal/apiv1/chip_test.go +++ b/internal/apiv1/chip_test.go @@ -254,10 +254,40 @@ func TestApplyChipIdentity_GermanyD(t *testing.T) { assert.Equal(t, "MUSTERMANN", dd.FamilyName) assert.Equal(t, "DEU", dd.Nationality) assert.Equal(t, "DEU", dd.IssuingCountry) - assert.Equal(t, "1990-01-01", dd.DateOfBirth, "a field the chip did not carry keeps its OCR value") + assert.Empty(t, dd.DateOfBirth, "a field the chip leaves empty is not filled from OCR") assert.Equal(t, "DE", toISO3166Alpha2(dd.Nationality)) } +// The chip is authoritative for the fields it leaves empty too: a holder with +// a single name, a date of birth with unknown parts and an unspecified sex are +// issued as such, never completed from the OCR'd values. +func TestAssessChip_EmptyChipFieldsNotFilledFromOCR(t *testing.T) { + chip := emrtdtest.New(emrtdtest.Options{NoGivenName: true, DG1BirthYYMMDD: "85<<<<", DG1Sex: "<"}) + c := &Client{chip: emrtd.NewChecker(&fakeEvaluator{dec: emrtd.TrustDecision{Trusted: true, CSCASHA256: "cafe"}}, true)} + scan := &facetec.ScanResult{IDScan: facetec.IDScanResult{ + ChipRaw: chip.Raw, + DocumentData: facetec.DocumentData{ + DocumentType: "passport", FamilyName: chip.FamilyName, GivenName: "OCR GIVEN", + DocumentNumber: chip.DocumentNumber, DateOfBirth: "1985-01-01", DateOfExpiry: chip.DateOfExpiry, + Sex: "M", Nationality: "SWE", + }, + }} + require.Nil(t, c.assessChip(t.Context(), scan)) + require.True(t, scan.IDScan.ChipIdentityApplied) + dd := scan.IDScan.DocumentData + assert.Equal(t, chip.FamilyName, dd.FamilyName) + assert.Empty(t, dd.GivenName) + assert.Empty(t, dd.DateOfBirth) + assert.Empty(t, dd.Sex) + + claims := MapPhotoIDClaims(dd, "ft-abc", PhotoIDIssuer{}, photoIDNow) + assert.Equal(t, "", claims["given_name_unicode"]) + assert.Equal(t, 0, claims["sex"], "ISO/IEC 5218 not known") + for _, k := range []string{"birth_date", "age_over_18", "age_in_years", "age_birth_year"} { + assert.NotContains(t, claims, k) + } +} + func TestSubmitIDScan_LegacyPathNeverChipTrusted(t *testing.T) { c, livenessID := newTestClientForIDScan(t, `{ "success": true, "faceMatchLevel": 7, "nfcVerified": true, "mrzVerified": true, "barcodeVerified": true, diff --git a/internal/emrtd/emrtdtest/emrtdtest.go b/internal/emrtd/emrtdtest/emrtdtest.go index f26c7cd..2192df6 100644 --- a/internal/emrtd/emrtdtest/emrtdtest.go +++ b/internal/emrtd/emrtdtest/emrtdtest.go @@ -57,6 +57,12 @@ type Options struct { DG11FullDateOfBirth string // DG1Sex is the MRZ sex character. Default "F". DG1Sex string + // NoGivenName encodes a holder with a single name (primary identifier + // only, "ERIKSSON<<"). + NoGivenName bool + // DG1BirthYYMMDD, when set, replaces the MRZ date of birth field verbatim + // (its check digit is computed over it), e.g. "85<<<<" for unknown parts. + DG1BirthYYMMDD string // DG1ExpiryYYMMDD, when set, replaces the MRZ expiry date field verbatim // (its check digit is computed over it), e.g. "311399" for an impossible // month. @@ -142,6 +148,9 @@ func New(o Options) *Chip { FamilyName: "ERIKSSON", GivenName: "ANNA MARIA", } + if o.NoGivenName { + c.GivenName = "" + } cscaKey := must(ecdsa.GenerateKey(elliptic.P256(), rand.Reader)) dscKey := must(ecdsa.GenerateKey(elliptic.P256(), rand.Reader)) @@ -280,6 +289,9 @@ func buildTD3(o Options, c *Chip) string { if o.DG1ExpiryYYMMDD != "" { exp = o.DG1ExpiryYYMMDD } + if o.DG1BirthYYMMDD != "" { + dob = o.DG1BirthYYMMDD + } opt := pad("", 14) sex := o.DG1Sex if sex == "" { From 7155f658e90d11e95308cc26c31bd091c4ab5ff7 Mon Sep 17 00:00:00 2001 From: Jesse van Muijden Date: Fri, 16 Oct 2026 10:46:29 -0400 Subject: [PATCH 4/4] Take the portrait from the chip whenever its identity is applied bindPortraitToChip only ran for a PDP-trusted chip, so a chip that verified locally without PDP trust (e.g. an ID card) got the chip's identity next to FaceTec's photoIDFaceCrop, cropped from the camera image of the document. The portrait now follows the identity: the SOD-verified DG2 image, or none. Refs #83 --- README.md | 6 +++-- internal/apiv1/chip.go | 15 ++++++------ internal/apiv1/chip_test.go | 36 ++++++++++++++++++++++++++++- internal/facetec/process_request.go | 10 ++++---- 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 6a735f8..5f5d10b 100644 --- a/README.md +++ b/README.md @@ -357,9 +357,11 @@ it. This holds whether or not the PDP trusts the signer, since signed chip data than OCR. The chip is authoritative for every identity field, including the ones it leaves empty: a holder with a single name gets no given name, an MRZ date of birth with unknown parts (e.g. `85<<<<`) gives no `birth_date` or age claims, and an unspecified sex is issued as ISO/IEC 5218 -`0` (not known); none of these is filled in from OCR. Where the OCR differed, the fields are +`0` (not known); none of these is filled in from OCR. The portrait follows the same rule: it is +the SOD-verified DG2 face image, never FaceTec's `photoIDFaceCrop` from the camera image, and is +left out when DG2 was not verified. Where the OCR differed, the fields are logged by name (never values). Documents without an ICAO chip (e.g. an ISO 18013 driving -licence) keep the OCR values for now (#85). +licence) keep the OCR values and FaceTec's portrait for now (#85). ## ETSI 119 461 §4.5 Compliance diff --git a/internal/apiv1/chip.go b/internal/apiv1/chip.go index 897ba85..a93c329 100644 --- a/internal/apiv1/chip.go +++ b/internal/apiv1/chip.go @@ -54,6 +54,7 @@ func (c *Client) assessChip(ctx context.Context, scan *facetec.ScanResult) *idve // chip signed is never worse than OCR of the page printed next to it. if local := out.Local; local != nil && local.Identity != nil { applyChipIdentity(&id.DocumentData, local.Identity) + bindPortraitToChip(id, local.DataGroups) id.ChipIdentityApplied = true if len(local.Mismatches) > 0 && c.log != nil { c.log.Info("chip identity differs from OCR; issuing from the chip", @@ -70,9 +71,6 @@ func (c *Client) assessChip(ctx context.Context, scan *facetec.ScanResult) *idve // Any sign of a chip read counts as presented, so an incomplete payload // (status reported but SOD missing, stray raw fields) cannot dodge the gate. presented := out.ChipPresented || id.ChipAuthStatus != 0 || len(id.ChipRaw) > 0 || id.NFCVerified - if out.Trusted { - bindPortraitToChip(id, out.Local.DataGroups) - } // trust.required is enforced for passports (and for scans that reported no // document type at all, which fail closed) only: the PDP's eMRTD registry // covers ICAO 9303 travel documents, and other documents' chips (e.g. an @@ -103,10 +101,13 @@ func chipAuditFields(id facetec.IDScanResult) []zap.Field { } } -// bindPortraitToChip makes sure a chip-trusted scan only carries a portrait -// the SOD vouches for. When DG2's hash was verified, the DG2 face image -// replaces whatever FaceTec supplied (e.g. photoIDFaceCrop); when it was not, -// the portrait is dropped rather than issued under a chip-trusted credential. +// bindPortraitToChip makes sure a scan issued with the chip's identity only +// carries a portrait the SOD vouches for, whether or not the PDP trusts the +// signer: the portrait follows the same source as the name and dates. When +// DG2's hash was verified, the DG2 face image replaces whatever FaceTec +// supplied (e.g. photoIDFaceCrop, cropped from the camera image of the +// document); when it was not, the portrait is dropped rather than issued +// next to the chip's identity. func bindPortraitToChip(id *facetec.IDScanResult, verifiedDGs []int) { if slices.Contains(verifiedDGs, 2) && id.ChipPortrait != "" { id.DocumentData.Portrait = id.ChipPortrait diff --git a/internal/apiv1/chip_test.go b/internal/apiv1/chip_test.go index b5d4a15..a72812a 100644 --- a/internal/apiv1/chip_test.go +++ b/internal/apiv1/chip_test.go @@ -363,7 +363,41 @@ func TestAssessChip_TrustedPortraitBoundToDG2(t *testing.T) { scan.IDScan.ChipPortrait = "" scan.IDScan.DocumentData.Portrait = "swapped-crop" require.Nil(t, c.assessChip(t.Context(), scan)) - assert.Empty(t, scan.IDScan.DocumentData.Portrait, "a portrait the SOD does not cover is not issued under a trusted chip") + assert.Empty(t, scan.IDScan.DocumentData.Portrait, "a portrait the SOD does not cover is not issued with the chip's identity") +} + +// The portrait follows the identity: a chip whose signature and hashes verify +// but whose signer the PDP does not trust (an ID card, where trust is not +// required) still replaces FaceTec's photoIDFaceCrop with the DG2 image, so +// the credential never pairs the chip's name with a crop from the camera. +func TestAssessChip_PortraitBoundToDG2WithoutPDPTrust(t *testing.T) { + chip := emrtdtest.New(emrtdtest.Options{DocumentCode: "ID"}) + c := &Client{chip: emrtd.NewChecker(&fakeEvaluator{dec: emrtd.TrustDecision{Trusted: false, Code: "unknown_csca"}}, true)} + scan := &facetec.ScanResult{IDScan: facetec.IDScanResult{ + ChipRaw: chip.Raw, + ChipPortrait: "dg2-portrait", + DocumentData: facetec.DocumentData{DocumentType: "id_card", Portrait: "camera-crop"}, + }} + require.Nil(t, c.assessChip(t.Context(), scan)) + require.False(t, scan.IDScan.ChipTrusted) + require.True(t, scan.IDScan.ChipIdentityApplied) + assert.Equal(t, "dg2-portrait", scan.IDScan.DocumentData.Portrait) + + scan.IDScan.ChipPortrait = "" + scan.IDScan.DocumentData.Portrait = "camera-crop" + require.Nil(t, c.assessChip(t.Context(), scan)) + assert.Empty(t, scan.IDScan.DocumentData.Portrait, "no crop next to the chip's identity") +} + +// Without verified chip data the portrait FaceTec supplied stays, like the +// OCR'd fields (#85). +func TestAssessChip_NoChipDataKeepsPortrait(t *testing.T) { + c := &Client{chip: emrtd.NewChecker(&fakeEvaluator{}, false)} + scan := &facetec.ScanResult{IDScan: facetec.IDScanResult{ + DocumentData: facetec.DocumentData{DocumentType: "dl", Portrait: "camera-crop"}, + }} + require.Nil(t, c.assessChip(t.Context(), scan)) + assert.Equal(t, "camera-crop", scan.IDScan.DocumentData.Portrait) } func TestAssessChip_RequiredRejectsMalformedRawData(t *testing.T) { diff --git a/internal/facetec/process_request.go b/internal/facetec/process_request.go index 5064845..23f6289 100644 --- a/internal/facetec/process_request.go +++ b/internal/facetec/process_request.go @@ -142,10 +142,12 @@ func ExtractScanResult(payload map[string]any) (*ScanResult, bool, error) { // parseFaceTecGroupedFields (extracted from the NFC chip's DG2 face // image -- see extractPortraitFromDG2). Some FaceTec configurations may // additionally return a separate, pre-cropped face photo directly under - // "photoIDFaceCrop"; prefer that when present, since it's already - // cropped/normalized, but don't clobber the NFC-derived portrait with an - // empty string when it's absent (confirmed absent in this deployment's - // FaceTec Server responses as of 2026-07-29). + // "photoIDFaceCrop" (confirmed absent in this deployment's FaceTec Server + // responses as of 2026-07-29); it is taken here, but it comes from the + // camera image of the document, not the chip. Whenever the chip's + // identity is applied, apiv1's bindPortraitToChip replaces it with the + // SOD-verified DG2 image kept in ChipPortrait, or drops it. Without a + // verified chip the crop stays (#85). chipPortrait := extractPortraitFromDG2(documentDataMap(results["documentData"])) if portrait, ok := lookupString(results["photoIDFaceCrop"]); ok { documentData.Portrait = portrait