Add PEM encoded certificate to external api response

[augustuswm]

• Add pem encoded certificate to certificate list and view endpoints

When managing multiple silos, it quickly becomes desirable to be able to use the oxide cli and/or sdk to manage them. Handling certificate rotation currently requires storing external state about what certificate is registered with which silo as our external API only returns identity metadata about the certificate. This PR proposes adding the the public certificate to the fields returned for a certificate.

This initial implementation tries to take the simplest approach to this:

1. Return the exact representation that we store in the database (PEM-encoded string)
2. Returns the certificate on both list and view endpoints

An alternative approach may be to only return the full certificate as a part of the view endpoint, but I'm not sure if that is a pattern that we use.

The other consideration is how this should handle the failure to construct a String from the bytes form of a certificate. Currently it is very disruptive, and hard to debug if bad data were to be stored in the database.

[davepacheco]

Good idea! I'd love another +1 from someone more familiar with working with certificates.

I wonder if it makes more sense to put a fingerprint here? If so, that'd make it less unwieldy to keep in the "list" endpoint. As it is, it does seem less than ideal to include it there. Maybe @ahl has an opinion?

[augustuswm]

@ahl Any thoughts on this? Should we use an identifier in the list endpoint and then display the full contents in a view endpoint?

Ran in to this again while doing a round of cert updates, and forgot that I had started on it.

[ahl]

For what it's worth, the view and list endpoints do seem to mostly (if not entirely) use the same type. If this seems unwieldy we can invent a summary type for the list endpoint. If the concern is consuming terminal space in the output, perhaps we can solve that with a custom command.

If we think most consumers of the API would want this as part of list then we should probably include it.

[david-crespo]

+1 to adding it in the list too. I don't see the extra bytes being a big problem. There's always oxidecomputer/dropshot#221 (auto gzip if the accept header says to).

[augustuswm]

@davepacheco - Thoughts on getting this in to release 9?

[davepacheco]

Thanks for the bump. Reading back through the comments:

• There's agreement that we'd like to include this in the "list" endpoint. I don't love having the full certificate bytes in the list endpoint because of the size of this request (which can have many certificates, at least in principle) and general unwieldiness of having giant requests. It's kind of a vague concern and if everyone else feels like it's fine, that's fine with me.
• But as a user, if I want to compare certificates, I think I want to compare fingerprints anyway. No? (I guess I'm looking for more input here, too.)
• Unrelated to that, I'd still like to get a +1 from somebody more fluent with best practices around transmitting certificates. Maybe @iliana?

[iliana]

Returning full certificates (not the private keys!) via the API is fine. The full certificate is sent to any HTTPS client anyway, whether the user is authenticated or not (for example, run openssl s_client -connect github.com:443 and see the -----BEGIN CERTIFICATE----- blob, or the steps to open Firefox's certificate viewer for a site).

I'm not sure I agree that the size of the certificates are that much of a concern; the wildcard certificates we use on our racks are less than 2 KiB PEM-encoded (so, already base64'd). An eventual snapshot block download endpoint would almost certainly return more than 10 or 20 times that that for efficiently backing up disks off-rack.

I don't think fingerprints are particularly useful. (Certificate serial numbers would honestly be more useful, and they're still not that useful on their own.) There's supposed to be a standard way to calculate fingerprints but I think different tools have failed to completely agree on that.

[augustuswm]

I do think that it is important to be able to retrieve the actual contents of the certificate (via some endpoint), and as @iliana noted making an https request retrieves the data of the the certificate that the Nexus decides to serve.

The intention with returning the certificate contents is to be able to analyze the individual certificate fields (in particular expiration dates). This allows storing the key and certificate "only" in the silo, and no where else. Then when we near expiration we can generate a new key pair to rotate in to the silo.

[davepacheco]

Sounds good! So the remaining thing then is: we accept certificates as bytes, not strings; here, we're returning them as strings, which is a little inconsistent and introduces the failure mode that they're not UTF-8 encoded (which produces a 500, which is a little weird, but there's not much else we can do). Should we just return them in bytes the same way we accept them? (@ahl?)

[ahl]

I'm not sure I agree that the size of the certificates are that much of a concern; the wildcard certificates we use on our racks are less than 2 KiB PEM-encoded (so, already base64'd). An eventual snapshot block download endpoint would almost certainly return more than 10 or 20 times that that for efficiently backing up disks off-rack.

I think the concern regard the size is not, say, the bandwidth but rather than it would make the CLI cumbersome. I think we can sort that out client-side if it becomes an issue--let's just make the API what we expect to be most useful. If fingerprints aren't useful then let's return the full cert.

So the remaining thing then is: we accept certificates as bytes, not strings; here, we're returning them as strings, which is a little inconsistent and introduces the failure mode that they're not UTF-8 encoded (which produces a 500, which is a little weird, but there's not much else we can do). Should we just return them in bytes the same way we accept them?

Ugh! Should we only accept them as a string? Practically is there a way to encounter that 500?

[iliana]

Is that difference because we accept certs in PEM or DER, but then always encode them internally as PEM? PEM-encoded certificates are always ASCII, so I don't think it'd be unreasonable to have input be bytes (to support whatever the user is planning to upload) but have output be strings.

[augustuswm]

I thought that we only accepted new certificates as PEM encoded Strings, and then write them to a binary field in the database:

https://github.com/oxidecomputer/omicron/blob/main/nexus/src/app/certificate.rs#L43
https://github.com/oxidecomputer/omicron/blob/main/nexus/types/src/external_api/params.rs#L821
https://github.com/oxidecomputer/omicron/blob/main/nexus/db-queries/src/db/datastore/certificate.rs#L46

Thought is there a different location where we accept them as bytes? I'm certainly not as familiar with Nexus.

It is definitely not great that a 500 is technically possible here.

[iliana]

I am also not familiar with the full certificate lifecycle, but if we're already only accepting PEM-encoded strings, that seems right to me.

[davepacheco]

Ugh, sorry -- I'm living in the past (pre-#3396). These have been Strings for a while now. Great!

We do still store them as BYTES in the database, unfortunately. But that's out of scope here. Could you file an issue on that @augustuswm?

[davepacheco]

Ugh, sorry for yet more confusion. I think my "approval" posted a comment I had drafted months ago and thought I sent but apparently didn't...and which we already talked about and resolved. Sorry again! This looks good.

[augustuswm]

All good! I'll re-run the failed tests here and get an issue open on looking into the database schema.