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() {