rpc: *almost* use tenant client certs

pkg/acceptance/localcluster/cluster.go

@@ -272,6 +272,7 @@ func (c *Cluster) makeNode(ctx context.Context, nodeIdx int, cfg NodeConfig) (*N
 		Insecure: true,
 	}
 	rpcCtx := rpc.NewContext(rpc.ContextOptions{
+		TenantID:   roachpb.SystemTenantID,
 		AmbientCtx: log.AmbientContext{Tracer: tracing.NewTracer()},
 		Config:     baseCtx,
 		Clock:      hlc.NewClock(hlc.UnixNano, 0),

pkg/ccl/serverccl/server_sql_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
@@ -42,7 +43,11 @@ func TestSQLServer(t *testing.T) {
 	tc := serverutils.StartTestCluster(t, 3, base.TestClusterArgs{})
 	defer tc.Stopper().Stop(ctx)
 
-	db := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10)})
+	db := serverutils.StartTenant(
+		t,
+		tc.Server(0),
+		base.TestTenantArgs{TenantID: roachpb.MakeTenantID(security.EmbeddedTenantID)},
+	)
 	defer db.Close()
 	r := sqlutils.MakeSQLRunner(db)
 	r.QueryStr(t, `SELECT 1`)

pkg/cli/client_url.go

