security: certificates without an end-of-line character will not be loaded properly by the UI's TLS logic

[catj-cockroach]

Describe the problem

When using certificate authority files that do not have an end-of-line character at the end of a file, the GetTenantTLSConfig and GetUIClientTLSConfig methods of CertificateManager will create unreadable certificate authority blobs. getTenantCACertLocked will resolve to whatever certificate authority exists, regardless of if the code is running in a multi-tenant environment or not, which means GetTenantTLSConfig and GetUIClientTLSConfig will always return two certificates in a blob.

While GetTenantTLSConfig is only called in a multi-tenant environment, GetUIClientTLSConfig is called in dedicated environments and will try to concatenate whatever certificates it receives from getUICACertLocked and getTenantCACertLocked. As mentioned before, this could result in two copies of ca.crt, or a combination of other certificate authorities.

If the certificate does not have an end-of-line character, the resulting loaded certificate would appear as follows:

-----BEGIN CERTIFICATE-----
MIIBITCB1KADAgECAgEBMAUGAytlcDASMRAwDgYDVQQDEwdUZXN0IENBMCAYDzAw
MDEwMTAxMDAwMDAwWhcNMjEwOTI5MTY1NTMwWjARMQ8wDQYDVQQDEwZDbGllbnQw
KjAFBgMrZXADIQDVQnGdqEybPlhb5PFCkHceGlAD7su0u8HP8gvIK/IoqaNOMEww
EwYDVR0lBAwwCgYIKwYBBQUHAwIwHwYDVR0jBBgwFoAU/aMVid2wZ4G0EYfp2cKK
j0tLDo4wFAYDVR0RBA0wC4IJbXktY2xpZW50MAUGAytlcANBAPBgz2jFGF3z4OUK
iXpxklAt6NRRFxSz/pUj/eayKnu4L4xi9UjRZ17mwDxWiwwkDuASD04+e8CvxMwS
O9dpkgY=
-----END CERTIFICATE----------BEGIN CERTIFICATE-----
MIIBITCB1KADAgECAgEBMAUGAytlcDASMRAwDgYDVQQDEwdUZXN0IENBMCAYDzAw
MDEwMTAxMDAwMDAwWhcNMjEwOTI5MTY1NTMwWjARMQ8wDQYDVQQDEwZDbGllbnQw
KjAFBgMrZXADIQDVQnGdqEybPlhb5PFCkHceGlAD7su0u8HP8gvIK/IoqaNOMEww
EwYDVR0lBAwwCgYIKwYBBQUHAwIwHwYDVR0jBBgwFoAU/aMVid2wZ4G0EYfp2cKK
j0tLDo4wFAYDVR0RBA0wC4IJbXktY2xpZW50MAUGAytlcANBAPBgz2jFGF3z4OUK
iXpxklAt6NRRFxSz/pUj/eayKnu4L4xi9UjRZ17mwDxWiwwkDuASD04+e8CvxMwS
O9dpkgY=
-----END CERTIFICATE-----

As this is not a valid PEM, an error will be received along the lines of:

problem using security settings: failed to parse PEM data to pool

To Reproduce

If possible, provide steps to reproduce the behavior:

1. Set up CockroachDB cluster with a normal set of certificate authorities (just ca.crt on its own is acceptable)
2. Delete the newlines from the end of your ca.crt and other certificate authorities. (For example, using truncate -s -1 ca.crt)
3. Attempt to start the server
4. See error

Expected behavior
Only the UI certificate authority should be loaded and no error should occur.

Additional context
This issue was introduced by a PR (#71248) to fix a security issue in the multi-tenant environment. Unfortunately I missed this issue during code review.

[celiala]

Marking this issue as a release-blocker, as per Slack thread:

Andy Kimball 17 hours ago
So I think it's clearly a release blocker, since it can cause 21.2 upgrades to fail. Put that into the justification field.

[blathers-crl]

Hi @celiala, please add branch-* labels to identify which branch(es) this release-blocker affects.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}