server,rpc,security: use a separate sql-node.crt for SQL pods

[knz]

Fixes #71106.

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.

[cockroach-teamcity]

This change is 

[andy-kimball]

Is this code that we'd expect to remove once we move to multiple CAs? If so, do we need to add to the CLI, since CC generates its own certificates rather than using the CLI? Also, if so, are there any changes that would be difficult to back-out later on?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @catj-cockroach and @dhartunian)

[knz]

The CLI change exist for the benefit of manual and automated testing. This is also true of the other cli commands that generate MT certs.

The code as implemented here uses the tenant CA to sign (and verify) the new sql-node. This was an arbitrary choice on my part, and I can revert that to use the "main" CA found by a SQL server in the file ca.crt (and we'd assume that CA is specific to SQL pods, not shared with the host.cluster). I can also implement a fallback behavior, where we try one CA cert first, and if that doesn't exist, try another one.

@catj-cockroach please instruct further.

[knz]

@chrisseto @catj-cockroach instead of the approach I've taken here (new sql-node cert), I could also update this PR to use the tenant cert directly (the one used to connect to KV pods) to connect SQL pods to each other. What do you think?

[chrisseto]

I think I like the separation of concerns by using a sql-node cert rather than reusing the tenant -> kv cert. It feels a bit more intuitive to me. I'll defer to @catj-cockroach as the TLS expert, however.

[andy-kimball]

I think the separate sql-node certificate is better from a separation of concerns point of view ... but if we plan to throw this out anyways, I'd lean towards whatever results in the minimum code change that's easiest to later remove.

[catj-cockroach]

I'm sorry I should have updated here as well! Personally I'm fine with using the same certificate for both. It's fewer keys to rotate in the event of a server compromise and all the certificate is doing is attesting that it is a SQL server to the KV layer and the rest of the SQL layer. So whatever is easier for you @knz! :)

[knz]

RFAL

[catj-cockroach]

Reviewed 33 of 37 files at r1, 29 of 29 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian and @knz)

---

pkg/security/certificate_loader.go, line 470 at r2 (raw file):

// values of certain fields.
// This should only be called on the NodePem CertInfo when there is no specific
// client certificate for the 'node' user.

Could we update this comment to:

// This should only be called on the NodePem CertInfo when there is no specific
// client certificate for the 'node' or 'sql-node' user.

---

pkg/security/certificate_manager.go, line 130 at r2 (raw file):

	uiCACert       *CertInfo // optional: certificate to verify UI certficates
	nodeCert       *CertInfo // certificate for nodes (always server cert, sometimes client cert)
	tenantNodeCert *CertInfo // certificate for SQL tenant servers (always server cert, sometimes client cert)

I'm not certain I understand this comment. Does this mean "sometimes the CertInfo value is referring to a client certificate in addition to when it's referring to a server certificate"?

---

pkg/security/certs.go, line 297 at r2 (raw file):

	overwrite bool,
	hosts []string,
	forTenant bool,

Why not expose baseNodeUser here instead of using a boolean?

Edit: oh, I see, we use this to determine the label and the filesystem paths! I'd argue we should have a createNodePair and createTenantNodePair and move the duplicated code that makes sense to into a new function. We could easily wrap everything from the start of the function to where we're setting the labels and determining filenames from the looks of things.

---

pkg/security/username.go, line 75 at r2 (raw file):

// IsNodeUser is true iff the username designates the node user.
func (s SQLUsername) IsNodeUser() bool { return s.u == NodeUser }

Should we have an equivalent version of this function for SQLNodeUser?

[knz]

we're going to deprioritize this approach in favor of #71248.