From 2ef4e90651ae2420e5caef56b6c74ef9e5169089 Mon Sep 17 00:00:00 2001 From: Naman Trivedi Date: Sat, 11 Jul 2026 08:47:35 +0000 Subject: [PATCH] crypto/x509: defer directory scan when cert bundle provides roots On systems where a cert bundle file (e.g. /etc/ssl/certs/ca-certificates.crt) is available and contains root certificates, skip eagerly scanning the default cert directories. On most systems the bundle is a concatenation of the individual certificates in the directory, so scanning reads the same certificates again redundantly. The deferred directories are stored on the CertPool and loaded on demand if a certificate lookup misses during verification. This ensures backward compatibility for setups where a directory contains certificates not present in the bundle. When SSL_CERT_DIR is explicitly set by the user, directories are always scanned eagerly, as the user may be intentionally combining certificates from multiple sources. On Fedora, RHEL, and Amazon Linux 2023 systems, /etc/ssl/certs/ contains ~430-750 entries. After the existing same-directory symlink filter, ~150-253 individual files remain. Reading and parsing these on a CPU-constrained environment (e.g. 128MB AWS Lambda with 0.08 vCPU) costs ~820ms and produces zero new certificates beyond what the bundle already provides. Benchmark on AWS Lambda (128MB, provided.al2023): Before: first TLS call takes ~1510ms After: first TLS call takes ~398ms (74% reduction) The optimization also benefits standard Fedora/RHEL systems: Fedora 39: directory scan reduced from ~11ms to 0ms Amazon Linux 2023: directory scan reduced from ~9ms to 0ms Fixes #38869 Updates #74613 --- src/crypto/x509/cert_pool.go | 34 +++- src/crypto/x509/root.go | 54 +++-- src/crypto/x509/root_test.go | 359 ++++++++++++++++++++++++++++++++- src/crypto/x509/verify.go | 35 +++- src/internal/godebugs/table.go | 1 + 5 files changed, 462 insertions(+), 21 deletions(-) diff --git a/src/crypto/x509/cert_pool.go b/src/crypto/x509/cert_pool.go index 4ca73b4e07072d..fecf9695e18b60 100644 --- a/src/crypto/x509/cert_pool.go +++ b/src/crypto/x509/cert_pool.go @@ -33,6 +33,37 @@ type CertPool struct { // verifications, one using the roots provided by the caller, and one using // the system platform verifier. systemPool bool + + // lazyRoots holds deferred cert directories that are loaded on demand + // if primary verification fails. The pointer is shared across clones + // so the directory scan happens at most once across all pools derived + // from the same system roots. + // See https://go.dev/issue/38869. + lazyRoots *lazyRootState +} + +// lazyRootState holds directories to be lazily scanned for certificates. +// It is shared by pointer across CertPool clones. +type lazyRootState struct { + once sync.Once + dirs []string + pool *CertPool +} + +// load triggers the one-time directory scan and returns the resulting pool. +// Returns nil if no directories were configured or no certificates were found. +func (s *lazyRootState) load() *CertPool { + if s == nil { + return nil + } + s.once.Do(func() { + p := NewCertPool() + readCertsFromDirs(p, s.dirs) + if p.len() > 0 { + s.pool = p + } + }) + return s.pool } // lazyCert is minimal metadata about a Cert and a func to retrieve it @@ -90,6 +121,7 @@ func (s *CertPool) Clone() *CertPool { lazyCerts: make([]lazyCert, len(s.lazyCerts)), haveSum: make(map[sum224]bool, len(s.haveSum)), systemPool: s.systemPool, + lazyRoots: s.lazyRoots, } for k, v := range s.byName { indexes := make([]int, len(v)) @@ -272,7 +304,7 @@ func (s *CertPool) Equal(other *CertPool) bool { if s == nil || other == nil { return s == other } - if s.systemPool != other.systemPool || len(s.haveSum) != len(other.haveSum) { + if s.systemPool != other.systemPool || s.lazyRoots != other.lazyRoots || len(s.haveSum) != len(other.haveSum) { return false } for h := range s.haveSum { diff --git a/src/crypto/x509/root.go b/src/crypto/x509/root.go index cb1c392f08fd70..170c4b18242662 100644 --- a/src/crypto/x509/root.go +++ b/src/crypto/x509/root.go @@ -9,6 +9,7 @@ import ( "io/fs" "os" "path/filepath" + "slices" "runtime" "strings" "sync" @@ -133,6 +134,7 @@ const ( ) var x509sslcertoverrideplatform = godebug.New("x509sslcertoverrideplatform") +var x509lazydirscan = godebug.New("x509lazydirscan") func loadSystemRoots() (*CertPool, error) { certFilePath, certDirPath := os.Getenv(certFileEnv), os.Getenv(certDirEnv) @@ -150,6 +152,28 @@ func loadSystemRoots() (*CertPool, error) { return loadOnDiskRoots(certFilePath, certDirPath) } +// readCertsFromDirs reads PEM certificates from each directory and appends +// them to pool. It returns the first non-IsNotExist error encountered, if any. +func readCertsFromDirs(pool *CertPool, dirs []string) error { + var firstErr error + for _, directory := range dirs { + fis, err := readUniqueDirectoryEntries(directory) + if err != nil { + if firstErr == nil && !os.IsNotExist(err) { + firstErr = err + } + continue + } + for _, fi := range fis { + data, err := os.ReadFile(filepath.Join(directory, fi.Name())) + if err == nil { + pool.AppendCertsFromPEM(data) + } + } + } + return firstErr +} + func loadOnDiskRoots(certFilePath, certDirPath string) (*CertPool, error) { roots := NewCertPool() @@ -171,6 +195,7 @@ func loadOnDiskRoots(certFilePath, certDirPath string) (*CertPool, error) { } dirs := certDirectories + userProvidedDirs := false if certDirPath != "" { // OpenSSL and BoringSSL both use ":" as the SSL_CERT_DIR separator on // Unix-like systems, and ";" on Windows. @@ -178,22 +203,25 @@ func loadOnDiskRoots(certFilePath, certDirPath string) (*CertPool, error) { // * https://golang.org/issue/35325 // * https://docs.openssl.org/4.0/man1/openssl-rehash/#environment dirs = filepath.SplitList(certDirPath) + userProvidedDirs = true } - for _, directory := range dirs { - fis, err := readUniqueDirectoryEntries(directory) - if err != nil { - if firstErr == nil && !os.IsNotExist(err) { - firstErr = err - } - continue - } - for _, fi := range fis { - data, err := os.ReadFile(filepath.Join(directory, fi.Name())) - if err == nil { - roots.AppendCertsFromPEM(data) - } + // If we already have roots from a cert file and the directories were not + // explicitly provided by the user, skip scanning them. On most systems + // the cert file is a bundling of the certificates in the directory, so + // scanning would just re-read the same certificates. + // The directories are stored for lazy loading in case chain construction + // using the bundle roots fails, ensuring backward compatibility. + // See https://go.dev/issue/38869. + if roots.len() > 0 && !userProvidedDirs && x509lazydirscan.Value() != "0" { + roots.lazyRoots = &lazyRootState{ + dirs: slices.Clone(dirs), } + return roots, nil + } + + if err := readCertsFromDirs(roots, dirs); err != nil && firstErr == nil { + firstErr = err } if roots.len() > 0 || firstErr == nil { diff --git a/src/crypto/x509/root_test.go b/src/crypto/x509/root_test.go index 75dd0e0971131c..2adb0434b5b9c0 100644 --- a/src/crypto/x509/root_test.go +++ b/src/crypto/x509/root_test.go @@ -6,6 +6,7 @@ package x509 import ( "bytes" + "encoding/pem" "fmt" "internal/testenv" "os" @@ -13,7 +14,9 @@ import ( "runtime" "slices" "strings" + "sync" "testing" + "time" ) func TestFallbackPanic(t *testing.T) { @@ -31,8 +34,8 @@ func TestFallback(t *testing.T) { // manipulate systemRoots without worrying about our working being overwritten systemRootsPool() if systemRoots != nil { - originalSystemRoots := *systemRoots - defer func() { systemRoots = &originalSystemRoots }() + originalSystemRoots := systemRoots + defer func() { systemRoots = originalSystemRoots }() } tests := []struct { @@ -126,9 +129,9 @@ const ( func TestEnvVars(t *testing.T) { tmpDir := t.TempDir() - testCert, err := os.ReadFile("testdata/test-dir.crt") + testCert, err := os.ReadFile(filepath.Join("testdata", "test-dir.crt")) if err != nil { - t.Fatalf("failed to read test cert: %s", err) + t.Fatalf("failed to read test cert: %v", err) } if err := os.WriteFile(filepath.Join(tmpDir, testFile), testCert, 0644); err != nil { t.Fatalf("failed to write test cert: %s", err) @@ -180,12 +183,13 @@ func TestEnvVars(t *testing.T) { }, { // Environment variable empty / unset uses default locations. + // When a bundle file has roots, directories are skipped (see #38869). name: "empty-fall-through", fileEnv: "", dirEnv: "", files: []string{testFile}, dirs: []string{tmpDir}, - cns: []string{testFileCN, testDirCN}, + cns: []string{testFileCN}, }, } @@ -348,6 +352,178 @@ func TestReadUniqueDirectoryEntries(t *testing.T) { } } +func TestLoadOnDiskRootsSkipsDirWhenFileHasRoots(t *testing.T) { + tmpDir := t.TempDir() + + // Create a bundle file with a cert. + testCert, err := os.ReadFile(filepath.Join("testdata", "test-dir.crt")) + if err != nil { + t.Fatalf("failed to read test cert: %v", err) + } + bundleFile := filepath.Join(tmpDir, "bundle.crt") + if err := os.WriteFile(bundleFile, testCert, 0644); err != nil { + t.Fatal(err) + } + + // Create a directory with a DISTINCT cert that should NOT be loaded + // when the bundle already has roots and dirs are not user-provided. + certDir := filepath.Join(tmpDir, "certs") + if err := os.MkdirAll(certDir, 0755); err != nil { + t.Fatal(err) + } + dirCA, _, err := generateCert("Distinct Dir CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(certDir, "extra.pem"), + pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: dirCA.Raw}), 0644); err != nil { + t.Fatal(err) + } + + // Save and override defaults. + origCertFiles, origCertDirectories := certFiles, certDirectories + defer func() { + certFiles = origCertFiles + certDirectories = origCertDirectories + }() + certFiles = []string{bundleFile} + certDirectories = []string{certDir} + + // Load with no user-provided env vars (empty strings = use defaults). + roots, err := loadOnDiskRoots("", "") + if err != nil { + t.Fatal(err) + } + + // Should have only the certs from the bundle file (1 cert). + // The directory cert is distinct, so if it were loaded we'd see 2. + if got := roots.len(); got != 1 { + t.Errorf("got %d certs, want 1 (directory should be skipped when bundle has roots)", got) + } + // Verify directory scan was deferred, not performed eagerly. + if roots.lazyRoots == nil { + t.Fatal("expected deferred root state") + } + if roots.lazyRoots.pool != nil { + t.Fatal("directory was scanned eagerly") + } +} + +func TestLoadOnDiskRootsScansDirWhenBundleHasNoRoots(t *testing.T) { + tmpDir := t.TempDir() + + // Create an empty bundle file (no certs) so it "succeeds" but has no roots. + bundleFile := filepath.Join(tmpDir, "empty-bundle.crt") + if err := os.WriteFile(bundleFile, []byte("not a cert"), 0644); err != nil { + t.Fatal(err) + } + + // Create a real cert only in the directory. + certDir := filepath.Join(tmpDir, "certs") + if err := os.MkdirAll(certDir, 0755); err != nil { + t.Fatal(err) + } + testCert, err := os.ReadFile(filepath.Join("testdata", "test-dir.crt")) + if err != nil { + t.Fatalf("failed to read test cert: %v", err) + } + if err := os.WriteFile(filepath.Join(certDir, "cert.pem"), testCert, 0644); err != nil { + t.Fatal(err) + } + + // Save and override defaults. + origCertFiles, origCertDirectories := certFiles, certDirectories + defer func() { + certFiles = origCertFiles + certDirectories = origCertDirectories + }() + certFiles = []string{bundleFile} + certDirectories = []string{certDir} + + // Load - bundle has no valid certs, so directories should be scanned eagerly. + roots, err := loadOnDiskRoots("", "") + if err != nil { + t.Fatal(err) + } + + // Since bundle had zero roots, the directory scan should have happened eagerly. + if got := roots.len(); got != 1 { + t.Errorf("got %d certs, want 1 (dir should be scanned when bundle has no roots)", got) + } +} + +func TestLoadOnDiskRootsScansDirWhenUserProvided(t *testing.T) { + tmpDir := t.TempDir() + + // Create a bundle with one cert. + bundlePath := filepath.Join(tmpDir, "bundle.crt") + bundleCA, _, err := generateCert("Bundle CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(bundlePath, + pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: bundleCA.Raw}), 0644); err != nil { + t.Fatal(err) + } + + // Create a directory with a different cert. + certDir := filepath.Join(tmpDir, "certs") + if err := os.MkdirAll(certDir, 0755); err != nil { + t.Fatal(err) + } + dirCA, _, err := generateCert("Dir CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(certDir, "cert.pem"), + pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: dirCA.Raw}), 0644); err != nil { + t.Fatal(err) + } + + // When user explicitly provides SSL_CERT_DIR, directories should be + // scanned eagerly even though the bundle already has roots. + t.Setenv(certFileEnv, bundlePath) + t.Setenv(certDirEnv, certDir) + + roots, err := loadSystemRoots() + if err != nil { + t.Fatal(err) + } + + // Should have 2 certs: one from bundle + one from user-provided dir. + if got := roots.len(); got != 2 { + t.Errorf("got %d certs, want 2 (user-provided dir should be scanned eagerly)", got) + } + // User-provided dirs are scanned eagerly — no lazy state should exist. + if roots.lazyRoots != nil { + t.Error("expected lazyRoots to be nil for user-provided dirs") + } +} + +func TestLazyDirScanGODEBUG(t *testing.T) { + testenv.SetGODEBUG(t, "x509lazydirscan=0") + + bundleCA, _, err := generateCert("Bundle CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + dirCA, _, err := generateCert("Dir CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + + roots := setupLazyDirTest(t, bundleCA, dirCA) + + // With x509lazydirscan=0, directories should be scanned eagerly. + // Pool should have 2 certs (bundle + dir) and no lazy state. + if got := roots.len(); got != 2 { + t.Errorf("got %d certs, want 2 (GODEBUG=0 should eagerly scan dirs)", got) + } + if roots.lazyRoots != nil { + t.Error("expected lazyRoots to be nil when GODEBUG disables lazy scan") + } +} + func TestSSLCertEnvOverride(t *testing.T) { testenv.SetGODEBUG(t, "x509sslcertoverrideplatform=0") t.Setenv(certFileEnv, "/tmp/nope") @@ -366,3 +542,176 @@ func TestSSLCertEnvOverride(t *testing.T) { t.Fatal("x509sslcertoverrideplatform caused a systemPool to be returned on OS other than windows or darwin") } } + +func TestLazyDirFallbackVerifiesCert(t *testing.T) { + // CA only in directory (not bundle) — verification should trigger lazy + // loading and succeed. + bundleCA, _, err := generateCert("Bundle Dummy", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + dirCA, dirKey, _ := generateCert("Dir Only CA", true, nil, nil, nil) + leaf, _, err := generateCert("leaf.example.com", false, dirCA, dirKey, nil) + if err != nil { + t.Fatal(err) + } + + roots := setupLazyDirTest(t, bundleCA, dirCA) + + if roots.len() != 1 { + t.Fatalf("expected 1 cert in pool, got %d", roots.len()) + } + if roots.lazyRoots == nil { + t.Fatal("expected lazyRoots to be set") + } + + _, err = leaf.Verify(VerifyOptions{Roots: roots, CurrentTime: time.Now()}) + if err != nil { + t.Fatalf("verification failed (lazy dir fallback did not work): %v", err) + } + + // Confirm the lazy pool was created (check directly, not via load()). + if roots.lazyRoots.pool == nil { + t.Error("expected lazyPool to be set after verification") + } +} + +func TestLazyDirConcurrentAccess(t *testing.T) { + bundleCA, bundleKey, err := generateCert("Bundle CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + dirCA, dirKey, err := generateCert("Lazy CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + bundleLeaf, _, err := generateCert("bundle.example.com", false, bundleCA, bundleKey, nil) + if err != nil { + t.Fatal(err) + } + lazyLeaf, _, err := generateCert("lazy.example.com", false, dirCA, dirKey, nil) + if err != nil { + t.Fatal(err) + } + + roots := setupLazyDirTest(t, bundleCA, dirCA) + + // Concurrent verification: some hit bundle, some trigger lazy load. + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(2) + go func() { + defer wg.Done() + if _, err := bundleLeaf.Verify(VerifyOptions{Roots: roots, CurrentTime: time.Now()}); err != nil { + t.Errorf("bundle leaf verify failed: %v", err) + } + }() + go func() { + defer wg.Done() + if _, err := lazyLeaf.Verify(VerifyOptions{Roots: roots, CurrentTime: time.Now()}); err != nil { + t.Errorf("lazy leaf verify failed: %v", err) + } + }() + } + wg.Wait() +} + +// setupLazyDirTest creates a bundle with bundleCA and a directory with dirCA, +// then calls loadOnDiskRoots with default paths. Returns the resulting pool. +func setupLazyDirTest(t *testing.T, bundleCA, dirCA *Certificate) *CertPool { + t.Helper() + tmpDir := t.TempDir() + + bundlePath := filepath.Join(tmpDir, "bundle.crt") + if err := os.WriteFile(bundlePath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: bundleCA.Raw}), 0644); err != nil { + t.Fatal(err) + } + + certDir := filepath.Join(tmpDir, "certs") + if err := os.MkdirAll(certDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(certDir, "dir-ca.pem"), pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: dirCA.Raw}), 0644); err != nil { + t.Fatal(err) + } + + origCertFiles, origCertDirectories := certFiles, certDirectories + t.Cleanup(func() { certFiles = origCertFiles; certDirectories = origCertDirectories }) + certFiles = []string{bundlePath} + certDirectories = []string{certDir} + + roots, err := loadOnDiskRoots("", "") + if err != nil { + t.Fatal(err) + } + return roots +} + +func TestLazyDirWorksAfterClone(t *testing.T) { + // CA only in directory, verified via Clone (simulates SystemCertPool()). + bundleCA, _, err := generateCert("Bundle CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + dirCA, dirKey, _ := generateCert("Dir CA", true, nil, nil, nil) + leaf, _, err := generateCert("clone.example.com", false, dirCA, dirKey, nil) + if err != nil { + t.Fatal(err) + } + + roots := setupLazyDirTest(t, bundleCA, dirCA) + cloned := roots.Clone() + + // Verify the clone preserved the lazy state (not eagerly resolved). + if cloned.lazyRoots == nil { + t.Fatal("clone lost lazyRoots") + } + if cloned.lazyRoots.pool != nil { + t.Fatal("clone eagerly loaded lazy dirs") + } + if cloned.lazyRoots != roots.lazyRoots { + t.Fatal("clone does not share lazyRoots pointer with original") + } + + if _, err := leaf.Verify(VerifyOptions{Roots: cloned, CurrentTime: time.Now()}); err != nil { + t.Fatalf("verification via cloned pool failed: %v", err) + } +} + +func TestLazyDirSameSubjectDifferentKey(t *testing.T) { + // Bundle and directory have CAs with the same subject but different keys. + // Leaf is signed by the directory CA's key. + bundleCA, _, err := generateCert("Shared CA Name", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + dirCA, dirKey, _ := generateCert("Shared CA Name", true, nil, nil, nil) + leaf, _, err := generateCert("samesubject.example.com", false, dirCA, dirKey, nil) + if err != nil { + t.Fatal(err) + } + + roots := setupLazyDirTest(t, bundleCA, dirCA) + + if _, err := leaf.Verify(VerifyOptions{Roots: roots, CurrentTime: time.Now()}); err != nil { + t.Fatalf("same-subject different-key verification failed: %v", err) + } +} + +func TestLazyDirDirectTrust(t *testing.T) { + // A self-signed cert in the directory is directly trusted (matched via containsRoot). + bundleCA, _, err := generateCert("Bundle CA", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + dirCert, _, err := generateCert("Directly Trusted", true, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + + roots := setupLazyDirTest(t, bundleCA, dirCert) + + if _, err := dirCert.Verify(VerifyOptions{Roots: roots, CurrentTime: time.Now()}); err != nil { + t.Fatalf("directly-trusted cert from lazy dir not recognized: %v", err) + } +} diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index cba05a1c4623ef..e5db90ae47118c 100644 --- a/src/crypto/x509/verify.go +++ b/src/crypto/x509/verify.go @@ -600,12 +600,43 @@ func (c *Certificate) Verify(opts VerifyOptions) ([][]*Certificate, error) { } var candidateChains [][]*Certificate + sigChecks := new(int) if opts.Roots.contains(c) { candidateChains = [][]*Certificate{{c}} } else { - candidateChains, err = c.buildChains([]*Certificate{c}, nil, &opts) + candidateChains, err = c.buildChains([]*Certificate{c}, sigChecks, &opts) if err != nil { - return nil, err + // If primary verification failed and there are deferred + // directories, load them and retry using the directory roots. + // This handles the case where a certificate was added to a + // directory without rebuilding the system bundle file. + // + // The retry occurs after chain construction fails, not after + // post-build filtering (EKU, policies, constraints). This + // follows the CAfile-first, CApath-on-demand behavior used by + // OpenSSL. Setting SSL_CERT_DIR explicitly restores eager loading. + // + // The retry searches only the lazy pool because the primary + // roots were already exhausted in the first attempt. + if err != errSignatureLimit && opts.Roots.lazyRoots != nil { + primaryErr := err + if lazyPool := opts.Roots.lazyRoots.load(); lazyPool != nil { + if lazyPool.contains(c) { + candidateChains = [][]*Certificate{{c}} + err = nil + } else { + lazyOpts := opts + lazyOpts.Roots = lazyPool + candidateChains, err = c.buildChains([]*Certificate{c}, sigChecks, &lazyOpts) + } + } + if err != nil { + err = primaryErr + } + } + if err != nil { + return nil, err + } } } diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index c0b2ab933addbe..7492c499cb9237 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -72,6 +72,7 @@ var All = []Info{ {Name: "urlstrictcolons", Package: "net/url", Changed: 26, Old: "0"}, {Name: "winreadlinkvolume", Package: "os", Changed: 23, Old: "0"}, {Name: "winsymlink", Package: "os", Changed: 23, Old: "0"}, + {Name: "x509lazydirscan", Package: "crypto/x509", Changed: 27, Old: "0"}, {Name: "x509negativeserial", Package: "crypto/x509", Changed: 23, Old: "1"}, {Name: "x509rsacrt", Package: "crypto/x509", Changed: 24, Old: "0"}, {Name: "x509sha256skid", Package: "crypto/x509", Changed: 25, Old: "0"},