diff --git a/pkg/tlsconfig/configmanager.go b/pkg/tlsconfig/configmanager.go index 941eca9d103..2c59ded0714 100644 --- a/pkg/tlsconfig/configmanager.go +++ b/pkg/tlsconfig/configmanager.go @@ -27,19 +27,9 @@ type TLSConfigManager struct { // TLSConfigManager can be used for client operations (e.g. Dial), but not for server operations (e.g. Listen). // The allowInsecure parameter has no effect on server operations. func NewTLSConfigManager(useTLS bool, baseConfig *tls.Config, certPath, keyPath string, allowInsecure bool, certLoaderOpts ...TLSCertLoaderOpt) (*TLSConfigManager, error) { - var tlsConfig *tls.Config var certLoader *TLSCertLoader - - if useTLS { - // Create / clone TLS configuration as necessary. - tlsConfig = baseConfig.Clone() // nil configs are clonable. - if tlsConfig == nil { - tlsConfig = new(tls.Config) - } - - // Modify configuration. - tlsConfig.InsecureSkipVerify = allowInsecure - + tlsConfig := newTLSConfig(useTLS, baseConfig, allowInsecure) + if tlsConfig != nil { // Create TLSCertLoader and configure tlsConfig to use it. No loader is created if no cert // is provided. This is useful for client-only use cases. if certPath != "" || keyPath != "" { @@ -59,6 +49,45 @@ func NewTLSConfigManager(useTLS bool, baseConfig *tls.Config, certPath, keyPath }, nil } +// newTLSConfig creates a new tls.Config based on the passed parameters. If useTLS is false, then +// no tls.Config is generated. +func newTLSConfig(useTLS bool, baseConfig *tls.Config, allowInsecure bool) *tls.Config { + var tlsConfig *tls.Config + + if useTLS { + // Create / clone TLS configuration as necessary. + tlsConfig = baseConfig.Clone() // nil configs are clonable. + if tlsConfig == nil { + tlsConfig = new(tls.Config) + } + + // Modify configuration. + tlsConfig.InsecureSkipVerify = allowInsecure + } + + return tlsConfig +} + +// NewClientTLSConfigManager creates a TLSConfigManager that is only useful for clients without +// client certificates. TLS is enabled when useTLS is true. Certificate verification is skipped +// when allowInsecure is true. +// This is convenience wrapper for NewTLSConfigManager(useTLS, baseConfig, "", "", allowInsecure). +// In addition to being slightly more compact, NewClientTLSConfigManager can not return an error. +func NewClientTLSConfigManager(useTLS bool, baseConfig *tls.Config, allowInsecure bool) *TLSConfigManager { + return &TLSConfigManager{ + useTLS: useTLS, + tlsConfig: newTLSConfig(useTLS, baseConfig, allowInsecure), + certLoader: nil, + } +} + +// NewDisabledTLSConfigManager creates a TLSConfigManager that has TLS disabled. +// This is a convenience function equivalent to NewTLSConfigManager(false, nil, "", "", false). +// In addition to being more concise, NewDisabledTLSConfigManager can not return an error. +func NewDisabledTLSConfigManager() *TLSConfigManager { + return &TLSConfigManager{} +} + func (cm *TLSConfigManager) TLSConfig() *tls.Config { return cm.tlsConfig } diff --git a/pkg/tlsconfig/configmanager_test.go b/pkg/tlsconfig/configmanager_test.go index bf8e7b838b8..04f24458489 100644 --- a/pkg/tlsconfig/configmanager_test.go +++ b/pkg/tlsconfig/configmanager_test.go @@ -658,6 +658,113 @@ func TestTLSConfigManager_Dial(t *testing.T) { }) } +func TestNewDisabledTLSConfigManager(t *testing.T) { + // NewDisabledTLSConfigManager should be equivalent to NewTLSConfigManager(false, nil, "", "", false). + // The functional behavior of a disabled manager is already tested in TestTLSConfigManager_UseTLSFalse, + // so we just verify the two constructors produce equivalent managers. + disabled := NewDisabledTLSConfigManager() + require.NotNil(t, disabled) + explicit, err := NewTLSConfigManager(false, nil, "", "", false) + require.NoError(t, err) + require.NotNil(t, explicit) + + require.Equal(t, explicit.TLSConfig(), disabled.TLSConfig()) + require.Equal(t, explicit.TLSCertLoader(), disabled.TLSCertLoader()) + + require.NoError(t, disabled.Close()) + require.NoError(t, explicit.Close()) +} + +func TestNewClientTLSConfigManager(t *testing.T) { + // NewClientTLSConfigManager should be equivalent to NewTLSConfigManager(useTLS, baseConfig, "", "", allowInsecure). + // The functional behavior is already tested in other tests (TestTLSConfigManager_UseTLSWithoutCert, etc.), + // so we just verify the two constructors produce equivalent managers for various parameter combinations. + + t.Run("TLS disabled", func(t *testing.T) { + client := NewClientTLSConfigManager(false, nil, false) + require.NotNil(t, client) + explicit, err := NewTLSConfigManager(false, nil, "", "", false) + require.NoError(t, err) + require.NotNil(t, explicit) + + require.Equal(t, explicit.TLSConfig(), client.TLSConfig()) + require.Equal(t, explicit.TLSCertLoader(), client.TLSCertLoader()) + + require.NoError(t, client.Close()) + require.NoError(t, explicit.Close()) + }) + + t.Run("TLS enabled allowInsecure false", func(t *testing.T) { + client := NewClientTLSConfigManager(true, nil, false) + require.NotNil(t, client) + explicit, err := NewTLSConfigManager(true, nil, "", "", false) + require.NoError(t, err) + require.NotNil(t, explicit) + + require.Equal(t, explicit.TLSConfig().InsecureSkipVerify, client.TLSConfig().InsecureSkipVerify) + require.Equal(t, explicit.TLSCertLoader(), client.TLSCertLoader()) + + require.NoError(t, client.Close()) + require.NoError(t, explicit.Close()) + }) + + t.Run("TLS enabled allowInsecure true", func(t *testing.T) { + client := NewClientTLSConfigManager(true, nil, true) + require.NotNil(t, client) + explicit, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + require.NotNil(t, explicit) + + require.Equal(t, explicit.TLSConfig().InsecureSkipVerify, client.TLSConfig().InsecureSkipVerify) + require.Equal(t, explicit.TLSCertLoader(), client.TLSCertLoader()) + + require.NoError(t, client.Close()) + require.NoError(t, explicit.Close()) + }) + + t.Run("with base config", func(t *testing.T) { + baseConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + MaxVersion: tls.VersionTLS13, + ServerName: "test.example.com", + } + + client := NewClientTLSConfigManager(true, baseConfig, false) + require.NotNil(t, client) + explicit, err := NewTLSConfigManager(true, baseConfig, "", "", false) + require.NoError(t, err) + require.NotNil(t, explicit) + + // Verify base config values are preserved in both + require.Equal(t, explicit.TLSConfig().MinVersion, client.TLSConfig().MinVersion) + require.Equal(t, explicit.TLSConfig().MaxVersion, client.TLSConfig().MaxVersion) + require.Equal(t, explicit.TLSConfig().ServerName, client.TLSConfig().ServerName) + require.Equal(t, explicit.TLSConfig().InsecureSkipVerify, client.TLSConfig().InsecureSkipVerify) + require.Equal(t, explicit.TLSCertLoader(), client.TLSCertLoader()) + + require.NoError(t, client.Close()) + require.NoError(t, explicit.Close()) + }) + + t.Run("base config is cloned", func(t *testing.T) { + baseConfig := &tls.Config{ + ServerName: "original.example.com", + } + + client := NewClientTLSConfigManager(true, baseConfig, false) + require.NotNil(t, client) + + // Verify config is cloned (not same instance) + require.NotSame(t, baseConfig, client.TLSConfig()) + + // Verify modifying base config doesn't affect client config + baseConfig.ServerName = "modified.example.com" + require.Equal(t, "original.example.com", client.TLSConfig().ServerName) + + require.NoError(t, client.Close()) + }) +} + func TestTLSConfigManager_DialWithDialer(t *testing.T) { testDialWithDialerConnection := func(t *testing.T, listener net.Listener, dial func(dialer *net.Dialer, addr string) (net.Conn, error)) { t.Helper()