[pmg-devel] [PATCH pmg-api v2] prefix message-id in attachment-quarantine

[Stoiko Ivanov]

This patch fixes #2785.

When using the attachment quarantine - the message is:
a) stored in the quarantine unaltered
b) sent on with the attachment removed

Currently we do not change the message in any other way - in particular
we do not change the message-id header of any of the 2 mails.

When a mail is released from the attachment quarantine it is sent by PMG
with the same message-id as the mail with the attachments removed.

This is a violation of RFC 5322 (see [0]), and additionally newer versions
of Exchange do accept 2 mails with the same message-id but silently discard
the second version, thus making the attachment quarantine unusable for
Exchange users.

This patch simply prefixes the message-id with 'pmg-aquar-$$' (where $$ is
the pid of the pmg-smtp-filter process) for the mail without attachment.

By keeping the original message-id in the headers tracing the mailflow should
be facilitated.

The Message-ID is left intact on the original message in order to keep DKIM
signatures valid (they are invalidated on the modified mail by the removal
of the attachment anyways).

[0] https://tools.ietf.org/html/rfc5322 (section 3.6.4)

Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
---
v1 -> v2:
* changed the patch to prefix the existing message_id instead of simply
  removing it, based on Thomas' suggestion, that it helps in tracing the
  mail (Thanks!)
* tested with message-ids without '<','>' and with a mail without a
  message-id (`swaks` is quite nice for these edge-cases where postfix inists
  on following sane SMTP behaviour)

 src/PMG/RuleDB/Remove.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/PMG/RuleDB/Remove.pm b/src/PMG/RuleDB/Remove.pm
index e57c216..6b27b91 100644
--- a/src/PMG/RuleDB/Remove.pm
+++ b/src/PMG/RuleDB/Remove.pm
@@ -224,6 +224,14 @@ sub execute {
 	    my $original_entity = $entity->dup;
 	    PMG::Utils::remove_marks($original_entity);
 	    if (my $qid = $queue->quarantine_mail($ruledb, 'A', $original_entity, $tg, $msginfo, $vars, $ldap)) {
+		# adapt the Message-ID header of the mail without attachment to
+		# prevent 2 different mails with the same Message-ID
+		my $message_id = $entity->head->get('Message-ID');
+		if (defined($message_id)) {
+		    $message_id =~ s/^(<?)(.+)(>?)$/$1pmg-aquar-$$-$2$3/;
+		    $entity->head->replace('Message-ID', $message_id);
+		}
+
 		foreach (@$tg) {
 		    syslog ('info', "$queue->{logid}: moved mail for <%s> to attachment quarantine - %s (rule: %s)", $_, $qid, $rulename);
 		}
-- 
2.20.1