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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 7 commits into from
Oct 9, 2024
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7cc4e84
Add support for 'server-prefer' TLS mode and make it default
tanvipise Sep 25, 2024
47e0f35
Fixed a typo
tanvipise Sep 25, 2024
6879ae0
Replace 'server-prefer' to 'prefer'
tanvipise Oct 2, 2024
0b005dd
Comment cleanup
tanvipise Oct 2, 2024
c49e8f6
minor formatting
tanvipise Oct 3, 2024
efe0d5d
addressing prefer logic issue
tanvipise Oct 4, 2024
e06be10
rearrange priority of tlsmode
tanvipise Oct 8, 2024
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
View file
Open in desktop
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 = don't use SSL for this connection</li><li>server = server must support SSL, but skip verification (INSECURE!)</li><li>server-prefer (default) = checks for SSL/TLS server support; if unsupported, SSL/TLS is not used for this connection. </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>server = server must support SSL, but skip verification (INSECURE!)</li><li>prefer (default) = checks for SSL/TLS server support; if unsupported, SSL/TLS is not used for this connection. </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> |
sitingren marked this conversation as resolved.
Show resolved Hide resolved
| 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
View file
Open in desktop
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' = don't use SSL/TLS for this connection</li><li>'server' = server must support SSL/TLS, but skip verification **(INSECURE!)**</li><li>'server-prefer' (default) = checks for SSL/TLS server support; if unsupported, SSL/TLS is not used for this connection. </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>'server' = server must support SSL/TLS, but skip verification **(INSECURE!)**</li><li>'prefer' (default) = checks for SSL/TLS server support; if unsupported, SSL/TLS is not used for this connection. </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
14 changes: 7 additions & 7 deletions connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ var (

const (
tlsModeServer = "server"
tlsModeServerPrefer = "server-prefer"
tlsModePrefer = "prefer"
tanvipise marked this conversation as resolved.
Show resolved Hide resolved
tlsModeServerStrict = "server-strict"
tlsModeNone = "none"
)
Expand All @@ -86,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-prefer', 'server-strict' or 'none'
// reserved modes: 'server', 'prefer', 'server-strict' or 'none'
func RegisterTLSConfig(name string, config *tls.Config) error {
if name == tlsModeServer || name == tlsModeServerPrefer || name == tlsModeServerStrict || name == tlsModeNone {
if name == tlsModeServer || name == tlsModePrefer || 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 @@ -259,8 +259,8 @@ func newConnection(connString string) (*connection, error) {
// Read SSL/TLS flag.
sslFlag := strings.ToLower(result.connURL.Query().Get("tlsmode"))
if sslFlag == "" {
// Set default to tlsModeServerPrefer
sslFlag = tlsModeServerPrefer
// Set default to tlsModePrefer
sslFlag = tlsModeNone
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this back to 'tlsModeNone' as it was initially which is an appropriate logic. This change made all the workflow tests pass.

}

// Read Workload flag
Expand Down Expand Up @@ -678,8 +678,8 @@ func (v *connection) initializeSSL(sslFlag string) error {
case tlsModeServer:
connectionLogger.Info("enabling SSL/TLS server mode")
v.conn = tls.Client(v.conn, &tls.Config{InsecureSkipVerify: true})
case tlsModeServerPrefer:
connectionLogger.Info("enabling SSL/TLS server prefer mode")
case tlsModePrefer:
connectionLogger.Info("enabling SSL/TLS prefer mode")
v.conn = tls.Client(v.conn, &tls.Config{ServerName: v.connURL.Hostname(), InsecureSkipVerify: true})
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason we need to set the ServerName in the tls Config?

case tlsModeServerStrict:
connectionLogger.Info("enabling SSL/TLS server strict mode")
Expand Down
4 changes: 2 additions & 2 deletions driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func TestTLSConfiguration(t *testing.T) {
switch *tlsMode {
case "none":
assertEqual(t, sslState, "None")
case "server", "server-prefer", "server-strict":
case "server", "prefer", "server-strict":
assertEqual(t, sslState, "Server")
case "custom":
assertEqual(t, sslState, "Mutual")
Expand Down Expand Up @@ -1227,7 +1227,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-prefer, server-strict, custom)")
var tlsMode = flag.String("tlsmode", "none", "SSL/TLS mode (none, server, prefer, 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