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.

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

Newlines in header fields added by ARC feature should be normalized #1168

Closed
dpc22 opened this issue May 11, 2021 · 1 comment · Fixed by #1171
Closed

Newlines in header fields added by ARC feature should be normalized #1168

dpc22 opened this issue May 11, 2021 · 1 comment · Fixed by #1171
Assignees
Labels
Milestone

Comments

@dpc22
Copy link
Contributor

dpc22 commented May 11, 2021

Version

Sympa 6.2.62.

Exim 4.94.2

Installation method

My own RPM on RHEL 7, derived from official source RPM

Expected behavior

Sympa should be able to add:

ARC-Seal:
ARC-Message-Signature:
ARC-Authentication-Results:

without breaking the message headers that it is signing.

Actual behavior

Exim indents all the following message headers by one space (yes really: I will explain why below) so the headers are not shown to Mail User Agents.

Additional information

./lib/Sympa/Message.pm arc_seal() adds \r characters to message headers before it calculates the ARC headers:

my $msg_as_string = $self->as_string;
$msg_as_string =~ s/\r?\n/\r\n/g;
$msg_as_string =~ s/\r?\z/\r\n/ unless $msg_as_string =~ /\n\z/;
unless (eval { $arc->PRINT($msg_as_string) and $arc->CLOSE }) {

The ARC headers which are calculated all contain "\r\n" line terminations.

./lib/Sympa/Message.pm "as_string()" then combines various headers with different
line terminations to end up with (^M is a "\r" carriage return character)

ARC-Seal: i=1; a=rsa-sha256; cv=none; d=lists.cam.ac.uk; s=^M
        20210430.lists; t=1620734904; b=LUqOjF+6Uc9n91RwnR8pxs3GPop/uIhB^M
        n7sLqP3Li58nhW9Fqg8TN8QR7gG59AZwFIrp+sG0wLYdxxlCdpVRioBCMlXBd0mN^M
        ZSUBdzNaNJyJtcCLTH3k8VwzSDvJf9mu8k6cSHWH9IX906bJTiXiyNIHgERKy2Z8^M
        +9fhH8InX3Shz9zpxR1TodOoEUNdoBCkJMFACAUwwqFJpL1gWoeleMGgslKh5fkE^M
        l7iZuaD5CGn5VTzJkbgn4OGCHi+q4zYFSuTyFsJIWhAji5Lu8GDEtYRSUqOjmUc3^M
        6OsqzAZYW/bw0Av/C6AUddmjbeL8Z6HryCrTUWCWJNGlR8rF1M752w==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=^M
        lists.cam.ac.uk; h=from:subject:to:message-id:date:sender^M
        :list-id:list-help:list-subscribe:list-unsubscribe:list-post^M
        :list-owner:list-archive; s=20210430.lists; bh=47DEQpj8HBSa+/TIm^M
        W+5JCeuQeRkm5NMpJWZG3hSuFU=; b=U3hnFhTOkvY+C76UfE0s3g+cB/0w2rrgl^M
        xhrEevRhdm/GsA2NRFBrTZ9xi1v32bRz9BKwhcPEnnJ08b/dmwEUqx57x6YBZq9G^M
        28gnJh3AylyFzAWgn+B2lsTy9qTECXgtKfsP0cqLpHLPAnWlowVLkQK9Py9W0HdZ^M
        7+e5/uQHrY8S6muBA0w0/GZYIJjlsTOd5ZkWBUwLZnV40bilnoQMN5zjLerfmx4z^M
        iWURvF6Hg3FTNha11DO5g3S9XnM0ymu2l/Qzr8Tx52bZ3EdRAUSh+F6qsNJIyXjl^M
        TgrENGWoO4j4jshz8Z7yiKydH0eKY+/cDR+43YwEr6Q95FDVAiOpA==
ARC-Authentication-Results: i=1; lists.cam.ac.uk;^M
        iprev=pass (XXX.cable.virginm.net) smtp.remote-ip=XXX.XXX.XXX.XXX;^M
        arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=lists.cam.ac.uk; h=from
        :subject:to:message-id:date:sender:list-id:list-help
        :list-subscribe:list-unsubscribe:list-post:list-owner
        :list-archive; s=20210430.lists; i=
        [email protected]; bh=frcCV1k9oG9oKj3dpUqdJg1Px
        RT2RSN/XKdLCPjaYaY=; b=mTl/GfeN3xxZiOqQRde0SKK71M5PR47DxQWcwd4NV
        fydB0PHojYKQzLpqO0DzNJEguIMdu093W6P54tUL/UTgGGOI6RGLRdvS+Z+opCZP
        2KcXbsd9+A3LhiKLe8kYW8N8fq+TTXwoTrBOjKh4P3xZ/skC7gOGItR5opXoG0um
        frlpHGG7iNBQPv1azBPCS3wWbpVbyNXnbtUOtCeTfELTO2inBI4filjrCkFErKop
        AqXyTZN0XX9g9uj9UhSwvdsmYKMVj1pEYV+IIgZG4K/j5Bvv0aU5QG/T/v8iF7Fp
        3qGBbD23sOpXxi5mq+uw6UMwog26rertGqCeNCwMHQCww==
Authentication-Results: lists.cam.ac.uk; ...
From: David Carter <[email protected]>
Subject: Test
To: [email protected]
Message-ID: <[email protected]>
Date: Tue, 11 May 2021 13:08:24 +0100
X-Loop: [email protected]

Postfix may be able to cope with the mixture of "\r\n" and "\n" line endings.

Unfortunately Exim concludes that the input is broken and takes (rather amusing) countermeasures:

https://www.exim.org/exim-html-current/doc/html/spec_html/ch-message_processing.html

  1. Line endings
  • If a bare CR is encountered within a header line, an extra space is added after the line terminator so as not to end the header line. The reasoning behind this is that bare CRs in header lines are most likely either to be mistakes, or people trying to play silly games.
  • If the first header line received in a message ends with CRLF, a subsequent bare LF in a header line is treated in the same way as a bare CR in a header line.

Consequently all of the headers after "ARC-Authentication-Results:" are indented by one character.

I think that the solution may be as simple as removing stray \r characters from the ARC headers.

$ diff -ud ./lib/Sympa/Message.pm-DIST ./lib/Sympa/Message.pm
--- ./lib/Sympa/Message.pm-DIST 2021-05-11 11:05:31.377902733 +0100
+++ ./lib/Sympa/Message.pm      2021-05-11 13:21:25.310592396 +0100
@@ -629,6 +629,8 @@
     if (grep { $_ and /\AARC-Seal:/i } @seal) {
         foreach my $ahdr (reverse @seal) {
             my ($ah, $av) = split /:\s*/, $ahdr, 2;
+            # DPC FIX
+            $av =~ s&\r&&g;
             $self->add_header($ah, $av, 0);
         }
     }
@ikedas ikedas added the bug label May 11, 2021
@ikedas ikedas added this to the 6.2.64 milestone May 11, 2021
@ikedas ikedas self-assigned this May 11, 2021
@ikedas ikedas changed the title Bad interaction between arc_feature and Exim Mail Transport Agent Newlines in header fields added by ARC feature should be normalized May 12, 2021
@ikedas ikedas added the ready A PR is waiting to be merged. Close to be solved label May 12, 2021
ikedas added a commit that referenced this issue May 12, 2021
Newlines in header fields added by ARC feature should be normalized (#1168)
Fix suggested by @dpc22.
@ikedas
Copy link
Member

ikedas commented May 12, 2021

@dpc22, the problem was fixed according to your suggestion. Thank you!

@ikedas ikedas removed the ready A PR is waiting to be merged. Close to be solved label May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants