From 42f739f00ffd98d11469184b231806092f2af4e9 Mon Sep 17 00:00:00 2001 From: Erik Hollensbe Date: Sun, 25 Jul 2021 22:01:53 -0700 Subject: [PATCH 1/3] When replacing the record, don't try to edit it This bug is caused by the following scenario: - Create two members, each with a name like `test1` and `test2`. - Rename it so that `test1` is now `test2`, and `test2` is now `test1` - Now, after zeronsd updates, each record will have two ip addresses, both the same list. This was caused by the rename causing it being added to a whitelist where it was edited, not replaced, therefore appending to the list. This was a design mistake in general and some additional work will be done to correct this in a refactoring pass. The new behavior is to replace the recordset entirely, which is probably much more correct in DNS terms anyway. Signed-off-by: Erik Hollensbe --- src/authority.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/authority.rs b/src/authority.rs index b9ef9e6..578f465 100644 --- a/src/authority.rs +++ b/src/authority.rs @@ -20,7 +20,7 @@ use trust_dns_resolver::{ use trust_dns_server::{ authority::{AuthorityObject, Catalog}, - client::rr::{Name, RData, Record, RecordType, RrKey}, + client::rr::{LowerName, Name, RData, Record, RecordType, RrKey}, store::forwarder::ForwardAuthority, store::{forwarder::ForwardConfig, in_memory::InMemoryAuthority}, }; @@ -162,17 +162,10 @@ fn upsert_address( ) { let serial = authority.serial() + 1; let records = authority.records_mut(); - let key = records - .into_iter() - .map(|(key, _)| key) - .find(|key| key.name().into_name().unwrap().eq(&fqdn)); - - if let Some(key) = key { - let key = key.clone(); - records - .get_mut(&key) - .replace(&mut Arc::new(RecordSet::new(&fqdn.clone(), rt, serial))); - } + let key = RrKey::new(LowerName::from(fqdn.clone()), rt); + + let mut inner = RecordSet::new(&fqdn.clone(), rt, serial); + records.remove(&key); for rdata in rdatas { if match rt { @@ -195,9 +188,11 @@ fn upsert_address( let mut address = Record::with(fqdn.clone(), rt, 60); address.set_rdata(rdata.clone()); info!("Adding new record {}: ({})", fqdn.clone(), rdata.clone()); - authority.upsert(address, serial); + inner.insert(address, serial); } } + + records.insert(key, Arc::new(inner)); } fn set_ptr_record( @@ -228,7 +223,7 @@ fn set_ptr_record( ); } -fn set_ip_record( +fn replace_ip_record( authority: &mut RwLockWriteGuard, name: Name, rt: RecordType, @@ -559,7 +554,7 @@ impl ZTAuthority { || !records.into_iter().all(|r| rdatas.contains(r.rdata())) { drop(lock); - set_ip_record( + replace_ip_record( &mut self.authority.write().expect("write lock"), name.clone(), rt, @@ -569,7 +564,7 @@ impl ZTAuthority { } None => { drop(lock); - set_ip_record( + replace_ip_record( &mut self.authority.write().expect("write lock"), name.clone(), rt, From 259a1f9e20deaced96dccba20466a20d4e19fcd6 Mon Sep 17 00:00:00 2001 From: Erik Hollensbe Date: Mon, 26 Jul 2021 11:54:05 -0700 Subject: [PATCH 2/3] Makefile: some changes to make skipping tests easier Signed-off-by: Erik Hollensbe --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index bef3244..9e7874c 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,11 @@ test: cargo test test-integration: +ifneq (${SKIP},) + TOKEN=$$(cat test-token.txt) sudo -E bash -c "$$(which cargo) test --features integration-tests -j1 -- --skip '${SKIP}' --nocapture --test-threads 1" +else TOKEN=$$(cat test-token.txt) sudo -E bash -c "$$(which cargo) test --features integration-tests -j1 -- --nocapture --test-threads 1" +endif generate: central service From 24b6e0ea30c93d4be605bdac458125f138ac568b Mon Sep 17 00:00:00 2001 From: Erik Hollensbe Date: Mon, 26 Jul 2021 12:05:34 -0700 Subject: [PATCH 3/3] hosts: replace at the last second; do not overwrite ips with empty list Signed-off-by: Erik Hollensbe --- src/authority.rs | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/authority.rs b/src/authority.rs index 578f465..773618d 100644 --- a/src/authority.rs +++ b/src/authority.rs @@ -164,7 +164,6 @@ fn upsert_address( let records = authority.records_mut(); let key = RrKey::new(LowerName::from(fqdn.clone()), rt); - let mut inner = RecordSet::new(&fqdn.clone(), rt, serial); records.remove(&key); for rdata in rdatas { @@ -188,11 +187,9 @@ fn upsert_address( let mut address = Record::with(fqdn.clone(), rt, 60); address.set_rdata(rdata.clone()); info!("Adding new record {}: ({})", fqdn.clone(), rdata.clone()); - inner.insert(address, serial); + authority.upsert(address, serial); } } - - records.insert(key, Arc::new(inner)); } fn set_ptr_record( @@ -533,12 +530,13 @@ impl ZTAuthority { .authority .read() .expect("Could not get authority read lock"); - let records = lock + + let rs = lock .records() .get(&RrKey::new(name.clone().into(), rt)) .clone(); - let ips = newips + let ips: Vec = newips .clone() .into_iter() .filter(|i| match i { @@ -547,13 +545,26 @@ impl ZTAuthority { }) .collect(); - match records { - Some(records) => { - let records = records.records(false, SupportedAlgorithms::all()); + match rs { + Some(rs) => { + let records = rs.records(false, SupportedAlgorithms::all()); if records.is_empty() || !records.into_iter().all(|r| rdatas.contains(r.rdata())) { drop(lock); + if !ips.is_empty() { + replace_ip_record( + &mut self.authority.write().expect("write lock"), + name.clone(), + rt, + ips, + ); + } + } + } + None => { + drop(lock); + if !ips.is_empty() { replace_ip_record( &mut self.authority.write().expect("write lock"), name.clone(), @@ -562,15 +573,6 @@ impl ZTAuthority { ); } } - None => { - drop(lock); - replace_ip_record( - &mut self.authority.write().expect("write lock"), - name.clone(), - rt, - ips, - ); - } } } }