Skip to content

Commit a324a5a

Browse files
mkrautzagl
authored andcommitted
crypto/x509: new home for root fetchers; build chains using Windows API
This moves the various CA root fetchers from crypto/tls into crypto/x509. The move was brought about by issue 2997. Windows doesn't ship with all its root certificates, but will instead download them as-needed when using CryptoAPI for certificate verification. This CL changes crypto/x509 to verify a certificate using the system root CAs when VerifyOptions.RootCAs == nil. On Windows, this verification is now implemented using Windows's CryptoAPI. All other root fetchers are unchanged, and still use Go's own verification code. The CL also fixes the hostname matching logic in crypto/tls/tls.go, in order to be able to test whether hostname mismatches are honored by the Windows verification code. The move to crypto/x509 also allows other packages to use the OS-provided root certificates, instead of hiding them inside the crypto/tls package. Fixes #2997. R=agl, golang-dev, alex.brainman, rsc, mikkel CC=golang-dev https://golang.org/cl/5700087
1 parent 807aadc commit a324a5a

16 files changed

+875
-349
lines changed

src/pkg/crypto/tls/common.go

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,6 @@ func (c *Config) time() time.Time {
198198
return t()
199199
}
200200

201-
func (c *Config) rootCAs() *x509.CertPool {
202-
s := c.RootCAs
203-
if s == nil {
204-
s = defaultRoots()
205-
}
206-
return s
207-
}
208-
209201
func (c *Config) cipherSuites() []uint16 {
210202
s := c.CipherSuites
211203
if s == nil {
@@ -311,28 +303,16 @@ func defaultConfig() *Config {
311303
return &emptyConfig
312304
}
313305

314-
var once sync.Once
315-
316-
func defaultRoots() *x509.CertPool {
317-
once.Do(initDefaults)
318-
return varDefaultRoots
319-
}
306+
var (
307+
once sync.Once
308+
varDefaultCipherSuites []uint16
309+
)
320310

321311
func defaultCipherSuites() []uint16 {
322-
once.Do(initDefaults)
312+
once.Do(initDefaultCipherSuites)
323313
return varDefaultCipherSuites
324314
}
325315

326-
func initDefaults() {
327-
initDefaultRoots()
328-
initDefaultCipherSuites()
329-
}
330-
331-
var (
332-
varDefaultRoots *x509.CertPool
333-
varDefaultCipherSuites []uint16
334-
)
335-
336316
func initDefaultCipherSuites() {
337317
varDefaultCipherSuites = make([]uint16, len(cipherSuites))
338318
for i, suite := range cipherSuites {

src/pkg/crypto/tls/handshake_client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (c *Conn) clientHandshake() error {
102102

103103
if !c.config.InsecureSkipVerify {
104104
opts := x509.VerifyOptions{
105-
Roots: c.config.rootCAs(),
105+
Roots: c.config.RootCAs,
106106
CurrentTime: c.config.time(),
107107
DNSName: c.config.ServerName,
108108
Intermediates: x509.NewCertPool(),

src/pkg/crypto/tls/root_test.go

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,25 @@
55
package tls
66

77
import (
8+
"crypto/x509"
9+
"runtime"
810
"testing"
911
)
1012

1113
var tlsServers = []string{
12-
"google.com:443",
13-
"github.com:443",
14-
"twitter.com:443",
14+
"google.com",
15+
"github.com",
16+
"twitter.com",
1517
}
1618

1719
func TestOSCertBundles(t *testing.T) {
18-
defaultRoots()
19-
2020
if testing.Short() {
2121
t.Logf("skipping certificate tests in short mode")
2222
return
2323
}
2424

2525
for _, addr := range tlsServers {
26-
conn, err := Dial("tcp", addr, nil)
26+
conn, err := Dial("tcp", addr+":443", &Config{ServerName: addr})
2727
if err != nil {
2828
t.Errorf("unable to verify %v: %v", addr, err)
2929
continue
@@ -34,3 +34,28 @@ func TestOSCertBundles(t *testing.T) {
3434
}
3535
}
3636
}
37+
38+
func TestCertHostnameVerifyWindows(t *testing.T) {
39+
if runtime.GOOS != "windows" {
40+
return
41+
}
42+
43+
if testing.Short() {
44+
t.Logf("skipping certificate tests in short mode")
45+
return
46+
}
47+
48+
for _, addr := range tlsServers {
49+
cfg := &Config{ServerName: "example.com"}
50+
conn, err := Dial("tcp", addr+":443", cfg)
51+
if err == nil {
52+
conn.Close()
53+
t.Errorf("should fail to verify for example.com: %v", addr, err)
54+
continue
55+
}
56+
_, ok := err.(x509.HostnameError)
57+
if !ok {
58+
t.Errorf("error type mismatch, got: %v", err)
59+
}
60+
}
61+
}

src/pkg/crypto/tls/root_windows.go

Lines changed: 0 additions & 47 deletions
This file was deleted.

src/pkg/crypto/tls/tls.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@ func Dial(network, addr string, config *Config) (*Conn, error) {
9797
if config == nil {
9898
config = defaultConfig()
9999
}
100-
if config.ServerName != "" {
100+
// If no ServerName is set, infer the ServerName
101+
// from the hostname we're connecting to.
102+
if config.ServerName == "" {
101103
// Make a copy to avoid polluting argument or default.
102104
c := *config
103105
c.ServerName = hostname

src/pkg/crypto/x509/root.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2012 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package x509
6+
7+
import "sync"
8+
9+
var (
10+
once sync.Once
11+
systemRoots *CertPool
12+
)
13+
14+
func systemRootsPool() *CertPool {
15+
once.Do(initSystemRoots)
16+
return systemRoots
17+
}

src/pkg/crypto/tls/root_darwin.go renamed to src/pkg/crypto/x509/root_darwin.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package tls
5+
package x509
66

77
/*
88
#cgo CFLAGS: -mmacosx-version-min=10.6 -D__MAC_OS_X_VERSION_MAX_ALLOWED=1060
@@ -59,13 +59,14 @@ int FetchPEMRoots(CFDataRef *pemRoots) {
5959
}
6060
*/
6161
import "C"
62-
import (
63-
"crypto/x509"
64-
"unsafe"
65-
)
62+
import "unsafe"
6663

67-
func initDefaultRoots() {
68-
roots := x509.NewCertPool()
64+
func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
65+
return nil, nil
66+
}
67+
68+
func initSystemRoots() {
69+
roots := NewCertPool()
6970

7071
var data C.CFDataRef = nil
7172
err := C.FetchPEMRoots(&data)
@@ -75,5 +76,5 @@ func initDefaultRoots() {
7576
roots.AppendCertsFromPEM(buf)
7677
}
7778

78-
varDefaultRoots = roots
79+
systemRoots = roots
7980
}

src/pkg/crypto/tls/root_stub.go renamed to src/pkg/crypto/x509/root_stub.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44

55
// +build plan9 darwin,!cgo
66

7-
package tls
7+
package x509
88

9-
func initDefaultRoots() {
9+
func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
10+
return nil, nil
11+
}
12+
13+
func initSystemRoots() {
14+
systemRoots = NewCertPool()
1015
}

src/pkg/crypto/tls/root_unix.go renamed to src/pkg/crypto/x509/root_unix.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@
44

55
// +build freebsd linux openbsd netbsd
66

7-
package tls
7+
package x509
88

9-
import (
10-
"crypto/x509"
11-
"io/ioutil"
12-
)
9+
import "io/ioutil"
1310

1411
// Possible certificate files; stop after finding one.
1512
var certFiles = []string{
@@ -20,14 +17,19 @@ var certFiles = []string{
2017
"/usr/local/share/certs/ca-root-nss.crt", // FreeBSD
2118
}
2219

23-
func initDefaultRoots() {
24-
roots := x509.NewCertPool()
20+
func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
21+
return nil, nil
22+
}
23+
24+
func initSystemRoots() {
25+
roots := NewCertPool()
2526
for _, file := range certFiles {
2627
data, err := ioutil.ReadFile(file)
2728
if err == nil {
2829
roots.AppendCertsFromPEM(data)
2930
break
3031
}
3132
}
32-
varDefaultRoots = roots
33+
34+
systemRoots = roots
3335
}

0 commit comments

Comments
 (0)