Merge pull request #103144 from cockroachdb/blathers/backport-release…

…-23.1-101987

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",

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

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
+}

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 {

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
 

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)
 }

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)
 }