@@ -19,6 +19,7 @@ import (
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/errors"
@@ -351,7 +352,7 @@ func (cliCtx *cliContext) makeClientConnURL() (url.URL, error) {
 		if userName == "" {
 			userName = security.RootUser
 		}
-		sCtx := rpc.MakeSecurityContext(cliCtx.Config)
+		sCtx := rpc.MakeSecurityContext(cliCtx.Config, roachpb.SystemTenantID)
 		if err := sCtx.LoadSecurityOptions(
 			opts, userName,
 		); err != nil {

pkg/cli/start.go

@@ -31,6 +31,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/build"
 	"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
 	"github.com/cockroachdb/cockroach/pkg/geo/geos"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/server"
@@ -637,7 +638,7 @@ func runStart(cmd *cobra.Command, args []string, disableReplication bool) error
 			// (Re-)compute the client connection URL. We cannot do this
 			// earlier (e.g. above, in the runStart function) because
 			// at this time the address and port have not been resolved yet.
-			sCtx := rpc.MakeSecurityContext(serverCfg.Config)
+			sCtx := rpc.MakeSecurityContext(serverCfg.Config, roachpb.SystemTenantID)
 			pgURL, err := sCtx.PGURL(url.User(security.RootUser))
 			if err != nil {
 				log.Errorf(ctx, "failed computing the URL: %v", err)
@@ -810,7 +811,7 @@ If problems persist, please see %s.`
 			// (Re-)compute the client connection URL. We cannot do this
 			// earlier (e.g. above, in the runStart function) because
 			// at this time the address and port have not been resolved yet.
-			sCtx := rpc.MakeSecurityContext(serverCfg.Config)
+			sCtx := rpc.MakeSecurityContext(serverCfg.Config, roachpb.SystemTenantID)
 			pgURL, err := sCtx.PGURL(url.User(security.RootUser))
 			if err != nil {
 				log.Errorf(ctx, "failed computing the URL: %v", err)
@@ -1352,6 +1353,7 @@ func getClientGRPCConn(
 	clock := hlc.NewClock(hlc.UnixNano, 0)
 	stopper := stop.NewStopper()
 	rpcContext := rpc.NewContext(rpc.ContextOptions{
+		TenantID:   roachpb.SystemTenantID,
 		AmbientCtx: log.AmbientContext{Tracer: cfg.Settings.Tracer},
 		Config:     cfg.Config,
 		Clock:      clock,

pkg/gossip/simulation/network.go

@@ -72,6 +72,7 @@ func NewNetwork(
 		Stopper: stopper,
 	}
 	n.RPCContext = rpc.NewContext(rpc.ContextOptions{
+		TenantID:   roachpb.SystemTenantID,
 		AmbientCtx: log.AmbientContext{Tracer: tracing.NewTracer()},
 		Config:     &base.Config{Insecure: true},
 		Clock:      hlc.NewClock(hlc.UnixNano, time.Nanosecond),

pkg/rpc/context.go

@@ -13,6 +13,7 @@ package rpc
 import (
 	"bytes"
 	"context"
+	"crypto/tls"
 	"encoding/binary"
 	"fmt"
 	"io"
@@ -398,8 +399,9 @@ type connKey struct {
 }
 
 // ContextOptions are passed to NewContext to set up a new *Context.
-// All pointer fields are required.
+// All pointer fields and TenantID are required.
 type ContextOptions struct {
+	TenantID   roachpb.TenantID
 	AmbientCtx log.AmbientContext
 	Config     *base.Config
 	Clock      *hlc.Clock
@@ -409,6 +411,9 @@ type ContextOptions struct {
 }
 
 func (c ContextOptions) validate() error {
+	if c.TenantID == (roachpb.TenantID{}) {
+		return errors.New("must specify TenantID")
+	}
 	if c.Config == nil {
 		return errors.New("Config must be set")
 	}
@@ -434,7 +439,7 @@ func NewContext(opts ContextOptions) *Context {
 
 	ctx := &Context{
 		ContextOptions:  opts,
-		SecurityContext: MakeSecurityContext(opts.Config),
+		SecurityContext: MakeSecurityContext(opts.Config, opts.TenantID),
 		breakerClock: breakerClock{
 			clock: opts.Clock,
 		},
@@ -712,7 +717,17 @@ func (ctx *Context) grpcDialOptions(
 	if ctx.Config.Insecure {
 		dialOpts = append(dialOpts, grpc.WithInsecure())
 	} else {
-		tlsConfig, err := ctx.GetClientTLSConfig()
+		var err error
+		var tlsConfig *tls.Config
+		// TODO(tbg): remove this override when the KV layer can authenticate tenant
+		// client certs.
+		const override = true
+		if override || ctx.tenID == roachpb.SystemTenantID {
+			tlsConfig, err = ctx.GetClientTLSConfig()
+		} else {
+			tlsConfig, err = ctx.GetTenantClientTLSConfig()
+		}
+
 		if err != nil {
 			return nil, err
 		}

pkg/rpc/context_testutils.go

@@ -12,6 +12,7 @@ package rpc
 
 import (
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -71,6 +72,7 @@ func NewInsecureTestingContextWithKnobs(
 	clock *hlc.Clock, stopper *stop.Stopper, knobs ContextTestingKnobs,
 ) *Context {
 	return NewContext(ContextOptions{
+		TenantID:   roachpb.SystemTenantID,
 		AmbientCtx: log.AmbientContext{Tracer: tracing.NewTracer()},
 		Config:     &base.Config{Insecure: true},
 		Clock:      clock,

pkg/rpc/tls.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
@@ -55,6 +56,7 @@ func wrapError(err error) error {
 type SecurityContext struct {
 	security.CertsLocator
 	config *base.Config
+	tenID  roachpb.TenantID
 	lazy   struct {
 		// The certificate manager. Must be accessed through GetCertificateManager.
 		certificateManager lazyCertificateManager
@@ -66,10 +68,11 @@ type SecurityContext struct {
 // MakeSecurityContext makes a SecurityContext.
 //
 // TODO(tbg): don't take a whole Config. This can be trimmed down significantly.
-func MakeSecurityContext(cfg *base.Config) SecurityContext {
+func MakeSecurityContext(cfg *base.Config, tenID roachpb.TenantID) SecurityContext {
 	return SecurityContext{
 		CertsLocator: security.MakeCertsLocator(cfg.SSLCertsDir),
 		config:       cfg,
+		tenID:        tenID,
 	}
 }
 
@@ -78,8 +81,13 @@ func MakeSecurityContext(cfg *base.Config) SecurityContext {
 // fails eagerly.
 func (ctx *SecurityContext) GetCertificateManager() (*security.CertificateManager, error) {
 	ctx.lazy.certificateManager.Do(func() {
+		var opts []security.Option
+		if !roachpb.IsSystemTenantID(ctx.tenID.ToUint64()) {
+			opts = append(opts, security.ForTenant(ctx.tenID.String()))
+		}
 		ctx.lazy.certificateManager.cm, ctx.lazy.certificateManager.err =
-			security.NewCertificateManager(ctx.config.SSLCertsDir)
+			security.NewCertificateManager(ctx.config.SSLCertsDir, opts...)
+
 		if ctx.lazy.certificateManager.err == nil && !ctx.config.Insecure {
 			infos, err := ctx.lazy.certificateManager.cm.ListCertificates()
 			if err != nil {
@@ -138,6 +146,30 @@ func (ctx *SecurityContext) GetClientTLSConfig() (*tls.Config, error) {
 	return tlsCfg, nil
 }
 
+// GetTenantClientTLSConfig returns the client TLS config for the tenant, provided
+// the SecurityContext operates on behalf of a secondary tenant (i.e. not the
+// system tenant).
+//
+// If Insecure is true, return a nil config, otherwise retrieves the client
+// certificate for the configured tenant from the cert manager.
+func (ctx *SecurityContext) GetTenantClientTLSConfig() (*tls.Config, error) {
+	// Early out.
+	if ctx.config.Insecure {
+		return nil, nil
+	}
+
+	cm, err := ctx.GetCertificateManager()
+	if err != nil {
+		return nil, wrapError(err)
+	}
+
+	tlsCfg, err := cm.GetTenantClientTLSConfig()
+	if err != nil {
+		return nil, wrapError(err)
+	}
+	return tlsCfg, nil
+}
+
 // 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.

pkg/security/certificate_manager.go

@@ -165,7 +165,7 @@ type CertificateMetrics struct {
 	TenantClientExpiration   *metric.Gauge
 }
 
-func makeCertificateManager(certsDir string, opts ...func(*cmOptions)) *CertificateManager {
+func makeCertificateManager(certsDir string, opts ...Option) *CertificateManager {
 	var o cmOptions
 	for _, fn := range opts {
 		fn(&o)
@@ -195,17 +195,20 @@ type cmOptions struct {
 	tenantIdentifier string
 }
 
+// Option is an option to NewCertificateManager.
+type Option func(*cmOptions)
+
 // ForTenant is an option to NewCertificateManager which ties the manager to
 // the provided tenant. Without this option, tenant client certs are not
 // available.
-func ForTenant(tenantIdentifier string) func(*cmOptions) {
+func ForTenant(tenantIdentifier string) Option {
 	return func(opts *cmOptions) {
 		opts.tenantIdentifier = tenantIdentifier
 	}
 }
 
 // NewCertificateManager creates a new certificate manager.
-func NewCertificateManager(certsDir string, opts ...func(*cmOptions)) (*CertificateManager, error) {
+func NewCertificateManager(certsDir string, opts ...Option) (*CertificateManager, error) {
 	cm := makeCertificateManager(certsDir, opts...)
 	return cm, cm.LoadCertificates()
 }
@@ -214,9 +217,7 @@ func NewCertificateManager(certsDir string, opts ...func(*cmOptions)) (*Certific
 // The certsDir is created if it does not exist.
 // This should only be called when generating certificates, the server has
 // no business creating the certs directory.
-func NewCertificateManagerFirstRun(
-	certsDir string, opts ...func(*cmOptions),
-) (*CertificateManager, error) {
+func NewCertificateManagerFirstRun(certsDir string, opts ...Option) (*CertificateManager, error) {
 	cm := makeCertificateManager(certsDir, opts...)
 	if err := NewCertificateLoader(cm.certsDir).MaybeCreateCertsDir(); err != nil {
 		return nil, err

pkg/security/certs_rotation_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
@@ -99,7 +100,7 @@ func TestRotateCerts(t *testing.T) {
 	// Test client with the same certs.
 	clientContext := testutils.NewNodeTestBaseContext()
 	clientContext.SSLCertsDir = certsDir
-	firstSCtx := rpc.MakeSecurityContext(clientContext)
+	firstSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 	firstClient, err := firstSCtx.GetHTTPClient()
 	if err != nil {
 		t.Fatalf("could not create http client: %v", err)
@@ -131,7 +132,7 @@ func TestRotateCerts(t *testing.T) {
 	clientContext = testutils.NewNodeTestBaseContext()
 	clientContext.SSLCertsDir = certsDir
 
-	secondSCtx := rpc.MakeSecurityContext(clientContext)
+	secondSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 	secondClient, err := secondSCtx.GetHTTPClient()
 	if err != nil {
 		t.Fatalf("could not create http client: %v", err)
@@ -200,7 +201,7 @@ func TestRotateCerts(t *testing.T) {
 	// This is HTTP and succeeds because we do not ask for or verify client certificates.
 	clientContext = testutils.NewNodeTestBaseContext()
 	clientContext.SSLCertsDir = certsDir
-	thirdSCtx := rpc.MakeSecurityContext(clientContext)
+	thirdSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 	thirdClient, err := thirdSCtx.GetHTTPClient()
 	if err != nil {
 		t.Fatalf("could not create http client: %v", err)

pkg/security/certs_tenant_test.go

@@ -91,7 +91,7 @@ func testTenantCertificatesInner(t *testing.T, embedded bool) {
 		certsDir, cleanup = makeTenantCerts(t, tenant)
 		defer cleanup()
 	} else {
-		certsDir = security.EmbeddedTenantCertsDir
+		certsDir = security.EmbeddedCertsDir
 		tenant = fmt.Sprint(security.EmbeddedTenantID)
 	}
 
@@ -113,6 +113,7 @@ func testTenantCertificatesInner(t *testing.T, embedded bool) {
 	// server (which will validate them using the tenant CA).
 	clientTLSConfig, err := cm.GetTenantClientTLSConfig()
 	require.NoError(t, err)
+	require.NotNil(t, clientTLSConfig)
 
 	// Set up a HTTPS server using server TLS config, set up a http client using the
 	// client TLS config, make a request.

pkg/security/certs_test.go

@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
@@ -382,7 +383,7 @@ func TestUseCerts(t *testing.T) {
 	// Insecure mode.
 	clientContext := testutils.NewNodeTestBaseContext()
 	clientContext.Insecure = true
-	sCtx := rpc.MakeSecurityContext(clientContext)
+	sCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 	httpClient, err := sCtx.GetHTTPClient()
 	if err != nil {
 		t.Fatal(err)
@@ -402,7 +403,7 @@ func TestUseCerts(t *testing.T) {
 	clientContext = testutils.NewNodeTestBaseContext()
 	clientContext.SSLCertsDir = certsDir
 	{
-		secondSCtx := rpc.MakeSecurityContext(clientContext)
+		secondSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 		httpClient, err = secondSCtx.GetHTTPClient()
 	}
 	if err != nil {
@@ -472,7 +473,7 @@ func TestUseSplitCACerts(t *testing.T) {
 	// Insecure mode.
 	clientContext := testutils.NewNodeTestBaseContext()
 	clientContext.Insecure = true
-	sCtx := rpc.MakeSecurityContext(clientContext)
+	sCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 	httpClient, err := sCtx.GetHTTPClient()
 	if err != nil {
 		t.Fatal(err)
@@ -492,7 +493,7 @@ func TestUseSplitCACerts(t *testing.T) {
 	clientContext = testutils.NewNodeTestBaseContext()
 	clientContext.SSLCertsDir = certsDir
 	{
-		secondSCtx := rpc.MakeSecurityContext(clientContext)
+		secondSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 		httpClient, err = secondSCtx.GetHTTPClient()
 	}
 	if err != nil {
@@ -596,7 +597,7 @@ func TestUseWrongSplitCACerts(t *testing.T) {
 	// Insecure mode.
 	clientContext := testutils.NewNodeTestBaseContext()
 	clientContext.Insecure = true
-	sCtx := rpc.MakeSecurityContext(clientContext)
+	sCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 	httpClient, err := sCtx.GetHTTPClient()
 	if err != nil {
 		t.Fatal(err)
@@ -616,7 +617,7 @@ func TestUseWrongSplitCACerts(t *testing.T) {
 	clientContext = testutils.NewNodeTestBaseContext()
 	clientContext.SSLCertsDir = certsDir
 	{
-		secondCtx := rpc.MakeSecurityContext(clientContext)
+		secondCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
 		httpClient, err = secondCtx.GetHTTPClient()
 	}
 	if err != nil {