fix(mdocrical): stop enforcing isTrustAnchor, it's being removed from the standard

pkg/registry/mdocrical/registry.go

@@ -78,6 +78,14 @@ type Config struct {
 	// CryptoExt provides extensible certificate parsing for non-standard
 	// curves (e.g. brainpool). If nil, standard x509.ParseCertificate is
 	// used.
+	//
+	// Note: this registry does not enforce CertificateInfo.IsTrustAnchor as
+	// a gate on path validation, even though F.3.2.2 currently documents it
+	// as Required - the interop event organizers have confirmed
+	// isTrustAnchor is being removed from the ISO/IEC 18013-5 standard
+	// going forward, and real published RICALs (e.g. the Geneva 2026
+	// event's live document at geneva2026.mdoc.online) already omit it in
+	// practice. See validateChainAgainstAnchors's doc comment for details.
 	CryptoExt *gocryptoutil.Extensions
 }
 
@@ -92,9 +100,11 @@ type TrustConstraint struct {
 // RICALCertificateInfo mirrors the RICAL structure's RICALCertificateInfo
 // (F.3.2.2).
 type RICALCertificateInfo struct {
-	Certificate      []byte            `cbor:"certificate"`
-	SerialNumber     *big.Int          `cbor:"serialNumber"`
-	SKI              []byte            `cbor:"ski"`
+	Certificate  []byte   `cbor:"certificate"`
+	SerialNumber *big.Int `cbor:"serialNumber"`
+	SKI          []byte   `cbor:"ski"`
+	// IsTrustAnchor is decoded for completeness but not enforced as a gate
+	// anywhere in this package - see Config.CryptoExt's doc comment for why.
 	IsTrustAnchor    bool              `cbor:"isTrustAnchor"`
 	AKI              []byte            `cbor:"aki,omitempty"`
 	Type             string            `cbor:"type,omitempty"`
@@ -423,38 +433,57 @@ func findFirstMatchingCertificateInfo(chain []*x509.Certificate, infos []RICALCe
 	return nil, -1
 }
 
-// validateChainAgainstAnchors builds a root pool from every
-// isTrustAnchor=true CertificateInfo and validates the presented chain
-// against it, using any non-anchor CertificateInfo entries (and the
-// presented chain's own intermediates) to help complete the path. Returns
+// validateChainAgainstAnchors builds a root pool from every CertificateInfo
+// in the RICAL and validates the presented chain against it, using the
+// presented chain's own intermediates to help complete the path. Returns
 // every verified chain (leaf-to-root) x509.Verify found, so a caller can
-// identify which specific RICAL trust anchor the chain actually resolved
-// to - see resolveCertificateInfo's doc comment for why that matters.
+// identify which specific RICAL entry the chain actually resolved to - see
+// resolveCertificateInfo's doc comment for why that matters.
+//
+// isTrustAnchor is deliberately NOT enforced as a gate here, even though
+// F.3.2.2 currently documents it as Required: the interop event organizers
+// have confirmed isTrustAnchor is being removed from the ISO/IEC 18013-5
+// standard going forward (word received directly from the organizers, not
+// inferred), and real published RICALs already omit it in practice - the
+// Geneva 2026 event's live document (geneva2026.mdoc.online) has it absent
+// on all 35 published certificateInfos entries. Enforcing a field the
+// standard itself is dropping would deny every reader against any
+// spec-current or spec-future RICAL, not just a non-conformant one - so
+// every entry is treated as usable for path validation regardless of this
+// field's presence or value, both today and once the field is gone
+// entirely.
 func validateChainAgainstAnchors(chain []*x509.Certificate, infos []RICALCertificateInfo, ext *gocryptoutil.Extensions) ([][]*x509.Certificate, error) {
 	if len(chain) == 0 {
 		return nil, fmt.Errorf("empty chain")
 	}
 
 	roots := x509.NewCertPool()
-	intermediates := x509.NewCertPool()
 	haveAnchor := false
 	for _, info := range infos {
 		cert, err := registry.ParseCertificate(info.Certificate, ext)
 		if err != nil {
 			continue
 		}
-		if info.IsTrustAnchor {
-			roots.AddCert(cert)
-			haveAnchor = true
-		} else {
-			intermediates.AddCert(cert)
+		// Only CA certificates are eligible as path-validation roots. A
+		// RICAL entry can also be a reader's own leaf/intermediate cert,
+		// enrolled solely to carry TrustConstraints per F.3.2.6 - adding
+		// those to the root pool would let a chain "validate" to a
+		// non-anchor cert just because it's listed, which is a different
+		// (and much weaker) trust question than "does this chain lead to a
+		// CA the RICAL vouches for". Non-CA entries are still reachable via
+		// findFirstMatchingCertificateInfo's exact chain-member match.
+		if !cert.IsCA {
+			continue
 		}
+		roots.AddCert(cert)
+		haveAnchor = true
 	}
 	if !haveAnchor {
-		return nil, fmt.Errorf("RICAL has no isTrustAnchor=true certificate")
+		return nil, fmt.Errorf("RICAL has no usable certificate")
 	}
 
 	leaf := chain[0]
+	intermediates := x509.NewCertPool()
 	for _, c := range chain[1:] {
 		intermediates.AddCert(c)
 	}
@@ -514,9 +543,6 @@ func resolveCertificateInfo(chain []*x509.Certificate, infos []RICALCertificateI
 	for _, verifiedChain := range verifiedChains {
 		root := verifiedChain[len(verifiedChain)-1]
 		for i := range infos {
-			if !infos[i].IsTrustAnchor {
-				continue
-			}
 			infoCert, err := registry.ParseCertificate(infos[i].Certificate, ext)
 			if err != nil {
 				continue
@@ -527,9 +553,9 @@ func resolveCertificateInfo(chain []*x509.Certificate, infos []RICALCertificateI
 		}
 	}
 	// Should be unreachable: validateChainAgainstAnchors only builds its
-	// root pool from isTrustAnchor entries, so a successful Verify's root
-	// must be one of them.
-	return nil, -1, fmt.Errorf("chain validated but its root isn't a RICAL trust anchor entry")
+	// root pool from parseable RICAL entries, so a successful Verify's
+	// root must be one of them.
+	return nil, -1, fmt.Errorf("chain validated but its root isn't a RICAL entry")
 }
 
 // anyTrustConstraintSatisfied is a placeholder: this specification (per

pkg/registry/mdocrical/registry_test.go

@@ -339,6 +339,193 @@ func TestEvaluate_TrustedReaderChainOmittingRoot(t *testing.T) {
 	}
 }
 
+// TestEvaluate_TrustedDespiteMissingIsTrustAnchor documents that isTrustAnchor
+// is not enforced as a gate: a RICAL CertificateInfo whose isTrustAnchor is
+// false (F.3.2.2 currently documents it as Required) still trusts a chain
+// that validates to it. The field isn't read anywhere in this package's
+// decision-making, so whether a real producer encodes it as false or omits
+// the CBOR key entirely (as the Geneva 2026 event's live document does -
+// geneva2026.mdoc.online has it absent on all 35 published entries) makes no
+// behavioral difference; this fixture exercises the encoded-false case since
+// the struct tag has no `omitempty` and always writes the key. The interop
+// event organizers have confirmed isTrustAnchor is being removed from the
+// ISO/IEC 18013-5 standard going forward.
+func TestEvaluate_TrustedDespiteMissingIsTrustAnchor(t *testing.T) {
+	ricalRoot, ricalRootKey := generateCA(t, "Test RICAL Root")
+	signerCert, signerKey := generateLeaf(t, ricalRoot, ricalRootKey, "Test RICAL Signer", 2)
+
+	readerCA, readerCAKey := generateCA(t, "Test Reader CA")
+	readerLeaf, _ := generateLeaf(t, readerCA, readerCAKey, "Test Reader", 3)
+
+	rical := &RICAL{
+		Version:  "1.0",
+		Provider: "test-provider",
+		Date:     time.Now().UTC().Format(time.RFC3339),
+		Type:     "org.iso.18013.5.1.reader_authentication",
+		CertificateInfos: []RICALCertificateInfo{
+			{
+				Certificate:  readerCA.Raw,
+				SerialNumber: readerCA.SerialNumber,
+				SKI:          readerCA.SubjectKeyId,
+				// IsTrustAnchor left at its Go zero value (false) - see the
+				// doc comment above for why the CBOR-encoded-false vs
+				// key-absent distinction doesn't matter here.
+			},
+		},
+	}
+	body := buildSignedRical(t, rical, signerCert, signerKey)
+	mock := newMockRicalServer(t, body)
+	defer mock.Close()
+
+	reg, err := New(&Config{
+		RicalProviderURL:        mock.URL(),
+		RicalRootCertificatePEM: certPEM(ricalRoot),
+		AllowHTTP:               true,
+		AllowPrivateIPs:         true,
+	})
+	if err != nil {
+		t.Fatalf("New() error = %v", err)
+	}
+
+	req := &authzen.EvaluationRequest{
+		Resource: authzen.Resource{
+			Type: "x5c",
+			Key:  []interface{}{certBase64(readerLeaf), certBase64(readerCA)},
+		},
+	}
+
+	resp, err := reg.Evaluate(context.Background(), req)
+	if err != nil {
+		t.Fatalf("Evaluate() error = %v", err)
+	}
+	if !resp.Decision {
+		t.Fatalf("expected trusted decision even though no CertificateInfo has isTrustAnchor=true, got denied: %+v", resp.Context)
+	}
+}
+
+// TestEvaluate_SkipsUnparseableCertificateInfoEntry documents that a
+// malformed/unparseable CertificateInfo entry in the RICAL doesn't abort
+// evaluation for the rest - validateChainAgainstAnchors builds its root pool
+// from every entry it CAN parse, silently skipping ones it can't, so a single
+// bad entry doesn't deny readers that validate against a different, good
+// entry.
+func TestEvaluate_SkipsUnparseableCertificateInfoEntry(t *testing.T) {
+	ricalRoot, ricalRootKey := generateCA(t, "Test RICAL Root")
+	signerCert, signerKey := generateLeaf(t, ricalRoot, ricalRootKey, "Test RICAL Signer", 2)
+
+	readerCA, readerCAKey := generateCA(t, "Test Reader CA")
+	readerLeaf, _ := generateLeaf(t, readerCA, readerCAKey, "Test Reader", 3)
+
+	rical := &RICAL{
+		Version:  "1.0",
+		Provider: "test-provider",
+		Date:     time.Now().UTC().Format(time.RFC3339),
+		Type:     "org.iso.18013.5.1.reader_authentication",
+		CertificateInfos: []RICALCertificateInfo{
+			{
+				Certificate: []byte("not-a-real-certificate"),
+			},
+			{
+				Certificate:  readerCA.Raw,
+				SerialNumber: readerCA.SerialNumber,
+				SKI:          readerCA.SubjectKeyId,
+			},
+		},
+	}
+	body := buildSignedRical(t, rical, signerCert, signerKey)
+	mock := newMockRicalServer(t, body)
+	defer mock.Close()
+
+	reg, err := New(&Config{
+		RicalProviderURL:        mock.URL(),
+		RicalRootCertificatePEM: certPEM(ricalRoot),
+		AllowHTTP:               true,
+		AllowPrivateIPs:         true,
+	})
+	if err != nil {
+		t.Fatalf("New() error = %v", err)
+	}
+
+	req := &authzen.EvaluationRequest{
+		Resource: authzen.Resource{
+			Type: "x5c",
+			Key:  []interface{}{certBase64(readerLeaf), certBase64(readerCA)},
+		},
+	}
+
+	resp, err := reg.Evaluate(context.Background(), req)
+	if err != nil {
+		t.Fatalf("Evaluate() error = %v", err)
+	}
+	if !resp.Decision {
+		t.Fatalf("expected trusted decision despite an unparseable CertificateInfo entry, got denied: %+v", resp.Context)
+	}
+}
+
+// TestEvaluate_NonCACertificateInfoNotUsedAsRoot documents that a
+// non-CA CertificateInfo entry (e.g. a reader's own leaf, enrolled solely to
+// carry TrustConstraints per F.3.2.6) is skipped when building the
+// path-validation root pool - it can't itself act as a trust anchor, even
+// though isTrustAnchor is no longer enforced. A reader chain still validates
+// successfully via a separate, genuine CA entry in the same RICAL.
+func TestEvaluate_NonCACertificateInfoNotUsedAsRoot(t *testing.T) {
+	ricalRoot, ricalRootKey := generateCA(t, "Test RICAL Root")
+	signerCert, signerKey := generateLeaf(t, ricalRoot, ricalRootKey, "Test RICAL Signer", 2)
+
+	readerCA, readerCAKey := generateCA(t, "Test Reader CA")
+	readerLeaf, _ := generateLeaf(t, readerCA, readerCAKey, "Test Reader", 3)
+
+	otherCA, otherCAKey := generateCA(t, "Test Other CA")
+	nonCALeaf, _ := generateLeaf(t, otherCA, otherCAKey, "Test Non-CA Entry", 4)
+
+	rical := &RICAL{
+		Version:  "1.0",
+		Provider: "test-provider",
+		Date:     time.Now().UTC().Format(time.RFC3339),
+		Type:     "org.iso.18013.5.1.reader_authentication",
+		CertificateInfos: []RICALCertificateInfo{
+			{
+				Certificate:  nonCALeaf.Raw,
+				SerialNumber: nonCALeaf.SerialNumber,
+				SKI:          nonCALeaf.SubjectKeyId,
+			},
+			{
+				Certificate:  readerCA.Raw,
+				SerialNumber: readerCA.SerialNumber,
+				SKI:          readerCA.SubjectKeyId,
+			},
+		},
+	}
+	body := buildSignedRical(t, rical, signerCert, signerKey)
+	mock := newMockRicalServer(t, body)
+	defer mock.Close()
+
+	reg, err := New(&Config{
+		RicalProviderURL:        mock.URL(),
+		RicalRootCertificatePEM: certPEM(ricalRoot),
+		AllowHTTP:               true,
+		AllowPrivateIPs:         true,
+	})
+	if err != nil {
+		t.Fatalf("New() error = %v", err)
+	}
+
+	req := &authzen.EvaluationRequest{
+		Resource: authzen.Resource{
+			Type: "x5c",
+			Key:  []interface{}{certBase64(readerLeaf), certBase64(readerCA)},
+		},
+	}
+
+	resp, err := reg.Evaluate(context.Background(), req)
+	if err != nil {
+		t.Fatalf("Evaluate() error = %v", err)
+	}
+	if !resp.Decision {
+		t.Fatalf("expected trusted decision via the genuine CA entry, got denied: %+v", resp.Context)
+	}
+}
+
 func TestEvaluate_RicalSignedByWrongRoot(t *testing.T) {
 	ricalRoot, _ := generateCA(t, "Real RICAL Root")
 	// Signed by a DIFFERENT root than the one configured as trusted.
@@ -430,3 +617,23 @@ func TestRefresh_ClearsCache(t *testing.T) {
 		t.Fatal("expected fetch to fail after Refresh() cleared the cache while the server is down")
 	}
 }
+
+func TestValidateChainAgainstAnchors_EmptyChain(t *testing.T) {
+	_, err := validateChainAgainstAnchors(nil, nil, nil)
+	if err == nil {
+		t.Fatal("expected error for an empty chain")
+	}
+}
+
+func TestValidateChainAgainstAnchors_NoUsableCertificate(t *testing.T) {
+	readerCA, readerCAKey := generateCA(t, "Test Reader CA")
+	readerLeaf, _ := generateLeaf(t, readerCA, readerCAKey, "Test Reader", 2)
+
+	infos := []RICALCertificateInfo{
+		{Certificate: []byte("not-a-real-certificate")},
+	}
+	_, err := validateChainAgainstAnchors([]*x509.Certificate{readerLeaf, readerCA}, infos, nil)
+	if err == nil {
+		t.Fatal("expected error when the RICAL has no parseable certificate")
+	}
+}