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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>

diff --git a/src/crypto/x509/root_unix.go b/src/crypto/x509/root_unix.go
index 1be4058bab7234b5a50bfbf3e38de0c745b1d25f..b48e618a652cb9dd8fef91c4311c5a7bb24ee952 100644 (file)
--- 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 5a27d639b5383f1b5d970da8df35b84a937b4fe0..39556ae60d379f036f9522451a5c4f00c90c1099 100644 (file)
--- 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)
+       }
+}