lann opened issue #8748:
Currently, the TLS roots are hard-coded to the
webpki-roots
set. This is a good default, but in some scenarios private roots are required. We should be able to add options toOutgoingRequestConfig
to extend and/or replace that default set of roots with custom root(s).
lann edited issue #8748:
Currently, the TLS roots are hard-coded to the
webpki-roots
set. This is a good default, but in some scenarios private roots are required. We should be able to add options toOutgoingRequestConfig
to extend and/or replace that default set of roots with custom root(s).
bacongobbler commented on issue #8748:
Do you want the ability to add a few TLS certs to the existing webpki-roots set, or add a list of system certificates?
RootCertStore
has two functions:
add()
: Add a single DER-encoded certificate to the store. This is suitable for a small set of root certificates that are expected to parse successfully.add_parsable_certificates()
: Parse the given DER-encoded certificates and add all that can be parsed in a best-effort fashion.The former would be good when adding a small number of certificates to the store, whereas the latter would be better when adding a large collection of root certificates.
I am assuming we'd probably want to use
add
to add only a few select certificates on top of thewebpki-roots
set, but thought I'd check to make sure.
bacongobbler edited a comment on issue #8748:
Do you want the ability to add a small list of certificates to the existing webpki-roots set, or add a list of system certificates?
RootCertStore
has two functions:
add()
: Add a single DER-encoded certificate to the store. This is suitable for a small set of root certificates that are expected to parse successfully.add_parsable_certificates()
: Parse the given DER-encoded certificates and add all that can be parsed in a best-effort fashion.The former would be good when adding a small number of certificates to the store, whereas the latter would be better when adding a large collection of root certificates.
I am assuming we'd probably want to use
add
to add only a few select certificates on top of thewebpki-roots
set, but thought I'd check to make sure.
bacongobbler commented on issue #8748:
to extend and/or replace that default set of roots with custom root(s).
Assuming we're going the route of "replace", would it be better if the caller provided its own
RootCertStore
? If not present, then we initialize aroot_cert_store
with thewebpki-roots
set?
bacongobbler edited a comment on issue #8748:
to extend and/or replace that default set of roots with custom root(s).
Assuming we're going the route of "extend and/or replace", would it be better if the caller provided its own
RootCertStore
? If not present, then we initialize aroot_cert_store
with thewebpki-roots
set?
bacongobbler edited a comment on issue #8748:
to extend and/or replace that default set of roots with custom root(s).
Assuming we're going the route of "extend and/or replace", would it be better if the caller provided its own
RootCertStore
to use in place of the store provided by wasmtime? If not present, then we initialize aroot_cert_store
with thewebpki-roots
set?
bacongobbler edited a comment on issue #8748:
to extend and/or replace that default set of roots with custom root(s).
Assuming we're going the route of "extend and/or replace", would it be better if the caller provided its own
RootCertStore
to use in place of the store provided by thedefault_send_request_handler
? If not present, then we initialize aroot_cert_store
with thewebpki-roots
set?
lann commented on issue #8748:
It looks like
add
is more general purpose so if default to that approach.
bacongobbler edited a comment on issue #8748:
to extend and/or replace that default set of roots with custom root(s).
Assuming we're going the route of "extend and/or replace", would it be better if the caller provided its own
RootCertStore
to use in place of the store provided by thedefault_send_request_handler
? If it isn't present in theOutgoingRequestConfig
, then we can initialize aroot_cert_store
with thewebpki-roots
set.
bacongobbler commented on issue #8748:
Okay great. That solves the "extend" part of the equation.
Do we want to solve for replacement of the webpki-roots set? Would that be valuable to most callers of this function? Or would we just assume the caller should pass their own custom
send_request_handler
at that point?
lann commented on issue #8748:
Maybe it can take an
Option<rustls::RootCertStore>
whereNone
represents the current behavior. It isn't too hard to reconstruct the defaults and extend if that's what you need.
rajatjindal commented on issue #8748:
I have been exploring what it will take to implement this. one question I have is how do folks expect the additional root ca config to be passed.
- should this be a reference to a file name that contains the additional custom root ca certs or
- should this be a string representation of the additional certs
The problem (with my limited understanding of how wasmtime works) with reference to a filename is that they may be different from the guest and host perspective. so I was thinking maybe passing a string representation of additional certs may work out better. but that would mean we are assuming that guest has access to the certs in the first place.
do you have any suggestions/established-patterns about something like this?
rajatjindal commented on issue #8748:
one additional functionality i am exploring while trying this out is to support
client cert auth
. in addition to above question (file vs string representation of cert), another question I have is around structuring the types.so far I was thinking:
- add following to the
OutgoingRequestConfig
/// The custom root ca to add to root ca store pub custom_root_ca: Option<String>, /// The client auth configuration pub client_cert_auth: Option<ClientCertAuth>,
ClientCertAuth
looks like following:#[derive(Clone)] /// Configuration for client cert auth. pub struct ClientCertAuth { /// The auth cert chain to use for client-auth pub cert_chain: String, /// The private key to use for client-auth pub private_key: String, }
- if
custom_root_ca
is provided,add
this to the default cert root store- if
ClientCertAuth
is provided, change the tls connector to useClientConfig
withwith_client_auth_cert(cert_chain_der, private_key_der)
. The default behavior remains same as how it works today and useswith_no_client_auth
.does this sound like a reasonable approach?
alexcrichton commented on issue #8748:
Given the complexity of TLS configuration and the litany of options/formats I might throw another possibility into the ring which would be to keep the rustls bits we have right now as-is and require further customization to go through a different trait method such as:
trait WasiHttpView { fn sender(&mut self, tls: bool, authority: &str, timeout: Option<Duration>) -> Result<SendRequest<HyperOutgoingBody>>; // ... }
While more difficult to integrate with that would expose the ability to make custom TCP connections using any TLS library. Additionally it would enable configurations such as pooling in theory. We'd need to refactor the
default_send_request
function a bit to plumb this through to there though.
lann commented on issue #8748:
a different trait method
That seems like it would probably account for roughly 50% of the existing default send impl:
I think its totally fine to say that client certs represents too much customization for this kind of common implementation and given Spin's needs that originally motivated this issue I think we would be better off just forking the impl rather than introduce another trait method here.
lann edited a comment on issue #8748:
a different trait method
That seems like it would probably account for roughly 50% of the existing default send impl:
I think its totally fine to say that client certs represent too much customization for this kind of common implementation and given Spin's needs that originally motivated this issue I think we would be better off just forking the impl rather than introduce another trait method here.
lann edited a comment on issue #8748:
a different trait method
That seems like it would probably account for roughly 50% of the existing default send impl:
I think its totally fine to say that client certs represent too much customization for this kind of common implementation. Given Spin's needs that originally motivated this issue I think we would be better off just forking the impl rather than introduce another trait method here.
alexcrichton commented on issue #8748:
That's a good point yeah, and I agree with the conclusion that the best option here might be to copy what's currently done instead of having more hooks for customization (assuming that's ok for Spin of course)
lann commented on issue #8748:
After finishing some refactoring around Spin's implementation of this, I'd like to consider adding a field to
OutgoingRequestConfig
, e.g.tls_config: Option<Arc<rustls::ClientConfig>>
.This would capture pretty much anything that a client would want to configure about TLS and should be easy to implement, basically just hooking in here: https://github.com/bytecodealliance/wasmtime/blob/c8a5acd9831f4ec3780f34d01965b45ddf44a297/crates/wasi-http/src/types.rs#L355-L361
The main downside is that it would strongly couple the interface to
rustls
.
pchickey commented on issue #8748:
Given that the implementation already implies
rustls
for the (overridable) default behavior, I would be in favor of adding a cargo feature for any tls support viarustls
, and then allowing the use of that crate in public interfaces.(Context: I just built an integration by overriding the default handler to proxy the plaintext to another part of the system which handles TLS, but
rustls
is in mycargo vet
s even though it is unreachable.)
Last updated: Dec 23 2024 at 12:05 UTC