Skip to content

Commit 593f13d

Browse files
committed
Deprecate tlsconfig.SystemCertPool
The wrapper originally existed for historical compatibility reasons that no longer apply: - 55aadc3 removed the pre-Go-1.7 fallback implementation, leaving the wrapper mainly as a shim around x509.SystemCertPool. - 3723764 kept a Windows-specific exception because x509.SystemCertPool on Windows was not yet reliable at the time. - f652133 and 21876c5 preserved that behavior while the package continued to support older Go versions and older Windows handling. With current Go versions, x509.SystemCertPool is the correct API on all supported platforms, including Windows, and the old special-case is no longer needed. Keeping a package-local wrapper only adds indirection and retains stale historical behavior and comments. This change keeps the existing behavior of using the system trust store as the base when building non-exclusive root pools; it only deprecates the local wrapper in favor of the standard library entrypoint. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent dad033c commit 593f13d

3 files changed

Lines changed: 12 additions & 16 deletions

File tree

tlsconfig/certpool.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,12 @@
11
package tlsconfig
22

3-
import (
4-
"crypto/x509"
5-
"runtime"
6-
)
3+
import "crypto/x509"
74

8-
// SystemCertPool returns a copy of the system cert pool,
9-
// returns an error if failed to load or empty pool on windows.
5+
// SystemCertPool returns a copy of the system cert pool.
6+
//
7+
// Deprecated: use [x509.SystemCertPool] instead.
8+
//
9+
//go:fix inline
1010
func SystemCertPool() (*x509.CertPool, error) {
11-
certpool, err := x509.SystemCertPool()
12-
if err != nil && runtime.GOOS == "windows" {
13-
return x509.NewCertPool(), nil
14-
}
15-
return certpool, err
11+
return x509.SystemCertPool()
1612
}

tlsconfig/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func certPool(caFile string, exclusivePool bool) (*x509.CertPool, error) {
8484
if exclusivePool {
8585
pool = x509.NewCertPool()
8686
} else {
87-
pool, err = SystemCertPool()
87+
pool, err = x509.SystemCertPool()
8888
if err != nil {
8989
return nil, fmt.Errorf("failed to read system certificates: %v", err)
9090
}

tlsconfig/config_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,9 @@ func TestConfigServerTLSClientCASet(t *testing.T) {
183183
if tlsConfig.ClientAuth != tls.VerifyClientCertIfGiven {
184184
t.Fatal("ClientAuth was not set to what was in the options")
185185
}
186-
basePool, err := SystemCertPool()
186+
basePool, err := x509.SystemCertPool()
187187
if err != nil {
188-
basePool = x509.NewCertPool()
188+
t.Fatal("Failed to get SystemCertPool", err)
189189
}
190190
// because we are not enabling `ExclusiveRootPools`, any root pool will also contain the system roots
191191
if tlsConfig.ClientCAs == nil || len(tlsConfig.ClientCAs.Subjects()) != len(basePool.Subjects())+2 { //nolint:staticcheck // Ignore SA1019: tlsConfig.ClientCAs.Subjects has been deprecated since Go 1.18: if s was returned by SystemCertPool, Subjects will not include the system roots.
@@ -441,9 +441,9 @@ func TestConfigClientTLSRootCAFileWithOneCert(t *testing.T) {
441441
if err != nil || tlsConfig == nil {
442442
t.Fatal("Unable to configure client TLS", err)
443443
}
444-
basePool, err := SystemCertPool()
444+
basePool, err := x509.SystemCertPool()
445445
if err != nil {
446-
basePool = x509.NewCertPool()
446+
t.Fatal("Failed to get SystemCertPool", err)
447447
}
448448
// because we are not enabling `ExclusiveRootPools`, any root pool will also contain the system roots
449449
if tlsConfig.RootCAs == nil || len(tlsConfig.RootCAs.Subjects()) != len(basePool.Subjects())+2 { //nolint:staticcheck // Ignore SA1019: tlsConfig.ClientCAs.Subjects has been deprecated since Go 1.18: if s was returned by SystemCertPool, Subjects will not include the system roots.

0 commit comments

Comments
 (0)