]> Cypherpunks repositories - gostls13.git/commitdiff
crypto/x509: add non-cgo darwin system anchor certs
authorJosh Bleecher Snyder <josharian@gmail.com>
Wed, 18 Dec 2013 15:57:07 +0000 (10:57 -0500)
committerAdam Langley <agl@golang.org>
Wed, 18 Dec 2013 15:57:07 +0000 (10:57 -0500)
The set of certs fetched via exec'ing `security` is not quite identical
to the certs fetched via the cgo call. The cgo fetch includes
any trusted root certs that the user may have added; exec does not.
The exec fetch includes an Apple-specific root cert; the cgo fetch
does not. Other than that, they appear to be the same.

Unfortunately, os/exec depends on crypto/x509, via net/http. Break the
circular dependency by moving the exec tests to their own package.

This will not work in iOS; we'll cross that bridge when we get to it.

R=golang-dev, minux.ma, agl
CC=golang-dev
https://golang.org/cl/22020045

src/pkg/crypto/x509/root_cgo_darwin.go [new file with mode: 0644]
src/pkg/crypto/x509/root_darwin.go
src/pkg/crypto/x509/root_darwin_test.go [new file with mode: 0644]
src/pkg/crypto/x509/root_nocgo_darwin.go [new file with mode: 0644]
src/pkg/crypto/x509/root_stub.go [deleted file]
src/pkg/os/exec/exec_test.go

diff --git a/src/pkg/crypto/x509/root_cgo_darwin.go b/src/pkg/crypto/x509/root_cgo_darwin.go
new file mode 100644 (file)
index 0000000..bdcc2c1
--- /dev/null
@@ -0,0 +1,79 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build cgo
+
+package x509
+
+/*
+#cgo CFLAGS: -mmacosx-version-min=10.6 -D__MAC_OS_X_VERSION_MAX_ALLOWED=1060
+#cgo LDFLAGS: -framework CoreFoundation -framework Security
+
+#include <CoreFoundation/CoreFoundation.h>
+#include <Security/Security.h>
+
+// FetchPEMRoots fetches the system's list of trusted X.509 root certificates.
+//
+// On success it returns 0 and fills pemRoots with a CFDataRef that contains the extracted root
+// certificates of the system. On failure, the function returns -1.
+//
+// Note: The CFDataRef returned in pemRoots must be released (using CFRelease) after
+// we've consumed its content.
+int FetchPEMRoots(CFDataRef *pemRoots) {
+       if (pemRoots == NULL) {
+               return -1;
+       }
+
+       CFArrayRef certs = NULL;
+       OSStatus err = SecTrustCopyAnchorCertificates(&certs);
+       if (err != noErr) {
+               return -1;
+       }
+
+       CFMutableDataRef combinedData = CFDataCreateMutable(kCFAllocatorDefault, 0);
+       int i, ncerts = CFArrayGetCount(certs);
+       for (i = 0; i < ncerts; i++) {
+               CFDataRef data = NULL;
+               SecCertificateRef cert = (SecCertificateRef)CFArrayGetValueAtIndex(certs, i);
+               if (cert == NULL) {
+                       continue;
+               }
+
+               // Note: SecKeychainItemExport is deprecated as of 10.7 in favor of SecItemExport.
+               // Once we support weak imports via cgo we should prefer that, and fall back to this
+               // for older systems.
+               err = SecKeychainItemExport(cert, kSecFormatX509Cert, kSecItemPemArmour, NULL, &data);
+               if (err != noErr) {
+                       continue;
+               }
+
+               if (data != NULL) {
+                       CFDataAppendBytes(combinedData, CFDataGetBytePtr(data), CFDataGetLength(data));
+                       CFRelease(data);
+               }
+       }
+
+       CFRelease(certs);
+
+       *pemRoots = combinedData;
+       return 0;
+}
+*/
+import "C"
+import "unsafe"
+
+func initSystemRoots() {
+       roots := NewCertPool()
+
+       var data C.CFDataRef = nil
+       err := C.FetchPEMRoots(&data)
+       if err == -1 {
+               return
+       }
+
+       defer C.CFRelease(C.CFTypeRef(data))
+       buf := C.GoBytes(unsafe.Pointer(C.CFDataGetBytePtr(data)), C.int(C.CFDataGetLength(data)))
+       roots.AppendCertsFromPEM(buf)
+       systemRoots = roots
+}
index ad3bfb4b432c9ff527f7f7f4cffff1c72c4e6521..2a61d36eaeaf2a29bfaccfd3139f834e538f6dec 100644 (file)
@@ -1,81 +1,23 @@
-// Copyright 2011 The Go Authors. All rights reserved.
+// Copyright 2013 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package x509
 
