diff --git a/pkg/acceptance/generated_cli_test.go b/pkg/acceptance/generated_cli_test.go index 3da13aaa5414..791de85c063d 100644 --- a/pkg/acceptance/generated_cli_test.go +++ b/pkg/acceptance/generated_cli_test.go @@ -172,6 +172,13 @@ func TestDockerCLI_test_disable_replication(t *testing.T) { runTestDockerCLI(t, "test_disable_replication", "../cli/interactive_tests/test_disable_replication.tcl") } +func TestDockerCLI_test_distinguished_name_validation(t *testing.T) { + s := log.Scope(t) + defer s.Close(t) + + runTestDockerCLI(t, "test_distinguished_name_validation", "../cli/interactive_tests/test_distinguished_name_validation.tcl") +} + func TestDockerCLI_test_dump_sig(t *testing.T) { s := log.Scope(t) defer s.Close(t) diff --git a/pkg/cli/cliflags/flags.go b/pkg/cli/cliflags/flags.go index d2ca2cd83d5a..0907bec6fcac 100644 --- a/pkg/cli/cliflags/flags.go +++ b/pkg/cli/cliflags/flags.go @@ -873,6 +873,26 @@ fields. It is permissible for the string to contain colons. `, } + RootCertDistinguishedName = FlagInfo{ + Name: "root-cert-distinguished-name", + Description: ` +A string of comma separated list of distinguished-name += 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 += 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", diff --git a/pkg/cli/context.go b/pkg/cli/context.go index 1d8ffd02f499..311e8f5b6555 100644 --- a/pkg/cli/context.go +++ b/pkg/cli/context.go @@ -479,6 +479,8 @@ var startCtx struct { serverSSLCertsDir string serverCertPrincipalMap []string serverListenAddr string + serverRootCertDN string + serverNodeCertDN string // The TLS auto-handshake parameters. initToken string @@ -532,6 +534,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 = "" diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index 74783ba1e03d..56ee3b52c970 100644 --- a/pkg/cli/flags.go +++ b/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 diff --git a/pkg/cli/interactive_tests/test_distinguished_name_validation.tcl b/pkg/cli/interactive_tests/test_distinguished_name_validation.tcl new file mode 100755 index 000000000000..7313aa583a11 --- /dev/null +++ b/pkg/cli/interactive_tests/test_distinguished_name_validation.tcl @@ -0,0 +1,144 @@ +#! /usr/bin/env expect -f + +source [file join [file dirname $argv0] common.tcl] +variable certs_dir "certs" +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 "cat $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" + system "$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; + $argv sql --certs-dir=$certs_dir -e 'select 1'; + $argv sql --certs-dir=$certs_dir -e 'SET CLUSTER SETTING enterprise.license = \"crl-0-EJL04ukFGAEiI0NvY2tyb2FjaCBMYWJzIC0gUHJvZHVjdGlvbiBUZXN0aW5n\"'; + $argv sql --certs-dir=$certs_dir -e 'SET CLUSTER SETTING cluster.organization = \"Cockroach Labs - Production Testing\"';" + 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" + system "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;" + 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" + system "openssl genrsa -out $::certs_dir/client.$name.key" + system "openssl req -new -key $::certs_dir/client.$name.key -out client.$name.csr -batch -subj /O=Cockroach/CN=$name" + system "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" + # Uncomment the next line to see more details about the generated cert + #system "openssl x509 -in $::certs_dir/client.$name.crt -text" + system "$argv sql --certs-dir=$::certs_dir --user=$name -e 'select 1'" +} + +proc set_role_subject_for_user {argv name role_subject} { + report "SETTING SUBJECT ROLE OPTION for USER $name with SUBJECT $role_subject" + system "$argv sql --certs-dir=$::certs_dir -e 'alter role $name with subject \"$role_subject\" login'" +} + +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 +system "$argv sql --certs-dir=$certs_dir --user=goofus -e 'select 1'" + +report "Validating subject role option can be set and enforced" + +start_test "invalid role option for cert user goofus" +# Set invalid role subject option for user and check login fails +set_role_subject_for_user $argv goofus "O=foo" +spawn $argv "sql" "--certs-dir=$certs_dir" "--user=goofus" "-e" "select 1" +eexpect "certificate authentication failed for user \"goofus\"" +expect_exit_status 1 +end_test + +start_test "valid role option for cert user goofus" +# Set valid role subject option for user and check login fails +set_role_subject_for_user $argv goofus "O=Cockroach,CN=goofus" +spawn $argv "sql" "--certs-dir=$certs_dir" "--user=goofus" "-e" "select 1" +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 + +start_test "invalid root-cert-distinguished-name" +# 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" "" +spawn $argv "sql" "--certs-dir=$certs_dir" "--user=root" "-e" "select 1" +eexpect "certificate authentication failed for user \"root\" (DN: o=foo,cn=invalid)" +expect_exit_status 1 +end_test + +start_test "valid node-cert-distinguished-nane and root-cert-distinguished-name" +# Set valid role subject option for user and check login fails +stop_server $argv +restart_secure_server_distinguished_name_flags $argv $certs_dir "O=Cockroach,CN=root" "O=Cockroach,CN=node" "" +spawn $argv "sql" "--certs-dir=$certs_dir" "--user=root" "-e" "select 1" +end_test + +stop_server $argv diff --git a/pkg/rpc/auth.go b/pkg/rpc/auth.go index 7fd6251ae8e7..d4452ee29d95 100644 --- a/pkg/rpc/auth.go +++ b/pkg/rpc/auth.go @@ -306,19 +306,28 @@ 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("certificate dn did not match set root dn 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() { diff --git a/pkg/rpc/auth_test.go b/pkg/rpc/auth_test.go index e1ee28155240..fdae75db3206 100644 --- a/pkg/rpc/auth_test.go +++ b/pkg/rpc/auth_test.go @@ -15,8 +15,10 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" "fmt" "io" + "strings" "testing" "github.com/cockroachdb/cockroach/pkg/keys" @@ -106,10 +108,14 @@ func TestAuthenticateTenant(t *testing.T) { systemID roachpb.TenantID ous []string commonName string + rootDNString string + nodeDNString string expTenID roachpb.TenantID expErr string tenantScope uint64 clientTenantInMD string + certPrincipalMap string + certDNSName string }{ {systemID: stid, ous: correctOU, commonName: "10", expTenID: tenTen}, {systemID: stid, ous: correctOU, commonName: roachpb.MinTenantID.String(), expTenID: roachpb.MinTenantID}, @@ -170,14 +176,54 @@ func TestAuthenticateTenant(t *testing.T) { {clientTenantInMD: "123", systemID: tenTen, ous: nil, commonName: "root", expErr: `client tenant identity \(123\) does not match server`}, + {systemID: stid, ous: nil, commonName: "foo", rootDNString: "CN=foo"}, + {systemID: stid, ous: nil, commonName: "foo", nodeDNString: "CN=foo"}, + {systemID: stid, ous: nil, commonName: "foo", rootDNString: "CN=bar", + expErr: `need root or node client cert to perform RPCs on this server \(this is tenant system; cert is valid for "foo" on all tenants\)`}, + {systemID: stid, ous: nil, commonName: "foo", nodeDNString: "CN=bar", + expErr: `need root or node client cert to perform RPCs on this server \(this is tenant system; cert is valid for "foo" on all tenants\)`}, + {systemID: stid, ous: nil, commonName: "foo", certPrincipalMap: "foo:root"}, + {systemID: stid, ous: nil, commonName: "foo", certPrincipalMap: "foo:node"}, + {systemID: stid, ous: nil, commonName: "foo", certPrincipalMap: "foo:bar", + expErr: `need root or node client cert to perform RPCs on this server \(this is tenant system; cert is valid for "bar" on all tenants\)`}, + {systemID: stid, ous: nil, commonName: "foo", certDNSName: "root"}, + {systemID: stid, ous: nil, commonName: "foo", certDNSName: "node"}, + {systemID: stid, ous: nil, commonName: "foo", certDNSName: "bar", + expErr: `need root or node client cert to perform RPCs on this server \(this is tenant system; cert is valid for "foo" on all tenants, "bar" on all tenants\)`}, + {systemID: stid, ous: nil, commonName: "foo", certDNSName: "bar", certPrincipalMap: "bar:root"}, + {systemID: stid, ous: nil, commonName: "foo", certDNSName: "bar", certPrincipalMap: "bar:node"}, } { t.Run(fmt.Sprintf("from %v to %v (md %q)", tc.commonName, tc.systemID, tc.clientTenantInMD), func(t *testing.T) { + err := security.SetCertPrincipalMap(strings.Split(tc.certPrincipalMap, ",")) + if err != nil { + t.Fatal(err) + } + err = security.SetRootSubject(tc.rootDNString) + if err != nil { + t.Fatalf("could not set root subject DN, err: %v", err) + } + err = security.SetNodeSubject(tc.nodeDNString) + if err != nil { + t.Fatalf("could not set node subject DN, err: %v", err) + } + defer func() { + security.UnsetRootSubject() + security.UnsetNodeSubject() + }() + cert := &x509.Certificate{ Subject: pkix.Name{ CommonName: tc.commonName, OrganizationalUnit: tc.ous, }, } + cert.RawSubject, err = asn1.Marshal(cert.Subject.ToRDNSequence()) + if err != nil { + t.Fatalf("unable to marshal rdn sequence to raw subject, err: %v", err) + } + if tc.certDNSName != "" { + cert.DNSNames = append(cert.DNSNames, tc.certDNSName) + } if tc.tenantScope > 0 { tenantSANs, err := security.MakeTenantURISANs( username.MakeSQLUsernameFromPreNormalizedString(tc.commonName), diff --git a/pkg/security/auth.go b/pkg/security/auth.go index 4b52445f4159..dcb915965219 100644 --- a/pkg/security/auth.go +++ b/pkg/security/auth.go @@ -32,6 +32,16 @@ var certPrincipalMap struct { m map[string]string } +var rootSubject struct { + syncutil.RWMutex + dn *ldap.DN +} + +var nodeSubject struct { + syncutil.RWMutex + dn *ldap.DN +} + // CertificateUserScope indicates the scope of a user certificate i.e. which // tenant the user is allowed to authenticate on. Older client certificates // without a tenant scope are treated as global certificates which can @@ -63,6 +73,92 @@ type UserAuthHook func( clientConnection bool, ) error +func SetRootSubject(rootDNString string) error { + if len(rootDNString) == 0 { + return nil + } + rootDN, err := distinguishedname.ParseDN(rootDNString) + if err != nil { + return errors.Errorf("invalid root cert distinguished name: %q", rootDNString) + } + rootSubject.Lock() + rootSubject.dn = rootDN + rootSubject.Unlock() + return nil +} + +func UnsetRootSubject() { + rootSubject.Lock() + rootSubject.dn = nil + rootSubject.Unlock() +} + +// evaluateRootOrNodeDN returns distinguished name set for root or node user via +// root-cert-distinguished-name and node-cert-distinguished flags respectively +// if systemIdentity conforms to one of these 2 users. It may also return +// original subject option if it is set for the role. Root and Node roles cannot +// have subject role option set for them. +func evaluateRootOrNodeDN( + setRoleSubject *ldap.DN, systemIdentity username.SQLUsername, +) (dn *ldap.DN) { + dn = setRoleSubject + switch { + case systemIdentity.IsRootUser(): + rootSubject.RLock() + dn = rootSubject.dn + rootSubject.RUnlock() + case systemIdentity.IsNodeUser(): + nodeSubject.RLock() + dn = nodeSubject.dn + nodeSubject.RUnlock() + } + return dn +} + +func CheckCertDNMatchesRootDNorNodeDN( + cert *x509.Certificate, +) (rootOrNodeDNSet bool, certDNMatchesRootOrNodeDN bool) { + rootSubject.RLock() + rootDN := rootSubject.dn + rootSubject.RUnlock() + + nodeSubject.RLock() + nodeDN := nodeSubject.dn + nodeSubject.RUnlock() + + if rootDN != nil || nodeDN != nil { + certDN, err := distinguishedname.ParseDNFromCertificate(cert) + if err != nil { + return true, false + } + if rootDN != nil && certDN.Equal(rootDN) || nodeDN != nil && certDN.Equal(nodeDN) { + return true, true + } + } + + return false, false +} + +func SetNodeSubject(nodeDNString string) error { + if len(nodeDNString) == 0 { + return nil + } + nodeDN, err := distinguishedname.ParseDN(nodeDNString) + if err != nil { + return errors.Errorf("invalid node cert distinguished name: %q", nodeDNString) + } + nodeSubject.Lock() + nodeSubject.dn = nodeDN + nodeSubject.Unlock() + return nil +} + +func UnsetNodeSubject() { + nodeSubject.Lock() + nodeSubject.dn = nil + nodeSubject.Unlock() +} + // SetCertPrincipalMap sets the global principal map. Each entry in the mapping // list must either be empty or have the format :. The principal // map is used to transform principal names found in the Subject.CommonName or @@ -199,6 +295,8 @@ func UserAuthCertHook( return errors.Errorf("using tenant client certificate as user certificate is not allowed") } + roleSubject = evaluateRootOrNodeDN(roleSubject, systemIdentity) + var certSubject *ldap.DN if roleSubject != nil { var err error @@ -331,8 +429,8 @@ func ValidateUserScope( certSubject *ldap.DN, ) bool { // if subject role option is set, it must match the certificate subject - if roleSubject != nil && !roleSubject.Equal(certSubject) { - return false + if roleSubject != nil { + return roleSubject.Equal(certSubject) } for _, scope := range certUserScope { if scope.Username == user { diff --git a/pkg/security/auth_test.go b/pkg/security/auth_test.go index ff6c50b67447..5ba66a4d3203 100644 --- a/pkg/security/auth_test.go +++ b/pkg/security/auth_test.go @@ -338,94 +338,124 @@ func TestAuthenticationHook(t *testing.T) { fieldMismatchSubjectDNString := "O=Cockroach,OU=Marketing Team,UID=b8b40653-7f74-4f14-8a61-59f7f3b18184,CN=foo" subsetSubjectDNString := "O=Cockroach,OU=Order Processing Team,CN=foo" fieldMismatchOnlyOnCommonNameString := "O=Cockroach,OU=Order Processing Team,UID=b8b40653-7f74-4f14-8a61-59f7f3b18184,CN=bar" + rootDNString := "O=Cockroach,OU=Order Processing Team,UID=b8b40653-7f74-4f14-8a61-59f7f3b18184,CN=root" + nodeDNString := "O=Cockroach,OU=Order Processing Team,UID=b8b40653-7f74-4f14-8a61-59f7f3b18184,CN=node" testCases := []struct { - insecure bool - tlsSpec string - username username.SQLUsername - principalMap string - buildHookSuccess bool - publicHookSuccess bool - privateHookSuccess bool - tenantID roachpb.TenantID - isSubjectRoleOptionSet bool - expectedErr string + insecure bool + tlsSpec string + username username.SQLUsername + distinguishedNameString string + principalMap string + buildHookSuccess bool + publicHookSuccess bool + privateHookSuccess bool + tenantID roachpb.TenantID + isSubjectRoleOptionOrDNSet bool + expectedErr string }{ // Insecure mode, empty username. - {true, "", username.SQLUsername{}, "", true, false, false, roachpb.SystemTenantID, false, `user is missing`}, + {true, "", username.SQLUsername{}, subjectDNString, "", true, false, false, roachpb.SystemTenantID, false, `user is missing`}, // Insecure mode, non-empty username. - {true, "", fooUser, "", true, true, false, roachpb.SystemTenantID, false, `user "foo" is not allowed`}, + {true, "", fooUser, subjectDNString, "", true, true, false, roachpb.SystemTenantID, false, `user "foo" is not allowed`}, // Secure mode, no TLS state. - {false, "", username.SQLUsername{}, "", false, false, false, roachpb.SystemTenantID, false, `no client certificates in request`}, + {false, "", username.SQLUsername{}, subjectDNString, "", false, false, false, roachpb.SystemTenantID, false, `no client certificates in request`}, // Secure mode, bad user. - {false, "(CN=foo)", username.NodeUserName(), "", true, false, false, roachpb.SystemTenantID, + {false, "(CN=foo)", username.NodeUserName(), nodeDNString, "", true, false, false, roachpb.SystemTenantID, false, `certificate authentication failed for user "node"`}, // Secure mode, node user. - {false, "(CN=node)", username.NodeUserName(), "", true, true, true, roachpb.SystemTenantID, false, ``}, + {false, "(CN=node)", username.NodeUserName(), nodeDNString, "", true, true, true, roachpb.SystemTenantID, false, ``}, // Secure mode, node cert and unrelated user. - {false, "(CN=node)", fooUser, "", true, false, false, roachpb.SystemTenantID, + {false, "(CN=node)", fooUser, subjectDNString, "", true, false, false, roachpb.SystemTenantID, false, `certificate authentication failed for user "foo"`}, // Secure mode, root user. - {false, "(CN=root)", username.NodeUserName(), "", true, false, false, roachpb.SystemTenantID, + {false, "(CN=root)", username.NodeUserName(), nodeDNString, "", true, false, false, roachpb.SystemTenantID, false, `certificate authentication failed for user "node"`}, // Secure mode, tenant cert, foo user. - {false, "(OU=Tenants,CN=foo)", fooUser, "", true, false, false, roachpb.SystemTenantID, + {false, "(OU=Tenants,CN=foo)", fooUser, subjectDNString, "", true, false, false, roachpb.SystemTenantID, false, `using tenant client certificate as user certificate is not allowed`}, // Secure mode, multiple cert principals. - {false, "(CN=foo)dns:bar", fooUser, "", true, true, false, roachpb.SystemTenantID, false, `user "foo" is not allowed`}, - {false, "(CN=foo)dns:bar", barUser, "", true, true, false, roachpb.SystemTenantID, false, `user "bar" is not allowed`}, + {false, "(CN=foo)dns:bar", fooUser, subjectDNString, "", true, true, false, roachpb.SystemTenantID, false, `user "foo" is not allowed`}, + {false, "(CN=foo)dns:bar", barUser, subjectDNString, "", true, true, false, roachpb.SystemTenantID, false, `user "bar" is not allowed`}, // Secure mode, principal map. - {false, "(CN=foo)dns:bar", blahUser, "foo:blah", true, true, false, roachpb.SystemTenantID, false, `user "blah" is not allowed`}, - {false, "(CN=foo)dns:bar", blahUser, "bar:blah", true, true, false, roachpb.SystemTenantID, false, `user "blah" is not allowed`}, - {false, "(CN=foo)uri:crdb://tenant/123/user/foo", fooUser, "", true, true, false, roachpb.MustMakeTenantID(123), + {false, "(CN=foo)dns:bar", blahUser, subjectDNString, "foo:blah", true, true, false, roachpb.SystemTenantID, false, `user "blah" is not allowed`}, + {false, "(CN=foo)dns:bar", blahUser, subjectDNString, "bar:blah", true, true, false, roachpb.SystemTenantID, false, `user "blah" is not allowed`}, + {false, "(CN=foo)uri:crdb://tenant/123/user/foo", fooUser, subjectDNString, "", true, true, false, roachpb.MustMakeTenantID(123), false, `user "foo" is not allowed`}, - {false, "(CN=foo)uri:crdb://tenant/123/user/foo", fooUser, "", true, false, false, roachpb.SystemTenantID, + {false, "(CN=foo)uri:crdb://tenant/123/user/foo", fooUser, subjectDNString, "", true, false, false, roachpb.SystemTenantID, false, `certificate authentication failed for user "foo"`}, - {false, "(CN=foo)", fooUser, "", true, true, false, roachpb.MustMakeTenantID(123), + {false, "(CN=foo)", fooUser, subjectDNString, "", true, true, false, roachpb.MustMakeTenantID(123), false, `user "foo" is not allowed`}, - {false, "(CN=foo)uri:crdb://tenant/1/user/foo", fooUser, "", true, false, false, roachpb.MustMakeTenantID(123), + {false, "(CN=foo)uri:crdb://tenant/1/user/foo", fooUser, subjectDNString, "", true, false, false, roachpb.MustMakeTenantID(123), false, `certificate authentication failed for user "foo"`}, - {false, "(CN=foo)uri:crdb://tenant/123/user/foo", blahUser, "", true, false, false, roachpb.MustMakeTenantID(123), + {false, "(CN=foo)uri:crdb://tenant/123/user/foo", blahUser, subjectDNString, "", true, false, false, roachpb.MustMakeTenantID(123), false, `certificate authentication failed for user "blah"`}, // Secure mode, client cert having full DN, foo user with subject role // option not set. - {false, "(" + subjectDNString + ")", fooUser, "", true, true, false, roachpb.MustMakeTenantID(123), + {false, "(" + subjectDNString + ")", fooUser, subjectDNString, "", true, true, false, roachpb.MustMakeTenantID(123), false, `user "foo" is not allowed`}, // Secure mode, client cert having full DN, foo user with subject role // option set matching TLS cert subject. - {false, "(" + subjectDNString + ")", fooUser, "", true, true, false, roachpb.MustMakeTenantID(123), + {false, "(" + subjectDNString + ")", fooUser, subjectDNString, "", true, true, false, roachpb.MustMakeTenantID(123), true, `user "foo" is not allowed`}, // Secure mode, client cert having full DN, foo user with subject role // option set, TLS cert DN empty. - {false, "(CN=foo)", fooUser, "", true, false, false, roachpb.MustMakeTenantID(123), + {false, "(CN=foo)", fooUser, subjectDNString, "", true, false, false, roachpb.MustMakeTenantID(123), true, `certificate authentication failed for user "foo"`}, // Secure mode, client cert having full DN, foo user with subject role // option set, TLS cert DN mismatches on OU field. - {false, "(" + fieldMismatchSubjectDNString + ")", fooUser, "", true, false, false, roachpb.MustMakeTenantID(123), + {false, "(" + fieldMismatchSubjectDNString + ")", fooUser, subjectDNString, "", true, false, false, roachpb.MustMakeTenantID(123), true, `certificate authentication failed for user "foo"`}, // Secure mode, client cert having full DN, foo user with subject role // option set, TLS cert DN subset of role subject DN. - {false, "(" + subsetSubjectDNString + ")", fooUser, "", true, false, false, roachpb.MustMakeTenantID(123), + {false, "(" + subsetSubjectDNString + ")", fooUser, subjectDNString, "", true, false, false, roachpb.MustMakeTenantID(123), true, `certificate authentication failed for user "foo"`}, // Secure mode, client cert having full DN, foo user with subject role // option set mismatching TLS cert subject only on CN(required for // matching) having DNS as foo. - {false, "(" + fieldMismatchOnlyOnCommonNameString + ")dns:foo", fooUser, "", true, false, false, roachpb.MustMakeTenantID(123), + {false, "(" + fieldMismatchOnlyOnCommonNameString + ")dns:foo", fooUser, subjectDNString, "", true, false, false, roachpb.MustMakeTenantID(123), true, `certificate authentication failed for user "foo"`}, + {false, "(" + rootDNString + ")", username.RootUserName(), rootDNString, "", true, true, false, roachpb.MustMakeTenantID(123), + true, `user "root" is not allowed`}, + {false, "(" + nodeDNString + ")", username.NodeUserName(), nodeDNString, "", true, true, true, roachpb.MustMakeTenantID(123), + true, ""}, + // tls cert dn matching root dn set, where CN != root + {false, "(" + subjectDNString + ")", username.RootUserName(), subjectDNString, "", true, true, false, roachpb.MustMakeTenantID(123), + true, `user "root" is not allowed`}, } ctx := context.Background() for i, tc := range testCases { t.Run(fmt.Sprintf("%d: tls:%s user:%v tenant:%v", i, tc.tlsSpec, tc.username, tc.tenantID), func(t *testing.T) { + defer func() { + security.UnsetRootSubject() + security.UnsetNodeSubject() + }() err := security.SetCertPrincipalMap(strings.Split(tc.principalMap, ",")) if err != nil { t.Fatal(err) } var roleSubject *ldap.DN - if tc.isSubjectRoleOptionSet { - roleSubject, _ = distinguishedname.ParseDN(subjectDNString) + if tc.isSubjectRoleOptionOrDNSet { + switch tc.username { + case username.RootUserName(): + err = security.SetRootSubject(tc.distinguishedNameString) + if err != nil { + t.Fatalf("could not set root subject DN, err: %v", err) + } + case username.NodeUserName(): + err = security.SetNodeSubject(tc.distinguishedNameString) + if err != nil { + t.Fatalf("could not set node subject DN, err: %v", err) + } + default: + roleSubject, err = distinguishedname.ParseDN(tc.distinguishedNameString) + if err != nil { + t.Fatalf("could not set role subject, err: %v", err) + } + } } hook, err := security.UserAuthCertHook(