diff --git a/pkg/cli/clisqlshell/BUILD.bazel b/pkg/cli/clisqlshell/BUILD.bazel index f5cceb24007f..2da958ea419b 100644 --- a/pkg/cli/clisqlshell/BUILD.bazel +++ b/pkg/cli/clisqlshell/BUILD.bazel @@ -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", diff --git a/pkg/cli/clisqlshell/context.go b/pkg/cli/clisqlshell/context.go index a69ab572e63a..4f1662c72981 100644 --- a/pkg/cli/clisqlshell/context.go +++ b/pkg/cli/clisqlshell/context.go @@ -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 diff --git a/pkg/cli/clisqlshell/sql.go b/pkg/cli/clisqlshell/sql.go index a46c68cabf54..9e6d7de47dc9 100644 --- a/pkg/cli/clisqlshell/sql.go +++ b/pkg/cli/clisqlshell/sql.go @@ -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" ) @@ -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 @@ -1629,7 +1631,7 @@ func (c *cliState) handleConnect( cmd []string, loopState, errState cliStateEnum, ) (resState cliStateEnum) { if err := c.handleConnectInternal(cmd, false /*omitConnString*/); err != nil { - fmt.Fprintln(c.iCtx.stderr, err) + clierror.OutputError(c.iCtx.stderr, err, true /*showSeverity*/, false /*verbose*/) c.exitErr = err return errState } @@ -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. + // it accepts newdb, user, host, port and options 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 { @@ -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") } @@ -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 +} diff --git a/pkg/cli/demo.go b/pkg/cli/demo.go index 93b2e8c81f88..db6e95a1fd2f 100644 --- a/pkg/cli/demo.go +++ b/pkg/cli/demo.go @@ -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 { diff --git a/pkg/cli/interactive_tests/test_connect_cmd.tcl b/pkg/cli/interactive_tests/test_connect_cmd.tcl index 386917527ac5..7f12190d8633 100644 --- a/pkg/cli/interactive_tests/test_connect_cmd.tcl +++ b/pkg/cli/interactive_tests/test_connect_cmd.tcl @@ -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 - unknownuser - - 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 diff --git a/pkg/cli/sql_shell_cmd.go b/pkg/cli/sql_shell_cmd.go index 5b8ae110a2df..764b0491fc44 100644 --- a/pkg/cli/sql_shell_cmd.go +++ b/pkg/cli/sql_shell_cmd.go @@ -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) } diff --git a/pkg/cmd/cockroach-sql/main.go b/pkg/cmd/cockroach-sql/main.go index 206b71a3d7fb..61b7351652b1 100644 --- a/pkg/cmd/cockroach-sql/main.go +++ b/pkg/cmd/cockroach-sql/main.go @@ -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) }