Skip to content

Commit

Permalink
cli/sql: new option autocerts for TLS client cert auto-discovery
Browse files Browse the repository at this point in the history
See the release note below.
An additional benefit not mentioned in the release note is that
it simplifies switching from one tenant to another when using
shared-process multitenancy. For example, this becomes possible:

```
> CREATE TENANT foo;
> ALTER TENANT foo START SERVICE SHARED;
> \c cluster:foo root - - autocerts
```

Alternatively, this can also be used to quickly switch from a non-root
user in an app tenant to the root user in the system tenant:
```
> \c cluster:system root - - autocerts
```

This works because (currently) all tenant servers running side-by-side
use the same TLS CA to validate SQL client certs.

Release note (cli change): The `\connect` client-side command for the
SQL shell (included in `cockroach sql`, `cockroach demo`,
`cockroach-sql`) now recognizes an option `autocerts` as last
argument.

When provided, `\c` will now try to discover a TLS client
certificate and key in the same directory(ies) as used by the previous
connection URL.

This feature makes it easier to switch usernames when
TLS client/key files are available for both the previous and the new
username.
  • Loading branch information
knz committed Apr 21, 2023
1 parent acabf6f commit 0932ac6
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 4 deletions.
1 change: 1 addition & 0 deletions pkg/cli/clisqlshell/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ go_library(
"//pkg/util/sysutil",
"@com_github_charmbracelet_bubbles//cursor",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_errors//oserror",
"@com_github_knz_bubbline//:bubbline",
"@com_github_knz_bubbline//computil",
"@com_github_knz_bubbline//editline",
Expand Down
4 changes: 4 additions & 0 deletions pkg/cli/clisqlshell/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ type Context struct {
// CockroachDB's own CLI package has a more advanced URL
// parser that is used instead.
ParseURL URLParser

// CertsDir is an extra directory to look for client certs in,
// when the \c command is used.
CertsDir string
}

// internalContext represents the internal configuration state of the
Expand Down
120 changes: 118 additions & 2 deletions pkg/cli/clisqlshell/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/sysutil"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/errors/oserror"
"github.com/knz/bubbline/editline"
"github.com/knz/bubbline/history"
)
Expand All @@ -66,9 +67,10 @@ Query Buffer
Connection
\info display server details including connection strings.
\c, \connect {[DB] [USER] [HOST] [PORT] | [URL]}
\c, \connect {[DB] [USER] [HOST] [PORT] [options] | [URL]}
connect to a server or print the current connection URL.
(Omitted values reuse previous parameters. Use '-' to skip a field.)
Omitted values reuse previous parameters. Use '-' to skip a field.
The option "autocerts" attempts to auto-discover TLS client certs.
\password [USERNAME]
securely change the password for a user
Expand Down Expand Up @@ -1691,10 +1693,21 @@ func (c *cliState) handleConnectInternal(cmd []string, omitConnString bool) erro
return err
}

var autoCerts bool

