Server Side internal DoT/DoH opt out (#5577)

Cargo.lock

@@ -6355,6 +6355,7 @@ dependencies = [
  "nym-contracts-common",
  "nym-crypto",
  "nym-explorer-client",
+ "nym-http-api-client",
  "nym-network-defaults",
  "nym-node-metrics",
  "nym-node-requests",

common/client-libs/validator-client/src/coconut/mod.rs

@@ -83,6 +83,12 @@ impl TryFrom<ContractVKShare> for EcashApiClient {
 
         let url_address = Url::parse(&share.announce_address)?;
 
+        // The NymApiClient constructed here uses the default (hickory DoT/DoH) resolver because
+        // this EcashApiClient is used by both client and non-client applications.
+        //
+        // In non-client applications this resolver can cause warning logs about H2 connection
+        // failure. This indicates that the long lived https connection was closed by the remote
+        // peer and the resolver will have to reconnect. It should not impact actual functionality
         Ok(EcashApiClient {
             api_client: NymApiClient::new(url_address),
             verification_key: VerificationKeyAuth::try_from_bs58(&share.share)?,

common/http-api-client/src/dns.rs

@@ -1,3 +1,6 @@
+// Copyright 2023 - Nym Technologies SA <contact@nymtech.net>
+// SPDX-License-Identifier: Apache-2.0
+
 //! DNS resolver configuration for internal lookups.
 //!
 //! The resolver itself is the set combination of the google, cloudflare, and quad9 endpoints
@@ -9,6 +12,19 @@
 //!
 //! Requires the `dns-over-https-rustls`, `webpki-roots` feature for the
 //! `hickory-resolver` crate
+//!
+//!
+//! Note: The hickory DoH resolver can cause warning logs about H2 connection failure. This
+//! indicates that the long lived https connection was closed by the remote peer and the resolver
+//! will have to reconnect. It should not impact actual functionality.
+//!
+//! code ref: https://github.com/hickory-dns/hickory-dns/blob/06a8b1ce9bd9322d8e6accf857d30257e1274427/crates/proto/src/h2/h2_client_stream.rs#L534
+//!
+//! example log:
+//!
+//! ```txt
+//!   WARN /home/ubuntu/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/hickory-proto-0.24.3/src/h2/h2_client_stream.rs:493: h2 connection failed: unexpected end of file
+//! ```
 #![deny(missing_docs)]
 
 use crate::ClientBuilder;
@@ -33,6 +49,13 @@ impl ClientBuilder {
     /// Override the DNS resolver implementation used by the underlying http client.
     pub fn dns_resolver<R: Resolve + 'static>(mut self, resolver: Arc<R>) -> Self {
         self.reqwest_client_builder = self.reqwest_client_builder.dns_resolver(resolver);
+        self.use_secure_dns = false;
+        self
+    }
+
+    /// Override the DNS resolver implementation used by the underlying http client.
+    pub fn no_hickory_dns(mut self) -> Self {
+        self.use_secure_dns = false;
         self
     }
 }

common/http-api-client/src/lib.rs

@@ -228,6 +228,8 @@ pub struct ClientBuilder {
     timeout: Option<Duration>,
     custom_user_agent: bool,
     reqwest_client_builder: reqwest::ClientBuilder,
+    #[allow(dead_code)] // not dead code, just unused in wasm
+    use_secure_dns: bool,
 }
 
 impl ClientBuilder {
@@ -239,37 +241,46 @@ impl ClientBuilder {
         U: IntoUrl,
         E: Display,
     {
-        // a naive check: if the provided URL does not start with http(s), add that scheme
         let str_url = url.as_str();
 
+        // a naive check: if the provided URL does not start with http(s), add that scheme
         if !str_url.starts_with("http") {
             let alt = format!("http://{str_url}");
             warn!("the provided url ('{str_url}') does not contain scheme information. Changing it to '{alt}' ...");
             // TODO: or should we maybe default to https?
             Self::new(alt)
         } else {
-            #[cfg(target_arch = "wasm32")]
-            let reqwest_client_builder = reqwest::ClientBuilder::new();
+            Ok(Self::new_with_url(url.into_url()?))
+        }
+    }
 
-            #[cfg(not(target_arch = "wasm32"))]
-            let reqwest_client_builder = {
-                let r = reqwest::ClientBuilder::new()
-                    .dns_resolver(Arc::new(HickoryDnsResolver::default()));
+    /// Constructs a new http `ClientBuilder` from a valid url.
+    pub fn new_with_url(url: Url) -> Self {
+        if !url.scheme().starts_with("http") {
+            warn!("the provided url ('{url}') does not use HTTP / HTTPS scheme");
+        }
 
-                // Note this is extra as the `gzip` feature for `reqwest` crate should be enabled which
-                // `"Enable[s] auto gzip decompression by checking the Content-Encoding response header."`
-                //
-                // I am going to leave it here anyways so that gzip decompression is attempted even if
-                // that feature is removed.
-                r.gzip(true)
-            };
+        #[cfg(target_arch = "wasm32")]
+        let reqwest_client_builder = reqwest::ClientBuilder::new();
 
-            Ok(ClientBuilder {
-                url: url.into_url()?,
-                timeout: None,
-                custom_user_agent: false,
-                reqwest_client_builder,
-            })
+        #[cfg(not(target_arch = "wasm32"))]
+        let reqwest_client_builder = {
+            let r = reqwest::ClientBuilder::new();
+
+            // Note this is extra as the `gzip` feature for `reqwest` crate should be enabled which
+            // `"Enable[s] auto gzip decompression by checking the Content-Encoding response header."`
+            //
+            // I am going to leave it here anyways so that gzip decompression is attempted even if
+            // that feature is removed.
+            r.gzip(true)
+        };
+
+        ClientBuilder {
+            url,
+            timeout: None,
+            custom_user_agent: false,
+            reqwest_client_builder,
+            use_secure_dns: true,
         }
     }
 
@@ -325,10 +336,18 @@ impl ClientBuilder {
             let mut builder = self
                 .reqwest_client_builder
                 .timeout(self.timeout.unwrap_or(DEFAULT_TIMEOUT));
+
+            // if no custom user agent was set, use a default
             if !self.custom_user_agent {
                 builder =
                     builder.user_agent(format!("nym-http-api-client/{}", env!("CARGO_PKG_VERSION")))
             }
+
+            // unless explicitly disabled use the DoT/DoH enabled resolver
+            if self.use_secure_dns {
+                builder = builder.dns_resolver(Arc::new(HickoryDnsResolver::default()));
+            }
+
             builder.build()?
         };
 
@@ -355,6 +374,9 @@ pub struct Client {
 impl Client {
     /// Create a new http `Client`
     // no timeout until https://github.com/seanmonstar/reqwest/issues/1135 is fixed
+    //
+    // In order to prevent interference in API requests at the DNS phase we default to a resolver
+    // that uses DoT and DoH.
     pub fn new(base_url: Url, timeout: Option<Duration>) -> Self {
         Self::new_url::<_, String>(base_url, timeout).expect(
             "we provided valid url and we were unwrapping previous construction errors anyway",

nym-api/src/node_describe_cache/mod.rs

@@ -191,6 +191,7 @@ async fn try_get_client(
         // if provided host was malformed, no point in continuing
         let client = match nym_node_requests::api::Client::builder(address).and_then(|b| {
             b.with_timeout(Duration::from_secs(5))
+                .no_hickory_dns()
                 .with_user_agent("nym-api-describe-cache")
                 .build()
         }) {

nym-node-status-api/nym-node-status-api/Cargo.toml

@@ -29,6 +29,7 @@ nym-bin-common = { path = "../../common/bin-common", features = ["models"] }
 nym-node-status-client = { path = "../nym-node-status-client" }
 nym-crypto = { path = "../../common/crypto", features = ["asymmetric", "serde"] }
 nym-explorer-client = { path = "../../explorer-api/explorer-client" }
+nym-http-api-client = { path = "../../common/http-api-client" }
 nym-network-defaults = { path = "../../common/network-defaults" }
 nym-serde-helpers = { path = "../../common/serde-helpers"}
 nym-statistics-common = { path = "../../common/statistics" }

nym-node-status-api/nym-node-status-api/src/monitor/mod.rs

@@ -97,8 +97,12 @@ impl Monitor {
             .clone()
             .expect("rust sdk mainnet default missing api_url");
 
-        let api_client =
-            NymApiClient::new_with_timeout(default_api_url, self.nym_api_client_timeout);
+        let nym_api = nym_http_api_client::ClientBuilder::new_with_url(default_api_url)
+            .no_hickory_dns()
+            .with_timeout(self.nym_api_client_timeout)
+            .build::<&str>()?;
+
+        let api_client = NymApiClient { nym_api };
 
         let described_nodes = api_client
             .get_all_described_nodes()

nym-node-status-api/nym-node-status-api/src/node_scraper/mod.rs

@@ -57,7 +57,12 @@ async fn run(
         .clone()
         .expect("rust sdk mainnet default missing api_url");
 
-    let api_client = NymApiClient::new_with_timeout(default_api_url, nym_api_client_timeout);
+    let nym_api = nym_http_api_client::ClientBuilder::new_with_url(default_api_url)
+        .no_hickory_dns()
+        .with_timeout(nym_api_client_timeout)
+        .build::<&str>()?;
+
+    let api_client = NymApiClient { nym_api };
 
     //SW TBC what nodes exactly need to be scraped, the skimmed node endpoint seems to return more nodes
     let bonded_nodes = api_client.get_all_bonded_nym_nodes().await?;
@@ -170,6 +175,7 @@ impl MetricsScrapingData {
             let client = match nym_node_requests::api::Client::builder(address).and_then(|b| {
                 b.with_timeout(Duration::from_secs(5))
                     .with_user_agent("node-status-api-metrics-scraper")
+                    .no_hickory_dns()
                     .build()
             }) {
                 Ok(client) => client,

nym-node/Cargo.toml

@@ -71,7 +71,7 @@ nym-verloc = { path = "../common/verloc" }
 nym-metrics = { path = "../common/nym-metrics" }
 nym-gateway-stats-storage = { path = "../common/gateway-stats-storage" }
 nym-topology = { path = "../common/topology" }
-
+nym-http-api-client = { path = "../common/http-api-client" }
 
 # http server
 # useful for `#[axum_macros::debug_handler]`

nym-node/src/error.rs

@@ -3,6 +3,7 @@
 
 use crate::node::http::error::NymNodeHttpError;
 use crate::wireguard::error::WireguardError;
+use nym_http_api_client::HttpClientError;
 use nym_ip_packet_router::error::ClientCoreError;
 use nym_validator_client::ValidatorClientError;
 use std::io;
@@ -209,3 +210,9 @@ pub enum ServiceProvidersError {
     #[error(transparent)]
     ExternalClientCore(#[from] ClientCoreError),
 }
+
+impl From<HttpClientError> for NymNodeError {
+    fn from(value: HttpClientError) -> Self {
+        Self::HttpFailure(NymNodeHttpError::ClientError { source: value })
+    }
+}

nym-node/src/node/http/error.rs

@@ -1,6 +1,7 @@
 // Copyright 2024 - Nym Technologies SA <contact@nymtech.net>
 // SPDX-License-Identifier: GPL-3.0-only
 
+use nym_http_api_client::HttpClientError;
 use std::io;
 use std::net::SocketAddr;
 use thiserror::Error;
@@ -24,4 +25,10 @@ pub enum NymNodeHttpError {
         #[from]
         source: nym_crypto::asymmetric::encryption::KeyRecoveryError,
     },
+
+    #[error("error building or using HTTP client: {source}")]
+    ClientError {
+        #[from]
+        source: HttpClientError,
+    },
 }

nym-node/src/node/mod.rs

@@ -814,9 +814,23 @@ impl NymNode {
             return;
         }
 
-        for nym_api in &self.config.mixnet.nym_api_urls {
-            info!("trying {nym_api}...");
-            let client = NymApiClient::new_with_user_agent(nym_api.clone(), self.user_agent());
+        for nym_api_url in &self.config.mixnet.nym_api_urls {
+            info!("trying {nym_api_url}...");
+
+            let nym_api =
+                match nym_http_api_client::ClientBuilder::new_with_url(nym_api_url.clone())
+                    .no_hickory_dns()
+                    .with_user_agent(self.user_agent())
+                    .build::<&str>()
+                {
+                    Ok(b) => b,
+                    Err(e) => {
+                        warn!("failed to build http client for \"{nym_api_url}\": {e}",);
+                        continue;
+                    }
+                };
+
+            let client = NymApiClient { nym_api };
 
             // make new request every time in case previous one takes longer and invalidates the signature
             let request = NodeRefreshBody::new(self.ed25519_identity_keys.private_key());

nym-node/src/node/shared_network.rs

@@ -9,6 +9,7 @@ use nym_node_metrics::prometheus_wrapper::{PrometheusMetric, PROMETHEUS_METRICS}
 use nym_task::ShutdownToken;
 use nym_topology::node::RoutingNode;
 use nym_topology::{EpochRewardedSet, NymTopology, Role, TopologyProvider};
+use nym_validator_client::nym_api::NymApiClientExt;
 use nym_validator_client::nym_nodes::{NodesByAddressesResponse, SkimmedNode};
 use nym_validator_client::{NymApiClient, ValidatorClientError};
 use std::collections::HashSet;
@@ -167,6 +168,7 @@ impl NodesQuerier {
     ) -> Result<NodesByAddressesResponse, ValidatorClientError> {
         let res = self
             .client
+            .nym_api
             .nodes_by_addresses(ips)
             .await
             .inspect_err(|err| error!("failed to obtain node information: {err}"));
@@ -174,7 +176,7 @@ impl NodesQuerier {
         if res.is_err() {
             self.use_next_nym_api()
         }
-        res
+        Ok(res?)
     }
 }
 
@@ -263,9 +265,14 @@ impl NetworkRefresher {
         pending_check_interval: Duration,
         shutdown_token: ShutdownToken,
     ) -> Result<Self, NymNodeError> {
+        let nym_api = nym_http_api_client::Client::builder(nym_api_urls[0].clone())?
+            .no_hickory_dns()
+            .with_user_agent(user_agent)
+            .build()?;
+
         let mut this = NetworkRefresher {
             querier: NodesQuerier {
-                client: NymApiClient::new_with_user_agent(nym_api_urls[0].clone(), user_agent),
+                client: NymApiClient { nym_api },
                 nym_api_urls,
                 currently_used_api: 0,
             },