diff --git a/pkg/cli/cliflags/flags.go b/pkg/cli/cliflags/flags.go index e028b470214a..57e845515a44 100644 --- a/pkg/cli/cliflags/flags.go +++ b/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{ diff --git a/pkg/cli/context.go b/pkg/cli/context.go index b97de80a087d..4103bc8cf933 100644 --- a/pkg/cli/context.go +++ b/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{ diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index fa8dbc23a29a..d30325c381a1 100644 --- a/pkg/cli/flags.go +++ b/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) diff --git a/pkg/cmd/roachtest/tests/BUILD.bazel b/pkg/cmd/roachtest/tests/BUILD.bazel index 444c541f4d65..61d976e7d765 100644 --- a/pkg/cmd/roachtest/tests/BUILD.bazel +++ b/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", diff --git a/pkg/cmd/roachtest/tests/multitenant_utils.go b/pkg/cmd/roachtest/tests/multitenant_utils.go index 686feb9d78a7..35603c449a53 100644 --- a/pkg/cmd/roachtest/tests/multitenant_utils.go +++ b/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 diff --git a/pkg/roachprod/install/cluster_synced.go b/pkg/roachprod/install/cluster_synced.go index e08fb8c182b4..dd4f7c21190f 100644 --- a/pkg/roachprod/install/cluster_synced.go +++ b/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. diff --git a/pkg/security/securitytest/test_certs/regenerate.sh b/pkg/security/securitytest/test_certs/regenerate.sh index 67d7e2fa903e..135764f4ed70 100755 --- a/pkg/security/securitytest/test_certs/regenerate.sh +++ b/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