]> Cypherpunks repositories - gostls13.git/commitdiff
crypto/x509: add SystemCertPool, refactor system cert pool loading
authorBrad Fitzpatrick <bradfitz@golang.org>
Wed, 30 Mar 2016 05:41:18 +0000 (16:41 +1100)
committerBrad Fitzpatrick <bradfitz@golang.org>
Thu, 31 Mar 2016 07:52:10 +0000 (07:52 +0000)
This exports the system cert pool.

The system cert loading was refactored to let it be run multiple times
(so callers get a copy, and can't mutate global state), and also to
not discard errors.

SystemCertPool returns an error on Windows. Maybe it's fixable later,
but so far we haven't used it, since the system verifies TLS.

Fixes #13335

Change-Id: I3dfb4656a373f241bae8529076d24c5f532f113c
Reviewed-on: https://go-review.googlesource.com/21293
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
src/crypto/x509/cert_pool.go
src/crypto/x509/root.go
src/crypto/x509/root_cgo_darwin.go
src/crypto/x509/root_darwin_arm_gen.go
src/crypto/x509/root_darwin_armx.go
src/crypto/x509/root_nocgo_darwin.go
src/crypto/x509/root_plan9.go
src/crypto/x509/root_unix.go
src/crypto/x509/root_windows.go
src/crypto/x509/verify.go

index 2362e84688d140179f5e77a0ef111056a13dc328..59ab8871054aef95e176c376409989b625ad7883 100644 (file)
@@ -6,6 +6,8 @@ package x509
 
 import (
        "encoding/pem"
+       "errors"
+       "runtime"
 )
 
 // CertPool is a set of certificates.
@@ -18,12 +20,22 @@ type CertPool struct {
 // NewCertPool returns a new, empty CertPool.
 func NewCertPool() *CertPool {
        return &CertPool{
-               make(map[string][]int),
-               make(map[string][]int),
-               nil,
+               bySubjectKeyId: make(map[string][]int),
+               byName:         make(map[string][]int),
        }
 }
 
+// SystemCertPool returns a copy of the system cert pool.
+//
+// Any mutations to the returned pool are not written to disk and do
+// not affect any other pool.
+func SystemCertPool() (*CertPool, error) {
+       if runtime.GOOS == "windows" {
+               return nil, errors.New("crypto/x509: system root pool is not available on Windows")
+       }
+       return loadSystemRoots()
+}
+
 // findVerifiedParents attempts to find certificates in s which have signed the
 // given certificate. If any candidates were rejected then errCert will be set
 // to one of them, arbitrarily, and err will contain the reason that it was
@@ -107,10 +119,10 @@ func (s *CertPool) AppendCertsFromPEM(pemCerts []byte) (ok bool) {
 
 // Subjects returns a list of the DER-encoded subjects of
 // all of the certificates in the pool.
-func (s *CertPool) Subjects() (res [][]byte) {
-       res = make([][]byte, len(s.certs))
+func (s *CertPool) Subjects() [][]byte {
+       res := make([][]byte, len(s.certs))
        for i, c := range s.certs {
                res[i] = c.RawSubject
        }
-       return
+       return res
 }
index 8aae14e09ee72626b64895c8ffd282938c62faeb..787d955be47db2e80be9416c20ea2c6613499f81 100644 (file)
@@ -7,11 +7,16 @@ package x509
 import "sync"
 
 var (
-       once        sync.Once
-       systemRoots *CertPool
+       once           sync.Once
+       systemRoots    *CertPool
+       systemRootsErr error
 )
 
 func systemRootsPool() *CertPool {
        once.Do(initSystemRoots)
        return systemRoots
 }
+
+func initSystemRoots() {
+       systemRoots, systemRootsErr = loadSystemRoots()
+}
index bf4a5cdfeeef2fcf15475e300fa332c74ba9bb8c..f067cd7cf432fcd134400d569e94c8531a1f0491 100644 (file)
@@ -61,19 +61,23 @@ int FetchPEMRoots(CFDataRef *pemRoots) {
 }
 */
 import "C"
-import "unsafe"
+import (
+       "errors"
+       "unsafe"
+)
 
-func initSystemRoots() {
+func loadSystemRoots() (*CertPool, error) {
        roots := NewCertPool()
 
        var data C.CFDataRef = nil
        err := C.FetchPEMRoots(&data)
        if err == -1 {
-               return
+               // TODO: better error message
+               return nil, errors.New("crypto/x509: failed to load darwin system roots with cgo")
        }
 
        defer C.CFRelease(C.CFTypeRef(data))
        buf := C.GoBytes(unsafe.Pointer(C.CFDataGetBytePtr(data)), C.int(C.CFDataGetLength(data)))
        roots.AppendCertsFromPEM(buf)
-       systemRoots = roots
+       return roots, nil
 }
index 5817158c33b6e90b9c040cddb6d4542d7f76ccb4..6b373b9d536b0504fc680180e778ecc745e5d101 100644 (file)
@@ -184,8 +184,9 @@ const header = `
 
 package x509
 
-func initSystemRoots() {
-       systemRoots = NewCertPool()
-       systemRoots.AppendCertsFromPEM([]byte(systemRootsPEM))
+func loadSystemRoots() (*CertPool, error) {
+       p := NewCertPool()
+       p.AppendCertsFromPEM([]byte(systemRootsPEM))
+       return p
 }
 `
index 37675b48a39035ac2e2036a2fe4f9d7b9171bb53..66b70516843018eb60fb02316497e3449d9920e1 100644 (file)
 
 package x509
 
-func initSystemRoots() {
-       systemRoots = NewCertPool()
-       systemRoots.AppendCertsFromPEM([]byte(systemRootsPEM))
+func loadSystemRoots() (*CertPool, error) {
+       p := NewCertPool()
+       p.AppendCertsFromPEM([]byte(systemRootsPEM))
+       return p
 }
 
 const systemRootsPEM = `
index d00e2576624ad5c67cd325846e07171090579b0e..2ac4666aff65b6c010fe94caefa2b485f8240f17 100644 (file)
@@ -6,6 +6,6 @@
 
 package x509
 
-func initSystemRoots() {
-       systemRoots, _ = execSecurityRoots()
+func loadSystemRoots() (*CertPool, error) {
+       return execSecurityRoots()
 }
index 9965caadee3a887dd744f30051deacb51692cb57..ebeb7dfccd86c068c4b42bb6b7f84b58ca010637 100644 (file)
@@ -6,7 +6,10 @@
 
 package x509
 
-import "io/ioutil"
+import (
+       "io/ioutil"
+       "os"
+)
 
 // Possible certificate files; stop after finding one.
 var certFiles = []string{
@@ -17,17 +20,18 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
        return nil, nil
 }
 
-func initSystemRoots() {
+func loadSystemRoots() (*CertPool, error) {
        roots := NewCertPool()
+       var bestErr error
        for _, file := range certFiles {
                data, err := ioutil.ReadFile(file)
                if err == nil {
                        roots.AppendCertsFromPEM(data)
-                       systemRoots = roots
-                       return
+                       return roots, nil
+               }
+               if bestErr == nil || (os.IsNotExist(bestErr) && !os.IsNotExist(err)) {
+                       bestErr = err
                }
        }
-
-       // All of the files failed to load. systemRoots will be nil which will
-       // trigger a specific error at verification time.
+       return nil, bestErr
 }
index 9f06f9dabbe67781e8799654724b759c3f5697d1..7bcb3d63d1f935e2a9897dac62b5d4a10b3cab8b 100644 (file)
@@ -6,7 +6,10 @@
 
 package x509
 
-import "io/ioutil"
+import (
+       "io/ioutil"
+       "os"
+)
 
 // Possible directories with certificate files; stop after successfully
 // reading at least one file from a directory.
@@ -19,20 +22,26 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
        return nil, nil
 }
 
-func initSystemRoots() {
+func loadSystemRoots() (*CertPool, error) {
        roots := NewCertPool()
+       var firstErr error
        for _, file := range certFiles {
                data, err := ioutil.ReadFile(file)
                if err == nil {
                        roots.AppendCertsFromPEM(data)
-                       systemRoots = roots
-                       return
+                       return roots, nil
+               }
+               if firstErr == nil && !os.IsNotExist(err) {
+                       firstErr = err
                }
        }
 
        for _, directory := range certDirectories {
                fis, err := ioutil.ReadDir(directory)
                if err != nil {
+                       if firstErr == nil && !os.IsNotExist(err) {
+                               firstErr = err
+                       }
                        continue
                }
                rootsAdded := false
@@ -43,11 +52,9 @@ func initSystemRoots() {
                        }
                }
                if rootsAdded {
-                       systemRoots = roots
-                       return
+                       return roots, nil
                }
        }
 
-       // All of the files failed to load. systemRoots will be nil which will
-       // trigger a specific error at verification time.
+       return nil, firstErr
 }
index 51c3be3fa461686bdfb1b78458d7908922fa2efc..392c869012d8fc2f5d4b6ef730b49043b828984d 100644 (file)
@@ -225,5 +225,4 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
        return chains, nil
 }
 
-func initSystemRoots() {
-}
+func loadSystemRoots() (*CertPool, error) { return nil, nil }
index d3b62d174c70531ab8f0a17da3aca736c8950df0..85c083fbb2c9c40481c48cb84936cb2c1bf8d4de 100644 (file)
@@ -117,10 +117,16 @@ func (e UnknownAuthorityError) Error() string {
 }
 
 // SystemRootsError results when we fail to load the system root certificates.
-type SystemRootsError struct{}
+type SystemRootsError struct {
+       Err error
+}
 
-func (SystemRootsError) Error() string {
-       return "x509: failed to load system roots and no roots provided"
+func (se SystemRootsError) Error() string {
+       msg := "x509: failed to load system roots and no roots provided"
+       if se.Err != nil {
+               return msg + "; " + se.Err.Error()
+       }
+       return msg
 }
 
 // errNotParsed is returned when a certificate without ASN.1 contents is
@@ -240,7 +246,7 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
        if opts.Roots == nil {
                opts.Roots = systemRootsPool()
                if opts.Roots == nil {
-                       return nil, SystemRootsError{}
+                       return nil, SystemRootsError{systemRootsErr}
                }
        }