Merge pull request #1898 from ikedas/issue-1852 by ikedas

DKIM-Signature header fields should not be removed even if invalid (#1852)

src/lib/Sympa/Config/Schema.pm

@@ -4888,6 +4888,19 @@ our %pinfo = (
         not_after => '6.2.56',
     },
 
+    remove_dkim_headers => {
+        context    => [qw(list domain site)],
+        order      => 70.08,
+        group      => 'dkim',
+        gettext_id => 'Remove DKIM signatures in incoming messages',
+        gettext_comment =>
+            'Normally this should be turned off. It can be turned on when DKIM signatures that cannot be verified at the recipient site cause problems.',
+        format     => ['on', 'off'],
+        occurrence => '1',
+        default    => 'off',
+        not_before => '6.2.74',
+    },
+
     ### Optional features
 
     ### List address verification

src/lib/Sympa/Message.pm

@@ -852,20 +852,8 @@ sub check_arc_seals {
 
 # Old name: tools::remove_invalid_dkim_signature() which takes a message as
 # string and outputs idem without signature if invalid.
-sub remove_invalid_dkim_signature {
-    $log->syslog('debug2', '(%s)', @_);
-    my $self = shift;
-
-    return unless $self->get_header('DKIM-Signature');
-
-    my ($dkim_pass, @dummy) = $self->check_dkim_sigs;
-    unless ($dkim_pass) {
-        $log->syslog('info',
-            'DKIM signature of message %s is invalid, removing', $self);
-        $self->delete_header('DKIM-Signature');
-        delete $self->{dkim_pass};
-    }
-}
+# Deprecated.
+#sub remove_invalid_dkim_signature;
 
 sub as_entity {
     my $self = shift;
@@ -3848,6 +3836,8 @@ An array of the overall result of checking and authentication result(s).
 
 =item remove_invalid_dkim_signature ( )
 
+B<Deprecated> on Sympa 6.2.74.
+
 I<Instance method>.
 Verifies DKIM signatures included in the message,
 and if any of them are invalid, removes them.

src/lib/Sympa/Spindle/ProcessOutgoing.pm

@@ -245,8 +245,13 @@ sub __twist_one {
         }
     }
 
-    $message->remove_invalid_dkim_signature
-        if $rm_sig;
+    if ($rm_sig) {
+        # If it is set up, remove header fields related to DKIM signature
+        # given by upstream MTAs.
+        # AR should be removed after it is included into AAR: See below.
+        $message->delete_header('DKIM-Signature');
+        $message->delete_header('Domainkey-Signature');
+    }
 
     if ($message->{shelved}{dkim_sign} or %arc) {
         # apply DKIM signature AFTER any other message transformation.
@@ -256,6 +261,10 @@ sub __twist_one {
     # DKIM signing must be done before ARC sealing. See RFC 8617, 5.1.
     $message->arc_seal(%arc) if %arc;
 
+    if ($rm_sig) {
+        $message->delete_header('Authentication-Results');
+    }
+
     # Determine envelope sender and envelope ID.
     my $envid = undef;
     if ($tracking) {
@@ -304,24 +313,27 @@ sub _twist {
     my $message = shift;
 
     # Get list/robot context.
-    my ($list, $robot, $arc_enabled, $dkim_enabled);
+    my ($list, $robot, $arc_enabled, $dkim_enabled, $rm_sig);
     if (ref($message->{context}) eq 'Sympa::List') {
         $list  = $message->{context};
         $robot = $message->{context}->{'domain'};
 
         $arc_enabled = 'on' eq $list->{'admin'}{'arc_feature'};
         $dkim_enabled =
             'on' eq Conf::get_robot_conf($list->{'domain'}, 'dkim_feature');
+        $rm_sig = 'on' eq $list->{'admin'}{'remove_dkim_headers'};
     } elsif ($message->{context} and $message->{context} ne '*') {
         $robot = $message->{context};
 
         $arc_enabled  = 'on' eq Conf::get_robot_conf($robot, 'arc_feature');
         $dkim_enabled = 'on' eq Conf::get_robot_conf($robot, 'dkim_feature');
+        $rm_sig = 'on' eq Conf::get_robot_conf($robot, 'remove_dkim_headers');
     } else {
         $robot = '*';
 
         $arc_enabled  = 'on' eq $Conf::Conf{'arc_feature'};
         $dkim_enabled = 'on' eq $Conf::Conf{'dkim_feature'};
+        $rm_sig       = 'on' eq $Conf::Conf{'remove_dkim_headers'};
     }
 
     if ($message->{serial} eq '0' or $message->{serial} eq 's') {