From 9b189686a53d7fec7deb93d7521531157aa023cb Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 24 Apr 2020 12:15:04 -0700 Subject: [PATCH] crypto/x509: don't read symlinked root certs from disk twice On Linux distros at least, it's common for cert directories to have symlinks pointing to other certs or even other symlinks. An example from Debian stretch's /etc/ssl/certs directory: ... lrwxrwxrwx 1 root root 46 Aug 13 2018 106f3e4d.0 -> Entrust_Root_Certification_Authority_-_EC1.pem lrwxrwxrwx 1 root root 49 Aug 13 2018 116bf586.0 -> GeoTrust_Primary_Certification_Authority_-_G2.pem lrwxrwxrwx 1 root root 35 Aug 13 2018 128805a3.0 -> EE_Certification_Centre_Root_CA.pem lrwxrwxrwx 1 root root 26 Aug 13 2018 157753a5.0 -> AddTrust_External_Root.pem lrwxrwxrwx 1 root root 59 Aug 13 2018 1636090b.0 -> Hellenic_Academic_and_Research_Institutions_RootCA_2011.pem lrwxrwxrwx 1 root root 23 Aug 13 2018 18856ac4.0 -> SecureSign_RootCA11.pem lrwxrwxrwx 1 root root 31 Aug 13 2018 1d3472b9.0 -> GlobalSign_ECC_Root_CA_-_R5.pem lrwxrwxrwx 1 root root 37 Aug 13 2018 1e08bfd1.0 -> IdenTrust_Public_Sector_Root_CA_1.pem lrwxrwxrwx 1 root root 35 Nov 8 21:13 773e07ad.0 -> OISTE_WISeKey_Global_Root_GC_CA.pem -rw-r--r-- 1 root root 200061 Nov 8 21:24 ca-certificates.crt lrwxrwxrwx 1 root root 27 Nov 8 21:13 dc4d6a89.0 -> GlobalSign_Root_CA_-_R6.pem lrwxrwxrwx 1 root root 62 Nov 8 21:13 GlobalSign_Root_CA_-_R6.pem -> /usr/share/ca-certificates/mozilla/GlobalSign_Root_CA_-_R6.crt drwxr-xr-x 2 root root 4096 Jan 26 2019 java lrwxrwxrwx 1 root root 70 Nov 8 21:13 OISTE_WISeKey_Global_Root_GC_CA.pem -> /usr/share/ca-certificates/mozilla/OISTE_WISeKey_Global_Root_GC_CA.crt ... The root_unix.go code read those certs with same-directory twice before. This drops the number of files read from 258 to 130. Saves about 20 ms. Change-Id: I36a1b1e8bb8d89ed3dac8b6255f9048cb7f08fe8 Reviewed-on: https://go-review.googlesource.com/c/go/+/229918 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Filippo Valsorda --- src/crypto/x509/root_unix.go | 29 ++++++++++++++++++++++++++++- src/crypto/x509/root_unix_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/crypto/x509/root_unix.go b/src/crypto/x509/root_unix.go index 1be4058bab..b48e618a65 100644 --- a/src/crypto/x509/root_unix.go +++ b/src/crypto/x509/root_unix.go @@ -9,6 +9,7 @@ package x509 import ( "io/ioutil" "os" + "path/filepath" "strings" ) @@ -69,7 +70,7 @@ func loadSystemRoots() (*CertPool, error) { } for _, directory := range dirs { - fis, err := ioutil.ReadDir(directory) + fis, err := readUniqueDirectoryEntries(directory) if err != nil { if firstErr == nil && !os.IsNotExist(err) { firstErr = err @@ -90,3 +91,29 @@ func loadSystemRoots() (*CertPool, error) { return nil, firstErr } + +// readUniqueDirectoryEntries is like ioutil.ReadDir but omits +// symlinks that point within the directory. +func readUniqueDirectoryEntries(dir string) ([]os.FileInfo, error) { + fis, err := ioutil.ReadDir(dir) + if err != nil { + return nil, err + } + uniq := fis[:0] + for _, fi := range fis { + if !isSameDirSymlink(fi, dir) { + uniq = append(uniq, fi) + } + } + return uniq, nil +} + +// isSameDirSymlink reports whether fi in dir is a symlink with a +// target not containing a slash. +func isSameDirSymlink(fi os.FileInfo, dir string) bool { + if fi.Mode()&os.ModeSymlink == 0 { + return false + } + target, err := os.Readlink(filepath.Join(dir, fi.Name())) + return err == nil && !strings.Contains(target, "/") +} diff --git a/src/crypto/x509/root_unix_test.go b/src/crypto/x509/root_unix_test.go index 5a27d639b5..39556ae60d 100644 --- a/src/crypto/x509/root_unix_test.go +++ b/src/crypto/x509/root_unix_test.go @@ -202,3 +202,30 @@ func TestLoadSystemCertsLoadColonSeparatedDirs(t *testing.T) { t.Fatalf("Mismatched certPools\nGot:\n%s\n\nWant:\n%s", g, w) } } + +func TestReadUniqueDirectoryEntries(t *testing.T) { + temp := func(base string) string { return filepath.Join(t.TempDir(), base) } + if f, err := os.Create(temp("file")); err != nil { + t.Fatal(err) + } else { + f.Close() + } + if err := os.Symlink("target-in", temp("link-in")); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../target-out", temp("link-out")); err != nil { + t.Fatal(err) + } + got, err := readUniqueDirectoryEntries(t.TempDir()) + if err != nil { + t.Fatal(err) + } + gotNames := []string{} + for _, fi := range got { + gotNames = append(gotNames, fi.Name()) + } + wantNames := []string{"file", "link-out"} + if !reflect.DeepEqual(gotNames, wantNames) { + t.Errorf("got %q; want %q", gotNames, wantNames) + } +} -- 2.50.0