-/*
-#cgo CFLAGS: -mmacosx-version-min=10.6 -D__MAC_OS_X_VERSION_MAX_ALLOWED=1060
-#cgo LDFLAGS: -framework CoreFoundation -framework Security
-
-#include <CoreFoundation/CoreFoundation.h>
-#include <Security/Security.h>
-
-// FetchPEMRoots fetches the system's list of trusted X.509 root certificates.
-//
-// On success it returns 0 and fills pemRoots with a CFDataRef that contains the extracted root
-// certificates of the system. On failure, the function returns -1.
-//
-// Note: The CFDataRef returned in pemRoots must be released (using CFRelease) after
-// we've consumed its content.
-int FetchPEMRoots(CFDataRef *pemRoots) {
-       if (pemRoots == NULL) {
-               return -1;
-       }
-
-       CFArrayRef certs = NULL;
-       OSStatus err = SecTrustCopyAnchorCertificates(&certs);
-       if (err != noErr) {
-               return -1;
-       }
-
-       CFMutableDataRef combinedData = CFDataCreateMutable(kCFAllocatorDefault, 0);
-       int i, ncerts = CFArrayGetCount(certs);
-       for (i = 0; i < ncerts; i++) {
-               CFDataRef data = NULL;
-               SecCertificateRef cert = (SecCertificateRef)CFArrayGetValueAtIndex(certs, i);
-               if (cert == NULL) {
-                       continue;
-               }
-
-               // Note: SecKeychainItemExport is deprecated as of 10.7 in favor of SecItemExport.
-               // Once we support weak imports via cgo we should prefer that, and fall back to this
-               // for older systems.
-               err = SecKeychainItemExport(cert, kSecFormatX509Cert, kSecItemPemArmour, NULL, &data);
-               if (err != noErr) {
-                       continue;
-               }
-
-               if (data != NULL) {
-                       CFDataAppendBytes(combinedData, CFDataGetBytePtr(data), CFDataGetLength(data));
-                       CFRelease(data);
-               }
-       }
-
-       CFRelease(certs);
-
-       *pemRoots = combinedData;
-       return 0;
-}
-*/
-import "C"
-import "unsafe"
+import "os/exec"
 
 func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
        return nil, nil
 }
 
