Skip to content

Commit 0fe8f7a

Browse files
authored
refactor: reorganize security policy into dedicated packag (#5088)
1 parent 2e2802e commit 0fe8f7a

File tree

8 files changed

+133
-68
lines changed

8 files changed

+133
-68
lines changed

client/service.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/fatedier/frp/pkg/auth"
3232
v1 "github.com/fatedier/frp/pkg/config/v1"
3333
"github.com/fatedier/frp/pkg/msg"
34+
"github.com/fatedier/frp/pkg/policy/security"
3435
httppkg "github.com/fatedier/frp/pkg/util/http"
3536
"github.com/fatedier/frp/pkg/util/log"
3637
netpkg "github.com/fatedier/frp/pkg/util/net"
@@ -64,7 +65,7 @@ type ServiceOptions struct {
6465
ProxyCfgs []v1.ProxyConfigurer
6566
VisitorCfgs []v1.VisitorConfigurer
6667

67-
UnsafeFeatures v1.UnsafeFeatures
68+
UnsafeFeatures *security.UnsafeFeatures
6869

6970
// ConfigFilePath is the path to the configuration file used to initialize.
7071
// If it is empty, it means that the configuration file is not used for initialization.
@@ -124,7 +125,7 @@ type Service struct {
124125
visitorCfgs []v1.VisitorConfigurer
125126
clientSpec *msg.ClientSpec
126127

127-
unsafeFeatures v1.UnsafeFeatures
128+
unsafeFeatures *security.UnsafeFeatures
128129

129130
// The configuration file used to initialize this client, or an empty
130131
// string if no configuration file was used.

cmd/frpc/sub/proxy.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/fatedier/frp/pkg/config"
2525
v1 "github.com/fatedier/frp/pkg/config/v1"
2626
"github.com/fatedier/frp/pkg/config/v1/validation"
27+
"github.com/fatedier/frp/pkg/policy/security"
2728
)
2829

2930
var proxyTypes = []v1.ProxyType{
@@ -78,7 +79,7 @@ func NewProxyCommand(name string, c v1.ProxyConfigurer, clientCfg *v1.ClientComm
7879
os.Exit(1)
7980
}
8081

81-
unsafeFeatures := v1.NewUnsafeFeatures(allowUnsafe)
82+
unsafeFeatures := security.NewUnsafeFeatures(allowUnsafe)
8283
if _, err := validation.ValidateClientCommonConfig(clientCfg, unsafeFeatures); err != nil {
8384
fmt.Println(err)
8485
os.Exit(1)
@@ -108,7 +109,7 @@ func NewVisitorCommand(name string, c v1.VisitorConfigurer, clientCfg *v1.Client
108109
fmt.Println(err)
109110
os.Exit(1)
110111
}
111-
unsafeFeatures := v1.NewUnsafeFeatures(allowUnsafe)
112+
unsafeFeatures := security.NewUnsafeFeatures(allowUnsafe)
112113
if _, err := validation.ValidateClientCommonConfig(clientCfg, unsafeFeatures); err != nil {
113114
fmt.Println(err)
114115
os.Exit(1)

cmd/frpc/sub/root.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"os"
2222
"os/signal"
2323
"path/filepath"
24+
"strings"
2425
"sync"
2526
"syscall"
2627
"time"
@@ -31,7 +32,8 @@ import (
3132
"github.com/fatedier/frp/pkg/config"
3233
v1 "github.com/fatedier/frp/pkg/config/v1"
3334
"github.com/fatedier/frp/pkg/config/v1/validation"
34-
"github.com/fatedier/frp/pkg/featuregate"
35+
"github.com/fatedier/frp/pkg/policy/featuregate"
36+
"github.com/fatedier/frp/pkg/policy/security"
3537
"github.com/fatedier/frp/pkg/util/log"
3638
"github.com/fatedier/frp/pkg/util/version"
3739
)
@@ -49,7 +51,9 @@ func init() {
4951
rootCmd.PersistentFlags().StringVarP(&cfgDir, "config_dir", "", "", "config directory, run one frpc service for each file in config directory")
5052
rootCmd.PersistentFlags().BoolVarP(&showVersion, "version", "v", false, "version of frpc")
5153
rootCmd.PersistentFlags().BoolVarP(&strictConfigMode, "strict_config", "", true, "strict config parsing mode, unknown fields will cause an errors")
52-
rootCmd.PersistentFlags().StringSliceVarP(&allowUnsafe, "allow-unsafe", "", []string{}, "allowed unsafe features, one or more of: TokenSourceExec")
54+
55+
rootCmd.PersistentFlags().StringSliceVarP(&allowUnsafe, "allow-unsafe", "", []string{},
56+
fmt.Sprintf("allowed unsafe features, one or more of: %s", strings.Join(security.ClientUnsafeFeatures, ", ")))
5357
}
5458

5559
var rootCmd = &cobra.Command{
@@ -61,7 +65,7 @@ var rootCmd = &cobra.Command{
6165
return nil
6266
}
6367

64-
unsafeFeatures := v1.NewUnsafeFeatures(allowUnsafe)
68+
unsafeFeatures := security.NewUnsafeFeatures(allowUnsafe)
6569

6670
// If cfgDir is not empty, run multiple frpc service for each config file in cfgDir.
6771
// Note that it's only designed for testing. It's not guaranteed to be stable.
@@ -80,7 +84,7 @@ var rootCmd = &cobra.Command{
8084
},
8185
}
8286

83-
func runMultipleClients(cfgDir string, unsafeFeatures v1.UnsafeFeatures) error {
87+
func runMultipleClients(cfgDir string, unsafeFeatures *security.UnsafeFeatures) error {
8488
var wg sync.WaitGroup
8589
err := filepath.WalkDir(cfgDir, func(path string, d fs.DirEntry, err error) error {
8690
if err != nil || d.IsDir() {
@@ -115,7 +119,7 @@ func handleTermSignal(svr *client.Service) {
115119
svr.GracefulClose(500 * time.Millisecond)
116120
}
117121

118-
func runClient(cfgFilePath string, unsafeFeatures v1.UnsafeFeatures) error {
122+
func runClient(cfgFilePath string, unsafeFeatures *security.UnsafeFeatures) error {
119123
cfg, proxyCfgs, visitorCfgs, isLegacyFormat, err := config.LoadClientConfig(cfgFilePath, strictConfigMode)
120124
if err != nil {
121125
return err
@@ -145,7 +149,7 @@ func startService(
145149
cfg *v1.ClientCommonConfig,
146150
proxyCfgs []v1.ProxyConfigurer,
147151
visitorCfgs []v1.VisitorConfigurer,
148-
unsafeFeatures v1.UnsafeFeatures,
152+
unsafeFeatures *security.UnsafeFeatures,
149153
cfgFile string,
150154
) error {
151155
log.InitLogger(cfg.Log.To, cfg.Log.Level, int(cfg.Log.MaxDays), cfg.Log.DisablePrintColor)

cmd/frpc/sub/verify.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import (
2121
"github.com/spf13/cobra"
2222

2323
"github.com/fatedier/frp/pkg/config"
24-
v1 "github.com/fatedier/frp/pkg/config/v1"
2524
"github.com/fatedier/frp/pkg/config/v1/validation"
25+
"github.com/fatedier/frp/pkg/policy/security"
2626
)
2727

2828
func init() {
@@ -43,7 +43,7 @@ var verifyCmd = &cobra.Command{
4343
fmt.Println(err)
4444
os.Exit(1)
4545
}
46-
unsafeFeatures := v1.NewUnsafeFeatures(allowUnsafe)
46+
unsafeFeatures := security.NewUnsafeFeatures(allowUnsafe)
4747
warning, err := validation.ValidateAllClientConfig(cliCfg, proxyCfgs, visitorCfgs, unsafeFeatures)
4848
if warning != nil {
4949
fmt.Printf("WARNING: %v\n", warning)

pkg/config/v1/client.go

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -248,23 +248,3 @@ type AuthOIDCClientConfig struct {
248248
type VirtualNetConfig struct {
249249
Address string `json:"address,omitempty"`
250250
}
251-
252-
const (
253-
UnsafeFeatureTokenSourceExec = "TokenSourceExec"
254-
)
255-
256-
type UnsafeFeatures struct {
257-
features map[string]bool
258-
}
259-
260-
func NewUnsafeFeatures(allowed []string) UnsafeFeatures {
261-
features := make(map[string]bool)
262-
for _, f := range allowed {
263-
features[f] = true
264-
}
265-
return UnsafeFeatures{features: features}
266-
}
267-
268-
func (u UnsafeFeatures) IsEnabled(feature string) bool {
269-
return u.features[feature]
270-
}

pkg/config/v1/validation/client.go

Lines changed: 81 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,87 +23,132 @@ import (
2323
"github.com/samber/lo"
2424

2525
v1 "github.com/fatedier/frp/pkg/config/v1"
26-
"github.com/fatedier/frp/pkg/featuregate"
26+
"github.com/fatedier/frp/pkg/policy/featuregate"
27+
"github.com/fatedier/frp/pkg/policy/security"
2728
)
2829

29-
func ValidateClientCommonConfig(c *v1.ClientCommonConfig, unsafeFeatures v1.UnsafeFeatures) (Warning, error) {
30+
func ValidateClientCommonConfig(c *v1.ClientCommonConfig, unsafeFeatures *security.UnsafeFeatures) (Warning, error) {
3031
var (
3132
warnings Warning
3233
errs error
3334
)
34-
// validate feature gates
35+
36+
validators := []func() (Warning, error){
37+
func() (Warning, error) { return validateFeatureGates(c) },
38+
func() (Warning, error) { return validateAuthConfig(&c.Auth, unsafeFeatures) },
39+
func() (Warning, error) { return nil, validateLogConfig(&c.Log) },
40+
func() (Warning, error) { return nil, validateWebServerConfig(&c.WebServer) },
41+
func() (Warning, error) { return validateTransportConfig(&c.Transport) },
42+
func() (Warning, error) { return validateIncludeFiles(c.IncludeConfigFiles) },
43+
}
44+
45+
for _, v := range validators {
46+
w, err := v()
47+
warnings = AppendError(warnings, w)
48+
errs = AppendError(errs, err)
49+
}
50+
return warnings, errs
51+
}
52+
53+
func validateFeatureGates(c *v1.ClientCommonConfig) (Warning, error) {
3554
if c.VirtualNet.Address != "" {
3655
if !featuregate.Enabled(featuregate.VirtualNet) {
37-
return warnings, fmt.Errorf("VirtualNet feature is not enabled; enable it by setting the appropriate feature gate flag")
56+
return nil, fmt.Errorf("VirtualNet feature is not enabled; enable it by setting the appropriate feature gate flag")
3857
}
3958
}
59+
return nil, nil
60+
}
4061

41-
if !slices.Contains(SupportedAuthMethods, c.Auth.Method) {
62+
func validateAuthConfig(c *v1.AuthClientConfig, unsafeFeatures *security.UnsafeFeatures) (Warning, error) {
63+
var errs error
64+
if !slices.Contains(SupportedAuthMethods, c.Method) {
4265
errs = AppendError(errs, fmt.Errorf("invalid auth method, optional values are %v", SupportedAuthMethods))
4366
}
44-
if !lo.Every(SupportedAuthAdditionalScopes, c.Auth.AdditionalScopes) {
67+
if !lo.Every(SupportedAuthAdditionalScopes, c.AdditionalScopes) {
4568
errs = AppendError(errs, fmt.Errorf("invalid auth additional scopes, optional values are %v", SupportedAuthAdditionalScopes))
4669
}
4770

4871
// Validate token/tokenSource mutual exclusivity
49-
if c.Auth.Token != "" && c.Auth.TokenSource != nil {
72+
if c.Token != "" && c.TokenSource != nil {
5073
errs = AppendError(errs, fmt.Errorf("cannot specify both auth.token and auth.tokenSource"))
5174
}
5275

5376
// Validate tokenSource if specified
54-
if c.Auth.TokenSource != nil {
55-
if c.Auth.TokenSource.Type == "exec" && !unsafeFeatures.IsEnabled(v1.UnsafeFeatureTokenSourceExec) {
56-
errs = AppendError(errs, fmt.Errorf("unsafe 'exec' not allowed for auth.tokenSource.type"))
77+
if c.TokenSource != nil {
78+
if c.TokenSource.Type == "exec" {
79+
if !unsafeFeatures.IsEnabled(security.TokenSourceExec) {
80+
errs = AppendError(errs, fmt.Errorf("unsafe feature %q is not enabled. "+
81+
"To enable it, start frpc with '--allow-unsafe %s'", security.TokenSourceExec, security.TokenSourceExec))
82+
}
5783
}
58-
if err := c.Auth.TokenSource.Validate(); err != nil {
84+
if err := c.TokenSource.Validate(); err != nil {
5985
errs = AppendError(errs, fmt.Errorf("invalid auth.tokenSource: %v", err))
6086
}
6187
}
6288

63-
if c.Auth.OIDC.TokenSource != nil {
64-
// Validate oidc.tokenSource mutual exclusivity with other fields of oidc
65-
if c.Auth.OIDC.ClientID != "" || c.Auth.OIDC.ClientSecret != "" || c.Auth.OIDC.Audience != "" ||
66-
c.Auth.OIDC.Scope != "" || c.Auth.OIDC.TokenEndpointURL != "" || len(c.Auth.OIDC.AdditionalEndpointParams) > 0 ||
67-
c.Auth.OIDC.TrustedCaFile != "" || c.Auth.OIDC.InsecureSkipVerify || c.Auth.OIDC.ProxyURL != "" {
68-
errs = AppendError(errs, fmt.Errorf("cannot specify both auth.oidc.tokenSource and any other field of auth.oidc"))
69-
}
70-
if c.Auth.OIDC.TokenSource.Type == "exec" && !unsafeFeatures.IsEnabled(v1.UnsafeFeatureTokenSourceExec) {
71-
errs = AppendError(errs, fmt.Errorf("unsafe 'exec' not allowed for auth.oidc.tokenSource.type"))
72-
}
73-
}
74-
75-
if err := validateLogConfig(&c.Log); err != nil {
89+
if err := validateOIDCConfig(&c.OIDC, unsafeFeatures); err != nil {
7690
errs = AppendError(errs, err)
7791
}
92+
return nil, errs
93+
}
7894

79-
if err := validateWebServerConfig(&c.WebServer); err != nil {
80-
errs = AppendError(errs, err)
95+
func validateOIDCConfig(c *v1.AuthOIDCClientConfig, unsafeFeatures *security.UnsafeFeatures) error {
96+
if c.TokenSource == nil {
97+
return nil
98+
}
99+
var errs error
100+
// Validate oidc.tokenSource mutual exclusivity with other fields of oidc
101+
if c.ClientID != "" || c.ClientSecret != "" || c.Audience != "" ||
102+
c.Scope != "" || c.TokenEndpointURL != "" || len(c.AdditionalEndpointParams) > 0 ||
103+
c.TrustedCaFile != "" || c.InsecureSkipVerify || c.ProxyURL != "" {
104+
errs = AppendError(errs, fmt.Errorf("cannot specify both auth.oidc.tokenSource and any other field of auth.oidc"))
105+
}
106+
if c.TokenSource.Type == "exec" {
107+
if !unsafeFeatures.IsEnabled(security.TokenSourceExec) {
108+
errs = AppendError(errs, fmt.Errorf("unsafe feature %q is not enabled. "+
109+
"To enable it, start frpc with '--allow-unsafe %s'", security.TokenSourceExec, security.TokenSourceExec))
110+
}
81111
}
112+
if err := c.TokenSource.Validate(); err != nil {
113+
errs = AppendError(errs, fmt.Errorf("invalid auth.oidc.tokenSource: %v", err))
114+
}
115+
return errs
116+
}
117+
118+
func validateTransportConfig(c *v1.ClientTransportConfig) (Warning, error) {
119+
var (
120+
warnings Warning
121+
errs error
122+
)
82123

83-
if c.Transport.HeartbeatTimeout > 0 && c.Transport.HeartbeatInterval > 0 {
84-
if c.Transport.HeartbeatTimeout < c.Transport.HeartbeatInterval {
124+
if c.HeartbeatTimeout > 0 && c.HeartbeatInterval > 0 {
125+
if c.HeartbeatTimeout < c.HeartbeatInterval {
85126
errs = AppendError(errs, fmt.Errorf("invalid transport.heartbeatTimeout, heartbeat timeout should not less than heartbeat interval"))
86127
}
87128
}
88129

89-
if !lo.FromPtr(c.Transport.TLS.Enable) {
130+
if !lo.FromPtr(c.TLS.Enable) {
90131
checkTLSConfig := func(name string, value string) Warning {
91132
if value != "" {
92133
return fmt.Errorf("%s is invalid when transport.tls.enable is false", name)
93134
}
94135
return nil
95136
}
96137

97-
warnings = AppendError(warnings, checkTLSConfig("transport.tls.certFile", c.Transport.TLS.CertFile))
98-
warnings = AppendError(warnings, checkTLSConfig("transport.tls.keyFile", c.Transport.TLS.KeyFile))
99-
warnings = AppendError(warnings, checkTLSConfig("transport.tls.trustedCaFile", c.Transport.TLS.TrustedCaFile))
138+
warnings = AppendError(warnings, checkTLSConfig("transport.tls.certFile", c.TLS.CertFile))
139+
warnings = AppendError(warnings, checkTLSConfig("transport.tls.keyFile", c.TLS.KeyFile))
140+
warnings = AppendError(warnings, checkTLSConfig("transport.tls.trustedCaFile", c.TLS.TrustedCaFile))
100141
}
101142

102-
if !slices.Contains(SupportedTransportProtocols, c.Transport.Protocol) {
143+
if !slices.Contains(SupportedTransportProtocols, c.Protocol) {
103144
errs = AppendError(errs, fmt.Errorf("invalid transport.protocol, optional values are %v", SupportedTransportProtocols))
104145
}
146+
return warnings, errs
147+
}
105148

106-
for _, f := range c.IncludeConfigFiles {
149+
func validateIncludeFiles(files []string) (Warning, error) {
150+
var errs error
151+
for _, f := range files {
107152
absDir, err := filepath.Abs(filepath.Dir(f))
108153
if err != nil {
109154
errs = AppendError(errs, fmt.Errorf("include: parse directory of %s failed: %v", f, err))
@@ -113,14 +158,14 @@ func ValidateClientCommonConfig(c *v1.ClientCommonConfig, unsafeFeatures v1.Unsa
113158
errs = AppendError(errs, fmt.Errorf("include: directory of %s not exist", f))
114159
}
115160
}
116-
return warnings, errs
161+
return nil, errs
117162
}
118163

119164
func ValidateAllClientConfig(
120165
c *v1.ClientCommonConfig,
121166
proxyCfgs []v1.ProxyConfigurer,
122167
visitorCfgs []v1.VisitorConfigurer,
123-
unsafeFeatures v1.UnsafeFeatures,
168+
unsafeFeatures *security.UnsafeFeatures,
124169
) (Warning, error) {
125170
var warnings Warning
126171
if c != nil {

pkg/policy/security/unsafe.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package security
2+
3+
const (
4+
TokenSourceExec = "TokenSourceExec"
5+
)
6+
7+
var (
8+
ClientUnsafeFeatures = []string{
9+
TokenSourceExec,
10+
}
11+
12+
ServerUnsafeFeatures = []string{
13+
TokenSourceExec,
14+
}
15+
)
16+
17+
type UnsafeFeatures struct {
18+
features map[string]bool
19+
}
20+
21+
func NewUnsafeFeatures(allowed []string) *UnsafeFeatures {
22+
features := make(map[string]bool)
23+
for _, f := range allowed {
24+
features[f] = true
25+
}
26+
return &UnsafeFeatures{features: features}
27+
}
28+
29+
func (u *UnsafeFeatures) IsEnabled(feature string) bool {
30+
if u == nil {
31+
return false
32+
}
33+
return u.features[feature]
34+
}

0 commit comments

Comments
 (0)