Merge pull request #224 from ydb-platform/tls-config

define tls.Config explicitly instead redefine default TLS config options

Author: asmyasnikov

CHANGELOG.md

@@ -1,3 +1,6 @@
+* Added `WithTLSConfig` option for redefine TLS config
+* Added `sugar.LoadCertificatesFromFile` and `sugar.LoadCertificatesFromPem` helpers
+
 ## v3.22.0
 * Supported `json.Unmarshaler` type for scanning row to values 
 * Reimplement `sugar.DSN` with `net/url`

config/config.go

@@ -24,45 +24,46 @@ import (
 type Config struct {
 	config.Common
 
-	trace                            trace.Driver
-	dialTimeout                      time.Duration
-	connectionTTL                    time.Duration
-	balancer                         balancer.Balancer
-	secure                           bool
-	endpoint                         string
-	database                         string
-	requestsType                     string
-	userAgent                        string
+	trace         trace.Driver
+	dialTimeout   time.Duration
+	connectionTTL time.Duration
+	balancer      balancer.Balancer
+	secure        bool
+	endpoint      string
+	database      string
+	requestsType  string
+	userAgent     string
+	grpcOptions   []grpc.DialOption
+	credentials   credentials.Credentials
+	tlsConfig     *tls.Config
+	meta          meta.Meta
+
 	excludeGRPCCodesForPessimization []grpcCodes.Code
-	grpcOptions                      []grpc.DialOption
-	credentials                      credentials.Credentials
-	tlsConfig                        *tls.Config
-	meta                             meta.Meta
 }
 
 // ExcludeGRPCCodesForPessimization defines grpc codes for exclude its from pessimization trigger
 func (c Config) ExcludeGRPCCodesForPessimization() []grpcCodes.Code {
 	return c.excludeGRPCCodesForPessimization
 }
 
-// GrpcDialOptions is an custom client grpc dial options which will appends to
-// default grpc dial options
+// GrpcDialOptions reports about used grpc dialing options
 func (c Config) GrpcDialOptions() []grpc.DialOption {
 	return c.grpcOptions
 }
 
-// Meta is an internal option which contains meta information about database connection
+// Meta reports meta information about database connection
 func (c Config) Meta() meta.Meta {
 	return c.meta
 }
 
-// ConnectionTTL is a time to live of a connection
-// If ConnectionTTL is zero then TTL is not used.
+// ConnectionTTL defines interval for parking grpc connections.
+//
+// If ConnectionTTL is zero - connections are not park.
 func (c Config) ConnectionTTL() time.Duration {
 	return c.connectionTTL
 }
 
-// Secure is an flag for secure connection
+// Secure is a flag for secure connection
 func (c Config) Secure() bool {
 	return c.secure
 }
@@ -72,6 +73,7 @@ func (c Config) Endpoint() string {
 	return c.endpoint
 }
 
+// TLSConfig reports about TLS configuration
 func (c Config) TLSConfig() *tls.Config {
 	return c.tlsConfig
 }
@@ -89,7 +91,7 @@ func (c Config) Database() string {
 	return c.database
 }
 
-// Credentials is an ydb client credentials.
+// Credentials is a ydb client credentials.
 // In most cases Credentials are required.
 func (c Config) Credentials() credentials.Credentials {
 	return c.credentials
@@ -116,7 +118,7 @@ type Option func(c *Config)
 
 // WithInternalDNSResolver
 //
-// Deprecated: already used internal dns-resolver
+// Deprecated: always used internal dns-resolver
 func WithInternalDNSResolver() Option {
 	return func(c *Config) {}
 }
@@ -127,6 +129,9 @@ func WithEndpoint(endpoint string) Option {
 	}
 }
 
+// WithSecure changes secure connection flag.
+//
+// Warning: if secure is false - TLS config options has no effect.
 func WithSecure(secure bool) Option {
 	return func(c *Config) {
 		c.secure = secure
@@ -139,12 +144,22 @@ func WithDatabase(database string) Option {
 	}
 }
 
+// WithCertificate appends certificate to TLS config root certificates
 func WithCertificate(certificate *x509.Certificate) Option {
 	return func(c *Config) {
 		c.tlsConfig.RootCAs.AddCert(certificate)
 	}
 }
 
+// WithTLSConfig replaces older TLS config
+//
+// Warning: all early changes of TLS config will be lost
+func WithTLSConfig(tlsConfig *tls.Config) Option {
+	return func(c *Config) {
+		c.tlsConfig = tlsConfig
+	}
+}
+
 func WithTrace(t trace.Driver, opts ...trace.DriverComposeOption) Option {
 	return func(c *Config) {
 		c.trace = c.trace.Compose(t, opts...)
@@ -218,12 +233,14 @@ func WithRequestsType(requestsType string) Option {
 	}
 }
 
+// WithMinTLSVersion applies minimum TLS version that is acceptable.
 func WithMinTLSVersion(minVersion uint16) Option {
 	return func(c *Config) {
 		c.tlsConfig.MinVersion = minVersion
 	}
 }
 
+// WithTLSSInsecureSkipVerify applies InsecureSkipVerify flag to TLS config
 func WithTLSSInsecureSkipVerify() Option {
 	return func(c *Config) {
 		c.tlsConfig.InsecureSkipVerify = true
@@ -267,32 +284,28 @@ func New(opts ...Option) Config {
 	return c
 }
 
-func certPool() (certPool *x509.CertPool) {
-	defer func() {
-		// on darwin system panic raced on checking system security
-		if e := recover(); e != nil {
-			certPool = x509.NewCertPool()
-		}
-	}()
-	var err error
-	certPool, err = x509.SystemCertPool()
-	if err != nil {
-		certPool = x509.NewCertPool()
+func certPool() *x509.CertPool {
+	certPool, err := x509.SystemCertPool()
+	if err == nil {
+		return certPool
+	}
+	return x509.NewCertPool()
+}
+
+func defaultTLSConfig() *tls.Config {
+	return &tls.Config{
+		MinVersion: tls.VersionTLS12,
+		RootCAs:    certPool(),
 	}
-	return
 }
 
 func defaultConfig() (c Config) {
 	return Config{
 		credentials: credentials.NewAnonymousCredentials(
 			credentials.WithSourceInfo("default"),
 		),
-		balancer: balancers.Default(),
-		secure:   true,
-		tlsConfig: &tls.Config{
-			MinVersion: tls.VersionTLS12,
-			RootCAs:    certPool(),
-		},
+		balancer:  balancers.Default(),
+		tlsConfig: defaultTLSConfig(),
 		grpcOptions: []grpc.DialOption{
 			grpc.WithContextDialer(
 				func(ctx context.Context, address string) (net.Conn, error) {

internal/dsn/dsn.go

@@ -16,13 +16,8 @@ var (
 			}, nil
 		},
 	}
-	schemasSecure = map[string]bool{
-		"":      true,
-		"grpcs": true,
-		"grpc":  false,
-	}
-	errSchemeNotValid = xerrors.Wrap(fmt.Errorf("schema not valid"))
-	errParserExists   = xerrors.Wrap(fmt.Errorf("already exists parser. newest parser replaced old. param"))
+	insecureSchema  = "grpc"
+	errParserExists = xerrors.Wrap(fmt.Errorf("already exists parser. newest parser replaced old. param"))
 )
 
 type Parser func(value string) ([]config.Option, error)
@@ -41,15 +36,10 @@ func Parse(dsn string) (options []config.Option, err error) {
 	if err != nil {
 		return nil, xerrors.WithStackTrace(err)
 	}
-	if _, has := schemasSecure[uri.Scheme]; !has {
-		return nil, xerrors.WithStackTrace(
-			fmt.Errorf("%w: %v", errSchemeNotValid, uri.Scheme),
-		)
-	}
 	options = append(
 		options,
 		config.WithEndpoint(uri.Host),
-		config.WithSecure(schemasSecure[uri.Scheme]),
+		config.WithSecure(uri.Scheme != insecureSchema),
 	)
 	for param, values := range uri.Query() {
 		if p, has := parsers[param]; has {

internal/dsn/dsn_test.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/ydb-platform/ydb-go-sdk/v3/config"
 	"github.com/ydb-platform/ydb-go-sdk/v3/internal/credentials"
-	"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
 	"github.com/ydb-platform/ydb-go-sdk/v3/testutil"
 )
 
@@ -27,7 +26,6 @@ func TestParseConnectionString(t *testing.T) {
 		endpoint         string
 		database         string
 		token            string
-		error            error
 	}{
 		{
 			"grpc://ydb-ru.yandex.net:2135/?" +
@@ -36,7 +34,6 @@ func TestParseConnectionString(t *testing.T) {
 			"ydb-ru.yandex.net:2135",
 			"/ru/home/gvit/mydb",
 			"123",
-			nil,
 		},
 		{
 			"grpcs://ydb.serverless.yandexcloud.net:2135/?" +
@@ -45,7 +42,6 @@ func TestParseConnectionString(t *testing.T) {
 			"ydb.serverless.yandexcloud.net:2135",
 			"/ru-central1/b1g8skpblkos03malf3s/etn02qso4v3isjb00te1",
 			"123",
-			nil,
 		},
 		{
 			"grpcs://lb.etn03r9df42nb631unbv.ydb.mdb.yandexcloud.net:2135/?" +
@@ -54,20 +50,18 @@ func TestParseConnectionString(t *testing.T) {
 			"lb.etn03r9df42nb631unbv.ydb.mdb.yandexcloud.net:2135",
 			"/ru-central1/b1g8skpblkos03malf3s/etn03r9df42nb631unbv",
 			"123",
-			nil,
 		},
 		{
 			"abcd://ydb-ru.yandex.net:2135/?database=/ru/home/gvit/mydb",
 			true,
+			"ydb-ru.yandex.net:2135",
+			"/ru/home/gvit/mydb",
 			"",
-			"",
-			"",
-			errSchemeNotValid,
 		},
 	} {
 		t.Run(test.connectionString, func(t *testing.T) {
 			options, err := Parse(test.connectionString)
-			if !xerrors.Is(err, test.error) {
+			if err != nil {
 				t.Fatalf("Received unexpected error:\n%+v", err)
 			}
 			config := config.New(options...)

options.go

@@ -2,6 +2,7 @@ package ydb
 
 import (
 	"context"
+	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
@@ -109,17 +110,17 @@ func WithDatabase(database string) Option {
 
 // WithSecure defines secure option
 //
-// Warning: use WithConnectionString or dsn package instead
+// Warning: if secure is false - TLS config options has no effect.
 func WithSecure(secure bool) Option {
 	return func(ctx context.Context, c *connection) error {
 		c.options = append(c.options, config.WithSecure(secure))
 		return nil
 	}
 }
 
-// WithInsecure defines secure option
+// WithInsecure defines secure option.
 //
-// Warning: use WithConnectionString or dsn package instead
+// Warning: WithInsecure lost current TLS config.
 func WithInsecure() Option {
 	return func(ctx context.Context, c *connection) error {
 		c.options = append(c.options, config.WithSecure(false))
@@ -135,6 +136,7 @@ func WithMinTLSVersion(minVersion uint16) Option {
 	}
 }
 
+// WithTLSSInsecureSkipVerify applies InsecureSkipVerify flag to TLS config
 func WithTLSSInsecureSkipVerify() Option {
 	return func(ctx context.Context, c *connection) error {
 		c.options = append(c.options, config.WithTLSSInsecureSkipVerify())
@@ -143,6 +145,7 @@ func WithTLSSInsecureSkipVerify() Option {
 }
 
 // WithLogger add enables logging for selected tracing events.
+//
 // See trace package documentation for details.
 func WithLogger(details trace.Details, opts ...LoggerOption) Option {
 	loggerOpts := make([]logger.Option, 0, len(opts))
@@ -205,7 +208,7 @@ func WithDialTimeout(timeout time.Duration) Option {
 }
 
 // With collects additional configuration options.
-// This option does not replace collected option, instead it will appen provided options.
+// This option does not replace collected option, instead it will append provided options.
 func With(options ...config.Option) Option {
 	return func(ctx context.Context, c *connection) error {
 		c.options = append(c.options, options...)
@@ -241,15 +244,15 @@ func WithTraceDriver(trace trace.Driver, opts ...trace.DriverComposeOption) Opti
 	}
 }
 
-// WithCertificate provides custom CA certificate.
+// WithCertificate appends certificate to TLS config root certificates
 func WithCertificate(cert *x509.Certificate) Option {
 	return func(ctx context.Context, c *connection) error {
 		c.options = append(c.options, config.WithCertificate(cert))
 		return nil
 	}
 }
 
-// WithCertificate provides filepath to load custom CA certificates.
+// WithCertificatesFromFile appends certificates by filepath to TLS config root certificates
 func WithCertificatesFromFile(caFile string) Option {
 	return func(ctx context.Context, c *connection) error {
 		if len(caFile) > 0 && caFile[0] == '~' {
@@ -270,7 +273,18 @@ func WithCertificatesFromFile(caFile string) Option {
 	}
 }
 
-// WithCertificate provides PEM encoded custom CA certificates.
+// WithTLSConfig replaces older TLS config
+//
+// Warning: all early TLS config changes (such as WithCertificate, WithCertificatesFromFile, WithCertificatesFromPem,
+// WithMinTLSVersion, WithTLSSInsecureSkipVerify) will be lost
+func WithTLSConfig(tlsConfig *tls.Config) Option {
+	return func(ctx context.Context, c *connection) error {
+		c.options = append(c.options, config.WithTLSConfig(tlsConfig))
+		return nil
+	}
+}
+
+// WithCertificatesFromPem appends certificates by filepath to TLS config root certificates
 func WithCertificatesFromPem(bytes []byte) Option {
 	return func(ctx context.Context, c *connection) error {
 		if ok, err := func(bytes []byte) (ok bool, err error) {