search

cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.05k stars 3.8k forks source link

security: Distinguish CA and other `.crt` files during parsing #66477

Open bdarnell opened 3 years ago

bdarnell commented 3 years ago

The function security.parseCertificate is used to parse both single cert chains (like node.crt) and collections of CA certs (like ca.crt). This is incorrect in at least one way: the expiration time of the first certificate in the file is taken to be the expiration time of the whole file. (https://github.com/cockroachdb/cockroach/blob/4d987b3f0788c98fa358715b336a1c55e254ee50/pkg/security/certificate_loader.go#L412). This is correct for single cert chains, but for CAs a major reason why multiple certificates are put in one file is to allow for rotations, in which case they expire at different times and the order in the file should not matter.

I think this expiration time is only used in exported metrics, so it doesn't cause a serious problem, but we should verify this. To fix it, we need to update CertificateLoader to recognize that CA certificate files are different from single cert chains, and they contain a set of expiration times. (and I think we want to export the max of the expiration times. There's also an argument for exposing the min expiration time, but I think if we want to get it down to a single metric the max is more useful).

Jira issue: CRDB-8059

knz commented 2 years ago

cc @jtsiros for triage

github-actions[bot] commented 11 months ago

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!