Fix MTA: Inbound DMARC and TLS aggregate reports that a reporter sends more than once are imported again as a duplicate entry

CHANGELOG.md

@@ -18,6 +18,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If
   - EventSource `ping` events advertise the interval in seconds rather than milliseconds.
 - IMAP: Every command in a pipelined `STATUS` or `FETCH` batch receives its tagged completion, instead of the first failing command dropping the responses for all commands queued behind it.
 - WebDAV: Accounts without a storage quota no longer advertise a 4 GiB limit in `DAV:quota-available-bytes`.
+- MTA: Inbound DMARC and TLS aggregate reports that a reporter sends more than once are imported again as a duplicate entry.
 - iTIP: Detaching an occurrence that the recurrence rule already generates is sent as a `METHOD:REQUEST` carrying the `RECURRENCE-ID` instead of a `METHOD:ADD`.
 - Sieve: `fileinto :specialuse` and `specialuse_exists` accept special-use attributes in the `\Trash` form.
 - LDAP: Active Directory servers that answer an unauthenticated bind (a non-empty DN with a zero-length password) with success no longer authenticate accounts without a password.

crates/common/src/config/mailstore/scripts.rs

@@ -135,7 +135,6 @@ impl Scripting {
             .with_max_variable_size(trusted.max_var_size as usize)
             .with_max_header_size(10240)
             .with_valid_notification_uri("mailto")
-            //.with_valid_ext_lists(stores.in_memory_stores.keys().map(|k| k.to_string()))
             .with_functions(&mut fnc_map_trusted)
             .with_max_redirects(trusted.max_redirects as usize)
             .with_max_out_messages(trusted.max_out_messages as usize)
@@ -222,19 +221,19 @@ impl Scripting {
                 .unwrap_or_default(),
             max_received_headers: untrusted.max_received_headers as usize,
             from_addr: bp.compile_expr(
-                ObjectType::SieveSystemScript.singleton(),
+                ObjectType::SieveSystemInterpreter.singleton(),
                 &trusted.ctx_default_from_address(),
             ),
             from_name: bp.compile_expr(
-                ObjectType::SieveSystemScript.singleton(),
+                ObjectType::SieveSystemInterpreter.singleton(),
                 &trusted.ctx_default_from_name(),
             ),
             return_path: bp.compile_expr(
-                ObjectType::SieveSystemScript.singleton(),
+                ObjectType::SieveSystemInterpreter.singleton(),
                 &trusted.ctx_default_return_path(),
             ),
             sign: bp.compile_expr(
-                ObjectType::SieveSystemScript.singleton(),
+                ObjectType::SieveSystemInterpreter.singleton(),
                 &trusted.ctx_dkim_sign_domain(),
             ),
             untrusted_sign,

crates/smtp/src/reporting/analysis.rs

@@ -341,7 +341,9 @@ impl AnalyzeReport for Server {
                         }
                     }
 
-                    if let Err(err) = core.core.storage.data.write(batch.build_all()).await {
+                    if let Err(err) = core.core.storage.data.write(batch.build_all()).await
+                        && !err.is_assertion_failure()
+                    {
                         trc::error!(
                             err.span_id(session_id)
                                 .caused_by(trc::location!())

crates/smtp/src/reporting/index.rs

@@ -13,7 +13,12 @@ use registry::{
             TaskStatus, TaskTlsReport, TlsExternalReport, TlsInternalReport,
         },
     },
-    types::{EnumImpl, ObjectImpl, datetime::UTCDateTime, id::ObjectId, index::IndexBuilder},
+    types::{
+        EnumImpl, ObjectImpl,
+        datetime::UTCDateTime,
+        id::ObjectId,
+        index::{IndexBuilder, IndexValue},
+    },
 };
 use store::{
     SerializeInfallible, U64_LEN,
@@ -22,6 +27,7 @@ use store::{
         BatchBuilder, RegistryClass, TaskQueueClass, ValueClass, assert::AssertValue,
         key::KeySerializer,
     },
+    xxhash_rust::xxh3::Xxh3,
 };
 use types::id::Id;
 
@@ -111,6 +117,8 @@ pub trait ExternalReportIndex: ObjectImpl {
 
     fn success_fail_count(&self) -> (u64, u64);
 
+    fn unique_key(&self) -> Option<[u8; 16]>;
+
     fn write_ops(&self, batch: &mut BatchBuilder, item_id: u64, is_set: bool) {
         let object_id = Self::OBJECT.to_id();
         let mut index_builder = IndexBuilder::default();
@@ -127,6 +135,11 @@ pub trait ExternalReportIndex: ObjectImpl {
         index_builder.search(Property::TotalFailedSessions, fail_count);
 
         index_builder.search(Property::ExpiresAt, self.expires_at());
+
+        if let Some(unique_key) = self.unique_key() {
+            index_builder.unique(Property::ReportId, IndexValue::Bytes(unique_key.to_vec()));
+        }
+
         batch.registry_index(object_id, item_id, index_builder.keys.iter(), is_set);
 
         let key = ValueClass::Registry(RegistryClass::Item { object_id, item_id });
@@ -239,6 +252,10 @@ impl ExternalReportIndex for ArfExternalReport {
     fn success_fail_count(&self) -> (u64, u64) {
         (self.report.incidents, 0)
     }
+
+    fn unique_key(&self) -> Option<[u8; 16]> {
+        None
+    }
 }
 
 impl ExternalReportIndex for DmarcExternalReport {
@@ -292,6 +309,20 @@ impl ExternalReportIndex for DmarcExternalReport {
 
         (success_count, fail_count)
     }
+
+    fn unique_key(&self) -> Option<[u8; 16]> {
+        let report = &self.report;
+
+        Some(report_key(
+            [
+                report.org_name.as_str(),
+                report.policy_domain.as_str(),
+                report.report_id.as_str(),
+            ],
+            report.date_range_begin,
+            report.date_range_end,
+        ))
+    }
 }
 
 impl ExternalReportIndex for TlsExternalReport {
@@ -342,6 +373,32 @@ impl ExternalReportIndex for TlsExternalReport {
 
         (success_count, fail_count)
     }
+
+    fn unique_key(&self) -> Option<[u8; 16]> {
+        let report = &self.report;
+
+        Some(report_key(
+            [
+                report.organization_name.as_deref().unwrap_or_default(),
+                report.report_id.as_str(),
+            ],
+            report.date_range_start,
+            report.date_range_end,
+        ))
+    }
+}
+
+fn report_key<const N: usize>(fields: [&str; N], from: UTCDateTime, to: UTCDateTime) -> [u8; 16] {
+    let mut hasher = Xxh3::new();
+
+    for field in fields {
+        hasher.update(field.as_bytes());
+        hasher.update(&[0u8]);
+    }
+    hasher.update(&(from.timestamp() as u64).to_be_bytes());
+    hasher.update(&(to.timestamp() as u64).to_be_bytes());
+
+    hasher.digest128().to_be_bytes()
 }
 
 #[inline(always)]

tests/src/smtp/reporting/analyze.rs

@@ -230,6 +230,30 @@ async fn report_analyze() {
         1
     );
 
+    // Redeliveries of a previously stored report must not be imported again
+    session
+        .send_message(
+            "john@test.org",
+            &["reports@foobar.org"],
+            &report_message(
+                "application/zip",
+                &format!("{attachment_name}.zip"),
+                &zip("report.xml", DMARC_REPORT.as_bytes(), None, None),
+            ),
+            "250",
+        )
+        .await;
+    test.assert_no_events();
+
+    let admin = test.account("admin");
+    for _ in 0..10 {
+        tokio::time::sleep(Duration::from_millis(100)).await;
+        assert_eq!(
+            admin.registry_get_all::<DmarcExternalReport>().await.len(),
+            1
+        );
+    }
+
     // Test delivery to non-report addresses
     session
         .send_message("john@test.org", &["bill@foobar.org"], "test:no_dkim", "250")