[go] crypto/x509: load certs from env vars + extra locations
Steven Hartland (Gerrit)
Steven Hartland has uploaded this change for review.
crypto/x509: load certs from env vars + extra locations Add the ability to override the default file and directory from which certificates are loaded by settings the OpenSSL compatible environment variables: SSL_CERT_FILE, SSL_CERT_DIR. If the variables are set the default locations are not checked. Added new default file "/usr/local/etc/ssl/cert.pem" for FreeBSD. Certificates found in the first valid location found for both file and directory are added, instead of only the first file location if a valid one was found, which is consistent with OpenSSL. Fixes #3905 Fixes #14022 Fixes #14311 Fixes #16920 Fixes #18813 - If user sets SSL_CERT_FILE. Change-Id: Ia24fb7c1c2ffff4338b4cf214bd040326ce27bb0 --- M src/crypto/x509/root_bsd.go M src/crypto/x509/root_unix.go A src/crypto/x509/root_unix_test.go A src/crypto/x509/test-file.crt A src/crypto/x509/testdata/test-dir.crt 5 files changed, 220 insertions(+), 2 deletions(-)
diff --git a/src/crypto/x509/root_bsd.go b/src/crypto/x509/root_bsd.go
index 9317283..1371933 100644
--- a/src/crypto/x509/root_bsd.go
+++ b/src/crypto/x509/root_bsd.go
@@ -8,7 +8,8 @@
// Possible certificate files; stop after finding one.
var certFiles = []string{
- "/usr/local/share/certs/ca-root-nss.crt", // FreeBSD/DragonFly
+ "/usr/local/etc/ssl/cert.pem", // FreeBSD
"/etc/ssl/cert.pem", // OpenBSD
+ "/usr/local/share/certs/ca-root-nss.crt", // DragonFly
"/etc/openssl/certs/ca-certificates.crt", // NetBSD
}
diff --git a/src/crypto/x509/root_unix.go b/src/crypto/x509/root_unix.go
index 7bcb3d6..15262e7 100644
--- a/src/crypto/x509/root_unix.go
+++ b/src/crypto/x509/root_unix.go
@@ -16,7 +16,20 @@
var certDirectories = []string{
"/etc/ssl/certs", // SLES10/SLES11, https://golang.org/issue/12139
"/system/etc/security/cacerts", // Android
+ "/usr/local/share/certs", // FreeBSD
+ "/etc/pki/tls/certs", // Fedora/RHEL
+ "/etc/openssl/certs", // NetBSD
}
+
+const (
+ // CertFileEnv is the environment variable which identifies where to locate
+ // the SSL certificate file. If set this overrides the system default.
+ CertFileEnv = "SSL_CERT_FILE"
+
+ // CertDirEnv is the environment variable which identifies which directory
+ // to check for SSL certificate files. If set this overrides the system default.
+ CertDirEnv = "SSL_CERT_DIR"
+)
func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
return nil, nil
@@ -24,16 +37,25 @@
func loadSystemRoots() (*CertPool, error) {
roots := NewCertPool()
+
+ if f := os.Getenv(CertFileEnv); f != "" {
+ certFiles = []string{f}
+ }
+
var firstErr error
for _, file := range certFiles {
data, err := ioutil.ReadFile(file)
if err == nil {
roots.AppendCertsFromPEM(data)
- return roots, nil
+ break
}
if firstErr == nil && !os.IsNotExist(err) {
firstErr = err
}
+ }
+
+ if d := os.Getenv(CertDirEnv); d != "" {
+ certDirectories = []string{d}
}
for _, directory := range certDirectories {
@@ -56,5 +78,9 @@
}
}
+ if len(roots.certs) > 0 {
+ return roots, nil
+ }
+
return nil, firstErr
}
diff --git a/src/crypto/x509/root_unix_test.go b/src/crypto/x509/root_unix_test.go
new file mode 100644
index 0000000..ceb6378
--- /dev/null
+++ b/src/crypto/x509/root_unix_test.go
@@ -0,0 +1,128 @@
+// 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 dragonfly freebsd linux nacl netbsd openbsd solaris
+
+package x509
+
+import (
+ "fmt"
+ "os"
+ "testing"
+)
+
+const (
+ testDir = "testdata"
+ testDirCN = "test-dir"
+ testFile = "test-file.crt"
+ testFileCN = "test-file"
+ testMissing = "missing"
+)
+
+func TestEnvVars(t *testing.T) {
+ testCases := []struct {
+ name string
+ fileEnv string
+ dirEnv string
+ files []string
+ dirs []string
+ cns []string
+ }{
+ {
+ // Environment variables override the default locations preventing fall through.
+ name: "overrides defaults",
+ fileEnv: testMissing,
+ dirEnv: testMissing,
+ files: []string{testFile},
+ dirs: []string{testDir},
+ cns: nil,
+ },
+ {
+ // File environment overrides default file locations.
+ name: "file",
+ fileEnv: testFile,
+ dirEnv: "",
+ files: nil,
+ dirs: nil,
+ cns: []string{testFileCN},
+ },
+ {
+ // Directory environment overrides default directory locations.
+ name: "dir",
+ fileEnv: "",
+ dirEnv: testDir,
+ files: nil,
+ dirs: nil,
+ cns: []string{testDirCN},
+ },
+ {
+ // File & directory environment overrides both default locations.
+ name: "file+dir",
+ fileEnv: testFile,
+ dirEnv: testDir,
+ files: nil,
+ dirs: nil,
+ cns: []string{testFileCN, testDirCN},
+ },
+ {
+ // Environment variable empty / unset uses default locations.
+ name: "empty fall through",
+ fileEnv: "",
+ dirEnv: "",
+ files: []string{testFile},
+ dirs: []string{testDir},
+ cns: []string{testFileCN, testDirCN},
+ },
+ }
+
+ // Save old settings so we can restore before the test ends.
+ origCertFiles, origCertDirectories := certFiles, certDirectories
+ origFile, origDir := os.Getenv(CertFileEnv), os.Getenv(CertDirEnv)
+ defer func() {
+ certFiles = origCertFiles
+ certDirectories = origCertDirectories
+ os.Setenv(CertFileEnv, origFile)
+ os.Setenv(CertDirEnv, origDir)
+ }()
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+
+ if err := os.Setenv(CertFileEnv, tc.fileEnv); err != nil {
+ t.Fatalf("setenv %q failed: %v", CertFileEnv, err)
+ }
+ if err := os.Setenv(CertDirEnv, tc.dirEnv); err != nil {
+ t.Fatalf("setenv %q failed: %v", CertDirEnv, err)
+ }
+
+ certFiles, certDirectories = tc.files, tc.dirs
+
+ r, err := loadSystemRoots()
+ if err != nil {
+ t.Fatal("unexpected failure: ", err)
+ }
+
+ if r == nil {
+ if tc.cns == nil {
+ // Expected nil
+ return
+ }
+ t.Fatal("nil roots")
+ }
+
+ for i, cn := range tc.cns {
+ if i > len(r.certs) {
+ t.Errorf("missing cert %v @ %v", cn, i)
+ } else if r.certs[i].Subject.CommonName != cn {
+ fmt.Printf("%#v\n", r.certs[0].Subject)
+ t.Errorf("unexpected cert common name %q expected %q", r.certs[i].Subject.CommonName, cn)
+ }
+ }
+
+ if len(r.certs) > len(tc.cns) {
+ t.Errorf("expected %v certs got %v", len(tc.cns), len(r.certs))
+ }
+ })
+ }
+}
diff --git a/src/crypto/x509/test-file.crt b/src/crypto/x509/test-file.crt
new file mode 100644
index 0000000..caa83b9
--- /dev/null
+++ b/src/crypto/x509/test-file.crt
@@ -0,0 +1,32 @@
+-----BEGIN CERTIFICATE-----
+MIIFbTCCA1WgAwIBAgIJAN338vEmMtLsMA0GCSqGSIb3DQEBCwUAME0xCzAJBgNV
+BAYTAlVLMRMwEQYDVQQIDApUZXN0LVN0YXRlMRUwEwYDVQQKDAxHb2xhbmcgVGVz
+dHMxEjAQBgNVBAMMCXRlc3QtZmlsZTAeFw0xNzAyMDEyMzUyMDhaFw0yNzAxMzAy
+MzUyMDhaME0xCzAJBgNVBAYTAlVLMRMwEQYDVQQIDApUZXN0LVN0YXRlMRUwEwYD
+VQQKDAxHb2xhbmcgVGVzdHMxEjAQBgNVBAMMCXRlc3QtZmlsZTCCAiIwDQYJKoZI
+hvcNAQEBBQADggIPADCCAgoCggIBAPMGiLjdiffQo3Xc8oUe7wsDhSaAJFOhO6Qs
+i0xYrYl7jmCuz9rGD2fdgk5cLqGazKuQ6fIFzHXFU2BKs4CWXt9KO0KFEhfvZeuW
+jG5d7C1ZUiuKOrPqjKVu8SZtFPc7y7Ke7msXzY+Z2LLyiJJ93LCMq4+cTSGNXVlI
+KqUxhxeoD5/QkUPyQy/ilu3GMYfx/YORhDP6Edcuskfj8wRh1UxBejP8YPMvI6St
+cE2GkxoEGqDWnQ/61F18te6WI3MD29tnKXOkXVhnSC+yvRLljotW2/tAhHKBG4tj
+iQWT5Ri4Wrw2tXxPKRLsVWc7e1/hdxhnuvYpXkWNhKsm002jzkFXlzfEwPd8nZdw
+5aT6gPUBN2AAzdoqZI7E200i0orEF7WaSoMfjU1tbHvExp3vyAPOfJ5PS2MQ6W03
+Zsy5dTVH+OBH++rkRzQCFcnIv/OIhya5XZ9KX9nFPgBEP7Xq2A+IjH7B6VN/S/bv
+8lhp2V+SQvlew9GttKC4hKuPsl5o7+CMbcqcNUdxm9gGkN8epGEKCuix97bpNlxN
+fHZxHE5+8GMzPXMkCD56y5TNKR6ut7JGHMPtGl5lPCLqzG/HzYyFgxsDfDUu2B0A
+GKj0lGpnLfGqwhs2/s3jpY7+pcvVQxEpvVTId5byDxu1ujP4HjO/VTQ2P72rE8Ft
+C6J2Av0tAgMBAAGjUDBOMB0GA1UdDgQWBBTLT/RbyfBB/Pa07oBnaM+QSJPO9TAf
+BgNVHSMEGDAWgBTLT/RbyfBB/Pa07oBnaM+QSJPO9TAMBgNVHRMEBTADAQH/MA0G
+CSqGSIb3DQEBCwUAA4ICAQB3sCntCcQwhMgRPPyvOCMyTcQ/Iv+cpfxz2Ck14nlx
+AkEAH2CH0ov5GWTt07/ur3aa5x+SAKi0J3wTD1cdiw4U/6Uin6jWGKKxvoo4IaeK
+SbM8w/6eKx6UbmHx7PA/eRABY9tTlpdPCVgw7/o3WDr03QM+IAtatzvaCPPczake
+pbdLwmBZB/v8V+6jUajy6jOgdSH0PyffGnt7MWgDETmNC6p/Xigp5eh+C8Fb4NGT
+xgHES5PBC+sruWp4u22bJGDKTvYNdZHsnw/CaKQWNsQqwisxa3/8N5v+PCff/pxl
+r05pE3PdHn9JrCl4iWdVlgtiI9BoPtQyDfa/OEFaScE8KYR8LxaAgdgp3zYncWls
+BpwQ6Y/A2wIkhlD9eEp5Ib2hz7isXOs9UwjdriKqrBXqcIAE5M+YIk3+KAQKxAtd
+4YsK3CSJ010uphr12YKqlScj4vuKFjuOtd5RyyMIxUG3lrrhAu2AzCeKCLdVgA8+
+75FrYMApUdvcjp4uzbBoED4XRQlx9kdFHVbYgmE/+yddBYJM8u4YlgAL0hW2/D8p
+z9JWIfxVmjJnBnXaKGBuiUyZ864A3PJndP6EMMo7TzS2CDnfCYuJjvI0KvDjFNmc
+rQA04+qfMSEz3nmKhbbZu4eYLzlADhfH8tT4GMtXf71WLA5AUHGf2Y4+HIHTsmHG
+vQ==
+-----END CERTIFICATE-----
diff --git a/src/crypto/x509/testdata/test-dir.crt b/src/crypto/x509/testdata/test-dir.crt
new file mode 100644
index 0000000..b7fc9c5
--- /dev/null
+++ b/src/crypto/x509/testdata/test-dir.crt
@@ -0,0 +1,31 @@
+-----BEGIN CERTIFICATE-----
+MIIFazCCA1OgAwIBAgIJAL8a/lsnspOqMA0GCSqGSIb3DQEBCwUAMEwxCzAJBgNV
+BAYTAlVLMRMwEQYDVQQIDApUZXN0LVN0YXRlMRUwEwYDVQQKDAxHb2xhbmcgVGVz
+dHMxETAPBgNVBAMMCHRlc3QtZGlyMB4XDTE3MDIwMTIzNTAyN1oXDTI3MDEzMDIz
+NTAyN1owTDELMAkGA1UEBhMCVUsxEzARBgNVBAgMClRlc3QtU3RhdGUxFTATBgNV
+BAoMDEdvbGFuZyBUZXN0czERMA8GA1UEAwwIdGVzdC1kaXIwggIiMA0GCSqGSIb3
+DQEBAQUAA4ICDwAwggIKAoICAQDzBoi43Yn30KN13PKFHu8LA4UmgCRToTukLItM
+WK2Je45grs/axg9n3YJOXC6hmsyrkOnyBcx1xVNgSrOAll7fSjtChRIX72Xrloxu
+XewtWVIrijqz6oylbvEmbRT3O8uynu5rF82Pmdiy8oiSfdywjKuPnE0hjV1ZSCql
+MYcXqA+f0JFD8kMv4pbtxjGH8f2DkYQz+hHXLrJH4/MEYdVMQXoz/GDzLyOkrXBN
+hpMaBBqg1p0P+tRdfLXuliNzA9vbZylzpF1YZ0gvsr0S5Y6LVtv7QIRygRuLY4kF
+k+UYuFq8NrV8TykS7FVnO3tf4XcYZ7r2KV5FjYSrJtNNo85BV5c3xMD3fJ2XcOWk
++oD1ATdgAM3aKmSOxNtNItKKxBe1mkqDH41NbWx7xMad78gDznyeT0tjEOltN2bM
+uXU1R/jgR/vq5Ec0AhXJyL/ziIcmuV2fSl/ZxT4ARD+16tgPiIx+welTf0v27/JY
+adlfkkL5XsPRrbSguISrj7JeaO/gjG3KnDVHcZvYBpDfHqRhCgrosfe26TZcTXx2
+cRxOfvBjMz1zJAg+esuUzSkerreyRhzD7RpeZTwi6sxvx82MhYMbA3w1LtgdABio
+9JRqZy3xqsIbNv7N46WO/qXL1UMRKb1UyHeW8g8btboz+B4zv1U0Nj+9qxPBbQui
+dgL9LQIDAQABo1AwTjAdBgNVHQ4EFgQUy0/0W8nwQfz2tO6AZ2jPkEiTzvUwHwYD
+VR0jBBgwFoAUy0/0W8nwQfz2tO6AZ2jPkEiTzvUwDAYDVR0TBAUwAwEB/zANBgkq
+hkiG9w0BAQsFAAOCAgEAvEVnUYsIOt87rggmLPqEueynkuQ+562M8EDHSQl82zbe
+xDCxeg3DvPgKb+RvaUdt1362z/szK10SoeMgx6+EQLoV9LiVqXwNqeYfixrhrdw3
+ppAhYYhymdkbUQCEMHypmXP1vPhAz4o8Bs+eES1M+zO6ErBiD7SqkmBElT+GixJC
+6epC9ZQFs+dw3lPlbiZSsGE85sqc3VAs0/JgpL/pb1/Eg4s0FUhZD2C2uWdSyZGc
+g0/v3aXJCp4j/9VoNhI1WXz3M45nysZIL5OQgXymLqJElQa1pZ3Wa4i/nidvT4AT
+Xlxc/qijM8set/nOqp7hVd5J0uG6qdwLRILUddZ6OpXd7ZNi1EXg+Bpc7ehzGsDt
+3UFGzYXDjxYnK2frQfjLS8stOQIqSrGthW6x0fdkVx0y8BByvd5J6+JmZl4UZfzA
+m99VxXSt4B9x6BvnY7ktzcFDOjtuLc4B/7yg9fv1eQuStA4cHGGAttsCg1X/Kx8W
+PvkkeH0UWDZ9vhH9K36703z89da6MWF+bz92B0+4HoOmlVaXRkvblsNaynJnL0LC
+Ayry7QBxuh5cMnDdRwJB3AVJIiJ1GVpb7aGvBOnx+s2lwRv9HWtghb+cbwwktx1M
+JHyBf3GZNSWTpKY7cD8V+NnBv3UuioOVVo+XAU4LF/bYUjdRpxWADJizNtZrtFo=
+-----END CERTIFICATE-----
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland uploaded patch set #2 to this change.
crypto/x509: load certs from env vars + extra locations Add the ability to override the default file and directory from which certificates are loaded by setting the OpenSSL compatible environment variables: SSL_CERT_FILE, SSL_CERT_DIR. If the variables are set the default locations are not checked. Added new default file "/usr/local/etc/ssl/cert.pem" for FreeBSD. Certificates in the first valid location found for both file and directory are added, instead of only the first file location if a valid one was found, which is consistent with OpenSSL. Fixes #3905 Fixes #14022 Fixes #14311 Fixes #16920 Fixes #18813 - If user sets SSL_CERT_FILE. Change-Id: Ia24fb7c1c2ffff4338b4cf214bd040326ce27bb0 --- M src/crypto/x509/root_bsd.go M src/crypto/x509/root_unix.go A src/crypto/x509/root_unix_test.go A src/crypto/x509/test-file.crt A src/crypto/x509/testdata/test-dir.crt 5 files changed, 220 insertions(+), 2 deletions(-)
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland posted comments on this change.
Patch set 2:
I was assuming this would be a 1.9 fix.
I basically had a hunt through the unplanned list, emailed out the other day, to see if I could pick up and outstanding FreeBSD tagged issues and found a few which seemed like low hanging fruit.
(2 comments)
File src/crypto/x509/root_unix.go:
CertFileEnv = "SSL_CERT_FILE" // CertDirEnv is the environment variable which identifies which directory // to check for SSL certificate files. If set this overrides the system default. CertDirEnv = "SSL_CERT_DIR"
I think its too late for 1.8 isn't it?
We can make them private if that's preferred, I exported them so their existence is self documenting.
File src/crypto/x509/root_unix_test.go:
Oops copied from root_unix.go fixing...
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland uploaded patch set #3 to this change.
crypto/x509: load certs from env vars + extra locations Add the ability to override the default file and directory from which certificates are loaded by setting the OpenSSL compatible environment variables: SSL_CERT_FILE, SSL_CERT_DIR. If the variables are set the default locations are not checked. Added new default file "/usr/local/etc/ssl/cert.pem" for FreeBSD. Certificates in the first valid location found for both file and directory are added, instead of only the first file location if a valid one was found, which is consistent with OpenSSL. Fixes #3905 Fixes #14022 Fixes #14311 Fixes #16920 Fixes #18813 - If user sets SSL_CERT_FILE. Change-Id: Ia24fb7c1c2ffff4338b4cf214bd040326ce27bb0 --- M src/crypto/x509/root_bsd.go M src/crypto/x509/root_unix.go A src/crypto/x509/root_unix_test.go A src/crypto/x509/test-file.crt A src/crypto/x509/testdata/test-dir.crt 5 files changed, 220 insertions(+), 2 deletions(-)
To view, visit change 36093. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 2:
I assume this is for Go 1.9?
It's too much for Go 1.8.
(2 comments)
CertFileEnv = "SSL_CERT_FILE" // CertDirEnv is the environment variable which identifies which directory // to check for SSL certificate files. If set this overrides the system default. CertDirEnv = "SSL_CERT_DIR"
new API? Definitely not for Go 1.8.
File src/crypto/x509/root_unix_test.go:
This is 6 years ago.
To view, visit change 36093. To unsubscribe, visit settings.
Russ Cox (Gerrit)
Russ Cox posted comments on this change.
Patch set 3:
Agree this is too much for Go 1.8.
Russ Cox (Gerrit)
Russ Cox posted comments on this change.
Patch set 3:Run-TryBot +1Code-Review +1
Looks pretty good. I didn't read the test very carefully. Are the new locations from OpenSSL or something else?
(2 comments)
File src/crypto/x509/root_bsd.go:
Patch Set #3, Line 11:
"/usr/local/etc/ssl/cert.pem", // FreeBSDWhy is this list being reordered? I don't see a reason in the commit message. Please add one or revert this file.
File src/crypto/x509/root_unix.go:
Patch Set #3, Line 27:
CertFileEnv = "SSL_CERT_FILE"Can we unexport these? It's fine to document them but I don't see a reason to export them as new API.
To view, visit change 36093. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 3:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=d4ecabab
Gobot Gobot (Gerrit)
Build is still in progress... This change failed on nacl-386: See https://storage.googleapis.com/go-build-log/d4ecabab/nacl-386_c9a1f3c4.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 3:TryBot-Result -1
2 of 16 TryBots failed: Failed on nacl-386: https://storage.googleapis.com/go-build-log/d4ecabab/nacl-386_c9a1f3c4.log Failed on nacl-amd64p32: https://storage.googleapis.com/go-build-log/d4ecabab/nacl-amd64p32_66df347a.log
Consult https://build.golang.org/ to see whether they are new failures.
Russ Cox (Gerrit)
Russ Cox posted comments on this change.
Patch set 3:
(1 comment)
File src/crypto/x509/root_unix_test.go:
Patch Set #3, Line 5:
// +build dragonfly freebsd linux nacl netbsd openbsd solarisYou can just drop nacl here. No point.
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland uploaded patch set #4 to this change.
crypto/x509: load certs from env vars + extra locations Add the ability to override the default file and directory from which certificates are loaded by setting the OpenSSL compatible environment variables: SSL_CERT_FILE, SSL_CERT_DIR. If the variables are set the default locations are not checked. Added new default file "/usr/local/etc/ssl/cert.pem" for FreeBSD. Certificates in the first valid location found for both file and directory are added, instead of only the first file location if a valid one was found, which is consistent with OpenSSL. Fixes #3905 Fixes #14022 Fixes #14311 Fixes #16920 Fixes #18813 - If user sets SSL_CERT_FILE. Change-Id: Ia24fb7c1c2ffff4338b4cf214bd040326ce27bb0 --- M src/crypto/x509/root_bsd.go M src/crypto/x509/root_unix.go A src/crypto/x509/root_unix_test.go A src/crypto/x509/test-file.crt A src/crypto/x509/testdata/test-dir.crt 5 files changed, 220 insertions(+), 2 deletions(-)
"/system/etc/security/cacerts", // Android
+ "/usr/local/share/certs", // FreeBSD
+ "/etc/pki/tls/certs", // Fedora/RHEL
+ "/etc/openssl/certs", // NetBSD
}
+
+const (
+ // CertFileEnv is the environment variable which identifies where to locate
+ // the SSL certificate file. If set this overrides the system default.
+ CertFileEnv = "SSL_CERT_FILE"
+
+ // CertDirEnv is the environment variable which identifies which directory
+ // to check for SSL certificate files. If set this overrides the system default.
+ CertDirEnv = "SSL_CERT_DIR"
+)
func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
return nil, nil
@@ -24,16 +37,25 @@
func loadSystemRoots() (*CertPool, error) {
roots := NewCertPool()
+
+ if f := os.Getenv(CertFileEnv); f != "" {
+ certFiles = []string{f}
+ }
+
var firstErr error
for _, file := range certFiles {
data, err := ioutil.ReadFile(file)
if err == nil {
roots.AppendCertsFromPEM(data)
- return roots, nil
+ break
}
if firstErr == nil && !os.IsNotExist(err) {
firstErr = err
}
+ }
+
+ if d := os.Getenv(CertDirEnv); d != "" {
+ certDirectories = []string{d}
}
for _, directory := range certDirectories {
@@ -56,5 +78,9 @@
}
}
+ if len(roots.certs) > 0 {
+ return roots, nil
+ }
+
return nil, firstErr
}
diff --git a/src/crypto/x509/root_unix_test.go b/src/crypto/x509/root_unix_test.go
new file mode 100644
index 0000000..c74892c
--- /dev/null
+++ b/src/crypto/x509/root_unix_test.go
@@ -0,0 +1,128 @@
+// Copyright 2017 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 dragonfly freebsd linux netbsd openbsd solaris
+
+package x509
+
+import (
+ "fmt"
+ "os"
+ "testing"
+)
+
+const (
+ testDir = "testdata"
+ testDirCN = "test-dir"
+ testFile = "test-file.crt"
+ testFileCN = "test-file"
+ testMissing = "missing"
+)
+
+func TestEnvVars(t *testing.T) {
+ testCases := []struct {
+ name string
+ fileEnv string
+ dirEnv string
+ files []string
+ dirs []string
+ cns []string
+ }{
+ {
+ // Environment variables override the default locations preventing fall through.
+ name: "overrides defaults",
+ fileEnv: testMissing,
+ dirEnv: testMissing,
+ files: []string{testFile},
+ dirs: []string{testDir},
+ cns: nil,
+ },
+ {
+ // File environment overrides default file locations.
+ name: "file",
+ fileEnv: testFile,
+ dirEnv: "",
+ files: nil,
+ dirs: nil,
+ cns: []string{testFileCN},
+ },
+ {
+ // Directory environment overrides default directory locations.
+ name: "dir",
+ fileEnv: "",
+ dirEnv: testDir,
+ files: nil,
+ dirs: nil,
+ cns: []string{testDirCN},
+ },
+ {
+ // File & directory environment overrides both default locations.
+ name: "file+dir",
+ fileEnv: testFile,
+ dirEnv: testDir,
+ files: nil,
+ dirs: nil,
+ cns: []string{testFileCN, testDirCN},
+ },
+ {
+ // Environment variable empty / unset uses default locations.
+ name: "empty fall through",
+ fileEnv: "",
+ dirEnv: "",
+ files: []string{testFile},
+ dirs: []string{testDir},
+ cns: []string{testFileCN, testDirCN},
+ },
+ }
+
+ // Save old settings so we can restore before the test ends.
+ origCertFiles, origCertDirectories := certFiles, certDirectories
+ origFile, origDir := os.Getenv(CertFileEnv), os.Getenv(CertDirEnv)
+ defer func() {
+ certFiles = origCertFiles
+ certDirectories = origCertDirectories
+ os.Setenv(CertFileEnv, origFile)
+ os.Setenv(CertDirEnv, origDir)
+ }()
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+
+ if err := os.Setenv(CertFileEnv, tc.fileEnv); err != nil {
+ t.Fatalf("setenv %q failed: %v", CertFileEnv, err)
+ }
+ if err := os.Setenv(CertDirEnv, tc.dirEnv); err != nil {
+ t.Fatalf("setenv %q failed: %v", CertDirEnv, err)
+ }
+
+ certFiles, certDirectories = tc.files, tc.dirs
+
+ r, err := loadSystemRoots()
+ if err != nil {
+ t.Fatal("unexpected failure: ", err)
+ }
+
+ if r == nil {
+ if tc.cns == nil {
+ // Expected nil
+ return
+ }
+ t.Fatal("nil roots")
+ }
+
+ for i, cn := range tc.cns {
+ if i > len(r.certs) {
+ t.Errorf("missing cert %v @ %v", cn, i)
+ } else if r.certs[i].Subject.CommonName != cn {
+ fmt.Printf("%#v\n", r.certs[0].Subject)
+ t.Errorf("unexpected cert common name %q expected %q", r.certs[i].Subject.CommonName, cn)
+ }
+ }
+
+ if len(r.certs) > len(tc.cns) {
+ t.Errorf("expected %v certs got %v", len(tc.cns), len(r.certs))
+ }
+ })
+ }
+}
diff --git a/src/crypto/x509/test-file.crt b/src/crypto/x509/test-file.crt
new file mode 100644
index 0000000..caa83b9
--- /dev/null
+++ b/src/crypto/x509/test-file.crt
@@ -0,0 +1,32 @@
+-----BEGIN CERTIFICATE-----
+MIIFbTCCA1WgAwIBAgIJAN338vEmMtLsMA0GCSqGSIb3DQEBCwUAME0xCzAJBgNV
+BAYTAlVLMRMwEQYDVQQIDApUZXN0LVN0YXRlMRUwEwYDVQQKDAxHb2xhbmcgVGVz
+dHMxEjAQBgNVBAMMCXRlc3QtZmlsZTAeFw0xNzAyMDEyMzUyMDhaFw0yNzAxMzAy
+MzUyMDhaME0xCzAJBgNVBAYTAlVLMRMwEQYDVQQIDApUZXN0LVN0YXRlMRUwEwYD
+VQQKDAxHb2xhbmcgVGVzdHMxEjAQBgNVBAMMCXRlc3QtZmlsZTCCAiIwDQYJKoZI
+hvcNAQEBBQADggIPADCCAgoCggIBAPMGiLjdiffQo3Xc8oUe7wsDhSaAJFOhO6Qs
+i0xYrYl7jmCuz9rGD2fdgk5cLqGazKuQ6fIFzHXFU2BKs4CWXt9KO0KFEhfvZeuW
+jG5d7C1ZUiuKOrPqjKVu8SZtFPc7y7Ke7msXzY+Z2LLyiJJ93LCMq4+cTSGNXVlI
+KqUxhxeoD5/QkUPyQy/ilu3GMYfx/YORhDP6Edcuskfj8wRh1UxBejP8YPMvI6St
+cE2GkxoEGqDWnQ/61F18te6WI3MD29tnKXOkXVhnSC+yvRLljotW2/tAhHKBG4tj
+iQWT5Ri4Wrw2tXxPKRLsVWc7e1/hdxhnuvYpXkWNhKsm002jzkFXlzfEwPd8nZdw
+5aT6gPUBN2AAzdoqZI7E200i0orEF7WaSoMfjU1tbHvExp3vyAPOfJ5PS2MQ6W03
+Zsy5dTVH+OBH++rkRzQCFcnIv/OIhya5XZ9KX9nFPgBEP7Xq2A+IjH7B6VN/S/bv
+8lhp2V+SQvlew9GttKC4hKuPsl5o7+CMbcqcNUdxm9gGkN8epGEKCuix97bpNlxN
+fHZxHE5+8GMzPXMkCD56y5TNKR6ut7JGHMPtGl5lPCLqzG/HzYyFgxsDfDUu2B0A
+GKj0lGpnLfGqwhs2/s3jpY7+pcvVQxEpvVTId5byDxu1ujP4HjO/VTQ2P72rE8Ft
+C6J2Av0tAgMBAAGjUDBOMB0GA1UdDgQWBBTLT/RbyfBB/Pa07oBnaM+QSJPO9TAf
+BgNVHSMEGDAWgBTLT/RbyfBB/Pa07oBnaM+QSJPO9TAMBgNVHRMEBTADAQH/MA0G
+CSqGSIb3DQEBCwUAA4ICAQB3sCntCcQwhMgRPPyvOCMyTcQ/Iv+cpfxz2Ck14nlx
+AkEAH2CH0ov5GWTt07/ur3aa5x+SAKi0J3wTD1cdiw4U/6Uin6jWGKKxvoo4IaeK
+SbM8w/6eKx6UbmHx7PA/eRABY9tTlpdPCVgw7/o3WDr03QM+IAtatzvaCPPczake
+pbdLwmBZB/v8V+6jUajy6jOgdSH0PyffGnt7MWgDETmNC6p/Xigp5eh+C8Fb4NGT
+xgHES5PBC+sruWp4u22bJGDKTvYNdZHsnw/CaKQWNsQqwisxa3/8N5v+PCff/pxl
+r05pE3PdHn9JrCl4iWdVlgtiI9BoPtQyDfa/OEFaScE8KYR8LxaAgdgp3zYncWls
+BpwQ6Y/A2wIkhlD9eEp5Ib2hz7isXOs9UwjdriKqrBXqcIAE5M+YIk3+KAQKxAtd
+4YsK3CSJ010uphr12YKqlScj4vuKFjuOtd5RyyMIxUG3lrrhAu2AzCeKCLdVgA8+
+75FrYMApUdvcjp4uzbBoED4XRQlx9kdFHVbYgmE/+yddBYJM8u4YlgAL0hW2/D8p
+z9JWIfxVmjJnBnXaKGBuiUyZ864A3PJndP6EMMo7TzS2CDnfCYuJjvI0KvDjFNmc
+rQA04+qfMSEz3nmKhbbZu4eYLzlADhfH8tT4GMtXf71WLA5AUHGf2Y4+HIHTsmHG
+vQ==
+-----END CERTIFICATE-----
diff --git a/src/crypto/x509/testdata/test-dir.crt b/src/crypto/x509/testdata/test-dir.crt
new file mode 100644
index 0000000..b7fc9c5
--- /dev/null
+++ b/src/crypto/x509/testdata/test-dir.crt
@@ -0,0 +1,31 @@
+-----BEGIN CERTIFICATE-----
+MIIFazCCA1OgAwIBAgIJAL8a/lsnspOqMA0GCSqGSIb3DQEBCwUAMEwxCzAJBgNV
+BAYTAlVLMRMwEQYDVQQIDApUZXN0LVN0YXRlMRUwEwYDVQQKDAxHb2xhbmcgVGVz
+dHMxETAPBgNVBAMMCHRlc3QtZGlyMB4XDTE3MDIwMTIzNTAyN1oXDTI3MDEzMDIz
+NTAyN1owTDELMAkGA1UEBhMCVUsxEzARBgNVBAgMClRlc3QtU3RhdGUxFTATBgNV
+BAoMDEdvbGFuZyBUZXN0czERMA8GA1UEAwwIdGVzdC1kaXIwggIiMA0GCSqGSIb3
+DQEBAQUAA4ICDwAwggIKAoICAQDzBoi43Yn30KN13PKFHu8LA4UmgCRToTukLItM
+WK2Je45grs/axg9n3YJOXC6hmsyrkOnyBcx1xVNgSrOAll7fSjtChRIX72Xrloxu
+XewtWVIrijqz6oylbvEmbRT3O8uynu5rF82Pmdiy8oiSfdywjKuPnE0hjV1ZSCql
+MYcXqA+f0JFD8kMv4pbtxjGH8f2DkYQz+hHXLrJH4/MEYdVMQXoz/GDzLyOkrXBN
+hpMaBBqg1p0P+tRdfLXuliNzA9vbZylzpF1YZ0gvsr0S5Y6LVtv7QIRygRuLY4kF
+k+UYuFq8NrV8TykS7FVnO3tf4XcYZ7r2KV5FjYSrJtNNo85BV5c3xMD3fJ2XcOWk
++oD1ATdgAM3aKmSOxNtNItKKxBe1mkqDH41NbWx7xMad78gDznyeT0tjEOltN2bM
+uXU1R/jgR/vq5Ec0AhXJyL/ziIcmuV2fSl/ZxT4ARD+16tgPiIx+welTf0v27/JY
+adlfkkL5XsPRrbSguISrj7JeaO/gjG3KnDVHcZvYBpDfHqRhCgrosfe26TZcTXx2
+cRxOfvBjMz1zJAg+esuUzSkerreyRhzD7RpeZTwi6sxvx82MhYMbA3w1LtgdABio
+9JRqZy3xqsIbNv7N46WO/qXL1UMRKb1UyHeW8g8btboz+B4zv1U0Nj+9qxPBbQui
+dgL9LQIDAQABo1AwTjAdBgNVHQ4EFgQUy0/0W8nwQfz2tO6AZ2jPkEiTzvUwHwYD
+VR0jBBgwFoAUy0/0W8nwQfz2tO6AZ2jPkEiTzvUwDAYDVR0TBAUwAwEB/zANBgkq
+hkiG9w0BAQsFAAOCAgEAvEVnUYsIOt87rggmLPqEueynkuQ+562M8EDHSQl82zbe
+xDCxeg3DvPgKb+RvaUdt1362z/szK10SoeMgx6+EQLoV9LiVqXwNqeYfixrhrdw3
+ppAhYYhymdkbUQCEMHypmXP1vPhAz4o8Bs+eES1M+zO6ErBiD7SqkmBElT+GixJC
+6epC9ZQFs+dw3lPlbiZSsGE85sqc3VAs0/JgpL/pb1/Eg4s0FUhZD2C2uWdSyZGc
+g0/v3aXJCp4j/9VoNhI1WXz3M45nysZIL5OQgXymLqJElQa1pZ3Wa4i/nidvT4AT
+Xlxc/qijM8set/nOqp7hVd5J0uG6qdwLRILUddZ6OpXd7ZNi1EXg+Bpc7ehzGsDt
+3UFGzYXDjxYnK2frQfjLS8stOQIqSrGthW6x0fdkVx0y8BByvd5J6+JmZl4UZfzA
+m99VxXSt4B9x6BvnY7ktzcFDOjtuLc4B/7yg9fv1eQuStA4cHGGAttsCg1X/Kx8W
+PvkkeH0UWDZ9vhH9K36703z89da6MWF+bz92B0+4HoOmlVaXRkvblsNaynJnL0LC
+Ayry7QBxuh5cMnDdRwJB3AVJIiJ1GVpb7aGvBOnx+s2lwRv9HWtghb+cbwwktx1M
+JHyBf3GZNSWTpKY7cD8V+NnBv3UuioOVVo+XAU4LF/bYUjdRpxWADJizNtZrtFo=
+-----END CERTIFICATE-----
Steven Hartland (Gerrit)
Steven Hartland posted comments on this change.
Patch set 3:
Sorry for the slow reply I didn't get any notification of your comments Russ, so only noticed them when I did an addition change request this morning.
(3 comments)
Patch Set #3, Line 11:
"/usr/local/etc/ssl/cert.pem", // FreeBSDWhy is this list being reordered? I don't see a reason in the commit messag
This isn't a reorder its correcting the FreeBSD entry and adding the separate DragonFly one, which is already mentioned in the commit message.
File src/crypto/x509/root_unix.go:
Patch Set #3, Line 27:
CertFileEnv = "SSL_CERT_FILE"Can we unexport these? It's fine to document them but I don't see a reason
I exported these for compatibility as there are different environment variables used by various software for this purpose.
Exporting an hence allowing the user to update these gives the ability to be compatible with the expected environment variables.
File src/crypto/x509/root_unix_test.go:
Patch Set #3, Line 5:
// +build dragonfly freebsd linux nacl netbsd openbsd solarisYou can just drop nacl here. No point.
Done
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland posted comments on this change.
Patch set 4:
Patch Set 3:
Anything else that needs closing off before this can be merged Russ?
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 5:Run-TryBot +1
(1 comment)
File src/crypto/x509/root_unix.go:
CertFileEnv = "SSL_CERT_FILE"
// CertDirEnv is the environment variable which identifies which directory
// to check for SSL certificate files. If set this overrides the system default.
CertDirEnv = "SSL_CERT_DIR"
do we need these to be exported and add new API to Go? It seems that this could also just be documented in the package doc.
To view, visit change 36093. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 5:
TryBots beginning. Status page: https://farmer.golang.org/try?commit=62d6fa61
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on nacl-386:
See https://storage.googleapis.com/go-build-log/62d6fa61/nacl-386_061c926a.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
To view, visit change 36093. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 5:TryBot-Result -1
2 of 17 TryBots failed:
Failed on nacl-386: https://storage.googleapis.com/go-build-log/62d6fa61/nacl-386_061c926a.log
Failed on windows-386-gce: https://storage.googleapis.com/go-build-log/62d6fa61/windows-386-gce_0244f575.log
Consult https://build.golang.org/ to see whether they are new failures.
Steven Hartland (Gerrit)
Steven Hartland uploaded patch set #6 to this change.
crypto/x509: load certs from env vars + extra locations
Add the ability to override the default file and directory from
which certificates are loaded by setting the OpenSSL compatible
environment variables: SSL_CERT_FILE, SSL_CERT_DIR.
If the variables are set the default locations are not checked.
Added new default file "/usr/local/etc/ssl/cert.pem" for FreeBSD.
Certificates in the first valid location found for both file and
directory are added, instead of only the first file location if
a valid one was found, which is consistent with OpenSSL.
Fixes #3905
Fixes #14022
Fixes #14311
Fixes #16920
Fixes #18813 - If user sets SSL_CERT_FILE.
Change-Id: Ia24fb7c1c2ffff4338b4cf214bd040326ce27bb0
---
M src/crypto/x509/root_bsd.go
M src/crypto/x509/root_unix.go
A src/crypto/x509/root_unix_test.go
A src/crypto/x509/test-file.crt
A src/crypto/x509/testdata/test-dir.crt
M src/crypto/x509/x509.go
6 files changed, 223 insertions(+), 2 deletions(-)
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland posted comments on this change.
Patch set 5:
Patch Set 5: Run-TryBot+1
(1 comment)
Not 100% needed so unexported and added comment to package doc.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 6:
(3 comments)
File src/crypto/x509/root_unix.go:
Patch Set #6, Line 42:
certFiles =I'd prefer if you don't overwrite global variables.
Can you only modify this locally instead?
Patch Set #6, Line 58:
certDirectories = []string{d}likewise
Patch Set #6, Line 9:
respectivelycomma before this?
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland uploaded patch set #7 to this change.
crypto/x509: load certs from env vars + extra locations
Add the ability to override the default file and directory from
which certificates are loaded by setting the OpenSSL compatible
environment variables: SSL_CERT_FILE, SSL_CERT_DIR.
If the variables are set the default locations are not checked.
Added new default file "/usr/local/etc/ssl/cert.pem" for FreeBSD.
Certificates in the first valid location found for both file and
directory are added, instead of only the first file location if
a valid one was found, which is consistent with OpenSSL.
Fixes #3905
Fixes #14022
Fixes #14311
Fixes #16920
Fixes #18813 - If user sets SSL_CERT_FILE.
Change-Id: Ia24fb7c1c2ffff4338b4cf214bd040326ce27bb0
---
M src/crypto/x509/root_bsd.go
M src/crypto/x509/root_unix.go
A src/crypto/x509/root_unix_test.go
A src/crypto/x509/test-file.crt
A src/crypto/x509/testdata/test-dir.crt
M src/crypto/x509/x509.go
6 files changed, 227 insertions(+), 4 deletions(-)
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland posted comments on this change.
Patch set 7:
(3 comments)
I'd prefer if you don't overwrite global variables.
Done
Patch Set #6, Line 58:
dirs := certDirectorieslikewise
Done
Patch Set #6, Line 9:
respectivel
comma before this?
Done
To view, visit change 36093. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 7:Run-TryBot +1Code-Review +2
Brad Fitzpatrick (Gerrit)
RELNOTE=yes
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 7:
TryBots beginning. Status page: https://farmer.golang.org/try?commit=14db93e1
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 7:TryBot-Result +1
TryBots are happy.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick merged this change.
crypto/x509: load certs from env vars + extra locations
Add the ability to override the default file and directory from
which certificates are loaded by setting the OpenSSL compatible
environment variables: SSL_CERT_FILE, SSL_CERT_DIR.
If the variables are set the default locations are not checked.
Added new default file "/usr/local/etc/ssl/cert.pem" for FreeBSD.
Certificates in the first valid location found for both file and
directory are added, instead of only the first file location if
a valid one was found, which is consistent with OpenSSL.
Fixes #3905
Fixes #14022
Fixes #14311
Fixes #16920
Fixes #18813 - If user sets SSL_CERT_FILE.
Change-Id: Ia24fb7c1c2ffff4338b4cf214bd040326ce27bb0
Reviewed-on: https://go-review.googlesource.com/36093
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
Run-TryBot: Brad Fitzpatrick <brad...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
---
M src/crypto/x509/root_bsd.go
M src/crypto/x509/root_unix.go
A src/crypto/x509/root_unix_test.go
A src/crypto/x509/test-file.crt
A src/crypto/x509/testdata/test-dir.crt
M src/crypto/x509/x509.go
6 files changed, 227 insertions(+), 4 deletions(-)
diff --git a/src/crypto/x509/root_bsd.go b/src/crypto/x509/root_bsd.go
index 9317283..1371933 100644
--- a/src/crypto/x509/root_bsd.go
+++ b/src/crypto/x509/root_bsd.go
@@ -8,7 +8,8 @@
// Possible certificate files; stop after finding one.
var certFiles = []string{
- "/usr/local/share/certs/ca-root-nss.crt", // FreeBSD/DragonFly
+ "/usr/local/etc/ssl/cert.pem", // FreeBSD
"/etc/ssl/cert.pem", // OpenBSD
+ "/usr/local/share/certs/ca-root-nss.crt", // DragonFly
"/etc/openssl/certs/ca-certificates.crt", // NetBSD
}
diff --git a/src/crypto/x509/root_unix.go b/src/crypto/x509/root_unix.go
index 7bcb3d6..65b5a5f 100644
--- a/src/crypto/x509/root_unix.go
+++ b/src/crypto/x509/root_unix.go
@@ -16,27 +16,51 @@
var certDirectories = []string{
"/etc/ssl/certs", // SLES10/SLES11, https://golang.org/issue/12139
"/system/etc/security/cacerts", // Android
+ "/usr/local/share/certs", // FreeBSD
+ "/etc/pki/tls/certs", // Fedora/RHEL
+ "/etc/openssl/certs", // NetBSD
}
+const (
+ // certFileEnv is the environment variable which identifies where to locate
+ // the SSL certificate file. If set this overrides the system default.
+ certFileEnv = "SSL_CERT_FILE"
+
+ // certDirEnv is the environment variable which identifies which directory
+ // to check for SSL certificate files. If set this overrides the system default.
+ certDirEnv = "SSL_CERT_DIR"
+)
+
func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
return nil, nil
}
func loadSystemRoots() (*CertPool, error) {
roots := NewCertPool()
+
+ files := certFiles
+ if f := os.Getenv(certFileEnv); f != "" {
+ files = []string{f}
+ }
+
var firstErr error
- for _, file := range certFiles {
+ for _, file := range files {
data, err := ioutil.ReadFile(file)
if err == nil {
roots.AppendCertsFromPEM(data)
- return roots, nil
+ break
}
if firstErr == nil && !os.IsNotExist(err) {
firstErr = err
}
}
- for _, directory := range certDirectories {
+ dirs := certDirectories
+ if d := os.Getenv(certDirEnv); d != "" {
+ dirs = []string{d}
+ }
+
+ for _, directory := range dirs {
fis, err := ioutil.ReadDir(directory)
if err != nil {
if firstErr == nil && !os.IsNotExist(err) {
@@ -56,5 +80,9 @@
}
}
+ if len(roots.certs) > 0 {
+ return roots, nil
+ }
+
return nil, firstErr
}
diff --git a/src/crypto/x509/root_unix_test.go b/src/crypto/x509/root_unix_test.go
new file mode 100644
index 0000000..b6659d9
--- /dev/null
+++ b/src/crypto/x509/root_unix_test.go
@@ -0,0 +1,127 @@
+ name: "override-defaults",
+ name: "empty-fall-through",
+ fileEnv: "",
+ dirEnv: "",
+ files: []string{testFile},
+ dirs: []string{testDir},
+ cns: []string{testFileCN, testDirCN},
+ },
+ }
+
+ // Save old settings so we can restore before the test ends.
+ origCertFiles, origCertDirectories := certFiles, certDirectories
+ origFile, origDir := os.Getenv(certFileEnv), os.Getenv(certDirEnv)
+ defer func() {
+ certFiles = origCertFiles
+ certDirectories = origCertDirectories
+ os.Setenv(certFileEnv, origFile)
+ os.Setenv(certDirEnv, origDir)
+ }()
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ if err := os.Setenv(certFileEnv, tc.fileEnv); err != nil {
+ t.Fatalf("setenv %q failed: %v", certFileEnv, err)
+ }
+ if err := os.Setenv(certDirEnv, tc.dirEnv); err != nil {
+ t.Fatalf("setenv %q failed: %v", certDirEnv, err)diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 6979c68..3065fe2 100644
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -3,6 +3,10 @@
// license that can be found in the LICENSE file.
// Package x509 parses X.509-encoded keys and certificates.
+//
+// On UNIX systems the environment variables SSL_CERT_FILE and SSL_CERT_DIR
+// can be used to override the system default locations for the SSL certificate
+// file and SSL certificate files directory, respectively.
package x509
import (
To view, visit change 36093. To unsubscribe, visit settings.
Dmitri Shuralyov (Gerrit)
Dmitri Shuralyov posted comments on this change.
Patch set 8:
I ran into a potential index out of range panic in the TestEnvVars test.
I'll send a CL to fix it. I can also fix the other inconsistency if someone familiar tells me which direction is correct.
(2 comments)
File src/crypto/x509/root_unix_test.go:
Patch Set #8, Line 114:
if i > len(r.certs) {Off by one error here, it should be i >= len(r.certs). Otherwise, this test can panic due to index out of range on line 116.
Patch Set #8, Line 123:
t.Errorf("expected %v certs got %v", len(tc.cns), len(r.certs))If condition checks for "greater than" but error text message implies inequality. I want to fix the inconsistency with either:
1. update error string to say "expected no more than %v certs, got %v"
2. update if condition to assert for equalityWhich direction is correct?
To view, visit change 36093. To unsubscribe, visit settings.
Dmitri Shuralyov (Gerrit)
Sent CL https://golang.org/cl/46715 to follow up.
(1 comment)
File src/crypto/x509/root_unix_test.go:
Patch Set #8, Line 123:
t.Errorf("expected %v certs got %v", len(tc.cns), len(r.certs))
If condition checks for "greater than" but error text message implies inequ
Ok, it took a while, but I can now see that this code does try to ensure that len(r.certs) == len(tc.cns), albeit indirectly. The for loop above errors out if len(r.certs) < len(tc.cns), and this if statement only needs to check if len(r.certs) > len(tc.cns) to detect inequality. It could also check if len(r.certs) != len(tc.cns) without change of behavior.
So the current error string is correct. But I think the code can be made simpler and more readable. Will send a CL.
To view, visit change 36093. To unsubscribe, visit settings.
Steven Hartland (Gerrit)
Steven Hartland posted comments on this change.
Patch set 8:
I agree with both counts, if you don't get chance to submit a fix lmk and I'll get it done.
(2 comments)
File src/crypto/x509/root_unix_test.go:
Patch Set #8, Line 114:
if i > len(r.certs) {
Off by one error here, it should be i >= len(r.certs). Otherwise, this test
Ack
Patch Set #8, Line 123:
t.Errorf("expected %v certs got %v", len(tc.cns), len(r.certs))
Ok, it took a while, but I can now see that this code does try to ensure th
Ack
To view, visit change 36093. To unsubscribe, visit settings.
Dmitri Shuralyov (Gerrit)
Dmitri Shuralyov posted comments on this change.
Patch set 8:
Patch Set 8:
(2 comments)
I agree with both counts, if you don't get chance to submit a fix lmk and I'll get it done.
Steven, I already sent and submitted https://go-review.googlesource.com/c/46715/. Thanks.