From 6145be410869442ee0c6a6fbca3ebbc734904a8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jagoda=20Estera=20=C5=9Al=C4=85zak?= <128227338+j-g00da@users.noreply.github.com> Date: Mon, 23 Feb 2026 19:44:30 +0100 Subject: [PATCH] fix(dkim): Accept TXT records with no `v=` tag (#62) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cache at most 3 TXT records instead of trying to find one that includes `DKIM`. Closes #59 Signed-off-by: Jagoda Ślązak --- filtermail/src/dkim_verifier.rs | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/filtermail/src/dkim_verifier.rs b/filtermail/src/dkim_verifier.rs index d7fd03a6..00452667 100644 --- a/filtermail/src/dkim_verifier.rs +++ b/filtermail/src/dkim_verifier.rs @@ -21,7 +21,7 @@ struct CachedResolver { // Note: Arc is required despite we are holding the whole handler in an Arc, // because viadkim will internally clone the resolver (LookupTxt + Clone + 'static) // to parallelize lookups in case of multiple signatures... - cache: Arc>>>, + cache: Arc>>>>, } impl CachedResolver { @@ -76,41 +76,39 @@ impl LookupTxt for CachedResolver { { let mut cache = self.cache.lock(); - if let Some(rdata) = cache.get(&name) { - let txts: Self::Answer = Box::new(std::iter::once(Ok(rdata.clone()))); - log::debug!("Using cached RDATA for {}", name); + if let Some(txts) = cache.get(&name) { + let txts: Self::Answer = Box::new(txts.clone().into_iter().map(Ok)); + log::debug!("Using cached TXT records for {}", name); return Ok(txts); } } - log::debug!("Trying to resolve TXT for {}", name); - let txt = { + log::debug!("Trying to resolve TXT records for {}", name); + let txts: Vec> = { let lookup = self.dns_resolver.txt_lookup(name.clone()).await?; // viadkim would filter out non-DKIM TXT records, // but we filter it here anyway so that we know which one should be cached. lookup .into_iter() - .find_map(|txt| { + // We don't check all records, as this can be a DoS attack vector. + // In theory, selector domains should only have a single TXT record. + // In practice, we check at most 3, just in case of weird configuration. + .take(3) + .map(|txt| { let rdata_raw = txt.txt_data().concat(); let rdata = String::from_utf8_lossy(&rdata_raw); - // naive check, but this can't be an attack vector, - // and selector domain should only have a single TXT record anyway. - if rdata.contains("DKIM") { - Some(Self::normalize_rdata(&rdata).into_bytes()) - } else { - None - } + Self::normalize_rdata(&rdata).into_bytes() }) - .ok_or(io::ErrorKind::NotFound)? + .collect() }; { let mut cache = self.cache.lock(); - cache.put(name, txt.clone()); + cache.put(name, txts.clone()); } - let txts: Self::Answer = Box::new(std::iter::once(Ok(txt))); + let txts: Self::Answer = Box::new(txts.into_iter().map(Ok)); Ok(txts) }) }