Skip to content

Commit

Permalink
server,rpc,security: use a separate sql-node.crt for SQL pods
Browse files Browse the repository at this point in the history
Release note (security update): Multitenant SQL servers now use a
separate certificate with `CN=sql-node` and filename `sql-node.crt`.
Using the `node.crt` generated for the system tenant is not possible
any more.
  • Loading branch information
knz committed Oct 6, 2021
1 parent 04d6d93 commit 9062f79
Show file tree
Hide file tree
Showing 37 changed files with 1,225 additions and 887 deletions.
1 change: 1 addition & 0 deletions pkg/cli/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ var certCmds = []*cobra.Command{
mtCreateTenantClientCACertCmd,
createNodeCertCmd,
createClientCertCmd,
mtCreateSQLNodeCertCmd,
mtCreateTenantClientCertCmd,
listCertsCmd,
}
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/mt.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func init() {
mtCmd.AddCommand(mtTestDirectorySvr)

mtCertsCmd.AddCommand(
mtCreateSQLNodeCertCmd,
mtCreateTenantClientCACertCmd,
mtCreateTenantClientCertCmd,
)
Expand Down
42 changes: 42 additions & 0 deletions pkg/cli/mt_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,45 @@ Creation fails if the CA expiration time is before the desired certificate expir
"failed to write tenant client certificate and key")
}),
}

// A mtCreateSQLNodeCert command generates a SQL server node certificate
// and stores it in the cert directory.
var mtCreateSQLNodeCertCmd = &cobra.Command{
Use: "create-tenant-node --certs-dir=<path to cockroach certs dir> --ca-key=<path-to-ca-key> <host 1> <host 2> ... <host N>",
Short: "create SQL server certificate and key",
Long: `
Generate a node certificate "<certs-dir>/sql-node.crt" and key "<certs-dir>/sql-node.key".
If --overwrite is true, any existing files are overwritten.
At least one host should be passed in (either IP address or dns name).
Requires a CA cert in "<certs-dir>/ca.crt" and matching key in "--ca-key".
If "ca.crt" contains more than one certificate, the first is used.
Creation fails if the CA expiration time is before the desired certificate expiration.
`,
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return errors.Errorf("create-sql-node requires at least one host name or address, none was specified")
}
return nil
},
RunE: clierrorplus.MaybeDecorateError(runCreateSQLNodeCert),
}