// Parse the arguments to \connect:
// it accepts newdb, user, host, port in that order.
// Each field can be marked as "-" to reuse the current defaults.
switch len(cmd) {
case 5:
if cmd[4] != "-" {
if cmd[4] == "autocerts" {
autoCerts = true
} else {
return errors.Newf(`unknown syntax: \c %s`, strings.Join(cmd, " "))
}
}
fallthrough
case 4:
if cmd[3] != "-" {
if err := newURL.SetOption("port", cmd[3]); err != nil {
Expand Down Expand Up @@ -1757,6 +1770,12 @@ func (c *cliState) handleConnectInternal(cmd []string, omitConnString bool) erro
newURL.WithAuthn(prevAuthn)
}

if autoCerts {
if err := autoFillClientCerts(newURL, currURL, c.sqlCtx.CertsDir); err != nil {
return err
}
}

if err := newURL.Validate(); err != nil {
return errors.Wrap(err, "validating the new URL")
}
Expand Down Expand Up @@ -2712,3 +2731,100 @@ func (c *cliState) closeOutputFile() error {
c.iCtx.queryOutputBuf = nil
return err
}

// autoFillClientCerts tries to discover a TLS client certificate and key
// for use in newURL. This is used from the \connect command with option
// "autocerts".
func autoFillClientCerts(newURL, currURL *pgurl.URL, extraCertsDir string) error {
username := newURL.GetUsername()
// We could use methods from package "certnames" here but we're
// avoiding extra package dependencies for the sake of keeping
// the standalone shell binary (cockroach-sql) small.
desiredKeyFile := "client." + username + ".key"
desiredCertFile := "client." + username + ".crt"
// Try to discover a TLS client certificate and key.
// First we'll try to find them in the directory specified in the shell config.
// This is coming from --certs-dir on the command line (of COCKROACH_CERTS_DIR).
//
// If not found there, we'll try to find the client cert in the
// same directory as the cert in the original URL; and the key in
// the same directory as the key in the original URL (cert and key
// may be in different places).
//
// If the original URL doesn't have a cert, we'll try to find a
// cert in the directory where the CA cert is stored.

// If all fails, we'll tell the user where we tried to search.
candidateDirs := make(map[string]struct{})
var newCert, newKey string
if extraCertsDir != "" {
candidateDirs[extraCertsDir] = struct{}{}
if candidateCert := filepath.Join(extraCertsDir, desiredCertFile); fileExists(candidateCert) {
newCert = candidateCert
}
if candidateKey := filepath.Join(extraCertsDir, desiredKeyFile); fileExists(candidateKey) {
newKey = candidateKey
}
}
if newCert == "" || newKey == "" {
var caCertDir string
if tlsUsed, _, caCertPath := currURL.GetTLSOptions(); tlsUsed {
caCertDir = filepath.Dir(caCertPath)
candidateDirs[caCertDir] = struct{}{}
}
var prevCertDir, prevKeyDir string
if authnCertEnabled, certPath, keyPath := currURL.GetAuthnCert(); authnCertEnabled {
prevCertDir = filepath.Dir(certPath)
prevKeyDir = filepath.Dir(keyPath)
candidateDirs[prevCertDir] = struct{}{}
candidateDirs[prevKeyDir] = struct{}{}
}
if newKey == "" {
if candidateKey := filepath.Join(prevKeyDir, desiredKeyFile); fileExists(candidateKey) {
newKey = candidateKey
} else if candidateKey := filepath.Join(caCertDir, desiredKeyFile); fileExists(candidateKey) {
newKey = candidateKey
}
}
if newCert == "" {
if candidateCert := filepath.Join(prevCertDir, desiredCertFile); fileExists(candidateCert) {
newCert = candidateCert
} else if candidateCert := filepath.Join(caCertDir, desiredCertFile); fileExists(candidateCert) {
newCert = candidateCert
}
}
}
if newCert == "" || newKey == "" {
err := errors.Newf("unable to find TLS client cert and key for user %q", username)
if len(candidateDirs) == 0 {
err = errors.WithHint(err, "No candidate directories; try specifying --certs-dir on the command line.")
} else {
sortedDirs := make([]string, 0, len(candidateDirs))
for dir := range candidateDirs {
sortedDirs = append(sortedDirs, dir)
}
sort.Strings(sortedDirs)
err = errors.WithDetailf(err, "Candidate directories:\n- %s", strings.Join(sortedDirs, "\n- "))
}
return err
}

newURL.WithAuthn(pgurl.AuthnClientCert(newCert, newKey))

return nil
}

func fileExists(path string) bool {
_, err := os.Stat(path)
if err == nil {
return true
}
if oserror.IsNotExist(err) {
return false
}
// Stat() returned an error that is not "does not exist".
// This is unexpected, but we'll treat it as if the file does exist.
// The connection will try to use the file, and then fail with a
// more specific error.
return true
}
2 changes: 2 additions & 0 deletions pkg/cli/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,8 @@ func runDemoInternal(
}
defer func() { resErr = errors.CombineErrors(resErr, conn.Close()) }()

_, _, certsDir := c.GetSQLCredentials()
sqlCtx.ShellCtx.CertsDir = certsDir
sqlCtx.ShellCtx.ParseURL = clienturl.MakeURLParserFn(cmd, cliCtx.clientOpts)

if err := extraInit(ctx, conn); err != nil {
Expand Down
18 changes: 16 additions & 2 deletions pkg/cli/interactive_tests/test_connect_cmd.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,31 @@ eexpect foo@
eexpect "/system>"
end_test

start_test "Check that the client-side connect cmd can change users with automatic client cert detection"
send "\\c - root - - autocerts\r"
eexpect "using new connection URL"
eexpect root@
eexpect "/system>"
end_test

start_test "Check that the auto-cert feature properly fails if certs were not found"
send "\\c - root - - autocerts\r"
eexpect "unable to find TLS client cert and key"
eexpect root@
eexpect "/system>"
end_test

start_test "Check that the client-side connect cmd can detect syntax errors"
send "\\c - - - - abc\r"
eexpect "unknown syntax"
eexpect foo@
eexpect root@
eexpect "/system>"
end_test

start_test "Check that the client-side connect cmd recognizes invalid URLs"
send "\\c postgres://root@localhost:26257/defaultdb?sslmode=invalid&sslcert=$certs_dir%2Fclient.root.crt&sslkey=$certs_dir%2Fclient.root.key&sslrootcert=$certs_dir%2Fca.crt\r"
eexpect "unrecognized sslmode parameter"
eexpect foo@
eexpect root@
eexpect "/system>"
end_test

Expand Down
1 change: 1 addition & 0 deletions pkg/cli/sql_shell_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func runTerm(cmd *cobra.Command, args []string) (resErr error) {
}
defer func() { resErr = errors.CombineErrors(resErr, conn.Close()) }()

sqlCtx.ShellCtx.CertsDir = baseCfg.SSLCertsDir
sqlCtx.ShellCtx.ParseURL = clienturl.MakeURLParserFn(cmd, cliCtx.clientOpts)
return sqlCtx.Run(context.Background(), conn)
}
1 change: 1 addition & 0 deletions pkg/cmd/cockroach-sql/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ func runSQL(cmd *cobra.Command, args []string) (resErr error) {
}
defer func() { resErr = errors.CombineErrors(resErr, conn.Close()) }()

cfg.ShellCtx.CertsDir = copts.CertsDir
cfg.ShellCtx.ParseURL = clienturl.MakeURLParserFn(cmd, copts)
return cfg.Run(context.Background(), conn)
}

0 comments on commit 0932ac6

Please sign in to comment.