From 58239028f4c94495f11b7427abec733059dc7960 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Mon, 11 Mar 2024 17:22:28 +0100 Subject: [PATCH] turn dns_test::{subject,peer} into immutable statics using `std::env::set_var` to set or change the value of either DNS_TEST_SUBJECT or DNS_TEST_PEER is A Bad Idea, specially so when tests are running in parallel we can't forbid the use of `env::set_var` _but_ at least we can ensure that even in its presence the return value of `dns_test::{subject,peer}` will not change this is accomplished using a "lazy" static variable that gets initialized at most once during the lifetime of the process instead of reading the env var each time `{subject,peer}` is called to better convey the fact that the return value of `{subject,peer}` won't change, we present them as static variables instead --- Cargo.lock | 7 ++ .../section_3/section_3_1/section_3_1_1.rs | 4 +- .../src/name_server/scenarios.rs | 2 +- .../src/resolver/dns/scenarios.rs | 10 ++- .../dnssec/rfc4035/section_4/section_4_1.rs | 4 +- .../src/resolver/dnssec/scenarios/bogus.rs | 5 +- .../src/resolver/dnssec/scenarios/ede.rs | 6 +- .../src/resolver/dnssec/scenarios/secure.rs | 9 ++- packages/dns-test/Cargo.toml | 1 + packages/dns-test/examples/explore.rs | 10 +-- packages/dns-test/src/implementation.rs | 4 +- packages/dns-test/src/lib.rs | 65 +++++++++++++++++-- 12 files changed, 92 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 35f08284..5927e0b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -155,6 +155,7 @@ name = "dns-test" version = "0.1.0" dependencies = [ "ctrlc", + "lazy_static", "minijinja", "pretty_assertions", "serde", @@ -295,6 +296,12 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "lazy_static" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" + [[package]] name = "libc" version = "0.2.153" diff --git a/packages/conformance-tests/src/name_server/rfc4035/section_3/section_3_1/section_3_1_1.rs b/packages/conformance-tests/src/name_server/rfc4035/section_3/section_3_1/section_3_1_1.rs index 20a1a9e3..02a9d6ac 100644 --- a/packages/conformance-tests/src/name_server/rfc4035/section_3/section_3_1/section_3_1_1.rs +++ b/packages/conformance-tests/src/name_server/rfc4035/section_3/section_3_1/section_3_1_1.rs @@ -8,7 +8,7 @@ use dns_test::{Network, Result, FQDN}; fn rrsig_in_answer_section() -> Result<()> { let network = Network::new()?; - let ns = NameServer::new(&dns_test::subject(), FQDN::ROOT, &network)? + let ns = NameServer::new(&dns_test::SUBJECT, FQDN::ROOT, &network)? .sign()? .start()?; @@ -37,7 +37,7 @@ fn rrsig_in_answer_section() -> Result<()> { fn rrsig_in_authority_section() -> Result<()> { let network = Network::new()?; - let ns = NameServer::new(&dns_test::subject(), FQDN::ROOT, &network)? + let ns = NameServer::new(&dns_test::SUBJECT, FQDN::ROOT, &network)? .sign()? .start()?; diff --git a/packages/conformance-tests/src/name_server/scenarios.rs b/packages/conformance-tests/src/name_server/scenarios.rs index 17578ad7..ee26a788 100644 --- a/packages/conformance-tests/src/name_server/scenarios.rs +++ b/packages/conformance-tests/src/name_server/scenarios.rs @@ -6,7 +6,7 @@ use dns_test::{Network, Result, FQDN}; #[test] fn authoritative_answer() -> Result<()> { let network = &Network::new()?; - let ns = NameServer::new(&dns_test::subject(), FQDN::ROOT, network)?.start()?; + let ns = NameServer::new(&dns_test::SUBJECT, FQDN::ROOT, network)?.start()?; let client = Client::new(network)?; let ans = client.dig( diff --git a/packages/conformance-tests/src/resolver/dns/scenarios.rs b/packages/conformance-tests/src/resolver/dns/scenarios.rs index 2481bbd4..d3d7fc5c 100644 --- a/packages/conformance-tests/src/resolver/dns/scenarios.rs +++ b/packages/conformance-tests/src/resolver/dns/scenarios.rs @@ -11,9 +11,8 @@ fn can_resolve() -> Result<()> { let needle_fqdn = FQDN("example.nameservers.com.")?; let network = Network::new()?; - let peer = dns_test::peer(); - let mut leaf_ns = NameServer::new(&peer, FQDN::NAMESERVERS, &network)?; + let mut leaf_ns = NameServer::new(&dns_test::PEER, FQDN::NAMESERVERS, &network)?; leaf_ns.add(Record::a(needle_fqdn.clone(), expected_ipv4_addr)); let Graph { @@ -22,7 +21,7 @@ fn can_resolve() -> Result<()> { .. } = Graph::build(leaf_ns, Sign::No)?; - let resolver = Resolver::new(&network, root).start(&dns_test::subject())?; + let resolver = Resolver::new(&network, root).start(&dns_test::SUBJECT)?; let resolver_ip_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; @@ -47,9 +46,8 @@ fn nxdomain() -> Result<()> { let needle_fqdn = FQDN("unicorn.nameservers.com.")?; let network = Network::new()?; - let peer = dns_test::peer(); - let leaf_ns = NameServer::new(&peer, FQDN::NAMESERVERS, &network)?; + let leaf_ns = NameServer::new(&dns_test::PEER, FQDN::NAMESERVERS, &network)?; let Graph { nameservers: _nameservers, @@ -57,7 +55,7 @@ fn nxdomain() -> Result<()> { .. } = Graph::build(leaf_ns, Sign::No)?; - let resolver = Resolver::new(&network, root).start(&dns_test::subject())?; + let resolver = Resolver::new(&network, root).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 6bfbacf2..61244953 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 @@ -9,9 +9,9 @@ use dns_test::{Network, Resolver, Result, FQDN}; #[ignore] fn edns_support() -> Result<()> { let network = &Network::new()?; - let ns = NameServer::new(&dns_test::peer(), FQDN::ROOT, network)?.start()?; + let ns = NameServer::new(&dns_test::PEER, FQDN::ROOT, network)?.start()?; let resolver = Resolver::new(network, Root::new(ns.fqdn().clone(), ns.ipv4_addr())) - .start(&dns_test::subject())?; + .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 3e33ed90..b41db3a3 100644 --- a/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs +++ b/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs @@ -13,9 +13,8 @@ fn bad_signature_in_leaf_nameserver() -> Result<()> { let needle_fqdn = FQDN("example.nameservers.com.")?; let network = Network::new()?; - let peer = dns_test::peer(); - let mut leaf_ns = NameServer::new(&peer, FQDN::NAMESERVERS, &network)?; + let mut leaf_ns = NameServer::new(&dns_test::PEER, FQDN::NAMESERVERS, &network)?; leaf_ns.add(Record::a(needle_fqdn.clone(), expected_ipv4_addr)); let Graph { @@ -48,7 +47,7 @@ fn bad_signature_in_leaf_nameserver() -> Result<()> { let trust_anchor = &trust_anchor.unwrap(); let resolver = Resolver::new(&network, root) .trust_anchor(trust_anchor) - .start(&dns_test::subject())?; + .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/ede.rs b/packages/conformance-tests/src/resolver/dnssec/scenarios/ede.rs index ccae7178..8f9cdc2d 100644 --- a/packages/conformance-tests/src/resolver/dnssec/scenarios/ede.rs +++ b/packages/conformance-tests/src/resolver/dnssec/scenarios/ede.rs @@ -125,14 +125,14 @@ fn fixture( expected: ExtendedDnsError, amend: fn(needle_fqdn: &FQDN, zone: &FQDN, records: &mut Vec), ) -> Result<()> { - let subject = dns_test::subject(); + let subject = &dns_test::SUBJECT; let supports_ede = subject.supports_ede(); let expected_ipv4_addr = Ipv4Addr::new(1, 2, 3, 4); let needle_fqdn = FQDN("example.nameservers.com.")?; let network = Network::new()?; - let mut leaf_ns = NameServer::new(&dns_test::peer(), FQDN::NAMESERVERS, &network)?; + let mut leaf_ns = NameServer::new(&dns_test::PEER, FQDN::NAMESERVERS, &network)?; leaf_ns.add(Record::a(needle_fqdn.clone(), expected_ipv4_addr)); let Graph { @@ -153,7 +153,7 @@ fn fixture( } let trust_anchor = &trust_anchor.unwrap(); - let resolver = resolver.trust_anchor(trust_anchor).start(&subject)?; + let resolver = resolver.trust_anchor(trust_anchor).start(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 9a0b2515..bf076c0b 100644 --- a/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs +++ b/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs @@ -11,7 +11,7 @@ use dns_test::{Network, Resolver, Result, TrustAnchor, FQDN}; #[test] fn can_validate_without_delegation() -> Result<()> { let network = Network::new()?; - let mut ns = NameServer::new(&dns_test::peer(), FQDN::ROOT, &network)?; + let mut ns = NameServer::new(&dns_test::PEER, FQDN::ROOT, &network)?; ns.add(Record::a(ns.fqdn().clone(), ns.ipv4_addr())); let ns = ns.sign()?; @@ -27,7 +27,7 @@ fn can_validate_without_delegation() -> Result<()> { 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())?; + .start(&dns_test::SUBJECT)?; let resolver_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; @@ -49,10 +49,9 @@ fn can_validate_with_delegation() -> Result<()> { let expected_ipv4_addr = Ipv4Addr::new(1, 2, 3, 4); let needle_fqdn = FQDN("example.nameservers.com.")?; - let peer = dns_test::peer(); let network = Network::new()?; - let mut leaf_ns = NameServer::new(&peer, FQDN::NAMESERVERS, &network)?; + let mut leaf_ns = NameServer::new(&dns_test::PEER, FQDN::NAMESERVERS, &network)?; leaf_ns.add(Record::a(needle_fqdn.clone(), expected_ipv4_addr)); let Graph { @@ -64,7 +63,7 @@ fn can_validate_with_delegation() -> Result<()> { let trust_anchor = &trust_anchor.unwrap(); let resolver = Resolver::new(&network, root) .trust_anchor(trust_anchor) - .start(&dns_test::subject())?; + .start(&dns_test::SUBJECT)?; let resolver_addr = resolver.ipv4_addr(); let client = Client::new(&network)?; diff --git a/packages/dns-test/Cargo.toml b/packages/dns-test/Cargo.toml index eacbbf9c..2d595812 100644 --- a/packages/dns-test/Cargo.toml +++ b/packages/dns-test/Cargo.toml @@ -6,6 +6,7 @@ publish = false version = "0.1.0" [dependencies] +lazy_static = "1.4.0" minijinja = "1.0.12" serde = { version = "1.0.196", features = ["derive"] } serde_json = "1.0.113" diff --git a/packages/dns-test/examples/explore.rs b/packages/dns-test/examples/explore.rs index 6e8de222..277e16b9 100644 --- a/packages/dns-test/examples/explore.rs +++ b/packages/dns-test/examples/explore.rs @@ -8,16 +8,16 @@ use dns_test::{Network, Resolver, Result, TrustAnchor, FQDN}; fn main() -> Result<()> { let network = Network::new()?; - let peer = dns_test::peer(); + let peer = &dns_test::PEER; println!("building docker image..."); - let mut root_ns = NameServer::new(&peer, FQDN::ROOT, &network)?; + let mut root_ns = NameServer::new(peer, FQDN::ROOT, &network)?; println!("DONE"); println!("setting up name servers..."); - let mut com_ns = NameServer::new(&peer, FQDN::COM, &network)?; + let mut com_ns = NameServer::new(peer, FQDN::COM, &network)?; - let mut nameservers_ns = NameServer::new(&peer, FQDN("nameservers.com.")?, &network)?; + let mut nameservers_ns = NameServer::new(peer, FQDN("nameservers.com.")?, &network)?; nameservers_ns .add(Record::a(root_ns.fqdn().clone(), root_ns.ipv4_addr())) .add(Record::a(com_ns.fqdn().clone(), com_ns.ipv4_addr())); @@ -53,7 +53,7 @@ fn main() -> Result<()> { Root::new(root_ns.fqdn().clone(), root_ns.ipv4_addr()), ) .trust_anchor(&trust_anchor) - .start(&dns_test::subject())?; + .start(&dns_test::SUBJECT)?; println!("DONE\n\n"); let resolver_addr = resolver.ipv4_addr(); diff --git a/packages/dns-test/src/implementation.rs b/packages/dns-test/src/implementation.rs index 08465d45..9cac8786 100644 --- a/packages/dns-test/src/implementation.rs +++ b/packages/dns-test/src/implementation.rs @@ -34,7 +34,7 @@ pub enum Role { Resolver, } -#[derive(Clone)] +#[derive(Clone, Debug)] pub enum Implementation { Bind, Hickory(Repository<'static>), @@ -178,7 +178,7 @@ impl fmt::Display for Implementation { } } -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Repository<'a> { inner: Cow<'a, str>, } diff --git a/packages/dns-test/src/lib.rs b/packages/dns-test/src/lib.rs index cfbcb96f..afc3773d 100644 --- a/packages/dns-test/src/lib.rs +++ b/packages/dns-test/src/lib.rs @@ -1,5 +1,9 @@ //! A test framework for all things DNS +use std::env; + +use lazy_static::lazy_static; + pub use crate::container::Network; pub use crate::fqdn::FQDN; pub use crate::implementation::{Implementation, Repository}; @@ -23,8 +27,13 @@ pub type Result = core::result::Result; // TODO maybe this should be a TLS variable that each unit test (thread) can override const DEFAULT_TTL: u32 = 24 * 60 * 60; // 1 day -pub fn subject() -> Implementation { - if let Ok(subject) = std::env::var("DNS_TEST_SUBJECT") { +lazy_static! { + pub static ref SUBJECT: Implementation = parse_subject(); + pub static ref PEER: Implementation = parse_peer(); +} + +fn parse_subject() -> Implementation { + if let Ok(subject) = env::var("DNS_TEST_SUBJECT") { if subject == "unbound" { return Implementation::Unbound; } @@ -47,14 +56,58 @@ pub fn subject() -> Implementation { } } -pub fn peer() -> Implementation { - if let Ok(subject) = std::env::var("DNS_TEST_PEER") { - match subject.as_str() { +fn parse_peer() -> Implementation { + if let Ok(peer) = env::var("DNS_TEST_PEER") { + match peer.as_str() { "unbound" => Implementation::Unbound, "bind" => Implementation::Bind, - _ => panic!("`{subject}` is not supported as a test peer implementation"), + _ => panic!("`{peer}` is not supported as a test peer implementation"), } } else { Implementation::default() } } + +#[cfg(test)] +mod tests { + use std::env; + + use super::*; + + impl PartialEq for Implementation { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::Hickory(_), Self::Hickory(_)) => true, + _ => core::mem::discriminant(self) == core::mem::discriminant(other), + } + } + } + + #[test] + fn immutable_subject() { + let before = super::SUBJECT.clone(); + let newval = if before == Implementation::Unbound { + "bind" + } else { + "unbound" + }; + env::set_var("DNS_TEST_SUBJECT", newval); + + let after = super::SUBJECT.clone(); + assert_eq!(before, after); + } + + #[test] + fn immutable_peer() { + let before = super::PEER.clone(); + let newval = if before == Implementation::Unbound { + "bind" + } else { + "unbound" + }; + env::set_var("DNS_TEST_PEER", newval); + + let after = super::PEER.clone(); + assert_eq!(before, after); + } +}