From 7441443134651ba7bf77f88a8e0e4c9662f672c7 Mon Sep 17 00:00:00 2001 From: Alexey Sviridov Date: Sun, 18 Oct 2026 08:08:05 +0200 Subject: [PATCH] feat(client): verify server certificates against the system root store (#5546) transport.tls.trustedCaFile is installed as RootCAs, and omitting it sets InsecureSkipVerify, so system-root verification cannot be selected directly. Add an opt-in transport.tls.useSystemRoots that enables certificate verification with RootCAs nil when no trusted CA file is set, allowing Go to use system roots and native verification on macOS and Windows. Defaults are unchanged. --- README.md | 2 + client/connector.go | 10 ++- conf/frpc_full_example.toml | 4 + pkg/config/v1/client.go | 3 + pkg/config/v1/validation/client.go | 4 + pkg/transport/tls.go | 16 +++- pkg/transport/tls_test.go | 139 +++++++++++++++++++++++++++++ test/e2e/v1/basic/client_server.go | 4 + 8 files changed, 176 insertions(+), 6 deletions(-) create mode 100644 pkg/transport/tls_test.go diff --git a/README.md b/README.md index 042a5a20..3510fa86 100644 --- a/README.md +++ b/README.md @@ -712,6 +712,8 @@ transport.tls.keyFile = "certificate.key" transport.tls.trustedCaFile = "ca.crt" ``` +By default, when neither `transport.tls.trustedCaFile` nor `transport.tls.useSystemRoots` is set, `frpc` does not verify the server certificate. Set `transport.tls.useSystemRoots = true` to verify it against the host's root store instead. With `wss`, verification applies to the TLS endpoint, which may be a reverse proxy. `useSystemRoots` is ignored when `transport.tls.trustedCaFile` is set. + **`frps` TLS settings:** ```toml diff --git a/client/connector.go b/client/connector.go index 41332378..7ffc6860 100644 --- a/client/connector.go +++ b/client/connector.go @@ -109,11 +109,12 @@ func (c *defaultConnectorImpl) Open() error { sn = c.cfg.ServerAddr } if lo.FromPtr(c.cfg.Transport.TLS.Enable) { - tlsConfig, err = transport.NewClientTLSConfig( + tlsConfig, err = transport.NewClientTLSConfigWithSystemRoots( c.cfg.Transport.TLS.CertFile, c.cfg.Transport.TLS.KeyFile, c.cfg.Transport.TLS.TrustedCaFile, - sn) + sn, + c.cfg.Transport.TLS.UseSystemRoots) } else { tlsConfig, err = transport.NewClientTLSConfig("", "", "", sn) } @@ -194,11 +195,12 @@ func (c *defaultConnectorImpl) realConnect() (net.Conn, error) { sn = c.cfg.ServerAddr } - tlsConfig, err = transport.NewClientTLSConfig( + tlsConfig, err = transport.NewClientTLSConfigWithSystemRoots( c.cfg.Transport.TLS.CertFile, c.cfg.Transport.TLS.KeyFile, c.cfg.Transport.TLS.TrustedCaFile, - sn) + sn, + c.cfg.Transport.TLS.UseSystemRoots) if err != nil { xl.Warnf("fail to build tls configuration, err: %v", err) return nil, err diff --git a/conf/frpc_full_example.toml b/conf/frpc_full_example.toml index b141b0ed..f779cc9a 100644 --- a/conf/frpc_full_example.toml +++ b/conf/frpc_full_example.toml @@ -131,6 +131,10 @@ transport.tls.enable = true # transport.tls.trustedCaFile = "ca.crt" # transport.tls.serverName = "example.com" +# If useSystemRoots is true and trustedCaFile is not set, frpc verifies the server certificate +# against the host's root store. +# transport.tls.useSystemRoots = false + # If the disableCustomTLSFirstByte is set to false, frpc will establish a connection with frps using the # first custom byte when tls is enabled. # Since v0.50.0, the default value has been changed to true, and the first custom byte is disabled by default. diff --git a/pkg/config/v1/client.go b/pkg/config/v1/client.go index 6736030a..e30a65c6 100644 --- a/pkg/config/v1/client.go +++ b/pkg/config/v1/client.go @@ -175,6 +175,9 @@ type TLSClientConfig struct { // first custom byte when tls is enabled. // Since v0.50.0, the default value has been changed to true, and the first custom byte is disabled by default. DisableCustomTLSFirstByte *bool `json:"disableCustomTLSFirstByte,omitempty"` + // UseSystemRoots enables server certificate verification using system roots. + // TrustedCaFile takes precedence. If neither is configured, verification is disabled. + UseSystemRoots bool `json:"useSystemRoots,omitempty"` TLSConfig } diff --git a/pkg/config/v1/validation/client.go b/pkg/config/v1/validation/client.go index 28409c4b..d51e4384 100644 --- a/pkg/config/v1/validation/client.go +++ b/pkg/config/v1/validation/client.go @@ -163,6 +163,10 @@ func validateTransportConfig(c *v1.ClientTransportConfig) (Warning, error) { warnings = AppendError(warnings, checkTLSConfig("transport.tls.certFile", c.TLS.CertFile)) warnings = AppendError(warnings, checkTLSConfig("transport.tls.keyFile", c.TLS.KeyFile)) warnings = AppendError(warnings, checkTLSConfig("transport.tls.trustedCaFile", c.TLS.TrustedCaFile)) + // wss keeps TLS on regardless of transport.tls.enable. + if c.TLS.UseSystemRoots && c.Protocol != "wss" { + warnings = AppendError(warnings, fmt.Errorf("transport.tls.useSystemRoots is ignored when transport.tls.enable is false")) + } } if !slices.Contains(SupportedTransportProtocols, c.Protocol) { diff --git a/pkg/transport/tls.go b/pkg/transport/tls.go index 19ebca73..4ffbde93 100644 --- a/pkg/transport/tls.go +++ b/pkg/transport/tls.go @@ -126,6 +126,13 @@ func NewServerTLSConfig(certPath, keyPath, caPath string) (*tls.Config, error) { } func NewClientTLSConfig(certPath, keyPath, caPath, serverName string) (*tls.Config, error) { + return NewClientTLSConfigWithSystemRoots(certPath, keyPath, caPath, serverName, false) +} + +// NewClientTLSConfigWithSystemRoots behaves like NewClientTLSConfig, except that when +// caPath is empty and useSystemRoots is true, the server certificate is verified against +// the host's root store. +func NewClientTLSConfigWithSystemRoots(certPath, keyPath, caPath, serverName string, useSystemRoots bool) (*tls.Config, error) { base := &tls.Config{} if certPath != "" && keyPath != "" { @@ -139,7 +146,8 @@ func NewClientTLSConfig(certPath, keyPath, caPath, serverName string) (*tls.Conf base.ServerName = serverName - if caPath != "" { + switch { + case caPath != "": pool, err := newCertPool(caPath) if err != nil { return nil, err @@ -147,7 +155,11 @@ func NewClientTLSConfig(certPath, keyPath, caPath, serverName string) (*tls.Conf base.RootCAs = pool base.InsecureSkipVerify = false - } else { + case useSystemRoots: + // A nil RootCAs makes crypto/tls use the host root store, through the + // platform verifier on macOS and Windows. + base.InsecureSkipVerify = false + default: base.InsecureSkipVerify = true } diff --git a/pkg/transport/tls_test.go b/pkg/transport/tls_test.go new file mode 100644 index 00000000..d1a32486 --- /dev/null +++ b/pkg/transport/tls_test.go @@ -0,0 +1,139 @@ +// Copyright 2026 The frp Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package transport + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "errors" + "math/big" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func newSelfSignedCert(t *testing.T) (tls.Certificate, []byte) { + t.Helper() + + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "localhost"}, + DNSNames: []string{"localhost"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + } + certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + require.NoError(t, err) + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) + + cert, err := tls.X509KeyPair(certPEM, keyPEM) + require.NoError(t, err) + return cert, certPEM +} + +func TestNewClientTLSConfigVerification(t *testing.T) { + _, caPEM := newSelfSignedCert(t) + caFile := filepath.Join(t.TempDir(), "ca.crt") + require.NoError(t, os.WriteFile(caFile, caPEM, 0o600)) + + tests := []struct { + name string + caPath string + useSystemRoots bool + insecureSkipVerify bool + rootCAsSet bool + }{ + { + name: "no trusted ca file, no system roots", + insecureSkipVerify: true, + }, + { + name: "system roots", + useSystemRoots: true, + }, + { + name: "trusted ca file", + caPath: caFile, + rootCAsSet: true, + }, + { + name: "trusted ca file takes precedence over system roots", + caPath: caFile, + useSystemRoots: true, + rootCAsSet: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + cfg, err := NewClientTLSConfigWithSystemRoots("", "", test.caPath, "example.com", test.useSystemRoots) + require.NoError(err) + require.Equal("example.com", cfg.ServerName) + require.Equal(test.insecureSkipVerify, cfg.InsecureSkipVerify) + require.Equal(test.rootCAsSet, cfg.RootCAs != nil) + }) + } +} + +func TestClientTLSConfigHandshakeWithUntrustedServer(t *testing.T) { + require := require.New(t) + + cert, _ := newSelfSignedCert(t) + ln, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{Certificates: []tls.Certificate{cert}}) + require.NoError(err) + t.Cleanup(func() { _ = ln.Close() }) + + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + _ = conn.(*tls.Conn).Handshake() + _ = conn.Close() + } + }() + + dial := func(useSystemRoots bool) error { + cfg, err := NewClientTLSConfigWithSystemRoots("", "", "", "localhost", useSystemRoots) + require.NoError(err) + conn, err := tls.Dial("tcp", ln.Addr().String(), cfg) + if err != nil { + return err + } + return conn.Close() + } + + require.NoError(dial(false)) + + err = dial(true) + require.Error(err) + var verificationErr *tls.CertificateVerificationError + require.True(errors.As(err, &verificationErr), "expected a certificate verification error, got %v", err) +} diff --git a/test/e2e/v1/basic/client_server.go b/test/e2e/v1/basic/client_server.go index 3a2fa28d..e8256cdb 100644 --- a/test/e2e/v1/basic/client_server.go +++ b/test/e2e/v1/basic/client_server.go @@ -177,6 +177,10 @@ var _ = ginkgo.Describe("[Feature: Client-Server]", func() { client: "transport.tls.enable = false", expectError: true, }) + defineClientServerTest("useSystemRoots rejects the default self-signed server certificate", f, &generalTestConfigures{ + client: "transport.tls.useSystemRoots = true", + expectError: true, + }) }) ginkgo.Describe("TLS with custom certificate", func() {