-func initSystemRoots() {
-       roots := NewCertPool()
-
-       var data C.CFDataRef = nil
-       err := C.FetchPEMRoots(&data)
-       if err == -1 {
-               return
+func execSecurityRoots() (*CertPool, error) {
+       cmd := exec.Command("/usr/bin/security", "find-certificate", "-a", "-p", "/System/Library/Keychains/SystemRootCertificates.keychain")
+       data, err := cmd.Output()
+       if err != nil {
+               return nil, err
        }
 
-       defer C.CFRelease(C.CFTypeRef(data))
-       buf := C.GoBytes(unsafe.Pointer(C.CFDataGetBytePtr(data)), C.int(C.CFDataGetLength(data)))
-       roots.AppendCertsFromPEM(buf)
-       systemRoots = roots
+       roots := NewCertPool()
+       roots.AppendCertsFromPEM(data)
+       return roots, nil
 }
diff --git a/src/pkg/crypto/x509/root_darwin_test.go b/src/pkg/crypto/x509/root_darwin_test.go
new file mode 100644 (file)
index 0000000..87ea4e3
--- /dev/null
@@ -0,0 +1,50 @@
+package x509
+
+import "testing"
+
+func TestSystemRoots(t *testing.T) {
+       sysRoots := systemRootsPool()         // actual system roots
+       execRoots, err := execSecurityRoots() // non-cgo roots
+
+       if err != nil {
+               t.Fatalf("failed to read system roots: %v", err)
+       }
+
+       for _, tt := range []*CertPool{sysRoots, execRoots} {
+               if tt == nil {
+                       t.Fatal("no system roots")
+               }
+               // On Mavericks, there are 212 bundled certs; require only
+               // 150 here, since this is just a sanity check, and the
+               // exact number will vary over time.
+               if want, have := 150, len(tt.certs); have < want {
+                       t.Fatalf("want at least %d system roots, have %d", want, have)
+               }
+       }
+
+       // Check that the two cert pools are roughly the same;
+       // |A∩B| > max(|A|, |B|) / 2 should be a reasonably robust check.
+
+       isect := make(map[string]bool, len(sysRoots.certs))
+       for _, c := range sysRoots.certs {
+               isect[string(c.Raw)] = true
+       }
+
+       have := 0
+       for _, c := range execRoots.certs {
+               if isect[string(c.Raw)] {
+                       have++
+               }
+       }
+
+       var want int
+       if nsys, nexec := len(sysRoots.certs), len(execRoots.certs); nsys > nexec {
+               want = nsys / 2
+       } else {
+               want = nexec / 2
+       }
+
+       if have < want {
+               t.Errorf("insufficent overlap between cgo and non-cgo roots; want at least %d, have %d", want, have)
+       }
+}
diff --git a/src/pkg/crypto/x509/root_nocgo_darwin.go b/src/pkg/crypto/x509/root_nocgo_darwin.go
new file mode 100644 (file)
index 0000000..d00e257
--- /dev/null
@@ -0,0 +1,11 @@
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !cgo
+
+package x509
+
+func initSystemRoots() {
+       systemRoots, _ = execSecurityRoots()
+}
diff --git a/src/pkg/crypto/x509/root_stub.go b/src/pkg/crypto/x509/root_stub.go
deleted file mode 100644 (file)
index 4c742cc..0000000
+++ /dev/null
@@ -1,14 +0,0 @@
-// Copyright 2011 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build darwin,!cgo
-
-package x509
-
-func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
-       return nil, nil
-}
-
-func initSystemRoots() {
-}
index c380d6506c4bb6e87933f9dd3d181433d5c73e28..5cf8437fbb783c41e7dcc1421aca582422c37cef 100644 (file)
@@ -2,7 +2,10 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package exec
+// Use an external test to avoid os/exec -> net/http -> crypto/x509 -> os/exec
+// circular dependency on non-cgo darwin.
+
+package exec_test
 
 import (
        "bufio"
@@ -14,6 +17,7 @@ import (
        "net/http"
        "net/http/httptest"
        "os"
+       "os/exec"
        "path/filepath"
        "runtime"
        "strconv"
@@ -22,10 +26,10 @@ import (
        "time"
 )
 
-func helperCommand(s ...string) *Cmd {
+func helperCommand(s ...string) *exec.Cmd {
        cs := []string{"-test.run=TestHelperProcess", "--"}
        cs = append(cs, s...)
-       cmd := Command(os.Args[0], cs...)
+       cmd := exec.Command(os.Args[0], cs...)
        cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"}
        return cmd
 }
@@ -58,8 +62,8 @@ func TestCatStdin(t *testing.T) {
 func TestCatGoodAndBadFile(t *testing.T) {
        // Testing combined output and error values.
        bs, err := helperCommand("cat", "/bogus/file.foo", "exec_test.go").CombinedOutput()
-       if _, ok := err.(*ExitError); !ok {
-               t.Errorf("expected *ExitError from cat combined; got %T: %v", err, err)
+       if _, ok := err.(*exec.ExitError); !ok {
+               t.Errorf("expected *exec.ExitError from cat combined; got %T: %v", err, err)
        }
        s := string(bs)
        sp := strings.SplitN(s, "\n", 2)
@@ -77,7 +81,7 @@ func TestCatGoodAndBadFile(t *testing.T) {
 
 func TestNoExistBinary(t *testing.T) {
        // Can't run a non-existent binary
-       err := Command("/no-exist-binary").Run()
+       err := exec.Command("/no-exist-binary").Run()
        if err == nil {
                t.Error("expected error from /no-exist-binary")
        }
@@ -92,12 +96,12 @@ func TestExitStatus(t *testing.T) {
        case "plan9":
                want = fmt.Sprintf("exit status: '%s %d: 42'", filepath.Base(cmd.Path), cmd.ProcessState.Pid())
        }
-       if werr, ok := err.(*ExitError); ok {
+       if werr, ok := err.(*exec.ExitError); ok {
                if s := werr.Error(); s != want {
                        t.Errorf("from exit 42 got exit %q, want %q", s, want)
                }
        } else {
-               t.Fatalf("expected *ExitError from exit 42; got %T: %v", err, err)
+               t.Fatalf("expected *exec.ExitError from exit 42; got %T: %v", err, err)
        }
 }
 
@@ -184,7 +188,7 @@ func TestStdinClose(t *testing.T) {
 func TestPipeLookPathLeak(t *testing.T) {
        fd0 := numOpenFDS(t)
        for i := 0; i < 4; i++ {
-               cmd := Command("something-that-does-not-exist-binary")
+               cmd := exec.Command("something-that-does-not-exist-binary")
                cmd.StdoutPipe()
                cmd.StderrPipe()
                cmd.StdinPipe()
@@ -199,7 +203,7 @@ func TestPipeLookPathLeak(t *testing.T) {
 }
 
 func numOpenFDS(t *testing.T) int {
-       lsof, err := Command("lsof", "-n", "-p", strconv.Itoa(os.Getpid())).Output()
+       lsof, err := exec.Command("lsof", "-n", "-p", strconv.Itoa(os.Getpid())).Output()
        if err != nil {
                t.Skip("skipping test; error finding or running lsof")
                return 0
@@ -425,7 +429,7 @@ func TestExtraFilesRace(t *testing.T) {
                }
                return f
        }
-       runCommand := func(c *Cmd, out chan<- string) {
+       runCommand := func(c *exec.Cmd, out chan<- string) {
                bout, err := c.CombinedOutput()
                if err != nil {
                        out <- "ERROR:" + err.Error()
@@ -577,7 +581,7 @@ func TestHelperProcess(*testing.T) {
                                }
                                if got := f.Fd(); got != wantfd {
                                        fmt.Printf("leaked parent file. fd = %d; want %d\n", got, wantfd)
-                                       out, _ := Command(ofcmd, "-p", fmt.Sprint(os.Getpid())).CombinedOutput()
+                                       out, _ := exec.Command(ofcmd, "-p", fmt.Sprint(os.Getpid())).CombinedOutput()
                                        fmt.Print(string(out))
                                        os.Exit(1)
                                }