cli: don't scope TLS client certs to a specific tenant by default

pkg/cli/cliflags/flags.go

@@ -793,9 +793,9 @@ Note: that --external-io-disable-http or --external-io-disable-implicit-credenti
 	TenantScope = FlagInfo{
 		Name: "tenant-scope",
 		Description: `Assign a tenant scope to the certificate.
-This will allow for the certificate to only be used specifically for a particular
-tenant. This flag is optional, when omitted, the certificate is scoped to the
-system tenant.`,
+This will restrict the certificate to only be valid for the specified tenants.
+This flag is optional. When omitted, the certificate is not scoped; i.e.
+it can be used with all tenants.`,
 	}
 
 	GeneratePKCS8Key = FlagInfo{

pkg/cli/context.go

@@ -265,7 +265,20 @@ func setCertContextDefaults() {
 	certCtx.generatePKCS8Key = false
 	certCtx.disableUsernameValidation = false
 	certCtx.certPrincipalMap = nil
-	certCtx.tenantScope = []roachpb.TenantID{roachpb.SystemTenantID}
+	// Note: we set tenantScope to nil so that by default, client certs
+	// are not scoped to a specific tenant and can be used to connect to
+	// any tenant.
+	//
+	// Note that the scoping is generally useful for security, and it is
+	// used in CockroachCloud. However, CockroachCloud does not use our
+	// CLI code to generate certs and sets its tenant scopes on its own.
+	//
+	// Given that our CLI code is provided for convenience and developer
+	// productivity, and we don't expect certs generated here to be used
+	// in multi-tenant deployments where tenants are adversarial to each
+	// other, defaulting to certs that are valid on every tenant is a
+	// good choice.
+	certCtx.tenantScope = nil
 }
 
 var sqlExecCtx = clisqlexec.Context{

pkg/cli/flags.go

@@ -580,7 +580,6 @@ func init() {
 		cliflagcfg.StringFlag(f, &certCtx.caKey, cliflags.CAKey)
 		cliflagcfg.IntFlag(f, &certCtx.keySize, cliflags.KeySize)
 		cliflagcfg.BoolFlag(f, &certCtx.overwriteFiles, cliflags.OverwriteFiles)
-		cliflagcfg.VarFlag(f, &tenantIDSetter{tenantIDs: &certCtx.tenantScope}, cliflags.TenantScope)
 
 		if strings.HasSuffix(cmd.Name(), "-ca") {
 			// CA-only commands.
@@ -596,6 +595,8 @@ func init() {
 		}
 
 		if cmd == createClientCertCmd {
+			cliflagcfg.VarFlag(f, &tenantIDSetter{tenantIDs: &certCtx.tenantScope}, cliflags.TenantScope)
+
 			// PKCS8 key format is only available for the client cert command.
 			cliflagcfg.BoolFlag(f, &certCtx.generatePKCS8Key, cliflags.GeneratePKCS8Key)
 			cliflagcfg.BoolFlag(f, &certCtx.disableUsernameValidation, cliflags.DisableUsernameValidation)

pkg/cmd/roachtest/tests/BUILD.bazel

@@ -205,7 +205,6 @@ go_library(
         "//pkg/roachprod/install",
         "//pkg/roachprod/logger",
         "//pkg/roachprod/prometheus",
-        "//pkg/security/username",
         "//pkg/server",
         "//pkg/server/serverpb",
         "//pkg/sql",

pkg/cmd/roachtest/tests/multitenant_utils.go

@@ -26,7 +26,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
 	"github.com/cockroachdb/cockroach/pkg/roachprod"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/config"
-	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/stretchr/testify/require"
@@ -100,26 +99,10 @@ func createTenantNode(
 		node:       node,
 		sqlPort:    sqlPort,
 	}
-	if tn.cockroachBinSupportsTenantScope(ctx, c) {
-		err := tn.recreateClientCertsWithTenantScope(ctx, c, createOptions.otherTenantIDs)
-		require.NoError(t, err)
-	}
 	tn.createTenantCert(ctx, t, c, createOptions.certNodes)
 	return tn
 }
 
-// cockroachBinSupportsTenantScope is a hack to figure out if the version of
-// cockroach on the node supports tenant scoped certificates. We can't use a
-// version comparison here because we need to compare alpha build versions which
-// are compared lexicographically. This is a problem because our alpha versions
-// contain an integer count of commits, which does not sort correctly.  Once
-// this feature ships in a release, it will be easier to do a version comparison
-// on whether this command line flag is supported.
-func (tn *tenantNode) cockroachBinSupportsTenantScope(ctx context.Context, c cluster.Cluster) bool {
-	err := c.RunE(ctx, c.Node(tn.node), "./cockroach cert create-client --help | grep '\\--tenant-scope'")
-	return err == nil
-}
-
 func (tn *tenantNode) createTenantCert(
 	ctx context.Context, t test.Test, c cluster.Cluster, certNodes option.NodeListOption,
 ) {
@@ -145,23 +128,6 @@ func (tn *tenantNode) createTenantCert(
 	c.Run(ctx, c.Node(tn.node), cmd)
 }
 
-func (tn *tenantNode) recreateClientCertsWithTenantScope(
-	ctx context.Context, c cluster.Cluster, otherIDs []int,
-) error {
-	tenantArgs := fmt.Sprintf("1,%d", tn.tenantID)
-	for _, id := range otherIDs {
-		tenantArgs = fmt.Sprintf("%s,%d", tenantArgs, id)
-	}
-
-	for _, user := range []username.SQLUsername{username.RootUserName(), username.TestUserName()} {
-		cmd := fmt.Sprintf(
-			"./cockroach cert create-client %s --certs-dir=certs --ca-key=certs/ca.key --tenant-scope %s --overwrite",
-			user.Normalized(), tenantArgs)
-		c.Run(ctx, c.Node(tn.node), cmd)
-	}
-	return c.RefetchCertsFromNode(ctx, tn.node)
-}
-
 func (tn *tenantNode) stop(ctx context.Context, t test.Test, c cluster.Cluster) {
 	if tn.errCh == nil {
 		return

pkg/roachprod/install/cluster_synced.go

@@ -1199,8 +1199,8 @@ func (c *SyncedCluster) DistributeCerts(ctx context.Context, l *logger.Logger) e
 rm -fr certs
 mkdir -p certs
 %[1]s cert create-ca --certs-dir=certs --ca-key=certs/ca.key
-%[1]s cert create-client root --certs-dir=certs --ca-key=certs/ca.key --tenant-scope 1,2,3,4
-%[1]s cert create-client testuser --certs-dir=certs --ca-key=certs/ca.key --tenant-scope 1,2,3,4
+%[1]s cert create-client root --certs-dir=certs --ca-key=certs/ca.key
+%[1]s cert create-client testuser --certs-dir=certs --ca-key=certs/ca.key
 %[1]s cert create-node %[2]s --certs-dir=certs --ca-key=certs/ca.key
 # Pre-create a few tenant-client
 %[1]s cert create-tenant-client 2 %[2]s --certs-dir=certs --ca-key=certs/ca.key
@@ -1284,9 +1284,6 @@ func (c *SyncedCluster) createTenantCertBundle(
 		node := c.Nodes[i]
 
 		var tenantScopeArg string
-		if c.cockroachBinSupportsTenantScope(l, ctx, node) {
-			tenantScopeArg = fmt.Sprintf("--tenant-scope %d", tenantID)
-		}
 
 		cmd := "set -e;"
 		if c.IsLocal() {
@@ -1327,23 +1324,6 @@ tar cvf %[5]s $CERT_DIR
 	}, DefaultSSHRetryOpts)
 }
 
-// cockroachBinSupportsTenantScope is a hack to figure out if the version of
-// cockroach on the node supports tenant scoped certificates. We can't use a
-// version comparison here because we need to compare alpha build versions which
-// are compared lexicographically. This is a problem because our alpha versions
-// contain an integer count of commits, which does not sort correctly.  Once
-// this feature ships in a release, it will be easier to do a version comparison
-// on whether this command line flag is supported.
-func (c *SyncedCluster) cockroachBinSupportsTenantScope(
-	l *logger.Logger, ctx context.Context, node Node,
-) bool {
-	cmd := fmt.Sprintf("%s cert create-client --help | grep '\\--tenant-scope'", cockroachNodeBinary(c, node))
-	sess := c.newSession(l, node, cmd)
-	defer sess.Close()
-
-	return sess.Run(ctx) == nil
-}
-
 // getFile retrieves the given file from the first node in the cluster. The
 // filename is assumed to be relative from the home directory of the node's
 // user.

pkg/security/securitytest/test_certs/regenerate.sh

@@ -7,9 +7,9 @@ rm -f "${dir_n}"/*.{crt,key}
 ./cockroach cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca.key" create-node 127.0.0.1 ::1 localhost *.local
 
 # Create client certs with tenant scopes.
-./cockroach cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca.key" create-client root --tenant-scope 1,2,10,11,20
-./cockroach cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca.key" create-client testuser --tenant-scope 1,2,10,11,20
-./cockroach cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca.key" create-client testuser2 --tenant-scope 1,2,10,11,20
+./cockroach cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca.key" create-client root
+./cockroach cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca.key" create-client testuser
+./cockroach cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca.key" create-client testuser2
 
 # Tenant certs
 ./cockroach mt cert --certs-dir="${dir_n}" --ca-key="${dir_n}/ca-client-tenant.key" create-tenant-client-ca