Postgres custom root certificates#3395
Conversation
| let pool_key = format!( | ||
| "{address}{}", | ||
| root_ca | ||
| .map(|s| format!(" **with root-ca** {s}")) | ||
| .unwrap_or_default() | ||
| ); |
There was a problem hiding this comment.
Should be able to use (String, Option<String>) (or a struct with all of the necessary derives) as the cache key here.
There was a problem hiding this comment.
🤦 I'm an idiot. Thank you.
| let mut builder = TlsConnector::builder(); | ||
| if let Some(root_ca) = root_ca { | ||
| let cert_bytes = root_ca.as_bytes(); | ||
| builder.add_root_certificate(native_tls::Certificate::from_pem(cert_bytes)?); |
There was a problem hiding this comment.
I imagine .context for from_pem might be helpful here.
| .get_mut(self_.rep()) | ||
| .ok_or_else(|| v4::Error::ConnectionFailed("no builder found".into()))?; | ||
| builder.root_ca = Some(certificate); | ||
| Ok(()) |
There was a problem hiding this comment.
I'd either eagerly parse the cert here or make this WIT method infallible.
There was a problem hiding this comment.
I tried making it infallible but had to handle the case of resource lookup failure. But yeah, eager parse would give a better locale for the error anyway.
| .builders | ||
| .get_mut(self_.rep()) | ||
| .ok_or_else(|| v4::Error::ConnectionFailed("no builder found".into()))?; | ||
| // borrow checker gets pedantic here, so we need to outsmart it |
There was a problem hiding this comment.
Oh right, I forgot how to WIT. I guess the "right" way (what WASI does) would look something like:
resource connection-options {
constructor();
set-root-ca: ...
}
resource connection {
open-with-options: static func(address: string, options: connection-options) -> ...
...
}
...or some variation thereof... that would make it safe to take the resource out of the table when you need it.
I don't have strong feelings here. 🤷
There was a problem hiding this comment.
Is there any reason why connection-options would benefit from being a resource in this case? It feels like it's not doing anything a record couldn't do, and passing a record directly to open-with-options would significantly reduce implementation noise. Is it forward compatibility considerations?
There was a problem hiding this comment.
Yep - records cannot be extended backward-compatibly.
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
2 participants
Take seven. This is part of the "guests have to include their root cert as an asset" genre. Based on Lann's suggestion at #3381 (comment).
This is pretty unergonomic as you can see from the example (in tests/manual - look at the commented out 'for testing with TLS' stuff), but hopefully we can wrapper it in the SDKs, and if not... well... at least folks have an escape hatch eh.