diff --git a/etc/config.sample.toml b/etc/config.sample.toml index aa51a28124a..83f6b49de2d 100644 --- a/etc/config.sample.toml +++ b/etc/config.sample.toml @@ -340,12 +340,15 @@ # Determines whether HTTPS is enabled. # https-enabled = false - # The SSL certificate to use when HTTPS is enabled. + # The TLS certificate to use when HTTPS is enabled. # https-certificate = "/etc/ssl/influxdb.pem" # Use a separate private key location. # https-private-key = "" + # Allows insecure file permissions for https-certificate and https-private-key when enabled. + # https-insecure-certificate = false + # The JWT auth shared secret to validate requests using JSON web tokens. # shared-secret = "" @@ -534,9 +537,19 @@ # database = "opentsdb" # retention-policy = "" # consistency-level = "one" + + # Turns on TLS for this OpenTSDB instance. # tls-enabled = false + + # TLS certificate to use when TLS is enabled. # certificate= "/etc/ssl/influxdb.pem" + # TLS private key when TLS is enabled. If blank, defaults to assuming key is in certificate. + # private-key = "" + + # Allows insecure file permissions on certificate and private-key when enabled. + # insecure-certificate = false + # Log an error for every malformed point. # log-point-errors = true diff --git a/pkg/tlsconfig/certconfig.go b/pkg/tlsconfig/certconfig.go index 685a998ad79..092bd249925 100644 --- a/pkg/tlsconfig/certconfig.go +++ b/pkg/tlsconfig/certconfig.go @@ -59,13 +59,35 @@ func (lc *LoadedCertificate) IsValid() bool { return lc.valid } +// loadCertificateConfig is an internal config for LoadCertificate. +type loadCertificateConfig struct { + // ignoreFilePermissions indicates if file permissions should be ignored during load. + ignoreFilePermissions bool +} + +// LoadCertificateOpt are functions to change the behavior of LoadCertificate. +type LoadCertificateOpt func(*loadCertificateConfig) + +// WithLoadCertificateIgnoreFilePermissions instructs LoadCertificate to ignore file permissions +// if ignore is true. +func WithLoadCertificateIgnoreFilePermissions(ignore bool) LoadCertificateOpt { + return func(c *loadCertificateConfig) { + c.ignoreFilePermissions = ignore + } +} + // LoadCertificate loads a key pair from certPath and keyPath, performing several checks // along the way. If any checks fail or an error occurs loading the files, then an error is returned. // If keyPath is empty, then certPath is assumed to contain both the certificate and the private key. // Only trusted input (standard configuration files) should be used for certPath and keyPath. -func LoadCertificate(certPath, keyPath string) (LoadedCertificate, error) { +func LoadCertificate(certPath, keyPath string, opts ...LoadCertificateOpt) (LoadedCertificate, error) { fail := func(err error) (LoadedCertificate, error) { return LoadedCertificate{valid: false}, err } + config := loadCertificateConfig{} + for _, o := range opts { + o(&config) + } + if certPath == "" { return fail(fmt.Errorf("LoadCertificate: certificate: %w", ErrPathEmpty)) } @@ -95,9 +117,11 @@ func LoadCertificate(certPath, keyPath string) (LoadedCertificate, error) { } }() - if err := file.VerifyFilePermissivenessF(f, maxPerms); err != nil { - // VerifyFilePermissivenessF includes a lot context in its errors. No need to add duplicate here. - return nil, fmt.Errorf("LoadCertificate: %w", err) + if !config.ignoreFilePermissions { + if err := file.VerifyFilePermissivenessF(f, maxPerms); err != nil { + // VerifyFilePermissivenessF includes a lot context in its errors. No need to add duplicate here. + return nil, fmt.Errorf("LoadCertificate: %w", err) + } } data, err := io.ReadAll(f) if err != nil { @@ -120,7 +144,7 @@ func LoadCertificate(certPath, keyPath string) (LoadedCertificate, error) { // Create key pair from loaded data cert, err := tls.X509KeyPair(certData, keyData) if err != nil { - return fail(fmt.Errorf("error loading x509 key pair: %w", err)) + return fail(fmt.Errorf("error loading x509 key pair (%q / %q): %w", certPath, keyPath, err)) } // Parse the first X509 certificate in cert's chain. @@ -129,11 +153,11 @@ func LoadCertificate(certPath, keyPath string) (LoadedCertificate, error) { if err != nil { // This should be impossible to reach because `tls.X509KeyPair` will fail // if the leaf certificate can't be parsed. - return fail(fmt.Errorf("error parsing leaf certificate: %w", err)) + return fail(fmt.Errorf("error parsing leaf certificate (%q / %q): %w", certPath, keyPath, err)) } if leaf == nil { // This shouldn't happen, but we should be extra careful with TLS certs. - return fail(fmt.Errorf("error parsing leaf certificate: %w", ErrCertificateNil)) + return fail(fmt.Errorf("error parsing leaf certificate (%q / %q): %w", certPath, keyPath, ErrCertificateNil)) } return LoadedCertificate{ @@ -157,12 +181,18 @@ type TLSCertLoader struct { // certificateCheckInterval determines the duration between each certificate check. certificateCheckInterval time.Duration + // ignoreFilePermissions is true if file permission checks should be bypassed. + ignoreFilePermissions bool + // closeOnce is used to close closeCh exactly one time. closeOnce sync.Once // closeCh is used to trigger closing the monitor. closeCh chan struct{} + // monitorStartWg can be used to detect if the monitor goroutine has started. + monitorStartWg sync.WaitGroup + // mu protects all members below. mu sync.Mutex @@ -181,28 +211,35 @@ type TLSCertLoader struct { type TLSCertLoaderOpt func(*TLSCertLoader) -// WithExpirationAdvanced sets the how far ahead a CertLoader will +// WithCertLoaderExpirationAdvanced sets the how far ahead a CertLoader will // warn about a certificate that is about to expire. -func WithExpirationAdvanced(d time.Duration) TLSCertLoaderOpt { +func WithCertLoaderExpirationAdvanced(d time.Duration) TLSCertLoaderOpt { return func(cl *TLSCertLoader) { cl.expirationAdvanced = d } } -// WithCertificateCheckInterval sets how often to check for certificate expiration. -func WithCertificateCheckInterval(d time.Duration) TLSCertLoaderOpt { +// WithCertLoaderCertificateCheckInterval sets how often to check for certificate expiration. +func WithCertLoaderCertificateCheckInterval(d time.Duration) TLSCertLoaderOpt { return func(cl *TLSCertLoader) { cl.certificateCheckInterval = d } } -// WithLogger assigns a logger for to use. -func WithLogger(logger *zap.Logger) TLSCertLoaderOpt { +// WithCertLoaderLogger assigns a logger for to use. +func WithCertLoaderLogger(logger *zap.Logger) TLSCertLoaderOpt { return func(cl *TLSCertLoader) { cl.logger = logger } } +// WithCertLoaderIgnoreFilePermissions skips file permission checking when loading certificates. +func WithCertLoaderIgnoreFilePermissions(ignore bool) TLSCertLoaderOpt { + return func(cl *TLSCertLoader) { + cl.ignoreFilePermissions = ignore + } +} + // NewTLSCertLoader creates a TLSCertLoader loaded with the certifcate found in certPath and keyPath. // Only trusted input (standard configuration files) should be used for certPath and keyPath. // If the certificate can not be loaded, an error is returned. On success, a monitor is setup to @@ -230,10 +267,8 @@ func NewTLSCertLoader(certPath, keyPath string, opts ...TLSCertLoaderOpt) (rCert } // Start monitoring certificate. - var monitorWg sync.WaitGroup - monitorWg.Add(1) - go cl.monitorCert(&monitorWg) - monitorWg.Wait() + cl.monitorStartWg.Add(1) + go cl.monitorCert(&cl.monitorStartWg) return cl, nil } @@ -308,16 +343,22 @@ func (cl *TLSCertLoader) GetCertificate(*tls.ClientHelloInfo) (*tls.Certificate, // certificate. func (cl *TLSCertLoader) GetClientCertificate(cri *tls.CertificateRequestInfo) (*tls.Certificate, error) { if cri == nil { - return new(tls.Certificate), ErrCertificateRequestInfoNil + return new(tls.Certificate), fmt.Errorf("tls client: %w", ErrCertificateRequestInfoNil) } cert := cl.Certificate() if cert == nil { - return new(tls.Certificate), ErrCertificateNil + return new(tls.Certificate), fmt.Errorf("tls client: %w", ErrCertificateNil) } - if err := cri.SupportsCertificate(cert); err != nil { - return new(tls.Certificate), err + + // Will our certificate be accepted by server? + if err := cri.SupportsCertificate(cert); err == nil { + return cert, nil } - return cert, nil + + // We don't have a certificate that would be accepted by the server. Don't return an error. + // This replicates Go's behavior when tls.Config.Certificates is used instead of GetClientCertificate + // and gives a better error on both the client and server side. + return new(tls.Certificate), nil } // Leaf returns the parsed x509 certificate of the currently loaded certificate. @@ -328,13 +369,17 @@ func (cl *TLSCertLoader) Leaf() *x509.Certificate { return cl.leaf } +func (cl *TLSCertLoader) loadCertificate(certPath, keyPath string) (LoadedCertificate, error) { + return LoadCertificate(certPath, keyPath, WithLoadCertificateIgnoreFilePermissions(cl.ignoreFilePermissions)) +} + // Load loads the certificate at the given certificate path and private keyfile path. // Only trusted input (standard configuration files) should be used for certPath and keyPath. func (cl *TLSCertLoader) Load(certPath, keyPath string) (rErr error) { log, logEnd := logger.NewOperation(cl.logger, "Loading TLS certificate", "tls_load_cert", zap.String("cert", certPath), zap.String("key", keyPath)) defer logEnd() - loadedCert, err := LoadCertificate(certPath, keyPath) + loadedCert, err := cl.loadCertificate(certPath, keyPath) cl.mu.Lock() defer cl.mu.Unlock() @@ -365,7 +410,7 @@ func (cl *TLSCertLoader) Load(certPath, keyPath string) (rErr error) { // If the certificate can be loaded, a function that will apply the certificate reload is // returned. Otherwise, an error is returned. func (cl *TLSCertLoader) PrepareLoad(certPath, keyPath string) (func() error, error) { - loadedCert, err := LoadCertificate(certPath, keyPath) + loadedCert, err := cl.loadCertificate(certPath, keyPath) if err != nil { return nil, err } @@ -457,6 +502,12 @@ func (cl *TLSCertLoader) checkCurrentCert() { } } +// WaitForMonitorStart will wait for the certificate monitor goroutine to start. This is mainly useful +// for tests to avoid race conditions. +func (cl *TLSCertLoader) WaitForMonitorStart() { + cl.monitorStartWg.Wait() +} + // monitorCert periodically logs errors with the currently loaded certificate. func (cl *TLSCertLoader) monitorCert(wg *sync.WaitGroup) { cl.logger.Info("Starting TLS certificate monitor") diff --git a/pkg/tlsconfig/certconfig_test.go b/pkg/tlsconfig/certconfig_test.go index 4a78051a77f..b818b6ca26a 100644 --- a/pkg/tlsconfig/certconfig_test.go +++ b/pkg/tlsconfig/certconfig_test.go @@ -25,12 +25,14 @@ func TestTLSCertLoader_HappyPath(t *testing.T) { logger := zap.New(core) // Start cert loader - cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithLogger(logger)) + cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithCertLoaderLogger(logger)) require.NoError(t, err) require.NotNil(t, cl) defer func() { require.NoError(t, cl.Close()) }() + cl.WaitForMonitorStart() + cl.WaitForMonitorStart() // should be able to safely call multiple times { // Check for expected log output @@ -108,12 +110,13 @@ func TestTLSCertLoader_GoodCertPersists(t *testing.T) { logger := zap.New(core) // Start cert loader - cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithLogger(logger)) + cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithCertLoaderLogger(logger)) require.NoError(t, err) require.NotNil(t, cl) defer func() { require.NoError(t, cl.Close()) }() + cl.WaitForMonitorStart() var goodSerial big.Int { @@ -144,7 +147,7 @@ func TestTLSCertLoader_GoodCertPersists(t *testing.T) { require.NoError(t, emptyFile.Close()) loadErr := cl.Load(emptyPath, emptyPath) - require.ErrorContains(t, loadErr, "error loading x509 key pair: tls: failed to find any PEM data in certificate input") + require.ErrorContains(t, loadErr, fmt.Sprintf("error loading x509 key pair (%q / %q): tls: failed to find any PEM data in certificate input", emptyPath, emptyPath)) // Check that we are still using the previously loaded certificate cp, kp := cl.Paths() @@ -281,12 +284,13 @@ func TestTLSCertLoader_PrematureCertificateLogging(t *testing.T) { core, logs := observer.New(zapcore.InfoLevel) logger := zap.New(core) - cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithLogger(logger), WithCertificateCheckInterval(testCheckTime)) + cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithCertLoaderLogger(logger), WithCertLoaderCertificateCheckInterval(testCheckTime)) require.NoError(t, err) require.NotNil(t, cl) defer func() { require.NoError(t, cl.Close()) }() + cl.WaitForMonitorStart() checkWarning := func(t *testing.T) { warning := logs.FilterMessage("Certificate is not valid yet").TakeAll() @@ -313,12 +317,13 @@ func TestTLSCertLoader_ExpiredCertificateLogging(t *testing.T) { core, logs := observer.New(zapcore.InfoLevel) logger := zap.New(core) - cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithLogger(logger), WithCertificateCheckInterval(testCheckTime)) + cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithCertLoaderLogger(logger), WithCertLoaderCertificateCheckInterval(testCheckTime)) require.NoError(t, err) require.NotNil(t, cl) defer func() { require.NoError(t, cl.Close()) }() + cl.WaitForMonitorStart() checkWarning := func(t *testing.T) { warning := logs.FilterMessage("Certificate is expired").TakeAll() @@ -346,12 +351,16 @@ func TestTLSCertLoader_CertificateExpiresSoonLogging(t *testing.T) { core, logs := observer.New(zapcore.InfoLevel) logger := zap.New(core) - cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, WithLogger(logger), WithCertificateCheckInterval(testCheckTime), WithExpirationAdvanced(2*24*time.Hour)) + cl, err := NewTLSCertLoader(ss.CertPath, ss.KeyPath, + WithCertLoaderLogger(logger), + WithCertLoaderCertificateCheckInterval(testCheckTime), + WithCertLoaderExpirationAdvanced(2*24*time.Hour)) require.NoError(t, err) require.NotNil(t, cl) defer func() { require.NoError(t, cl.Close()) }() + cl.WaitForMonitorStart() checkWarning := func(t *testing.T) { warning := logs.FilterMessage("Certificate will expire soon").TakeAll() @@ -534,8 +543,10 @@ func TestTLSCertLoader_GetClientCertificate(t *testing.T) { }, } + // We should get an empty certificate with no error. This replicates Go's behavior when + // tls.Config.Certificates is used and none of the certificates are accepted by the server. cert, err := cl.GetClientCertificate(cri) - require.ErrorContains(t, err, "doesn't support any of the certificate's signature algorithms") + require.NoError(t, err) // GetClientCertificate must return a non-nil certificate even on error // (per the tls.Config.GetClientCertificate contract). require.NotNil(t, cert) @@ -591,8 +602,10 @@ func TestTLSCertLoader_GetClientCertificate(t *testing.T) { AcceptableCAs: [][]byte{parsedCA2.RawSubject}, } + // This should return an empty certificate with no error to replicate + // Go's behavior when tls.Config.Certificates is used. cert, err := cl.GetClientCertificate(cri) - require.ErrorContains(t, err, "not signed by an acceptable CA") + require.NoError(t, err) require.NotNil(t, cert) require.Empty(t, cert.Certificate) }) diff --git a/pkg/tlsconfig/configmanager.go b/pkg/tlsconfig/configmanager.go new file mode 100644 index 00000000000..5835b232acb --- /dev/null +++ b/pkg/tlsconfig/configmanager.go @@ -0,0 +1,386 @@ +package tlsconfig + +import ( + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "net" + "os" + "time" + + "go.uber.org/zap" +) + +var ( + // ErrNoCertLoader indicates that an operation requiring a TLSCertLoader did not have one available. + // This can happen if the TLSConfigManager was created without a certificate for client-side use only. + ErrNoCertLoader = errors.New("no TLSCertLoader available") +) + +// TLSConfigManager will manage a TLS configuration and make sure that only one instance of its tls.Config exists. +// Different TLSConfigManager objects will have different configurations, even if they are instantiated in exactly +// the same way. No struct member is modified once the NewTLSConfigManager constructor is finished. +type TLSConfigManager struct { + tlsConfig *tls.Config + certLoader *TLSCertLoader +} + +// caConfig contains configuration to configure a TLS CA config. +type caConfig struct { + // custom indicates if caConfig specifies a custom CA config. If false, + // the rest of the configuration is ignored. + custom bool + + // includeSystem indicates if system root CA should be included. + includeSystem bool + + // files lists paths to PEM files to include in CA. + files []string +} + +// setIncludeSystem sets includeSystem and marks the config as custom. +func (c *caConfig) setIncludeSystem(include bool) { + c.custom = true + c.includeSystem = include +} + +// addFiles adds a list of files to be included in CA configuration and marks config as custom. +func (c *caConfig) addFiles(files ...string) { + c.custom = true + c.files = append(c.files, files...) +} + +// newCertPool returns a x509.CertPool for the configuration in c. If c is not a custom config, then +// nil is returned. +func (c *caConfig) newCertPool() (*x509.CertPool, error) { + // Only create a CertPool for a custom CA config. + if !c.custom { + return nil, nil + } + + // Create new CertPool, with system CA store if requested. + var cp *x509.CertPool + if c.includeSystem { + var err error + cp, err = x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("error getting system CA pool during newCertPool: %w", err) + } + } else { + cp = x509.NewCertPool() + } + + // Add PEM files to CA store. + for _, fn := range c.files { + pem, err := os.ReadFile(fn) + if err != nil { + return nil, fmt.Errorf("error reading file %q for CA store: %w", fn, err) + } + if ok := cp.AppendCertsFromPEM(pem); !ok { + return nil, fmt.Errorf("error adding certificates from %q to CA store: no valid certificates found", fn) + } + } + + return cp, nil +} + +// tlsConfigManagerConfig holds all options for a TLSConfigManager. +type tlsConfigManagerConfig struct { + // useTLS indicates if TLS should be used. If not, the rest of the configuration is ignored. + useTLS bool + + // baseConfig is the *tls.Config to use as the basis for the manager's *tls.Config. + baseConfig *tls.Config + + // certPath is the path to the server certificate. + certPath string + + // keyPath is the path to the server private key. + keyPath string + + // allowInsecure indicates if certificate checks should be ignored. + allowInsecure bool + + // rootCAConfig is the root CA config. + rootCAConfig caConfig + + // clientCAConfig is the CA config for servers to use for verifying client certificates. + clientCAConfig caConfig + + // clientAuth indicates the type of ClientAuth required by a server. + clientAuth tls.ClientAuthType + + // certLoaderOpts are options for the underlying TLSCertLoader. + certLoaderOpts []TLSCertLoaderOpt +} + +// addCertLoaderOpt adds a TLSCertLoaderOpt to the configuration for the TLSCertLoader. +func (cl *tlsConfigManagerConfig) addCertLoaderOpt(o TLSCertLoaderOpt) { + cl.certLoaderOpts = append(cl.certLoaderOpts, o) +} + +// TLSConfigManagerOpt is an option for use with NewTLSConfigManager and related constructors. +type TLSConfigManagerOpt func(*tlsConfigManagerConfig) + +// WithUseTLS sets if the config manager should use TLS. +func WithUseTLS(useTLS bool) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.useTLS = useTLS + } +} + +// WithBaseConfig sets the config manager's base *tls.Config. +func WithBaseConfig(baseConfig *tls.Config) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.baseConfig = baseConfig + } +} + +// WithCertificate sets the config manager's certificate and private key path. +func WithCertificate(certPath, keyPath string) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.certPath = certPath + cp.keyPath = keyPath + } +} + +// WithAllowInsecure sets if the config manager should allow insecure TLS. +func WithAllowInsecure(allowInsecure bool) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.allowInsecure = allowInsecure + } +} + +// WithRootCAIncludeSystem specifies if the system CA should be included in the root CA. +func WithRootCAIncludeSystem(includeSystem bool) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.rootCAConfig.setIncludeSystem(includeSystem) + } +} + +// WithRootCAFiles specifies a list of paths to PEM files that contain root CAs. +func WithRootCAFiles(pemFiles ...string) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.rootCAConfig.addFiles(pemFiles...) + } +} + +// WithClientCAIncludeSystem specifies if the system CA should be included in the client CA for client authentication. +func WithClientCAIncludeSystem(includeSystem bool) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.clientCAConfig.setIncludeSystem(includeSystem) + } +} + +// WithClientCAFiles specifies a list of paths to PEM files that contain root CA for client authentication. +func WithClientCAFiles(pemFiles ...string) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.clientCAConfig.addFiles(pemFiles...) + } +} + +// WithClientAuth specifies the type TLS client authentication a server should perform. +func WithClientAuth(auth tls.ClientAuthType) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.clientAuth = auth + } +} + +// WithExpirationAdvanced sets the how far ahead the underlying CertLoader will +// warn about a certificate that is about to expire. +func WithExpirationAdvanced(d time.Duration) TLSConfigManagerOpt { + return func(cp *tlsConfigManagerConfig) { + cp.addCertLoaderOpt(WithCertLoaderExpirationAdvanced(d)) + } +} + +// WithCertificateCheckInterval sets how often to check for certificate expiration. +func WithCertificateCheckInterval(d time.Duration) TLSConfigManagerOpt { + return func(cl *tlsConfigManagerConfig) { + cl.addCertLoaderOpt(WithCertLoaderCertificateCheckInterval(d)) + } +} + +// WithLogger assigns a logger for to use. +func WithLogger(logger *zap.Logger) TLSConfigManagerOpt { + return func(cl *tlsConfigManagerConfig) { + cl.addCertLoaderOpt(WithCertLoaderLogger(logger)) + } +} + +// WithIgnoreFilePermissions ignores file permissions when loading certificates. +func WithIgnoreFilePermissions(ignore bool) TLSConfigManagerOpt { + return func(cl *tlsConfigManagerConfig) { + cl.addCertLoaderOpt(WithCertLoaderIgnoreFilePermissions(ignore)) + } +} + +// newTLSConfigManager returns a TLSConfigManager configured by opts. +func newTLSConfigManager(opts ...TLSConfigManagerOpt) (*TLSConfigManager, error) { + c := tlsConfigManagerConfig{} + for _, o := range opts { + o(&c) + } + + // Create and setup base tls.Config + var tlsConfig *tls.Config + var certLoader *TLSCertLoader + if c.useTLS { + // Create / clone TLS configuration as necessary. + tlsConfig = c.baseConfig.Clone() // nil configs are clonable. + if tlsConfig == nil { + tlsConfig = new(tls.Config) + } + + // Modify configuration. + tlsConfig.InsecureSkipVerify = c.allowInsecure + + // Only overwrite default value of ClientAuth. + if tlsConfig.ClientAuth == tls.NoClientCert { + tlsConfig.ClientAuth = c.clientAuth + } + + // Setup CA pools. + setupPool := func(pool **x509.CertPool, c caConfig) error { + if p, err := c.newCertPool(); err != nil { + return err + } else if p != nil { + // Don't overwrite an existing pool from baseConfig with a nil pool. + *pool = p + } + return nil + } + + if err := setupPool(&tlsConfig.RootCAs, c.rootCAConfig); err != nil { + return nil, fmt.Errorf("error creating root CA pool: %w", err) + } + if err := setupPool(&tlsConfig.ClientCAs, c.clientCAConfig); err != nil { + return nil, fmt.Errorf("error creating client CA pool: %w", err) + } + + // Create TLSCertLoader and configure it. + // 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 c.certPath != "" || c.keyPath != "" { + if cl, err := NewTLSCertLoader(c.certPath, c.keyPath, c.certLoaderOpts...); err != nil { + return nil, err + } else { + certLoader = cl + } + certLoader.SetupTLSConfig(tlsConfig) + } + } + + return &TLSConfigManager{ + tlsConfig: tlsConfig, + certLoader: certLoader, + }, nil +} + +// NewTLSConfigManager returns a TLSConfigManager with the given configuration. If useTLS is true, +// then the certificate is loaded immediately if specified and the tls.Config instantiated. +// If no certPath and no keyPath is provided, then no TLSCertLoader is created. For this case, the returned +// 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, opts ...TLSConfigManagerOpt) (*TLSConfigManager, error) { + // Values from explicit parameters should override values set in opts. + co := make([]TLSConfigManagerOpt, 0, len(opts)+4) + co = append(co, opts...) + co = append(co, + WithUseTLS(useTLS), + WithBaseConfig(baseConfig), + WithCertificate(certPath, keyPath), + WithAllowInsecure(allowInsecure)) + return newTLSConfigManager(co...) +} + +// 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). +func NewClientTLSConfigManager(useTLS bool, baseConfig *tls.Config, allowInsecure bool, opts ...TLSConfigManagerOpt) (*TLSConfigManager, error) { + co := make([]TLSConfigManagerOpt, 0, len(opts)+3) + co = append(co, opts...) + co = append(co, + WithUseTLS(useTLS), + WithBaseConfig(baseConfig), + WithAllowInsecure(allowInsecure)) + return newTLSConfigManager(co...) +} + +// 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{} +} + +// TLSConfig returns a tls.Config for use with dial and listen functions. When TLS is disabled the return is nil. +// The returned tls.Config is a clone and does not need to be cloned again. +func (cm *TLSConfigManager) TLSConfig() *tls.Config { + // Clone returns nil for a nil tlsConfig + return cm.tlsConfig.Clone() +} + +// TLSCertLoader returns the certificate loader for this TLSConfigManager. When no certificate is provided +// the return value is nil. +func (cm *TLSConfigManager) TLSCertLoader() *TLSCertLoader { + return cm.certLoader +} + +// UseTLS returns true if this TLSConfigManager is configured to use TLS. It is a convenience wrapper +// around TLSConfig. +func (cm *TLSConfigManager) UseTLS() bool { + // Don't use TLSConfig() to avoid cloning a tlsConfig only to throw it away. + return cm.tlsConfig != nil +} + +// Return a net.Listener for network and address based on current configuration. +func (cm *TLSConfigManager) Listen(network, address string) (net.Listener, error) { + if tlsConfig := cm.TLSConfig(); tlsConfig != nil { + return tls.Listen(network, address, tlsConfig) + } else { + return net.Listen(network, address) + } +} + +// Dial a remote for network and addressing using the current configuration. +func (cm *TLSConfigManager) Dial(network, address string) (net.Conn, error) { + if tlsConfig := cm.TLSConfig(); tlsConfig != nil { + return tls.Dial(network, address, tlsConfig) + } else { + return net.Dial(network, address) + } +} + +// Dial a remote for network and addressing using the given dialer and current configuration. +func (cm *TLSConfigManager) DialWithDialer(dialer *net.Dialer, network, address string) (net.Conn, error) { + if tlsConfig := cm.TLSConfig(); tlsConfig != nil { + return tls.DialWithDialer(dialer, network, address, tlsConfig) + } else { + return dialer.Dial(network, address) + } +} + +// PrepareCertificateLoad is a wrapper for the TLSCertLoader's PrepareLoad method. If TLS is not +// enabled, then a NOP callback is returned. +func (cm *TLSConfigManager) PrepareCertificateLoad(certPath, keyPath string) (func() error, error) { + if !cm.UseTLS() { + return func() error { return nil }, nil + } + + if certLoader := cm.TLSCertLoader(); certLoader != nil { + return certLoader.PrepareLoad(certPath, keyPath) + } else { + return nil, ErrNoCertLoader + } +} + +// Close closes the underlying TLSCertLoader, if present. This is safe to call multiple times. +func (cm *TLSConfigManager) Close() error { + if cm.certLoader != nil { + return cm.certLoader.Close() + } + return nil +} diff --git a/pkg/tlsconfig/configmanager_test.go b/pkg/tlsconfig/configmanager_test.go new file mode 100644 index 00000000000..d338b47008f --- /dev/null +++ b/pkg/tlsconfig/configmanager_test.go @@ -0,0 +1,1572 @@ +package tlsconfig + +import ( + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "net" + "os" + "path" + "testing" + "time" + + "github.com/influxdata/influxdb/pkg/testing/selfsigned" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +func TestTLSConfigManager_ConsistentClonedConfigs(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Create an initial baseConfig for manager. + baseTLSConfig := ss.ClientTLSConfig(t, false, false) + require.NotNil(t, baseTLSConfig.RootCAs, "ClientTLSConfig should have set RootCAs") + + manager, err := NewTLSConfigManager(true, baseTLSConfig, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Get TLS config + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + + // The manager's TLS config and baseTLSConfig should not be the same and and should not be shared, + // but we should be able to set that the config was cloned from baseTLSConfig by looking at RootCAs. + require.NotSame(t, tlsConfig, baseTLSConfig) + require.NotEqual(t, tlsConfig, baseTLSConfig) + require.NotNil(t, tlsConfig.RootCAs) + require.Equal(t, baseTLSConfig.RootCAs, tlsConfig.RootCAs) + + // Subsequent calls should return different instances that are equal. + tlsConfig2 := manager.TLSConfig() + require.NotSame(t, tlsConfig, tlsConfig2) + // We can't compare the function pointers directly, just that they are non-nil. + // Clear out the function pointers before calling require.Equal. + require.NotNil(t, tlsConfig.GetCertificate) + require.NotNil(t, tlsConfig2.GetCertificate) + require.NotNil(t, tlsConfig.GetClientCertificate) + require.NotNil(t, tlsConfig2.GetClientCertificate) + tlsConfig.GetCertificate = nil + tlsConfig.GetClientCertificate = nil + tlsConfig2.GetCertificate = nil + tlsConfig2.GetClientCertificate = nil + require.Equal(t, tlsConfig, tlsConfig2) + + // TLSCertLoader should also be available + certLoader := manager.TLSCertLoader() + require.NotNil(t, certLoader) + + // Subsequent calls should return the same instance + certLoader2 := manager.TLSCertLoader() + require.Same(t, certLoader, certLoader2) +} + +func TestTLSConfigManager_BaseConfigCloned(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + baseConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + MaxVersion: tls.VersionTLS13, + ServerName: "test.example.com", + } + + manager, err := NewTLSConfigManager(true, baseConfig, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + + // Verify base config values are preserved + require.Equal(t, tls.VersionTLS12, int(tlsConfig.MinVersion)) + require.Equal(t, tls.VersionTLS13, int(tlsConfig.MaxVersion)) + require.Equal(t, "test.example.com", tlsConfig.ServerName) + + // Verify that modifying the original base config doesn't affect the loaded config + baseConfig.ServerName = "modified.example.com" + require.Equal(t, "test.example.com", tlsConfig.ServerName) + + // Verify that loaded config is a different instance + require.NotSame(t, baseConfig, tlsConfig) +} + +func TestTLSConfigManager_NilBaseConfig(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + + // Should have default zero values for a new tls.Config + require.Equal(t, uint16(0), tlsConfig.MinVersion) + require.Equal(t, uint16(0), tlsConfig.MaxVersion) +} + +func TestTLSConfigManager_CertLoaderCallbacksSet(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + + // Verify that TLSCertLoader.SetupTLSConfig was called by checking callbacks are set + require.NotNil(t, tlsConfig.GetCertificate, "GetCertificate callback should be set") + require.NotNil(t, tlsConfig.GetClientCertificate, "GetClientCertificate callback should be set") +} + +func TestTLSConfigManager_CertLoaderPathsCorrect(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + certLoader := manager.TLSCertLoader() + require.NotNil(t, certLoader) + + // Verify the paths were passed correctly to TLSCertLoader + certPath, keyPath := certLoader.Paths() + require.Equal(t, ss.CertPath, certPath) + require.Equal(t, ss.KeyPath, keyPath) +} + +func TestTLSConfigManager_ConstructorError(t *testing.T) { + // Use non-existent paths to verify NewTLSConfigLoader returns error + manager, err := NewTLSConfigManager(true, nil, "/nonexistent/cert.pem", "/nonexistent/key.pem", false) + require.ErrorContains(t, err, "LoadCertificate: error opening \"/nonexistent/cert.pem\" for reading: open /nonexistent/cert.pem: no such file or directory") + require.Nil(t, manager) +} + +func TestTLSConfigManager_InsecureSkipVerify(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + t.Run("allowInsecure true", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.True(t, tlsConfig.InsecureSkipVerify) + }) + + t.Run("allowInsecure false", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.False(t, tlsConfig.InsecureSkipVerify) + }) + + t.Run("overrides base config", func(t *testing.T) { + baseConfig := &tls.Config{ + InsecureSkipVerify: true, + } + + manager, err := NewTLSConfigManager(true, baseConfig, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.False(t, tlsConfig.InsecureSkipVerify, "allowInsecure should override base config") + }) +} + +func TestTLSConfigManager_MultipleLoadersIndependent(t *testing.T) { + ss1 := selfsigned.NewSelfSignedCert(t, selfsigned.WithDNSName("server1.example.com")) + ss2 := selfsigned.NewSelfSignedCert(t, selfsigned.WithDNSName("server2.example.com")) + + loader1, err := NewTLSConfigManager(true, nil, ss1.CertPath, ss1.KeyPath, false) + require.NoError(t, err) + loader2, err := NewTLSConfigManager(true, nil, ss2.CertPath, ss2.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, loader1.Close()) + require.NoError(t, loader2.Close()) + }() + + tlsConfig1 := loader1.TLSConfig() + tlsConfig2 := loader2.TLSConfig() + + // Configs should be different instances + require.NotSame(t, tlsConfig1, tlsConfig2) + + // Cert loaders should be different instances + certLoader1 := loader1.TLSCertLoader() + certLoader2 := loader2.TLSCertLoader() + require.NotSame(t, certLoader1, certLoader2) + + // Each manager should have its own certificate paths + cp1, kp1 := certLoader1.Paths() + cp2, kp2 := certLoader2.Paths() + require.Equal(t, ss1.CertPath, cp1) + require.Equal(t, ss1.KeyPath, kp1) + require.Equal(t, ss2.CertPath, cp2) + require.Equal(t, ss2.KeyPath, kp2) +} + +func TestTLSConfigManager_UseTLSFalse(t *testing.T) { + t.Run("returns nil config and no error", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.Nil(t, tlsConfig) + require.False(t, manager.UseTLS()) + }) + + t.Run("returns nil cert manager and no error", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + certLoader := manager.TLSCertLoader() + require.Nil(t, certLoader) + }) + + t.Run("ignores invalid paths when disabled", func(t *testing.T) { + // Even with nonexistent paths, useTLS=false should not error + manager, err := NewTLSConfigManager(false, nil, "/nonexistent/cert.pem", "/nonexistent/key.pem", false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.Nil(t, tlsConfig) + + certLoader := manager.TLSCertLoader() + require.Nil(t, certLoader) + }) + + t.Run("ignores base config when disabled", func(t *testing.T) { + baseConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + ServerName: "test.example.com", + } + + manager, err := NewTLSConfigManager(false, baseConfig, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.Nil(t, tlsConfig, "should return nil config even with base config provided") + }) + + t.Run("close works when disabled", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + + // Close should succeed + require.NoError(t, manager.Close()) + require.NoError(t, manager.Close()) // idempotent + }) + + t.Run("explicit parameters override opts", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "", "", false, WithUseTLS(true)) + require.NoError(t, err) + require.NotNil(t, manager) + require.False(t, manager.UseTLS()) + }) +} + +func TestTLSConfigManager_UseTLSWithoutCert(t *testing.T) { + t.Run("constructor succeeds with empty paths", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + require.NotNil(t, manager) + require.NoError(t, manager.Close()) + }) + + t.Run("returns non-nil TLSConfig", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.True(t, manager.UseTLS()) + }) + + t.Run("returns nil TLSCertLoader", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + certLoader := manager.TLSCertLoader() + require.Nil(t, certLoader) + }) + + t.Run("PrepareCertificateLoad returns ErrNoCertLoader", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + callback, err := manager.PrepareCertificateLoad("/any/cert.pem", "/any/key.pem") + require.ErrorIs(t, err, ErrNoCertLoader) + require.Nil(t, callback) + }) + + t.Run("Listen fails without certificate", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + _, err = manager.Listen("tcp", "127.0.0.1:0") + require.ErrorContains(t, err, "tls: neither Certificates, GetCertificate, nor GetConfigForClient set in Config") + }) + + t.Run("respects base config", func(t *testing.T) { + baseConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + ServerName: "test.example.com", + } + + manager, err := NewTLSConfigManager(true, baseConfig, "", "", false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.Equal(t, uint16(tls.VersionTLS12), tlsConfig.MinVersion) + require.Equal(t, "test.example.com", tlsConfig.ServerName) + }) + + t.Run("close works without cert manager", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + + require.NoError(t, manager.Close()) + require.NoError(t, manager.Close()) // idempotent + }) +} + +func TestTLSConfigManager_Close(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + t.Run("close after construction", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + + require.NoError(t, manager.Close()) + }) + + t.Run("close is idempotent", func(t *testing.T) { + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + + require.NoError(t, manager.Close()) + require.NoError(t, manager.Close()) + }) +} + +func TestTLSConfigManager_PrepareCertificateLoad(t *testing.T) { + t.Run("useTLS false returns NOP callback", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Should return a NOP callback even with invalid paths + callback, err := manager.PrepareCertificateLoad("/nonexistent/cert.pem", "/nonexistent/key.pem") + require.NoError(t, err) + require.NotNil(t, callback) + + // Executing the NOP callback should succeed + require.NoError(t, callback()) + }) + + t.Run("useTLS true with valid paths returns callback", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Prepare loading the same cert (valid paths) + callback, err := manager.PrepareCertificateLoad(ss.CertPath, ss.KeyPath) + require.NoError(t, err) + require.NotNil(t, callback) + + // Executing the callback should succeed + require.NoError(t, callback()) + }) + + t.Run("useTLS true with invalid paths returns error", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Prepare loading with nonexistent paths should fail + callback, err := manager.PrepareCertificateLoad("/nonexistent/cert.pem", "/nonexistent/key.pem") + require.ErrorContains(t, err, "LoadCertificate: error opening \"/nonexistent/cert.pem\" for reading: open /nonexistent/cert.pem: no such file or directory") + require.Nil(t, callback) + }) + + t.Run("callback applies new certificate", func(t *testing.T) { + ss1 := selfsigned.NewSelfSignedCert(t, selfsigned.WithDNSName("server1.example.com")) + ss2 := selfsigned.NewSelfSignedCert(t, selfsigned.WithDNSName("server2.example.com")) + + manager, err := NewTLSConfigManager(true, nil, ss1.CertPath, ss1.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Verify initial certificate paths + certLoader := manager.TLSCertLoader() + certPath, keyPath := certLoader.Paths() + require.Equal(t, ss1.CertPath, certPath) + require.Equal(t, ss1.KeyPath, keyPath) + + // Prepare and execute loading a different certificate + callback, err := manager.PrepareCertificateLoad(ss2.CertPath, ss2.KeyPath) + require.NoError(t, err) + require.NoError(t, callback()) + + // Verify certificate paths have been updated + certPath, keyPath = certLoader.Paths() + require.Equal(t, ss2.CertPath, certPath) + require.Equal(t, ss2.KeyPath, keyPath) + }) +} + +func TestTLSConfigManager_Listen(t *testing.T) { + testListenerConnection := func(t *testing.T, listener net.Listener, dial func(addr string) (net.Conn, error)) { + t.Helper() + + testData := []byte("hello from client") + + // Server: accept connection and read data + serverResult := make(chan error, 1) + serverData := make(chan []byte, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + serverResult <- err + return + } + + buf := make([]byte, len(testData)) + var n int + n, err = conn.Read(buf) + err = errors.Join(err, conn.Close()) + serverData <- buf[:n] + serverResult <- err + }() + + // Client: connect and send data + conn, err := dial(listener.Addr().String()) + require.NoError(t, err) + defer func() { + require.NoError(t, conn.Close()) + }() + _, err = conn.Write(testData) + require.NoError(t, err) + + require.NoError(t, <-serverResult) + require.Equal(t, testData, <-serverData) + } + + t.Run("useTLS false returns plain TCP listener", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + listener, err := manager.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + testListenerConnection(t, listener, func(addr string) (net.Conn, error) { + return net.Dial("tcp", addr) + }) + }) + + t.Run("useTLS true returns TLS listener", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + listener, err := manager.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + testListenerConnection(t, listener, func(addr string) (net.Conn, error) { + return tls.Dial("tcp", addr, &tls.Config{ + InsecureSkipVerify: true, + }) + }) + }) + + t.Run("invalid address returns error", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + _, err = manager.Listen("tcp", "invalid:address:format") + require.ErrorContains(t, err, "address invalid:address:format: too many colons in address") + }) +} + +func TestTLSConfigManager_Dial(t *testing.T) { + testDialConnection := func(t *testing.T, listener net.Listener, dial func(addr string) (net.Conn, error)) { + t.Helper() + + testData := []byte("hello from client") + + // Server: accept connection and read data + serverResult := make(chan error, 1) + serverData := make(chan []byte, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + serverResult <- err + return + } + + buf := make([]byte, len(testData)) + var n int + n, err = conn.Read(buf) + err = errors.Join(err, conn.Close()) + serverData <- buf[:n] + serverResult <- err + }() + + // Client: connect and send data + conn, err := dial(listener.Addr().String()) + require.NoError(t, err) + _, err = conn.Write(testData) + require.NoError(t, err) + defer func() { + require.NoError(t, conn.Close()) + }() + + require.NoError(t, <-serverResult) + require.Equal(t, testData, <-serverData) + } + + t.Run("useTLS false dials plain TCP", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Create plain TCP server + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + testDialConnection(t, listener, func(addr string) (net.Conn, error) { + return manager.Dial("tcp", addr) + }) + }) + + t.Run("useTLS true dials TLS", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Create TLS server + cert, err := tls.LoadX509KeyPair(ss.CertPath, ss.KeyPath) + require.NoError(t, err) + listener, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{cert}, + }) + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + testDialConnection(t, listener, func(addr string) (net.Conn, error) { + return manager.Dial("tcp", addr) + }) + }) + + t.Run("useTLS true without cert dials TLS", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Create manager without client cert (client-only mode) + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Create TLS server + cert, err := tls.LoadX509KeyPair(ss.CertPath, ss.KeyPath) + require.NoError(t, err) + listener, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{cert}, + }) + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + testDialConnection(t, listener, func(addr string) (net.Conn, error) { + return manager.Dial("tcp", addr) + }) + }) + + t.Run("invalid address returns error", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + _, err = manager.Dial("tcp", "invalid:address:format") + require.ErrorContains(t, err, "address invalid:address:format: too many colons in address") + }) +} + +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.False(t, disabled.UseTLS()) + + 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, err := NewClientTLSConfigManager(false, nil, false) + require.NoError(t, err) + 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.False(t, client.UseTLS()) + + require.NoError(t, client.Close()) + require.NoError(t, explicit.Close()) + }) + + t.Run("TLS enabled allowInsecure false", func(t *testing.T) { + client, err := NewClientTLSConfigManager(true, nil, false) + require.NoError(t, err) + 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.True(t, client.UseTLS()) + + require.NoError(t, client.Close()) + require.NoError(t, explicit.Close()) + }) + + t.Run("TLS enabled allowInsecure true", func(t *testing.T) { + client, err := NewClientTLSConfigManager(true, nil, true) + require.NoError(t, err) + 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, err := NewClientTLSConfigManager(true, baseConfig, false) + require.NoError(t, err) + 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, err := NewClientTLSConfigManager(true, baseConfig, false) + require.NoError(t, err) + 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()) + }) + + t.Run("explicit parameters override opts", func(t *testing.T) { + client, err := NewClientTLSConfigManager(false, nil, false, WithUseTLS(true)) + require.NoError(t, err) + require.NotNil(t, client) + require.False(t, client.UseTLS()) + }) +} + +func simpleEchoServer(serverDone chan error, listener net.Listener, bufSize int) (rErr error) { + defer func() { + serverDone <- rErr + }() + conn, err := listener.Accept() + if err != nil { + return fmt.Errorf("error in Accept: %w", err) + } + defer func() { + if err := conn.Close(); err != nil { + rErr = errors.Join(rErr, fmt.Errorf("error in Close: %w", err)) + } + }() + buf := make([]byte, bufSize) + n, err := conn.Read(buf) + if err != nil { + return fmt.Errorf("error in Read: %w", err) + } + if _, err = conn.Write(buf[:n]); err != nil { + return fmt.Errorf("error in Write: %w", err) + } + return nil +} + +func TestTLSConfigManager_WithRootCAFiles(t *testing.T) { + t.Run("sets RootCAs from file", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Build expected CertPool + expectedPool := x509.NewCertPool() + caCert, err := os.ReadFile(ss.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert)) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithRootCAFiles(ss.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.RootCAs) + require.True(t, tlsConfig.RootCAs.Equal(expectedPool), "RootCAs should match expected pool") + }) + + t.Run("client trusts server with CA", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Server manager with certificate + serverManager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, serverManager.Close()) + }() + + // Client manager trusting the CA that signed the server certificate + clientManager, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAFiles(ss.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, clientManager.Close()) + }() + + // Start server + listener, err := serverManager.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + // Server accepts and echoes + testData := []byte("hello") + serverDone := make(chan error, 1) + go simpleEchoServer(serverDone, listener, len(testData)) + + // Client connects with CA-trusted connection + conn, err := clientManager.Dial("tcp", listener.Addr().String()) + require.NoError(t, err) + defer func() { + require.NoError(t, conn.Close()) + }() + + n, err := conn.Write(testData) + require.NoError(t, err) + require.Equal(t, 5, n) + + buf := make([]byte, len(testData)) + n, err = conn.Read(buf) + require.NoError(t, err) + require.Equal(t, len(testData), n) + require.Equal(t, testData, buf) + + require.NoError(t, <-serverDone) + }) + + t.Run("client rejects server without CA", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Server manager + serverManager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, serverManager.Close()) + }() + + // Client manager with empty RootCA pool (explicitly set, no system CAs) + clientManager, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAIncludeSystem(false)) + require.NoError(t, err) + defer func() { + require.NoError(t, clientManager.Close()) + }() + + // Start server + listener, err := serverManager.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + // Server accepts and waits for handshake to complete (will fail due to client rejecting cert) + testData := []byte("test") + serverDone := make(chan error, 1) + go simpleEchoServer(serverDone, listener, len(testData)) + + // Client connection should fail certificate verification + conn, err := clientManager.Dial("tcp", listener.Addr().String()) + require.ErrorContains(t, err, "tls: failed to verify certificate: x509: certificate signed by unknown authority") + require.Nil(t, conn) + + // Server will see a handshake failure error (client sent bad_certificate alert) + serverErr := <-serverDone + require.ErrorContains(t, serverErr, "tls: bad certificate") + }) + + t.Run("multiple CA files", func(t *testing.T) { + ss1 := selfsigned.NewSelfSignedCert(t, selfsigned.WithCASubject("org1", "CA1")) + ss2 := selfsigned.NewSelfSignedCert(t, selfsigned.WithCASubject("org2", "CA2")) + + // Build expected CertPool with both CAs + expectedPool := x509.NewCertPool() + caCert1, err := os.ReadFile(ss1.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert1)) + caCert2, err := os.ReadFile(ss2.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert2)) + + manager, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAFiles(ss1.CACertPath, ss2.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.RootCAs) + require.True(t, tlsConfig.RootCAs.Equal(expectedPool), "RootCAs should contain both CAs") + }) + + t.Run("error on nonexistent file", func(t *testing.T) { + _, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAFiles("/nonexistent/ca.pem")) + require.ErrorIs(t, err, os.ErrNotExist) + require.ErrorContains(t, err, "error creating root CA pool: error reading file \"/nonexistent/ca.pem\" for CA store: open /nonexistent/ca.pem: no such file or directory") + }) + + t.Run("error on invalid PEM file", func(t *testing.T) { + // Create a temporary file with invalid PEM content + tmpDir := t.TempDir() + tmpFile := path.Join(tmpDir, "invalid.pem") + require.NoError(t, os.WriteFile(tmpFile, []byte("not a valid PEM file"), 0644)) + + manager, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAFiles(tmpFile)) + require.ErrorContains(t, err, "error creating root CA pool: error adding certificates from \""+tmpFile+"\" to CA store: no valid certificates found") + require.Nil(t, manager) + }) +} + +func TestTLSConfigManager_WithRootCAIncludeSystem(t *testing.T) { + t.Run("includes system CA pool", func(t *testing.T) { + // Get expected system pool + expectedPool, err := x509.SystemCertPool() + require.NoError(t, err) + + manager, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAIncludeSystem(true)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.RootCAs) + require.True(t, tlsConfig.RootCAs.Equal(expectedPool), "RootCAs should equal system pool") + }) + + t.Run("excludes system CA pool", func(t *testing.T) { + // Expected: empty pool + expectedPool := x509.NewCertPool() + + manager, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAIncludeSystem(false)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.RootCAs) + require.True(t, tlsConfig.RootCAs.Equal(expectedPool), "RootCAs should be empty pool") + }) + + t.Run("combined with CA files", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Build expected pool: system + custom CA + expectedPool, err := x509.SystemCertPool() + require.NoError(t, err) + caCert, err := os.ReadFile(ss.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert)) + + manager, err := NewClientTLSConfigManager(true, nil, false, + WithRootCAIncludeSystem(true), + WithRootCAFiles(ss.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.RootCAs) + require.True(t, tlsConfig.RootCAs.Equal(expectedPool), "RootCAs should contain system + custom CA") + }) +} + +func TestTLSConfigManager_WithClientCAFiles(t *testing.T) { + t.Run("sets ClientCAs from file", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Build expected CertPool + expectedPool := x509.NewCertPool() + caCert, err := os.ReadFile(ss.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert)) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientCAFiles(ss.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.ClientCAs) + require.True(t, tlsConfig.ClientCAs.Equal(expectedPool), "ClientCAs should match expected pool") + }) + + t.Run("server verifies client certificate", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Server manager that requires client certificates. + serverManager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientCAFiles(ss.CACertPath), + WithClientAuth(tls.RequireAndVerifyClientCert)) + require.NoError(t, err) + defer func() { + require.NoError(t, serverManager.Close()) + }() + + // Start server + listener, err := serverManager.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + // Server accepts and echoes + testData := []byte("hello") + serverDone := make(chan error, 1) + go simpleEchoServer(serverDone, listener, len(testData)) + + // Client config with client certificate. Ignore certificate validity from server, we're testing + // client certificate functionality here. + clientManager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, true) + require.NoError(t, err) + defer func() { + require.NoError(t, clientManager.Close()) + }() + conn, err := clientManager.Dial("tcp", listener.Addr().String()) + require.NoError(t, err) + defer func() { + require.NoError(t, conn.Close()) + }() + + n, err := conn.Write(testData) + require.NoError(t, err) + require.Equal(t, len(testData), n) + + buf := make([]byte, len(testData)) + n, err = conn.Read(buf) + require.NoError(t, err) + require.Equal(t, len(testData), n) + require.Equal(t, testData, buf) + + require.NoError(t, <-serverDone) + }) + + t.Run("server rejects client without valid certificate", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + otherSS := selfsigned.NewSelfSignedCert(t, selfsigned.WithCASubject("other", "Other CA")) + + // Server manager that requires client certificates + serverManager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientAuth(tls.RequireAndVerifyClientCert), + WithClientCAFiles(ss.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, serverManager.Close()) + }() + serverTLSConfig := serverManager.TLSConfig() + require.NotNil(t, serverTLSConfig) + require.Equal(t, tls.RequireAndVerifyClientCert, serverTLSConfig.ClientAuth) + + // Client with certificate signed by different CA. Ignore server certificate validity. We're + // checking client certificate functionality here. + clientManager, err := NewTLSConfigManager(true, nil, otherSS.CertPath, otherSS.KeyPath, true) + require.NoError(t, err) + defer func() { + require.NoError(t, clientManager.Close()) + }() + + // Start server + listener, err := serverManager.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + // Server accepts and waits for handshake (will fail due to untrusted client cert) + testData := []byte("hello") + serverDone := make(chan error, 1) + go simpleEchoServer(serverDone, listener, len(testData)) + + // Client connects - Dial may succeed but handshake fails during Read + conn, dialErr := clientManager.Dial("tcp", listener.Addr().String()) + if dialErr == nil { + defer func() { + require.NoError(t, conn.Close()) + }() + // Read to wait for server response - this will fail when server rejects our cert + buf := make([]byte, 1) + _, dialErr = conn.Read(buf) + } + // Client sees remote error when server requires cert but client sends untrusted cert. + // (TLS client doesn't send cert if it's not signed by a CA the server trusts) + require.ErrorContains(t, dialErr, "remote error: tls: certificate required") + + // Server sees that client didn't provide a certificate (because client's cert + // wasn't signed by a CA in server's ClientCAs, so TLS stack didn't send it) + serverErr := <-serverDone + require.ErrorContains(t, serverErr, "tls: client didn't provide a certificate") + }) + + t.Run("multiple CA files", func(t *testing.T) { + ss1 := selfsigned.NewSelfSignedCert(t, selfsigned.WithCASubject("org1", "CA1")) + ss2 := selfsigned.NewSelfSignedCert(t, selfsigned.WithCASubject("org2", "CA2")) + + // Build expected CertPool with both CAs + expectedPool := x509.NewCertPool() + caCert1, err := os.ReadFile(ss1.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert1)) + caCert2, err := os.ReadFile(ss2.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert2)) + + manager, err := NewTLSConfigManager(true, nil, ss1.CertPath, ss1.KeyPath, false, + WithClientCAFiles(ss1.CACertPath, ss2.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.ClientCAs) + require.True(t, tlsConfig.ClientCAs.Equal(expectedPool), "ClientCAs should contain both CAs") + }) + + t.Run("error on nonexistent file", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + _, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientCAFiles("/nonexistent/ca.pem")) + require.ErrorIs(t, err, os.ErrNotExist) + require.ErrorContains(t, err, "error creating client CA pool: error reading file \"/nonexistent/ca.pem\" for CA store: open /nonexistent/ca.pem: no such file or directory") + }) + + t.Run("error on invalid PEM file", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Create a temporary file with invalid PEM content + tmpDir := t.TempDir() + tmpFile := path.Join(tmpDir, "invalid.pem") + require.NoError(t, os.WriteFile(tmpFile, []byte("not a valid PEM file"), 0644)) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientCAFiles(tmpFile)) + require.ErrorContains(t, err, "error creating client CA pool: error adding certificates from \""+tmpFile+"\" to CA store: no valid certificates found") + require.Nil(t, manager) + }) +} + +func TestTLSConfigManager_WithClientCAIncludeSystem(t *testing.T) { + t.Run("includes system CA pool", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Get expected system pool + expectedPool, err := x509.SystemCertPool() + require.NoError(t, err) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientCAIncludeSystem(true)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.ClientCAs) + require.True(t, tlsConfig.ClientCAs.Equal(expectedPool), "ClientCAs should equal system pool") + }) + + t.Run("excludes system CA pool", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Expected: empty pool + expectedPool := x509.NewCertPool() + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientCAIncludeSystem(false)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.ClientCAs) + require.True(t, tlsConfig.ClientCAs.Equal(expectedPool), "ClientCAs should be empty pool") + }) + + t.Run("combined with CA files", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + // Build expected pool: system + custom CA + expectedPool, err := x509.SystemCertPool() + require.NoError(t, err) + caCert, err := os.ReadFile(ss.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(caCert)) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithClientCAIncludeSystem(true), + WithClientCAFiles(ss.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.ClientCAs) + require.True(t, tlsConfig.ClientCAs.Equal(expectedPool), "ClientCAs should contain system + custom CA") + }) +} + +func TestTLSConfigManager_CAOptionsNotSetByDefault(t *testing.T) { + // When no CA options are specified, the TLS config should not have + // custom CA pools set (allowing Go's default behavior) + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.Nil(t, tlsConfig.RootCAs, "RootCAs should be nil when no CA options specified") + require.Nil(t, tlsConfig.ClientCAs, "ClientCAs should be nil when no CA options specified") +} + +func TestTLSConfigManager_CAOptionsWithBaseConfig(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + anotherSS := selfsigned.NewSelfSignedCert(t, selfsigned.WithCASubject("another", "Another CA")) + + // Create a base config with existing RootCAs + basePool := x509.NewCertPool() + baseCACert, err := os.ReadFile(ss.CACertPath) + require.NoError(t, err) + require.True(t, basePool.AppendCertsFromPEM(baseCACert)) + + baseConfig := &tls.Config{ + RootCAs: basePool, + } + + // Build expected pool from anotherSS CA only + expectedPool := x509.NewCertPool() + anotherCACert, err := os.ReadFile(anotherSS.CACertPath) + require.NoError(t, err) + require.True(t, expectedPool.AppendCertsFromPEM(anotherCACert)) + + // Create manager with different CA file - should override base config + manager, err := NewClientTLSConfigManager(true, baseConfig, false, + WithRootCAFiles(anotherSS.CACertPath)) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + require.NotNil(t, tlsConfig.RootCAs) + // The RootCAs should match the new CA, not the base config + require.True(t, tlsConfig.RootCAs.Equal(expectedPool), "RootCAs should be overridden by WithRootCAFiles") + require.False(t, tlsConfig.RootCAs.Equal(basePool), "RootCAs should not equal base pool") +} + +// testManagerCheckTime is the TLS certificate check time in logging tests. +const testManagerCheckTime = 333 * time.Millisecond + +// testManagerCheckCapture time is how long to capture logs during logging tests. To prevent flaky tests, +// it should be more than testManagerCheckTime, but less than 2 * testManagerCheckTime. +const testManagerCheckCapture = 500 * time.Millisecond + +func TestTLSConfigManager_WithCertLoaderOptions(t *testing.T) { + // This test verifies that WithLogger, WithCertificateCheckInterval, and WithExpirationAdvanced + // properly pass through to the underlying TLSCertLoader. + + // Create a certificate that expires in 24 hours + notBefore := time.Now().UTC().Truncate(time.Minute).Add(-7 * 24 * time.Hour) + notAfter := time.Now().UTC().Truncate(time.Hour).Add(24 * time.Hour) + + ss := selfsigned.NewSelfSignedCert(t, selfsigned.WithNotBefore(notBefore), selfsigned.WithNotAfter(notAfter)) + + core, logs := observer.New(zapcore.InfoLevel) + logger := zap.New(core) + + // Create TLSConfigManager with all three options: + // - WithLogger: to capture log output + // - WithCertificateCheckInterval: to set a short check interval + // - WithExpirationAdvanced: to set the expiration warning window to 2 days (> 24 hours remaining) + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithLogger(logger), + WithCertificateCheckInterval(testManagerCheckTime), + WithExpirationAdvanced(2*24*time.Hour)) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Wait for the certificate monitor to start + certLoader := manager.TLSCertLoader() + require.NotNil(t, certLoader) + certLoader.WaitForMonitorStart() + + // Verify the "Certificate will expire soon" warning is logged + checkWarning := func(t *testing.T) { + t.Helper() + warning := logs.FilterMessage("Certificate will expire soon").TakeAll() + require.Len(t, warning, 1) + require.Equal(t, zap.WarnLevel, warning[0].Level) + require.Equal(t, ss.CertPath, warning[0].ContextMap()["cert"]) + require.Equal(t, ss.KeyPath, warning[0].ContextMap()["key"]) + require.Equal(t, notAfter, warning[0].ContextMap()["NotAfter"]) + untilExpires, ok := warning[0].ContextMap()["untilExpires"].(time.Duration) + require.True(t, ok, "untilExpires should be a time.Duration") + timeExpires := time.Now().Add(untilExpires) + require.WithinDuration(t, notAfter, timeExpires, 2*time.Minute, "untilExpires varies more than expected") + logs.TakeAll() // dump all logs + } + checkWarning(t) + + // Check for warning during monitor (after another check interval) + require.Zero(t, logs.Len(), "init logs not dumped properly") + time.Sleep(testManagerCheckCapture) + checkWarning(t) +} + +func TestTLSConfigManager_WithIgnoreFilePermissions(t *testing.T) { + t.Run("fails without ignore when cert permissions too open", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + require.NoError(t, os.Chmod(ss.CertPath, 0660)) + _, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.ErrorContains(t, err, fmt.Sprintf("LoadCertificate: file permissions are too open: for %q, maximum is 0644 (-rw-r--r--) but found 0660 (-rw-rw----); extra permissions: 0020 (-----w----)", ss.CertPath)) + }) + + t.Run("succeeds with ignore when cert permissions too open", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + require.NoError(t, os.Chmod(ss.CertPath, 0660)) + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithIgnoreFilePermissions(true)) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Verify the manager works correctly + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + certLoader := manager.TLSCertLoader() + require.NotNil(t, certLoader) + }) + + t.Run("fails without ignore when key permissions too open", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + require.NoError(t, os.Chmod(ss.KeyPath, 0644)) + _, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false) + require.ErrorContains(t, err, fmt.Sprintf("LoadCertificate: file permissions are too open: for %q, maximum is 0600 (-rw-------) but found 0644 (-rw-r--r--); extra permissions: 0044 (----r--r--)", ss.KeyPath)) + }) + + t.Run("succeeds with ignore when key permissions too open", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + require.NoError(t, os.Chmod(ss.KeyPath, 0644)) + manager, err := NewTLSConfigManager(true, nil, ss.CertPath, ss.KeyPath, false, + WithIgnoreFilePermissions(true)) + require.NoError(t, err) + require.NotNil(t, manager) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Verify the manager works correctly + tlsConfig := manager.TLSConfig() + require.NotNil(t, tlsConfig) + certLoader := manager.TLSCertLoader() + require.NotNil(t, certLoader) + }) +} + +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() + + testData := []byte("hello from client") + + // Server: accept connection and read data + serverResult := make(chan error, 1) + serverData := make(chan []byte, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + serverResult <- err + return + } + + buf := make([]byte, len(testData)) + var n int + n, err = conn.Read(buf) + err = errors.Join(err, conn.Close()) + serverData <- buf[:n] + serverResult <- err + }() + + // Bind to a specific local address to verify the dialer is used + localAddr, err := net.ResolveTCPAddr("tcp", "127.0.0.1:0") + require.NoError(t, err) + dialer := &net.Dialer{LocalAddr: localAddr} + + // Client: connect and send data + conn, err := dial(dialer, listener.Addr().String()) + require.NoError(t, err) + + // Verify the connection's local address is from 127.0.0.1 (dialer's LocalAddr) + localTCPAddr, ok := conn.LocalAddr().(*net.TCPAddr) + require.True(t, ok) + require.Equal(t, "127.0.0.1", localTCPAddr.IP.String()) + + _, err = conn.Write(testData) + require.NoError(t, err) + defer func() { + require.NoError(t, conn.Close()) + }() + + require.NoError(t, <-serverResult) + require.Equal(t, testData, <-serverData) + } + + t.Run("dialer LocalAddr is used for plain TCP", func(t *testing.T) { + manager, err := NewTLSConfigManager(false, nil, "/any/cert.pem", "/any/key.pem", false) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Create plain TCP server + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + testDialWithDialerConnection(t, listener, func(dialer *net.Dialer, addr string) (net.Conn, error) { + return manager.DialWithDialer(dialer, "tcp", addr) + }) + }) + + t.Run("dialer LocalAddr is used for TLS", func(t *testing.T) { + ss := selfsigned.NewSelfSignedCert(t) + + manager, err := NewTLSConfigManager(true, nil, "", "", true) + require.NoError(t, err) + defer func() { + require.NoError(t, manager.Close()) + }() + + // Create TLS server + cert, err := tls.LoadX509KeyPair(ss.CertPath, ss.KeyPath) + require.NoError(t, err) + listener, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{cert}, + }) + require.NoError(t, err) + defer func() { + require.NoError(t, listener.Close()) + }() + + testDialWithDialerConnection(t, listener, func(dialer *net.Dialer, addr string) (net.Conn, error) { + return manager.DialWithDialer(dialer, "tcp", addr) + }) + }) +} diff --git a/services/httpd/config.go b/services/httpd/config.go index 1145f7969f0..667a467860a 100644 --- a/services/httpd/config.go +++ b/services/httpd/config.go @@ -72,27 +72,28 @@ type Config struct { // NewConfig returns a new Config with default settings. func NewConfig() Config { return Config{ - Enabled: true, - FluxEnabled: false, - FluxLogEnabled: false, - FluxTesting: false, - BindAddress: DefaultBindAddress, - LogEnabled: true, - PprofEnabled: true, - PprofAuthEnabled: false, - DebugPprofEnabled: false, - PingAuthEnabled: false, - PromReadAuthEnabled: false, - HTTPSEnabled: false, - HTTPSCertificate: "/etc/ssl/influxdb.pem", - MaxRowLimit: 0, - Realm: DefaultRealm, - UnixSocketEnabled: false, - UnixSocketPermissions: 0777, - BindSocket: DefaultBindSocket, - MaxBodySize: DefaultMaxBodySize, - EnqueuedWriteTimeout: DefaultEnqueuedWriteTimeout, - AccessLogStatusFilters: make([]StatusFilter, 0), + Enabled: true, + FluxEnabled: false, + FluxLogEnabled: false, + FluxTesting: false, + BindAddress: DefaultBindAddress, + LogEnabled: true, + PprofEnabled: true, + PprofAuthEnabled: false, + DebugPprofEnabled: false, + PingAuthEnabled: false, + PromReadAuthEnabled: false, + HTTPSEnabled: false, + HTTPSCertificate: "/etc/ssl/influxdb.pem", + HTTPSInsecureCertificate: false, + MaxRowLimit: 0, + Realm: DefaultRealm, + UnixSocketEnabled: false, + UnixSocketPermissions: 0777, + BindSocket: DefaultBindSocket, + MaxBodySize: DefaultMaxBodySize, + EnqueuedWriteTimeout: DefaultEnqueuedWriteTimeout, + AccessLogStatusFilters: make([]StatusFilter, 0), } } @@ -105,13 +106,14 @@ func (c Config) Diagnostics() (*diagnostics.Diagnostics, error) { } return diagnostics.RowFromMap(map[string]interface{}{ - "enabled": true, - "bind-address": c.BindAddress, - "https-enabled": c.HTTPSEnabled, - "max-row-limit": c.MaxRowLimit, - "max-connection-limit": c.MaxConnectionLimit, - "access-log-path": c.AccessLogPath, - "flux-enabled": c.FluxEnabled, + "enabled": true, + "bind-address": c.BindAddress, + "https-enabled": c.HTTPSEnabled, + "https-insecure-certificate": c.HTTPSInsecureCertificate, + "max-row-limit": c.MaxRowLimit, + "max-connection-limit": c.MaxConnectionLimit, + "access-log-path": c.AccessLogPath, + "flux-enabled": c.FluxEnabled, }), nil } diff --git a/services/httpd/service.go b/services/httpd/service.go index 711321e3f4c..5fb97b43f99 100644 --- a/services/httpd/service.go +++ b/services/httpd/service.go @@ -77,6 +77,9 @@ type Service struct { // exposed to client code because a key certificate might be loaded on a config reload. key string + // insecureCert is true if certificate file permissions should be ignored. + insecureCert bool + limit int tlsConfig *tls.Config err chan error @@ -95,7 +98,7 @@ type Service struct { ln net.Listener unixSocketListener net.Listener - certLoader *tlsconfig.TLSCertLoader + tlsManager *tlsconfig.TLSConfigManager // tcpServerStarted indicates if httpServer is started for ln, which in turn indicates who is // responsible for closing ln. @@ -118,6 +121,7 @@ func NewService(c Config) *Service { https: c.HTTPSEnabled, cert: c.HTTPSCertificate, key: c.HTTPSPrivateKey, + insecureCert: c.HTTPSInsecureCertificate, limit: c.MaxConnectionLimit, tlsConfig: c.TLS, err: make(chan error, 2), // There could be two serve calls that fail. @@ -155,29 +159,18 @@ func (s *Service) Open() error { s.Handler.Open() // Open listener. - if s.https { - if certLoader, err := tlsconfig.NewTLSCertLoader(s.cert, s.key, tlsconfig.WithLogger(s.Logger)); err == nil { - s.certLoader = certLoader - } else { - return err - } - - tlsConfig := s.tlsConfig.Clone() - s.certLoader.SetupTLSConfig(tlsConfig) - - listener, err := tls.Listen("tcp", s.addr, tlsConfig) - if err != nil { - return err - } + tm, err := tlsconfig.NewTLSConfigManager(s.https, s.tlsConfig, s.cert, s.key, false, + tlsconfig.WithAllowInsecure(s.insecureCert), + tlsconfig.WithLogger(s.Logger)) + if err != nil { + return fmt.Errorf("httpd: error creating TLS manager: %w", err) + } + s.tlsManager = tm - s.ln = listener + if ln, err := s.tlsManager.Listen("tcp", s.addr); err != nil { + return fmt.Errorf("httpd: error creating listener: %w", err) } else { - listener, err := net.Listen("tcp", s.addr) - if err != nil { - return err - } - - s.ln = listener + s.ln = ln } s.Logger.Info("Listening on HTTP", zap.Stringer("addr", s.ln.Addr()), @@ -281,9 +274,9 @@ func (s *Service) doClose() error { } } - if s.certLoader != nil { - // It is safe to call certLoader.Close multiple times. - if err := s.certLoader.Close(); err != nil { + if s.tlsManager != nil { + // It is safe to call tlsManager.Close multiple times. + if err := s.tlsManager.Close(); err != nil { return err } } @@ -310,15 +303,15 @@ func (s *Service) PrepareReloadConfig(c Config) (func() error, error) { } if s.https { - // Sanity check to make sure we have a certLoader. It's possible this could happen if a + // Sanity check to make sure we have a tlsManager. It's possible this could happen if a // reload signal is sent to the process after NewService but before Open. By returning an // error here the reload will fail and no changes will be made. - if s.certLoader == nil { - return nil, errors.New("httpd: no certLoader available") + if s.tlsManager == nil { + return nil, errors.New("httpd: no TLS manager available") } // Make sure the specified certificate will load correctly and return an apply function. - if apply, err := s.certLoader.PrepareLoad(c.HTTPSCertificate, c.HTTPSPrivateKey); err == nil { + if apply, err := s.tlsManager.PrepareCertificateLoad(c.HTTPSCertificate, c.HTTPSPrivateKey); err == nil { return apply, nil } else { return nil, fmt.Errorf("httpd: error loading certificate at (%q, %q): %w", c.HTTPSCertificate, c.HTTPSPrivateKey, err) diff --git a/services/opentsdb/config.go b/services/opentsdb/config.go index d7004993a28..8d3bc5d9dd4 100644 --- a/services/opentsdb/config.go +++ b/services/opentsdb/config.go @@ -37,19 +37,20 @@ const ( // Config represents the configuration of the OpenTSDB service. type Config struct { - Enabled bool `toml:"enabled"` - BindAddress string `toml:"bind-address"` - Database string `toml:"database"` - RetentionPolicy string `toml:"retention-policy"` - ConsistencyLevel string `toml:"consistency-level"` - TLSEnabled bool `toml:"tls-enabled"` - Certificate string `toml:"certificate"` - PrivateKey string `toml:"private-key"` - BatchSize int `toml:"batch-size"` - BatchPending int `toml:"batch-pending"` - BatchTimeout toml.Duration `toml:"batch-timeout"` - LogPointErrors bool `toml:"log-point-errors"` - TLS *tls.Config `toml:"-"` + Enabled bool `toml:"enabled"` + BindAddress string `toml:"bind-address"` + Database string `toml:"database"` + RetentionPolicy string `toml:"retention-policy"` + ConsistencyLevel string `toml:"consistency-level"` + TLSEnabled bool `toml:"tls-enabled"` + Certificate string `toml:"certificate"` + PrivateKey string `toml:"private-key"` + InsecureCertificate bool `toml:"insecure-certificate"` + BatchSize int `toml:"batch-size"` + BatchPending int `toml:"batch-pending"` + BatchTimeout toml.Duration `toml:"batch-timeout"` + LogPointErrors bool `toml:"log-point-errors"` + TLS *tls.Config `toml:"-"` } // NewConfig returns a new config for the service. diff --git a/services/opentsdb/service.go b/services/opentsdb/service.go index 0de3867376e..363e937e52c 100644 --- a/services/opentsdb/service.go +++ b/services/opentsdb/service.go @@ -50,12 +50,13 @@ type Service struct { ln net.Listener // main listener httpln *chanListener // http channel-based listener - wg sync.WaitGroup - tls bool - certLoader *tlsconfig.TLSCertLoader - tlsConfig *tls.Config - cert string - privateKey string + wg sync.WaitGroup + tls bool + tlsManager *tlsconfig.TLSConfigManager + tlsConfig *tls.Config + cert string + privateKey string + insecureCert bool mu sync.RWMutex ready bool // Has the required database been created? @@ -95,6 +96,7 @@ func NewService(c Config) (*Service, error) { tlsConfig: d.TLS, cert: d.Certificate, privateKey: d.PrivateKey, + insecureCert: d.InsecureCertificate, BindAddress: d.BindAddress, Database: d.Database, RetentionPolicy: d.RetentionPolicy, @@ -133,30 +135,20 @@ func (s *Service) Open() error { go func() { defer s.wg.Done(); s.processBatches(s.batcher) }() // Open listener. - if s.tls { - certLoader, err := tlsconfig.NewTLSCertLoader(s.cert, s.privateKey, tlsconfig.WithLogger(s.Logger)) - if err != nil { - return err - } - s.certLoader = certLoader - - tlsConfig := s.tlsConfig.Clone() - s.certLoader.SetupTLSConfig(tlsConfig) - - listener, err := tls.Listen("tcp", s.BindAddress, tlsConfig) - if err != nil { - return err - } - - s.ln = listener - } else { - listener, err := net.Listen("tcp", s.BindAddress) - if err != nil { - return err - } + cm, err := tlsconfig.NewTLSConfigManager(s.tls, s.tlsConfig, s.cert, s.privateKey, false, + tlsconfig.WithIgnoreFilePermissions(s.insecureCert), + tlsconfig.WithLogger(s.Logger)) + if err != nil { + return fmt.Errorf("opentsdb: error creating TLS manager: %w", err) + } + s.tlsManager = cm - s.ln = listener + ln, err := s.tlsManager.Listen("tcp", s.BindAddress) + if err != nil { + return fmt.Errorf("opentsdb: error creating listener: %w", err) } + s.ln = ln + s.Logger.Info("Listening on TCP", zap.Stringer("addr", s.ln.Addr()), zap.Bool("tls", s.tls)) @@ -188,10 +180,10 @@ func (s *Service) Close() error { if err := s.httpln.Close(); err != nil { return false, err } - if s.certLoader != nil { - cl := s.certLoader - s.certLoader = nil - if err := cl.Close(); err != nil { + if s.tlsManager != nil { + tm := s.tlsManager + s.tlsManager = nil + if err := tm.Close(); err != nil { return false, err } } @@ -222,13 +214,13 @@ func (s *Service) PrepareReloadTLSCertificates() (func() error, error) { return nil, nil } - // Sanity check that we have a certLoader. - if s.certLoader == nil { + // Sanity check that we have a tlsManager. + if s.tlsManager == nil { // This shouldn't happen. - return nil, errors.New("opentsdb: no certLoader available") + return nil, errors.New("opentsdb: no TLS manager available") } - if apply, err := s.certLoader.PrepareLoad(s.cert, s.privateKey); err == nil { + if apply, err := s.tlsManager.PrepareCertificateLoad(s.cert, s.privateKey); err == nil { return apply, nil } else { return nil, fmt.Errorf("opentsdb: TLS certificate reload failed (%q, %q): %w", s.cert, s.privateKey, err)