security: new sub-package for certificate/key paths (cli refactor sequence 1/n)

pkg/BUILD.bazel

@@ -57,7 +57,6 @@ ALL_TESTS = [
     "//pkg/ccl/telemetryccl:telemetryccl_test",
     "//pkg/ccl/testccl/sqlccl:sqlccl_test",
     "//pkg/ccl/testccl/workload/schemachange:schemachange_test",
-    "//pkg/ccl/upgradeccl/upgradessccl:upgradesccl_test",
     "//pkg/ccl/upgradeccl/upgradessccl:upgradessccl_test",
     "//pkg/ccl/utilccl/sampledataccl:sampledataccl_test",
     "//pkg/ccl/utilccl:utilccl_test",

pkg/ccl/upgradeccl/upgradessccl/BUILD.bazel

@@ -1,34 +1,5 @@
 load("@io_bazel_rules_go//go:def.bzl", "go_test")
 
-go_test(
-    name = "upgradesccl_test",
-    srcs = [
-        "main_test.go",
-        "seed_span_counts_external_test.go",
-        "seed_tenant_span_configs_external_test.go",
-    ],
-    deps = [
-        "//pkg/base",
-        "//pkg/ccl/kvccl/kvtenantccl",
-        "//pkg/clusterversion",
-        "//pkg/keys",
-        "//pkg/roachpb",
-        "//pkg/security",
-        "//pkg/security/securitytest",
-        "//pkg/security/username",
-        "//pkg/server",
-        "//pkg/settings/cluster",
-        "//pkg/spanconfig",
-        "//pkg/testutils",
-        "//pkg/testutils/serverutils",
-        "//pkg/testutils/sqlutils",
-        "//pkg/testutils/testcluster",
-        "//pkg/util/leaktest",
-        "//pkg/util/log",
-        "@com_github_stretchr_testify//require",
-    ],
-)
-
 go_test(
     name = "upgradessccl_test",
     srcs = [

pkg/cli/BUILD.bazel

@@ -132,6 +132,7 @@ go_library(
         "//pkg/roachpb",
         "//pkg/rpc",
         "//pkg/security",
+        "//pkg/security/certnames",
         "//pkg/security/securitytest",
         "//pkg/security/username",
         "//pkg/server",

pkg/cli/client_url.go

@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/security/certnames"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/server/pgurl"
 	"github.com/cockroachdb/errors"
@@ -283,14 +284,14 @@ func (u urlParser) setInternal(v string, warn bool) error {
 			if cliCtx.sqlConnUser != "" {
 				userName, _ = username.MakeSQLUsernameFromUserInput(cliCtx.sqlConnUser, username.PurposeValidation)
 			}
-			if err := tryCertsDir("sslrootcert", caCertPath, security.CACertFilename()); err != nil {
+			if err := tryCertsDir("sslrootcert", caCertPath, certnames.CACertFilename()); err != nil {
 				return err
 			}
 			if clientCertEnabled, clientCertPath, clientKeyPath := parsedURL.GetAuthnCert(); clientCertEnabled {
-				if err := tryCertsDir("sslcert", clientCertPath, security.ClientCertFilename(userName)); err != nil {
+				if err := tryCertsDir("sslcert", clientCertPath, certnames.ClientCertFilename(userName)); err != nil {
 					return err
 				}
-				if err := tryCertsDir("sslkey", clientKeyPath, security.ClientKeyFilename(userName)); err != nil {
+				if err := tryCertsDir("sslkey", clientKeyPath, certnames.ClientKeyFilename(userName)); err != nil {
 					return err
 				}
 			}

pkg/cli/connect.go

@@ -17,7 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
 	"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
-	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/security/certnames"
 	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
@@ -65,7 +65,7 @@ func runConnectInit(cmd *cobra.Command, args []string) (retErr error) {
 
 	// If the node cert already exists, skip all the complexity of setting up
 	// servers, etc.
-	cl := security.MakeCertsLocator(baseCfg.SSLCertsDir)
+	cl := certnames.MakeLocator(baseCfg.SSLCertsDir)
 	if exists, err := cl.HasNodeCert(); err != nil {
 		return err
 	} else if exists {

pkg/rpc/BUILD.bazel

@@ -30,6 +30,7 @@ go_library(
         "//pkg/keys",
         "//pkg/roachpb",
         "//pkg/security",
+        "//pkg/security/certnames",
         "//pkg/security/securityassets",
         "//pkg/security/username",
         "//pkg/server/pgurl",

pkg/rpc/pg.go

@@ -65,7 +65,7 @@ func (ctx *SecurityContext) LoadSecurityOptions(u *pgurl.URL, user username.SQLU
 				}
 				if ourCACert := cm.CACert(); ourCACert != nil {
 					// The CM has a CA cert. Use that.
-					caCertPath = cm.FullPath(ourCACert)
+					caCertPath = cm.FullPath(ourCACert.Filename)
 				}
 			}
 			// Fallback: if caCertPath was not assigned above, either

pkg/rpc/tls.go

@@ -25,6 +25,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/security/certnames"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/severity"
 	"github.com/cockroachdb/errors"
@@ -55,7 +56,7 @@ func wrapError(err error) error {
 // SecurityContext is a wrapper providing transport security helpers such as
 // the certificate manager.
 type SecurityContext struct {
-	security.CertsLocator
+	certnames.Locator
 	security.TLSSettings
 	config *base.Config
 	tenID  roachpb.TenantID
@@ -77,10 +78,10 @@ func MakeSecurityContext(
 		panic(errors.AssertionFailedf("programming error: tenant ID not defined"))
 	}
 	return SecurityContext{
-		CertsLocator: security.MakeCertsLocator(cfg.SSLCertsDir),
-		TLSSettings:  tlsSettings,
-		config:       cfg,
-		tenID:        tenID,
+		Locator:     certnames.MakeLocator(cfg.SSLCertsDir),
+		TLSSettings: tlsSettings,
+		config:      cfg,
+		tenID:       tenID,
 	}
 }
 

pkg/security/BUILD.bazel

@@ -22,6 +22,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/clusterversion",
+        "//pkg/security/certnames",
         "//pkg/security/password",
         "//pkg/security/securityassets",
         "//pkg/security/username",

pkg/security/certificate_loader.go

@@ -19,6 +19,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/security/certnames"
 	"github.com/cockroachdb/cockroach/pkg/security/securityassets"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
@@ -73,9 +74,6 @@ const (
 	// Maximum allowable permissions if file is owned by root.
 	maxGroupKeyPermissions os.FileMode = 0740
 
-	// Filename extensions.
-	certExtension = `.crt`
-	keyExtension  = `.key`
 	// Certificate directory permissions.
 	defaultCertsDirPerm = 0700
 )
@@ -143,10 +141,6 @@ type CertInfo struct {
 	Error error
 }
 
-func isCertificateFile(filename string) bool {
-	return strings.HasSuffix(filename, certExtension)
-}
-
 // CertInfoFromFilename takes a filename and attempts to determine the
 // certificate usage (ca, node, etc..).
 func CertInfoFromFilename(filename string) (*CertInfo, error) {
@@ -164,53 +158,56 @@ func CertInfoFromFilename(filename string) (*CertInfo, error) {
 	case `ca`:
 		fileUsage = CAPem
 		if numParts != 2 {
-			return nil, errors.Errorf("CA certificate filename should match ca%s", certExtension)
+			return nil, errors.Errorf("CA certificate filename should match %s", certnames.CACertFilename())
 		}
 	case `ca-client`:
 		fileUsage = ClientCAPem
 		if numParts != 2 {
-			return nil, errors.Errorf("client CA certificate filename should match ca-client%s", certExtension)
+			return nil, errors.Errorf("client CA certificate filename should match %s", certnames.ClientCACertFilename())
 		}
 	case `ca-client-tenant`:
 		fileUsage = TenantCAPem
 		if numParts != 2 {
-			return nil, errors.Errorf("tenant CA certificate filename should match ca%s", certExtension)
+			return nil, errors.Errorf("tenant CA certificate filename should match %s", certnames.TenantClientCACertFilename())
 		}
 	case `ca-ui`:
 		fileUsage = UICAPem
 		if numParts != 2 {
-			return nil, errors.Errorf("UI CA certificate filename should match ca-ui%s", certExtension)
+			return nil, errors.Errorf("UI CA certificate filename should match %s", certnames.UICACertFilename())
 		}
 	case `node`:
 		fileUsage = NodePem
 		if numParts != 2 {
-			return nil, errors.Errorf("node certificate filename should match node%s", certExtension)
+			return nil, errors.Errorf("node certificate filename should match %s", certnames.NodeCertFilename())
 		}
 	case `ui`:
 		fileUsage = UIPem
 		if numParts != 2 {
-			return nil, errors.Errorf("UI certificate filename should match ui%s", certExtension)
+			return nil, errors.Errorf("UI certificate filename should match %s", certnames.UIServerCertFilename())
 		}
 	case `client`:
 		fileUsage = ClientPem
 		// Strip prefix and suffix and re-join middle parts.
 		name = strings.Join(parts[1:numParts-1], `.`)
 		if len(name) == 0 {
-			return nil, errors.Errorf("client certificate filename should match client.<user>%s", certExtension)
+			return nil, errors.Errorf("client certificate filename should match %s",
+				certnames.ClientCertFilename(username.MakeSQLUsernameFromPreNormalizedString("<user>")))
 		}
 	case `client-tenant`:
 		fileUsage = TenantPem
 		// Strip prefix and suffix and re-join middle parts.
 		name = strings.Join(parts[1:numParts-1], `.`)
 		if len(name) == 0 {
-			return nil, errors.Errorf("tenant certificate filename should match client-tenant.<tenantid>%s", certExtension)
+			return nil, errors.Errorf("tenant certificate filename should match %s",
+				certnames.TenantCertFilename("<tenantid>"))
 		}
 	case `tenant-signing`:
 		fileUsage = TenantSigningPem
 		// Strip prefix and suffix and re-join middle parts.
 		name = strings.Join(parts[1:numParts-1], `.`)
 		if len(name) == 0 {
-			return nil, errors.Errorf("tenant signing certificate filename should match tenant-signing.<tenantid>%s", certExtension)
+			return nil, errors.Errorf("tenant signing certificate filename should match %s",
+				certnames.TenantSigningCertFilename("<tenantid>"))
 		}
 	default:
 		return nil, errors.Errorf("unknown prefix %q", prefix)
@@ -304,7 +301,7 @@ func (cl *CertificateLoader) Load() error {
 			continue
 		}
 
-		if !isCertificateFile(filename) {
+		if !certnames.IsCertificateFilename(filename) {
 			if log.V(3) {
 				log.Infof(context.Background(), "skipping non-certificate file %s", filename)
 			}
@@ -352,7 +349,7 @@ func (cl *CertificateLoader) findKey(ci *CertInfo) error {
 		return nil
 	}
 
-	keyFilename := strings.TrimSuffix(ci.Filename, certExtension) + keyExtension
+	keyFilename := certnames.KeyForCert(ci.Filename)
 	fullKeyPath := filepath.Join(cl.certsDir, keyFilename)
 
 	// Stat the file. This follows symlinks.