SNOW-1821505 Introduce disableOCSPChecks config option (#1253)

Author: sfc-gh-pfus

connection.go

@@ -787,7 +787,7 @@ func buildSnowflakeConn(ctx context.Context, config Config) (*snowflakeConn, err
 	}
 	var st http.RoundTripper = SnowflakeTransport
 	if sc.cfg.Transporter == nil {
-		if sc.cfg.InsecureMode {
+		if sc.cfg.DisableOCSPChecks || sc.cfg.InsecureMode {
 			// no revocation check with OCSP. Think twice when you want to enable this option.
 			st = snowflakeInsecureTransport
 		} else {

connection_configuration.go

@@ -133,7 +133,10 @@ func handleSingleParam(cfg *Config, key string, value interface{}) error {
 			return err
 		}
 		err = determineAuthenticatorType(cfg, v)
+	case "disableocspchecks":
+		cfg.DisableOCSPChecks, err = parseBool(value)
 	case "insecuremode":
+		logInsecureModeDeprecationInfo()
 		cfg.InsecureMode, err = parseBool(value)
 	case "ocspfailopen":
 		var vv ConfigBool

connection_configuration_test.go

@@ -94,6 +94,7 @@ func TestLoadConnectionConfigForOAuth(t *testing.T) {
 	assertEqualF(t, cfg.Authenticator, AuthTypeOAuth)
 	assertEqualF(t, cfg.Token, "token_value")
 	assertEqualF(t, cfg.Port, 443)
+	assertEqualE(t, cfg.DisableOCSPChecks, true)
 }
 
 func TestReadTokenValueWithTokenFilePath(t *testing.T) {
@@ -110,6 +111,7 @@ func TestReadTokenValueWithTokenFilePath(t *testing.T) {
 	assertNilF(t, err, "The error should not occur")
 	assertEqualF(t, cfg.Authenticator, AuthTypeOAuth)
 	assertEqualF(t, cfg.Token, "mock_token123456")
+	assertEqualE(t, cfg.InsecureMode, true)
 }
 
 func TestLoadConnectionConfigWitNonExistingDSN(t *testing.T) {

doc.go

@@ -106,10 +106,12 @@ The following connection parameters are supported:
 
   - application: Identifies your application to Snowflake Support.
 
-  - insecureMode: false by default. Set to true to bypass the Online
+  - disableOCSPChecks: false by default. Set to true to bypass the Online
     Certificate Status Protocol (OCSP) certificate revocation check.
     IMPORTANT: Change the default value for testing or emergency situations only.
 
+  - insecureMode: deprecated. Use disableOCSPChecks instead.
+
   - token: a token that can be used to authenticate. Should be used in conjunction with the "oauth" authenticator.
 
   - client_session_keep_alive: Set to true have a heartbeat in the background every hour to keep the connection alive

driver_test.go

@@ -1791,6 +1791,22 @@ func TestOpenWithTransport(t *testing.T) {
 		t.Fatal("transport did not receive any requests")
 	}
 
+	// Test that transport override also works in OCSP checks disabled.
+	countingTransport.requests = 0
+	config.DisableOCSPChecks = true
+	db, err = driver.OpenWithConfig(context.Background(), *config)
+	if err != nil {
+		t.Fatalf("failed to open with config. config: %v, err: %v", config, err)
+	}
+	conn = db.(*snowflakeConn)
+	if conn.rest.Client.Transport != transport {
+		t.Fatal("transport doesn't match")
+	}
+	db.Close()
+	if countingTransport.requests == 0 {
+		t.Fatal("transport did not receive any requests")
+	}
+
 	// Test that transport override also works in insecure mode
 	countingTransport.requests = 0
 	config.InsecureMode = true

dsn.go

@@ -79,7 +79,9 @@ type Config struct {
 	ExternalBrowserTimeout time.Duration // Timeout for external browser login
 	MaxRetryCount          int           // Specifies how many times non-periodic HTTP request can be retried
 
-	Application  string           // application name.
+	Application       string // application name.
+	DisableOCSPChecks bool   // driver doesn't check certificate revocation status
+	// Deprecated: InsecureMode use DisableOCSPChecks instead
 	InsecureMode bool             // driver doesn't check certificate revocation status
 	OCSPFailOpen OCSPFailOpenMode // OCSP Fail Open
 
@@ -126,7 +128,7 @@ func (c *Config) Validate() error {
 
 // ocspMode returns the OCSP mode in string INSECURE, FAIL_OPEN, FAIL_CLOSED
 func (c *Config) ocspMode() string {
-	if c.InsecureMode {
+	if c.DisableOCSPChecks || c.InsecureMode {
 		return ocspModeInsecure
 	} else if c.OCSPFailOpen == ocspFailOpenNotSet || c.OCSPFailOpen == OCSPFailOpenTrue {
 		// by default or set to true
@@ -241,6 +243,9 @@ func DSN(cfg *Config) (dsn string, err error) {
 	if cfg.InsecureMode {
 		params.Add("insecureMode", strconv.FormatBool(cfg.InsecureMode))
 	}
+	if cfg.DisableOCSPChecks {
+		params.Add("disableOCSPChecks", strconv.FormatBool(cfg.DisableOCSPChecks))
+	}
 	if cfg.Tracing != "" {
 		params.Add("tracing", cfg.Tracing)
 	}
@@ -637,7 +642,14 @@ func parseParams(cfg *Config, posQuestion int, dsn string) (err error) {
 // parseDSNParams parses the DSN "query string". Values must be url.QueryEscape'ed
 func parseDSNParams(cfg *Config, params string) (err error) {
 	logger.Infof("Query String: %v\n", params)
-	for _, v := range strings.Split(params, "&") {
+	paramsSlice := strings.Split(params, "&")
+	insecureModeIdx := findByPrefix(paramsSlice, "insecureMode")
+	disableOCSPChecksIdx := findByPrefix(paramsSlice, "disableOCSPChecks")
+	if insecureModeIdx > -1 && disableOCSPChecksIdx > -1 {
+		logger.Warn("duplicated insecureMode and disableOCSPChecks. disableOCSPChecks takes precedence")
+		paramsSlice = append(paramsSlice[:insecureModeIdx-1], paramsSlice[insecureModeIdx+1:]...)
+	}
+	for _, v := range paramsSlice {
 		param := strings.SplitN(v, "=", 2)
 		if len(param) != 2 {
 			continue
@@ -715,12 +727,20 @@ func parseDSNParams(cfg *Config, params string) (err error) {
 				return err
 			}
 		case "insecureMode":
+			logInsecureModeDeprecationInfo()
 			var vv bool
 			vv, err = strconv.ParseBool(value)
 			if err != nil {
 				return
 			}
 			cfg.InsecureMode = vv
+		case "disableOCSPChecks":
+			var vv bool
+			vv, err = strconv.ParseBool(value)
+			if err != nil {
+				return
+			}
+			cfg.DisableOCSPChecks = vv
 		case "ocspFailOpen":
 			var vv bool
 			vv, err = strconv.ParseBool(value)
@@ -839,6 +859,10 @@ func parseDSNParams(cfg *Config, params string) (err error) {
 	return
 }
 
+func logInsecureModeDeprecationInfo() {
+	logger.Warn("insecureMode is deprecated. Use disableOCSPChecks instead.")
+}
+
 func parseTimeout(value string) (time.Duration, error) {
 	var vv int64
 	var err error

dsn_test.go

@@ -540,6 +540,58 @@ func TestParseDSN(t *testing.T) {
 			ocspMode: ocspModeInsecure,
 			err:      nil,
 		},
+		{
+			dsn: "u:p@a?database=d&schema=s&role=r&application=aa&authenticator=snowflake&disableOCSPChecks=true&passcode=pp&passcodeInPassword=true",
+			config: &Config{
+				Account: "a", User: "u", Password: "p",
+				Protocol: "https", Host: "a.snowflakecomputing.com", Port: 443,
+				Database: "d", Schema: "s", Role: "r", Authenticator: AuthTypeSnowflake, Application: "aa",
+				DisableOCSPChecks: true, Passcode: "pp", PasscodeInPassword: true,
+				OCSPFailOpen:              OCSPFailOpenTrue,
+				ValidateDefaultParameters: ConfigBoolTrue,
+				ClientTimeout:             defaultClientTimeout,
+				JWTClientTimeout:          defaultJWTClientTimeout,
+				ExternalBrowserTimeout:    defaultExternalBrowserTimeout,
+				IncludeRetryReason:        ConfigBoolTrue,
+			},
+			ocspMode: ocspModeInsecure,
+			err:      nil,
+		},
+		{
+			dsn: "u:p@a?database=d&schema=s&role=r&application=aa&authenticator=snowflake&disableOCSPChecks=true&passcode=pp&passcodeInPassword=true",
+			config: &Config{
+				Account: "a", User: "u", Password: "p",
+				Protocol: "https", Host: "a.snowflakecomputing.com", Port: 443,
+				Database: "d", Schema: "s", Role: "r", Authenticator: AuthTypeSnowflake, Application: "aa",
+				InsecureMode: false, DisableOCSPChecks: true, Passcode: "pp", PasscodeInPassword: true,
+				OCSPFailOpen:              OCSPFailOpenTrue,
+				ValidateDefaultParameters: ConfigBoolTrue,
+				ClientTimeout:             defaultClientTimeout,
+				JWTClientTimeout:          defaultJWTClientTimeout,
+				ExternalBrowserTimeout:    defaultExternalBrowserTimeout,
+				IncludeRetryReason:        ConfigBoolTrue,
+			},
+			ocspMode: ocspModeInsecure,
+			err:      nil,
+		},
+		// disableOCSPChecks should take precedence over insecureMode
+		{
+			dsn: "u:p@a?database=d&schema=s&role=r&application=aa&authenticator=snowflake&disableOCSPChecks=false&insecureMode=true&passcode=pp&passcodeInPassword=true",
+			config: &Config{
+				Account: "a", User: "u", Password: "p",
+				Protocol: "https", Host: "a.snowflakecomputing.com", Port: 443,
+				Database: "d", Schema: "s", Role: "r", Authenticator: AuthTypeSnowflake, Application: "aa",
+				DisableOCSPChecks: false, Passcode: "pp", PasscodeInPassword: true,
+				OCSPFailOpen:              OCSPFailOpenTrue,
+				ValidateDefaultParameters: ConfigBoolTrue,
+				ClientTimeout:             defaultClientTimeout,
+				JWTClientTimeout:          defaultJWTClientTimeout,
+				ExternalBrowserTimeout:    defaultExternalBrowserTimeout,
+				IncludeRetryReason:        ConfigBoolTrue,
+			},
+			ocspMode: ocspModeFailOpen,
+			err:      nil,
+		},
 		{
 			// schema should be ignored as no value is specified.
 			dsn: "u:p@a?database=d&schema",
@@ -1412,6 +1464,35 @@ func TestDSN(t *testing.T) {
 			},
 			dsn: "u:[email protected]:443?insecureMode=true&ocspFailOpen=true&validateDefaultParameters=true",
 		},
+		{
+			cfg: &Config{
+				User:              "u",
+				Password:          "p",
+				Account:           "a",
+				DisableOCSPChecks: true,
+			},
+			dsn: "u:[email protected]:443?disableOCSPChecks=true&ocspFailOpen=true&validateDefaultParameters=true",
+		},
+		{
+			cfg: &Config{
+				User:              "u",
+				Password:          "p",
+				Account:           "a",
+				InsecureMode:      true,
+				DisableOCSPChecks: false,
+			},
+			dsn: "u:[email protected]:443?insecureMode=true&ocspFailOpen=true&validateDefaultParameters=true",
+		},
+		{
+			cfg: &Config{
+				User:              "u",
+				Password:          "p",
+				Account:           "a",
+				InsecureMode:      false,
+				DisableOCSPChecks: true,
+			},
+			dsn: "u:[email protected]:443?disableOCSPChecks=true&ocspFailOpen=true&validateDefaultParameters=true",
+		},
 		{
 			cfg: &Config{
 				User:     "u",

ocsp.go

@@ -723,11 +723,7 @@ func canEarlyExitForOCSP(results []*ocspStatus, chainSize int) *ocspStatus {
 		}
 	}
 	if len(msg) > 0 {
-		logger.Warnf(
-			"WARNING!!! Using fail-open to connect. Driver is connecting to an "+
-				"HTTPS endpoint without OCSP based Certificate Revocation checking "+
-				"as it could not obtain a valid OCSP Response to use from the CA OCSP "+
-				"responder. Detail: %v", msg[1:])
+		logger.Debugf("OCSP responder didn't respond correctly. Assuming certificate is not revoked. Detail: %v", msg[1:])
 	}
 	return nil
 }

test_data/connections.toml

@@ -20,6 +20,7 @@ port = '443'
 authenticator = 'oauth'
 testNot = 'problematicParameter'
 token = 'token_value'
+disableOCSPChecks = true
 
 [aws-oauth-file]
 account = 'snowdriverswarsaw.us-west-2.aws'
@@ -43,4 +44,5 @@ database = 'test_default_db'
 schema = 'test_default_go'
 protocol = 'https'
 authenticator = 'oauth'
-token_file_path = './test_data/snowflake/session/token'
+token_file_path = './test_data/snowflake/session/token'
+insecureMode = true

util.go

@@ -339,3 +339,12 @@ func withLowerKeys[T any](in map[string]T) map[string]T {
 	}
 	return out
 }
+
+func findByPrefix(in []string, prefix string) int {
+	for i, v := range in {
+		if strings.HasPrefix(v, prefix) {
+			return i
+		}
+	}
+	return -1
+}

util_test.go

@@ -409,3 +409,14 @@ func TestWithLowerKeys(t *testing.T) {
 	assertEqualE(t, lowerM["abc"], "def")
 	assertEqualE(t, lowerM["ghi"], "KLM")
 }
+
+func TestFindByPrefix(t *testing.T) {
+	nonEmpty := []string{"aaa", "bbb", "ccc"}
+	assertEqualE(t, findByPrefix(nonEmpty, "a"), 0)
+	assertEqualE(t, findByPrefix(nonEmpty, "aa"), 0)
+	assertEqualE(t, findByPrefix(nonEmpty, "aaa"), 0)
+	assertEqualE(t, findByPrefix(nonEmpty, "bb"), 1)
+	assertEqualE(t, findByPrefix(nonEmpty, "ccc"), 2)
+	assertEqualE(t, findByPrefix(nonEmpty, "dd"), -1)
+	assertEqualE(t, findByPrefix([]string{}, "dd"), -1)
+}