From 380e3ebd525a2b4d82ac81d3da180f03b434777b Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Sat, 14 Dec 2024 17:31:27 -0500 Subject: [PATCH 1/2] tests: support multiple HTTPS RRs for ECH configs Similar to a change in the upstream Rustls ech-client.rs demo we want to be able to process _multiple_ HTTPS records for a given domain, and look at each ECH config list from each record for a potential compatible config. Mechanically this means: 1. Updating the `test/ech_fetch.rs` helper to support writing multiple `.bin` files when there are multiple HTTPS records w/ ECH configs. The tool now outputs to stdout a comma separated list of the files it writes to make it easier to use with the `client.c` example. 2. Updating the `tests/client.c` example to treat the `RUSTLS_ECH_CONFIG_LIST` env var as a comma separated list of ECH config lists. We now loop through each and only fail if all of the provided files are unable to be used to configure the client config with a compatible ECH config. Doing string manipulation with C remains "a delight". For Windows compat we achieve tokenizing the string by the comma delim with a define to call either `strtok_r` with GCC/clang, or `strtok_s` with MSCV. You can test this update with: ``` ECH_CONFIG_LISTS=$(cargo test --test ech_fetch -- curves1-ng.test.defo.ie /tmp/curves1-ng.test.defo.ie) RUSTLS_PLATFORM_VERIFIER=1 RUSTLS_ECH_CONFIG_LIST="$ECH_CONFIG_LISTS" ./cmake-build-debug/tests/client curves1-ng.test.defo.ie 443 /echstat.php?format=json ``` If you're unlucky and the first HTTPS record served is the one with invalid configs you should see output like the following showing the client skipping over the `.1` config list and using the `.2` one instead: ``` client[188911]: no compatible/valid ECH configs found in '/tmp/curves1-ng.test.defo.ie.1' client[188911]: using ECH with config list from '/tmp/curves1-ng.test.defo.ie.2' ``` --- tests/client.c | 93 ++++++++++++++++++++++++++++++++++------------ tests/common.h | 6 +++ tests/ech_fetch.rs | 69 +++++++++++++++++++++++++--------- 3 files changed, 126 insertions(+), 42 deletions(-) diff --git a/tests/client.c b/tests/client.c index 2fafc305..c93ae196 100644 --- a/tests/client.c +++ b/tests/client.c @@ -474,7 +474,9 @@ main(int argc, const char **argv) config_builder = rustls_client_config_builder_new(); } - if(getenv("RUSTLS_ECH_GREASE")) { + const char *rustls_ech_grease = getenv("RUSTLS_ECH_GREASE"); + const char *rustls_ech_config_list = getenv("RUSTLS_ECH_CONFIG_LIST"); + if(rustls_ech_grease) { const rustls_hpke *hpke = rustls_supported_hpke(); if(hpke == NULL) { fprintf(stderr, "client: no HPKE suites for ECH available\n"); @@ -489,39 +491,82 @@ main(int argc, const char **argv) } fprintf(stderr, "configured for ECH GREASE\n"); } - else if(getenv("RUSTLS_ECH_CONFIG_LIST")) { + else if(rustls_ech_config_list) { const rustls_hpke *hpke = rustls_supported_hpke(); if(hpke == NULL) { fprintf(stderr, "client: no HPKE suites for ECH available\n"); goto cleanup; } - char ech_config_list_buf[10000]; - size_t ech_config_list_len; - - unsigned int read_result = read_file(getenv("RUSTLS_ECH_CONFIG_LIST"), - ech_config_list_buf, - sizeof(ech_config_list_buf), - &ech_config_list_len); - if(read_result != DEMO_OK) { - fprintf(stderr, - "client: failed to read ECH config list file: '%s'\n", - getenv("RUSTLS_ECH_CONFIG_LIST")); + // Duplicate the ENV var value - calling STRTOK_R will modify the string + // to add null terminators between tokens. + char *ech_config_list_copy = strdup(rustls_ech_config_list); + if(!ech_config_list_copy) { + LOG_SIMPLE("failed to allocate memory for ECH config list"); goto cleanup; } - result = - rustls_client_config_builder_enable_ech(config_builder, - (uint8_t *)ech_config_list_buf, - ech_config_list_len, - hpke); - if(result != RUSTLS_RESULT_OK) { - fprintf(stderr, "client: failed to configure ECH"); - goto cleanup; + + bool ech_configured = false; + // Tokenize the ech_config_list_copy by comma. The first invocation takes + // ech_config_list_copy. This is reentrant by virtue of saving state to + // saveptr. Only the _first_ invocation is given the original string. + // Subsequent calls should pass NULL and the same delim/saveptr. + const char *delim = ","; + char *saveptr = NULL; + char *ech_config_list_path = + STRTOK_R(ech_config_list_copy, delim, &saveptr); + + while(ech_config_list_path) { + // Skip leading spaces + while(*ech_config_list_path == ' ') { + ech_config_list_path++; + } + + // Try to read the token as a file path to an ECH config list. + char ech_config_list_buf[10000]; + size_t ech_config_list_len; + const enum demo_result read_result = + read_file(ech_config_list_path, + ech_config_list_buf, + sizeof(ech_config_list_buf), + &ech_config_list_len); + + // If we can't read the file, warn and continue + if(read_result != DEMO_OK) { + // Continue to the next token. + LOG("unable to read ECH config list from '%s'", ech_config_list_path); + ech_config_list_path = STRTOK_R(NULL, delim, &saveptr); + continue; + } + + // Try to enable ECH with the config list. This may error if none + // of the ECH configs are valid/compatible. + result = + rustls_client_config_builder_enable_ech(config_builder, + (uint8_t *)ech_config_list_buf, + ech_config_list_len, + hpke); + + // If we successfully configured ECH with the config list then break. + if(result == RUSTLS_RESULT_OK) { + LOG("using ECH with config list from '%s'", ech_config_list_path); + ech_configured = true; + break; + } + + // Otherwise continue to the next token. + LOG("no compatible/valid ECH configs found in '%s'", + ech_config_list_path); + ech_config_list_path = STRTOK_R(NULL, delim, &saveptr); } - fprintf(stderr, - "client: using ECH with config list from '%s'\n", - getenv("RUSTLS_ECH_CONFIG_LIST")); + // Free the copy of the env var we made. + free(ech_config_list_copy); + + if(!ech_configured) { + LOG_SIMPLE("failed to configure ECH with any provided config files"); + goto cleanup; + } } if(getenv("RUSTLS_PLATFORM_VERIFIER")) { diff --git a/tests/common.h b/tests/common.h index fd234904..075af3f3 100644 --- a/tests/common.h +++ b/tests/common.h @@ -23,6 +23,12 @@ const char *ws_strerror(int err); #endif /* !STDOUT_FILENO */ #endif /* _WIN32 */ +#if defined(_MSC_VER) +#define STRTOK_R strtok_s +#else +#define STRTOK_R strtok_r +#endif + enum demo_result { DEMO_OK, diff --git a/tests/ech_fetch.rs b/tests/ech_fetch.rs index 9c7ef4fa..92d9de21 100644 --- a/tests/ech_fetch.rs +++ b/tests/ech_fetch.rs @@ -11,7 +11,7 @@ use std::io::Write; use hickory_resolver::config::{ResolverConfig, ResolverOpts}; use hickory_resolver::proto::rr::rdata::svcb::{SvcParamKey, SvcParamValue}; use hickory_resolver::proto::rr::{RData, RecordType}; -use hickory_resolver::{Resolver, TokioResolver}; +use hickory_resolver::{ResolveError, Resolver, TokioResolver}; use rustls::pki_types::EchConfigListBytes; @@ -24,27 +24,60 @@ async fn main() -> Result<(), Box> { .unwrap_or(format!("testdata/{}.ech.configs.bin", domain)); let resolver = Resolver::tokio(ResolverConfig::google_https(), ResolverOpts::default()); - let tls_encoded_list = lookup_ech(&resolver, &domain).await; - let mut encoded_list_file = File::create(output_path)?; - encoded_list_file.write_all(&tls_encoded_list)?; + let all_lists = lookup_ech_configs(&resolver, &domain).await?; + + // If there was only one HTTPS record with an ech config, write it to the output file. + if all_lists.len() == 1 { + let mut encoded_list_file = File::create(&output_path)?; + encoded_list_file.write_all(&all_lists.first().unwrap())?; + println!("{output_path}"); + } else { + // Otherwise write each to its own file with a numeric suffix + for (i, ech_config_lists) in all_lists.iter().enumerate() { + let mut encoded_list_file = File::create(format!("{output_path}.{}", i + 1))?; + encoded_list_file.write_all(&ech_config_lists)?; + } + // And print a comma separated list of the file paths. + let paths = (1..=all_lists.len()) + .map(|i| format!("{}.{}", output_path, i)) + .collect::>() + .join(","); + println!("{paths}") + } Ok(()) } -async fn lookup_ech(resolver: &TokioResolver, domain: &str) -> EchConfigListBytes<'static> { - resolver - .lookup(domain, RecordType::HTTPS) - .await - .expect("failed to lookup HTTPS record type") - .record_iter() - .find_map(|r| match r.data() { - RData::HTTPS(svcb) => svcb.svc_params().iter().find_map(|sp| match sp { - (SvcParamKey::EchConfigList, SvcParamValue::EchConfigList(e)) => Some(e.clone().0), - _ => None, - }), +/// Collect up all `EchConfigListBytes` found in the HTTPS record(s) for a given domain name. +/// +/// Assumes the port will be 443. For a more complete example see the Rustls' ech-client.rs +/// example's `lookup_ech_configs` function. +/// +/// The domain name should be the **inner** name used for Encrypted Client Hello (ECH). The +/// lookup is done using DNS-over-HTTPS to protect that inner name from being disclosed in +/// plaintext ahead of the TLS handshake that negotiates ECH for the inner name. +/// +/// Returns an empty vec if no HTTPS records with ECH configs are found. +async fn lookup_ech_configs( + resolver: &TokioResolver, + domain: &str, +) -> Result>, ResolveError> { + let lookup = resolver.lookup(domain, RecordType::HTTPS).await?; + + let mut ech_config_lists = Vec::new(); + for r in lookup.record_iter() { + let RData::HTTPS(svcb) = r.data() else { + continue; + }; + + ech_config_lists.extend(svcb.svc_params().iter().find_map(|sp| match sp { + (SvcParamKey::EchConfigList, SvcParamValue::EchConfigList(e)) => { + Some(EchConfigListBytes::from(e.clone().0)) + } _ => None, - }) - .expect("missing expected HTTPS SvcParam EchConfig record") - .into() + })) + } + + Ok(ech_config_lists) } From 280762ad233981b59e09a8115d69841fbcfff678 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 16 Dec 2024 10:30:58 -0500 Subject: [PATCH 2/2] clarify rustls_client_config_builder_enable_ech arg lifetime --- src/client.rs | 3 ++- src/rustls.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client.rs b/src/client.rs index 64b94525..6a38d0bb 100644 --- a/src/client.rs +++ b/src/client.rs @@ -385,7 +385,8 @@ impl rustls_client_config_builder { /// /// The provided `ech_config_list_bytes` and `rustls_hpke` must not be NULL or an /// error will be returned. The caller maintains ownership of the ECH config list TLS bytes - /// and `rustls_hpke` instance. + /// and `rustls_hpke` instance. This function does not retain any reference to + /// `ech_config_list_bytes`. /// /// A `RUSTLS_RESULT_BUILDER_INCOMPATIBLE_TLS_VERSIONS` error is returned if the builder's /// TLS versions have been customized via `rustls_client_config_builder_new_custom()` diff --git a/src/rustls.h b/src/rustls.h index fba30a1d..1432df51 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -1768,7 +1768,8 @@ rustls_result rustls_client_config_builder_set_key_log(struct rustls_client_conf * * The provided `ech_config_list_bytes` and `rustls_hpke` must not be NULL or an * error will be returned. The caller maintains ownership of the ECH config list TLS bytes - * and `rustls_hpke` instance. + * and `rustls_hpke` instance. This function does not retain any reference to + * `ech_config_list_bytes`. * * A `RUSTLS_RESULT_BUILDER_INCOMPATIBLE_TLS_VERSIONS` error is returned if the builder's * TLS versions have been customized via `rustls_client_config_builder_new_custom()`