diff --git a/CHANGELOG.md b/CHANGELOG.md index 5891c9d7..a2f16701 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,53 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add shell tab completion for all enum flags (`--format`, `--algorithm`, `--curve`, `--log-level`, `--trust-store`) and directory flags (`--bundle-path`, `--out-path`) ([#56]) +- Add `ValidateAIAURL` to block SSRF via non-HTTP schemes and literal private/loopback IP addresses in AIA URLs ([#56]) +- Add shell tab completion for all enum flags (`--format`, `--algorithm`, `--curve`, `--log-level`, `--trust-store`), directory flags (`--bundle-path`, `--out-path`), and file flags (`--out-file`) ([#56]) +- Add expired and untrusted certificate counts to scan summary (e.g., `Leaves: 6 (2 expired, 1 untrusted)`) ([`b5969b0`]) +- Add AIA resolution to scan summary path — fetch missing intermediates before trust checking ([`b5969b0`]) +- Add expired and trusted status to `inspect` command output for each certificate ([`b5969b0`]) +- Add `VerifyChainTrust` shared function for consistent chain verification across CLI, WASM, inspect, and `--dump-certs` ([#56]) +- Strengthen `IsMozillaRoot` to verify public key in addition to Subject — prevents spoofed trust anchors ([#56]) + +### Changed + +- **Breaking:** `VerifyChainTrust` now takes a `VerifyChainTrustInput` struct instead of positional arguments (CS-5 compliance) ([#57]) +- Use `NotBefore + 1s` instead of `NotAfter - 1s` for expired certificate time-shift in chain verification — more robust when intermediates expired before the leaf ([#56]) ### Fixed +- Fix bare `return err` without context wrapping (ERR-1) in scan command `MozillaRootPool` and `httpAIAFetcher` calls ([#57]) +- Fix silent error swallowing (ERR-5) in `mozillaRootPublicKeys`, `ResolveAIA`, and WASM `json.Marshal` calls — now log with `slog` ([#57]) +- Fix WASM `jsFetchURL` catch callback panic when JS promise rejects with null or non-Error value ([#57]) +- Fix WASM `exportBundlesJS` missing `defer` on `RUnlock` — panic during export would permanently deadlock the store ([#57]) +- Fix SSRF bypass via unspecified addresses (`0.0.0.0`, `::`) in `ValidateAIAURL` ([#57]) +- Fix SSRF bypass via CGN/shared address space (`100.64.0.0/10`) in `ValidateAIAURL` ([#57]) +- Fix `ValidateAIAURL` re-parsing CIDR ranges on every call — now parsed once at init ([#57]) +- Fix missing SSRF validation in `ResolveAIA` — AIA URLs are now validated before fetching, not just in caller-provided callbacks ([#57]) +- Fix `VerifyChainTrust` silently falling back to system roots when `roots` is nil — now returns false ([#57]) +- Fix WASM `jsFetchURL` accepting unbounded response data — now enforces 1MB limit consistent with CLI ([#57]) +- Fix WASM `getState`/`resetStore` deadlocking JS event loop when AIA resolution holds the store lock — now uses `TryRLock`/`TryLock` ([#57]) +- Fix WASM `js.FuncOf` promise executor callbacks leaking in `addFiles`, `exportBundlesJS`, and `jsError` — now released after `Promise.New` ([#57]) +- Fix WASM `jsFetchURL` panic when context is cancelled before JS promise settles — callbacks are no longer released prematurely ([#57]) +- Fix WASM `addFiles` leaking `js.FuncOf` callback on every AIA completion notification ([#57]) +- Fix WASM `jsFetchURL` ignoring context cancellation — now returns `ctx.Err()` when context is done ([#56]) +- Fix `AllKeys()` returning internal map — callers could corrupt store state by modifying the returned map ([#56]) - Fix `FormatCN` panic when certificate has no CN, no DNS SANs, and nil SerialNumber — now returns "unknown" ([`e70e8e5`]) +- Fix WASM `getState` silently ignoring `MozillaRootPool()` error — now logs error and continues without trust checking ([#56]) +- Fix WASM `globalStore` race condition — add `sync.RWMutex` for concurrent access from goroutines ([#56]) +- Fix `--dump-certs` using inconsistent chain verification (missing `ExtKeyUsageAny`, no `IsMozillaRoot` bypass) ([#56]) +- Fix expired certificates double-counted as both expired and untrusted in scan summary — now only counted as expired ([#56]) +- Fix `certkit inspect` bare `return err` without context wrapping (ERR-1) ([#56]) +- Fix bare `io.ReadAll` return in `httpAIAFetcher` missing error context (ERR-1) ([#57]) +- Fix WASM `addFiles` resolving with empty string when `json.Marshal` fails — now rejects the promise (ERR-5) ([#57]) +- Fix silent `continue` in `MozillaRootSubjects` when certificate parsing fails — now logs with `slog.Debug` (ERR-5) ([#57]) + +### Tests + +- Add `ResolveAIA` SSRF URL rejection test — verifies private/loopback AIA URLs produce warnings without invoking the fetcher ([#57]) +- Add `VerifyChainTrust` edge case tests: nil intermediates pool, expired intermediate valid at leaf's NotBefore ([#57]) +- Add `ScanSummary` nil-pool test — verifies expired counts are computed but untrusted counts are skipped when no root pool is provided ([#57]) +- Strengthen `ValidateAIAURL` empty-scheme assertion to verify specific error message ([#57]) ### Removed @@ -512,6 +554,7 @@ Initial release. [0.1.1]: https://github.com/sensiblebit/certkit/compare/v0.1.0...v0.1.1 [0.1.0]: https://github.com/sensiblebit/certkit/releases/tag/v0.1.0 +[`b5969b0`]: https://github.com/sensiblebit/certkit/commit/b5969b0 [`e70e8e5`]: https://github.com/sensiblebit/certkit/commit/e70e8e5 [`0fa55af`]: https://github.com/sensiblebit/certkit/commit/0fa55af [`b69caef`]: https://github.com/sensiblebit/certkit/commit/b69caef @@ -567,6 +610,7 @@ Initial release. [`55b5c1e`]: https://github.com/sensiblebit/certkit/commit/55b5c1e [`8cf81d9`]: https://github.com/sensiblebit/certkit/commit/8cf81d9 [`3569926`]: https://github.com/sensiblebit/certkit/commit/3569926 +[#57]: https://github.com/sensiblebit/certkit/pull/57 [#56]: https://github.com/sensiblebit/certkit/pull/56 [#48]: https://github.com/sensiblebit/certkit/pull/48 [#46]: https://github.com/sensiblebit/certkit/pull/46 diff --git a/bundle.go b/bundle.go index a5146460..b9ba0787 100644 --- a/bundle.go +++ b/bundle.go @@ -1,6 +1,7 @@ package certkit import ( + "bytes" "context" "crypto/tls" "crypto/x509" @@ -8,6 +9,7 @@ import ( "errors" "fmt" "io" + "log/slog" "net" "net/http" "net/url" @@ -24,8 +26,30 @@ var ( mozillaPoolErr error mozillaSubjectsOnce sync.Once mozillaSubjects map[string]bool + mozillaRootKeysOnce sync.Once + mozillaRootKeys map[string][]byte // RawSubject → marshaled PKIX public key ) +// privateNetworks contains CIDR ranges for private, reserved, and shared +// address space. Parsed once at init to avoid repeated net.ParseCIDR calls. +var privateNetworks []*net.IPNet + +func init() { + for _, cidr := range []string{ + "10.0.0.0/8", // RFC 1918 + "172.16.0.0/12", // RFC 1918 + "192.168.0.0/16", // RFC 1918 + "100.64.0.0/10", // RFC 6598 CGN / shared address space + "fc00::/7", // IPv6 ULA + } { + _, network, err := net.ParseCIDR(cidr) + if err != nil { + panic(fmt.Sprintf("invalid CIDR %q: %v", cidr, err)) + } + privateNetworks = append(privateNetworks, network) + } +} + // MozillaRootPEM returns the raw PEM-encoded Mozilla root certificate bundle. func MozillaRootPEM() []byte { return []byte(embedded.MozillaCACertificatesPEM()) @@ -50,6 +74,11 @@ func MozillaRootPool() (*x509.CertPool, error) { // MozillaRootSubjects returns a set of raw ASN.1 subject byte strings from all // Mozilla root certificates. The result is initialized once and cached for the // lifetime of the process. +// +// The returned map is shared and must not be modified by callers. We +// intentionally return the backing map directly rather than a defensive copy +// because all callers perform read-only lookups, and copying ~150 entries on +// every call would violate PERF-2 for no practical safety gain. func MozillaRootSubjects() map[string]bool { mozillaSubjectsOnce.Do(func() { mozillaSubjects = make(map[string]bool) @@ -65,6 +94,7 @@ func MozillaRootSubjects() map[string]bool { } cert, err := x509.ParseCertificate(block.Bytes) if err != nil { + slog.Debug("skipping unparseable certificate in Mozilla root bundle", "error", err) continue } mozillaSubjects[string(cert.RawSubject)] = true @@ -73,12 +103,150 @@ func MozillaRootSubjects() map[string]bool { return mozillaSubjects } +// mozillaRootPublicKeys returns a map of raw ASN.1 subject byte strings to +// their corresponding marshaled PKIX public key. Initialized once and cached. +func mozillaRootPublicKeys() map[string][]byte { + mozillaRootKeysOnce.Do(func() { + mozillaRootKeys = make(map[string][]byte) + pemData := MozillaRootPEM() + for { + var block *pem.Block + block, pemData = pem.Decode(pemData) + if block == nil { + break + } + if block.Type != "CERTIFICATE" { + continue + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + slog.Debug("skipping unparseable root certificate", "error", err) + continue + } + pubBytes, err := x509.MarshalPKIXPublicKey(cert.PublicKey) + if err != nil { + slog.Debug("skipping root with unmarshalable public key", "error", err) + continue + } + mozillaRootKeys[string(cert.RawSubject)] = pubBytes + } + }) + return mozillaRootKeys +} + +// IsMozillaRoot reports whether the certificate matches a Mozilla root +// certificate by both Subject (raw ASN.1 bytes) and public key (marshaled +// PKIX). This identifies self-signed roots and cross-signed variants that +// share the same key pair. A Subject-only match is insufficient because an +// attacker could forge the Subject; the public key check ensures the +// certificate holds the same key as the genuine root. +// +// AKI (Authority Key Identifier) is intentionally not checked: cross-signed +// roots have a different AKI (pointing to the cross-signer) than the +// self-signed version, and the cross-signer may have been removed from the +// Mozilla trust store. The public key match is cryptographically sufficient. +func IsMozillaRoot(cert *x509.Certificate) bool { + expectedPub, ok := mozillaRootPublicKeys()[string(cert.RawSubject)] + if !ok { + return false + } + actualPub, err := x509.MarshalPKIXPublicKey(cert.PublicKey) + if err != nil { + return false + } + return bytes.Equal(expectedPub, actualPub) +} + // IsIssuedByMozillaRoot reports whether the certificate's issuer matches a -// Mozilla root certificate's subject (by raw ASN.1 bytes). +// Mozilla root certificate's subject (by raw ASN.1 bytes). This is used as a +// performance optimization to skip AIA fetching when the issuer is a well-known +// root — it is NOT a trust decision. Trust verification requires full chain +// validation via VerifyChainTrust. func IsIssuedByMozillaRoot(cert *x509.Certificate) bool { return MozillaRootSubjects()[string(cert.RawIssuer)] } +// ValidateAIAURL checks whether a URL is safe to fetch for AIA certificate +// resolution. It rejects non-HTTP(S) schemes and literal private/loopback/ +// link-local IP addresses to prevent SSRF. +// +// Known limitation: hostnames that resolve to private IPs are intentionally +// allowed. This means DNS rebinding (a hostname resolving to a public IP at +// validation time, then to a private IP at connection time) is theoretically +// possible. We accept this because: +// +// 1. certkit is a short-lived CLI process — the window between ValidateAIAURL +// and the HTTP request is ~2ms, making rebinding impractical to exploit. +// 2. Blocking hostnames that resolve to private IPs would break legitimate +// internal CAs whose AIA endpoints are on private networks. +// 3. Adding net.Dialer.Control to check resolved IPs doesn't help: if we +// allow private IPs for internal CAs, the check is the same TOCTOU race. +func ValidateAIAURL(rawURL string) error { + parsed, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("parsing URL: %w", err) + } + switch parsed.Scheme { + case "http", "https": + // allowed + default: + return fmt.Errorf("unsupported scheme %q (only http and https are allowed)", parsed.Scheme) + } + host := parsed.Hostname() + ip := net.ParseIP(host) + if ip == nil { + return nil // hostname, not a literal IP — allow (see doc comment) + } + if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() { + return fmt.Errorf("blocked address %s (loopback, link-local, or unspecified)", host) + } + for _, network := range privateNetworks { + if network.Contains(ip) { + return fmt.Errorf("blocked private address %s", host) + } + } + return nil +} + +// VerifyChainTrust reports whether the given certificate chains to a trusted +// root. Cross-signed roots (same Subject and public key as a Mozilla root) +// are trusted directly. For expired certificates, verification is performed +// at a time just after the certificate's NotBefore to determine if the chain +// was ever valid — this is more robust than checking just before NotAfter, +// because intermediates that expired between issuance and the leaf's expiry +// will still be valid at NotBefore time. +// +// Known limitation: if an intermediate expired before the leaf's NotBefore, +// the time-shifted verification will still fail because the intermediate is +// invalid at the leaf's issuance time. This is an uncommon edge case in +// practice (intermediates outlive the leaves they sign). +// VerifyChainTrustInput holds parameters for VerifyChainTrust. +type VerifyChainTrustInput struct { + Cert *x509.Certificate + Roots *x509.CertPool + Intermediates *x509.CertPool +} + +func VerifyChainTrust(input VerifyChainTrustInput) bool { + if input.Roots == nil { + return false + } + if IsMozillaRoot(input.Cert) { + return true + } + opts := x509.VerifyOptions{ + Roots: input.Roots, + Intermediates: input.Intermediates, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + } + if time.Now().After(input.Cert.NotAfter) { + // Use NotBefore + 1s: the issuing chain was necessarily valid at issuance. + opts.CurrentTime = input.Cert.NotBefore.Add(time.Second) + } + _, err := input.Cert.Verify(opts) + return err == nil +} + // BundleResult holds the resolved chain and metadata. type BundleResult struct { // Leaf is the end-entity certificate. @@ -164,7 +332,19 @@ func FetchAIACertificates(ctx context.Context, cert *x509.Certificate, timeout t var fetched []*x509.Certificate var warnings []string - client := &http.Client{Timeout: timeout} + const maxRedirects = 3 + client := &http.Client{ + Timeout: timeout, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return fmt.Errorf("stopped after %d redirects", maxRedirects) + } + if err := ValidateAIAURL(req.URL.String()); err != nil { + return fmt.Errorf("redirect blocked: %w", err) + } + return nil + }, + } seen := make(map[string]bool) queue := []*x509.Certificate{cert} @@ -178,6 +358,11 @@ func FetchAIACertificates(ctx context.Context, cert *x509.Certificate, timeout t } seen[aiaURL] = true + if err := ValidateAIAURL(aiaURL); err != nil { + warnings = append(warnings, fmt.Sprintf("AIA URL rejected for %s: %v", aiaURL, err)) + continue + } + certs, err := fetchCertificatesFromURL(ctx, client, aiaURL) if err != nil { warnings = append(warnings, fmt.Sprintf("AIA fetch failed for %s: %v", aiaURL, err)) diff --git a/bundle_test.go b/bundle_test.go index 52b43d8c..069f2c95 100644 --- a/bundle_test.go +++ b/bundle_test.go @@ -560,12 +560,16 @@ func TestFetchAIACertificates_duplicateURLs(t *testing.T) { if err != nil { t.Fatal(err) } + // Replace 127.0.0.1 with localhost to avoid ValidateAIAURL SSRF blocking + // of literal loopback IPs. Hostname-based URLs pass SSRF validation. + srvURL := strings.Replace(srv.URL, "127.0.0.1", "localhost", 1) + leafTemplate := &x509.Certificate{ SerialNumber: randomSerial(t), Subject: pkix.Name{CommonName: "dup-aia-leaf"}, NotBefore: time.Now().Add(-1 * time.Hour), NotAfter: time.Now().Add(24 * time.Hour), - IssuingCertificateURL: []string{srv.URL + "/ca.cer", srv.URL + "/ca.cer"}, + IssuingCertificateURL: []string{srvURL + "/ca.cer", srvURL + "/ca.cer"}, } issuerCert, err := x509.ParseCertificate(issuerBytes) if err != nil { diff --git a/certkit_test.go b/certkit_test.go index b1bb4312..b6170d63 100644 --- a/certkit_test.go +++ b/certkit_test.go @@ -1324,6 +1324,388 @@ func TestComputeSKI_UnsupportedType(t *testing.T) { } } +func TestIsMozillaRoot(t *testing.T) { + // WHY: IsMozillaRoot must match genuine Mozilla roots by Subject+PublicKey + // and reject certs that share the same Subject but use a different key. + // This prevents spoofed trust anchors from bypassing chain verification. + t.Parallel() + + // Parse the first Mozilla root cert from the embedded bundle. + pemData := MozillaRootPEM() + block, _ := pem.Decode(pemData) + if block == nil { + t.Fatal("failed to decode first PEM block from Mozilla bundle") + } + realRoot, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("parsing Mozilla root certificate: %v", err) + } + + t.Run("genuine Mozilla root matches", func(t *testing.T) { + t.Parallel() + if !IsMozillaRoot(realRoot) { + t.Errorf("IsMozillaRoot returned false for genuine Mozilla root %q", realRoot.Subject.CommonName) + } + }) + + t.Run("spoofed subject with different key is rejected", func(t *testing.T) { + t.Parallel() + spoofKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + spoofTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: realRoot.Subject, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + spoofDER, err := x509.CreateCertificate(rand.Reader, spoofTmpl, spoofTmpl, &spoofKey.PublicKey, spoofKey) + if err != nil { + t.Fatal(err) + } + spoofCert, err := x509.ParseCertificate(spoofDER) + if err != nil { + t.Fatal(err) + } + if IsMozillaRoot(spoofCert) { + t.Error("IsMozillaRoot should reject a cert with matching Subject but different key") + } + }) + + t.Run("unrelated cert returns false", func(t *testing.T) { + t.Parallel() + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + caTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Definitely Not Mozilla"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + } + caDER, err := x509.CreateCertificate(rand.Reader, caTmpl, caTmpl, &caKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + cert, err := x509.ParseCertificate(caDER) + if err != nil { + t.Fatal(err) + } + if IsMozillaRoot(cert) { + t.Error("IsMozillaRoot should return false for unrelated cert") + } + }) +} + +func TestVerifyChainTrust(t *testing.T) { + // WHY: VerifyChainTrust is the shared trust verification function used by + // scan summary, inspect, WASM, and --dump-certs. Covers trusted chain, + // untrusted self-signed, expired-but-chained (time-shift to NotBefore+1s), + // and Mozilla root bypass. + t.Parallel() + + // Build a test root → intermediate → leaf chain with wide validity. + rootKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + rootTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Trust Test Root"}, + NotBefore: time.Now().Add(-5 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + rootDER, err := x509.CreateCertificate(rand.Reader, rootTmpl, rootTmpl, &rootKey.PublicKey, rootKey) + if err != nil { + t.Fatal(err) + } + rootCert, err := x509.ParseCertificate(rootDER) + if err != nil { + t.Fatal(err) + } + + interKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + interTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Trust Test Intermediate"}, + NotBefore: time.Now().Add(-5 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(5 * 365 * 24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + interDER, err := x509.CreateCertificate(rand.Reader, interTmpl, rootCert, &interKey.PublicKey, rootKey) + if err != nil { + t.Fatal(err) + } + interCert, err := x509.ParseCertificate(interDER) + if err != nil { + t.Fatal(err) + } + + validLeafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + validLeafTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "valid.example.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + validLeafDER, err := x509.CreateCertificate(rand.Reader, validLeafTmpl, interCert, &validLeafKey.PublicKey, interKey) + if err != nil { + t.Fatal(err) + } + validLeaf, err := x509.ParseCertificate(validLeafDER) + if err != nil { + t.Fatal(err) + } + + // Expired leaf signed by the intermediate — valid 2y ago to yesterday + expiredLeafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + expiredLeafTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "expired.example.com"}, + NotBefore: time.Now().Add(-2 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(-24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + expiredLeafDER, err := x509.CreateCertificate(rand.Reader, expiredLeafTmpl, interCert, &expiredLeafKey.PublicKey, interKey) + if err != nil { + t.Fatal(err) + } + expiredLeaf, err := x509.ParseCertificate(expiredLeafDER) + if err != nil { + t.Fatal(err) + } + + rootPool := x509.NewCertPool() + rootPool.AddCert(rootCert) + interPool := x509.NewCertPool() + interPool.AddCert(interCert) + + // Untrusted self-signed leaf (not in root pool) + untrustedKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + untrustedTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "untrusted.example.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + } + untrustedDER, err := x509.CreateCertificate(rand.Reader, untrustedTmpl, untrustedTmpl, &untrustedKey.PublicKey, untrustedKey) + if err != nil { + t.Fatal(err) + } + untrustedCert, err := x509.ParseCertificate(untrustedDER) + if err != nil { + t.Fatal(err) + } + + // Not-yet-valid leaf — NotBefore in the future + futureLeafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + futureLeafTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "future.example.com"}, + NotBefore: time.Now().Add(24 * time.Hour), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + futureLeafDER, err := x509.CreateCertificate(rand.Reader, futureLeafTmpl, interCert, &futureLeafKey.PublicKey, interKey) + if err != nil { + t.Fatal(err) + } + futureLeaf, err := x509.ParseCertificate(futureLeafDER) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + cert *x509.Certificate + want bool + }{ + {"valid leaf with chain", validLeaf, true}, + {"expired leaf with chain (time-shift)", expiredLeaf, true}, + {"self-signed untrusted", untrustedCert, false}, + {"root cert in pool", rootCert, true}, + {"not-yet-valid leaf", futureLeaf, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := VerifyChainTrust(VerifyChainTrustInput{Cert: tt.cert, Roots: rootPool, Intermediates: interPool}); got != tt.want { + t.Errorf("VerifyChainTrust(%q) = %v, want %v", tt.cert.Subject.CommonName, got, tt.want) + } + }) + } + + t.Run("nil roots returns false", func(t *testing.T) { + t.Parallel() + if VerifyChainTrust(VerifyChainTrustInput{Cert: validLeaf, Roots: nil, Intermediates: interPool}) { + t.Error("VerifyChainTrust should return false when roots is nil") + } + }) + + t.Run("nil intermediates pool", func(t *testing.T) { + t.Parallel() + // Root cert should be trusted even with nil intermediates pool. + if !VerifyChainTrust(VerifyChainTrustInput{Cert: rootCert, Roots: rootPool}) { + t.Error("root cert in pool should be trusted with nil intermediates") + } + }) + + t.Run("expired intermediate valid at leaf NotBefore", func(t *testing.T) { + t.Parallel() + // Build a chain where the intermediate was valid when the leaf was + // issued (at leaf.NotBefore) but has since expired. + expInterKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + expInterTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Expired Intermediate"}, + NotBefore: time.Now().Add(-3 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(-24 * time.Hour), // expired yesterday + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + expInterDER, err := x509.CreateCertificate(rand.Reader, expInterTmpl, rootCert, &expInterKey.PublicKey, rootKey) + if err != nil { + t.Fatal(err) + } + expInterCert, err := x509.ParseCertificate(expInterDER) + if err != nil { + t.Fatal(err) + } + + // Leaf issued 2y ago (intermediate was valid then), expired yesterday. + leafKey2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + leafTmpl2 := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "expired-inter-leaf.example.com"}, + NotBefore: time.Now().Add(-2 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(-24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + leafDER2, err := x509.CreateCertificate(rand.Reader, leafTmpl2, expInterCert, &leafKey2.PublicKey, expInterKey) + if err != nil { + t.Fatal(err) + } + leaf2, err := x509.ParseCertificate(leafDER2) + if err != nil { + t.Fatal(err) + } + + expInterPool := x509.NewCertPool() + expInterPool.AddCert(expInterCert) + + // Time-shift to leaf.NotBefore+1s — intermediate was valid then. + if !VerifyChainTrust(VerifyChainTrustInput{Cert: leaf2, Roots: rootPool, Intermediates: expInterPool}) { + t.Error("VerifyChainTrust should trust leaf with expired intermediate that was valid at leaf NotBefore") + } + }) + + // Separate test for Mozilla root bypass — uses a real Mozilla root cert. + t.Run("mozilla root bypasses chain verification", func(t *testing.T) { + t.Parallel() + pemData := MozillaRootPEM() + block, _ := pem.Decode(pemData) + if block == nil { + t.Fatal("no PEM block in Mozilla bundle") + } + mozRoot, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("parsing Mozilla root: %v", err) + } + // Pass an empty root pool — the cert should still be trusted via IsMozillaRoot. + emptyPool := x509.NewCertPool() + if !VerifyChainTrust(VerifyChainTrustInput{Cert: mozRoot, Roots: emptyPool, Intermediates: emptyPool}) { + t.Errorf("VerifyChainTrust should trust Mozilla root %q even with empty pool", mozRoot.Subject.CommonName) + } + }) +} + +func TestValidateAIAURL(t *testing.T) { + // WHY: ValidateAIAURL prevents SSRF by rejecting non-HTTP schemes and + // literal private/loopback/link-local IP addresses. Each case covers a + // distinct rejection rule or an allowed pattern. + t.Parallel() + + tests := []struct { + name string + url string + wantErr bool + errSub string + }{ + {"valid http", "http://ca.example.com/issuer.cer", false, ""}, + {"valid https", "https://ca.example.com/issuer.cer", false, ""}, + {"ftp rejected", "ftp://ca.example.com/issuer.cer", true, "unsupported scheme"}, + {"file rejected", "file:///etc/passwd", true, "unsupported scheme"}, + {"empty scheme rejected", "://foo", true, "parsing URL"}, + {"loopback IPv4", "http://127.0.0.1/ca.cer", true, "loopback"}, + {"loopback IPv6", "http://[::1]/ca.cer", true, "loopback"}, + {"link-local IPv4", "http://169.254.1.1/ca.cer", true, "loopback, link-local, or unspecified"}, + {"unspecified IPv4", "http://0.0.0.0/ca.cer", true, "loopback, link-local, or unspecified"}, + {"unspecified IPv6", "http://[::]/ca.cer", true, "loopback, link-local, or unspecified"}, + {"private IPv6 ULA", "http://[fd12::1]/ca.cer", true, "blocked private"}, + {"private 10.x", "http://10.0.0.1/ca.cer", true, "blocked private"}, + {"private 172.16.x", "http://172.16.0.1/ca.cer", true, "blocked private"}, + {"private 192.168.x", "http://192.168.1.1/ca.cer", true, "blocked private"}, + {"CGN 100.64.x", "http://100.64.0.1/ca.cer", true, "blocked private"}, + {"public IP allowed", "http://8.8.8.8/ca.cer", false, ""}, + {"hostname allowed even if resolves to private", "http://internal.company.com/ca.cer", false, ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateAIAURL(tt.url) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error for %q", tt.url) + } + if tt.errSub != "" && !strings.Contains(err.Error(), tt.errSub) { + t.Errorf("error = %v, want substring %q", err, tt.errSub) + } + } else { + if err != nil { + t.Errorf("unexpected error for %q: %v", tt.url, err) + } + } + }) + } +} + func TestAlgorithmName(t *testing.T) { // WHY: KeyAlgorithmName and PublicKeyAlgorithmName produce display strings // for CLI output and JSON; wrong names would confuse users and break JSON diff --git a/cmd/certkit/bundle.go b/cmd/certkit/bundle.go index 570fbc57..9a2d9224 100644 --- a/cmd/certkit/bundle.go +++ b/cmd/certkit/bundle.go @@ -4,6 +4,7 @@ import ( "crypto" "crypto/x509" "fmt" + "log/slog" "os" "slices" "time" @@ -50,6 +51,7 @@ func init() { registerCompletion(bundleCmd, completionInput{"format", fixedCompletion("pem", "chain", "fullchain", "p12", "jks")}) registerCompletion(bundleCmd, completionInput{"trust-store", fixedCompletion("system", "mozilla")}) + registerCompletion(bundleCmd, completionInput{"out-file", fileCompletion}) } func runBundle(cmd *cobra.Command, args []string) error { @@ -101,7 +103,7 @@ func runBundle(cmd *cobra.Command, args []string) error { } for _, w := range bundle.Warnings { - fmt.Fprintf(os.Stderr, "WARNING: %s\n", w) + slog.Warn("bundle", "warning", w) } output, err := formatBundleOutput(bundle, key, bundleFormat, passwords) diff --git a/cmd/certkit/completions.go b/cmd/certkit/completions.go index 7ce66262..42e90875 100644 --- a/cmd/certkit/completions.go +++ b/cmd/certkit/completions.go @@ -34,3 +34,9 @@ func fixedCompletion(values ...string) func(*cobra.Command, []string, string) ([ func directoryCompletion(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { return nil, cobra.ShellCompDirectiveFilterDirs } + +// fileCompletion is a shell completion function that suggests files using the +// shell's default file completion behavior. +func fileCompletion(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + return nil, cobra.ShellCompDirectiveDefault +} diff --git a/cmd/certkit/inspect.go b/cmd/certkit/inspect.go index ec83578f..c7630e96 100644 --- a/cmd/certkit/inspect.go +++ b/cmd/certkit/inspect.go @@ -3,7 +3,6 @@ package main import ( "fmt" "slices" - "time" "github.com/sensiblebit/certkit/internal" "github.com/spf13/cobra" @@ -39,13 +38,13 @@ func runInspect(cmd *cobra.Command, args []string) error { return fmt.Errorf("inspecting %s: %w", args[0], err) } + if err := internal.AnnotateInspectTrust(results); err != nil { + return fmt.Errorf("annotating trust: %w", err) + } + if !allowExpired { results = slices.DeleteFunc(results, func(r internal.InspectResult) bool { - if r.Type != "certificate" || r.NotAfter == "" { - return false - } - t, err := time.Parse(time.RFC3339, r.NotAfter) - return err == nil && time.Now().After(t) + return r.Expired != nil && *r.Expired }) if len(results) == 0 { return fmt.Errorf("no valid (non-expired) certificates, keys, or CSRs found in %s (use --allow-expired to include expired)", args[0]) diff --git a/cmd/certkit/scan.go b/cmd/certkit/scan.go index 11e6b90a..a75c4b71 100644 --- a/cmd/certkit/scan.go +++ b/cmd/certkit/scan.go @@ -1,13 +1,15 @@ package main import ( - "crypto/x509" + "context" "crypto/x509/pkix" "encoding/json" "encoding/pem" "fmt" + "io" "io/fs" "log/slog" + "net/http" "os" "path/filepath" "strings" @@ -166,6 +168,18 @@ func runScan(cmd *cobra.Command, args []string) error { internal.AssignBundleNames(store, bundleConfigs) } + // Resolve missing intermediates via AIA before trust checking + if certstore.HasUnresolvedIssuers(store) { + slog.Info("resolving certificate chains") + aiaWarnings := certstore.ResolveAIA(cmd.Context(), certstore.ResolveAIAInput{ + Store: store, + Fetch: httpAIAFetcher, + }) + for _, w := range aiaWarnings { + slog.Warn("AIA resolution", "warning", w) + } + } + if scanDumpKeys != "" { keys := store.AllKeysFlat() if len(keys) > 0 { @@ -176,36 +190,39 @@ func runScan(cmd *cobra.Command, args []string) error { if err := os.WriteFile(scanDumpKeys, data, 0600); err != nil { return fmt.Errorf("writing keys to %s: %w", scanDumpKeys, err) } - fmt.Fprintf(os.Stderr, "Wrote %d key(s) to %s\n", len(keys), scanDumpKeys) + slog.Info("dumped keys", "count", len(keys), "path", scanDumpKeys) } else { - fmt.Fprintln(os.Stderr, "No keys found to dump") + slog.Info("no keys found to dump") } } if scanDumpCerts != "" { certs := store.AllCertsFlat() if len(certs) > 0 { - // Build mozilla root pool for verification (consistent with other commands) + // Build mozilla root pool for verification (consistent with summary) mozillaPool, err := certkit.MozillaRootPool() if err != nil { - return err + return fmt.Errorf("loading Mozilla root pool: %w", err) } + intermediatePool := store.IntermediatePool() var data []byte var count, skipped int + now := time.Now() for _, c := range certs { cert := c.Cert - // Validate chain unless --force is set + // Skip expired certificates unless --allow-expired is set + if !allowExpired && now.After(cert.NotAfter) { + slog.Debug("skipping expired certificate", "subject", cert.Subject) + skipped++ + continue + } + + // Validate chain unless --force is set (uses same logic as summary) if !scanForceExport { - verifyOpts := x509.VerifyOptions{Roots: mozillaPool} - if allowExpired { - // Set CurrentTime far in the future to bypass expiry checks - verifyOpts.CurrentTime = time.Date(9999, 1, 1, 0, 0, 0, 0, time.UTC) - } - _, verifyErr := cert.Verify(verifyOpts) - if verifyErr != nil { - slog.Debug("skipping unverified certificate", "subject", cert.Subject, "error", verifyErr) + if !certkit.VerifyChainTrust(certkit.VerifyChainTrustInput{Cert: cert, Roots: mozillaPool, Intermediates: intermediatePool}) { + slog.Debug("skipping untrusted certificate", "subject", cert.Subject) skipped++ continue } @@ -222,18 +239,18 @@ func runScan(cmd *cobra.Command, args []string) error { count++ } if skipped > 0 { - fmt.Fprintf(os.Stderr, "Skipped %d unverified certificate(s) (use --force to include)\n", skipped) + slog.Warn("skipped certificates", "count", skipped) } if count > 0 { if err := os.WriteFile(scanDumpCerts, data, 0644); err != nil { return fmt.Errorf("writing certificates to %s: %w", scanDumpCerts, err) } - fmt.Fprintf(os.Stderr, "Wrote %d certificate(s) to %s\n", count, scanDumpCerts) + slog.Info("dumped certificates", "count", count, "path", scanDumpCerts) } else { - fmt.Fprintln(os.Stderr, "No verified certificates found to dump") + slog.Info("no verified certificates found to dump") } } else { - fmt.Fprintln(os.Stderr, "No certificates found to dump") + slog.Info("no certificates found to dump") } } @@ -247,8 +264,14 @@ func runScan(cmd *cobra.Command, args []string) error { } store.DumpDebug() } else { - // Print summary - summary := store.ScanSummary() + // Print summary with trust and expiry annotations + mozillaPool, err := certkit.MozillaRootPool() + if err != nil { + return fmt.Errorf("loading Mozilla root pool: %w", err) + } + summary := store.ScanSummary(certstore.ScanSummaryInput{ + RootPool: mozillaPool, + }) switch scanFormat { case "json": data, err := json.MarshalIndent(summary, "", " ") @@ -260,9 +283,12 @@ func runScan(cmd *cobra.Command, args []string) error { total := summary.Roots + summary.Intermediates + summary.Leaves fmt.Printf("\nFound %d certificate(s) and %d key(s)\n", total, summary.Keys) if total > 0 { - fmt.Printf(" Roots: %d\n", summary.Roots) - fmt.Printf(" Intermediates: %d\n", summary.Intermediates) - fmt.Printf(" Leaves: %d\n", summary.Leaves) + fmt.Printf(" Roots: %d%s\n", summary.Roots, + internal.CertAnnotation(summary.ExpiredRoots, summary.UntrustedRoots)) + fmt.Printf(" Intermediates: %d%s\n", summary.Intermediates, + internal.CertAnnotation(summary.ExpiredIntermediates, summary.UntrustedIntermediates)) + fmt.Printf(" Leaves: %d%s\n", summary.Leaves, + internal.CertAnnotation(summary.ExpiredLeaves, summary.UntrustedLeaves)) } if summary.Keys > 0 { fmt.Printf(" Key-cert pairs: %d\n", summary.Matched) @@ -281,6 +307,45 @@ func runScan(cmd *cobra.Command, args []string) error { return nil } +// aiaHTTPClient is reused across AIA fetches to enable TCP connection reuse. +// Redirects are limited to 3 and validated against SSRF rules. +var aiaHTTPClient = &http.Client{ + Timeout: 2 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 3 { + return fmt.Errorf("stopped after 3 redirects") + } + if err := certkit.ValidateAIAURL(req.URL.String()); err != nil { + return fmt.Errorf("redirect blocked: %w", err) + } + return nil + }, +} + +// httpAIAFetcher fetches raw certificate bytes from a URL via HTTP. +func httpAIAFetcher(ctx context.Context, rawURL string) ([]byte, error) { + if err := certkit.ValidateAIAURL(rawURL); err != nil { + return nil, fmt.Errorf("AIA URL rejected: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) + if err != nil { + return nil, fmt.Errorf("creating AIA request: %w", err) + } + resp, err := aiaHTTPClient.Do(req) + if err != nil { + return nil, fmt.Errorf("fetching AIA URL %s: %w", rawURL, err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("HTTP %d from %s", resp.StatusCode, rawURL) + } + data, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) // 1MB limit + if err != nil { + return nil, fmt.Errorf("reading AIA response from %s: %w", rawURL, err) + } + return data, nil +} + // formatDN formats a pkix.Name as a one-line distinguished name string // matching the OpenSSL one-line format (e.g. "CN=example.com, O=Acme, C=US"). func formatDN(name pkix.Name) string { diff --git a/cmd/wasm/aia.go b/cmd/wasm/aia.go index e31dbca6..30703d74 100644 --- a/cmd/wasm/aia.go +++ b/cmd/wasm/aia.go @@ -22,9 +22,8 @@ func resolveAIA(ctx context.Context, s *certstore.MemStore) []string { // jsFetchURL calls the JavaScript certkitFetchURL function which handles // direct fetch with automatic CORS proxy fallback. Blocks until the JS -// Promise resolves or rejects. The ctx parameter is accepted for AIAFetcher -// compatibility but not currently used (JS promises lack cancellation). -func jsFetchURL(_ context.Context, url string) ([]byte, error) { +// Promise resolves or rejects, or the context is cancelled. +func jsFetchURL(ctx context.Context, url string) ([]byte, error) { fetchFn := js.Global().Get("certkitFetchURL") if fetchFn.Type() != js.TypeFunction { return nil, fmt.Errorf("certkitFetchURL not defined") @@ -38,24 +37,45 @@ func jsFetchURL(_ context.Context, url string) ([]byte, error) { promise := fetchFn.Invoke(url) + const maxAIAResponseSize = 1 << 20 // 1MB, consistent with CLI httpAIAFetcher + thenCb := js.FuncOf(func(_ js.Value, args []js.Value) any { uint8Array := args[0] - data := make([]byte, uint8Array.Length()) + size := uint8Array.Length() + if size > maxAIAResponseSize { + ch <- result{err: fmt.Errorf("AIA response too large (%d bytes, max %d)", size, maxAIAResponseSize)} + return nil + } + data := make([]byte, size) js.CopyBytesToGo(data, uint8Array) ch <- result{data: data} return nil }) catchCb := js.FuncOf(func(_ js.Value, args []js.Value) any { - errMsg := args[0].Get("message").String() - ch <- result{err: fmt.Errorf("%s", errMsg)} + val := args[0] + var errMsg string + if val.Type() == js.TypeObject || val.Type() == js.TypeFunction { + errMsg = val.Get("message").String() + } else { + errMsg = val.String() + } + ch <- result{err: fmt.Errorf("AIA fetch: %s", errMsg)} return nil }) promise.Call("then", thenCb).Call("catch", catchCb) - r := <-ch - thenCb.Release() - catchCb.Release() - return r.data, r.err + select { + case r := <-ch: + thenCb.Release() + catchCb.Release() + return r.data, r.err + case <-ctx.Done(): + // Do NOT release callbacks here. The JS promise is still pending and + // will eventually invoke one of them. Calling a released js.Func panics. + // The buffered channel (cap 1) absorbs the late send harmlessly. + // The callbacks leak, but that is preferable to a crash. + return nil, ctx.Err() + } } diff --git a/cmd/wasm/main.go b/cmd/wasm/main.go index 6db6d579..763acaca 100644 --- a/cmd/wasm/main.go +++ b/cmd/wasm/main.go @@ -7,9 +7,10 @@ package main import ( "context" - "crypto/x509" "encoding/hex" "encoding/json" + "fmt" + "log/slog" "strings" "syscall/js" "time" @@ -59,7 +60,9 @@ func addFiles(_ js.Value, args []js.Value) any { handler := js.FuncOf(func(_ js.Value, promiseArgs []js.Value) any { resolve := promiseArgs[0] + reject := promiseArgs[1] go func() { + storeMu.Lock() var results []map[string]any for i := range length { file := filesArg.Index(i) @@ -86,8 +89,13 @@ func addFiles(_ js.Value, args []js.Value) any { "error": errMsg, }) } + storeMu.Unlock() - jsonBytes, _ := json.Marshal(results) + jsonBytes, err := json.Marshal(results) + if err != nil { + reject.Invoke(fmt.Sprintf("marshaling addFiles results: %v", err)) + return + } resolve.Invoke(string(jsonBytes)) // Eagerly resolve AIA intermediates in the background. @@ -99,13 +107,21 @@ func addFiles(_ js.Value, args []js.Value) any { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() + storeMu.Lock() warnings := resolveAIA(ctx, globalStore) + storeMu.Unlock() + if warnings == nil { warnings = []string{} } - warnJSON, _ := json.Marshal(warnings) - cb := js.FuncOf(func(_ js.Value, _ []js.Value) any { + warnJSON, err := json.Marshal(warnings) + if err != nil { + slog.Error("marshaling AIA warnings", "error", err) + } + var cb js.Func + cb = js.FuncOf(func(_ js.Value, _ []js.Value) any { + defer cb.Release() onComplete := js.Global().Get("certkitOnAIAComplete") if onComplete.Type() == js.TypeFunction { onComplete.Invoke(string(warnJSON)) @@ -117,11 +133,15 @@ func addFiles(_ js.Value, args []js.Value) any { }() return nil }) - - return js.Global().Get("Promise").New(handler) + // Promise.New calls the executor synchronously; release immediately after. + p := js.Global().Get("Promise").New(handler) + handler.Release() + return p } // getState returns all certificates and keys with metadata as JSON. +// Uses TryRLock to avoid deadlocking the JS event loop when AIA resolution +// holds the write lock (AIA blocks on JS promises that need the event loop). // JS signature: certkitGetState() → string func getState(_ js.Value, _ []js.Value) any { type certInfo struct { @@ -151,13 +171,31 @@ func getState(_ js.Value, _ []js.Value) any { Certs []certInfo `json:"certs"` Keys []keyInfo `json:"keys"` MatchedPairs int `json:"matched_pairs"` + Busy bool `json:"busy,omitempty"` } + if !storeMu.TryRLock() { + // Store is locked by AIA resolution; return a busy indicator + // instead of blocking the JS event loop (which would deadlock). + resp := stateResponse{Busy: true} + jsonBytes, err := json.Marshal(resp) + if err != nil { + slog.Error("marshaling busy state", "error", err) + return `{"busy":true}` + } + return string(jsonBytes) + } + defer storeMu.RUnlock() + now := time.Now() resp := stateResponse{} // Build pools for chain verification. - roots, _ := certkit.MozillaRootPool() + roots, err := certkit.MozillaRootPool() + if err != nil { + slog.Error("loading Mozilla root pool", "error", err) + // Continue without trust checking — certs will all show as untrusted. + } intermediatePool := globalStore.IntermediatePool() allCerts := globalStore.AllCerts() allKeys := globalStore.AllKeys() @@ -165,15 +203,10 @@ func getState(_ js.Value, _ []js.Value) any { for ski, rec := range allCerts { _, hasKey := allKeys[ski] - // Check if cert chains to a Mozilla root at the current time. - // Expired certs will fail verification (Expired field shown separately). + expired := now.After(rec.NotAfter) trusted := false - _, verifyErr := rec.Cert.Verify(x509.VerifyOptions{ - Roots: roots, - Intermediates: intermediatePool, - }) - if verifyErr == nil { - trusted = true + if roots != nil { + trusted = certkit.VerifyChainTrust(certkit.VerifyChainTrustInput{Cert: rec.Cert, Roots: roots, Intermediates: intermediatePool}) } ci := certInfo{ @@ -183,7 +216,7 @@ func getState(_ js.Value, _ []js.Value) any { KeyType: rec.KeyType, NotBefore: rec.NotBefore.UTC().Format(time.RFC3339), NotAfter: rec.NotAfter.UTC().Format(time.RFC3339), - Expired: now.After(rec.NotAfter), + Expired: expired, HasKey: hasKey, Trusted: trusted, Issuer: rec.Cert.Issuer.CommonName, @@ -207,7 +240,11 @@ func getState(_ js.Value, _ []js.Value) any { resp.MatchedPairs = len(globalStore.MatchedPairs()) - jsonBytes, _ := json.Marshal(resp) + jsonBytes, err := json.Marshal(resp) + if err != nil { + slog.Error("marshaling state response", "error", err) + return `{"error":"internal marshal failure"}` + } return string(jsonBytes) } @@ -231,7 +268,10 @@ func exportBundlesJS(_ js.Value, args []js.Value) any { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() + storeMu.RLock() + defer storeMu.RUnlock() zipData, err := exportBundles(ctx, globalStore, filterSKIs) + if err != nil { reject.Invoke(js.Global().Get("Error").New(err.Error())) return @@ -243,15 +283,23 @@ func exportBundlesJS(_ js.Value, args []js.Value) any { }() return nil }) - - return js.Global().Get("Promise").New(handler) + // Promise.New calls the executor synchronously; release immediately after. + p := js.Global().Get("Promise").New(handler) + handler.Release() + return p } // resetStore clears all stored certificates and keys. -// JS signature: certkitReset() → void +// Uses TryLock to avoid deadlocking the JS event loop when AIA resolution +// holds the lock. Returns false if the store is busy. +// JS signature: certkitReset() → boolean func resetStore(_ js.Value, _ []js.Value) any { + if !storeMu.TryLock() { + return false + } globalStore.Reset() - return nil + storeMu.Unlock() + return true } // hexToBytes decodes a hex string to bytes, returning nil on error. @@ -267,5 +315,8 @@ func jsError(msg string) any { reject.Invoke(js.Global().Get("Error").New(msg)) return nil }) - return js.Global().Get("Promise").New(handler) + // Promise.New calls the executor synchronously; release immediately after. + p := js.Global().Get("Promise").New(handler) + handler.Release() + return p } diff --git a/cmd/wasm/store.go b/cmd/wasm/store.go index c7e1f278..335a4e45 100644 --- a/cmd/wasm/store.go +++ b/cmd/wasm/store.go @@ -2,7 +2,15 @@ package main -import "github.com/sensiblebit/certkit/internal/certstore" +import ( + "sync" + + "github.com/sensiblebit/certkit/internal/certstore" +) // globalStore is the shared in-memory certificate and key store. var globalStore = certstore.NewMemStore() + +// storeMu protects globalStore against concurrent access from goroutines +// (addFiles, AIA resolution) and synchronous calls (getState, resetStore). +var storeMu sync.RWMutex diff --git a/internal/certstore/aia.go b/internal/certstore/aia.go index c0764fb4..90ec9a7b 100644 --- a/internal/certstore/aia.go +++ b/internal/certstore/aia.go @@ -3,6 +3,7 @@ package certstore import ( "context" "fmt" + "log/slog" "github.com/sensiblebit/certkit" ) @@ -18,6 +19,24 @@ type ResolveAIAInput struct { MaxDepth int // 0 defaults to 5 } +// HasUnresolvedIssuers reports whether any non-root certificate in the store +// is missing its issuer (not in the store and not a Mozilla root). +func HasUnresolvedIssuers(store *MemStore) bool { + for _, rec := range store.AllCertsFlat() { + if rec.CertType == "root" { + continue + } + if store.HasIssuer(rec.Cert) { + continue + } + if certkit.IsIssuedByMozillaRoot(rec.Cert) { + continue + } + return true + } + return false +} + // ResolveAIA walks AIA CA Issuers URLs for all non-root certificates in the // store, fetching any missing intermediate issuers. Certificates whose issuer // is already in the store or is a Mozilla root are skipped. @@ -60,6 +79,14 @@ func ResolveAIA(ctx context.Context, input ResolveAIAInput) []string { } seen[aiaURL] = true + if err := certkit.ValidateAIAURL(aiaURL); err != nil { + warnings = append(warnings, fmt.Sprintf( + "AIA URL rejected for %q: %v", + rec.Cert.Subject.CommonName, err, + )) + continue + } + body, err := input.Fetch(ctx, aiaURL) if err != nil { warnings = append(warnings, fmt.Sprintf( @@ -81,6 +108,7 @@ func ResolveAIA(ctx context.Context, input ResolveAIAInput) []string { for _, issuer := range issuers { if err := input.Store.HandleCertificate(issuer, "AIA: "+aiaURL); err != nil { + slog.Debug("skipping AIA certificate", "url", aiaURL, "error", err) continue } fetched++ diff --git a/internal/certstore/aia_test.go b/internal/certstore/aia_test.go index 19f0065c..d7e4a4fa 100644 --- a/internal/certstore/aia_test.go +++ b/internal/certstore/aia_test.go @@ -7,14 +7,156 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" + "encoding/pem" "fmt" "strings" + "sync/atomic" "testing" "time" "github.com/sensiblebit/certkit" ) +func TestHasUnresolvedIssuers(t *testing.T) { + // WHY: HasUnresolvedIssuers gates AIA resolution — it must return false for + // empty stores, root-only stores, and stores where all issuers are present + // or issued by Mozilla roots. It must return true only when a non-root + // cert's issuer is genuinely missing. + t.Parallel() + + tests := []struct { + name string + setup func(t *testing.T, store *MemStore) + want bool + }{ + { + name: "empty store", + setup: func(t *testing.T, store *MemStore) {}, + want: false, + }, + { + name: "only roots", + setup: func(t *testing.T, store *MemStore) { + ca := newRSACA(t) + if err := store.HandleCertificate(ca.cert, "ca.pem"); err != nil { + t.Fatal(err) + } + }, + want: false, + }, + { + name: "leaf with issuer in store", + setup: func(t *testing.T, store *MemStore) { + ca := newRSACA(t) + leaf := newRSALeaf(t, ca, "has-issuer.example.com", []string{"has-issuer.example.com"}) + if err := store.HandleCertificate(ca.cert, "ca.pem"); err != nil { + t.Fatal(err) + } + if err := store.HandleCertificate(leaf.cert, "leaf.pem"); err != nil { + t.Fatal(err) + } + }, + want: false, + }, + { + name: "leaf issued by Mozilla root returns false", + setup: func(t *testing.T, store *MemStore) { + // Parse the first Mozilla root to get its RawSubject. + pemData := certkit.MozillaRootPEM() + block, _ := pem.Decode(pemData) + if block == nil { + t.Fatal("no PEM block in Mozilla root bundle") + } + mozRoot, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("parsing Mozilla root: %v", err) + } + + // Create a fake CA with the exact same RawSubject as the + // Mozilla root. This ensures the leaf's RawIssuer matches. + // HasUnresolvedIssuers/IsIssuedByMozillaRoot only checks + // RawIssuer bytes, not cryptographic signatures. + fakeKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + fakeCATmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + RawSubject: mozRoot.RawSubject, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + SubjectKeyId: mozRoot.SubjectKeyId, + } + fakeDER, err := x509.CreateCertificate(rand.Reader, fakeCATmpl, fakeCATmpl, &fakeKey.PublicKey, fakeKey) + if err != nil { + t.Fatalf("create fake CA: %v", err) + } + fakeCert, err := x509.ParseCertificate(fakeDER) + if err != nil { + t.Fatal(err) + } + + leafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + leafTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "mozilla-issued.example.com"}, + DNSNames: []string{"mozilla-issued.example.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + AuthorityKeyId: mozRoot.SubjectKeyId, + } + leafDER, err := x509.CreateCertificate(rand.Reader, leafTmpl, fakeCert, &leafKey.PublicKey, fakeKey) + if err != nil { + t.Fatalf("create leaf signed by fake CA: %v", err) + } + leafCert, err := x509.ParseCertificate(leafDER) + if err != nil { + t.Fatal(err) + } + + if string(leafCert.RawIssuer) != string(mozRoot.RawSubject) { + t.Fatal("leaf RawIssuer does not match Mozilla root RawSubject") + } + + if err := store.HandleCertificate(leafCert, "leaf.pem"); err != nil { + t.Fatal(err) + } + }, + want: false, + }, + { + name: "leaf with missing issuer", + setup: func(t *testing.T, store *MemStore) { + ca := newRSACA(t) + leaf := newRSALeaf(t, ca, "orphan.example.com", []string{"orphan.example.com"}) + // Only add the leaf, not the CA + if err := store.HandleCertificate(leaf.cert, "leaf.pem"); err != nil { + t.Fatal(err) + } + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + store := NewMemStore() + tt.setup(t, store) + if got := HasUnresolvedIssuers(store); got != tt.want { + t.Errorf("HasUnresolvedIssuers() = %v, want %v", got, tt.want) + } + }) + } +} + func TestResolveAIA_FetchesMissingIssuer(t *testing.T) { // WHY: The core AIA resolution path must fetch an intermediate when the // store has a leaf whose issuer is not present. Without this, chains @@ -176,9 +318,9 @@ func TestResolveAIA_SkipsResolvedAndRoots(t *testing.T) { store := NewMemStore() tt.setup(t, store) - fetchCount := 0 + var fetchCount atomic.Int32 fetcher := func(_ context.Context, _ string) ([]byte, error) { - fetchCount++ + fetchCount.Add(1) return nil, fmt.Errorf("should not be called") } @@ -190,8 +332,8 @@ func TestResolveAIA_SkipsResolvedAndRoots(t *testing.T) { if len(warnings) != 0 { t.Errorf("expected 0 warnings, got %v", warnings) } - if fetchCount != 0 { - t.Errorf("expected 0 fetches, got %d", fetchCount) + if fetchCount.Load() != 0 { + t.Errorf("expected 0 fetches, got %d", fetchCount.Load()) } }) } @@ -335,9 +477,9 @@ func TestResolveAIA_DeduplicatesURLs(t *testing.T) { } } - fetchCount := 0 + var fetchCount atomic.Int32 fetcher := func(_ context.Context, _ string) ([]byte, error) { - fetchCount++ + fetchCount.Add(1) return caDER, nil } @@ -346,13 +488,13 @@ func TestResolveAIA_DeduplicatesURLs(t *testing.T) { Fetch: fetcher, }) - if fetchCount != 1 { - t.Errorf("expected 1 fetch (URL deduped), got %d", fetchCount) + if fetchCount.Load() != 1 { + t.Errorf("expected 1 fetch (URL deduped), got %d", fetchCount.Load()) } // Verify the fetched cert is actually in the store — without this, // a fetcher that returned valid DER but HandleCertificate silently - // failed would still show fetchCount==1. + // failed would still show fetchCount.Load()==1. allCerts := store.AllCertsFlat() if len(allCerts) != 3 { t.Errorf("expected 3 certs in store (2 leaves + 1 fetched CA), got %d", len(allCerts)) @@ -399,6 +541,10 @@ func TestResolveAIA_MaxDepth(t *testing.T) { if err != nil { t.Fatal(err) } + rootCert, err := x509.ParseCertificate(rootDER) + if err != nil { + t.Fatal(err) + } intKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { @@ -414,7 +560,11 @@ func TestResolveAIA_MaxDepth(t *testing.T) { KeyUsage: x509.KeyUsageCertSign, IssuingCertificateURL: []string{"http://example.com/root.cer"}, } - intDER, err := x509.CreateCertificate(rand.Reader, intTmpl, rootTmpl, &intKey.PublicKey, rootKey) + intDER, err := x509.CreateCertificate(rand.Reader, intTmpl, rootCert, &intKey.PublicKey, rootKey) + if err != nil { + t.Fatal(err) + } + intCert, err := x509.ParseCertificate(intDER) if err != nil { t.Fatal(err) } @@ -430,7 +580,7 @@ func TestResolveAIA_MaxDepth(t *testing.T) { NotAfter: time.Now().Add(24 * time.Hour), IssuingCertificateURL: []string{"http://example.com/intermediate.cer"}, } - leafDER, err := x509.CreateCertificate(rand.Reader, leafTmpl, intTmpl, &leafKey.PublicKey, intKey) + leafDER, err := x509.CreateCertificate(rand.Reader, leafTmpl, intCert, &leafKey.PublicKey, intKey) if err != nil { t.Fatal(err) } @@ -443,9 +593,9 @@ func TestResolveAIA_MaxDepth(t *testing.T) { t.Fatal(err) } - fetchCount := 0 + var fetchCount atomic.Int32 fetcher := func(_ context.Context, url string) ([]byte, error) { - fetchCount++ + fetchCount.Add(1) if strings.Contains(url, "intermediate") { return intDER, nil } @@ -458,8 +608,8 @@ func TestResolveAIA_MaxDepth(t *testing.T) { MaxDepth: tt.maxDepth, }) - if fetchCount != tt.wantFetchCount { - t.Errorf("expected %d fetch(es), got %d", tt.wantFetchCount, fetchCount) + if int(fetchCount.Load()) != tt.wantFetchCount { + t.Errorf("expected %d fetch(es), got %d", tt.wantFetchCount, fetchCount.Load()) } allCerts := store.AllCertsFlat() if len(allCerts) != tt.wantStoreCount { @@ -655,3 +805,97 @@ func TestResolveAIA_CancelledContext(t *testing.T) { t.Error("expected at least one warning from cancelled fetch") } } + +func TestResolveAIA_RejectsSSRFURL(t *testing.T) { + // WHY: When a certificate has a private/loopback AIA URL, ResolveAIA must + // reject it with a warning before invoking the fetcher — this is the + // integration point for ValidateAIAURL within the AIA resolution loop. + t.Parallel() + + tests := []struct { + name string + aiaURL string + errSub string + }{ + {"loopback IPv4", "http://127.0.0.1/ca.cer", "AIA URL rejected"}, + {"private 10.x", "http://10.0.0.1/ca.cer", "AIA URL rejected"}, + {"file scheme", "file:///etc/passwd", "AIA URL rejected"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + store := NewMemStore() + + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + caTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "SSRF CA"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + caDER, err := x509.CreateCertificate(rand.Reader, caTmpl, caTmpl, &caKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + caCert, err := x509.ParseCertificate(caDER) + if err != nil { + t.Fatal(err) + } + + leafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + leafTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "ssrf.example.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IssuingCertificateURL: []string{tt.aiaURL}, + } + leafDER, err := x509.CreateCertificate(rand.Reader, leafTmpl, caCert, &leafKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + leafCert, err := x509.ParseCertificate(leafDER) + if err != nil { + t.Fatal(err) + } + + if err := store.HandleCertificate(leafCert, "leaf.pem"); err != nil { + t.Fatal(err) + } + + var fetchCount atomic.Int32 + fetcher := func(_ context.Context, _ string) ([]byte, error) { + fetchCount.Add(1) + return caDER, nil + } + + warnings := ResolveAIA(context.Background(), ResolveAIAInput{ + Store: store, + Fetch: fetcher, + }) + + if fetchCount.Load() != 0 { + t.Errorf("fetcher should not be called for SSRF URL, got %d calls", fetchCount.Load()) + } + if len(warnings) != 1 { + t.Fatalf("expected 1 warning, got %d: %v", len(warnings), warnings) + } + if !strings.Contains(warnings[0], tt.errSub) { + t.Errorf("warning = %q, want substring %q", warnings[0], tt.errSub) + } + if len(store.AllCertsFlat()) != 1 { + t.Errorf("store should still have only the leaf, got %d certs", len(store.AllCertsFlat())) + } + }) + } +} diff --git a/internal/certstore/memstore.go b/internal/certstore/memstore.go index 9b4d2c90..f53dbde6 100644 --- a/internal/certstore/memstore.go +++ b/internal/certstore/memstore.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "log/slog" + "maps" "slices" "sort" "time" @@ -181,9 +182,11 @@ func (s *MemStore) AllCerts() map[string]*CertRecord { return result } -// AllKeys returns a snapshot of all key records keyed by SKI. +// AllKeys returns a copy of all key records keyed by SKI. func (s *MemStore) AllKeys() map[string]*KeyRecord { - return s.keys + result := make(map[string]*KeyRecord, len(s.keys)) + maps.Copy(result, s.keys) + return result } // AllCertsFlat returns all certificate records as a flat slice. @@ -295,18 +298,54 @@ func (s *MemStore) BundleNames() []string { } // ScanSummary returns aggregate counts of stored certificates and keys. -func (s *MemStore) ScanSummary() ScanSummary { +// When input.RootPool is non-nil, it also counts expired and untrusted +// certificates. Expired certificates are only counted as expired, never +// as untrusted, to avoid misleading double-counts in the summary output. +func (s *MemStore) ScanSummary(input ScanSummaryInput) ScanSummary { summary := ScanSummary{ Keys: len(s.keys), } + + var intermediatePool *x509.CertPool + if input.RootPool != nil { + intermediatePool = s.IntermediatePool() + } + + now := time.Now() for _, rec := range s.certsByID { + expired := now.After(rec.NotAfter) + switch rec.CertType { case "root": summary.Roots++ + if expired { + summary.ExpiredRoots++ + } + if input.RootPool != nil && !expired { + if !certkit.VerifyChainTrust(certkit.VerifyChainTrustInput{Cert: rec.Cert, Roots: input.RootPool, Intermediates: intermediatePool}) { + summary.UntrustedRoots++ + } + } case "intermediate": summary.Intermediates++ + if expired { + summary.ExpiredIntermediates++ + } + if input.RootPool != nil && !expired { + if !certkit.VerifyChainTrust(certkit.VerifyChainTrustInput{Cert: rec.Cert, Roots: input.RootPool, Intermediates: intermediatePool}) { + summary.UntrustedIntermediates++ + } + } case "leaf": summary.Leaves++ + if expired { + summary.ExpiredLeaves++ + } + if input.RootPool != nil && !expired { + if !certkit.VerifyChainTrust(certkit.VerifyChainTrustInput{Cert: rec.Cert, Roots: input.RootPool, Intermediates: intermediatePool}) { + summary.UntrustedLeaves++ + } + } } } summary.Matched = len(s.MatchedPairs()) diff --git a/internal/certstore/memstore_test.go b/internal/certstore/memstore_test.go index 6edf6fda..6fd46403 100644 --- a/internal/certstore/memstore_test.go +++ b/internal/certstore/memstore_test.go @@ -348,10 +348,15 @@ func TestMemStore_EmptyStore(t *testing.T) { t.Error("IntermediatePool returned nil for empty store") } - summary := store.ScanSummary() + summary := store.ScanSummary(ScanSummaryInput{}) if summary.Roots != 0 || summary.Intermediates != 0 || summary.Leaves != 0 || summary.Keys != 0 || summary.Matched != 0 { t.Errorf("expected all zeros in empty scan summary, got %+v", summary) } + if summary.ExpiredRoots != 0 || summary.UntrustedRoots != 0 || + summary.ExpiredIntermediates != 0 || summary.UntrustedIntermediates != 0 || + summary.ExpiredLeaves != 0 || summary.UntrustedLeaves != 0 { + t.Errorf("expected all trust/expiry fields zero in empty scan summary, got %+v", summary) + } } func TestMemStore_MultipleCertsAndKeys(t *testing.T) { @@ -685,7 +690,7 @@ func TestMemStore_ScanSummary(t *testing.T) { t.Fatal(err) } - summary := store.ScanSummary() + summary := store.ScanSummary(ScanSummaryInput{}) if summary.Roots != 1 { t.Errorf("Roots = %d, want 1", summary.Roots) } @@ -703,6 +708,229 @@ func TestMemStore_ScanSummary(t *testing.T) { } } +func TestMemStore_ScanSummaryTrust(t *testing.T) { + // WHY: The scan summary must distinguish expired from untrusted certs. + // Expired certs are only counted as expired, never as untrusted, to avoid + // misleading double-counts. Non-expired certs without a chain to the root + // pool are counted as untrusted. + t.Parallel() + + // Build a chain with temporally consistent validity periods. + // The root and intermediate must be valid during the expired leaf's + // lifetime (NotBefore well before the expired leaf's NotAfter). + rootKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + rootTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Trust Test Root CA"}, + NotBefore: time.Now().Add(-5 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + BasicConstraintsValid: true, + IsCA: true, + } + rootDER, err := x509.CreateCertificate(rand.Reader, rootTmpl, rootTmpl, &rootKey.PublicKey, rootKey) + if err != nil { + t.Fatal(err) + } + rootCert, err := x509.ParseCertificate(rootDER) + if err != nil { + t.Fatal(err) + } + + interKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + interTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Trust Test Intermediate CA"}, + NotBefore: time.Now().Add(-5 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(5 * 365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + BasicConstraintsValid: true, + IsCA: true, + AuthorityKeyId: rootCert.SubjectKeyId, + } + interDER, err := x509.CreateCertificate(rand.Reader, interTmpl, rootCert, &interKey.PublicKey, rootKey) + if err != nil { + t.Fatal(err) + } + interCert, err := x509.ParseCertificate(interDER) + if err != nil { + t.Fatal(err) + } + + // Expired leaf signed by the intermediate — valid 2y ago to yesterday + expiredKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + expiredTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "expired.example.com"}, + DNSNames: []string{"expired.example.com"}, + NotBefore: time.Now().Add(-2 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(-24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + AuthorityKeyId: interCert.SubjectKeyId, + } + expiredDER, err := x509.CreateCertificate(rand.Reader, expiredTmpl, interCert, &expiredKey.PublicKey, interKey) + if err != nil { + t.Fatal(err) + } + expiredCert, err := x509.ParseCertificate(expiredDER) + if err != nil { + t.Fatal(err) + } + + rootPool := x509.NewCertPool() + rootPool.AddCert(rootCert) + + tests := []struct { + name string + addExpiredLeaf bool + addUntrustedLeaf bool + addExpiredRoot bool + addUntrustedIntermediate bool + wantExpiredRoots int + wantUntrustedRoots int + wantExpiredIntermediates int + wantUntrustedIntermediates int + wantExpiredLeaves int + wantUntrustedLeaves int + }{ + { + name: "expired leaf is counted as expired not untrusted", + addExpiredLeaf: true, + wantExpiredLeaves: 1, + }, + { + name: "untrusted leaf without chain", + addUntrustedLeaf: true, + wantUntrustedLeaves: 1, + }, + { + name: "expired and untrusted leaves counted separately", + addExpiredLeaf: true, + addUntrustedLeaf: true, + wantExpiredLeaves: 1, + wantUntrustedLeaves: 1, + }, + { + name: "expired root is counted as expired not untrusted", + addExpiredRoot: true, + wantExpiredRoots: 1, + }, + { + name: "untrusted intermediate without trusted root", + addUntrustedIntermediate: true, + wantUntrustedIntermediates: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + store := NewMemStore() + pool := rootPool + + if err := store.HandleCertificate(rootCert, "root.pem"); err != nil { + t.Fatal(err) + } + if err := store.HandleCertificate(interCert, "inter.pem"); err != nil { + t.Fatal(err) + } + + if tt.addExpiredLeaf { + if err := store.HandleCertificate(expiredCert, "expired.pem"); err != nil { + t.Fatal(err) + } + } + if tt.addUntrustedLeaf { + untrustedCA := newRSACA(t) + untrusted := newRSALeaf(t, untrustedCA, "untrusted.example.com", []string{"untrusted.example.com"}) + if err := store.HandleCertificate(untrusted.cert, "untrusted.pem"); err != nil { + t.Fatal(err) + } + } + if tt.addExpiredRoot { + expiredRoot := newExpiredRoot(t) + if err := store.HandleCertificate(expiredRoot, "expired-root.pem"); err != nil { + t.Fatal(err) + } + // Add expired root to pool so it's trusted when time-shifted + pool = x509.NewCertPool() + pool.AddCert(rootCert) + pool.AddCert(expiredRoot) + } + if tt.addUntrustedIntermediate { + // Intermediate from a private CA not in the root pool + privateCA := newRSACA(t) + inter := newIntermediateCA(t, privateCA) + if err := store.HandleCertificate(inter.cert, "untrusted-inter.pem"); err != nil { + t.Fatal(err) + } + } + + summary := store.ScanSummary(ScanSummaryInput{ + RootPool: pool, + }) + + if summary.ExpiredRoots != tt.wantExpiredRoots { + t.Errorf("ExpiredRoots = %d, want %d", summary.ExpiredRoots, tt.wantExpiredRoots) + } + if summary.UntrustedRoots != tt.wantUntrustedRoots { + t.Errorf("UntrustedRoots = %d, want %d", summary.UntrustedRoots, tt.wantUntrustedRoots) + } + if summary.ExpiredIntermediates != tt.wantExpiredIntermediates { + t.Errorf("ExpiredIntermediates = %d, want %d", summary.ExpiredIntermediates, tt.wantExpiredIntermediates) + } + if summary.UntrustedIntermediates != tt.wantUntrustedIntermediates { + t.Errorf("UntrustedIntermediates = %d, want %d", summary.UntrustedIntermediates, tt.wantUntrustedIntermediates) + } + if summary.ExpiredLeaves != tt.wantExpiredLeaves { + t.Errorf("ExpiredLeaves = %d, want %d", summary.ExpiredLeaves, tt.wantExpiredLeaves) + } + if summary.UntrustedLeaves != tt.wantUntrustedLeaves { + t.Errorf("UntrustedLeaves = %d, want %d", summary.UntrustedLeaves, tt.wantUntrustedLeaves) + } + }) + } +} + +func TestMemStore_ScanSummaryNilPool(t *testing.T) { + // WHY: When RootPool is nil, ScanSummary must skip trust checking entirely. + // Expired counts are still computed, but untrusted counts must be zero. + t.Parallel() + + ca := newRSACA(t) + expired := newExpiredLeaf(t, ca) + + store := NewMemStore() + if err := store.HandleCertificate(ca.cert, "ca.pem"); err != nil { + t.Fatal(err) + } + if err := store.HandleCertificate(expired.cert, "expired.pem"); err != nil { + t.Fatal(err) + } + + // Nil pool — trust checking is skipped + summary := store.ScanSummary(ScanSummaryInput{}) + + if summary.ExpiredLeaves != 1 { + t.Errorf("ExpiredLeaves = %d, want 1", summary.ExpiredLeaves) + } + if summary.UntrustedLeaves != 0 { + t.Errorf("UntrustedLeaves = %d, want 0 (nil pool skips trust)", summary.UntrustedLeaves) + } + if summary.UntrustedRoots != 0 { + t.Errorf("UntrustedRoots = %d, want 0 (nil pool skips trust)", summary.UntrustedRoots) + } +} + func TestMemStore_AllCertsFlat(t *testing.T) { // WHY: AllCertsFlat must return every stored cert, including multiple certs // per SKI, for operations like --dump-certs that need the full list. @@ -1020,6 +1248,36 @@ func TestMemStore_HandleKey_NilPEM(t *testing.T) { } } +func TestMemStore_AllKeys_ReturnsCopy(t *testing.T) { + // WHY: AllKeys must return a copy of the internal map so callers cannot + // corrupt store state by modifying the returned map. + t.Parallel() + + store := NewMemStore() + ca := newRSACA(t) + leaf := newRSALeaf(t, ca, "copy.example.com", []string{"copy.example.com"}) + + if err := store.HandleKey(leaf.key, leaf.keyPEM, "copy.key"); err != nil { + t.Fatal(err) + } + + keys := store.AllKeys() + if len(keys) != 1 { + t.Fatalf("expected 1 key, got %d", len(keys)) + } + + // Mutate the returned map — should not affect the store + for ski := range keys { + delete(keys, ski) + } + + // Store must still have the key + keys2 := store.AllKeys() + if len(keys2) != 1 { + t.Errorf("store was mutated via returned map: got %d keys, want 1", len(keys2)) + } +} + func TestMemStore_MatchedPairs_OrphanedKey(t *testing.T) { // WHY: A key without any matching certificate must NOT appear in MatchedPairs. // MatchedPairs iterates certsBySKI and checks for matching keys; an orphaned diff --git a/internal/certstore/sqlite_test.go b/internal/certstore/sqlite_test.go index 30991125..76e9ae7e 100644 --- a/internal/certstore/sqlite_test.go +++ b/internal/certstore/sqlite_test.go @@ -72,7 +72,7 @@ func TestSaveToSQLite_RoundTrip(t *testing.T) { t.Error("stored key does not Equal original after round-trip") } - summary := store2.ScanSummary() + summary := store2.ScanSummary(ScanSummaryInput{}) if summary.Roots != 1 { t.Errorf("roots: got %d, want 1", summary.Roots) } diff --git a/internal/certstore/summary.go b/internal/certstore/summary.go index 0662eb8e..31389a98 100644 --- a/internal/certstore/summary.go +++ b/internal/certstore/summary.go @@ -1,10 +1,23 @@ package certstore +import "crypto/x509" + +// ScanSummaryInput holds parameters for ScanSummary. +type ScanSummaryInput struct { + RootPool *x509.CertPool // nil skips trust checking +} + // ScanSummary holds aggregate counts from a scan operation. type ScanSummary struct { - Roots int `json:"roots"` - Intermediates int `json:"intermediates"` - Leaves int `json:"leaves"` - Keys int `json:"keys"` - Matched int `json:"key_cert_pairs"` + Roots int `json:"roots"` + Intermediates int `json:"intermediates"` + Leaves int `json:"leaves"` + Keys int `json:"keys"` + Matched int `json:"key_cert_pairs"` + ExpiredRoots int `json:"expired_roots"` + ExpiredIntermediates int `json:"expired_intermediates"` + ExpiredLeaves int `json:"expired_leaves"` + UntrustedRoots int `json:"untrusted_roots"` + UntrustedIntermediates int `json:"untrusted_intermediates"` + UntrustedLeaves int `json:"untrusted_leaves"` } diff --git a/internal/certstore/testhelpers_test.go b/internal/certstore/testhelpers_test.go index b175121a..7e8cf598 100644 --- a/internal/certstore/testhelpers_test.go +++ b/internal/certstore/testhelpers_test.go @@ -218,6 +218,35 @@ func newExpiredLeaf(t *testing.T, ca testCA) testLeaf { return testLeaf{cert: cert, certPEM: certPEM, certDER: certDER, key: key, keyPEM: keyPEM} } +// newExpiredRoot generates a self-signed root CA certificate that has expired. +func newExpiredRoot(t *testing.T) *x509.Certificate { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generate expired root key: %v", err) + } + + tmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Expired Test Root CA"}, + NotBefore: time.Now().Add(-3 * 365 * 24 * time.Hour), + NotAfter: time.Now().Add(-24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + BasicConstraintsValid: true, + IsCA: true, + } + + certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create expired root cert: %v", err) + } + cert, err := x509.ParseCertificate(certDER) + if err != nil { + t.Fatalf("parse expired root cert: %v", err) + } + return cert +} + // newIntermediateCA generates an intermediate CA signed by the given root CA. func newIntermediateCA(t *testing.T, root testCA) testCA { t.Helper() diff --git a/internal/format.go b/internal/format.go new file mode 100644 index 00000000..d0b9f3a8 --- /dev/null +++ b/internal/format.go @@ -0,0 +1,22 @@ +package internal + +import ( + "fmt" + "strings" +) + +// CertAnnotation returns a parenthetical annotation like " (2 expired, 1 untrusted)" +// for non-zero counts, or an empty string if both are zero. +func CertAnnotation(expired, untrusted int) string { + var parts []string + if expired > 0 { + parts = append(parts, fmt.Sprintf("%d expired", expired)) + } + if untrusted > 0 { + parts = append(parts, fmt.Sprintf("%d untrusted", untrusted)) + } + if len(parts) == 0 { + return "" + } + return " (" + strings.Join(parts, ", ") + ")" +} diff --git a/internal/format_test.go b/internal/format_test.go new file mode 100644 index 00000000..32f18e1e --- /dev/null +++ b/internal/format_test.go @@ -0,0 +1,30 @@ +package internal + +import "testing" + +func TestCertAnnotation(t *testing.T) { + // WHY: CertAnnotation formats the parenthetical trust/expiry annotations + // in scan summary output. All four code paths must produce correct output. + t.Parallel() + + tests := []struct { + name string + expired int + untrusted int + want string + }{ + {"both zero", 0, 0, ""}, + {"only expired", 3, 0, " (3 expired)"}, + {"only untrusted", 0, 2, " (2 untrusted)"}, + {"both non-zero", 1, 4, " (1 expired, 4 untrusted)"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := CertAnnotation(tt.expired, tt.untrusted) + if got != tt.want { + t.Errorf("CertAnnotation(%d, %d) = %q, want %q", tt.expired, tt.untrusted, got, tt.want) + } + }) + } +} diff --git a/internal/inspect.go b/internal/inspect.go index 50048c8e..c555f91c 100644 --- a/internal/inspect.go +++ b/internal/inspect.go @@ -26,6 +26,8 @@ type InspectResult struct { NotBefore string `json:"not_before,omitempty"` NotAfter string `json:"not_after,omitempty"` CertType string `json:"cert_type,omitempty"` + Expired *bool `json:"expired,omitempty"` + Trusted *bool `json:"trusted,omitempty"` KeyAlgo string `json:"key_algorithm,omitempty"` KeySize string `json:"key_size,omitempty"` SANs []string `json:"sans,omitempty"` @@ -38,6 +40,8 @@ type InspectResult struct { KeyType string `json:"key_type,omitempty"` CSRSubject string `json:"csr_subject,omitempty"` CSRDNSNames []string `json:"csr_dns_names,omitempty"` + + cert *x509.Certificate // unexported; retained for trust annotation } // InspectFile reads a file and returns inspection results for all objects found. @@ -171,6 +175,7 @@ func inspectCert(cert *x509.Certificate) InspectResult { SKI: certkit.CertSKIEmbedded(cert), AKI: certkit.CertAKIEmbedded(cert), SigAlg: cert.SignatureAlgorithm.String(), + cert: cert, } } @@ -203,6 +208,13 @@ func inspectKey(key any) InspectResult { return r } +func boolYesNo(v bool) string { + if v { + return "yes" + } + return "no" +} + func publicKeySize(pub any) string { switch k := pub.(type) { case *rsa.PublicKey: @@ -229,6 +241,38 @@ func privateKeySize(key any) string { } } +// AnnotateInspectTrust sets the Expired and Trusted fields on certificate +// results using Mozilla roots for chain verification. Intermediate certificates +// found in the results are used to build chains. +func AnnotateInspectTrust(results []InspectResult) error { + mozillaPool, err := certkit.MozillaRootPool() + if err != nil { + return fmt.Errorf("loading Mozilla root pool: %w", err) + } + + // Build intermediate pool from all certs in the results + intermediatePool := x509.NewCertPool() + for i := range results { + if results[i].cert != nil && results[i].CertType == "intermediate" { + intermediatePool.AddCert(results[i].cert) + } + } + + now := time.Now() + for i := range results { + if results[i].cert == nil { + continue + } + cert := results[i].cert + expired := now.After(cert.NotAfter) + results[i].Expired = &expired + + trusted := certkit.VerifyChainTrust(certkit.VerifyChainTrustInput{Cert: cert, Roots: mozillaPool, Intermediates: intermediatePool}) + results[i].Trusted = &trusted + } + return nil +} + // FormatInspectResults formats inspection results as text or JSON. func FormatInspectResults(results []InspectResult, format string) (string, error) { switch format { @@ -263,6 +307,12 @@ func formatInspectText(results []InspectResult) string { fmt.Fprintf(&sb, " Type: %s\n", r.CertType) fmt.Fprintf(&sb, " Not Before: %s\n", r.NotBefore) fmt.Fprintf(&sb, " Not After: %s\n", r.NotAfter) + if r.Expired != nil { + fmt.Fprintf(&sb, " Expired: %s\n", boolYesNo(*r.Expired)) + } + if r.Trusted != nil { + fmt.Fprintf(&sb, " Trusted: %s\n", boolYesNo(*r.Trusted)) + } fmt.Fprintf(&sb, " Key: %s %s\n", r.KeyAlgo, r.KeySize) fmt.Fprintf(&sb, " Signature: %s\n", r.SigAlg) fmt.Fprintf(&sb, " SHA-256: %s\n", r.SHA256) diff --git a/internal/inspect_test.go b/internal/inspect_test.go index 9bc31e6f..77a2846b 100644 --- a/internal/inspect_test.go +++ b/internal/inspect_test.go @@ -2,8 +2,12 @@ package internal import ( "crypto" + "crypto/rand" + "crypto/rsa" "crypto/x509" + "crypto/x509/pkix" "encoding/json" + "encoding/pem" "errors" "net" "os" @@ -397,6 +401,135 @@ func TestInspectFile_GarbageData(t *testing.T) { } } +func TestAnnotateInspectTrust(t *testing.T) { + // WHY: AnnotateInspectTrust must set Expired and Trusted fields on cert + // results. Self-signed certs (not in Mozilla roots) should be untrusted. + // Expired certs should be marked expired but still get trust-checked + // (at a time just before expiry) so "expired" and "untrusted" are independent. + t.Parallel() + ca := newRSACA(t) + validLeaf := newRSALeaf(t, ca, "valid.example.com", []string{"valid.example.com"}, nil) + expiredLeaf := newExpiredLeaf(t, ca) + + tests := []struct { + name string + results []InspectResult + wantExpired []bool + wantTrusted []bool + }{ + { + name: "valid self-signed leaf is not expired but untrusted", + results: []InspectResult{inspectCert(validLeaf.cert)}, + wantExpired: []bool{false}, + wantTrusted: []bool{false}, // self-signed CA, not in Mozilla roots + }, + { + name: "expired self-signed leaf is expired and untrusted", + results: []InspectResult{inspectCert(expiredLeaf.cert)}, + wantExpired: []bool{true}, + wantTrusted: []bool{false}, + }, + { + name: "Mozilla root is trusted", + results: func() []InspectResult { + pemData := certkit.MozillaRootPEM() + block, _ := pem.Decode(pemData) + cert, _ := x509.ParseCertificate(block.Bytes) + return []InspectResult{inspectCert(cert)} + }(), + wantExpired: []bool{false}, + wantTrusted: []bool{true}, + }, + { + name: "chain in results exercises intermediate pool building", + results: func() []InspectResult { + ca := newRSACA(t) + inter := newIntermediateCA(t, ca) + + leafKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + leafTmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "chain-leaf.example.com"}, + DNSNames: []string{"chain-leaf.example.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + AuthorityKeyId: inter.cert.SubjectKeyId, + } + leafDER, err := x509.CreateCertificate(rand.Reader, leafTmpl, inter.cert, &leafKey.PublicKey, inter.key) + if err != nil { + t.Fatal(err) + } + leafCert, err := x509.ParseCertificate(leafDER) + if err != nil { + t.Fatal(err) + } + + return []InspectResult{ + inspectCert(ca.cert), + inspectCert(inter.cert), + inspectCert(leafCert), + } + }(), + wantExpired: []bool{false, false, false}, + wantTrusted: []bool{false, false, false}, // private CA, not in Mozilla roots + }, + { + name: "non-cert results are skipped", + results: []InspectResult{ + {Type: "private_key", KeyType: "RSA", KeySize: "2048"}, + {Type: "csr", CSRSubject: "CN=test"}, + }, + wantExpired: []bool{}, + wantTrusted: []bool{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + // Make a copy so parallel tests don't interfere + results := make([]InspectResult, len(tt.results)) + copy(results, tt.results) + + if err := AnnotateInspectTrust(results); err != nil { + t.Fatalf("AnnotateInspectTrust: %v", err) + } + + var gotExpired, gotTrusted []bool + for _, r := range results { + if r.Expired != nil { + gotExpired = append(gotExpired, *r.Expired) + } + if r.Trusted != nil { + gotTrusted = append(gotTrusted, *r.Trusted) + } + } + + if len(gotExpired) != len(tt.wantExpired) { + t.Fatalf("got %d Expired annotations, want %d", len(gotExpired), len(tt.wantExpired)) + } + for i, want := range tt.wantExpired { + if gotExpired[i] != want { + t.Errorf("results[%d].Expired = %v, want %v", i, gotExpired[i], want) + } + } + + if len(gotTrusted) != len(tt.wantTrusted) { + t.Fatalf("got %d Trusted annotations, want %d", len(gotTrusted), len(tt.wantTrusted)) + } + for i, want := range tt.wantTrusted { + if gotTrusted[i] != want { + t.Errorf("results[%d].Trusted = %v, want %v", i, gotTrusted[i], want) + } + } + }) + } +} + func TestFormatInspectResults_UnsupportedFormat(t *testing.T) { // WHY: Only "text" and "json" formats are supported; an unsupported format must return an error, not silently produce empty output. t.Parallel() @@ -576,6 +709,17 @@ func TestFormatInspectResults_Text(t *testing.T) { mustContain: []string{"Certificate Signing Request:"}, mustNotContain: []string{"DNS Names:"}, }, + { + name: "certificate with expired and trusted annotations", + results: func() []InspectResult { + expired := true + trusted := false + return []InspectResult{ + {Type: "certificate", Subject: "CN=test", Expired: &expired, Trusted: &trusted}, + } + }(), + mustContain: []string{"Expired: yes", "Trusted: no"}, + }, } for _, tt := range tests { diff --git a/internal/testhelpers_test.go b/internal/testhelpers_test.go index 1de763be..42ec62e7 100644 --- a/internal/testhelpers_test.go +++ b/internal/testhelpers_test.go @@ -256,6 +256,37 @@ func newExpiredLeaf(t *testing.T, ca testCA) testLeaf { return testLeaf{cert: cert, certPEM: certPEM, certDER: certDER, key: key, keyPEM: keyPEM} } +// newIntermediateCA generates an intermediate CA signed by the given root CA. +func newIntermediateCA(t *testing.T, root testCA) testCA { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generate intermediate CA key: %v", err) + } + + tmpl := &x509.Certificate{ + SerialNumber: randomSerial(t), + Subject: pkix.Name{CommonName: "Test Intermediate CA", Organization: []string{"TestOrg"}}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(5 * 365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + BasicConstraintsValid: true, + IsCA: true, + AuthorityKeyId: root.cert.SubjectKeyId, + } + + certDER, err := x509.CreateCertificate(rand.Reader, tmpl, root.cert, &key.PublicKey, root.key) + if err != nil { + t.Fatalf("create intermediate CA cert: %v", err) + } + cert, err := x509.ParseCertificate(certDER) + if err != nil { + t.Fatalf("parse intermediate CA cert: %v", err) + } + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + return testCA{cert: cert, certPEM: certPEM, certDER: certDER, key: key} +} + // newPKCS12Bundle creates a PKCS#12 bundle from a leaf cert and its key. func newPKCS12Bundle(t *testing.T, leaf testLeaf, ca testCA, password string) []byte { t.Helper()