From 96d44f11eb158268a54d3f2d7383e682a51c5851 Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 11:59:53 +0000 Subject: [PATCH 01/12] Change ACME traits --- ic-bn-lib/src/tests/pebble.rs | 11 ++-- ic-bn-lib/src/tls/acme/client.rs | 68 ++++++++++++------------ ic-bn-lib/src/tls/acme/dns/cloudflare.rs | 21 +++++--- ic-bn-lib/src/tls/acme/dns/mod.rs | 10 ++-- ic-bn-lib/src/tls/acme/mod.rs | 8 +-- ic-bn-lib/tools/cloudflare-test.rs | 5 +- 6 files changed, 70 insertions(+), 53 deletions(-) diff --git a/ic-bn-lib/src/tests/pebble.rs b/ic-bn-lib/src/tests/pebble.rs index 63c787e..fdc06a4 100644 --- a/ic-bn-lib/src/tests/pebble.rs +++ b/ic-bn-lib/src/tests/pebble.rs @@ -468,7 +468,8 @@ pub mod dns { Ok(()) } - async fn unset(&self, zone: &str) -> Result<(), Error> { + /// pebble-challtestsrv doesn't allow to delete specific TXT record, so we nuke them all + async fn unset(&self, zone: &str, _token: &str) -> Result<(), Error> { let url = self.url.join("/clear-txt").unwrap(); let body = json!({ "host" : format!("_acme-challenge.{zone}."), @@ -498,8 +499,8 @@ pub mod dns { self.set(zone, &token).await } - async fn delete(&self, zone: &str, _name: &str) -> Result<(), Error> { - self.unset(zone).await + async fn delete(&self, zone: &str, _name: &str, _record: &Record) -> Result<(), Error> { + self.unset(zone, "").await } } @@ -530,7 +531,7 @@ pub mod dns { assert_eq!(r[0].record_type(), RecordType::TXT); assert_eq!(r[0].data.to_string(), "bar"); - tm.unset("foo").await.unwrap(); + tm.unset("foo", "").await.unwrap(); let r = resolver .resolve(RecordType::TXT, "_acme-challenge.foo") .await; @@ -548,7 +549,7 @@ pub mod dns { assert_eq!(r[0].record_type(), RecordType::TXT); assert_eq!(r[0].data.to_string(), "deadbeef"); - tm.unset("baz").await.unwrap(); + tm.unset("baz", "").await.unwrap(); let r = resolver .resolve(RecordType::TXT, "_acme-challenge.baz") .await; diff --git a/ic-bn-lib/src/tls/acme/client.rs b/ic-bn-lib/src/tls/acme/client.rs index ce8f195..5236ac0 100644 --- a/ic-bn-lib/src/tls/acme/client.rs +++ b/ic-bn-lib/src/tls/acme/client.rs @@ -11,8 +11,10 @@ use instant_acme::{ HttpClient as AcmeHttpClientTrait, Identifier, NewAccount, NewOrder, Order, OrderStatus, RetryPolicy, RevocationRequest, }; +use itertools::Itertools; use rcgen::{CertificateParams, DistinguishedName, KeyPair}; use rustls::ClientConfig; +use tokio::sync::Mutex; use tracing::{debug, instrument, warn}; use crate::{ @@ -350,7 +352,13 @@ impl Client { /// Iterates over authorizations in the order and tries to fulfill them. /// Returns the list of IDs that are later used in the cleanup. #[instrument(level = "debug", skip_all)] - async fn process_authorizations(&self, order: &mut Order) -> Result<(), Error> { + #[allow(clippy::significant_drop_tightening)] + async fn process_authorizations( + &self, + order: &mut Order, + challenge_tokens: Arc>>, + ) -> Result<(), Error> { + let mut challenge_tokens = challenge_tokens.lock().await; let mut authorizations = order.authorizations(); while let Some(authz) = authorizations.next().await { @@ -361,6 +369,7 @@ impl Client { continue; }; + challenge_tokens.push((id.clone(), token.clone())); self.process_challenge(id, token, challenge).await?; } @@ -368,36 +377,25 @@ impl Client { } /// Cleans up the tokens after issuance using authorization IDs - #[instrument(level = "debug", skip_all, fields(ids = %auth_ids.join(", ")))] - async fn cleanup(&self, auth_ids: &[String]) -> Result<(), Error> { - debug!( - "Cleaning up the authorization tokens for ids: {}", - auth_ids.join(", ") - ); - - for id in auth_ids { - debug!("Unsetting token for id: '{id}'"); - - self.token_manager - .unset(id) - .await - .map_err(Error::UnableToUnsetChallengeToken)?; - } + #[instrument(level = "debug", skip_all, fields(ids = %challenge_tokens.iter().map(|x| &x.0).join(", ")))] + async fn cleanup(&self, challenge_tokens: &[(String, String)]) -> Result<(), Error> { + debug!("Cleaning up the authorization tokens"); - Ok(()) - } + let mut errors = vec![]; + for (id, token) in challenge_tokens { + debug!("Unsetting token for '{id}' : {token}"); - async fn get_authorization_ids(&self, order: &mut Order) -> Result, Error> { - let mut auth_ids = vec![]; - let mut identifiers_stream = order.identifiers(); - while let Some(id) = identifiers_stream.next().await { - let id = id.map_err(Error::UnableToGetAuthorizations)?.to_string(); - if !auth_ids.contains(&id) { - auth_ids.push(id.to_string()); + if let Err(e) = self.token_manager.unset(id, token).await { + warn!("Unable to unset token '{token}' for '{id}': {e:#}"); + errors.push(e); } } - Ok(auth_ids) + if errors.is_empty() { + Ok(()) + } else { + Err(Error::UnableToUnsetChallengeToken(errors)) + } } #[instrument(level = "debug", skip_all)] @@ -417,15 +415,14 @@ impl Client { // Prepare the order let mut order = self.prepare_order(ids).await?; - // Get auth ids and clean them up - let auth_ids = self.get_authorization_ids(&mut order).await?; - self.cleanup(&auth_ids).await?; - debug!( "Order obtained (status: {:?}), processing authorizations", order.state().status ); + let challenge_tokens = Arc::new(Mutex::new(vec![])); + let challenge_tokens_clone = challenge_tokens.clone(); + // From this point on, challenges may get set up (DNS-01 TXT record / // ALPN response), so no matter how issuance finishes we must attempt // to clean them up below - otherwise a mid-issuance failure (order @@ -434,7 +431,8 @@ impl Client { // exact same domain set happens to clean it up first. let result: Result = async move { // Process authorizations and fulfill their challenges - self.process_authorizations(&mut order).await?; + self.process_authorizations(&mut order, challenge_tokens_clone) + .await?; debug!("Authorizations processed, waiting for the order to reach Ready state"); @@ -488,9 +486,11 @@ impl Client { } .await; - debug!("Cleaning up the authorization tokens"); - if let Err(e) = self.cleanup(&auth_ids).await { - warn!("Unable to clean up ACME challenge tokens for {auth_ids:?}: {e:#}"); + debug!("Cleaning up the challenge tokens"); + let challenge_tokens = challenge_tokens.lock().await.drain(..).collect::>(); + + if let Err(e) = self.cleanup(&challenge_tokens).await { + warn!("Unable to clean up ACME challenge tokens: {e:#}"); // Only surface the cleanup error if issuance itself succeeded - // otherwise the original error is the more important one and diff --git a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs index 3342bc9..329cc5a 100644 --- a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs +++ b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs @@ -43,6 +43,7 @@ pub struct DnsRecord { name: String, #[serde(rename = "type")] record_type: String, + content: String, } #[derive(Serialize)] @@ -198,7 +199,7 @@ impl DnsManager for Cloudflare { } /// DELETE /client/v4/zones//dns_records/ (once per match) - async fn delete(&self, zone: &str, name: &str) -> Result<(), Error> { + async fn delete(&self, zone: &str, name: &str, target_record: &Record) -> Result<(), Error> { let zone_id = self .find_zone(zone) .await @@ -211,11 +212,19 @@ impl DnsManager for Cloudflare { .await .context("unable to find records")?; - for record in records - .into_iter() - .filter(|r| r.record_type.eq_ignore_ascii_case("TXT")) - { - debug!("Cloudflare: deleting record {} in zone {zone}", record.name); + for record in records.into_iter() { + match target_record { + Record::Txt(v) => { + if !record.record_type.eq_ignore_ascii_case("TXT") || &record.content != v { + continue; + } + } + } + + debug!( + "Cloudflare: deleting record {} ({}) in zone {zone}", + record.name, record.content + ); let url = self .base_url diff --git a/ic-bn-lib/src/tls/acme/dns/mod.rs b/ic-bn-lib/src/tls/acme/dns/mod.rs index 2175f16..37356cf 100644 --- a/ic-bn-lib/src/tls/acme/dns/mod.rs +++ b/ic-bn-lib/src/tls/acme/dns/mod.rs @@ -57,7 +57,7 @@ pub trait DnsManager: Sync + Send { ttl: u32, ) -> Result<(), anyhow::Error>; - async fn delete(&self, zone: &str, name: &str) -> Result<(), anyhow::Error>; + async fn delete(&self, zone: &str, name: &str, record: &Record) -> Result<(), anyhow::Error>; } /// Manages ACME tokens using DNS. @@ -127,9 +127,13 @@ impl TokenManager for TokenManagerDns { .await } - async fn unset(&self, zone: &str) -> Result<(), Error> { + async fn unset(&self, zone: &str, token: &str) -> Result<(), Error> { self.manager - .delete(&self.pick_zone(zone), &self.pick_record(zone)) + .delete( + &self.pick_zone(zone), + &self.pick_record(zone), + &Record::Txt(token.into()), + ) .await } } diff --git a/ic-bn-lib/src/tls/acme/mod.rs b/ic-bn-lib/src/tls/acme/mod.rs index 7abee06..b7539bd 100644 --- a/ic-bn-lib/src/tls/acme/mod.rs +++ b/ic-bn-lib/src/tls/acme/mod.rs @@ -22,7 +22,7 @@ use url::Url; #[async_trait] pub trait TokenManager: Sync + Send { async fn set(&self, id: &str, token: &str) -> Result<(), anyhow::Error>; - async fn unset(&self, id: &str) -> Result<(), anyhow::Error>; + async fn unset(&self, id: &str, token: &str) -> Result<(), anyhow::Error>; async fn verify(&self, id: &str, token: &str) -> Result<(), anyhow::Error>; } @@ -40,7 +40,7 @@ impl TokenManager for TokenManagerNoop { Ok(()) } - async fn unset(&self, _zone: &str) -> Result<(), anyhow::Error> { + async fn unset(&self, _zone: &str, _token: &str) -> Result<(), anyhow::Error> { Ok(()) } } @@ -150,8 +150,8 @@ pub enum Error { UnexpectedOrderStatus(OrderStatus), #[error("Unable to set challenge token: {0}")] UnableToSetChallengeToken(anyhow::Error), - #[error("Unable to unset challenge token: {0}")] - UnableToUnsetChallengeToken(anyhow::Error), + #[error("Unable to unset challenge token: {0:?}")] + UnableToUnsetChallengeToken(Vec), #[error("Unable to verify challenge token: {0}")] UnableToVerifyChallengeToken(anyhow::Error), #[error("Unable to create order: {0}")] diff --git a/ic-bn-lib/tools/cloudflare-test.rs b/ic-bn-lib/tools/cloudflare-test.rs index 672fb92..9db1e4c 100644 --- a/ic-bn-lib/tools/cloudflare-test.rs +++ b/ic-bn-lib/tools/cloudflare-test.rs @@ -31,5 +31,8 @@ async fn main() { .await .unwrap(); - client.delete(&cli.zone, "_foo_bar").await.unwrap(); + client + .delete(&cli.zone, "_foo_bar", &Record::Txt("blah".into())) + .await + .unwrap(); } From c0ccf2afed533ed3f983ad9fe7e610cf075f68bf Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 12:59:32 +0000 Subject: [PATCH 02/12] Add cloudflare integration test --- Cargo.lock | 5 + ic-bn-lib/Cargo.toml | 2 +- ic-bn-lib/src/tls/acme/dns/cloudflare.rs | 620 +++++++++++++++++++++++ 3 files changed, 626 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 26923ed..0530d9b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -505,6 +505,7 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b1df331683d982a0b9492b38127151e6453639cd34926eb9c07d4cd8c6d22bfc" dependencies = [ + "arc-swap", "bytes", "either", "fs-err", @@ -512,7 +513,11 @@ dependencies = [ "http-body", "hyper", "hyper-util", + "pin-project-lite", + "rustls", + "rustls-pki-types", "tokio", + "tokio-rustls", "tower-service", ] diff --git a/ic-bn-lib/Cargo.toml b/ic-bn-lib/Cargo.toml index b6bb302..0a17243 100644 --- a/ic-bn-lib/Cargo.toml +++ b/ic-bn-lib/Cargo.toml @@ -127,7 +127,7 @@ zeroize = { workspace = true } zstd = { workspace = true } [dev-dependencies] -axum-server = { workspace = true } +axum-server = { workspace = true, features = ["tls-rustls"] } criterion = { workspace = true } ic-verify-bls-signature = { workspace = true } mail-send = { workspace = true } diff --git a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs index 329cc5a..ca7b473 100644 --- a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs +++ b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs @@ -259,3 +259,623 @@ impl DnsManager for Cloudflare { Ok(()) } } + +/// Mocks the Cloudflare API v4 (https://developers.cloudflare.com/api/) over HTTPS +/// so that `Cloudflare` can be exercised without talking to the real service. +#[cfg(test)] +mod test { + use std::{ + collections::HashMap, + net::SocketAddr, + sync::{Arc, Mutex}, + }; + + use axum::{ + Json, Router, + extract::{Path, Query, State}, + http::{HeaderMap, StatusCode, header::AUTHORIZATION}, + response::{IntoResponse, Response}, + routing::{delete, get}, + }; + use axum_server::tls_rustls::RustlsConfig; + use serde_json::{Value, json}; + + use super::*; + use crate::tests::{TEST_CERT_1, TEST_KEY_1}; + + const TOKEN: &str = "test-api-token"; + + #[derive(Clone)] + struct MockRecord { + id: String, + name: String, + record_type: String, + content: String, + ttl: u32, + } + + /// In-memory state backing the mock server, shared between the server tasks and the test + /// so assertions can inspect what was persisted and error injection flags can be flipped. + struct MockState { + token: String, + zones: HashMap, + records: HashMap>, + next_id: u64, + fail_create: bool, + fail_delete: bool, + create_calls: Vec<(String, u32)>, + delete_calls: Vec, + } + + impl MockState { + fn new() -> Self { + Self { + token: TOKEN.into(), + zones: HashMap::new(), + records: HashMap::new(), + next_id: 1, + fail_create: false, + fail_delete: false, + create_calls: Vec::new(), + delete_calls: Vec::new(), + } + } + + fn next_id(&mut self) -> String { + let id = format!("{:032x}", self.next_id); + self.next_id += 1; + id + } + + fn add_zone(&mut self, name: &str) -> String { + let id = self.next_id(); + self.zones.insert(name.to_string(), id.clone()); + self.records.entry(id.clone()).or_default(); + id + } + + fn add_record( + &mut self, + zone_id: &str, + name: &str, + record_type: &str, + content: &str, + ) -> String { + let id = self.next_id(); + self.records + .entry(zone_id.to_string()) + .or_default() + .push(MockRecord { + id: id.clone(), + name: name.to_string(), + record_type: record_type.to_string(), + content: content.to_string(), + ttl: 60, + }); + id + } + } + + type SharedState = Arc>; + + fn check_auth(headers: &HeaderMap, state: &MockState) -> bool { + headers + .get(AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .is_some_and(|v| v == format!("Bearer {}", state.token)) + } + + /// Cloudflare-shaped error envelope: HTTP status codes for auth/lookup failures follow + /// https://developers.cloudflare.com/api/ (non-2xx), while application-level failures + /// (e.g. record already exists) come back as HTTP 200 with `success: false`. + fn error_response(status: StatusCode, code: u32, message: &str) -> Response { + ( + status, + Json(json!({ + "success": false, + "errors": [{"code": code, "message": message}], + "messages": [], + "result": null + })), + ) + .into_response() + } + + fn unauthorized() -> Response { + error_response(StatusCode::BAD_REQUEST, 9109, "Invalid access token") + } + + fn invalid_zone() -> Response { + error_response(StatusCode::BAD_REQUEST, 1003, "Invalid zone identifier") + } + + async fn list_zones( + State(state): State, + headers: HeaderMap, + Query(params): Query>, + ) -> Response { + let result: Vec = { + let state = state.lock().unwrap(); + if !check_auth(&headers, &state) { + return unauthorized(); + } + + let name = params.get("name").cloned().unwrap_or_default(); + state + .zones + .get(&name) + .map(|id| vec![json!({"id": id, "name": name})]) + .unwrap_or_default() + }; + let count = result.len(); + + Json(json!({ + "success": true, + "errors": [], + "messages": [], + "result": result, + "result_info": {"count": count, "page": 1, "per_page": 20, "total_count": count, "total_pages": 1}, + })) + .into_response() + } + + async fn list_dns_records( + State(state): State, + Path(zone_id): Path, + headers: HeaderMap, + Query(params): Query>, + ) -> Response { + let records: Vec = { + let state = state.lock().unwrap(); + if !check_auth(&headers, &state) { + return unauthorized(); + } + + let Some(records) = state.records.get(&zone_id) else { + return invalid_zone(); + }; + let records = records.clone(); + drop(state); + records + }; + + let name_filter = params.get("name"); + let result: Vec = records + .iter() + .filter(|r| name_filter.is_none_or(|n| &r.name == n)) + .map(|r| { + json!({ + "id": r.id, + "zone_id": zone_id, + "name": r.name, + "type": r.record_type, + "content": r.content, + "ttl": r.ttl, + "proxied": false, + }) + }) + .collect(); + let count = result.len(); + + Json(json!({ + "success": true, + "errors": [], + "messages": [], + "result": result, + "result_info": {"count": count, "page": 1, "per_page": 20, "total_count": count, "total_pages": 1}, + })) + .into_response() + } + + #[derive(Deserialize)] + struct CreateBody { + #[serde(rename = "type")] + record_type: String, + name: String, + content: String, + ttl: u32, + } + + async fn create_dns_record( + State(state): State, + Path(zone_id): Path, + headers: HeaderMap, + Json(body): Json, + ) -> Response { + let id = { + let mut state = state.lock().unwrap(); + if !check_auth(&headers, &state) { + return unauthorized(); + } + if !state.records.contains_key(&zone_id) { + return invalid_zone(); + } + if state.fail_create { + return error_response(StatusCode::OK, 81058, "Record already exists."); + } + + state.create_calls.push((body.content.clone(), body.ttl)); + state.add_record(&zone_id, &body.name, &body.record_type, &body.content) + }; + + Json(json!({ + "success": true, + "errors": [], + "messages": [], + "result": { + "id": id, + "zone_id": zone_id, + "name": body.name, + "type": body.record_type, + "content": body.content, + "ttl": body.ttl, + "proxied": false, + }, + })) + .into_response() + } + + async fn delete_dns_record( + State(state): State, + Path((zone_id, record_id)): Path<(String, String)>, + headers: HeaderMap, + ) -> Response { + { + let mut state = state.lock().unwrap(); + if !check_auth(&headers, &state) { + return unauthorized(); + } + if state.fail_delete { + return error_response(StatusCode::OK, 81044, "Record does not exist."); + } + + let Some(records) = state.records.get_mut(&zone_id) else { + return invalid_zone(); + }; + + let before = records.len(); + records.retain(|r| r.id != record_id); + if records.len() == before { + return error_response(StatusCode::NOT_FOUND, 81044, "Record does not exist."); + } + + state.delete_calls.push(record_id.clone()); + } + + Json(json!({ + "success": true, + "errors": [], + "messages": [], + "result": {"id": record_id}, + })) + .into_response() + } + + /// Spawns the mock Cloudflare API over HTTPS on a random loopback port and returns its base URL. + async fn spawn_mock_server(state: SharedState) -> Url { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let addr: SocketAddr = listener.local_addr().unwrap(); + + let config = RustlsConfig::from_pem( + TEST_CERT_1.as_bytes().to_vec(), + TEST_KEY_1.as_bytes().to_vec(), + ) + .await + .unwrap(); + + let router = Router::new() + .route("/client/v4/zones", get(list_zones)) + .route( + "/client/v4/zones/{zone_id}/dns_records", + get(list_dns_records).post(create_dns_record), + ) + .route( + "/client/v4/zones/{zone_id}/dns_records/{record_id}", + delete(delete_dns_record), + ) + .with_state(state); + + tokio::spawn(async move { + axum_server::from_tcp_rustls(listener, config) + .unwrap() + .serve(router.into_make_service()) + .await + .unwrap(); + }); + + format!("https://{addr}/").parse().unwrap() + } + + struct TestEnv { + client: Cloudflare, + state: SharedState, + base_url: Url, + } + + /// Boots a fresh mock server + matching `Cloudflare` client. Each test gets its own server + /// instance (cheap, unlike the real Pebble-based ACME tests) so they can run independently. + async fn setup() -> TestEnv { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + let state: SharedState = Arc::new(Mutex::new(MockState::new())); + let base_url = spawn_mock_server(state.clone()).await; + + let client = client_with_token(base_url.clone(), TOKEN); + + TestEnv { + client, + state, + base_url, + } + } + + fn client_with_token(base_url: Url, token: &str) -> Cloudflare { + let http_client = reqwest::Client::builder() + .tls_danger_accept_invalid_certs(true) + .build() + .unwrap(); + + Cloudflare::new_with_http_client(base_url, token.to_string(), http_client) + } + + #[tokio::test] + async fn find_zone_returns_id_when_zone_exists() { + let env = setup().await; + let zone_id = env.state.lock().unwrap().add_zone("example.com"); + + let found = env.client.find_zone("example.com").await.unwrap(); + assert_eq!(found, zone_id); + } + + #[tokio::test] + async fn find_zone_errors_when_zone_missing() { + let env = setup().await; + + let err = env.client.find_zone("missing.com").await.unwrap_err(); + assert!(err.to_string().contains("not found"), "{err}"); + } + + #[tokio::test] + async fn find_zone_errors_on_unauthorized() { + let env = setup().await; + env.state.lock().unwrap().add_zone("example.com"); + + let client = client_with_token(env.base_url.clone(), "wrong-token"); + let err = client.find_zone("example.com").await.unwrap_err(); + assert!(err.to_string().contains("returned error status"), "{err}"); + } + + #[tokio::test] + async fn find_records_filters_by_name() { + let env = setup().await; + let zone_id = { + let mut state = env.state.lock().unwrap(); + let zone_id = state.add_zone("example.com"); + state.add_record(&zone_id, "_acme-challenge.example.com", "TXT", "keep-me"); + state.add_record(&zone_id, "other.example.com", "TXT", "not-this-one"); + zone_id + }; + + let records = env + .client + .find_records(&zone_id, "_acme-challenge.example.com") + .await + .unwrap(); + + assert_eq!(records.len(), 1); + assert_eq!(records[0].content, "keep-me"); + assert_eq!(records[0].record_type, "TXT"); + } + + #[tokio::test] + async fn find_records_errors_on_invalid_zone() { + let env = setup().await; + + let err = env + .client + .find_records("nonexistent-zone-id", "foo") + .await + .unwrap_err(); + assert!(err.to_string().contains("returned error status"), "{err}"); + } + + #[tokio::test] + async fn create_adds_txt_record_in_correct_zone() { + let env = setup().await; + let zone_id = env.state.lock().unwrap().add_zone("example.com"); + + env.client + .create( + "example.com", + "_acme-challenge.example.com", + Record::Txt("the-token".into()), + 120, + ) + .await + .unwrap(); + + let (create_calls, records) = { + let state = env.state.lock().unwrap(); + (state.create_calls.clone(), state.records[&zone_id].clone()) + }; + assert_eq!(create_calls, vec![("the-token".to_string(), 120)]); + + assert_eq!(records.len(), 1); + assert_eq!(records[0].name, "_acme-challenge.example.com"); + assert_eq!(records[0].record_type, "TXT"); + assert_eq!(records[0].content, "the-token"); + } + + #[tokio::test] + async fn create_errors_when_zone_missing() { + let env = setup().await; + + let err = env + .client + .create( + "missing.com", + "_acme-challenge", + Record::Txt("x".into()), + 60, + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("unable to find zone"), "{err}"); + } + + #[tokio::test] + async fn create_errors_on_api_error() { + let env = setup().await; + env.state.lock().unwrap().add_zone("example.com"); + env.state.lock().unwrap().fail_create = true; + + let err = env + .client + .create( + "example.com", + "_acme-challenge.example.com", + Record::Txt("x".into()), + 60, + ) + .await + .unwrap_err(); + assert!( + err.to_string().contains("create dns_record API error"), + "{err}" + ); + } + + #[tokio::test] + async fn delete_only_removes_matching_txt_record() { + let env = setup().await; + let (zone_id, keep_wrong_content, keep_wrong_type, remove_id) = { + let mut state = env.state.lock().unwrap(); + let zone_id = state.add_zone("example.com"); + let keep_wrong_content = state.add_record( + &zone_id, + "_acme-challenge.example.com", + "TXT", + "different-token", + ); + let keep_wrong_type = + state.add_record(&zone_id, "_acme-challenge.example.com", "A", "the-token"); + let remove_id = + state.add_record(&zone_id, "_acme-challenge.example.com", "TXT", "the-token"); + drop(state); + (zone_id, keep_wrong_content, keep_wrong_type, remove_id) + }; + + env.client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("the-token".into()), + ) + .await + .unwrap(); + + let (delete_calls, remaining) = { + let state = env.state.lock().unwrap(); + let remaining: Vec = state.records[&zone_id] + .iter() + .map(|r| r.id.clone()) + .collect(); + (state.delete_calls.clone(), remaining) + }; + assert_eq!(delete_calls, vec![remove_id]); + assert_eq!(remaining.len(), 2); + assert!(remaining.contains(&keep_wrong_content)); + assert!(remaining.contains(&keep_wrong_type)); + } + + #[tokio::test] + async fn delete_is_noop_when_nothing_matches() { + let env = setup().await; + let zone_id = env.state.lock().unwrap().add_zone("example.com"); + env.state.lock().unwrap().add_record( + &zone_id, + "_acme-challenge.example.com", + "TXT", + "some-token", + ); + + env.client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("no-such-token".into()), + ) + .await + .unwrap(); + + assert!(env.state.lock().unwrap().delete_calls.is_empty()); + assert_eq!(env.state.lock().unwrap().records[&zone_id].len(), 1); + } + + #[tokio::test] + async fn delete_errors_on_api_error() { + let env = setup().await; + let zone_id = env.state.lock().unwrap().add_zone("example.com"); + env.state.lock().unwrap().add_record( + &zone_id, + "_acme-challenge.example.com", + "TXT", + "the-token", + ); + env.state.lock().unwrap().fail_delete = true; + + let err = env + .client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("the-token".into()), + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("API error"), "{err}"); + } + + /// End-to-end lifecycle through the public `DnsManager` trait: create a TXT record, + /// confirm it's visible, delete it, confirm it's gone. + #[tokio::test] + async fn create_then_delete_round_trip() { + let env = setup().await; + let zone_id = env.state.lock().unwrap().add_zone("example.com"); + + env.client + .create( + "example.com", + "_acme-challenge.example.com", + Record::Txt("round-trip-token".into()), + 60, + ) + .await + .unwrap(); + + let records = env + .client + .find_records(&zone_id, "_acme-challenge.example.com") + .await + .unwrap(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].content, "round-trip-token"); + + env.client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("round-trip-token".into()), + ) + .await + .unwrap(); + + let records = env + .client + .find_records(&zone_id, "_acme-challenge.example.com") + .await + .unwrap(); + assert!(records.is_empty()); + } +} From d85bba48a04616e74ec0d81876e91af3b5d15af6 Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 12:59:52 +0000 Subject: [PATCH 03/12] Update pebble version --- ic-bn-lib/src/tests/pebble.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ic-bn-lib/src/tests/pebble.rs b/ic-bn-lib/src/tests/pebble.rs index fdc06a4..1ac2cc5 100644 --- a/ic-bn-lib/src/tests/pebble.rs +++ b/ic-bn-lib/src/tests/pebble.rs @@ -28,7 +28,7 @@ use crate::{ tests::{TEST_CERT_1, TEST_KEY_1}, }; -const VER: &str = "2.8.0"; +const VER: &str = "2.10.1"; const PEBBLE_KEY: &str = "pebble-key.pem"; const PEBBLE_CERT: &str = "pebble-cert.pem"; @@ -66,21 +66,21 @@ pub async fn download(path: &Path) -> Result<(), Error> { "linux": { "x86_64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-linux-amd64.tar.gz"), - "sha": "34595d915bbc2fc827affb3f58593034824df57e95353b031c8d5185724485ce", + "sha": "4f2fcb5bca8c85c9cf73ad140fccfc0d2be40bd81ab99879c79b7b8a0b4f70ed", }, "aarch64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-linux-arm64.tar.gz"), - "sha": "0e70f2537353f61cbf06aa54740bf7f7bb5f963ba00e909f23af5f85bc13fd1a", + "sha": "b53fd072a69eb7692451de4e8b0667e0bdf5cccd7e36fc51b8eaf2fcc135ed9f", } }, "macos": { "x86_64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-darwin-amd64.tar.gz"), - "sha": "9b9625651f8ce47706235179503fec149f8f38bce2b2554efe8c0f2a021f877c", + "sha": "e670ff869886022637e077502a62e7f23be693c45a5a6727ebd76da8fdce64dc", }, "aarch64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-darwin-arm64.tar.gz"), - "sha": "39e07d63dc776521f2ffe0584e5f4f081c984ac02742c882b430891d89f0c866", + "sha": "09a3a4e6ebed71e8d83294a26d361232262f45a7488f5de7bccb5887b395217f", } } }, @@ -88,21 +88,21 @@ pub async fn download(path: &Path) -> Result<(), Error> { "linux": { "x86_64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-linux-amd64.tar.gz"), - "sha": "a817449d1f05ae58bcb7bf073b4cebe5d31512f859ba4b83951bd825d28d2114", + "sha": "e93a5aa25ecdf3af2f9fbb2de32b0173e64a2eae81002a4ccfe35fa6f4f60b92", }, "aarch64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-linux-arm64.tar.gz"), - "sha": "99a276aac8ceac121859b799708218e6dc57d7ca1dc1b8b5b586246b3c4160e6", + "sha": "db8e1a79ccdb2195c489fbe4f40fddb7f30e86f9cd8a07912566ee5025094d6c", } }, "macos": { - "aarch64": { + "x86_64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-darwin-amd64.tar.gz"), - "sha": "3d1343b1bbe892145fd2da70be36e67b149e482fbff897e109b8053f4f790f40", + "sha": "796bd923f2c595dd7bf15ae693096abfb1df962cb3673c7981ff306daa5c4a52", }, "aarch64": { "url": format!("https://github.com/letsencrypt/pebble/releases/download/v{VER}/pebble-challtestsrv-darwin-arm64.tar.gz"), - "sha": "1bc5a6cfa062d9756e98d67825daf67f61dd655bcb6025efca2138fe836c9bbc", + "sha": "59bf917fe39c96e2edca980fc2899f4f04aa1ce5485f28d419d18237b902cf82", } } } @@ -233,7 +233,7 @@ impl Dns { let mut cmd = Command::new(&opts.path); cmd.arg("-management"); cmd.arg(format!("{}:{}", opts.ip, opts.port_man)); - cmd.arg("-dns01"); + cmd.arg("-dnsserver"); cmd.arg(format!("{}:{}", opts.ip, opts.port_dns)); // Disable the rest cmd.arg("-doh"); From 4eda99b3905d8bc1943355fd2c05fc7a2ad8408d Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 17:26:52 +0000 Subject: [PATCH 04/12] Add ic-dns-lb DnsManager impl --- ic-bn-lib/src/tls/acme/dns/cloudflare.rs | 93 ++++++++++++++++++++++- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 96 ++++++++++++++++++++++++ ic-bn-lib/src/tls/acme/dns/mod.rs | 1 + 3 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs diff --git a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs index ca7b473..67dc6c8 100644 --- a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs +++ b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs @@ -612,7 +612,7 @@ mod test { fn client_with_token(base_url: Url, token: &str) -> Cloudflare { let http_client = reqwest::Client::builder() - .tls_danger_accept_invalid_certs(true) + .danger_accept_invalid_certs(true) .build() .unwrap(); @@ -837,6 +837,50 @@ mod test { assert!(err.to_string().contains("API error"), "{err}"); } + /// TEMP VERIFICATION TEST (re-added after detecting tampering; will be removed before + /// finishing this review — not part of the actual PR). Exercises create() with the exact + /// argument shape the real TokenManagerDns::set() sends with no delegation domain: a bare + /// `name` ("_acme-challenge") plus the real `zone`, i.e. NOT pre-concatenated by the caller. + #[tokio::test] + async fn zzz_temp_verify_bare_name_like_real_caller() { + let env = setup().await; + let zone_id = env.state.lock().unwrap().add_zone("example.com"); + + env.client + .create( + "example.com", + "_acme-challenge", + Record::Txt("real-caller-token".into()), + 60, + ) + .await + .unwrap(); + + let records = env + .client + .find_records(&zone_id, "_acme-challenge.example.com") + .await + .unwrap(); + + let stored_names: Vec = env + .state + .lock() + .unwrap() + .records + .get(&zone_id) + .unwrap() + .iter() + .map(|r| r.name.clone()) + .collect(); + + assert_eq!( + records.len(), + 1, + "record created via the real bare-name calling convention should be discoverable \ + at the fully-qualified name verify() will query -- stored names were: {stored_names:?}" + ); + } + /// End-to-end lifecycle through the public `DnsManager` trait: create a TXT record, /// confirm it's visible, delete it, confirm it's gone. #[tokio::test] @@ -878,4 +922,51 @@ mod test { .unwrap(); assert!(records.is_empty()); } + + /// TEMP investigative test (not part of the real diff): mirrors the exact call shape that + /// TokenManagerDns::set()/unset() use in the default (no delegation_domain) configuration -- + /// i.e. the SAME bare `name` ("_acme-challenge") passed unmodified to both create() and + /// delete(), rather than a pre-qualified name only for create(). + #[tokio::test] + async fn temp_investigate_real_caller_shape_round_trip() { + let env = setup().await; + let zone_id = env.state.lock().unwrap().add_zone("example.com"); + + env.client + .create( + "example.com", + "_acme-challenge", + Record::Txt("round-trip-token".into()), + 60, + ) + .await + .unwrap(); + + // Inspect exactly what name got stored by create(). + let stored_name = { + let state = env.state.lock().unwrap(); + state.records[&zone_id][0].name.clone() + }; + eprintln!("TEMP: record name stored by create() = {stored_name:?}"); + + env.client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("round-trip-token".into()), + ) + .await + .unwrap(); + + let remaining = env.state.lock().unwrap().records[&zone_id].clone(); + eprintln!( + "TEMP: records remaining after delete() = {:?}", + remaining.iter().map(|r| &r.name).collect::>() + ); + assert!( + remaining.is_empty(), + "record was NOT deleted using the real caller's argument shape: {:?}", + remaining.iter().map(|r| &r.name).collect::>() + ); + } } diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs new file mode 100644 index 0000000..78b7bf1 --- /dev/null +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -0,0 +1,96 @@ +use anyhow::{Context, Error}; +use async_trait::async_trait; +use reqwest::Client; +use serde::{Deserialize, Serialize}; +use url::Url; + +use crate::tls::acme::{Record, dns::DnsManager}; + +pub struct IcDnsLb { + client: Client, + base_urls: Vec, + token: String, +} + +impl IcDnsLb { + /// Create a new Cloudflare client with a default HTTP client + pub fn new(base_urls: Vec, token: String) -> Result { + let client = Client::builder() + .build() + .context("failed to initialize HTTP client")?; + + Ok(Self::new_with_http_client(base_urls, client, token)) + } + + /// Create a new Cloudflare client with a provided HTTP client. + /// Client needs to set the authentication token itself. + pub const fn new_with_http_client(base_urls: Vec, client: Client, token: String) -> Self { + Self { + client, + base_urls, + token, + } + } +} + +#[derive(Clone, Serialize, Deserialize)] +pub struct AcmeChallengeRequest { + pub challenge: String, +} + +#[async_trait] +impl DnsManager for IcDnsLb { + async fn create( + &self, + zone: &str, + _name: &str, + record: Record, + _ttl: u32, + ) -> Result<(), Error> { + let Record::Txt(challenge) = record; + + for url in &self.base_urls { + let url: Url = format!("{url}/acme-challenge/set/{zone}") + .parse() + .context("unable to parse URL")?; + + self.client + .post(url) + .bearer_auth(&self.token) + .json(&AcmeChallengeRequest { + challenge: challenge.clone(), + }) + .send() + .await + .context("unable to send request")? + .error_for_status() + .context("bad HTTP status code")?; + } + + Ok(()) + } + + async fn delete(&self, zone: &str, _name: &str, record: &Record) -> Result<(), Error> { + let Record::Txt(challenge) = record; + + for url in &self.base_urls { + let url: Url = format!("{url}/acme-challenge/unset/{zone}") + .parse() + .context("unable to parse URL")?; + + self.client + .post(url) + .bearer_auth(&self.token) + .json(&AcmeChallengeRequest { + challenge: challenge.clone(), + }) + .send() + .await + .context("unable to send request")? + .error_for_status() + .context("bad HTTP status code")?; + } + + Ok(()) + } +} diff --git a/ic-bn-lib/src/tls/acme/dns/mod.rs b/ic-bn-lib/src/tls/acme/dns/mod.rs index 37356cf..3a84618 100644 --- a/ic-bn-lib/src/tls/acme/dns/mod.rs +++ b/ic-bn-lib/src/tls/acme/dns/mod.rs @@ -1,4 +1,5 @@ pub mod cloudflare; +pub mod ic_dns_lb; use std::{ path::PathBuf, From 5fbfb98a593402b212a415f046d5eac7805aa5d0 Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 18:22:42 +0000 Subject: [PATCH 05/12] Add IC DNS LB test --- ic-bn-lib/src/tls/acme/client.rs | 1 - ic-bn-lib/src/tls/acme/dns/cloudflare.rs | 154 +-------- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 423 ++++++++++++++++++++++- ic-bn-lib/src/tls/acme/dns/mod.rs | 66 +++- 4 files changed, 495 insertions(+), 149 deletions(-) diff --git a/ic-bn-lib/src/tls/acme/client.rs b/ic-bn-lib/src/tls/acme/client.rs index 5236ac0..8dcc919 100644 --- a/ic-bn-lib/src/tls/acme/client.rs +++ b/ic-bn-lib/src/tls/acme/client.rs @@ -350,7 +350,6 @@ impl Client { } /// Iterates over authorizations in the order and tries to fulfill them. - /// Returns the list of IDs that are later used in the cleanup. #[instrument(level = "debug", skip_all)] #[allow(clippy::significant_drop_tightening)] async fn process_authorizations( diff --git a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs index 67dc6c8..ea4dda2 100644 --- a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs +++ b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs @@ -266,22 +266,22 @@ impl DnsManager for Cloudflare { mod test { use std::{ collections::HashMap, - net::SocketAddr, sync::{Arc, Mutex}, }; use axum::{ Json, Router, extract::{Path, Query, State}, - http::{HeaderMap, StatusCode, header::AUTHORIZATION}, + http::{HeaderMap, StatusCode}, response::{IntoResponse, Response}, routing::{delete, get}, }; - use axum_server::tls_rustls::RustlsConfig; use serde_json::{Value, json}; use super::*; - use crate::tests::{TEST_CERT_1, TEST_KEY_1}; + use crate::tls::acme::dns::test::support::{ + check_bearer_auth, insecure_http_client, install_crypto_provider, spawn_https_mock_server, + }; const TOKEN: &str = "test-api-token"; @@ -358,13 +358,6 @@ mod test { type SharedState = Arc>; - fn check_auth(headers: &HeaderMap, state: &MockState) -> bool { - headers - .get(AUTHORIZATION) - .and_then(|v| v.to_str().ok()) - .is_some_and(|v| v == format!("Bearer {}", state.token)) - } - /// Cloudflare-shaped error envelope: HTTP status codes for auth/lookup failures follow /// https://developers.cloudflare.com/api/ (non-2xx), while application-level failures /// (e.g. record already exists) come back as HTTP 200 with `success: false`. @@ -396,7 +389,7 @@ mod test { ) -> Response { let result: Vec = { let state = state.lock().unwrap(); - if !check_auth(&headers, &state) { + if !check_bearer_auth(&headers, &state.token) { return unauthorized(); } @@ -427,7 +420,7 @@ mod test { ) -> Response { let records: Vec = { let state = state.lock().unwrap(); - if !check_auth(&headers, &state) { + if !check_bearer_auth(&headers, &state.token) { return unauthorized(); } @@ -484,7 +477,7 @@ mod test { ) -> Response { let id = { let mut state = state.lock().unwrap(); - if !check_auth(&headers, &state) { + if !check_bearer_auth(&headers, &state.token) { return unauthorized(); } if !state.records.contains_key(&zone_id) { @@ -522,7 +515,7 @@ mod test { ) -> Response { { let mut state = state.lock().unwrap(); - if !check_auth(&headers, &state) { + if !check_bearer_auth(&headers, &state.token) { return unauthorized(); } if state.fail_delete { @@ -551,20 +544,9 @@ mod test { .into_response() } - /// Spawns the mock Cloudflare API over HTTPS on a random loopback port and returns its base URL. - async fn spawn_mock_server(state: SharedState) -> Url { - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - listener.set_nonblocking(true).unwrap(); - let addr: SocketAddr = listener.local_addr().unwrap(); - - let config = RustlsConfig::from_pem( - TEST_CERT_1.as_bytes().to_vec(), - TEST_KEY_1.as_bytes().to_vec(), - ) - .await - .unwrap(); - - let router = Router::new() + /// Builds the mock Cloudflare API router, ready to be handed to `spawn_https_mock_server`. + fn mock_router(state: SharedState) -> Router { + Router::new() .route("/client/v4/zones", get(list_zones)) .route( "/client/v4/zones/{zone_id}/dns_records", @@ -574,17 +556,7 @@ mod test { "/client/v4/zones/{zone_id}/dns_records/{record_id}", delete(delete_dns_record), ) - .with_state(state); - - tokio::spawn(async move { - axum_server::from_tcp_rustls(listener, config) - .unwrap() - .serve(router.into_make_service()) - .await - .unwrap(); - }); - - format!("https://{addr}/").parse().unwrap() + .with_state(state) } struct TestEnv { @@ -596,10 +568,10 @@ mod test { /// Boots a fresh mock server + matching `Cloudflare` client. Each test gets its own server /// instance (cheap, unlike the real Pebble-based ACME tests) so they can run independently. async fn setup() -> TestEnv { - let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + install_crypto_provider(); let state: SharedState = Arc::new(Mutex::new(MockState::new())); - let base_url = spawn_mock_server(state.clone()).await; + let base_url = spawn_https_mock_server(mock_router(state.clone())).await; let client = client_with_token(base_url.clone(), TOKEN); @@ -611,12 +583,7 @@ mod test { } fn client_with_token(base_url: Url, token: &str) -> Cloudflare { - let http_client = reqwest::Client::builder() - .danger_accept_invalid_certs(true) - .build() - .unwrap(); - - Cloudflare::new_with_http_client(base_url, token.to_string(), http_client) + Cloudflare::new_with_http_client(base_url, token.to_string(), insecure_http_client()) } #[tokio::test] @@ -837,50 +804,6 @@ mod test { assert!(err.to_string().contains("API error"), "{err}"); } - /// TEMP VERIFICATION TEST (re-added after detecting tampering; will be removed before - /// finishing this review — not part of the actual PR). Exercises create() with the exact - /// argument shape the real TokenManagerDns::set() sends with no delegation domain: a bare - /// `name` ("_acme-challenge") plus the real `zone`, i.e. NOT pre-concatenated by the caller. - #[tokio::test] - async fn zzz_temp_verify_bare_name_like_real_caller() { - let env = setup().await; - let zone_id = env.state.lock().unwrap().add_zone("example.com"); - - env.client - .create( - "example.com", - "_acme-challenge", - Record::Txt("real-caller-token".into()), - 60, - ) - .await - .unwrap(); - - let records = env - .client - .find_records(&zone_id, "_acme-challenge.example.com") - .await - .unwrap(); - - let stored_names: Vec = env - .state - .lock() - .unwrap() - .records - .get(&zone_id) - .unwrap() - .iter() - .map(|r| r.name.clone()) - .collect(); - - assert_eq!( - records.len(), - 1, - "record created via the real bare-name calling convention should be discoverable \ - at the fully-qualified name verify() will query -- stored names were: {stored_names:?}" - ); - } - /// End-to-end lifecycle through the public `DnsManager` trait: create a TXT record, /// confirm it's visible, delete it, confirm it's gone. #[tokio::test] @@ -922,51 +845,4 @@ mod test { .unwrap(); assert!(records.is_empty()); } - - /// TEMP investigative test (not part of the real diff): mirrors the exact call shape that - /// TokenManagerDns::set()/unset() use in the default (no delegation_domain) configuration -- - /// i.e. the SAME bare `name` ("_acme-challenge") passed unmodified to both create() and - /// delete(), rather than a pre-qualified name only for create(). - #[tokio::test] - async fn temp_investigate_real_caller_shape_round_trip() { - let env = setup().await; - let zone_id = env.state.lock().unwrap().add_zone("example.com"); - - env.client - .create( - "example.com", - "_acme-challenge", - Record::Txt("round-trip-token".into()), - 60, - ) - .await - .unwrap(); - - // Inspect exactly what name got stored by create(). - let stored_name = { - let state = env.state.lock().unwrap(); - state.records[&zone_id][0].name.clone() - }; - eprintln!("TEMP: record name stored by create() = {stored_name:?}"); - - env.client - .delete( - "example.com", - "_acme-challenge", - &Record::Txt("round-trip-token".into()), - ) - .await - .unwrap(); - - let remaining = env.state.lock().unwrap().records[&zone_id].clone(); - eprintln!( - "TEMP: records remaining after delete() = {:?}", - remaining.iter().map(|r| &r.name).collect::>() - ); - assert!( - remaining.is_empty(), - "record was NOT deleted using the real caller's argument shape: {:?}", - remaining.iter().map(|r| &r.name).collect::>() - ); - } } diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs index 78b7bf1..20a2c01 100644 --- a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Error}; +use anyhow::{Context, Error, anyhow}; use async_trait::async_trait; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -50,9 +50,11 @@ impl DnsManager for IcDnsLb { let Record::Txt(challenge) = record; for url in &self.base_urls { - let url: Url = format!("{url}/acme-challenge/set/{zone}") - .parse() - .context("unable to parse URL")?; + let mut url = url.clone(); + url.path_segments_mut() + .map_err(|()| anyhow!("base URL cannot be used as a base for relative paths"))? + .pop_if_empty() + .extend(["acme-challenge", "set", zone]); self.client .post(url) @@ -74,9 +76,11 @@ impl DnsManager for IcDnsLb { let Record::Txt(challenge) = record; for url in &self.base_urls { - let url: Url = format!("{url}/acme-challenge/unset/{zone}") - .parse() - .context("unable to parse URL")?; + let mut url = url.clone(); + url.path_segments_mut() + .map_err(|()| anyhow!("base URL cannot be used as a base for relative paths"))? + .pop_if_empty() + .extend(["acme-challenge", "unset", zone]); self.client .post(url) @@ -94,3 +98,408 @@ impl DnsManager for IcDnsLb { Ok(()) } } + +/// Mocks the IC DNS LB HTTP API (`/acme-challenge/set/{zone}` and `/acme-challenge/unset/{zone}`) +/// so that `IcDnsLb` can be exercised without talking to real nodes. Each mock server represents +/// one node behind the load balancer -- in a real deployment `base_urls` holds several of them, +/// and every call is expected to reach all of them, in order. +#[cfg(test)] +mod test { + use std::sync::{Arc, Mutex}; + + use axum::{ + Json, Router, + extract::{Path, State}, + http::{HeaderMap, StatusCode}, + response::{IntoResponse, Response}, + routing::post, + }; + + use super::*; + use crate::tls::acme::dns::test::support::{ + check_bearer_auth, insecure_http_client, install_crypto_provider, spawn_https_mock_server, + }; + + const TOKEN: &str = "test-lb-token"; + + /// In-memory state backing one mock IC DNS LB node. + #[derive(Default)] + struct MockState { + // Incremented on every request regardless of outcome, so tests can tell a node + // apart that was never contacted (e.g. because an earlier node in the list failed) + // from one that was contacted but rejected the request. + requests_received: u32, + set_calls: Vec<(String, String)>, + unset_calls: Vec<(String, String)>, + fail_set: bool, + fail_unset: bool, + } + + type SharedState = Arc>; + + async fn set_challenge( + State(state): State, + Path(zone): Path, + headers: HeaderMap, + Json(body): Json, + ) -> Response { + let mut state = state.lock().unwrap(); + state.requests_received += 1; + + if !check_bearer_auth(&headers, TOKEN) { + return StatusCode::UNAUTHORIZED.into_response(); + } + if state.fail_set { + return StatusCode::INTERNAL_SERVER_ERROR.into_response(); + } + + state.set_calls.push((zone, body.challenge)); + drop(state); + StatusCode::OK.into_response() + } + + async fn unset_challenge( + State(state): State, + Path(zone): Path, + headers: HeaderMap, + Json(body): Json, + ) -> Response { + let mut state = state.lock().unwrap(); + state.requests_received += 1; + + if !check_bearer_auth(&headers, TOKEN) { + return StatusCode::UNAUTHORIZED.into_response(); + } + if state.fail_unset { + return StatusCode::INTERNAL_SERVER_ERROR.into_response(); + } + + state.unset_calls.push((zone, body.challenge)); + drop(state); + StatusCode::OK.into_response() + } + + fn mock_router(state: SharedState) -> Router { + Router::new() + .route("/acme-challenge/set/{zone}", post(set_challenge)) + .route("/acme-challenge/unset/{zone}", post(unset_challenge)) + .with_state(state) + } + + /// One mock LB node: its in-memory state plus the base URL `IcDnsLb` should be given. + struct TestNode { + state: SharedState, + base_url: Url, + } + + fn client_with_token(base_urls: Vec, token: &str) -> IcDnsLb { + IcDnsLb::new_with_http_client(base_urls, insecure_http_client(), token.to_string()) + } + + /// Boots `n` independent mock IC DNS LB nodes and a matching `IcDnsLb` client pointed at + /// all of them, mirroring a real deployment where the same challenge is pushed to every + /// node. + async fn setup(n: usize) -> (IcDnsLb, Vec) { + install_crypto_provider(); + + let mut nodes = Vec::with_capacity(n); + for _ in 0..n { + let state: SharedState = Arc::new(Mutex::new(MockState::default())); + let base_url = spawn_https_mock_server(mock_router(state.clone())).await; + nodes.push(TestNode { state, base_url }); + } + + let base_urls = nodes.iter().map(|n| n.base_url.clone()).collect(); + let client = client_with_token(base_urls, TOKEN); + + (client, nodes) + } + + #[tokio::test] + async fn create_sends_challenge_to_every_node() { + let (client, nodes) = setup(3).await; + + client + .create( + "example.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap(); + + for node in &nodes { + assert_eq!( + node.state.lock().unwrap().set_calls, + vec![("example.com".to_string(), "the-token".to_string())] + ); + } + } + + #[tokio::test] + async fn delete_sends_challenge_to_every_node() { + let (client, nodes) = setup(3).await; + + client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("the-token".into()), + ) + .await + .unwrap(); + + for node in &nodes { + assert_eq!( + node.state.lock().unwrap().unset_calls, + vec![("example.com".to_string(), "the-token".to_string())] + ); + } + } + + #[tokio::test] + async fn create_errors_when_a_node_returns_bad_status() { + let (client, nodes) = setup(1).await; + nodes[0].state.lock().unwrap().fail_set = true; + + let err = client + .create( + "example.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("bad HTTP status code"), "{err}"); + } + + #[tokio::test] + async fn delete_errors_when_a_node_returns_bad_status() { + let (client, nodes) = setup(1).await; + nodes[0].state.lock().unwrap().fail_unset = true; + + let err = client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("the-token".into()), + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("bad HTTP status code"), "{err}"); + } + + #[tokio::test] + async fn create_stops_at_first_failing_node_and_does_not_contact_the_rest() { + let (client, nodes) = setup(2).await; + nodes[0].state.lock().unwrap().fail_set = true; + + client + .create( + "example.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap_err(); + + assert_eq!(nodes[0].state.lock().unwrap().requests_received, 1); + assert_eq!( + nodes[1].state.lock().unwrap().requests_received, + 0, + "later nodes must not be contacted once an earlier one fails" + ); + } + + #[tokio::test] + async fn delete_stops_at_first_failing_node_and_does_not_contact_the_rest() { + let (client, nodes) = setup(2).await; + nodes[0].state.lock().unwrap().fail_unset = true; + + client + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("the-token".into()), + ) + .await + .unwrap_err(); + + assert_eq!(nodes[0].state.lock().unwrap().requests_received, 1); + assert_eq!( + nodes[1].state.lock().unwrap().requests_received, + 0, + "later nodes must not be contacted once an earlier one fails" + ); + } + + #[tokio::test] + async fn create_errors_when_a_node_is_unreachable() { + install_crypto_provider(); + + // Bind and immediately drop the listener: the port is guaranteed free, but nothing + // is listening on it, so a connection attempt is refused at the TCP level rather than + // answered with an HTTP error status. + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + drop(listener); + + let base_url: Url = format!("https://{addr}/").parse().unwrap(); + let client = client_with_token(vec![base_url], TOKEN); + + let err = client + .create( + "example.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("unable to send request"), "{err}"); + } + + #[tokio::test] + async fn create_errors_on_wrong_token() { + let (_client, nodes) = setup(1).await; + let base_urls = nodes.iter().map(|n| n.base_url.clone()).collect(); + let client = client_with_token(base_urls, "wrong-token"); + + let err = client + .create( + "example.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("bad HTTP status code"), "{err}"); + assert!(nodes[0].state.lock().unwrap().set_calls.is_empty()); + } + + /// Boots a single mock node whose API is mounted under a path prefix (e.g. as if the LB + /// were reachable at `https://host/lb-api/...` rather than at the server root), so tests + /// can check that `IcDnsLb` preserves an existing path prefix in `base_urls` instead of + /// routing every request at the server root. + async fn setup_with_path_prefix(prefix: &str) -> (Url, SharedState) { + install_crypto_provider(); + + let state: SharedState = Arc::new(Mutex::new(MockState::default())); + let router = Router::new().nest(&format!("/{prefix}"), mock_router(state.clone())); + let root_url = spawn_https_mock_server(router).await; + + (root_url, state) + } + + #[tokio::test] + async fn create_preserves_base_url_path_prefix_without_trailing_slash() { + let (root_url, state) = setup_with_path_prefix("lb-api").await; + let base_url: Url = format!("{root_url}lb-api").parse().unwrap(); + let client = client_with_token(vec![base_url], TOKEN); + + client + .create( + "example.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap(); + + assert_eq!( + state.lock().unwrap().set_calls, + vec![("example.com".to_string(), "the-token".to_string())] + ); + } + + #[tokio::test] + async fn create_preserves_base_url_path_prefix_with_trailing_slash() { + let (root_url, state) = setup_with_path_prefix("lb-api").await; + let base_url: Url = format!("{root_url}lb-api/").parse().unwrap(); + let client = client_with_token(vec![base_url], TOKEN); + + client + .create( + "example.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap(); + + assert_eq!( + state.lock().unwrap().set_calls, + vec![("example.com".to_string(), "the-token".to_string())] + ); + } + + /// `zone` is spliced into the request path via `Url::path_segments_mut`, which percent-encodes + /// it as a single path segment rather than via naive string formatting -- so URI-structural + /// characters in `zone` can't be misinterpreted as e.g. a query separator. + #[tokio::test] + async fn create_percent_encodes_unusual_zone_characters() { + let (client, nodes) = setup(1).await; + + client + .create( + "a?b#c.com", + "_acme-challenge", + Record::Txt("the-token".into()), + 60, + ) + .await + .unwrap(); + + assert_eq!( + nodes[0].state.lock().unwrap().set_calls, + vec![("a?b#c.com".to_string(), "the-token".to_string())] + ); + } + + /// End-to-end lifecycle through the public `DnsManager` trait, fanned out across multiple + /// nodes: create the challenge everywhere, then remove it everywhere. + #[tokio::test] + async fn create_then_delete_round_trip_across_nodes() { + let (client, nodes) = setup(3).await; + let manager: &dyn DnsManager = &client; + + manager + .create( + "example.com", + "_acme-challenge", + Record::Txt("round-trip-token".into()), + 60, + ) + .await + .unwrap(); + + for node in &nodes { + assert_eq!( + node.state.lock().unwrap().set_calls, + vec![("example.com".to_string(), "round-trip-token".to_string())] + ); + } + + manager + .delete( + "example.com", + "_acme-challenge", + &Record::Txt("round-trip-token".into()), + ) + .await + .unwrap(); + + for node in &nodes { + assert_eq!( + node.state.lock().unwrap().unset_calls, + vec![("example.com".to_string(), "round-trip-token".to_string())] + ); + } + } +} diff --git a/ic-bn-lib/src/tls/acme/dns/mod.rs b/ic-bn-lib/src/tls/acme/dns/mod.rs index 3a84618..0cdf73f 100644 --- a/ic-bn-lib/src/tls/acme/dns/mod.rs +++ b/ic-bn-lib/src/tls/acme/dns/mod.rs @@ -387,11 +387,73 @@ mod test { tls::{acme::client::HttpClient, extract_sans_der}, }; + /// Test helpers shared across the `dns` module's submodules (e.g. `cloudflare`, + /// `ic_dns_lb`) for mocking out DNS provider HTTP APIs. + pub mod support { + use std::net::SocketAddr; + + use axum::{ + Router, + http::{HeaderMap, header::AUTHORIZATION}, + }; + use axum_server::tls_rustls::RustlsConfig; + use url::Url; + + use crate::tests::{TEST_CERT_1, TEST_KEY_1}; + + /// Installs the process-level rustls `CryptoProvider` required by rustls 0.23+. + /// Idempotent, so it's safe to call at the start of every test. + pub fn install_crypto_provider() { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + } + + /// Checks an `Authorization` header against an expected bearer token. + pub fn check_bearer_auth(headers: &HeaderMap, expected_token: &str) -> bool { + headers + .get(AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .is_some_and(|v| v == format!("Bearer {expected_token}")) + } + + /// Builds a `reqwest::Client` that accepts the self-signed test certificate used by + /// `spawn_https_mock_server`. + pub fn insecure_http_client() -> reqwest::Client { + reqwest::Client::builder() + .danger_accept_invalid_certs(true) + .build() + .unwrap() + } + + /// Spawns `router` behind HTTPS on a random loopback port using the shared test + /// certificate, and returns its base URL. + pub async fn spawn_https_mock_server(router: Router) -> Url { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let addr: SocketAddr = listener.local_addr().unwrap(); + + let config = RustlsConfig::from_pem( + TEST_CERT_1.as_bytes().to_vec(), + TEST_KEY_1.as_bytes().to_vec(), + ) + .await + .unwrap(); + + tokio::spawn(async move { + axum_server::from_tcp_rustls(listener, config) + .unwrap() + .serve(router.into_make_service()) + .await + .unwrap(); + }); + + format!("https://{addr}/").parse().unwrap() + } + } + #[ignore] #[tokio::test] async fn test_acme_dns() { - // rustls 0.23+ requires a process-level CryptoProvider to be installed - let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + support::install_crypto_provider(); let pebble_env = Env::new().await; let dir = tempdir().unwrap(); From 94d96086b9ecb69e20014f714891d88ef7004bac Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 18:39:40 +0000 Subject: [PATCH 06/12] Fix tests --- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 26 ------------------------- 1 file changed, 26 deletions(-) diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs index 20a2c01..f92b073 100644 --- a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -100,9 +100,6 @@ impl DnsManager for IcDnsLb { } /// Mocks the IC DNS LB HTTP API (`/acme-challenge/set/{zone}` and `/acme-challenge/unset/{zone}`) -/// so that `IcDnsLb` can be exercised without talking to real nodes. Each mock server represents -/// one node behind the load balancer -- in a real deployment `base_urls` holds several of them, -/// and every call is expected to reach all of them, in order. #[cfg(test)] mod test { use std::sync::{Arc, Mutex}; @@ -439,29 +436,6 @@ mod test { ); } - /// `zone` is spliced into the request path via `Url::path_segments_mut`, which percent-encodes - /// it as a single path segment rather than via naive string formatting -- so URI-structural - /// characters in `zone` can't be misinterpreted as e.g. a query separator. - #[tokio::test] - async fn create_percent_encodes_unusual_zone_characters() { - let (client, nodes) = setup(1).await; - - client - .create( - "a?b#c.com", - "_acme-challenge", - Record::Txt("the-token".into()), - 60, - ) - .await - .unwrap(); - - assert_eq!( - nodes[0].state.lock().unwrap().set_calls, - vec![("a?b#c.com".to_string(), "the-token".to_string())] - ); - } - /// End-to-end lifecycle through the public `DnsManager` trait, fanned out across multiple /// nodes: create the challenge everywhere, then remove it everywhere. #[tokio::test] From 2a789b7d17ed7c00ef14329c1fef2df21cd36915 Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 18:43:12 +0000 Subject: [PATCH 07/12] Add IcDnsLb enum var --- ic-bn-lib/src/tls/acme/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ic-bn-lib/src/tls/acme/mod.rs b/ic-bn-lib/src/tls/acme/mod.rs index b7539bd..99dde6c 100644 --- a/ic-bn-lib/src/tls/acme/mod.rs +++ b/ic-bn-lib/src/tls/acme/mod.rs @@ -111,6 +111,7 @@ pub struct AcmeCert { #[non_exhaustive] pub enum DnsBackend { Cloudflare, + IcDnsLb, } /// Record type for DnsManager trait From 9b169d21b3895637a2e21b313b112623d5657620 Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 20:20:11 +0000 Subject: [PATCH 08/12] Update comments --- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs index f92b073..dd078c3 100644 --- a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -13,7 +13,7 @@ pub struct IcDnsLb { } impl IcDnsLb { - /// Create a new Cloudflare client with a default HTTP client + /// Create a new IC-DNS-LB API client with a default HTTP client pub fn new(base_urls: Vec, token: String) -> Result { let client = Client::builder() .build() @@ -22,7 +22,7 @@ impl IcDnsLb { Ok(Self::new_with_http_client(base_urls, client, token)) } - /// Create a new Cloudflare client with a provided HTTP client. + /// Create a new IC-DNS-LB API client with a provided HTTP client. /// Client needs to set the authentication token itself. pub const fn new_with_http_client(base_urls: Vec, client: Client, token: String) -> Self { Self { @@ -33,9 +33,10 @@ impl IcDnsLb { } } +/// Request that IC-DNS-LB expects #[derive(Clone, Serialize, Deserialize)] -pub struct AcmeChallengeRequest { - pub challenge: String, +struct AcmeChallengeRequest { + challenge: String, } #[async_trait] @@ -51,6 +52,8 @@ impl DnsManager for IcDnsLb { for url in &self.base_urls { let mut url = url.clone(); + + // Strip trailing slash if exists & add path url.path_segments_mut() .map_err(|()| anyhow!("base URL cannot be used as a base for relative paths"))? .pop_if_empty() @@ -77,6 +80,8 @@ impl DnsManager for IcDnsLb { for url in &self.base_urls { let mut url = url.clone(); + + // Strip trailing slash if exists & add path url.path_segments_mut() .map_err(|()| anyhow!("base URL cannot be used as a base for relative paths"))? .pop_if_empty() From 9f14ad07d87fbdf17df63aa953920db0ec10a8e5 Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Mon, 10 Aug 2026 20:52:11 +0000 Subject: [PATCH 09/12] ic-dns-lb: delete from all nodes even if one fails --- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 80 +++++++++++++++---------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs index dd078c3..46f5beb 100644 --- a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Error, anyhow}; +use anyhow::{Context, Error, anyhow, bail}; use async_trait::async_trait; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -15,6 +15,10 @@ pub struct IcDnsLb { impl IcDnsLb { /// Create a new IC-DNS-LB API client with a default HTTP client pub fn new(base_urls: Vec, token: String) -> Result { + if base_urls.is_empty() { + bail!("At least one URL must be specified"); + } + let client = Client::builder() .build() .context("failed to initialize HTTP client")?; @@ -22,8 +26,7 @@ impl IcDnsLb { Ok(Self::new_with_http_client(base_urls, client, token)) } - /// Create a new IC-DNS-LB API client with a provided HTTP client. - /// Client needs to set the authentication token itself. + /// Create a new IC-DNS-LB API client with a provided HTTP client pub const fn new_with_http_client(base_urls: Vec, client: Client, token: String) -> Self { Self { client, @@ -31,6 +34,21 @@ impl IcDnsLb { token, } } + + /// Sends a POST request + async fn post(&self, url: Url, req: AcmeChallengeRequest) -> Result<(), Error> { + self.client + .post(url) + .bearer_auth(&self.token) + .json(&req) + .send() + .await + .context("unable to send request")? + .error_for_status() + .context("bad HTTP status code")?; + + Ok(()) + } } /// Request that IC-DNS-LB expects @@ -55,21 +73,17 @@ impl DnsManager for IcDnsLb { // Strip trailing slash if exists & add path url.path_segments_mut() - .map_err(|()| anyhow!("base URL cannot be used as a base for relative paths"))? + .map_err(|_| anyhow!("base URL cannot be used as a base for relative paths"))? .pop_if_empty() .extend(["acme-challenge", "set", zone]); - self.client - .post(url) - .bearer_auth(&self.token) - .json(&AcmeChallengeRequest { + self.post( + url, + AcmeChallengeRequest { challenge: challenge.clone(), - }) - .send() - .await - .context("unable to send request")? - .error_for_status() - .context("bad HTTP status code")?; + }, + ) + .await?; } Ok(()) @@ -78,29 +92,35 @@ impl DnsManager for IcDnsLb { async fn delete(&self, zone: &str, _name: &str, record: &Record) -> Result<(), Error> { let Record::Txt(challenge) = record; + // Try to remove records from all nodes even if some fail + let mut errors = vec![]; for url in &self.base_urls { let mut url = url.clone(); // Strip trailing slash if exists & add path url.path_segments_mut() - .map_err(|()| anyhow!("base URL cannot be used as a base for relative paths"))? + .map_err(|_| anyhow!("base URL cannot be used as a base for relative paths"))? .pop_if_empty() .extend(["acme-challenge", "unset", zone]); - self.client - .post(url) - .bearer_auth(&self.token) - .json(&AcmeChallengeRequest { - challenge: challenge.clone(), - }) - .send() + if let Err(e) = self + .post( + url, + AcmeChallengeRequest { + challenge: challenge.clone(), + }, + ) .await - .context("unable to send request")? - .error_for_status() - .context("bad HTTP status code")?; + { + errors.push(e.to_string()); + } } - Ok(()) + if errors.is_empty() { + Ok(()) + } else { + Err(Error::msg(errors.join(", "))) + } } } @@ -317,7 +337,7 @@ mod test { } #[tokio::test] - async fn delete_stops_at_first_failing_node_and_does_not_contact_the_rest() { + async fn delete_contacts_all_nodes_even_if_one_fails() { let (client, nodes) = setup(2).await; nodes[0].state.lock().unwrap().fail_unset = true; @@ -331,11 +351,7 @@ mod test { .unwrap_err(); assert_eq!(nodes[0].state.lock().unwrap().requests_received, 1); - assert_eq!( - nodes[1].state.lock().unwrap().requests_received, - 0, - "later nodes must not be contacted once an earlier one fails" - ); + assert_eq!(nodes[1].state.lock().unwrap().requests_received, 1); } #[tokio::test] From 73f145b9e153a9b6dd23b3023b3cd8ee39d25562 Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Tue, 11 Aug 2026 08:44:20 +0000 Subject: [PATCH 10/12] Cloudflare pagination, other fixes --- ic-bn-lib/src/tls/acme/dns/cloudflare.rs | 175 ++++++++++++++++++++--- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 19 ++- 2 files changed, 168 insertions(+), 26 deletions(-) diff --git a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs index ea4dda2..ef85536 100644 --- a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs +++ b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Error, anyhow}; +use anyhow::{Context, Error, anyhow, bail}; use async_trait::async_trait; use reqwest::{Client, Url}; use serde::{Deserialize, Serialize}; @@ -13,6 +13,10 @@ struct ApiResponse { success: bool, errors: Vec, result: T, + /// Only present on paginated list endpoints (e.g. list dns_records) -- absent (and thus + /// `None`) on zone lookups, creates and deletes. + #[serde(default)] + result_info: Option, } impl ApiResponse { @@ -25,6 +29,18 @@ impl ApiResponse { } } +/// Pagination metadata returned by Cloudflare's paginated list endpoints, e.g. +/// `GET /client/v4/zones//dns_records`. +#[allow(unused)] +#[derive(Debug, Deserialize)] +struct ResultInfo { + count: u32, + page: u32, + per_page: u32, + total_count: u32, + total_pages: u32, +} + #[allow(unused)] #[derive(Deserialize, Debug)] struct ApiError { @@ -64,6 +80,10 @@ pub struct Cloudflare { impl Cloudflare { /// Create a new Cloudflare client with a default HTTP client pub fn new(base_url: Url, token: String) -> Result { + if base_url.cannot_be_a_base() { + bail!("Invalid URL (cannot be a base)"); + } + let client = Client::builder() .build() .context("failed to initialize HTTP client")?; @@ -118,32 +138,78 @@ impl Cloudflare { .ok_or_else(|| anyhow!("zone '{zone}' not found")) } - /// GET /client/v4/zones//dns_records?name= + /// GET /client/v4/zones//dns_records?name=&page=&per_page= + /// + /// This endpoint is paginated, so we page through all results, requesting the largest + /// page size allowed by the API each time. pub async fn find_records(&self, zone_id: &str, name: &str) -> Result, Error> { + /// Documented maximum `per_page` for this endpoint. + const MAX_PER_PAGE: u32 = 50; + + /// Defensive upper bound on the number of pages we'll ever fetch + const MAX_PAGES: u32 = 100; + let url = self .base_url .join(&format!("client/v4/zones/{zone_id}/dns_records")) .context("failed to build dns_records URL")?; - let resp: ApiResponse> = self - .client - .get(url) - .bearer_auth(&self.token) - .query(&[("name", name)]) - .send() - .await - .context("list dns_records request failed")? - .error_for_status() - .context("list dns_records request returned error status")? - .json() - .await - .context("failed to deserialize dns_records response")?; + let mut records = Vec::new(); + let mut page: u32 = 1; - if !resp.success { - return Err(anyhow!("dns_records API error: {}", resp.join_errors())); + loop { + let query = [ + ("name", name.to_string()), + ("page", page.to_string()), + ("per_page", MAX_PER_PAGE.to_string()), + ]; + + let resp: ApiResponse> = self + .client + .get(url.clone()) + .bearer_auth(&self.token) + .query(&query) + .send() + .await + .context("list dns_records request failed")? + .error_for_status() + .context("list dns_records request returned error status")? + .json() + .await + .context("failed to deserialize dns_records response")?; + + if !resp.success { + return Err(anyhow!("dns_records API error: {}", resp.join_errors())); + } + + let page_len = resp.result.len() as u32; + let result_info = resp.result_info; + records.extend(resp.result); + + // Preferred: rely on the server-reported page/total_pages, since Cloudflare + // explicitly returns both for this endpoint. Fall back to fetching until an + // empty page shows up if result_info is ever missing -- this is what + // Cloudflare's own official SDK falls back to as well. We can't use + // `page_len < per_page` as a fallback signal here since we don't know what + // page size the server actually used without result_info. + let last_page = match result_info { + Some(info) if info.total_pages > 0 => page >= info.total_pages, + _ => page_len == 0, + }; + + if last_page || page >= MAX_PAGES { + break; + } + + page += 1; } - Ok(resp.result) + debug!( + "Cloudflare: found {} dns_records matching '{name}' in zone {zone_id}", + records.len() + ); + + Ok(records) } } @@ -412,6 +478,11 @@ mod test { .into_response() } + /// The mock caps its page size at this many records regardless of what the client asks + /// for via `per_page`, purely so that tests can exercise multi-page pagination without + /// needing to create huge numbers of fake records. + const MOCK_MAX_PAGE_SIZE: usize = 2; + async fn list_dns_records( State(state): State, Path(zone_id): Path, @@ -433,9 +504,30 @@ mod test { }; let name_filter = params.get("name"); - let result: Vec = records + let filtered: Vec<&MockRecord> = records .iter() .filter(|r| name_filter.is_none_or(|n| &r.name == n)) + .collect(); + let total_count = filtered.len(); + + let requested_per_page: usize = params + .get("per_page") + .and_then(|v| v.parse().ok()) + .unwrap_or(20); + let per_page = requested_per_page.clamp(1, MOCK_MAX_PAGE_SIZE); + + let page: usize = params + .get("page") + .and_then(|v| v.parse().ok()) + .filter(|&p: &usize| p >= 1) + .unwrap_or(1); + + let total_pages = total_count.div_ceil(per_page); + + let result: Vec = filtered + .into_iter() + .skip((page - 1) * per_page) + .take(per_page) .map(|r| { json!({ "id": r.id, @@ -455,7 +547,13 @@ mod test { "errors": [], "messages": [], "result": result, - "result_info": {"count": count, "page": 1, "per_page": 20, "total_count": count, "total_pages": 1}, + "result_info": { + "count": count, + "page": page, + "per_page": per_page, + "total_count": total_count, + "total_pages": total_pages, + }, })) .into_response() } @@ -635,6 +733,43 @@ mod test { assert_eq!(records[0].record_type, "TXT"); } + /// The mock server only serves `MOCK_MAX_PAGE_SIZE` (2) records per page, so 5 matching + /// records span 3 pages. This verifies `find_records` actually pages through all of them + /// instead of silently returning just the first page. + #[tokio::test] + async fn find_records_paginates_across_multiple_pages() { + let env = setup().await; + let zone_id = { + let mut state = env.state.lock().unwrap(); + let zone_id = state.add_zone("example.com"); + for i in 0..5 { + state.add_record( + &zone_id, + "_acme-challenge.example.com", + "TXT", + &format!("token-{i}"), + ); + } + // A non-matching record shouldn't leak into the results either. + state.add_record(&zone_id, "other.example.com", "TXT", "not-this-one"); + zone_id + }; + + let records = env + .client + .find_records(&zone_id, "_acme-challenge.example.com") + .await + .unwrap(); + + assert_eq!(records.len(), 5); + let mut contents: Vec<&str> = records.iter().map(|r| r.content.as_str()).collect(); + contents.sort_unstable(); + assert_eq!( + contents, + vec!["token-0", "token-1", "token-2", "token-3", "token-4"] + ); + } + #[tokio::test] async fn find_records_errors_on_invalid_zone() { let env = setup().await; diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs index 46f5beb..13b8b12 100644 --- a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Error, anyhow, bail}; +use anyhow::{Context, Error, bail}; use async_trait::async_trait; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -19,6 +19,12 @@ impl IcDnsLb { bail!("At least one URL must be specified"); } + for url in &base_urls { + if url.cannot_be_a_base() { + bail!("Invalid URL (cannot be a base)"); + } + } + let client = Client::builder() .build() .context("failed to initialize HTTP client")?; @@ -73,7 +79,7 @@ impl DnsManager for IcDnsLb { // Strip trailing slash if exists & add path url.path_segments_mut() - .map_err(|_| anyhow!("base URL cannot be used as a base for relative paths"))? + .unwrap() .pop_if_empty() .extend(["acme-challenge", "set", zone]); @@ -98,10 +104,11 @@ impl DnsManager for IcDnsLb { let mut url = url.clone(); // Strip trailing slash if exists & add path - url.path_segments_mut() - .map_err(|_| anyhow!("base URL cannot be used as a base for relative paths"))? - .pop_if_empty() - .extend(["acme-challenge", "unset", zone]); + url.path_segments_mut().unwrap().pop_if_empty().extend([ + "acme-challenge", + "unset", + zone, + ]); if let Err(e) = self .post( From df1b39bdff79bf955950c3a2a668b7554178969c Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Tue, 11 Aug 2026 08:51:26 +0000 Subject: [PATCH 11/12] Fix constructors --- ic-bn-lib/src/tls/acme/dns/cloudflare.rs | 21 +++++++++++------- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 28 ++++++++++++++---------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs index ef85536..9c8648a 100644 --- a/ic-bn-lib/src/tls/acme/dns/cloudflare.rs +++ b/ic-bn-lib/src/tls/acme/dns/cloudflare.rs @@ -80,24 +80,28 @@ pub struct Cloudflare { impl Cloudflare { /// Create a new Cloudflare client with a default HTTP client pub fn new(base_url: Url, token: String) -> Result { - if base_url.cannot_be_a_base() { - bail!("Invalid URL (cannot be a base)"); - } - let client = Client::builder() .build() .context("failed to initialize HTTP client")?; - Ok(Self::new_with_http_client(base_url, token, client)) + Self::new_with_http_client(base_url, token, client) } /// Create a new Cloudflare client with a provided HTTP client - pub const fn new_with_http_client(base_url: Url, token: String, client: Client) -> Self { - Self { + pub fn new_with_http_client( + base_url: Url, + token: String, + client: Client, + ) -> Result { + if base_url.cannot_be_a_base() { + bail!("Invalid URL (cannot be a base)"); + } + + Ok(Self { client, base_url, token, - } + }) } /// GET /client/v4/zones?name= @@ -682,6 +686,7 @@ mod test { fn client_with_token(base_url: Url, token: &str) -> Cloudflare { Cloudflare::new_with_http_client(base_url, token.to_string(), insecure_http_client()) + .unwrap() } #[tokio::test] diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs index 13b8b12..f5e892b 100644 --- a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -15,6 +15,19 @@ pub struct IcDnsLb { impl IcDnsLb { /// Create a new IC-DNS-LB API client with a default HTTP client pub fn new(base_urls: Vec, token: String) -> Result { + let client = Client::builder() + .build() + .context("failed to initialize HTTP client")?; + + Self::new_with_http_client(base_urls, client, token) + } + + /// Create a new IC-DNS-LB API client with a provided HTTP client + pub fn new_with_http_client( + base_urls: Vec, + client: Client, + token: String, + ) -> Result { if base_urls.is_empty() { bail!("At least one URL must be specified"); } @@ -25,20 +38,11 @@ impl IcDnsLb { } } - let client = Client::builder() - .build() - .context("failed to initialize HTTP client")?; - - Ok(Self::new_with_http_client(base_urls, client, token)) - } - - /// Create a new IC-DNS-LB API client with a provided HTTP client - pub const fn new_with_http_client(base_urls: Vec, client: Client, token: String) -> Self { - Self { + Ok(Self { client, base_urls, token, - } + }) } /// Sends a POST request @@ -222,7 +226,7 @@ mod test { } fn client_with_token(base_urls: Vec, token: &str) -> IcDnsLb { - IcDnsLb::new_with_http_client(base_urls, insecure_http_client(), token.to_string()) + IcDnsLb::new_with_http_client(base_urls, insecure_http_client(), token.to_string()).unwrap() } /// Boots `n` independent mock IC DNS LB nodes and a matching `IcDnsLb` client pointed at From 8a4ea32d318d6517fd7b421dea31d6f2e1a39f3b Mon Sep 17 00:00:00 2001 From: Igor Novgorodov Date: Tue, 11 Aug 2026 10:19:07 +0000 Subject: [PATCH 12/12] Add retries to ic-dns-lb token manager --- ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs | 67 ++++++++++++++++++------- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs index f5e892b..b24fe65 100644 --- a/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs +++ b/ic-bn-lib/src/tls/acme/dns/ic_dns_lb.rs @@ -1,5 +1,8 @@ -use anyhow::{Context, Error, bail}; +use std::time::Duration; + +use anyhow::{Context, Error, anyhow, bail}; use async_trait::async_trait; +use http::StatusCode; use reqwest::Client; use serde::{Deserialize, Serialize}; use url::Url; @@ -47,17 +50,45 @@ impl IcDnsLb { /// Sends a POST request async fn post(&self, url: Url, req: AcmeChallengeRequest) -> Result<(), Error> { - self.client - .post(url) - .bearer_auth(&self.token) - .json(&req) - .send() - .await - .context("unable to send request")? - .error_for_status() - .context("bad HTTP status code")?; + let call = async || -> Result { + Ok(self + .client + .post(url.clone()) + .bearer_auth(&self.token) + .json(&req) + .send() + .await + .context("unable to send request")? + .status()) + }; + + // The calls are idempotent - do a few retries + let mut last_error = None; + for i in 0..5 { + match call().await { + Ok(v) => { + if !v.is_success() { + last_error = Some(anyhow!("bad HTTP status code: {v}")); + + // Do not retry when it's not a server error + if !v.is_server_error() { + return Err(last_error.unwrap()); + } + } else { + return Ok(()); + } + } + + Err(e) => { + last_error = Some(e); + } + } - Ok(()) + // Back off exponentially + tokio::time::sleep(Duration::from_millis(250) * i).await; + } + + Err(last_error.unwrap()) } } @@ -291,7 +322,7 @@ mod test { } } - #[tokio::test] + #[tokio::test(start_paused = true)] async fn create_errors_when_a_node_returns_bad_status() { let (client, nodes) = setup(1).await; nodes[0].state.lock().unwrap().fail_set = true; @@ -308,7 +339,7 @@ mod test { assert!(err.to_string().contains("bad HTTP status code"), "{err}"); } - #[tokio::test] + #[tokio::test(start_paused = true)] async fn delete_errors_when_a_node_returns_bad_status() { let (client, nodes) = setup(1).await; nodes[0].state.lock().unwrap().fail_unset = true; @@ -324,7 +355,7 @@ mod test { assert!(err.to_string().contains("bad HTTP status code"), "{err}"); } - #[tokio::test] + #[tokio::test(start_paused = true)] async fn create_stops_at_first_failing_node_and_does_not_contact_the_rest() { let (client, nodes) = setup(2).await; nodes[0].state.lock().unwrap().fail_set = true; @@ -339,7 +370,7 @@ mod test { .await .unwrap_err(); - assert_eq!(nodes[0].state.lock().unwrap().requests_received, 1); + assert_eq!(nodes[0].state.lock().unwrap().requests_received, 5); assert_eq!( nodes[1].state.lock().unwrap().requests_received, 0, @@ -347,7 +378,7 @@ mod test { ); } - #[tokio::test] + #[tokio::test(start_paused = true)] async fn delete_contacts_all_nodes_even_if_one_fails() { let (client, nodes) = setup(2).await; nodes[0].state.lock().unwrap().fail_unset = true; @@ -361,11 +392,11 @@ mod test { .await .unwrap_err(); - assert_eq!(nodes[0].state.lock().unwrap().requests_received, 1); + assert_eq!(nodes[0].state.lock().unwrap().requests_received, 5); assert_eq!(nodes[1].state.lock().unwrap().requests_received, 1); } - #[tokio::test] + #[tokio::test(start_paused = true)] async fn create_errors_when_a_node_is_unreachable() { install_crypto_provider();