From 64952519459e001f07b09e4305513f7507727a9e Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 19 Jul 2024 12:38:49 -0400 Subject: [PATCH 1/2] docs: clarify freeing client/server builder directly Previously the docs for the `rustls_client_config_builder` and `rustls_server_config_builder` structs seemed to imply the only safe way to dispose of a builder was to build it, and then free the resulting config. It's also possible (and clearer for most error-handling paths) to free the builder directly. This commit updates the relevant docs to make this alternative option clearer. --- src/client.rs | 21 ++++++++++++++------- src/rustls.h | 42 +++++++++++++++++++++++++++--------------- src/server.rs | 23 +++++++++++++++-------- 3 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/client.rs b/src/client.rs index a225765e..7a52318c 100644 --- a/src/client.rs +++ b/src/client.rs @@ -31,9 +31,11 @@ box_castable! { /// A client config being constructed. A builder can be modified by, /// e.g. rustls_client_config_builder_load_roots_from_file. Once you're /// done configuring settings, call rustls_client_config_builder_build - /// to turn it into a *rustls_client_config. This object is not safe - /// for concurrent mutation. Under the hood, it corresponds to a - /// `Box`. + /// to turn it into a *rustls_client_config. Alternatively, if an error + /// occurs or, you don't wish to build a config, call + /// `rustls_client_config_builder_free` to free the builder directly. + /// This object is not safe for concurrent mutation. Under the hood, + /// it corresponds to a `Box`. /// pub struct rustls_client_config_builder(ClientConfigBuilder); } @@ -96,9 +98,11 @@ impl ServerCertVerifier for NoneVerifier { impl rustls_client_config_builder { /// Create a rustls_client_config_builder. Caller owns the memory and must /// eventually call rustls_client_config_builder_build, then free the - /// resulting rustls_client_config. - /// This uses rustls safe default values - /// for the cipher suites, key exchange groups and protocol versions. + /// resulting rustls_client_config. Alternatively, if an error occurs + /// or, you don't wish to build a config, call `rustls_client_config_builder_free` + /// to free the builder directly. + /// This uses rustls safe default values for the cipher suites, key exchange + /// groups and protocol versions. /// This starts out with no trusted roots. /// Caller must add roots with rustls_client_config_builder_load_roots_from_file /// or provide a custom verifier. @@ -125,7 +129,9 @@ impl rustls_client_config_builder { /// Create a rustls_client_config_builder. Caller owns the memory and must /// eventually call rustls_client_config_builder_build, then free the - /// resulting rustls_client_config. Specify cipher suites in preference + /// resulting rustls_client_config. Alternatively, if an error occurs + /// or, you don't wish to build a config, call `rustls_client_config_builder_free` + /// to free the builder directly. Specify cipher suites in preference /// order; the `cipher_suites` parameter must point to an array containing /// `len` pointers to `rustls_supported_ciphersuite` previously obtained /// from `rustls_all_ciphersuites_get_entry()`, or to a provided array, @@ -244,6 +250,7 @@ struct Verifier { /// Safety: Verifier is Send because we don't allocate or deallocate any of its /// fields. unsafe impl Send for Verifier {} + /// Safety: Verifier is Sync if the C code that passes us a callback that /// obeys the concurrency safety requirements documented in /// rustls_client_config_builder_dangerous_set_certificate_verifier. diff --git a/src/rustls.h b/src/rustls.h index 738a3fed..5442b331 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -203,9 +203,11 @@ typedef struct rustls_client_config rustls_client_config; * A client config being constructed. A builder can be modified by, * e.g. rustls_client_config_builder_load_roots_from_file. Once you're * done configuring settings, call rustls_client_config_builder_build - * to turn it into a *rustls_client_config. This object is not safe - * for concurrent mutation. Under the hood, it corresponds to a - * `Box`. + * to turn it into a *rustls_client_config. Alternatively, if an error + * occurs or, you don't wish to build a config, call + * `rustls_client_config_builder_free` to free the builder directly. + * This object is not safe for concurrent mutation. Under the hood, + * it corresponds to a `Box`. * */ typedef struct rustls_client_config_builder rustls_client_config_builder; @@ -253,8 +255,10 @@ typedef struct rustls_server_config rustls_server_config; * A server config being constructed. A builder can be modified by, * e.g. rustls_server_config_builder_load_native_roots. Once you're * done configuring settings, call rustls_server_config_builder_build - * to turn it into a *const rustls_server_config. This object is not safe - * for concurrent mutation. + * to turn it into a *const rustls_server_config. Alternatively, if + * an error occurs or, you don't wish to build a config, call + * `rustls_server_config_builder_free` to free the builder directly. + * This object is not safe for concurrent mutation. * */ typedef struct rustls_server_config_builder rustls_server_config_builder; @@ -1271,9 +1275,11 @@ void rustls_server_cert_verifier_free(struct rustls_server_cert_verifier *verifi /** * Create a rustls_client_config_builder. Caller owns the memory and must * eventually call rustls_client_config_builder_build, then free the - * resulting rustls_client_config. - * This uses rustls safe default values - * for the cipher suites, key exchange groups and protocol versions. + * resulting rustls_client_config. Alternatively, if an error occurs + * or, you don't wish to build a config, call `rustls_client_config_builder_free` + * to free the builder directly. + * This uses rustls safe default values for the cipher suites, key exchange + * groups and protocol versions. * This starts out with no trusted roots. * Caller must add roots with rustls_client_config_builder_load_roots_from_file * or provide a custom verifier. @@ -1283,7 +1289,9 @@ struct rustls_client_config_builder *rustls_client_config_builder_new(void); /** * Create a rustls_client_config_builder. Caller owns the memory and must * eventually call rustls_client_config_builder_build, then free the - * resulting rustls_client_config. Specify cipher suites in preference + * resulting rustls_client_config. Alternatively, if an error occurs + * or, you don't wish to build a config, call `rustls_client_config_builder_free` + * to free the builder directly. Specify cipher suites in preference * order; the `cipher_suites` parameter must point to an array containing * `len` pointers to `rustls_supported_ciphersuite` previously obtained * from `rustls_all_ciphersuites_get_entry()`, or to a provided array, @@ -1687,7 +1695,9 @@ struct rustls_str rustls_slice_str_get(const struct rustls_slice_str *input, siz /** * Create a rustls_server_config_builder. Caller owns the memory and must * eventually call rustls_server_config_builder_build, then free the - * resulting rustls_server_config. This uses rustls safe default values + * resulting rustls_server_config. Alternatively, if an error occurs or, + * you don't wish to build a config, call `rustls_server_config_builder_free` + * to free the builder directly. This uses rustls safe default values * for the cipher suites, key exchange groups and protocol versions. */ struct rustls_server_config_builder *rustls_server_config_builder_new(void); @@ -1695,11 +1705,13 @@ struct rustls_server_config_builder *rustls_server_config_builder_new(void); /** * Create a rustls_server_config_builder. Caller owns the memory and must * eventually call rustls_server_config_builder_build, then free the - * resulting rustls_server_config. Specify cipher suites in preference - * order; the `cipher_suites` parameter must point to an array containing - * `len` pointers to `rustls_supported_ciphersuite` previously obtained - * from `rustls_all_ciphersuites_get_entry()`. Set the TLS protocol - * versions to use when negotiating a TLS session. + * resulting rustls_server_config. Alternatively, if + * an error occurs or, you don't wish to build a config, call + * `rustls_server_config_builder_free` to free the builder directly. Specify + * cipher suites in preference order; the `cipher_suites` parameter must + * point to an array containing `len` pointers to `rustls_supported_ciphersuite` + * previously obtained from `rustls_all_ciphersuites_get_entry()`. + * Set the TLS protocol versions to use when negotiating a TLS session. * * `tls_version` is the version of the protocol, as defined in rfc8446, * ch. 4.2.1 and end of ch. 5.1. Some values are defined in diff --git a/src/server.rs b/src/server.rs index 21ee8530..7854167a 100644 --- a/src/server.rs +++ b/src/server.rs @@ -34,8 +34,10 @@ box_castable! { /// A server config being constructed. A builder can be modified by, /// e.g. rustls_server_config_builder_load_native_roots. Once you're /// done configuring settings, call rustls_server_config_builder_build - /// to turn it into a *const rustls_server_config. This object is not safe - /// for concurrent mutation. + /// to turn it into a *const rustls_server_config. Alternatively, if + /// an error occurs or, you don't wish to build a config, call + /// `rustls_server_config_builder_free` to free the builder directly. + /// This object is not safe for concurrent mutation. /// pub struct rustls_server_config_builder(ServerConfigBuilder); } @@ -59,7 +61,9 @@ arc_castable! { impl rustls_server_config_builder { /// Create a rustls_server_config_builder. Caller owns the memory and must /// eventually call rustls_server_config_builder_build, then free the - /// resulting rustls_server_config. This uses rustls safe default values + /// resulting rustls_server_config. Alternatively, if an error occurs or, + /// you don't wish to build a config, call `rustls_server_config_builder_free` + /// to free the builder directly. This uses rustls safe default values /// for the cipher suites, key exchange groups and protocol versions. #[no_mangle] pub extern "C" fn rustls_server_config_builder_new() -> *mut rustls_server_config_builder { @@ -85,11 +89,13 @@ impl rustls_server_config_builder { /// Create a rustls_server_config_builder. Caller owns the memory and must /// eventually call rustls_server_config_builder_build, then free the - /// resulting rustls_server_config. Specify cipher suites in preference - /// order; the `cipher_suites` parameter must point to an array containing - /// `len` pointers to `rustls_supported_ciphersuite` previously obtained - /// from `rustls_all_ciphersuites_get_entry()`. Set the TLS protocol - /// versions to use when negotiating a TLS session. + /// resulting rustls_server_config. Alternatively, if + /// an error occurs or, you don't wish to build a config, call + /// `rustls_server_config_builder_free` to free the builder directly. Specify + /// cipher suites in preference order; the `cipher_suites` parameter must + /// point to an array containing `len` pointers to `rustls_supported_ciphersuite` + /// previously obtained from `rustls_all_ciphersuites_get_entry()`. + /// Set the TLS protocol versions to use when negotiating a TLS session. /// /// `tls_version` is the version of the protocol, as defined in rfc8446, /// ch. 4.2.1 and end of ch. 5.1. Some values are defined in @@ -524,6 +530,7 @@ impl ResolvesServerCert for ClientHelloResolver { /// as the registered callbacks are thread safe. This is /// documented as a requirement in the API. unsafe impl Sync for ClientHelloResolver {} + unsafe impl Send for ClientHelloResolver {} impl Debug for ClientHelloResolver { From b3563a09921bd8bbe2e80a24fd072d397871170d Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 29 Jul 2024 09:55:37 -0400 Subject: [PATCH 2/2] add more whitespace to doc comments, fix var names Small readability tweaks to the comments that were updated in the previous commit to add some whitespace. Also fixes some parameter names that were referenced in `rustls_client_config_builder_new_custom` with the incorrect name. --- src/client.rs | 47 ++++++++++++++++++++++++++++------------------- src/rustls.h | 47 ++++++++++++++++++++++++++++------------------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/src/client.rs b/src/client.rs index 7a52318c..7b3dd8e7 100644 --- a/src/client.rs +++ b/src/client.rs @@ -31,9 +31,11 @@ box_castable! { /// A client config being constructed. A builder can be modified by, /// e.g. rustls_client_config_builder_load_roots_from_file. Once you're /// done configuring settings, call rustls_client_config_builder_build - /// to turn it into a *rustls_client_config. Alternatively, if an error - /// occurs or, you don't wish to build a config, call - /// `rustls_client_config_builder_free` to free the builder directly. + /// to turn it into a *rustls_client_config. + /// + /// Alternatively, if an error occurs or, you don't wish to build a config, + /// call `rustls_client_config_builder_free` to free the builder directly. + /// /// This object is not safe for concurrent mutation. Under the hood, /// it corresponds to a `Box`. /// @@ -98,14 +100,17 @@ impl ServerCertVerifier for NoneVerifier { impl rustls_client_config_builder { /// Create a rustls_client_config_builder. Caller owns the memory and must /// eventually call rustls_client_config_builder_build, then free the - /// resulting rustls_client_config. Alternatively, if an error occurs - /// or, you don't wish to build a config, call `rustls_client_config_builder_free` - /// to free the builder directly. + /// resulting rustls_client_config. + /// + /// Alternatively, if an error occurs or, you don't wish to build a config, + /// call `rustls_client_config_builder_free` to free the builder directly. + /// /// This uses rustls safe default values for the cipher suites, key exchange /// groups and protocol versions. - /// This starts out with no trusted roots. - /// Caller must add roots with rustls_client_config_builder_load_roots_from_file - /// or provide a custom verifier. + /// + /// This starts out with no trusted roots. Caller must add roots with + /// rustls_client_config_builder_load_roots_from_file or provide a custom + /// verifier. #[no_mangle] pub extern "C" fn rustls_client_config_builder_new() -> *mut rustls_client_config_builder { ffi_panic_boundary! { @@ -129,22 +134,26 @@ impl rustls_client_config_builder { /// Create a rustls_client_config_builder. Caller owns the memory and must /// eventually call rustls_client_config_builder_build, then free the - /// resulting rustls_client_config. Alternatively, if an error occurs - /// or, you don't wish to build a config, call `rustls_client_config_builder_free` - /// to free the builder directly. Specify cipher suites in preference - /// order; the `cipher_suites` parameter must point to an array containing - /// `len` pointers to `rustls_supported_ciphersuite` previously obtained - /// from `rustls_all_ciphersuites_get_entry()`, or to a provided array, - /// RUSTLS_DEFAULT_CIPHER_SUITES or RUSTLS_ALL_CIPHER_SUITES. Set the TLS - /// protocol versions to use when negotiating a TLS session. + /// resulting rustls_client_config. + /// + /// Alternatively, if an error occurs or, you don't wish to build a config, + /// call `rustls_client_config_builder_free` to free the builder directly. + /// + /// Specify cipher suites in preference order; the `cipher_suites` parameter + /// must point to an array containing `cipher_suites_len` pointers to + /// `rustls_supported_ciphersuite` previously obtained from + /// `rustls_all_ciphersuites_get_entry()`, or to a provided array, + /// RUSTLS_DEFAULT_CIPHER_SUITES or RUSTLS_ALL_CIPHER_SUITES. /// + /// Set the TLS protocol versions to use when negotiating a TLS session. /// `tls_version` is the version of the protocol, as defined in rfc8446, /// ch. 4.2.1 and end of ch. 5.1. Some values are defined in /// `rustls_tls_version` for convenience, and the arrays /// RUSTLS_DEFAULT_VERSIONS or RUSTLS_ALL_VERSIONS can be used directly. /// - /// `versions` will only be used during the call and the application retains - /// ownership. `len` is the number of consecutive `uint16_t` pointed to by `versions`. + /// `tls_versions` will only be used during the call and the application retains + /// ownership. `tls_versions_len` is the number of consecutive `uint16_t` + /// pointed to by `tls_versions`. #[no_mangle] pub extern "C" fn rustls_client_config_builder_new_custom( cipher_suites: *const *const rustls_supported_ciphersuite, diff --git a/src/rustls.h b/src/rustls.h index 5442b331..a3184172 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -203,9 +203,11 @@ typedef struct rustls_client_config rustls_client_config; * A client config being constructed. A builder can be modified by, * e.g. rustls_client_config_builder_load_roots_from_file. Once you're * done configuring settings, call rustls_client_config_builder_build - * to turn it into a *rustls_client_config. Alternatively, if an error - * occurs or, you don't wish to build a config, call - * `rustls_client_config_builder_free` to free the builder directly. + * to turn it into a *rustls_client_config. + * + * Alternatively, if an error occurs or, you don't wish to build a config, + * call `rustls_client_config_builder_free` to free the builder directly. + * * This object is not safe for concurrent mutation. Under the hood, * it corresponds to a `Box`. * @@ -1275,36 +1277,43 @@ void rustls_server_cert_verifier_free(struct rustls_server_cert_verifier *verifi /** * Create a rustls_client_config_builder. Caller owns the memory and must * eventually call rustls_client_config_builder_build, then free the - * resulting rustls_client_config. Alternatively, if an error occurs - * or, you don't wish to build a config, call `rustls_client_config_builder_free` - * to free the builder directly. + * resulting rustls_client_config. + * + * Alternatively, if an error occurs or, you don't wish to build a config, + * call `rustls_client_config_builder_free` to free the builder directly. + * * This uses rustls safe default values for the cipher suites, key exchange * groups and protocol versions. - * This starts out with no trusted roots. - * Caller must add roots with rustls_client_config_builder_load_roots_from_file - * or provide a custom verifier. + * + * This starts out with no trusted roots. Caller must add roots with + * rustls_client_config_builder_load_roots_from_file or provide a custom + * verifier. */ struct rustls_client_config_builder *rustls_client_config_builder_new(void); /** * Create a rustls_client_config_builder. Caller owns the memory and must * eventually call rustls_client_config_builder_build, then free the - * resulting rustls_client_config. Alternatively, if an error occurs - * or, you don't wish to build a config, call `rustls_client_config_builder_free` - * to free the builder directly. Specify cipher suites in preference - * order; the `cipher_suites` parameter must point to an array containing - * `len` pointers to `rustls_supported_ciphersuite` previously obtained - * from `rustls_all_ciphersuites_get_entry()`, or to a provided array, - * RUSTLS_DEFAULT_CIPHER_SUITES or RUSTLS_ALL_CIPHER_SUITES. Set the TLS - * protocol versions to use when negotiating a TLS session. + * resulting rustls_client_config. * + * Alternatively, if an error occurs or, you don't wish to build a config, + * call `rustls_client_config_builder_free` to free the builder directly. + * + * Specify cipher suites in preference order; the `cipher_suites` parameter + * must point to an array containing `cipher_suites_len` pointers to + * `rustls_supported_ciphersuite` previously obtained from + * `rustls_all_ciphersuites_get_entry()`, or to a provided array, + * RUSTLS_DEFAULT_CIPHER_SUITES or RUSTLS_ALL_CIPHER_SUITES. + * + * Set the TLS protocol versions to use when negotiating a TLS session. * `tls_version` is the version of the protocol, as defined in rfc8446, * ch. 4.2.1 and end of ch. 5.1. Some values are defined in * `rustls_tls_version` for convenience, and the arrays * RUSTLS_DEFAULT_VERSIONS or RUSTLS_ALL_VERSIONS can be used directly. * - * `versions` will only be used during the call and the application retains - * ownership. `len` is the number of consecutive `uint16_t` pointed to by `versions`. + * `tls_versions` will only be used during the call and the application retains + * ownership. `tls_versions_len` is the number of consecutive `uint16_t` + * pointed to by `tls_versions`. */ rustls_result rustls_client_config_builder_new_custom(const struct rustls_supported_ciphersuite *const *cipher_suites, size_t cipher_suites_len,