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"},