feat: add trust and expiry annotations to scan, inspect, and WASM

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

bundle.go

@@ -1,13 +1,15 @@
 package certkit
 
 import (
+	"bytes"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
 	"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))

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 {