FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/


Description:
crypto/x509: return a better error when we fail to load system roots.

Please review this at https://codereview.appspot.com/7157044/

Affected files:
M src/pkg/crypto/x509/root_darwin.go
M src/pkg/crypto/x509/root_plan9.go
M src/pkg/crypto/x509/root_stub.go
M src/pkg/crypto/x509/root_unix.go
M src/pkg/crypto/x509/root_windows.go
M src/pkg/crypto/x509/verify.go
M src/pkg/crypto/x509/verify_test.go


Index: src/pkg/crypto/x509/root_darwin.go
===================================================================
--- a/src/pkg/crypto/x509/root_darwin.go
+++ b/src/pkg/crypto/x509/root_darwin.go
@@ -70,11 +70,12 @@

var data C.CFDataRef = nil
err := C.FetchPEMRoots(&data)
- if err != -1 {
- defer C.CFRelease(C.CFTypeRef(data))
- buf := C.GoBytes(unsafe.Pointer(C.CFDataGetBytePtr(data)),
C.int(C.CFDataGetLength(data)))
- roots.AppendCertsFromPEM(buf)
+ 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: src/pkg/crypto/x509/root_plan9.go
===================================================================
--- a/src/pkg/crypto/x509/root_plan9.go
+++ b/src/pkg/crypto/x509/root_plan9.go
@@ -23,9 +23,11 @@
data, err := ioutil.ReadFile(file)
if err == nil {
roots.AppendCertsFromPEM(data)
- break
+ systemRoots = roots
+ return
}
}

- systemRoots = roots
+ // All of the files failed to load. systemRoots will be nil which will
+ // trigger a specific error at verification time.
}
Index: src/pkg/crypto/x509/root_stub.go
===================================================================
--- a/src/pkg/crypto/x509/root_stub.go
+++ b/src/pkg/crypto/x509/root_stub.go
@@ -11,5 +11,4 @@
}

func initSystemRoots() {
- systemRoots = NewCertPool()
}
Index: src/pkg/crypto/x509/root_unix.go
===================================================================
--- a/src/pkg/crypto/x509/root_unix.go
+++ b/src/pkg/crypto/x509/root_unix.go
@@ -27,9 +27,11 @@
data, err := ioutil.ReadFile(file)
if err == nil {
roots.AppendCertsFromPEM(data)
- break
+ systemRoots = roots
+ return
}
}

- systemRoots = roots
+ // All of the files failed to load. systemRoots will be nil which will
+ // trigger a specific error at verification time.
}
Index: src/pkg/crypto/x509/root_windows.go
===================================================================
--- a/src/pkg/crypto/x509/root_windows.go
+++ b/src/pkg/crypto/x509/root_windows.go
@@ -226,5 +226,4 @@
}

func initSystemRoots() {
- systemRoots = NewCertPool()
}
Index: src/pkg/crypto/x509/verify.go
===================================================================
--- a/src/pkg/crypto/x509/verify.go
+++ b/src/pkg/crypto/x509/verify.go
@@ -82,6 +82,14 @@
return "x509: certificate signed by unknown authority"
}

+// SystemRootsError results when we fail to load the system root
certificates.
+type SystemRootsError struct {
+}
+
+func (e SystemRootsError) Error() string {
+ return "x509: failed to load system roots and no roots provided"
+}
+
// VerifyOptions contains parameters for Certificate.Verify. It's a
structure
// because other PKIX verification APIs have ended up needing many options.
type VerifyOptions struct {
@@ -170,6 +178,9 @@

if opts.Roots == nil {
opts.Roots = systemRootsPool()
+ if opts.Roots == nil {
+ return nil, SystemRootsError{}
+ }
}

err = c.isValid(leafCertificate, nil, &opts)
Index: src/pkg/crypto/x509/verify_test.go
===================================================================
--- a/src/pkg/crypto/x509/verify_test.go
+++ b/src/pkg/crypto/x509/verify_test.go
@@ -15,13 +15,14 @@
)

type verifyTest struct {
- leaf string
- intermediates []string
- roots []string
- currentTime int64
- dnsName string
- systemSkip bool
- keyUsages []ExtKeyUsage
+ leaf string
+ intermediates []string
+ roots []string
+ currentTime int64
+ dnsName string
+ systemSkip bool
+ keyUsages []ExtKeyUsage
+ testSystemRootsError bool

errorCallback func(*testing.T, int, error) bool
expectedChains [][]string
@@ -29,6 +30,17 @@

var verifyTests = []verifyTest{
{
+ leaf: googleLeaf,
+ intermediates: []string{thawteIntermediate},
+ currentTime: 1302726541,
+ dnsName: "www.google.com",
+ testSystemRootsError: true,
+
+ // Without any roots specified we should get a system roots
+ // error.
+ errorCallback: expectSystemRootsError,
+ },
+ {
leaf: googleLeaf,
intermediates: []string{thawteIntermediate},
roots: []string{verisignRoot},
@@ -180,6 +192,14 @@
return true
}

+func expectSystemRootsError(t *testing.T, i int, err error) bool {
+ if _, ok := err.(SystemRootsError); !ok {
+ t.Errorf("#%d: error was not SystemRootsError: %s", i, err)
+ return false
+ }
+ return true
+}
+
func certificateFromPEM(pemBytes string) (*Certificate, error) {
block, _ := pem.Decode([]byte(pemBytes))
if block == nil {
@@ -226,8 +246,19 @@
return
}

+ var oldSystemRoots *CertPool
+ if test.testSystemRootsError {
+ oldSystemRoots = systemRootsPool()
+ systemRoots = nil
+ opts.Roots = nil
+ }
+
chains, err := leaf.Verify(opts)

+ if test.testSystemRootsError {
+ systemRoots = oldSystemRoots
+ }
+
if test.errorCallback == nil && err != nil {
t.Errorf("#%d: unexpected error: %s", i, err)
}

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedJan 17, '13 at 8:24p
activeJan 17, '13 at 8:24p
posts1
users1
websitegolang.org

1 user in discussion

Agl: 1 post

People

Translate

site design / logo © 2022 Grokbase