Merge #48013

48013: cli: add --cert-principal-map to `cert list` command r=petermattis a=petermattis

Remove the use of `base.Config.GetCertificateManager()` from the `cert 
list` implementation as a first step in limiting use of that method, and 
possibly removing it in the future.

Fixes #48011

Release note (cli change): Support `list cert` with certificates which 
require `--cert-principal-map` to pass validation.

Co-authored-by: Peter Mattis <petermattis@gmail.com>

pkg/base/config.go

@@ -24,7 +24,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
-	"github.com/cockroachdb/cockroach/pkg/util/stop"
 )
 
 // Base config defaults.
@@ -267,17 +266,17 @@ func (cfg *Config) AdminURL() *url.URL {
 	}
 }
 
-// GetClientCertPaths returns the paths to the client cert and key.
-func (cfg *Config) GetClientCertPaths(user string) (string, string, error) {
+// getClientCertPaths returns the paths to the client cert and key.
+func (cfg *Config) getClientCertPaths(user string) (string, string, error) {
 	cm, err := cfg.GetCertificateManager()
 	if err != nil {
 		return "", "", err
 	}
 	return cm.GetClientCertPaths(user)
 }
 
-// GetCACertPath returns the path to the CA certificate.
-func (cfg *Config) GetCACertPath() (string, error) {
+// getCACertPath returns the path to the CA certificate.
+func (cfg *Config) getCACertPath() (string, error) {
 	cm, err := cfg.GetCertificateManager()
 	if err != nil {
 		return "", err
@@ -304,7 +303,7 @@ func (cfg *Config) LoadSecurityOptions(options url.Values, username string) erro
 			// verify-ca and verify-full need a CA certificate.
 			if options.Get("sslrootcert") == "" {
 				// Fetch CA cert. This is required.
-				caCertPath, err := cfg.GetCACertPath()
+				caCertPath, err := cfg.getCACertPath()
 				if err != nil {
 					return wrapError(err)
 				}
@@ -316,7 +315,7 @@ func (cfg *Config) LoadSecurityOptions(options url.Values, username string) erro
 		}
 
 		// Fetch certs, but don't fail, we may be using a password.
-		certPath, keyPath, err := cfg.GetClientCertPaths(username)
+		certPath, keyPath, err := cfg.getClientCertPaths(username)
 		if err == nil {
 			if options.Get("sslcert") == "" {
 				options.Set("sslcert", certPath)
@@ -354,36 +353,6 @@ func (cfg *Config) GetCertificateManager() (*security.CertificateManager, error)
 	return cfg.certificateManager.cm, cfg.certificateManager.err
 }
 
-// InitializeNodeTLSConfigs tries to load client and server-side TLS configs.
-// It also enables the reload-on-SIGHUP functionality on the certificate manager.
-// This should be called early in the life of the server to make sure there are no
-// issues with TLS configs.
-// Returns the certificate manager if successfully created and in secure mode.
-func (cfg *Config) InitializeNodeTLSConfigs(
-	stopper *stop.Stopper,
-) (*security.CertificateManager, error) {
-	if cfg.Insecure {
-		return nil, nil
-	}
-
-	if _, err := cfg.GetServerTLSConfig(); err != nil {
-		return nil, err
-	}
-	if _, err := cfg.GetUIServerTLSConfig(); err != nil {
-		return nil, err
-	}
-	if _, err := cfg.GetClientTLSConfig(); err != nil {
-		return nil, err
-	}
-
-	cm, err := cfg.GetCertificateManager()
-	if err != nil {
-		return nil, err
-	}
-	cm.RegisterSignalHandler(stopper)
-	return cm, nil
-}
-
 // GetClientTLSConfig returns the client TLS config, initializing it if needed.
 // If Insecure is true, return a nil config, otherwise ask the certificate
 // manager for a TLS config using certs for the config.User.
@@ -406,11 +375,11 @@ func (cfg *Config) GetClientTLSConfig() (*tls.Config, error) {
 	return tlsCfg, nil
 }
 
-// GetUIClientTLSConfig returns the client TLS config for Admin UI clients, initializing it if needed.
+// getUIClientTLSConfig returns the client TLS config for Admin UI clients, initializing it if needed.
 // If Insecure is true, return a nil config, otherwise ask the certificate
 // manager for a TLS config configured to talk to the Admin UI.
 // This TLSConfig is **NOT** suitable to talk to the GRPC or SQL servers, use GetClientTLSConfig instead.
-func (cfg *Config) GetUIClientTLSConfig() (*tls.Config, error) {
+func (cfg *Config) getUIClientTLSConfig() (*tls.Config, error) {
 	// Early out.
 	if cfg.Insecure {
 		return nil, nil
@@ -452,6 +421,9 @@ func (cfg *Config) GetServerTLSConfig() (*tls.Config, error) {
 // GetUIServerTLSConfig returns the server TLS config for the Admin UI, initializing it if needed.
 // If Insecure is true, return a nil config, otherwise ask the certificate
 // manager for a server UI TLS config.
+//
+// TODO(peter): This method is only used by `server.NewServer` and
+// `Server.Start`. Move it.
 func (cfg *Config) GetUIServerTLSConfig() (*tls.Config, error) {
 	// Early out.
 	if cfg.Insecure || cfg.DisableTLSForHTTP {
@@ -477,7 +449,7 @@ func (cfg *Config) GetHTTPClient() (http.Client, error) {
 		cfg.httpClient.httpClient.Timeout = 10 * time.Second
 		var transport http.Transport
 		cfg.httpClient.httpClient.Transport = &transport
-		transport.TLSClientConfig, cfg.httpClient.err = cfg.GetUIClientTLSConfig()
+		transport.TLSClientConfig, cfg.httpClient.err = cfg.getUIClientTLSConfig()
 	})
 
 	return cfg.httpClient.httpClient, cfg.httpClient.err

pkg/cli/cert.go

@@ -203,9 +203,12 @@ List certificates and keys found in the certificate directory.
 
 // runListCerts loads and lists all certs.
 func runListCerts(cmd *cobra.Command, args []string) error {
-	cm, err := baseCfg.GetCertificateManager()
+	if err := security.SetCertPrincipalMap(certCtx.certPrincipalMap); err != nil {
+		return err
+	}
+	cm, err := security.NewCertificateManager(baseCfg.SSLCertsDir)
 	if err != nil {
-		return errors.Wrap(err, "could not get certificate manager")
+		return errors.Wrap(err, "cannot load certificates")
 	}
 
 	fmt.Fprintf(os.Stdout, "Certificate directory: %s\n", baseCfg.SSLCertsDir)

pkg/cli/context.go

@@ -172,6 +172,8 @@ func initCLIDefaults() {
 
 	authCtx.validityPeriod = 1 * time.Hour
 
+	certCtx.certPrincipalMap = nil
+
 	initPreFlagsDefaults()
 
 	// Clear the "Changed" state of all the registered command-line flags.
@@ -399,3 +401,9 @@ var demoCtx struct {
 	insecure                  bool
 	geoLibsDir                string
 }
+
+// certCtx captures the command-line parameters of the `cert` command.
+// Defaults set by InitCLIDefaults() above.
+var certCtx struct {
+	certPrincipalMap []string
+}

pkg/cli/flags.go

@@ -443,6 +443,10 @@ func init() {
 		StringFlag(f, &baseCfg.SSLCertsDir, cliflags.CertsDir, baseCfg.SSLCertsDir)
 	}
 
+	// The list certs command needs the certificate principal map.
+	StringSlice(listCertsCmd.Flags(), &certCtx.certPrincipalMap,
+		cliflags.CertPrincipalMap, certCtx.certPrincipalMap)
+
 	for _, cmd := range []*cobra.Command{createCACertCmd, createClientCACertCmd} {
 		f := cmd.Flags()
 		// CA certificates have a longer expiration time.

pkg/cli/interactive_tests/test_cert_advisory_validation.tcl

@@ -71,3 +71,9 @@ interrupt
 eexpect "interrupted"
 expect $prompt
 end_test
+
+start_test "Check that 'cert list' can utilize cert principal map."
+send "$argv cert list --certs-dir=$certs_dir --cert-principal-map=foo.bar:node\r"
+eexpect "Certificate directory:"
+expect $prompt
+end_test

pkg/server/server.go

@@ -203,11 +203,24 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 	}
 
 	// Attempt to load TLS configs right away, failures are permanent.
-	if certMgr, err := cfg.InitializeNodeTLSConfigs(stopper); err != nil {
-		return nil, err
-	} else if certMgr != nil {
-		// The certificate manager is non-nil in secure mode.
-		registry.AddMetricStruct(certMgr.Metrics())
+	if !cfg.Insecure {
+		// TODO(peter): Call methods on CertificateManager directly. Need to call
+		// base.wrapError or similar on the resulting error.
+		if _, err := cfg.GetServerTLSConfig(); err != nil {
+			return nil, err
+		}
+		if _, err := cfg.GetUIServerTLSConfig(); err != nil {
+			return nil, err
+		}
+		if _, err := cfg.GetClientTLSConfig(); err != nil {
+			return nil, err
+		}
+		cm, err := cfg.GetCertificateManager()
+		if err != nil {
+			return nil, err
+		}
+		cm.RegisterSignalHandler(stopper)
+		registry.AddMetricStruct(cm.Metrics())
 	}
 
 	// Add a dynamic log tag value for the node ID.