From 70245e7ff8e75dcb053d62b0a106f2bfd69a4991 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 5 Mar 2024 14:41:45 +0100 Subject: [PATCH] refactor: use builder pattern in Resolver ctor the `start` constructor's parameter list was getting long and we want to add even more configuration options, like EDE, in the future. using the builder pattern lets us introduce new settings without breaking changes --- .../src/resolver/dns/scenarios.rs | 16 +- .../dnssec/rfc4035/section_4/section_4_1.rs | 10 +- .../src/resolver/dnssec/scenarios/bogus.rs | 13 +- .../src/resolver/dnssec/scenarios/secure.rs | 23 +-- packages/dns-test/examples/explore.rs | 9 +- packages/dns-test/src/resolver.rs | 160 ++++++++++-------- packages/dns-test/src/trust_anchor.rs | 4 + packages/dns-test/src/tshark.rs | 12 +- 8 files changed, 138 insertions(+), 109 deletions(-) diff --git a/packages/conformance-tests/src/resolver/dns/scenarios.rs b/packages/conformance-tests/src/resolver/dns/scenarios.rs index 20a90d07..121936cd 100644 --- a/packages/conformance-tests/src/resolver/dns/scenarios.rs +++ b/packages/conformance-tests/src/resolver/dns/scenarios.rs @@ -4,7 +4,7 @@ use dns_test::client::{Client, DigSettings}; use dns_test::name_server::NameServer; use dns_test::record::{Record, RecordType}; use dns_test::zone_file::Root; -use dns_test::{Network, Resolver, Result, TrustAnchor, FQDN}; +use dns_test::{Network, Resolver, Result, FQDN}; #[test] fn can_resolve() -> Result<()> { @@ -39,8 +39,11 @@ fn can_resolve() -> Result<()> { eprintln!("root.zone:\n{}", root_ns.zone_file()); - let roots = &[Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr())]; - let resolver = Resolver::start(&dns_test::subject(), roots, &TrustAnchor::empty(), &network)?; + let resolver = Resolver::new( + &network, + Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr()), + ) + .start(&dns_test::subject())?; let resolver_ip_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; @@ -85,8 +88,11 @@ fn nxdomain() -> Result<()> { root_ns.referral(FQDN::COM, com_ns.fqdn().clone(), com_ns.ipv4_addr()); let root_ns = root_ns.start()?; - let roots = &[Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr())]; - let resolver = Resolver::start(&dns_test::subject(), roots, &TrustAnchor::empty(), &network)?; + let resolver = Resolver::new( + &network, + Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr()), + ) + .start(&dns_test::subject())?; let resolver_ip_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; diff --git a/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_1.rs b/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_1.rs index 88a65eb8..6bfbacf2 100644 --- a/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_1.rs +++ b/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_1.rs @@ -3,19 +3,15 @@ use dns_test::name_server::NameServer; use dns_test::record::RecordType; use dns_test::tshark::{Capture, Direction}; use dns_test::zone_file::Root; -use dns_test::{Network, Resolver, Result, TrustAnchor, FQDN}; +use dns_test::{Network, Resolver, Result, FQDN}; #[test] #[ignore] fn edns_support() -> Result<()> { let network = &Network::new()?; let ns = NameServer::new(&dns_test::peer(), FQDN::ROOT, network)?.start()?; - let resolver = Resolver::start( - &dns_test::subject(), - &[Root::new(ns.fqdn().clone(), ns.ipv4_addr())], - &TrustAnchor::empty(), - network, - )?; + let resolver = Resolver::new(network, Root::new(ns.fqdn().clone(), ns.ipv4_addr())) + .start(&dns_test::subject())?; let mut tshark = resolver.eavesdrop()?; diff --git a/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs b/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs index 0dd99b51..5f51d7dc 100644 --- a/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs +++ b/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs @@ -5,7 +5,7 @@ use dns_test::client::{Client, DigSettings}; use dns_test::name_server::NameServer; use dns_test::record::{Record, RecordType}; use dns_test::zone_file::Root; -use dns_test::{Network, Resolver, Result, TrustAnchor, FQDN}; +use dns_test::{Network, Resolver, Result, FQDN}; #[ignore] #[test] @@ -64,10 +64,13 @@ fn bad_signature_in_leaf_nameserver() -> Result<()> { let root_ns = root_ns.start()?; - let roots = &[Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr())]; - - let trust_anchor = TrustAnchor::from_iter([root_ksk.clone(), root_zsk.clone()]); - let resolver = Resolver::start(&dns_test::subject(), roots, &trust_anchor, &network)?; + let resolver = Resolver::new( + &network, + Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr()), + ) + .trust_anchor_key(root_ksk) + .trust_anchor_key(root_zsk) + .start(&dns_test::subject())?; let resolver_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; diff --git a/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs b/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs index 6df21009..3ed3439b 100644 --- a/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs +++ b/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs @@ -24,10 +24,10 @@ fn can_validate_without_delegation() -> Result<()> { eprintln!("root.zone:\n{}", ns.zone_file()); - let roots = &[Root::new(ns.fqdn().clone(), ns.ipv4_addr())]; - - let trust_anchor = TrustAnchor::from_iter([root_ksk.clone(), root_zsk.clone()]); - let resolver = Resolver::start(&dns_test::subject(), roots, &trust_anchor, &network)?; + let trust_anchor = &TrustAnchor::from_iter([root_ksk.clone(), root_zsk.clone()]); + let resolver = Resolver::new(&network, Root::new(ns.fqdn().clone(), ns.ipv4_addr())) + .trust_anchor(trust_anchor) + .start(&dns_test::subject())?; let resolver_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; @@ -37,7 +37,7 @@ fn can_validate_without_delegation() -> Result<()> { assert!(output.status.is_noerror()); assert!(output.flags.authenticated_data); - let output = client.delv(resolver_addr, RecordType::SOA, &FQDN::ROOT, &trust_anchor)?; + let output = client.delv(resolver_addr, RecordType::SOA, &FQDN::ROOT, trust_anchor)?; assert!(output.starts_with("; fully validated")); Ok(()) @@ -91,10 +91,13 @@ fn can_validate_with_delegation() -> Result<()> { eprintln!("root.zone:\n{}", root_ns.zone_file()); - let roots = &[Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr())]; - - let trust_anchor = TrustAnchor::from_iter([root_ksk.clone(), root_zsk.clone()]); - let resolver = Resolver::start(&dns_test::subject(), roots, &trust_anchor, &network)?; + let trust_anchor = &TrustAnchor::from_iter([root_ksk, root_zsk]); + let resolver = Resolver::new( + &network, + Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr()), + ) + .trust_anchor(trust_anchor) + .start(&dns_test::subject())?; let resolver_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; @@ -111,7 +114,7 @@ fn can_validate_with_delegation() -> Result<()> { assert_eq!(needle_fqdn, a.fqdn); assert_eq!(expected_ipv4_addr, a.ipv4_addr); - let output = client.delv(resolver_addr, RecordType::A, &needle_fqdn, &trust_anchor)?; + let output = client.delv(resolver_addr, RecordType::A, &needle_fqdn, trust_anchor)?; assert!(output.starts_with("; fully validated")); Ok(()) diff --git a/packages/dns-test/examples/explore.rs b/packages/dns-test/examples/explore.rs index 038870a1..6e8de222 100644 --- a/packages/dns-test/examples/explore.rs +++ b/packages/dns-test/examples/explore.rs @@ -44,13 +44,16 @@ fn main() -> Result<()> { let root_zsk = root_ns.zone_signing_key().clone(); let root_ns = root_ns.start()?; - - let roots = &[Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr())]; println!("DONE"); let trust_anchor = TrustAnchor::from_iter([root_ksk.clone(), root_zsk.clone()]); println!("building docker image..."); - let resolver = Resolver::start(&dns_test::subject(), roots, &trust_anchor, &network)?; + let resolver = Resolver::new( + &network, + Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr()), + ) + .trust_anchor(&trust_anchor) + .start(&dns_test::subject())?; println!("DONE\n\n"); let resolver_addr = resolver.ipv4_addr(); diff --git a/packages/dns-test/src/resolver.rs b/packages/dns-test/src/resolver.rs index 4460a198..93d17f3a 100644 --- a/packages/dns-test/src/resolver.rs +++ b/packages/dns-test/src/resolver.rs @@ -3,6 +3,7 @@ use std::net::Ipv4Addr; use crate::container::{Child, Container, Network}; use crate::implementation::{Config, Role}; +use crate::record::DNSKEY; use crate::trust_anchor::TrustAnchor; use crate::tshark::Tshark; use crate::zone_file::Root; @@ -15,67 +16,13 @@ pub struct Resolver { } impl Resolver { - /// Starts a DNS server in the recursive resolver role - /// - /// This server is not an authoritative name server; it does not server a zone file to clients - /// - /// # Panics - /// - /// This constructor panics if `roots` is an empty slice - pub fn start( - implementation: &Implementation, - roots: &[Root], - trust_anchor: &TrustAnchor, - network: &Network, - ) -> Result { - assert!( - !roots.is_empty(), - "must configure at least one local root server" - ); - - let image = implementation.clone().into(); - let container = Container::run(&image, network)?; - - let mut hints = String::new(); - for root in roots { - writeln!(hints, "{root}").unwrap(); + #[allow(clippy::new_ret_no_self)] + pub fn new(network: &Network, root: Root) -> ResolverSettings { + ResolverSettings { + network: network.clone(), + roots: vec![root], + trust_anchor: TrustAnchor::empty(), } - - container.cp("/etc/root.hints", &hints)?; - - let use_dnssec = !trust_anchor.is_empty(); - let config = Config::Resolver { - use_dnssec, - netmask: network.netmask(), - }; - container.cp( - implementation.conf_file_path(config.role()), - &implementation.format_config(config), - )?; - - if use_dnssec { - let path = if implementation.is_bind() { - "/etc/bind/bind.keys" - } else { - "/etc/trusted-key.key" - }; - - let contents = if implementation.is_bind() { - trust_anchor.delv() - } else { - trust_anchor.to_string() - }; - - container.cp(path, &contents)?; - } - - let child = container.spawn(implementation.cmd_args(config.role()))?; - - Ok(Self { - child, - container, - implementation: implementation.clone(), - }) } pub fn eavesdrop(&self) -> Result { @@ -112,6 +59,83 @@ kill -TERM $(cat {pidfile})" } } +pub struct ResolverSettings { + network: Network, + roots: Vec, + trust_anchor: TrustAnchor, +} + +impl ResolverSettings { + /// Starts a DNS server in the recursive resolver role + /// + /// This server is not an authoritative name server; it does not serve a zone file to clients + pub fn start(&self, implementation: &Implementation) -> Result { + let image = implementation.clone().into(); + let container = Container::run(&image, &self.network)?; + + let mut hints = String::new(); + for root in &self.roots { + writeln!(hints, "{root}").unwrap(); + } + + container.cp("/etc/root.hints", &hints)?; + + let use_dnssec = !self.trust_anchor.is_empty(); + let config = Config::Resolver { + use_dnssec, + netmask: self.network.netmask(), + }; + container.cp( + implementation.conf_file_path(config.role()), + &implementation.format_config(config), + )?; + + if use_dnssec { + let path = if implementation.is_bind() { + "/etc/bind/bind.keys" + } else { + "/etc/trusted-key.key" + }; + + let contents = if implementation.is_bind() { + self.trust_anchor.delv() + } else { + self.trust_anchor.to_string() + }; + + container.cp(path, &contents)?; + } + + let child = container.spawn(implementation.cmd_args(config.role()))?; + + Ok(Resolver { + child, + container, + implementation: implementation.clone(), + }) + } + + /// Adds a root hint + pub fn root(&mut self, root: Root) -> &mut Self { + self.roots.push(root); + self + } + + /// Adds a DNSKEY record to the trust anchor + pub fn trust_anchor_key(&mut self, key: DNSKEY) -> &mut Self { + self.trust_anchor.add(key.clone()); + self + } + + /// Adds all the keys in the `other` trust anchor to ours + pub fn trust_anchor(&mut self, other: &TrustAnchor) -> &mut Self { + for key in other.keys() { + self.trust_anchor.add(key.clone()); + } + self + } +} + #[cfg(test)] mod tests { use crate::{name_server::NameServer, FQDN}; @@ -122,12 +146,8 @@ mod tests { fn terminate_unbound_works() -> Result<()> { let network = Network::new()?; let ns = NameServer::new(&Implementation::Unbound, FQDN::ROOT, &network)?.start()?; - let resolver = Resolver::start( - &Implementation::Unbound, - &[Root::new(ns.fqdn().clone(), ns.ipv4_addr())], - &TrustAnchor::empty(), - &network, - )?; + let resolver = Resolver::new(&network, Root::new(ns.fqdn().clone(), ns.ipv4_addr())) + .start(&Implementation::Unbound)?; let logs = resolver.terminate()?; eprintln!("{logs}"); @@ -140,12 +160,8 @@ mod tests { fn terminate_bind_works() -> Result<()> { let network = Network::new()?; let ns = NameServer::new(&Implementation::Unbound, FQDN::ROOT, &network)?.start()?; - let resolver = Resolver::start( - &Implementation::Bind, - &[Root::new(ns.fqdn().clone(), ns.ipv4_addr())], - &TrustAnchor::empty(), - &network, - )?; + let resolver = Resolver::new(&network, Root::new(ns.fqdn().clone(), ns.ipv4_addr())) + .start(&Implementation::Bind)?; let logs = resolver.terminate()?; eprintln!("{logs}"); diff --git a/packages/dns-test/src/trust_anchor.rs b/packages/dns-test/src/trust_anchor.rs index e6333bbe..01e1f631 100644 --- a/packages/dns-test/src/trust_anchor.rs +++ b/packages/dns-test/src/trust_anchor.rs @@ -20,6 +20,10 @@ impl TrustAnchor { self } + pub(crate) fn keys(&self) -> &[DNSKEY] { + &self.keys + } + /// formats the `TrustAnchor` in the format `delv` expects pub(super) fn delv(&self) -> String { let mut buf = "trust-anchors {".to_string(); diff --git a/packages/dns-test/src/tshark.rs b/packages/dns-test/src/tshark.rs index e736ea9d..8b850019 100644 --- a/packages/dns-test/src/tshark.rs +++ b/packages/dns-test/src/tshark.rs @@ -248,7 +248,7 @@ mod tests { use crate::name_server::NameServer; use crate::record::{Record, RecordType}; use crate::zone_file::Root; - use crate::{Implementation, Network, Resolver, TrustAnchor, FQDN}; + use crate::{Implementation, Network, Resolver, FQDN}; use super::*; @@ -310,13 +310,11 @@ mod tests { root_ns.referral(FQDN::COM, com_ns.fqdn().clone(), com_ns.ipv4_addr()); let root_ns = root_ns.start()?; - let roots = &[Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr())]; - let resolver = Resolver::start( - &Implementation::Unbound, - roots, - &TrustAnchor::empty(), + let resolver = Resolver::new( network, - )?; + Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr()), + ) + .start(&Implementation::Unbound)?; let mut tshark = resolver.eavesdrop()?; let resolver_addr = resolver.ipv4_addr();