Skip to content

Commit

Permalink
feat: add dns resiliency (#6629)
Browse files Browse the repository at this point in the history
Description
---
Added DNS resiliency to the DNS resolver. The config setting now accepts
a list of DNS servers, including system, and will try them one after the
other in case of failure to connect.

Motivation and Context
---
The DNS connection using system did not work on iPhone.

How Has This Been Tested?
---
System-level testing.

What process can a PR reviewer use to test or verify this change?
---
Code review.

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
  • Loading branch information
hansieodendaal authored Oct 15, 2024
1 parent 2d0ccb1 commit 8dd919f
Show file tree
Hide file tree
Showing 11 changed files with 390 additions and 74 deletions.
59 changes: 40 additions & 19 deletions base_layer/p2p/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,21 @@

use std::{
path::{Path, PathBuf},
str::FromStr,
time::Duration,
};

use serde::{Deserialize, Serialize};
use tari_common::{
configuration::{
deserialize_dns_name_server_list,
serializers,
utils::{deserialize_string_or_struct, serialize_string},
utils::serialize_string,
DnsNameServerList,
MultiaddrList,
Network,
StringList,
},
DnsNameServer,
SubConfigPath,
};
use tari_comms::multiaddr::Multiaddr;
Expand All @@ -54,13 +56,13 @@ pub struct PeerSeedsConfig {
/// peer list.
#[serde(default)]
pub dns_seeds: StringList,
/// DNS name server to use for DNS seeds.
#[serde(
default,
deserialize_with = "deserialize_string_or_struct",
deserialize_with = "deserialize_dns_name_server_list",
serialize_with = "serialize_string"
)]
/// DNS name server to use for DNS seeds.
pub dns_seeds_name_server: DnsNameServer,
pub dns_seed_name_servers: DnsNameServerList,
/// All DNS seed records must pass DNSSEC validation
#[serde(default)]
pub dns_seeds_use_dnssec: bool,
Expand All @@ -76,7 +78,10 @@ impl Default for PeerSeedsConfig {
Network::get_current_or_user_setting_or_default().as_key_str()
)]
.into(),
dns_seeds_name_server: DnsNameServer::default(),
dns_seed_name_servers: DnsNameServerList::from_str(
"system, 1.1.1.1:853/cloudflare-dns.com, 8.8.8.8:853/dns.google, 9.9.9.9:853/dns.quad9.net",
)
.expect("string is valid"),
dns_seeds_use_dnssec: false,
}
}
Expand Down Expand Up @@ -180,17 +185,30 @@ impl P2pConfig {

#[cfg(test)]
mod test {
use std::str::FromStr;

use tari_common::DnsNameServer;

use crate::PeerSeedsConfig;

#[test]
fn default_dns_seed_name_servers_test() {
let dns_seed_name_servers = PeerSeedsConfig::default().dns_seed_name_servers;
assert_eq!(dns_seed_name_servers.into_vec(), vec![
DnsNameServer::from_str("system").unwrap(),
DnsNameServer::from_str("1.1.1.1:853/cloudflare-dns.com").unwrap(),
DnsNameServer::from_str("8.8.8.8:853/dns.google").unwrap(),
DnsNameServer::from_str("9.9.9.9:853/dns.quad9.net").unwrap()
]);
}

#[test]
fn it_deserializes_from_toml() {
// No empty fields, no omitted fields
let config_str = r#"
dns_seeds = ["seeds.esmeralda.tari.com"]
peer_seeds = ["20605a28047938f851e3d0cd3f0ff771b2fb23036f0ab8eaa57947dccc834d15::/onion3/e4dsii6vc5f7frao23syonalgikd5kcd7fddrdjhab6bdo3cu47n3kyd:18141"]
dns_seeds_name_server = "1.1.1.1:853/cloudflare-dns.com"
dns_seed_name_servers = ["1.1.1.1:853/cloudflare-dns.com"]
dns_seeds_use_dnssec = false
"#;
let config = toml::from_str::<PeerSeedsConfig>(config_str).unwrap();
Expand All @@ -200,7 +218,7 @@ mod test {
e4dsii6vc5f7frao23syonalgikd5kcd7fddrdjhab6bdo3cu47n3kyd:18141"
]);
assert_eq!(
config.dns_seeds_name_server.to_string(),
config.dns_seed_name_servers.to_string(),
"1.1.1.1:853/cloudflare-dns.com".to_string()
);
assert!(!config.dns_seeds_use_dnssec);
Expand All @@ -209,54 +227,57 @@ mod test {
let config_str = r#"
dns_seeds = ["seeds.esmeralda.tari.com"]
peer_seeds = ["20605a28047938f851e3d0cd3f0ff771b2fb23036f0ab8eaa57947dccc834d15::/onion3/e4dsii6vc5f7frao23syonalgikd5kcd7fddrdjhab6bdo3cu47n3kyd:18141"]
dns_seeds_name_server = ""
dns_seed_name_servers = "111"
#dns_seeds_use_dnssec = false
"#;
match toml::from_str::<PeerSeedsConfig>(config_str) {
Ok(_) => panic!("Should fail"),
Err(e) => assert_eq!(
e.to_string(),
"invalid socket address syntax for key `dns_seeds_name_server` at line 4 column 37"
"invalid socket address syntax for key `dns_seed_name_servers` at line 4 column 37"
),
}

// Empty config list fields
let config_str = r#"
dns_seeds = []
peer_seeds = []
dns_seeds_name_server = "1.1.1.1:853/cloudflare-dns.com"
dns_seed_name_servers = ["system", "1.1.1.1:853/cloudflare-dns.com"]
dns_seeds_use_dnssec = false
"#;
let config = toml::from_str::<PeerSeedsConfig>(config_str).unwrap();
assert_eq!(config.dns_seeds.into_vec(), Vec::<String>::new());
assert_eq!(config.peer_seeds.into_vec(), Vec::<String>::new());
assert_eq!(
config.dns_seeds_name_server.to_string(),
"1.1.1.1:853/cloudflare-dns.com".to_string()
);
assert_eq!(config.dns_seed_name_servers.into_vec(), vec![
DnsNameServer::from_str("system").unwrap(),
DnsNameServer::from_str("1.1.1.1:853/cloudflare-dns.com").unwrap(),
]);
assert!(!config.dns_seeds_use_dnssec);

// Omitted config fields
let config_str = r#"
#dns_seeds = []
#peer_seeds = []
#dns_seeds_name_server = "1.1.1.1:853/cloudflare-dns.com"
#dns_seed_name_servers = []
#dns_seeds_use_dnssec = false
"#;
let config = toml::from_str::<PeerSeedsConfig>(config_str).unwrap();
assert_eq!(config.dns_seeds.into_vec(), Vec::<String>::new());
assert_eq!(config.peer_seeds.into_vec(), Vec::<String>::new());
assert!(matches!(config.dns_seeds_name_server, DnsNameServer::System));
assert_eq!(config.dns_seed_name_servers.into_vec(), vec![]);
assert!(!config.dns_seeds_use_dnssec);

// System
let config_str = r#"
#dns_seeds = []
#peer_seeds = []
dns_seeds_name_server = "system"
dns_seed_name_servers = "system"
#dns_seeds_use_dnssec = false
"#;
let config = toml::from_str::<PeerSeedsConfig>(config_str).unwrap();
assert!(matches!(config.dns_seeds_name_server, DnsNameServer::System));
assert_eq!(config.dns_seed_name_servers.into_vec(), vec![DnsNameServer::from_str(
"system"
)
.unwrap(),]);
}
}
2 changes: 2 additions & 0 deletions base_layer/p2p/src/dns/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,6 @@ pub enum DnsClientError {
SystemHasNoDnsServers,
#[error("DNS server name was not provided for DNSSEC connection e.g.1.1.1.1:853/cloudflare-dns.com")]
DnsNameRequiredForDnsSec,
#[error("Connection error: {0}")]
Connection(String),
}
63 changes: 47 additions & 16 deletions base_layer/p2p/src/initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ use lmdb_zero::open;
use log::*;
use rand::{distributions::Alphanumeric, thread_rng, Rng};
use tari_common::{
configuration::Network,
configuration::{DnsNameServerList, Network},
exit_codes::{ExitCode, ExitError},
DnsNameServer,
};
use tari_comms::{
backoff::ConstantBackoff,
Expand Down Expand Up @@ -81,12 +82,14 @@ use tower::ServiceBuilder;
use crate::{
comms_connector::{InboundDomainConnector, PubsubDomainConnector},
config::{P2pConfig, PeerSeedsConfig},
dns::DnsClientError,
peer_seeds::{DnsSeedResolver, SeedPeer},
transport::{TorTransportConfig, TransportType},
TransportConfig,
MAJOR_NETWORK_VERSION,
MINOR_NETWORK_VERSION,
};

const LOG_TARGET: &str = "p2p::initialization";

/// ProtocolId for minotari messaging protocol
Expand Down Expand Up @@ -490,8 +493,9 @@ impl P2pInitializer {

debug!(
target: LOG_TARGET,
"Resolving DNS seeds (NS:{}, addresses: {})...",
config.dns_seeds_name_server,
"Resolving DNS seeds (DNSSEC is enabled: {}, name servers: {}, addresses: {}) ...",
config.dns_seeds_use_dnssec,
config.dns_seed_name_servers,
config
.dns_seeds
.iter()
Expand All @@ -501,19 +505,8 @@ impl P2pInitializer {
);
let start = Instant::now();

let resolver = if config.dns_seeds_use_dnssec {
debug!(
target: LOG_TARGET,
"Using {} to resolve DNS seeds. DNSSEC is enabled", config.dns_seeds_name_server
);
DnsSeedResolver::connect_secure(config.dns_seeds_name_server.clone()).await?
} else {
debug!(
target: LOG_TARGET,
"Using {} to resolve DNS seeds. DNSSEC is disabled", config.dns_seeds_name_server
);
DnsSeedResolver::connect(config.dns_seeds_name_server.clone()).await?
};
let resolver =
P2pInitializer::get_dns_seed_resolver(config.dns_seeds_use_dnssec, &config.dns_seed_name_servers).await?;
let resolving = config.dns_seeds.iter().map(|addr| {
let mut resolver = resolver.clone();
async move { (resolver.resolve(addr).await, addr) }
Expand Down Expand Up @@ -545,6 +538,44 @@ impl P2pInitializer {

Ok(peers)
}

async fn get_dns_seed_resolver(
dns_seeds_use_dnssec: bool,
dns_seed_name_servers: &DnsNameServerList,
) -> Result<DnsSeedResolver, ServiceInitializationError> {
if dns_seed_name_servers.is_empty() {
return Err(ServiceInitializationError::from(DnsClientError::Connection(
"No DNS name servers configured!".to_string(),
)));
}
let mut dns_errors = Vec::new();
for dns in dns_seed_name_servers {
let res = match (dns_seeds_use_dnssec, dns == &DnsNameServer::System) {
(true, false) => DnsSeedResolver::connect_secure(dns.clone()).await,
(_, _) => DnsSeedResolver::connect(dns.clone()).await,
};
match res {
Ok(val) => {
trace!(target: LOG_TARGET, "Found DNS client at '{}'", dns);
return Ok(val);
},
Err(err) => {
warn!(
target: LOG_TARGET,
"DNS entry '{}' did not respond, trying the next one. You can edit 'dns_seed_name_servers' in \
the config file. (Error: {})",
dns,
err.to_string(),
);
dns_errors.push(err.to_string())
},
}
}
Err(ServiceInitializationError::from(DnsClientError::Connection(format!(
"{:?}",
dns_errors
))))
}
}

#[async_trait]
Expand Down
Loading

0 comments on commit 8dd919f

Please sign in to comment.