From e760777a9cad5f83f716aaeceac3c11440497f71 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 16 Dec 2024 09:28:03 -0500 Subject: [PATCH] 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 `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 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 | 94 +++++++++++++++++++++++++++++++++++----------- tests/client.h | 2 +- tests/common.h | 6 +++ tests/ech_fetch.rs | 69 +++++++++++++++++++++++++--------- 4 files changed, 130 insertions(+), 41 deletions(-) diff --git a/tests/client.c b/tests/client.c index 589259ed..56407811 100644 --- a/tests/client.c +++ b/tests/client.c @@ -158,8 +158,8 @@ options_from_env(demo_client_options *opts) // Consider ECH options. const char *ech_grease = getenv("ECH_GREASE"); - const char *ech_config_list = getenv("ECH_CONFIG_LIST"); - if(ech_grease && ech_config_list) { + const char *ech_config_lists = getenv("ECH_CONFIG_LIST"); + if(ech_grease && ech_config_lists) { LOG_SIMPLE( "must set at most one of ECH_GREASE or ECH_CONFIG_LIST env vars"); return 1; @@ -168,9 +168,9 @@ options_from_env(demo_client_options *opts) LOG_SIMPLE("using ECH grease"); opts->use_ech_grease = true; } - else if(ech_config_list) { - LOG("using ECH config list '%s'", ech_config_list); - opts->use_ech_config_list_file = ech_config_list; + else if(ech_config_lists) { + LOG("using ECH config lists '%s'", ech_config_lists); + opts->use_ech_config_list_files = ech_config_lists; } // Consider SSLKEYLOGFILE options. @@ -304,30 +304,80 @@ new_tls_config(const demo_client_options *opts) goto cleanup; } } - else if(opts->use_ech_config_list_file) { + else if(opts->use_ech_config_list_files) { const rustls_hpke *hpke = rustls_supported_hpke(); if(hpke == NULL) { LOG_SIMPLE("client: no HPKE suites for ECH available"); goto cleanup; } - char ech_config_list_buf[10000]; - size_t ech_config_list_len; - const demo_result read_result = read_file(opts->use_ech_config_list_file, - ech_config_list_buf, - sizeof(ech_config_list_buf), - &ech_config_list_len); - if(read_result != DEMO_OK) { - LOG("client: failed to read ECH config list file: '%s'", - getenv("RUSTLS_ECH_CONFIG_LIST")); + + // Duplicate the config lists var value - calling STRTOK_R will modify the + // string to add null terminators between tokens. + char *ech_config_list_copy = strdup(opts->use_ech_config_list_files); + if(!ech_config_list_copy) { + LOG_SIMPLE("failed to allocate memory for ECH config list"); goto cleanup; } - const rustls_result rr = - rustls_client_config_builder_enable_ech(config_builder, - (uint8_t *)ech_config_list_buf, - ech_config_list_len, - hpke); - if(rr != RUSTLS_RESULT_OK) { - print_error("failed to configure ECH", rr); + + 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. + const rustls_result rr = + 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(rr == 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); + } + + // 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; } } diff --git a/tests/client.h b/tests/client.h index 39dd867b..259b4dfe 100644 --- a/tests/client.h +++ b/tests/client.h @@ -21,7 +21,7 @@ typedef struct demo_client_options // Optional encrypted client hello (ECH) settings. Only one should be set. bool use_ech_grease; - const char *use_ech_config_list_file; + const char *use_ech_config_list_files; // Optional SSL keylog support. At most one should be set. const char *use_ssl_keylog_file; diff --git a/tests/common.h b/tests/common.h index 0db4d6aa..8707a72e 100644 --- a/tests/common.h +++ b/tests/common.h @@ -22,6 +22,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 + typedef 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) }