Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for 'prefer' TLS mode and make it default #179

Merged
merged 7 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ were args are one of the following:
| Query Argument | Description | Values |
|----------------|-------------|--------|
| use_prepared_statements | whether to use client-side query interpolation or server-side argument binding | <li>true = (default) use server-side bindings </li><li>false = user client side interpolation</li> |
| tlsmode | the ssl policy for this connection | <li>none (default) = don't use SSL for this connection</li><li>server = server must support SSL, but skip verification (INSECURE!)</li><li>server-strict = server must support SSL</li><li>custom = use custom TLS config (Need to generate certs with `resources/tests/genCerts.sh` in advance) </li> |
| tlsmode | the ssl policy for this connection | <li>none = don't use SSL for this connection</li><li>prefer (default) = checks for SSL/TLS server support; if unsupported, SSL/TLS is not used for this connection. </li><li>server = server must support SSL, but skip verification (INSECURE!)</li><li>server-strict = server must support SSL</li><li>custom = use custom TLS config (Need to generate certs with `resources/tests/genCerts.sh` in advance) </li> |
| locator | host and port of the Vertica connection | (default) localhost:5433 |
| user | Vertica user name | (default) dbadmin |
| password | Vertica password for the connecting user | (default) (empty) |
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Currently supported query arguments are:
|----------------|-------------|--------|
| use_prepared_statements | Whether to use client-side query interpolation or server-side argument binding. | 1 = (default) use server-side bindings <br>0 = user client side interpolation **(LESS SECURE)** |
| connection_load_balance | Whether to enable connection load balancing on the client side. | 0 = (default) disable load balancing <br>1 = enable load balancing |
| tlsmode | The ssl/tls policy for this connection. | <li>'none' (default) = don't use SSL/TLS for this connection</li><li>'server' = server must support SSL/TLS, but skip verification **(INSECURE!)**</li><li>'server-strict' = server must support SSL/TLS</li><li>{customName} = use custom registered `tls.Config` (see "Using custom TLS config" section below)</li> |
| tlsmode | The ssl/tls policy for this connection. | <li>'none' = don't use SSL/TLS for this connection</li><li>'prefer' (default) = checks for SSL/TLS server support; if unsupported, SSL/TLS is not used for this connection. </li><li>'server' = server must support SSL/TLS, but skip verification **(INSECURE!)**</li><li>'server-strict' = server must support SSL/TLS</li><li>{customName} = use custom registered `tls.Config` (see "Using custom TLS config" section below)</li> |
| backup_server_node | A list of backup hosts for the client to try to connect if the primary host is unreachable. | a comma-seperated list of backup host-port pairs. E.g.<br> 'host1:port1,host2:port2,host3:port3' |
| client_label | Sets a label for the connection on the server. This value appears in the `client_label` column of the SESSIONS system table. | (default) vertica-sql-go-{version}-{pid}-{timestamp} |
| autocommit | Controls whether the connection automatically commits transactions. | 1 = (default) on <br>0 = off|
Expand Down
12 changes: 10 additions & 2 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ var (
)

const (
tlsModePrefer = "prefer"
tlsModeServer = "server"
tlsModeServerStrict = "server-strict"
tlsModeNone = "none"
Expand Down Expand Up @@ -85,9 +86,9 @@ func (t *_tlsConfigs) get(name string) (*tls.Config, bool) {
var tlsConfigs = &_tlsConfigs{m: make(map[string]*tls.Config)}

// db, err := sql.Open("vertica", "user@tcp(localhost:3306)/test?tlsmode=custom")
// reserved modes: 'server', 'server-strict' or 'none'
// reserved modes: 'prefer', 'server', 'server-strict' or 'none'
func RegisterTLSConfig(name string, config *tls.Config) error {
if name == tlsModeServer || name == tlsModeServerStrict || name == tlsModeNone {
if name == tlsModePrefer || name == tlsModeServer || name == tlsModeServerStrict || name == tlsModeNone {
return fmt.Errorf("config name '%s' is reserved therefore cannot be used", name)
}
return tlsConfigs.add(name, config)
Expand Down Expand Up @@ -665,6 +666,10 @@ func (v *connection) initializeSSL(sslFlag string) error {
}

if buf[0] == 'N' {
if sslFlag == tlsModePrefer {
connectionLogger.Info("SSL/TLS is not supported, proceeding with non-SSL connection in prefer mode")
return nil
}
return fmt.Errorf("SSL/TLS is not enabled on this server")
}

Expand All @@ -673,6 +678,9 @@ func (v *connection) initializeSSL(sslFlag string) error {
}

switch sslFlag {
case tlsModePrefer:
connectionLogger.Info("enabling SSL/TLS prefer mode")
v.conn = tls.Client(v.conn, &tls.Config{InsecureSkipVerify: true})
case tlsModeServer:
connectionLogger.Info("enabling SSL/TLS server mode")
v.conn = tls.Client(v.conn, &tls.Config{InsecureSkipVerify: true})
Expand Down
6 changes: 5 additions & 1 deletion driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ func TestTLSConfiguration(t *testing.T) {
switch *tlsMode {
case "none":
assertEqual(t, sslState, "None")
case "prefer":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no new test adding to CI, have you manually tested these two cases?

  • tlsmode=prefer, TLS is not configured on server
  • tlsmode=prefer, TLS is enabled on server

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have tested them manually and also verified them with @DMickens.

if sslState != "None" && sslState != "Server" {
t.Fatalf("Expected ssl_state to be 'None' or 'Server', but got '%s'", sslState)
}
case "server", "server-strict":
assertEqual(t, sslState, "Server")
case "custom":
Expand Down Expand Up @@ -1227,7 +1231,7 @@ func TestClientOSHostnameProperty(t *testing.T) {
var verticaUserName = flag.String("user", "dbadmin", "the user name to connect to Vertica")
var verticaPassword = flag.String("password", os.Getenv("VERTICA_TEST_PASSWORD"), "Vertica password for this user")
var verticaHostPort = flag.String("locator", "localhost:5433", "Vertica's host and port")
var tlsMode = flag.String("tlsmode", "none", "SSL/TLS mode (none, server, server-strict, custom)")
var tlsMode = flag.String("tlsmode", "none", "SSL/TLS mode (none, prefer, server, server-strict, custom)")
var usePreparedStmts = flag.Bool("use_prepared_statements", true, "whether to use prepared statements for all queries/executes")
var oauthAccessToken = flag.String("oauth_access_token", os.Getenv("VERTICA_TEST_OAUTH_ACCESS_TOKEN"), "the OAuth Access Token to connect to Vertica")

Expand Down