Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to use a custom ClientCertResolver? #252

Open
kaisq opened this issue Jan 24, 2022 · 6 comments
Open

How to use a custom ClientCertResolver? #252

kaisq opened this issue Jan 24, 2022 · 6 comments

Comments

@kaisq
Copy link

kaisq commented Jan 24, 2022

I'd like to be able to use a custom ClientCertResolver from C code that uses rustls-ffi to perform TLS. I currently have a resolver that implements ClientCertResolver that works fine with rustls inside of Rust code, but I can't see any way to use that within C code via rustls-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:

  1. expose enough symbols on the rust side from rustls-ffi that it can be included as a library in another rust project, where that project exports a function rustls_result mylibrary_rustls_client_config_builder_set_custom_resolver(*rustls_client_config_builder builder);
  2. expose a struct rustls_client_cert_resolver on the rust side that lets you convert a dyn ClientCertResolver into a type that can be understood in C code from rustls-ffi, and can be used like this:
rustls_client_cert_resolver resolver = mylibrary_custom_cert_resolver();
rustls_client_config_builder_set_client_cert_resolver(resolver);

Any opinions on this? I'm happy to send a PR if there are any reasonable options.

@jsha
Copy link
Collaborator

jsha commented Jan 25, 2022

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:

https://docs.rs/rustls-ffi/latest/rustls_ffi/client/struct.rustls_client_config_builder.html#method.rustls_client_config_builder_set_certified_key

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.

@kaisq
Copy link
Author

kaisq commented Jan 25, 2022

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.

@jsha
Copy link
Collaborator

jsha commented Jan 25, 2022

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?

@kaisq
Copy link
Author

kaisq commented Jan 26, 2022

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?

@jsha
Copy link
Collaborator

jsha commented Jan 27, 2022

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.

@kaisq
Copy link
Author

kaisq commented Jan 27, 2022

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants