diff --git a/crates/temps-email/src/dns.rs b/crates/temps-email/src/dns.rs index 48df271f2..af1128b2e 100644 --- a/crates/temps-email/src/dns.rs +++ b/crates/temps-email/src/dns.rs @@ -57,10 +57,15 @@ impl DnsVerifier { debug!("Found TXT record: {}", txt_data); - // Check if the expected value is contained in the TXT record - // For DKIM, we check if the CNAME target matches - // For SPF, we check if the value contains the expected include - if txt_data.contains(expected_value) || expected_value.contains(&txt_data) { + // Only check that the published record contains the expected value + // (DKIM keys/SPF includes are checked as substrings since registrars + // sometimes wrap them with extra text). The reverse check — + // `expected_value.contains(&txt_data)` — used to also be accepted, + // which is a false-positive trap: any short, unrelated TXT record + // (e.g. a single stray character) is trivially a substring of a long + // expected DKIM key, so a domain could be reported "Verified" with a + // DNS record that has nothing to do with the expected value. + if txt_data.contains(expected_value) { return DnsRecordStatus::Verified; } } @@ -207,6 +212,44 @@ impl DnsVerifier { } } } + + /// Verify a DMARC policy record: a `_dmarc.` TXT record starting + /// with `v=DMARC1`. Unlike SPF/DKIM/MX, no provider API publishes or + /// manages this — it's a plain DNS record the domain owner sets + /// independently — so callers generate the expected record themselves + /// and use this purely to check whether it's actually live. + pub async fn verify_dmarc_record(&self, domain: &str) -> DnsRecordStatus { + let name = format!("_dmarc.{}", domain); + debug!("Verifying DMARC record: {}", name); + + match self.resolver.txt_lookup(&name).await { + Ok(lookup) => { + for record in lookup.answers() { + let RData::TXT(txt) = &record.data else { + continue; + }; + let txt_data: String = txt + .txt_data + .iter() + .map(|data| String::from_utf8_lossy(data).to_string()) + .collect(); + + debug!("Found TXT record: {}", txt_data); + + if txt_data.starts_with("v=DMARC1") { + return DnsRecordStatus::Verified; + } + } + + debug!("No matching DMARC record found for {}", name); + DnsRecordStatus::Pending + } + Err(e) => { + debug!("DMARC lookup failed for {}: {}", name, e); + DnsRecordStatus::Pending + } + } + } } #[cfg(test)] diff --git a/crates/temps-email/src/handlers/domains.rs b/crates/temps-email/src/handlers/domains.rs index caa1cebf4..ec174d205 100644 --- a/crates/temps-email/src/handlers/domains.rs +++ b/crates/temps-email/src/handlers/domains.rs @@ -587,9 +587,25 @@ pub async fn setup_dns( let email_domain = &domain_with_dns.domain.domain; let base_domain = extract_base_domain(email_domain); + // Create each DNS record — except DMARC. Unlike SPF/DKIM/MX, DMARC isn't + // additive: publishing `_dmarc.` sets a `p=quarantine` + // policy for the *entire* domain, which can affect mail from senders + // other than Temps (e.g. the company's regular Google Workspace/M365 + // mail) if their SPF/DKIM alignment isn't already clean. Bundling that + // into the same "create all records" click as the purely-additive + // records would be exactly the kind of silent-on-the-user's-behalf + // change CLAUDE.md's operator-control rule warns against, so DMARC stays + // informational-only here — surfaced for the operator to add manually + // once they've confirmed it's safe for their domain. + let auto_creatable_records: Vec<_> = domain_with_dns + .dns_records + .iter() + .filter(|r| !r.name.starts_with("_dmarc.")) + .collect(); + info!( "Setting up {} DNS records for {} using provider {}", - domain_with_dns.dns_records.len(), + auto_creatable_records.len(), email_domain, dns_provider.name ); @@ -597,8 +613,7 @@ pub async fn setup_dns( let mut results = Vec::new(); let mut records_created: u32 = 0; - // Create each DNS record - for dns_record in &domain_with_dns.dns_records { + for dns_record in auto_creatable_records { let result = create_dns_record(provider_instance.as_ref(), &base_domain, dns_record).await; if result.success { @@ -608,7 +623,7 @@ pub async fn setup_dns( results.push(result); } - let total_records = domain_with_dns.dns_records.len() as u32; + let total_records = results.len() as u32; let all_success = records_created == total_records; let message = if all_success { diff --git a/crates/temps-email/src/services/domain_service.rs b/crates/temps-email/src/services/domain_service.rs index 45a73fbf7..5193c56f5 100644 --- a/crates/temps-email/src/services/domain_service.rs +++ b/crates/temps-email/src/services/domain_service.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use temps_entities::email_domains; use tracing::{debug, error, info, warn}; +use crate::dns::DnsVerifier; use crate::errors::EmailError; use crate::providers::{DnsRecord, DnsRecordStatus, DomainIdentityDetails, VerificationStatus}; use crate::services::ProviderService; @@ -82,6 +83,8 @@ impl DomainService { dns_records.push(mx.clone()); } + dns_records.push(Self::dmarc_record_template(&request.domain)); + // Store domain in database let domain = email_domains::ActiveModel { provider_id: Set(request.provider_id), @@ -167,7 +170,9 @@ impl DomainService { records.push(mx); } - // Compute status based on all DNS records being verified + // Compute status based on all DNS records being verified. + // DMARC deliberately isn't part of this — see + // `dmarc_record_template`'s doc comment. let all_verified = Self::are_all_records_verified(&identity_details); let status = if all_verified { "verified".to_string() @@ -181,6 +186,8 @@ impl DomainService { } }; + records.push(Self::dmarc_record_live(&domain.domain).await); + (records, status) } None => { @@ -348,7 +355,8 @@ impl DomainService { .iter() .any(|r| r.status == DnsRecordStatus::Failed); - // Determine final status based on DNS record verification + // Determine final status based on DNS record verification. DMARC is + // intentionally excluded — see `dmarc_record_template`'s doc comment. let status = if all_dns_verified { debug!("All DNS records verified via DNS lookup, marking domain as verified"); VerificationStatus::Verified @@ -359,6 +367,8 @@ impl DomainService { identity_details.overall_status }; + dns_records.push(Self::dmarc_record_live(&domain.domain).await); + // Update domain status in database let mut active_model: email_domains::ActiveModel = domain.into(); @@ -431,6 +441,34 @@ impl DomainService { Ok(()) } + /// The recommended DMARC policy record for a domain. Unlike SPF/DKIM/MX, + /// no provider API publishes or manages DMARC — it's a plain DNS TXT + /// record the domain owner sets independently — so it's generated here + /// rather than surfaced by `EmailProvider::get_identity_details`, and + /// deliberately left out of `are_all_records_verified`/failure checks: + /// requiring it would regress every already-verified domain the next + /// time its status is recomputed. It's informational, not required. + fn dmarc_record_template(domain: &str) -> DnsRecord { + DnsRecord { + record_type: "TXT".to_string(), + name: format!("_dmarc.{}", domain), + // Quarantine (not reject) by default — a safe starting policy + // that won't silently drop mail for a domain that hasn't + // fully tuned SPF/DKIM alignment yet. + value: "v=DMARC1; p=quarantine; pct=100".to_string(), + priority: None, + status: DnsRecordStatus::Pending, + } + } + + /// DMARC record with its live-verified status, for paths that already + /// do a live DNS lookup for SPF/DKIM/MX. + async fn dmarc_record_live(domain: &str) -> DnsRecord { + let mut record = Self::dmarc_record_template(domain); + record.status = DnsVerifier::new().verify_dmarc_record(domain).await; + record + } + /// Build DNS records from stored domain data (fallback when provider API unavailable) fn build_dns_records(&self, domain: &email_domains::Model) -> Vec { let mut records = Vec::new(); @@ -468,6 +506,10 @@ impl DomainService { }); } + let mut dmarc = Self::dmarc_record_template(&domain.domain); + dmarc.status = DnsRecordStatus::Unknown; + records.push(dmarc); + records } } @@ -642,7 +684,7 @@ mod tests { let records = service.build_dns_records(&domain); - assert_eq!(records.len(), 3); // SPF, DKIM, MX + assert_eq!(records.len(), 4); // SPF, DKIM, MX, DMARC // Verify SPF record let spf = records @@ -659,6 +701,12 @@ mod tests { let mx = records.iter().find(|r| r.record_type == "MX"); assert!(mx.is_some()); assert_eq!(mx.unwrap().priority, Some(10)); + + // Verify DMARC record — always generated, unlike the provider-supplied + // records above, since no provider API manages it. + let dmarc = records.iter().find(|r| r.name == "_dmarc.example.com"); + assert!(dmarc.is_some()); + assert!(dmarc.unwrap().value.starts_with("v=DMARC1")); } // ========== Integration Tests (require Docker) ==========