diff --git a/pkg/registry/mdocrical/registry.go b/pkg/registry/mdocrical/registry.go index de9ffe5..0e9f73d 100644 --- a/pkg/registry/mdocrical/registry.go +++ b/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 diff --git a/pkg/registry/mdocrical/registry_test.go b/pkg/registry/mdocrical/registry_test.go index 8b7c05d..2558fca 100644 --- a/pkg/registry/mdocrical/registry_test.go +++ b/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") + } +}