Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 3, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/lib/Sympa/Config/Schema.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4764,6 +4764,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
Expand Down
18 changes: 4 additions & 14 deletions src/lib/Sympa/Message.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,20 +843,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;
Expand Down Expand Up @@ -3839,6 +3827,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.
Expand Down
18 changes: 15 additions & 3 deletions src/lib/Sympa/Spindle/ProcessOutgoing.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,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.
Expand All @@ -257,6 +262,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) {
Expand Down Expand Up @@ -305,24 +314,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') {
Expand Down
Loading