Skip to content

Commit

Permalink
SNOW-1821505 Introduce disableOCSPChecks config option
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-pfus committed Nov 22, 2024
1 parent ce3db31 commit 15f7990
Show file tree
Hide file tree
Showing 9 changed files with 136 additions and 12 deletions.
2 changes: 1 addition & 1 deletion connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions connection_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ func handleSingleParam(cfg *Config, key string, value interface{}) error {
}
err = determineAuthenticatorType(cfg, v)
case "insecuremode":
// TODO
logInsecureModeDeprecationInfo()
cfg.InsecureMode, err = parseBool(value)
case "ocspfailopen":
var vv ConfigBool
Expand Down
2 changes: 1 addition & 1 deletion doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ 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.
Expand Down
2 changes: 1 addition & 1 deletion driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,7 @@ func TestOpenWithTransport(t *testing.T) {

// Test that transport override also works in insecure mode
countingTransport.requests = 0
config.InsecureMode = true
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)
Expand Down
33 changes: 29 additions & 4 deletions dsn.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net/http"
"net/url"
"os"
"slices"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -79,8 +80,10 @@ 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.
InsecureMode bool // driver doesn't check certificate revocation status
Application string // application name.
DisableOCSPChecks bool // driver doesn't check certificate revocation status
// Deprecated: InsecureMode use DisableOCSPChecks instead
InsecureMode bool
OCSPFailOpen OCSPFailOpenMode // OCSP Fail Open

Token string // Token to use for OAuth other forms of token based auth
Expand Down Expand Up @@ -126,7 +129,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
Expand Down Expand Up @@ -241,6 +244,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)
}
Expand Down Expand Up @@ -637,7 +643,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 := getFirstIndexOfWithPrefix(paramsSlice, "insecureMode")
disableOCSPChecksIdx := getFirstIndexOfWithPrefix(paramsSlice, "disableOCSPChecks")
if insecureModeIdx > -1 && disableOCSPChecksIdx > -1 {
logger.Warn("duplicated insecureMode and disableOCSPChecks. disableOCSPChecks takes precedence")
paramsSlice = slices.Concat(paramsSlice[:insecureModeIdx-1], paramsSlice[insecureModeIdx+1:])

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / Check linter

undefined: slices.Concat) (typecheck)

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / Check linter

undefined: slices.Concat) (typecheck)

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / Check linter

undefined: slices.Concat) (typecheck)

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / Check linter

undefined: slices.Concat (typecheck)

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.21 on Mac

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.21 on Mac

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.21 on Mac

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.21 on Windows

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.21 on Ubuntu

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.21 on Ubuntu

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.21 on Ubuntu

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.21 on Windows

undefined: slices.Concat

Check failure on line 651 in dsn.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.21 on Windows

undefined: slices.Concat
}
for _, v := range paramsSlice {
param := strings.SplitN(v, "=", 2)
if len(param) != 2 {
continue
Expand Down Expand Up @@ -715,12 +728,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)
Expand Down Expand Up @@ -839,6 +860,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
Expand Down
81 changes: 81 additions & 0 deletions dsn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 1 addition & 5 deletions ocsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
9 changes: 9 additions & 0 deletions util.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,12 @@ func withLowerKeys[T any](in map[string]T) map[string]T {
}
return out
}

func getFirstIndexOfWithPrefix(in []string, prefix string) int {
for i, v := range in {
if strings.HasPrefix(v, prefix) {
return i
}
}
return -1
}
11 changes: 11 additions & 0 deletions util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,3 +409,14 @@ func TestWithLowerKeys(t *testing.T) {
assertEqualE(t, lowerM["abc"], "def")
assertEqualE(t, lowerM["ghi"], "KLM")
}

func TestGetFirstIndexOfWithPrefix(t *testing.T) {
nonEmpty := []string{"aaa", "bbb", "ccc"}
assertEqualE(t, getFirstIndexOfWithPrefix(nonEmpty, "a"), 0)
assertEqualE(t, getFirstIndexOfWithPrefix(nonEmpty, "aa"), 0)
assertEqualE(t, getFirstIndexOfWithPrefix(nonEmpty, "aaa"), 0)
assertEqualE(t, getFirstIndexOfWithPrefix(nonEmpty, "bb"), 1)
assertEqualE(t, getFirstIndexOfWithPrefix(nonEmpty, "ccc"), 2)
assertEqualE(t, getFirstIndexOfWithPrefix(nonEmpty, "dd"), -1)
assertEqualE(t, getFirstIndexOfWithPrefix([]string{}, "dd"), -1)
}

0 comments on commit 15f7990

Please sign in to comment.