Open kaisq opened 2 years ago
Hi @kaisq! Thanks for trying out rustls-ffi. Can you tell me a bit more about your application? It sounds like you have some Rust components and some C components, but it's not clear to me which does which.
Also, what are your needs in terms of how the client certificate is selected? Are you making a network call, or selecting among a set of certificates and keys already loaded in memory? Is the certificate likely to change with each connection?
Current client certificate support in rustls is done with the client config builder. You can set a certificate and private key that will be used unconditionally for all connections with that config:
If you need more dynamic certificate resolution, you're right that we would want something like rustls_client_config_builder_set_client_cert_resolver
, where client_cert_resolver would be a C callback (with a userdata parameter, acceptable_issuers, and sigschemes). Then your code could implement a C callback that calls your Rust implementation.
Thanks for getting back to me! Yes, let me provide a little more detail here.
First, I am aware of the *_set_certified_key
function. This will not work in my use case, because the key material is stored in the SecureEnclave on macOS and in a TPM on Linux/Windows, and therefore I never have access to the raw key bytes in memory. Signing/encryption operations need to be offloaded to the Secure Enclave or TPM, so, for example, I have a KeychainResolver
that implements ResolvesClientCert
and its resolve()
implementation returns a CertifiedKey
containing a key
with a custom SigningKey
implementation that calls the relevant Keychain API functions.
Second, let me clarify a little bit about which parts are written in rust and which are in C in my environment. I have two programs that I'm writing: one is built in rust and another is written in C. Each program has significantly different behavior best suited for the language it's written in, but both need to make TLS calls to the same server using the same somewhat complex client certificate authentication configuration. Here's the basic outline of dependencies in the two codebases:
rust-binary:
-> rustls
-> my rust library with a struct implementing the ResolvesClientCert trait from rustls
C-binary:
-> rustls-ffi
-> rustls
-> ??
My hope is to add a second dependency to the C-binary
which would be a C wrapper of the similar rust library, which would provide the same KeychainResolver
to rustls-ffi
that I provide to rustls
in the rust-binary
.
I appreciate the idea of creating a *_set_client_cert_resolver
function that takes a C callback that effectively implements the resolve()
function from the ResolvesClientCert
trait, but I worry that that isn't going to work well enough for me given that resolvee()
returns the CertifiedKey
type from rustls
, which includes, in my case, custom structs that implement things like the SigningKey
trait. Do you think that rustls-ffi
could be set up in a way where that would work properly?
And If that does work, I'm not sure I see the difference between providing rustls-ffi
wrapping types/functions for all the types that would be required to return a proper CertifiedKey
in a resolve()
implementation, versus providing a wrapping type for a struct implementing ResolvesClientCert
, although I will admit that I am much less familiar with C design patterns than rust design patterns. Basically, my main goal here is to try to share as much code as possible between these two programs I've listed in the comment here, and a significant number more programs that will be sharing the same client certificate authentication mechanism in the future. I'm definitely open to any ideas that will reduce the amount of duplicated code in the different implementations.
Ah, thanks for the clarification! I think what we need is a new function/method to build a rustls_certified_key (the opaque FFI view onto a rustls::CertifiedKey. Right now the only constructor accepts a cert chain and private key bytes.
We would need a new constructor that takes a cert chain, a pair of function pointers, and userdata for the function pointer. The function pointers would match the choose_scheme and algorithm methods of SigningKey, translated appropriately into C. The constructor might be called something like rustls_certified_key_build_dynamic
.
On your side, you would implement a pair of extern "C"
functions that forward to your implementation of SigningKey, and use those as the arguments to the rustls_certified_key_build_dynamic
.
Does that make sense?
Ok, I think I understand the implementation you're suggesting. Basically, we'd add a new rustls_certified_key_build_dynamic
to the impl rustls_certified_key
block, which would pass in a couple callback functions that would perform the duties of the two functions declared in the SigningKey
trait. The userdata would be some void *
that could be used to track whatever data would have been stored in the struct that implements the SigningKey
trait. Is that more or less correct? If so, it is true that this might cover my current use case.
However, I'd also like to propose another possible implementation and see what you think. Given that the ClientConfigBuilder
struct already has a cert_resolver
, and that we're already dealing with cases where we're modifying properties on the client config potentially dangerously, could we make a few of the relevant macros/traits public that are used in rustls-ffi and make it possible to set arbitrary values on a ClientConfigBuilder
from another library? Basically, we'd be arbitrarily extending the library to provide whichever client configurations are required if there isn't a native library function to support it. It would look something like this:
// mylib/src/lib.rs
use rustls_ffi::{
// these macros are already marked #[macro_export], but some of the things they reference are
// pub(crate) only and those would need to be set public.
ffi_panic_boundary, try_mut_from_ptr,
rustls_result,
ClientConfigBuilder,
}
#[no_mangle]
extern "C" fn mylib_rustls_client_config_builder_set_arbitrary(builder: *mut rustls_client_config_builder) {
ffi_panic_boundary! {
let config: &mut ClientConfigBuilder = try_mut_from_ptr!(builder);
let resolver = match KeychainResolver::new() {
Ok(r) => r,
Err(_) => return rustls_result::NullParameter,
}
config.cert_resolver = Some(Arc::new(resolver));
rustls_result::Ok
}
}
Any thoughts on that?
Basically, we'd add a new rustls_certified_key_build_dynamic to the impl rustls_certified_key block, which would pass in a couple callback functions that would perform the duties of the two functions declared in the SigningKey trait. The userdata would be some void * that could be used to track whatever data would have been stored in the struct that implements the SigningKey trait. Is that more or less correct? If so, it is true that this might cover my current use case.
Yep!
Given that the ClientConfigBuilder struct already has a cert_resolver, and that we're already dealing with cases where we're modifying properties on the client config potentially dangerously, could we make a few of the relevant macros/traits public that are used in rustls-ffi and make it possible to set arbitrary values on a ClientConfigBuilder from another library?
I don't want to take this approach because I don't want the representation of rustls_client_config_builder
to be public. It's changed a few times and is likely to change again. It's not always directly a ClientConfigBuilder
. Also, the first approach has the benefit that it would allow C programs to implement a hardware-backed SigningKey without writing Rust code.
// these macros are already marked #[macro_export], but some of the things they reference are
// pub(crate) only and those would need to be set public.
FWIW, these macros are not intended to be used publicly - this was just a matter of making them available across internal modules. I believe I can do this better and keep them properly private.
Definitely agreed that having pure C options available are good, and my proposal certainly wasn't meant to be a replacement for the pure C option -- I'm just trying to also come up with solutions that are for the admittedly rare case of shared code between rust and C programs. I may try implementing something like what you've proposed, although it may end up being easier for me to just continue to maintain a fork of rustls-ffi with support for my client cert resolvers built directly into it instead of having one C and one rust implementation of the exact same code to load the keys from the respective native keystores.
I did have one other idea I thought I might pass by you, although even I agree it's a little crazy. Basically, we'd have a rustls_client_config_builder_set_cert_resolver_dynamic()
that takes a callback that sets an arbitrary cert resolver to an output parameter. It would still require making some of the *CastPtr
traits public, but it would allow ClientCertBuilder
to still stay private. It would also not be likely to break in the future, even if the internal format of ClientCertBuilder
changed, since y'all already use a client cert resolver via ResolvesClientCertFromChoices
. It would look something like this:
// rustls-ffi/src/client.rs
pub type rustls_create_client_cert_resolver_callback =
unsafe extern "C" fn(
out: *mut *const rustls_client_cert_resolver,
) -> rustls_result;
impl rustls_client_config_builder {
/* current implementation */
#[no_mangle]
pub extern "C" fn rustls_client_config_builder_set_client_cert_resolver_dynamic(
builder: *mut rustls_client_config_builder,
resolver_cb: rustls_create_client_cert_resolver_callback,
) -> rustls_result {
ffi_panic_boundary! {
let config: &mut ClientConfigBuilder = try_mut_from_ptr!(builder);
let mut resolver = std::mem::MaybeUninit::<*const rustls_client_cert_resolver>::uninit();
let result = unsafe { resolver_cb(resolver.as_mut_ptr()) };
if result != rustls_result::Ok {
return result;
}
let resolver = unsafe { resolver.assume_init() };
let resolver: &Arc<dyn ResolvesClientCert> = try_ref_from_ptr!(resolver);
config.cert_resolver = Some(resolver.clone());
rustls_result::Ok
}
}
}
// rustls-ffi/src/cipher.rs
pub struct rustls_client_cert_resolver {
_private: [u8; 0],
}
impl CastPtr for rustls_client_cert_resolver {
type RustType = Arc<dyn ResolvesClientCert>;
}
impl ArcCastPtr for rustls_client_cert_resolver {}
// mylib/src/lib.rs
#[no_mangle]
extern "C" fn mylib_rustls_capture_client_cert_resolver(out: *mut *const rustls_client_cert_resolver) -> rustls_result {
let resolver = match KeychainResolver::new() {
Ok(r) => r,
Err(_) => return results_result::NullParameter,
};
let resolver: Arc<dyn ResolvesClientCert> = Arc::new(resolver);
unsafe { *out = ArcCastPtr::to_const_ptr(resolver); }
rustls_result::Ok
}
And then it would be used in a client like this:
struct rustls_client_config_builder *config_builder = // ...
rustls_client_config_builder_set_client_cert_resolver_dynamic(config_builder, mylib_rustls_capture_client_cert_resolver);
Any thoughts about that option?
I'd like to be able to use a custom
ClientCertResolver
from C code that usesrustls-ffi
to perform TLS. I currently have a resolver that implementsClientCertResolver
that works fine withrustls
inside of Rust code, but I can't see any way to use that within C code viarustls-ffi
. Am I missing something that would make this work?I've thought a little bit about how this sort of option might be exposed, and it seems like it might be a little complicated to make a good API for this. My thoughts are:
rustls-ffi
that it can be included as a library in another rust project, where that project exports a functionrustls_result mylibrary_rustls_client_config_builder_set_custom_resolver(*rustls_client_config_builder builder);
struct rustls_client_cert_resolver
on the rust side that lets you convert adyn ClientCertResolver
into a type that can be understood in C code fromrustls-ffi
, and can be used like this:Any opinions on this? I'm happy to send a PR if there are any reasonable options.