// runCreateSQLNodeCert generates key pair and CA certificate and writes them
// to their corresponding files.
// TODO(marc): there is currently no way to specify which CA cert to use if more
// than one is present. We shoult try to load each certificate along with the key
// and pick the one that works. That way, the key specifies the certificate.
func runCreateSQLNodeCert(cmd *cobra.Command, args []string) error {
return errors.Wrap(
security.CreateSQLNodePair(
certCtx.certsDir,
certCtx.caKey,
certCtx.keySize,
certCtx.certificateLifetime,
certCtx.overwriteFiles,
args),
"failed to generate SQL server certificate and key")
}
19 changes: 16 additions & 3 deletions pkg/security/certificate_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ const (
// NodePem describes the server certificate for the node, possibly a combined server/client
// certificate for user Node if a separate 'client.node.crt' is not present.
NodePem
// TenantNodePem describes the server certificate for a SQL tenant
// server, for connections across SQL tenant servers.
TenantNodePem
// UIPem describes the server certificate for the admin UI.
UIPem
// ClientPem describes a client certificate.
Expand Down Expand Up @@ -125,6 +128,8 @@ func (p PemUsage) String() string {
return "UI CA"
case NodePem:
return "Node"
case TenantNodePem:
return "Tenant Node"
case UIPem:
return "UI"
case ClientPem:
Expand Down Expand Up @@ -215,6 +220,11 @@ func CertInfoFromFilename(filename string) (*CertInfo, error) {
if numParts != 2 {
return nil, errors.Errorf("node certificate filename should match node%s", certExtension)
}
case `sql-node`:
fileUsage = TenantNodePem
if numParts != 2 {
return nil, errors.Errorf("SQL node certificate filename should match node%s", certExtension)
}
case `ui`:
fileUsage = UIPem
if numParts != 2 {
Expand Down Expand Up @@ -459,7 +469,7 @@ func parseCertificate(ci *CertInfo) error {
// This should only be called on the NodePem CertInfo when there is no specific
// client certificate for the 'node' user.
// Fields required for a valid server certificate are already checked.
func validateDualPurposeNodeCert(ci *CertInfo) error {
func validateDualPurposeNodeCert(ci *CertInfo, nodeUser string) error {
if ci == nil {
return errors.Errorf("no node certificate found")
}
Expand All @@ -471,9 +481,9 @@ func validateDualPurposeNodeCert(ci *CertInfo) error {
// The first certificate is used in client auth.
cert := ci.ParsedCertificates[0]
principals := getCertificatePrincipals(cert)
if !Contains(principals, NodeUser) {
if !Contains(principals, nodeUser) {
return errors.Errorf("client/server node certificate has principals %q, expected %q",
principals, NodeUser)
principals, nodeUser)
}

return nil
Expand All @@ -487,6 +497,9 @@ func validateCockroachCertificate(ci *CertInfo, cert *x509.Certificate) error {
case NodePem:
// Common Name is checked only if there is no client certificate for 'node'.
// This is done in validateDualPurposeNodeCert.
case TenantNodePem:
// Common Name is checked only if there is no client certificate for 'sql-node'.
// This is done in validateDualPurposeNodeCert.
case ClientPem:
// Check that CommonName matches the username extracted from the filename.
principals := getCertificatePrincipals(cert)
Expand Down
29 changes: 29 additions & 0 deletions pkg/security/certificate_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func TestCertNomenclature(t *testing.T) {
{"ca-client.crt", "", security.ClientCAPem, ""},
{"ca-ui.crt", "", security.UICAPem, ""},
{"node.crt", "", security.NodePem, ""},
{"sql-node.crt", "", security.TenantNodePem, ""},
{"ui.crt", "", security.UIPem, ""},
{"client.root.crt", "", security.ClientPem, "root"},
{"client.foo-bar.crt", "", security.ClientPem, "foo-bar"},
Expand Down Expand Up @@ -191,6 +192,9 @@ func TestNamingScheme(t *testing.T) {
parsedGoodNodeCert, goodNodeCert := makeTestCert(t, "node", fullKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth})
_, badUserNodeCert := makeTestCert(t, "notnode", fullKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth})

parsedGoodSQLNodeCert, goodSQLNodeCert := makeTestCert(t, "sql-node", fullKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth})
_, badUserSQLNodeCert := makeTestCert(t, "notsqlnode", fullKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth})

parsedGoodRootCert, goodRootCert := makeTestCert(t, "root", fullKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth})
_, notRootCert := makeTestCert(t, "notroot", fullKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth})

Expand Down Expand Up @@ -250,6 +254,7 @@ func TestNamingScheme(t *testing.T) {
{"cr..crt", 0777, []byte{}},
{"node.foo.crt", 0777, []byte{}},
{"node..crt", 0777, []byte{}},
{"sql-node..crt", 0777, []byte{}},
{"client.crt", 0777, []byte{}},
{"client..crt", 0777, []byte{}},
},
Expand All @@ -260,6 +265,7 @@ func TestNamingScheme(t *testing.T) {
files: []testFile{
{"ca.crt", 0777, caCert},
{"node.crt", 0777, goodNodeCert},
{"sql-node.crt", 0777, goodSQLNodeCert},
{"client.root.crt", 0777, goodRootCert},
},
certs: []security.CertInfo{
Expand All @@ -268,6 +274,8 @@ func TestNamingScheme(t *testing.T) {
Error: errors.New(".* no such file or directory")},
{FileUsage: security.NodePem, Filename: "node.crt",
Error: errors.New(".* no such file or directory")},
{FileUsage: security.TenantNodePem, Filename: "sql-node.crt",
Error: errors.New(".* no such file or directory")},
},
},
{
Expand All @@ -278,6 +286,8 @@ func TestNamingScheme(t *testing.T) {
{"ca.key", 0777, []byte{}},
{"node.crt", 0777, goodNodeCert},
{"node.key", 0704, []byte{}},
{"sql-node.crt", 0777, goodSQLNodeCert},
{"sql-node.key", 0704, []byte{}},
{"client.root.crt", 0777, goodRootCert},
{"client.root.key", 0704, []byte{}},
},
Expand All @@ -287,6 +297,8 @@ func TestNamingScheme(t *testing.T) {
Error: permissionRequirementErr},
{FileUsage: security.NodePem, Filename: "node.crt",
Error: permissionRequirementErr},
{FileUsage: security.TenantNodePem, Filename: "sql-node.crt",
Error: permissionRequirementErr},
},
skipWindows: true,
},
Expand All @@ -295,6 +307,8 @@ func TestNamingScheme(t *testing.T) {
files: []testFile{
{"node.crt", 0777, badUserNodeCert},
{"node.key", 0700, []byte("node.key")},
{"sql-node.crt", 0777, badUserSQLNodeCert},
{"sql-node.key", 0700, []byte("sql-node.key")},
{"client.root.crt", 0777, notRootCert},
{"client.root.key", 0700, []byte{}},
},
Expand All @@ -303,6 +317,8 @@ func TestNamingScheme(t *testing.T) {
Error: errors.New(`client certificate has principals \["notroot"\], expected "root"`)},
{FileUsage: security.NodePem, Filename: "node.crt", KeyFilename: "node.key",
FileContents: badUserNodeCert, KeyFileContents: []byte("node.key")},
{FileUsage: security.TenantNodePem, Filename: "sql-node.crt", KeyFilename: "sql-node.key",
FileContents: badUserSQLNodeCert, KeyFileContents: []byte("sql-node.key")},
},
},
{
Expand All @@ -312,6 +328,8 @@ func TestNamingScheme(t *testing.T) {
{"ca.key", 0700, []byte("ca.key")},
{"node.crt", 0777, goodNodeCert},
{"node.key", 0700, []byte("node.key")},
{"sql-node.crt", 0777, goodSQLNodeCert},
{"sql-node.key", 0700, []byte("sql-node.key")},
{"client.root.crt", 0777, goodRootCert},
{"client.root.key", 0700, []byte("client.root.key")},
},
Expand All @@ -321,6 +339,8 @@ func TestNamingScheme(t *testing.T) {
Name: "root", FileContents: goodRootCert, KeyFileContents: []byte("client.root.key")},
{FileUsage: security.NodePem, Filename: "node.crt", KeyFilename: "node.key",
FileContents: goodNodeCert, KeyFileContents: []byte("node.key")},
{FileUsage: security.TenantNodePem, Filename: "sql-node.crt", KeyFilename: "sql-node.key",
FileContents: goodSQLNodeCert, KeyFileContents: []byte("sql-node.key")},
},
},
{
Expand All @@ -330,6 +350,8 @@ func TestNamingScheme(t *testing.T) {
{"ca.key", 0700, []byte("ca.key")},
{"node.crt", 0777, append(goodNodeCert, caCert...)},
{"node.key", 0700, []byte("node.key")},
{"sql-node.crt", 0777, append(goodSQLNodeCert, caCert...)},
{"sql-node.key", 0700, []byte("sql-node.key")},
{"client.root.crt", 0777, append(goodRootCert, caCert...)},
{"client.root.key", 0700, []byte("client.root.key")},
},
Expand All @@ -341,6 +363,9 @@ func TestNamingScheme(t *testing.T) {
{FileUsage: security.NodePem, Filename: "node.crt", KeyFilename: "node.key",
FileContents: append(goodNodeCert, caCert...), KeyFileContents: []byte("node.key"),
ParsedCertificates: []*x509.Certificate{parsedGoodNodeCert, parsedCACert}},
{FileUsage: security.TenantNodePem, Filename: "sql-node.crt", KeyFilename: "sql-node.key",
FileContents: append(goodSQLNodeCert, caCert...), KeyFileContents: []byte("sql-node.key"),
ParsedCertificates: []*x509.Certificate{parsedGoodSQLNodeCert, parsedCACert}},
},
},
{
Expand All @@ -350,6 +375,8 @@ func TestNamingScheme(t *testing.T) {
{"ca.key", 0777, []byte("ca.key")},
{"node.crt", 0777, goodNodeCert},
{"node.key", 0777, []byte("node.key")},
{"sql-node.crt", 0777, goodSQLNodeCert},
{"sql-node.key", 0777, []byte("sql-node.key")},
{"client.root.crt", 0777, goodRootCert},
{"client.root.key", 0777, []byte("client.root.key")},
},
Expand All @@ -359,6 +386,8 @@ func TestNamingScheme(t *testing.T) {
Name: "root", FileContents: goodRootCert, KeyFileContents: []byte("client.root.key")},
{FileUsage: security.NodePem, Filename: "node.crt", KeyFilename: "node.key",
FileContents: goodNodeCert, KeyFileContents: []byte("node.key")},
{FileUsage: security.TenantNodePem, Filename: "sql-node.crt", KeyFilename: "sql-node.key",
FileContents: goodSQLNodeCert, KeyFileContents: []byte("sql-node.key")},
},
skipChecks: true,
},
Expand Down
Loading

0 comments on commit 9062f79

Please sign in to comment.