rpc,security, cli: Add cert-distinguished-name root/node cli flag

Previous in sequence: #119958
informs #110616, #118750
fixes CRDB-35884
Epic CRDB-34126

We will be adding 2 new cli flags `root-cert-distinguished-name` and
`node-cert-distinguished-name` to provide option to have subject DN for root and
node user during server startup. This will enforce that certificate provided by
client exactly matches the value set by the above flags both for sql client and
RPC authentication. This is needed because subject role option cannot be set for
root and node users.

Post this the plan is to add a cluster setting
`server.client_cert.subject_required` which will mandate that any auth which
happens should verify certSubject with rootSubject in case of root user, with
nodeSubject in case of node user, with roleSubject otherwise.

Release note: None

build/github/acceptance-test.sh

@@ -2,6 +2,10 @@
 
 set -euxo pipefail
 
+set +x
+export COCKROACH_DEV_LICENSE=$(gcloud secrets versions access 1 --secret=cockroach-dev-license)
+set -x
+
 bazel build --config crosslinux //pkg/cmd/cockroach-short \
     --bes_keywords integration-test-artifact-build \
     --jobs 100 $(./build/github/engflow-args.sh)
@@ -18,6 +22,7 @@ bazel test //pkg/acceptance:acceptance_test \
   "--test_tmpdir=$ARTIFACTSDIR" \
   --test_arg=-l="$ARTIFACTSDIR" \
   --test_arg=-b=$COCKROACH \
+  --test_env=COCKROACH_DEV_LICENSE \
   --test_env=TZ=America/New_York \
   --test_timeout=1800 \
   --build_event_binary_file=bes.bin

pkg/acceptance/BUILD.bazel

@@ -23,6 +23,7 @@ go_library(
         "//pkg/testutils/serverutils",  # keep
         "//pkg/testutils/skip",
         "//pkg/testutils/testcluster",  # keep
+        "//pkg/util/envutil",
         "//pkg/util/log",
         "//pkg/util/randutil",  # keep
         "//pkg/util/stop",

pkg/acceptance/util_docker.go

@@ -26,6 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/build/bazel"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
+	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/containerd/containerd/platforms"
 	"github.com/docker/docker/api/types"
@@ -221,6 +222,7 @@ func runTestDockerCLI(t *testing.T, testNameSuffix, testFilePath string) {
 	containerConfig.Env = []string{
 		"CI=1", // Disables the initial color query by the termenv library.
 		fmt.Sprintf("PGUSER=%s", username.RootUser),
+		fmt.Sprintf("COCKROACH_DEV_LICENSE=%s", envutil.EnvOrDefaultString("COCKROACH_DEV_LICENSE", "")),
 	}
 	ctx := context.Background()
 	if err := testDockerOneShot(ctx, t, "cli_test_"+testNameSuffix, containerConfig); err != nil {

pkg/cli/cliflags/flags.go

@@ -873,6 +873,26 @@ fields. It is permissible for the <cert-principal> string to contain colons.
 `,
 	}
 
+	RootCertDistinguishedName = FlagInfo{
+		Name: "root-cert-distinguished-name",
+		Description: `
+A string of comma separated list of distinguished-name
+<attribute-type>=<attribute-value> mappings in accordance with RFC4514 for the root
+user. This strictly needs to match the DN subject in the client certificate
+provided for root user if this flag is set.
+`,
+	}
+
+	NodeCertDistinguishedName = FlagInfo{
+		Name: "node-cert-distinguished-name",
+		Description: `
+A string of comma separated list of distinguished-name
+<attribute-type>=<attribute-value> mappings in accordance with RFC4514 for the node
+user. This strictly needs to match the DN subject in the client certificate
+provided for node user if this flag is set.
+`,
+	}
+
 	CAKey = FlagInfo{
 		Name:        "ca-key",
 		EnvVar:      "COCKROACH_CA_KEY",

pkg/cli/context.go

@@ -480,6 +480,8 @@ var startCtx struct {
 	serverSSLCertsDir      string
 	serverCertPrincipalMap []string
 	serverListenAddr       string
+	serverRootCertDN       string
+	serverNodeCertDN       string
 
 	// The TLS auto-handshake parameters.
 	initToken             string
@@ -533,6 +535,8 @@ func setStartContextDefaults() {
 	startCtx.serverInsecure = baseCfg.Insecure
 	startCtx.serverSSLCertsDir = base.DefaultCertsDirectory
 	startCtx.serverCertPrincipalMap = nil
+	startCtx.serverRootCertDN = ""
+	startCtx.serverNodeCertDN = ""
 	startCtx.serverListenAddr = ""
 	startCtx.unencryptedLocalhostHTTP = false
 	startCtx.tempDir = ""

pkg/cli/flags.go

@@ -493,6 +493,12 @@ func init() {
 		// Certificate principal map.
 		cliflagcfg.StringSliceFlag(f, &startCtx.serverCertPrincipalMap, cliflags.CertPrincipalMap)
 
+		// Root cert distinguished name
+		cliflagcfg.StringFlag(f, &startCtx.serverRootCertDN, cliflags.RootCertDistinguishedName)
+
+		// Node cert distinguished name
+		cliflagcfg.StringFlag(f, &startCtx.serverNodeCertDN, cliflags.NodeCertDistinguishedName)
+
 		// Cluster name verification.
 		cliflagcfg.VarFlag(f, clusterNameSetter{&baseCfg.ClusterName}, cliflags.ClusterName)
 		cliflagcfg.BoolFlag(f, &baseCfg.DisableClusterNameVerification, cliflags.DisableClusterNameVerification)
@@ -577,6 +583,12 @@ func init() {
 		// All certs command want to map CNs to SQL principals.
 		cliflagcfg.StringSliceFlag(f, &certCtx.certPrincipalMap, cliflags.CertPrincipalMap)
 
+		// Root cert distinguished name
+		cliflagcfg.StringFlag(f, &startCtx.serverRootCertDN, cliflags.RootCertDistinguishedName)
+
+		// Node cert distinguished name
+		cliflagcfg.StringFlag(f, &startCtx.serverNodeCertDN, cliflags.NodeCertDistinguishedName)
+
 		if cmd == listCertsCmd {
 			// The 'list' subcommand does not write to files and thus does
 			// not need the arguments below.
@@ -1083,6 +1095,12 @@ func extraServerFlagInit(cmd *cobra.Command) error {
 	if err := security.SetCertPrincipalMap(startCtx.serverCertPrincipalMap); err != nil {
 		return err
 	}
+	if err := security.SetRootSubject(startCtx.serverRootCertDN); err != nil {
+		return err
+	}
+	if err := security.SetNodeSubject(startCtx.serverNodeCertDN); err != nil {
+		return err
+	}
 	serverCfg.User = username.NodeUserName()
 	serverCfg.Insecure = startCtx.serverInsecure
 	serverCfg.SSLCertsDir = startCtx.serverSSLCertsDir

pkg/cli/interactive_tests/test_distinguished_name_validation.tcl

@@ -0,0 +1,161 @@
+#! /usr/bin/env expect -f
+
+source [file join [file dirname $argv0] common.tcl]
+variable certs_dir "my-safe-directory"
+variable custom_ca_dir "custom-ca-directory"
+variable db_dir "logs/db"
+
+set ::env(COCKROACH_INSECURE) "false"
+set ::env(COCKROACH_HOST) "localhost"
+spawn /bin/bash
+send "PS1=':''/# '\r"
+
+variable prompt ":/# "
+eexpect $prompt
+
+
+send "mkdir -p $certs_dir\r"
+eexpect $prompt
+send "mkdir -p $custom_ca_dir\r"
+eexpect $prompt
+
+send "$argv cert create-ca --certs-dir=$certs_dir --ca-key=$certs_dir/ca.key\r"
+eexpect $prompt
+
+# Copy openssl CA cnf for generating custom client certs.
+set ca_cnf_file [file join [file dirname $argv0] "ocsp_ca.cnf"]
+send "cp $ca_cnf_file $custom_ca_dir/ca.cnf\r"
+eexpect $prompt
+
+report "GENERATING serial.txt index.txt files"
+send "touch index.txt; echo '01' > serial.txt\r"
+eexpect $prompt
+
+send "$argv cert create-node localhost --certs-dir=$certs_dir --ca-key=$certs_dir/ca.key\r"
+eexpect $prompt
+send "$argv cert create-client root --certs-dir=$certs_dir --ca-key=$certs_dir/ca.key\r"
+eexpect $prompt
+
+proc start_secure_server {argv certs_dir extra} {
+    report "BEGIN START SECURE SERVER"
+    send "$argv start-single-node --host=localhost --socket-dir=. --certs-dir=$certs_dir --store=$::db_dir --pid-file=server_pid --background $extra >>expect-cmd.log 2>&1\r";
+    eexpect $::prompt
+    send "$argv sql --certs-dir=$certs_dir -e 'select 1'\r";
+    eexpect $::prompt
+    report "SETTING COCKROACH DEV LICENSE KEY"
+    set stmt1 "SET CLUSTER SETTING enterprise.license = \"$::env(COCKROACH_DEV_LICENSE)\";"
+    set stmt2 "SET CLUSTER SETTING cluster.organization = \"Cockroach Labs - Production Testing\";"
+    set license_sql_file "license_settings.sql"
+    set fo [open $license_sql_file w]
+    foreach sql_stmts {stmt1 stmt2} {
+        puts $fo [set $sql_stmts]
+    }
+    close $fo
+    system "$argv sql --certs-dir=$certs_dir --file $license_sql_file"
+    report "END START SECURE SERVER"
+}
+
+proc restart_secure_server_distinguished_name_flags {argv certs_dir root_dn node_dn extra} {
+    stop_server $argv
+    report "BEGIN START SECURE SERVER WITH DN FLAGS"
+    send "rm -f server_pid;
+            $argv start-single-node --host=localhost --root-cert-distinguished-name='$root_dn' --node-cert-distinguished-name='$node_dn' --socket-dir=. --certs-dir=$certs_dir --store=$::db_dir --pid-file=server_pid --background $extra >>expect-cmd.log 2>&1;\r"
+    eexpect $::prompt
+    report "END START SECURE SERVER WITH DN FLAGS"
+}
+
+proc expect_exit_status {expected} {
+    set status [lindex [wait] 3]
+    if {$status != $expected} {
+        report "unexpected exit status $status"
+        exit 1
+    }
+}
+
+proc create_user_cert {argv certs_dir name} {
+    report "GENERATING CERT FOR USER $name"
+    send "openssl genrsa -out $::certs_dir/client.$name.key\r"
+    eexpect $::prompt
+    send "openssl req -new -key $::certs_dir/client.$name.key -out client.$name.csr -batch -subj /O=Cockroach/CN=$name\r"
+    eexpect $::prompt
+    send "openssl ca -config $::custom_ca_dir/ca.cnf -keyfile $::certs_dir/ca.key -cert $::certs_dir/ca.crt -policy signing_policy -extensions signing_client_req -out $certs_dir/client.$name.crt -outdir . -in client.$name.csr -batch\r"
+    eexpect $::prompt
+    # Uncomment the next line to see more details about the generated cert
+    #system "openssl x509 -in $::certs_dir/client.$name.crt -text"
+    send "$argv sql --certs-dir=$::certs_dir -e 'create user $name'\r"
+    eexpect $::prompt
+    send "$argv sql --certs-dir=$::certs_dir --user=$name -e 'select 1'\r"
+    eexpect $::prompt
+}
+
+proc generate_root_or_node_cert {argv certs_dir name} {
+    report "GENERATING CERT FOR USER $name"
+    send "openssl genrsa -out $::certs_dir/client.$name.key \r"
+    eexpect $::prompt
+    send "openssl req -new -key $::certs_dir/client.$name.key -out client.$name.csr -batch -subj /O=Cockroach/CN=$name \r"
+    eexpect $::prompt
+    send "openssl ca -config $::custom_ca_dir/ca.cnf -keyfile $::certs_dir/ca.key -cert $::certs_dir/ca.crt -policy signing_policy -extensions signing_client_req -out $certs_dir/client.$name.crt -outdir . -in client.$name.csr -batch \r"
+    eexpect $::prompt
+    # Uncomment the next line to see more details about the generated cert
+    #system "openssl x509 -in $::certs_dir/client.$name.crt -text"
+    send "$argv sql --certs-dir=$::certs_dir --user=$name -e 'select 1' \r"
+    eexpect $::prompt
+}
+
+proc set_role_subject_for_user {argv name role_subject} {
+    report "SETTING SUBJECT ROLE OPTION for USER $name with SUBJECT $role_subject"
+    send "$argv sql --certs-dir=$::certs_dir -e 'alter role $name with subject \"$role_subject\" login';\r"
+    eexpect $::prompt
+}
+
+start_secure_server $argv $certs_dir ""
+
+# Create users and make sure they can each log in.
+create_user_cert $argv $certs_dir goofus
+create_user_cert $argv $certs_dir gallant
+
+# Check cert still works without setting role subject option
+send "$argv sql --certs-dir=$certs_dir --user=goofus -e 'select 1'\r"
+eexpect $::prompt
+
+report "Validating subject role option can be set and enforced"
+
+# Set invalid role subject option for user and check login fails
+set_role_subject_for_user $argv goofus "O=NotCockroach,CN=invalid"
+start_test "invalid role option for cert user goofus"
+send "$argv sql --certs-dir=$certs_dir --user=goofus -e 'select 1'\r"
+eexpect "certificate authentication failed for user \"goofus\""
+end_test
+
+# Set valid role subject option for user and check login passes
+set_role_subject_for_user $argv goofus "O=Cockroach,CN=goofus"
+start_test "valid role option for cert user goofus"
+send "$argv sql --certs-dir=$certs_dir --user=goofus -e 'select 1'\r"
+eexpect $::prompt
+end_test
+
+report "Validating node-cert-distinguished-nane and root-cert-distinguished-name can be set and enforced"
+# create a new root certificate using custom ca
+send "rm -f $certs_dir/client.root.*\r"
+eexpect $::prompt
+generate_root_or_node_cert $argv $certs_dir root
+
+# Set invalid root-cert-distinguished-nane for cockroach server and check root login fails
+set root_dn "O=foo,CN=invalid"
+# need to provide correct node dn to start cockroach server as it depends on this to be equal to node.crt dn subject
+set node_dn "O=Cockroach,CN=node"
+restart_secure_server_distinguished_name_flags $argv $certs_dir "O=foo,CN=invalid" "O=bar,CN=invalid" ""
+start_test "invalid root-cert-distinguished-name"
+send "$argv sql --certs-dir=$certs_dir --user=root -e 'select 1'\r"
+eexpect "certificate authentication failed for user \"root\""
+end_test
+
+# Set valid root-cert-distinguished-nane for cockroach server and check login passes
+stop_server $argv
+restart_secure_server_distinguished_name_flags $argv $certs_dir "O=Cockroach,CN=root" "O=Cockroach,CN=node" ""
+start_test "valid node-cert-distinguished-nane and root-cert-distinguished-name"
+send "$argv sql --certs-dir=$certs_dir --user=root -e 'select 1'\r"
+eexpect $::prompt
+end_test
+
+stop_server $argv

pkg/rpc/auth.go

@@ -306,19 +306,30 @@ func (a kvAuth) authenticateNetworkRequest(ctx context.Context) (authnResult, er
 	// In that case, we only allow RPCs if the principal is 'node' or
 	// 'root' and the tenant scope in the cert matches this server
 	// (either the cert has scope "global" or its scope tenant ID
-	// matches our own).
+	// matches our own). The client could also present a certificate with subject
+	// DN equalling rootSubject or nodeSubject set using
+	// root-cert-distinguished-name and node-cert-distinguished-name cli flags
+	// respectively.
 	//
 	// TODO(benesch): the vast majority of RPCs should be limited to
 	// just NodeUser. This is not a security concern, as RootUser has
 	// access to read and write all data, merely good hygiene. For
 	// example, there is no reason to permit the root user to send raw
 	// Raft RPCs.
-	certUserScope, err := security.GetCertificateUserScope(clientCert)
-	if err != nil {
-		return nil, err
+	rootOrNodeDNSet, certDNMatchesRootOrNodeDN := security.CheckCertDNMatchesRootDNorNodeDN(clientCert)
+	if rootOrNodeDNSet && !certDNMatchesRootOrNodeDN {
+		return nil, authErrorf(
+			"need root or node client cert to perform RPCs on this server. cert dn did not match set root or node dn",
+		)
 	}
-	if err := checkRootOrNodeInScope(certUserScope, a.tenant.tenantID); err != nil {
-		return nil, err
+	if !rootOrNodeDNSet {
+		certUserScope, err := security.GetCertificateUserScope(clientCert)
+		if err != nil {
+			return nil, err
+		}
+		if err := checkRootOrNodeInScope(certUserScope, a.tenant.tenantID); err != nil {
+			return nil, err
+		}
 	}
 
 	if tenantIDFromMetadata.IsSet() {