diff --git a/Release.md b/Release.md index 0d2500d6..4716e374 100644 --- a/Release.md +++ b/Release.md @@ -5,3 +5,4 @@ ## Fixes * Fixed a server panic and remote denial of service caused by a client sending a negative `pool_count`. Negative values are now rejected before work-connection pool resources are allocated. +* Fixed `frpc verify` ignoring configured `featureGates`, which caused VirtualNet configurations to be rejected even when the feature was enabled. diff --git a/client/config_manager_test.go b/client/config_manager_test.go index 07ae3297..2758b2d7 100644 --- a/client/config_manager_test.go +++ b/client/config_manager_test.go @@ -2,12 +2,16 @@ package client import ( "errors" + "os" "path/filepath" + "strings" "testing" "github.com/fatedier/frp/client/configmgmt" "github.com/fatedier/frp/pkg/config/source" v1 "github.com/fatedier/frp/pkg/config/v1" + "github.com/fatedier/frp/pkg/policy/security" + "github.com/fatedier/frp/pkg/vnet" ) func newTestRawTCPProxyConfig(name string) *v1.TCPProxyConfig { @@ -22,6 +26,256 @@ func newTestRawTCPProxyConfig(name string) *v1.TCPProxyConfig { } } +func newTestVirtualNetProxyConfig(name string) *v1.STCPProxyConfig { + return &v1.STCPProxyConfig{ + ProxyBaseConfig: v1.ProxyBaseConfig{ + Name: name, + Type: "stcp", + ProxyBackend: v1.ProxyBackend{ + Plugin: v1.TypedClientPluginOptions{ + Type: v1.PluginVirtualNet, + ClientPluginOptions: &v1.VirtualNetPluginOptions{Type: v1.PluginVirtualNet}, + }, + }, + }, + } +} + +func newTestVirtualNetVisitorConfig(name string) *v1.STCPVisitorConfig { + return &v1.STCPVisitorConfig{ + VisitorBaseConfig: v1.VisitorBaseConfig{ + Name: name, + Type: "stcp", + ServerName: "vnet-server", + SecretKey: "secret", + BindPort: -1, + Plugin: v1.TypedVisitorPluginOptions{ + Type: v1.VisitorPluginVirtualNet, + VisitorPluginOptions: &v1.VirtualNetVisitorPluginOptions{ + Type: v1.VisitorPluginVirtualNet, + DestinationIP: "100.86.0.1", + }, + }, + }, + } +} + +func TestServiceConfigManagerReloadVirtualNetRuntimeDependency(t *testing.T) { + const runtimeErr = "VirtualNet-dependent configuration requires a VirtualNet runtime enabled at startup" + + tests := []struct { + name string + startupVirtualNetAddr string + nextConfig string + wantRuntimeDependency bool + }{ + { + name: "unrelated common config", + nextConfig: `serverAddr = "0.0.0.0"`, + }, + { + name: "VirtualNet address without startup runtime", + nextConfig: `featureGates = { VirtualNet = true } +virtualNet.address = "100.86.0.4/24" +`, + wantRuntimeDependency: true, + }, + { + name: "VirtualNet proxy without startup runtime", + nextConfig: `[[proxies]] +name = "vnet-proxy" +type = "stcp" +secretKey = "secret" +[proxies.plugin] +type = "virtual_net" +`, + wantRuntimeDependency: true, + }, + { + name: "VirtualNet visitor without startup runtime", + nextConfig: `[[visitors]] +name = "vnet-visitor" +type = "stcp" +serverName = "vnet-server" +secretKey = "secret" +bindPort = -1 +[visitors.plugin] +type = "virtual_net" +destinationIP = "100.86.0.1" +`, + wantRuntimeDependency: true, + }, + { + name: "existing VirtualNet startup runtime", + startupVirtualNetAddr: "100.86.0.4/24", + nextConfig: `featureGates = { VirtualNet = true } +virtualNet.address = "100.86.0.5/24" +`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + current := &v1.ClientCommonConfig{} + if tc.startupVirtualNetAddr != "" { + current.FeatureGates = map[string]bool{"VirtualNet": true} + current.VirtualNet.Address = tc.startupVirtualNetAddr + } + if err := current.Complete(); err != nil { + t.Fatalf("complete current config: %v", err) + } + + configFile := filepath.Join(t.TempDir(), "frpc.toml") + if err := os.WriteFile(configFile, []byte(tc.nextConfig), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + + configSource := source.NewConfigSource() + aggregator := source.NewAggregator(configSource) + svr := &Service{ + common: current, + reloadCommon: current, + configFilePath: configFile, + unsafeFeatures: security.NewUnsafeFeatures(nil), + aggregator: aggregator, + configSource: configSource, + } + if tc.startupVirtualNetAddr != "" { + svr.vnetController = vnet.NewController(current.VirtualNet) + } + + err := (&serviceConfigManager{svr: svr}).ReloadFromFile(true) + if tc.wantRuntimeDependency { + if !errors.Is(err, configmgmt.ErrApplyConfig) || !strings.Contains(err.Error(), runtimeErr) { + t.Fatalf("expected VirtualNet runtime dependency error, got: %v", err) + } + return + } + + if err != nil { + t.Fatalf("reload config: %v", err) + } + if svr.common != current { + t.Fatal("reload should not replace startup common config") + } + if tc.startupVirtualNetAddr == "" && svr.vnetController != nil { + t.Fatal("reload should not enable startup-only VirtualNet runtime state") + } + }) + } +} + +func TestServiceConfigManagerReloadVirtualNetRuntimeDependencyUsesMergedSources(t *testing.T) { + const runtimeErr = "VirtualNet-dependent configuration requires a VirtualNet runtime enabled at startup" + + tests := []struct { + name string + nextConfig string + storeProxy v1.ProxyConfigurer + storeVisitor v1.VisitorConfigurer + wantRuntimeDependency bool + wantProxyPlugin string + }{ + { + name: "Store VirtualNet proxy is rejected", + nextConfig: `serverAddr = "0.0.0.0"`, + storeProxy: newTestVirtualNetProxyConfig("store-vnet"), + wantRuntimeDependency: true, + }, + { + name: "Store VirtualNet visitor is rejected", + nextConfig: `serverAddr = "0.0.0.0"`, + storeVisitor: newTestVirtualNetVisitorConfig("store-vnet"), + wantRuntimeDependency: true, + }, + { + name: "Store VirtualNet proxy overrides file proxy", + nextConfig: `[[proxies]] +name = "shared" +type = "tcp" +localPort = 10080 +remotePort = 10081 +`, + storeProxy: newTestVirtualNetProxyConfig("shared"), + wantRuntimeDependency: true, + }, + { + name: "Store non-VirtualNet proxy overrides file VirtualNet proxy", + nextConfig: `[[proxies]] +name = "shared" +type = "stcp" +secretKey = "secret" +[proxies.plugin] +type = "virtual_net" +`, + storeProxy: newTestRawTCPProxyConfig("shared"), + wantProxyPlugin: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + current := &v1.ClientCommonConfig{} + if err := current.Complete(); err != nil { + t.Fatalf("complete current config: %v", err) + } + + configFile := filepath.Join(t.TempDir(), "frpc.toml") + if err := os.WriteFile(configFile, []byte(tc.nextConfig), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + + storeSource, err := source.NewStoreSource(source.StoreSourceConfig{ + Path: filepath.Join(t.TempDir(), "store.json"), + }) + if err != nil { + t.Fatalf("new store source: %v", err) + } + if tc.storeProxy != nil { + if err := storeSource.AddProxy(tc.storeProxy); err != nil { + t.Fatalf("add store proxy: %v", err) + } + } + if tc.storeVisitor != nil { + if err := storeSource.AddVisitor(tc.storeVisitor); err != nil { + t.Fatalf("add store visitor: %v", err) + } + } + + configSource := source.NewConfigSource() + aggregator := source.NewAggregator(configSource) + aggregator.SetStoreSource(storeSource) + svr := &Service{ + common: current, + reloadCommon: current, + configFilePath: configFile, + unsafeFeatures: security.NewUnsafeFeatures(nil), + aggregator: aggregator, + configSource: configSource, + storeSource: storeSource, + } + + err = (&serviceConfigManager{svr: svr}).ReloadFromFile(true) + if tc.wantRuntimeDependency { + if !errors.Is(err, configmgmt.ErrApplyConfig) || !strings.Contains(err.Error(), runtimeErr) { + t.Fatalf("expected VirtualNet runtime dependency error, got: %v", err) + } + return + } + if err != nil { + t.Fatalf("reload config: %v", err) + } + + if len(svr.proxyCfgs) != 1 { + t.Fatalf("expected one applied proxy, got %d", len(svr.proxyCfgs)) + } + if got := svr.proxyCfgs[0].GetBaseConfig().Plugin.Type; got != tc.wantProxyPlugin { + t.Fatalf("unexpected applied proxy plugin: %q", got) + } + }) + } +} + func TestServiceConfigManagerCreateStoreProxyConflict(t *testing.T) { storeSource, err := source.NewStoreSource(source.StoreSourceConfig{ Path: filepath.Join(t.TempDir(), "store.json"), diff --git a/client/service.go b/client/service.go index 40428719..7964050f 100644 --- a/client/service.go +++ b/client/service.go @@ -33,6 +33,7 @@ import ( "github.com/fatedier/frp/pkg/config" "github.com/fatedier/frp/pkg/config/source" v1 "github.com/fatedier/frp/pkg/config/v1" + "github.com/fatedier/frp/pkg/config/v1/validation" "github.com/fatedier/frp/pkg/msg" "github.com/fatedier/frp/pkg/policy/security" httppkg "github.com/fatedier/frp/pkg/util/http" @@ -510,6 +511,13 @@ func (svr *Service) reloadConfigFromSourcesLocked() error { proxies, visitors = config.FilterClientConfigurers(reloadCommon, proxies, visitors) proxies = config.CompleteProxyConfigurers(proxies) visitors = config.CompleteVisitorConfigurers(visitors) + requirements := validation.GetClientConfigRequirements(reloadCommon, proxies, visitors) + if svr.vnetController == nil && requirements.VirtualNet { + return errors.New( + "VirtualNet-dependent configuration requires a VirtualNet runtime enabled at startup; " + + "restart frpc after configuring featureGates.VirtualNet and virtualNet.address", + ) + } // Atomically replace the entire configuration if err := svr.UpdateAllConfigurer(proxies, visitors); err != nil { diff --git a/cmd/frpc/sub/root.go b/cmd/frpc/sub/root.go index 5d83cabf..8a0f1842 100644 --- a/cmd/frpc/sub/root.go +++ b/cmd/frpc/sub/root.go @@ -33,7 +33,6 @@ import ( "github.com/fatedier/frp/pkg/config/source" v1 "github.com/fatedier/frp/pkg/config/v1" "github.com/fatedier/frp/pkg/config/v1/validation" - "github.com/fatedier/frp/pkg/policy/featuregate" "github.com/fatedier/frp/pkg/policy/security" "github.com/fatedier/frp/pkg/util/log" "github.com/fatedier/frp/pkg/util/version" @@ -131,12 +130,6 @@ func runClient(cfgFilePath string, unsafeFeatures *security.UnsafeFeatures) erro "please use yaml/json/toml format instead!\n") } - if len(result.Common.FeatureGates) > 0 { - if err := featuregate.SetFromMap(result.Common.FeatureGates); err != nil { - return err - } - } - return runClientWithAggregator(result, unsafeFeatures, cfgFilePath) } diff --git a/cmd/frpc/sub/verify.go b/cmd/frpc/sub/verify.go index 830f7bf1..d226c473 100644 --- a/cmd/frpc/sub/verify.go +++ b/cmd/frpc/sub/verify.go @@ -29,6 +29,18 @@ func init() { rootCmd.AddCommand(verifyCmd) } +func verifyClientConfig( + configFile string, + strict bool, + unsafeFeatures *security.UnsafeFeatures, +) (validation.Warning, error) { + cliCfg, proxyCfgs, visitorCfgs, _, err := config.LoadClientConfig(configFile, strict) + if err != nil { + return nil, err + } + return validation.ValidateAllClientConfig(cliCfg, proxyCfgs, visitorCfgs, unsafeFeatures) +} + var verifyCmd = &cobra.Command{ Use: "verify", Short: "Verify that the configures is valid", @@ -38,13 +50,8 @@ var verifyCmd = &cobra.Command{ return nil } - cliCfg, proxyCfgs, visitorCfgs, _, err := config.LoadClientConfig(cfgFile, strictConfigMode) - if err != nil { - fmt.Println(err) - os.Exit(1) - } unsafeFeatures := security.NewUnsafeFeatures(allowUnsafe) - warning, err := validation.ValidateAllClientConfig(cliCfg, proxyCfgs, visitorCfgs, unsafeFeatures) + warning, err := verifyClientConfig(cfgFile, strictConfigMode, unsafeFeatures) if warning != nil { fmt.Printf("WARNING: %v\n", warning) } diff --git a/cmd/frpc/sub/verify_test.go b/cmd/frpc/sub/verify_test.go new file mode 100644 index 00000000..b93db23b --- /dev/null +++ b/cmd/frpc/sub/verify_test.go @@ -0,0 +1,67 @@ +// 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 sub + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/fatedier/frp/pkg/policy/security" +) + +func TestVerifyClientConfigFeatureGates(t *testing.T) { + tests := []struct { + name string + content string + wantErr string + }{ + { + name: "VirtualNet enabled", + content: `featureGates = { VirtualNet = true } +virtualNet.address = "100.86.0.4/24" +`, + }, + { + name: "VirtualNet disabled", + content: `featureGates = { VirtualNet = false } +virtualNet.address = "100.86.0.4/24" +`, + wantErr: "VirtualNet feature is not enabled", + }, + { + name: "unknown feature gate", + content: `featureGates = { UnknownFeature = true }`, + wantErr: "unrecognized feature gate: UnknownFeature", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + configFile := filepath.Join(t.TempDir(), "frpc.toml") + require.NoError(t, os.WriteFile(configFile, []byte(tc.content), 0o600)) + + warning, err := verifyClientConfig(configFile, true, security.NewUnsafeFeatures(nil)) + require.NoError(t, warning) + if tc.wantErr == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, tc.wantErr) + }) + } +} diff --git a/pkg/config/v1/validation/client.go b/pkg/config/v1/validation/client.go index 77004317..28409c4b 100644 --- a/pkg/config/v1/validation/client.go +++ b/pkg/config/v1/validation/client.go @@ -51,14 +51,51 @@ func (v *ConfigValidator) ValidateClientCommonConfig(c *v1.ClientCommonConfig) ( } func validateFeatureGates(c *v1.ClientCommonConfig) (Warning, error) { + gates := featuregate.NewFeatureGate() + if err := gates.SetFromMap(c.FeatureGates); err != nil { + return nil, err + } + if c.VirtualNet.Address != "" { - if !featuregate.Enabled(featuregate.VirtualNet) { + if !gates.Enabled(featuregate.VirtualNet) { return nil, fmt.Errorf("VirtualNet feature is not enabled; enable it by setting the appropriate feature gate flag") } } return nil, nil } +// ClientConfigRequirements describes runtime capabilities needed by a client configuration. +type ClientConfigRequirements struct { + VirtualNet bool +} + +// GetClientConfigRequirements returns the runtime capabilities needed by a client configuration. +func GetClientConfigRequirements( + common *v1.ClientCommonConfig, + proxyCfgs []v1.ProxyConfigurer, + visitorCfgs []v1.VisitorConfigurer, +) ClientConfigRequirements { + requirements := ClientConfigRequirements{} + if common != nil && common.VirtualNet.Address != "" { + requirements.VirtualNet = true + } + for _, cfg := range proxyCfgs { + if cfg.GetBaseConfig().Plugin.Type == v1.PluginVirtualNet { + requirements.VirtualNet = true + break + } + } + if !requirements.VirtualNet { + for _, cfg := range visitorCfgs { + if cfg.GetBaseConfig().Plugin.Type == v1.VisitorPluginVirtualNet { + requirements.VirtualNet = true + break + } + } + } + return requirements +} + func (v *ConfigValidator) validateAuthConfig(c *v1.AuthClientConfig) (Warning, error) { var errs error if !slices.Contains(SupportedAuthMethods, c.Method) { diff --git a/pkg/config/v1/validation/client_test.go b/pkg/config/v1/validation/client_test.go new file mode 100644 index 00000000..1370e890 --- /dev/null +++ b/pkg/config/v1/validation/client_test.go @@ -0,0 +1,140 @@ +// 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 validation + +import ( + "testing" + + "github.com/stretchr/testify/require" + + v1 "github.com/fatedier/frp/pkg/config/v1" + "github.com/fatedier/frp/pkg/policy/featuregate" + "github.com/fatedier/frp/pkg/policy/security" +) + +func validateClientFeatureGates(t *testing.T, gates map[string]bool, virtualNetAddress string) error { + t.Helper() + + cfg := &v1.ClientCommonConfig{ + FeatureGates: gates, + VirtualNet: v1.VirtualNetConfig{ + Address: virtualNetAddress, + }, + } + require.NoError(t, cfg.Complete()) + + _, err := NewConfigValidator(security.NewUnsafeFeatures(nil)).ValidateClientCommonConfig(cfg) + return err +} + +func TestValidateClientFeatureGates(t *testing.T) { + tests := []struct { + name string + featureGates map[string]bool + virtualNetAddress string + wantErr string + }{ + { + name: "VirtualNet enabled", + featureGates: map[string]bool{"VirtualNet": true}, + virtualNetAddress: "100.86.0.4/24", + }, + { + name: "VirtualNet explicitly disabled", + featureGates: map[string]bool{"VirtualNet": false}, + virtualNetAddress: "100.86.0.4/24", + wantErr: "VirtualNet feature is not enabled", + }, + { + name: "VirtualNet disabled by default", + virtualNetAddress: "100.86.0.4/24", + wantErr: "VirtualNet feature is not enabled", + }, + { + name: "unknown feature gate", + featureGates: map[string]bool{"UnknownFeature": true}, + wantErr: "unrecognized feature gate: UnknownFeature", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateClientFeatureGates(t, tc.featureGates, tc.virtualNetAddress) + if tc.wantErr == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, tc.wantErr) + }) + } +} + +func TestGetClientConfigRequirements(t *testing.T) { + virtualNetProxy := &v1.STCPProxyConfig{ + ProxyBaseConfig: v1.ProxyBaseConfig{ + ProxyBackend: v1.ProxyBackend{ + Plugin: v1.TypedClientPluginOptions{Type: v1.PluginVirtualNet}, + }, + }, + } + virtualNetVisitor := &v1.STCPVisitorConfig{ + VisitorBaseConfig: v1.VisitorBaseConfig{ + Plugin: v1.TypedVisitorPluginOptions{Type: v1.VisitorPluginVirtualNet}, + }, + } + + tests := []struct { + name string + common *v1.ClientCommonConfig + proxies []v1.ProxyConfigurer + visitors []v1.VisitorConfigurer + wantVNet bool + }{ + {name: "no requirements"}, + { + name: "common VirtualNet address", + common: &v1.ClientCommonConfig{VirtualNet: v1.VirtualNetConfig{Address: "100.86.0.4/24"}}, + wantVNet: true, + }, + {name: "VirtualNet proxy", proxies: []v1.ProxyConfigurer{virtualNetProxy}, wantVNet: true}, + {name: "VirtualNet visitor", visitors: []v1.VisitorConfigurer{virtualNetVisitor}, wantVNet: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := GetClientConfigRequirements(tc.common, tc.proxies, tc.visitors) + require.Equal(t, tc.wantVNet, got.VirtualNet) + }) + } +} + +func TestValidateClientFeatureGatesAreConfigScoped(t *testing.T) { + defaultGatesBefore := featuregate.DefaultFeatureGates.String() + + require.NoError(t, validateClientFeatureGates( + t, + map[string]bool{"VirtualNet": true}, + "100.86.0.4/24", + )) + require.Equal(t, defaultGatesBefore, featuregate.DefaultFeatureGates.String()) + + err := validateClientFeatureGates( + t, + map[string]bool{"VirtualNet": false}, + "100.86.0.4/24", + ) + require.ErrorContains(t, err, "VirtualNet feature is not enabled") + require.Equal(t, defaultGatesBefore, featuregate.DefaultFeatureGates.String()) +}