From f88fae5f2b90740514d858aebbf85430d9ee81b8 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Fri, 22 Mar 2024 13:07:44 +0100 Subject: [PATCH] Support PDC file contents instead of file paths (#933) * read PDC info instead of using file paths * add request specific vars for pdc * update constant vars * undo some naming changes * fix new cfg names * move tests * remove gosec nolint * separate on comma * apply PR feedback * add check for client config --- backend/common_test.go | 4 +- backend/config.go | 2 +- backend/config_test.go | 14 +- backend/proxy/env.go | 112 +++++++++++++ backend/proxy/env_test.go | 122 ++++++++++++++ backend/proxy/options.go | 2 +- backend/proxy/secure_socks_proxy.go | 122 +++----------- backend/proxy/secure_socks_proxy_test.go | 196 ++++------------------- 8 files changed, 296 insertions(+), 278 deletions(-) create mode 100644 backend/proxy/env.go create mode 100644 backend/proxy/env_test.go diff --git a/backend/common_test.go b/backend/common_test.go index 040ea6bd3..8c6d3a538 100644 --- a/backend/common_test.go +++ b/backend/common_test.go @@ -365,7 +365,7 @@ func TestProxyOptions(t *testing.T) { proxyClientCfg: &proxy.ClientCfg{ ClientCert: "", ClientKey: "123abc", - RootCA: "", + RootCAs: []string{""}, ProxyAddress: "10.1.2.3", ServerName: "grafana-server", }, @@ -381,7 +381,7 @@ func TestProxyOptions(t *testing.T) { ClientCfg: &proxy.ClientCfg{ ClientCert: "", ClientKey: "123abc", - RootCA: "", + RootCAs: []string{""}, ProxyAddress: "10.1.2.3", ServerName: "grafana-server", }, diff --git a/backend/config.go b/backend/config.go index 541c3636b..f6b1bcf5f 100644 --- a/backend/config.go +++ b/backend/config.go @@ -127,7 +127,7 @@ func (c *GrafanaCfg) proxy() (Proxy, error) { clientCfg: &proxy.ClientCfg{ ClientCert: c.Get(proxy.PluginSecureSocksProxyClientCert), ClientKey: c.Get(proxy.PluginSecureSocksProxyClientKey), - RootCA: c.Get(proxy.PluginSecureSocksProxyRootCACert), + RootCAs: strings.Split(c.Get(proxy.PluginSecureSocksProxyRootCAs), ","), ProxyAddress: c.Get(proxy.PluginSecureSocksProxyProxyAddress), ServerName: c.Get(proxy.PluginSecureSocksProxyServerName), AllowInsecure: allowInsecure, diff --git a/backend/config_test.go b/backend/config_test.go index 6312ab4e2..1520ef4de 100644 --- a/backend/config_test.go +++ b/backend/config_test.go @@ -53,7 +53,7 @@ func TestConfig(t *testing.T) { proxy.PluginSecureSocksProxyServerName: "localhost", proxy.PluginSecureSocksProxyClientKey: "clientKey", proxy.PluginSecureSocksProxyClientCert: "clientCert", - proxy.PluginSecureSocksProxyRootCACert: "rootCACert", + proxy.PluginSecureSocksProxyRootCAs: "rootCACert", }), expectedFeatureToggles: FeatureToggles{ enabled: map[string]struct{}{ @@ -64,7 +64,7 @@ func TestConfig(t *testing.T) { clientCfg: &proxy.ClientCfg{ ClientCert: "clientCert", ClientKey: "clientKey", - RootCA: "rootCACert", + RootCAs: []string{"rootCACert"}, ProxyAddress: "localhost:1234", ServerName: "localhost", }, @@ -79,7 +79,7 @@ func TestConfig(t *testing.T) { proxy.PluginSecureSocksProxyServerName: "localhost", proxy.PluginSecureSocksProxyClientKey: "clientKey", proxy.PluginSecureSocksProxyClientCert: "clientCert", - proxy.PluginSecureSocksProxyRootCACert: "rootCACert", + proxy.PluginSecureSocksProxyRootCAs: "rootCACert", }), expectedFeatureToggles: FeatureToggles{ enabled: map[string]struct{}{ @@ -97,14 +97,14 @@ func TestConfig(t *testing.T) { proxy.PluginSecureSocksProxyServerName: "localhost", proxy.PluginSecureSocksProxyClientKey: "clientKey", proxy.PluginSecureSocksProxyClientCert: "clientCert", - proxy.PluginSecureSocksProxyRootCACert: "rootCACert", + proxy.PluginSecureSocksProxyRootCAs: "rootCACert", }), expectedFeatureToggles: FeatureToggles{}, expectedProxy: Proxy{ clientCfg: &proxy.ClientCfg{ ClientCert: "clientCert", ClientKey: "clientKey", - RootCA: "rootCACert", + RootCAs: []string{"rootCACert"}, ProxyAddress: "localhost:1234", ServerName: "localhost", }, @@ -119,7 +119,7 @@ func TestConfig(t *testing.T) { proxy.PluginSecureSocksProxyServerName: "localhost", proxy.PluginSecureSocksProxyClientKey: "clientKey", proxy.PluginSecureSocksProxyClientCert: "clientCert", - proxy.PluginSecureSocksProxyRootCACert: "rootCACert", + proxy.PluginSecureSocksProxyRootCAs: "rootCACert", proxy.PluginSecureSocksProxyAllowInsecure: "true", }), expectedFeatureToggles: FeatureToggles{}, @@ -127,7 +127,7 @@ func TestConfig(t *testing.T) { clientCfg: &proxy.ClientCfg{ ClientCert: "clientCert", ClientKey: "clientKey", - RootCA: "rootCACert", + RootCAs: []string{"rootCACert"}, ProxyAddress: "localhost:1234", ServerName: "localhost", AllowInsecure: true, diff --git a/backend/proxy/env.go b/backend/proxy/env.go new file mode 100644 index 000000000..6ff71089b --- /dev/null +++ b/backend/proxy/env.go @@ -0,0 +1,112 @@ +package proxy + +import ( + "os" + "strconv" + "strings" +) + +const ( + // Deprecated: PluginSecureSocksProxyEnabledEnvVarName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED + // environment variable used to specify if a secure socks proxy is allowed to be used for datasource connections. + PluginSecureSocksProxyEnabledEnvVarName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED" + // Deprecated: PluginSecureSocksProxyClientCertFilePathEnvVarName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT + // environment variable used to specify the file location of the client cert for the secure socks proxy. + PluginSecureSocksProxyClientCertFilePathEnvVarName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT" + // Deprecated: PluginSecureSocksProxyClientKeyFilePathEnvVarName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY + // environment variable used to specify the file location of the client key for the secure socks proxy. + PluginSecureSocksProxyClientKeyFilePathEnvVarName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY" + // Deprecated: PluginSecureSocksProxyRootCACertFilePathsEnvVarName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT + // environment variable used to specify the file location of the root ca for the secure socks proxy. + PluginSecureSocksProxyRootCACertFilePathsEnvVarName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT" + // Deprecated: PluginSecureSocksProxyAddressEnvVarName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS + // environment variable used to specify the secure socks proxy server address to proxy the connections to. + PluginSecureSocksProxyAddressEnvVarName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS" + // Deprecated: PluginSecureSocksProxyServerNameEnvVarName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME + // environment variable used to specify the server name of the secure socks proxy. + PluginSecureSocksProxyServerNameEnvVarName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME" + // Deprecated: PluginSecureSocksProxyAllowInsecureEnvVarName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_ALLOW_INSECURE + // environment variable used to specify if the proxy should use a TLS dialer. + PluginSecureSocksProxyAllowInsecureEnvVarName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ALLOW_INSECURE" +) + +// Deprecated: clientCfgFromEnv gets the needed proxy information from the env variables that Grafana set with the values from the config ini +func clientCfgFromEnv() *ClientCfg { + if value, ok := os.LookupEnv(PluginSecureSocksProxyEnabledEnvVarName); ok { + enabled, err := strconv.ParseBool(value) + if err != nil || !enabled { + return nil + } + } + + proxyAddress := "" + if value, ok := os.LookupEnv(PluginSecureSocksProxyAddressEnvVarName); ok { + proxyAddress = value + } else { + return nil + } + + allowInsecure := false + if value, ok := os.LookupEnv(PluginSecureSocksProxyAllowInsecureEnvVarName); ok { + allowInsecure, _ = strconv.ParseBool(value) + } + + // We only need to fill these fields on insecure mode. + if allowInsecure { + return &ClientCfg{ + ProxyAddress: proxyAddress, + AllowInsecure: allowInsecure, + } + } + + clientCert := "" + if value, ok := os.LookupEnv(PluginSecureSocksProxyClientCertFilePathEnvVarName); ok { + certPEMBlock, err := os.ReadFile(value) + if err != nil { + return nil + } + clientCert = string(certPEMBlock) + } else { + return nil + } + + clientKey := "" + if value, ok := os.LookupEnv(PluginSecureSocksProxyClientKeyFilePathEnvVarName); ok { + keyPEMBlock, err := os.ReadFile(value) + if err != nil { + return nil + } + clientKey = string(keyPEMBlock) + } else { + return nil + } + + var rootCAs []string + if value, ok := os.LookupEnv(PluginSecureSocksProxyRootCACertFilePathsEnvVarName); ok { + for _, rootCA := range strings.Split(value, " ") { + certPEMBlock, err := os.ReadFile(rootCA) + if err != nil { + return nil + } + rootCAs = append(rootCAs, string(certPEMBlock)) + } + } else { + return nil + } + + serverName := "" + if value, ok := os.LookupEnv(PluginSecureSocksProxyServerNameEnvVarName); ok { + serverName = value + } else { + return nil + } + + return &ClientCfg{ + ClientCert: clientCert, + ClientKey: clientKey, + RootCAs: rootCAs, + ProxyAddress: proxyAddress, + ServerName: serverName, + AllowInsecure: false, + } +} diff --git a/backend/proxy/env_test.go b/backend/proxy/env_test.go new file mode 100644 index 000000000..e30f01682 --- /dev/null +++ b/backend/proxy/env_test.go @@ -0,0 +1,122 @@ +package proxy + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestClientCfgFromEnv(t *testing.T) { + // create empty file for testing configs + tempDir := t.TempDir() + testFilePath := filepath.Join(tempDir, "test") + testFileData := "foobar" + err := os.WriteFile(testFilePath, []byte(testFileData), 0600) + require.NoError(t, err) + + cases := []struct { + description string + envVars map[string]string + expected *ClientCfg + }{ + { + description: "socks proxy not enabled, should return nil", + envVars: map[string]string{ + PluginSecureSocksProxyEnabledEnvVarName: "false", + PluginSecureSocksProxyAddressEnvVarName: "localhost", + PluginSecureSocksProxyClientCertFilePathEnvVarName: "cert", + PluginSecureSocksProxyClientKeyFilePathEnvVarName: "key", + PluginSecureSocksProxyRootCACertFilePathsEnvVarName: "root_ca", + PluginSecureSocksProxyServerNameEnvVarName: "server_name", + }, + expected: nil, + }, + { + description: "allowInsecure=true, should return config without tls fields filled", + envVars: map[string]string{ + PluginSecureSocksProxyEnabledEnvVarName: "true", + PluginSecureSocksProxyAddressEnvVarName: "localhost", + PluginSecureSocksProxyAllowInsecureEnvVarName: "true", + }, + expected: &ClientCfg{ + ProxyAddress: "localhost", + AllowInsecure: true, + }, + }, + { + description: "allowInsecure=false, client cert is required, should return nil", + envVars: map[string]string{ + PluginSecureSocksProxyEnabledEnvVarName: "true", + PluginSecureSocksProxyAddressEnvVarName: "localhost", + PluginSecureSocksProxyAllowInsecureEnvVarName: "false", + }, + expected: nil, + }, + { + description: "allowInsecure=false, client key is required, should return nil", + envVars: map[string]string{ + PluginSecureSocksProxyEnabledEnvVarName: "true", + PluginSecureSocksProxyAddressEnvVarName: "localhost", + PluginSecureSocksProxyAllowInsecureEnvVarName: "false", + PluginSecureSocksProxyClientCertFilePathEnvVarName: "cert", + }, + expected: nil, + }, + { + description: "allowInsecure=false, root ca is required, should return nil", + envVars: map[string]string{ + PluginSecureSocksProxyEnabledEnvVarName: "true", + PluginSecureSocksProxyAddressEnvVarName: "localhost", + PluginSecureSocksProxyAllowInsecureEnvVarName: "false", + PluginSecureSocksProxyClientCertFilePathEnvVarName: "cert", + PluginSecureSocksProxyClientKeyFilePathEnvVarName: "key", + }, + expected: nil, + }, + { + description: "allowInsecure=false, server name is required, should return nil", + envVars: map[string]string{ + PluginSecureSocksProxyEnabledEnvVarName: "true", + PluginSecureSocksProxyAddressEnvVarName: "localhost", + PluginSecureSocksProxyAllowInsecureEnvVarName: "false", + PluginSecureSocksProxyClientCertFilePathEnvVarName: "cert", + PluginSecureSocksProxyClientKeyFilePathEnvVarName: "key", + PluginSecureSocksProxyRootCACertFilePathsEnvVarName: "root", + }, + expected: nil, + }, + { + description: "allowInsecure=false, should return config with tls fields filled", + envVars: map[string]string{ + PluginSecureSocksProxyEnabledEnvVarName: "true", + PluginSecureSocksProxyAddressEnvVarName: "localhost", + PluginSecureSocksProxyAllowInsecureEnvVarName: "false", + PluginSecureSocksProxyClientCertFilePathEnvVarName: testFilePath, + PluginSecureSocksProxyClientKeyFilePathEnvVarName: testFilePath, + PluginSecureSocksProxyRootCACertFilePathsEnvVarName: fmt.Sprintf("%s %s", testFilePath, testFilePath), + PluginSecureSocksProxyServerNameEnvVarName: "name", + }, + expected: &ClientCfg{ + ProxyAddress: "localhost", + ClientCert: testFileData, + ClientKey: testFileData, + RootCAs: []string{testFileData, testFileData}, + ServerName: "name", + AllowInsecure: false, + }, + }, + } + + for _, tt := range cases { + t.Run(tt.description, func(t *testing.T) { + for key, value := range tt.envVars { + t.Setenv(key, value) + } + assert.Equal(t, tt.expected, clientCfgFromEnv()) + }) + } +} diff --git a/backend/proxy/options.go b/backend/proxy/options.go index 2c9abfe20..a36a0c51c 100644 --- a/backend/proxy/options.go +++ b/backend/proxy/options.go @@ -44,6 +44,6 @@ func (o *Options) setDefaults() { } if o.ClientCfg == nil { - o.ClientCfg = getConfigFromEnv() + o.ClientCfg = clientCfgFromEnv() } } diff --git a/backend/proxy/secure_socks_proxy.go b/backend/proxy/secure_socks_proxy.go index 94cfe4687..531c964e7 100644 --- a/backend/proxy/secure_socks_proxy.go +++ b/backend/proxy/secure_socks_proxy.go @@ -9,9 +9,7 @@ import ( "fmt" "net" "net/http" - "os" "regexp" - "strconv" "strings" "time" @@ -21,27 +19,13 @@ import ( "golang.org/x/net/proxy" ) -var ( - // PluginSecureSocksProxyEnabled is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED - // environment variable used to specify if a secure socks proxy is allowed to be used for datasource connections. - PluginSecureSocksProxyEnabled = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED" - // PluginSecureSocksProxyClientCert is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT - // environment variable used to specify the file location of the client cert for the secure socks proxy. - PluginSecureSocksProxyClientCert = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT" - // PluginSecureSocksProxyClientKey is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY - // environment variable used to specify the file location of the client key for the secure socks proxy. - PluginSecureSocksProxyClientKey = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY" - // PluginSecureSocksProxyRootCACert is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT - // environment variable used to specify the file location of the root ca for the secure socks proxy. - PluginSecureSocksProxyRootCACert = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT" - // PluginSecureSocksProxyProxyAddress is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS - // environment variable used to specify the secure socks proxy server address to proxy the connections to. - PluginSecureSocksProxyProxyAddress = "GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS" - // PluginSecureSocksProxyServerName is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME - // environment variable used to specify the server name of the secure socks proxy. - PluginSecureSocksProxyServerName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME" - // PluginSecureSocksProxyAllowInsecure is a constant for the GF_SECURE_SOCKS_DATASOURCE_PROXY_ALLOW_INSECURE - // environment variable used to specify if the proxy should use a TLS dialer. +const ( + PluginSecureSocksProxyEnabled = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED" + PluginSecureSocksProxyClientCert = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT" + PluginSecureSocksProxyClientKey = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY" + PluginSecureSocksProxyRootCAs = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT" + PluginSecureSocksProxyProxyAddress = "GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS" + PluginSecureSocksProxyServerName = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME" PluginSecureSocksProxyAllowInsecure = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ALLOW_INSECURE" ) @@ -66,7 +50,7 @@ type Client interface { type ClientCfg struct { ClientCert string ClientKey string - RootCA string + RootCAs []string ProxyAddress string ServerName string AllowInsecure bool @@ -124,8 +108,11 @@ func (p *cfgProxyWrapper) NewSecureSocksProxyContextDialer() (proxy.Dialer, erro return nil, errors.New("proxy not enabled") } - var dialer proxy.Dialer + if p.opts.ClientCfg == nil { + return nil, errors.New("client config is not set") + } + var dialer proxy.Dialer if p.opts.ClientCfg.AllowInsecure { dialer = &net.Dialer{ Timeout: p.opts.Timeouts.Timeout, @@ -157,26 +144,24 @@ func (p *cfgProxyWrapper) NewSecureSocksProxyContextDialer() (proxy.Dialer, erro func (p *cfgProxyWrapper) getTLSDialer() (*tls.Dialer, error) { certPool := x509.NewCertPool() - for _, rootCAFile := range strings.Split(p.opts.ClientCfg.RootCA, " ") { - // nolint:gosec - // The gosec G304 warning can be ignored because `rootCAFile` comes from config ini - // and we check below if it's the right file type - pemBytes, err := os.ReadFile(rootCAFile) - if err != nil { - return nil, err - } + if len(p.opts.ClientCfg.RootCAs) == 0 { + return nil, errors.New("one or more root ca are required") + } + + for _, rootCA := range p.opts.ClientCfg.RootCAs { + pemBytes := []byte(rootCA) pemDecoded, _ := pem.Decode(pemBytes) if pemDecoded == nil || pemDecoded.Type != "CERTIFICATE" { return nil, errors.New("root ca is invalid") } if !certPool.AppendCertsFromPEM(pemBytes) { - return nil, errors.New("failed to append CA certificate " + rootCAFile) + return nil, errors.New("failed to append CA certificate to pool") } } - cert, err := tls.LoadX509KeyPair(p.opts.ClientCfg.ClientCert, p.opts.ClientCfg.ClientKey) + cert, err := tls.X509KeyPair([]byte(p.opts.ClientCfg.ClientCert), []byte(p.opts.ClientCfg.ClientKey)) if err != nil { return nil, err } @@ -195,73 +180,6 @@ func (p *cfgProxyWrapper) getTLSDialer() (*tls.Dialer, error) { }, nil } -// getConfigFromEnv gets the needed proxy information from the env variables that Grafana set with the values from the config ini -func getConfigFromEnv() *ClientCfg { - if value, ok := os.LookupEnv(PluginSecureSocksProxyEnabled); ok { - enabled, err := strconv.ParseBool(value) - if err != nil || !enabled { - return nil - } - } - - proxyAddress := "" - if value, ok := os.LookupEnv(PluginSecureSocksProxyProxyAddress); ok { - proxyAddress = value - } else { - return nil - } - - allowInsecure := false - if value, ok := os.LookupEnv(PluginSecureSocksProxyAllowInsecure); ok { - allowInsecure, _ = strconv.ParseBool(value) - } - - // We only need to fill these fields on insecure mode. - if allowInsecure { - return &ClientCfg{ - ProxyAddress: proxyAddress, - AllowInsecure: allowInsecure, - } - } - - clientCert := "" - if value, ok := os.LookupEnv(PluginSecureSocksProxyClientCert); ok { - clientCert = value - } else { - return nil - } - - clientKey := "" - if value, ok := os.LookupEnv(PluginSecureSocksProxyClientKey); ok { - clientKey = value - } else { - return nil - } - - rootCA := "" - if value, ok := os.LookupEnv(PluginSecureSocksProxyRootCACert); ok { - rootCA = value - } else { - return nil - } - - serverName := "" - if value, ok := os.LookupEnv(PluginSecureSocksProxyServerName); ok { - serverName = value - } else { - return nil - } - - return &ClientCfg{ - ClientCert: clientCert, - ClientKey: clientKey, - RootCA: rootCA, - ProxyAddress: proxyAddress, - ServerName: serverName, - AllowInsecure: false, - } -} - // SecureSocksProxyEnabledOnDS checks the datasource json data for `enableSecureSocksProxy` // to determine if the secure socks proxy should be enabled on it func SecureSocksProxyEnabledOnDS(jsonData map[string]interface{}) bool { diff --git a/backend/proxy/secure_socks_proxy_test.go b/backend/proxy/secure_socks_proxy_test.go index 07f98dc40..6ac3c68fe 100644 --- a/backend/proxy/secure_socks_proxy_test.go +++ b/backend/proxy/secure_socks_proxy_test.go @@ -8,13 +8,13 @@ import ( "crypto/x509/pkix" "encoding/pem" "errors" - "fmt" "io/fs" "math/big" "net" "net/http" "os" "path/filepath" + "strings" "testing" "time" @@ -51,21 +51,13 @@ func TestNewSecureSocksProxy(t *testing.T) { } cli := New(opts) - // create empty file for testing invalid configs - tempDir := t.TempDir() - tempEmptyFile := filepath.Join(tempDir, "emptyfile.txt") - // nolint:gosec - // The gosec G304 warning can be ignored because all values come from the test - _, err := os.Create(tempEmptyFile) - require.NoError(t, err) - t.Run("New socks proxy should be properly configured when all settings are valid", func(t *testing.T) { require.NoError(t, cli.ConfigureSecureSocksHTTPProxy(&http.Transport{})) }) t.Run("Client cert must be valid", func(t *testing.T) { clientCertBefore := opts.ClientCfg.ClientCert - opts.ClientCfg.ClientCert = tempEmptyFile + opts.ClientCfg.ClientCert = "" cli = New(opts) t.Cleanup(func() { opts.ClientCfg.ClientCert = clientCertBefore @@ -76,7 +68,7 @@ func TestNewSecureSocksProxy(t *testing.T) { t.Run("Client key must be valid", func(t *testing.T) { clientKeyBefore := opts.ClientCfg.ClientKey - opts.ClientCfg.ClientKey = tempEmptyFile + opts.ClientCfg.ClientKey = "" cli = New(opts) t.Cleanup(func() { opts.ClientCfg.ClientKey = clientKeyBefore @@ -85,12 +77,23 @@ func TestNewSecureSocksProxy(t *testing.T) { require.Error(t, cli.ConfigureSecureSocksHTTPProxy(&http.Transport{})) }) + t.Run("Root CA must be not empty", func(t *testing.T) { + rootCABefore := opts.ClientCfg.RootCAs + opts.ClientCfg.RootCAs = []string{} + cli = New(opts) + t.Cleanup(func() { + opts.ClientCfg.RootCAs = rootCABefore + cli = New(opts) + }) + require.Error(t, cli.ConfigureSecureSocksHTTPProxy(&http.Transport{})) + }) + t.Run("Root CA must be valid", func(t *testing.T) { - rootCABefore := opts.ClientCfg.RootCA - opts.ClientCfg.RootCA = tempEmptyFile + rootCABefore := opts.ClientCfg.RootCAs + opts.ClientCfg.RootCAs = []string{""} cli = New(opts) t.Cleanup(func() { - opts.ClientCfg.RootCA = rootCABefore + opts.ClientCfg.RootCAs = rootCABefore cli = New(opts) }) require.Error(t, cli.ConfigureSecureSocksHTTPProxy(&http.Transport{})) @@ -112,130 +115,6 @@ func TestSecureSocksProxyEnabled(t *testing.T) { }) } -func TestGetConfigFromEnv(t *testing.T) { - cases := []struct { - description string - envVars map[string]string - expected *ClientCfg - }{ - { - description: "socks proxy not enabled, should return nil", - envVars: map[string]string{ - PluginSecureSocksProxyEnabled: "false", - PluginSecureSocksProxyProxyAddress: "localhost", - PluginSecureSocksProxyClientCert: "cert", - PluginSecureSocksProxyClientKey: "key", - PluginSecureSocksProxyRootCACert: "root_ca", - PluginSecureSocksProxyServerName: "server_name", - }, - expected: nil, - }, - { - description: "allowInsecure=true, should return config without tls fields filled", - envVars: map[string]string{ - PluginSecureSocksProxyEnabled: "true", - PluginSecureSocksProxyProxyAddress: "localhost", - PluginSecureSocksProxyAllowInsecure: "true", - }, - expected: &ClientCfg{ - ProxyAddress: "localhost", - AllowInsecure: true, - }, - }, - { - description: "allowInsecure=false, client cert is required, should return nil", - envVars: map[string]string{ - PluginSecureSocksProxyEnabled: "true", - PluginSecureSocksProxyProxyAddress: "localhost", - PluginSecureSocksProxyAllowInsecure: "false", - }, - expected: nil, - }, - { - description: "allowInsecure=false, client key is required, should return nil", - envVars: map[string]string{ - PluginSecureSocksProxyEnabled: "true", - PluginSecureSocksProxyProxyAddress: "localhost", - PluginSecureSocksProxyAllowInsecure: "false", - PluginSecureSocksProxyClientCert: "cert", - }, - expected: nil, - }, - { - description: "allowInsecure=false, root ca is required, should return nil", - envVars: map[string]string{ - PluginSecureSocksProxyEnabled: "true", - PluginSecureSocksProxyProxyAddress: "localhost", - PluginSecureSocksProxyAllowInsecure: "false", - PluginSecureSocksProxyClientCert: "cert", - PluginSecureSocksProxyClientKey: "key", - }, - expected: nil, - }, - { - description: "allowInsecure=false, server name is required, should return nil", - envVars: map[string]string{ - PluginSecureSocksProxyEnabled: "true", - PluginSecureSocksProxyProxyAddress: "localhost", - PluginSecureSocksProxyAllowInsecure: "false", - PluginSecureSocksProxyClientCert: "cert", - PluginSecureSocksProxyClientKey: "key", - PluginSecureSocksProxyRootCACert: "root", - }, - expected: nil, - }, - { - description: "allowInsecure=false, should return config with tls fields filled", - envVars: map[string]string{ - PluginSecureSocksProxyEnabled: "true", - PluginSecureSocksProxyProxyAddress: "localhost", - PluginSecureSocksProxyAllowInsecure: "false", - PluginSecureSocksProxyClientCert: "cert", - PluginSecureSocksProxyClientKey: "key", - PluginSecureSocksProxyRootCACert: "root", - PluginSecureSocksProxyServerName: "name", - }, - expected: &ClientCfg{ - ProxyAddress: "localhost", - ClientCert: "cert", - ClientKey: "key", - RootCA: "root", - ServerName: "name", - AllowInsecure: false, - }, - }, - } - - for _, tt := range cases { - t.Run(tt.description, func(t *testing.T) { - for key, value := range tt.envVars { - t.Setenv(key, value) - } - assert.Equal(t, tt.expected, getConfigFromEnv()) - }) - } -} - -func TestSecureSocksProxyConfig(t *testing.T) { - expected := ClientCfg{ - ProxyAddress: "localhost:8080", - AllowInsecure: true, - } - t.Setenv(PluginSecureSocksProxyEnabled, "true") - t.Setenv(PluginSecureSocksProxyProxyAddress, expected.ProxyAddress) - t.Setenv(PluginSecureSocksProxyAllowInsecure, fmt.Sprint(expected.AllowInsecure)) - - t.Run("test env variables", func(t *testing.T) { - assert.Equal(t, &expected, getConfigFromEnv()) - }) - - t.Run("test overriding env variables", func(t *testing.T) { - expected.ProxyAddress = "localhost:8082" - t.Setenv(PluginSecureSocksProxyProxyAddress, expected.ProxyAddress) - assert.Equal(t, &expected, getConfigFromEnv()) - }) -} - func TestSecureSocksProxyEnabledOnDS(t *testing.T) { t.Run("Secure socks proxy should only be enabled when the json data contains enableSecureSocksProxy=true", func(t *testing.T) { tests := []struct { @@ -281,15 +160,13 @@ func TestPreventInvalidRootCA(t *testing.T) { } t.Run("root ca must be of the type CERTIFICATE", func(t *testing.T) { - rootCACert := filepath.Join(tempDir, "ca.cert") - caCertFile, err := os.Create(rootCACert) - require.NoError(t, err) - err = pem.Encode(caCertFile, &pem.Block{ + pemStr := new(strings.Builder) + err := pem.Encode(pemStr, &pem.Block{ Type: "PUBLIC KEY", Bytes: []byte("testing"), }) require.NoError(t, err) - opts.ClientCfg.RootCA = rootCACert + opts.ClientCfg.RootCAs = []string{pemStr.String()} cli := New(opts) _, err = cli.NewSecureSocksProxyContextDialer() require.Contains(t, err.Error(), "root ca is invalid") @@ -298,7 +175,7 @@ func TestPreventInvalidRootCA(t *testing.T) { rootCACert := filepath.Join(tempDir, "ca.cert") err := os.WriteFile(rootCACert, []byte("this is not a pem encoded file"), fs.ModeAppend) require.NoError(t, err) - opts.ClientCfg.RootCA = rootCACert + opts.ClientCfg.RootCAs = []string{"this is not a pem encoded file"} cli := New(opts) _, err = cli.NewSecureSocksProxyContextDialer() require.Contains(t, err.Error(), "root ca is invalid") @@ -309,7 +186,6 @@ func setupTestSecureSocksProxySettings(t *testing.T) *ClientCfg { t.Helper() proxyAddress := "localhost:3000" serverName := "localhost" - tempDir := t.TempDir() // generate test rootCA ca := &x509.Certificate{ @@ -329,12 +205,9 @@ func setupTestSecureSocksProxySettings(t *testing.T) *ClientCfg { require.NoError(t, err) caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey) require.NoError(t, err) - rootCACert := filepath.Join(tempDir, "ca.cert") - // nolint:gosec - // The gosec G304 warning can be ignored because all values come from the test - caCertFile, err := os.Create(rootCACert) - require.NoError(t, err) - err = pem.Encode(caCertFile, &pem.Block{ + + caCert := new(strings.Builder) + err = pem.Encode(caCert, &pem.Block{ Type: "CERTIFICATE", Bytes: caBytes, }) @@ -356,32 +229,25 @@ func setupTestSecureSocksProxySettings(t *testing.T) *ClientCfg { certPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) require.NoError(t, err) certBytes, err := x509.CreateCertificate(rand.Reader, cert, ca, &certPrivKey.PublicKey, caPrivKey) + clientCert := new(strings.Builder) require.NoError(t, err) - clientCert := filepath.Join(tempDir, "client.cert") - // nolint:gosec - // The gosec G304 warning can be ignored because all values come from the test - certFile, err := os.Create(clientCert) - require.NoError(t, err) - err = pem.Encode(certFile, &pem.Block{ + err = pem.Encode(clientCert, &pem.Block{ Type: "CERTIFICATE", Bytes: certBytes, }) require.NoError(t, err) - clientKey := filepath.Join(tempDir, "client.key") - // nolint:gosec - // The gosec G304 warning can be ignored because all values come from the test - keyFile, err := os.Create(clientKey) + clientKey := new(strings.Builder) require.NoError(t, err) - err = pem.Encode(keyFile, &pem.Block{ + err = pem.Encode(clientKey, &pem.Block{ Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(certPrivKey), }) require.NoError(t, err) cfg := &ClientCfg{ - ClientCert: clientCert, - ClientKey: clientKey, - RootCA: rootCACert, + ClientCert: clientCert.String(), + ClientKey: clientKey.String(), + RootCAs: []string{caCert.String()}, ServerName: serverName, ProxyAddress: proxyAddress, }