Add option for more relaxed barelf handling and changed softdeny to deny

CHANGELOG.md

@@ -11,6 +11,15 @@ and [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 - qpsmtpd.service, a systemd unit for qpsmtpd-forkserver, now the
   recommended way to run qpsmtpd
+- config/barelf: `strict` (default) or `tolerant` handling of a bare LF
+  in the message data. Both only accept `<CRLF>.<CRLF>` as the end of the
+  data, so SMTP smuggling stays rejected
+
+### Changed
+
+- a bare LF/CR in the message data is rejected with a permanent
+  `554 (#5.6.0)` instead of a temporary 421, so senders bounce at once
+  instead of retrying a message that can never pass
 
 ### Removed
 

MANIFEST

@@ -6,6 +6,7 @@ config.sample/add_geoip_headers
 config.sample/badhelo
 config.sample/badmailfrom
 config.sample/badrcptto
+config.sample/barelf
 config.sample/dkim/dkim_key_gen.sh
 config.sample/dnsbl_allow
 config.sample/databytes

config.sample/barelf

@@ -0,0 +1,4 @@
+# How a bare LF/CR in the message data is handled, see docs/config.md.
+#   strict   - reject any line not ending in CRLF (default)
+#   tolerant - accept bare LF lines, but only <CRLF>.<CRLF> ends the data
+strict

docs/config.md

@@ -60,6 +60,21 @@ are shown below in ["Plugin settings"](#plugin-settings).
     size. Should be something less than the backend MTA has set as it's maximum
     message size (if there is one).
 
+- barelf
+
+    How a line in the message data that does not end in CRLF (a bare LF or
+    CR, RFC 5321 2.3.8) is handled. Such a message is rejected with
+    `554 (#5.6.0)` and the connection is closed. Defaults to `strict`.
+
+    - `strict`: reject any line not ending in CRLF.
+    - `tolerant`: accept bare LF lines, but the data only ends at
+      `<CRLF>.<CRLF>`. A bare LF/CR on the final dot, or a final dot right
+      after a bare LF, is rejected.
+
+    In both modes only `<CRLF>.<CRLF>` ends the data, so SMTP smuggling via
+    `<LF>.<LF>`, `<LF>.<CRLF>` or `<CRLF>.<LF>` is always rejected. An
+    unknown value logs a warning and falls back to `strict`.
+
 - size\_threshold
 
     When a message is greater than the size given in this config file, it will be

lib/Qpsmtpd/SMTP.pm

@@ -675,6 +675,22 @@ sub command_line_too_long {
     return 1;
 }
 
+# How a bare LF/CR in DATA is handled, from the 'barelf' config:
+#   strict   - reject any line not ending in CRLF (default)
+#   tolerant - accept bare LF lines, but only <CRLF>.<CRLF> ends the data
+# There is deliberately no mode that accepts another end of data sequence,
+# as that allows SMTP smuggling.
+sub barelf_mode {
+    my $self = shift;
+    my ($mode) = $self->config('barelf');
+    return 'strict' if !defined $mode;
+    $mode = lc $mode;
+    $mode =~ s/^\s+|\s+$//g;
+    return $mode if $mode =~ /^(?:strict|tolerant)$/;
+    $self->log(LOGWARN, "unknown barelf mode '$mode', using strict");
+    return 'strict';
+}
+
 sub disconnect {
     my $self = shift;
     $self->run_hooks("disconnect");
@@ -736,13 +752,28 @@ sub data_respond {
     my $header = Mail::Header->new(Modify => 0, MailFrom => 'COERCE');
 
     my $timeout = $self->config('timeout');
+    my $barelf  = $self->barelf_mode;
+    my $prev_bare = 0;
     while (defined($_ = $self->getline($timeout))) {
 
-        if ($_ !~ /\r\n$/) {
-            $self->respond(421, 'See http://smtpd.develooper.com/barelf.html');
+        # A bare LF/CR is a permanent defect of the message, a retry would
+        # fail the same way, so reject with a 5xx (RFC 5321 2.3.8). The
+        # 'barelf' config relaxes this, see docs/config.md.
+        # Only <CRLF>.<CRLF> ends the data in any mode: a bare LF/CR on the
+        # final dot, or a final dot right after a bare LF (<LF>.<CRLF>), is
+        # always rejected to prevent SMTP smuggling.
+        my $bare = $_ !~ /\r\n$/;
+        my $reject
+          = $barelf eq 'tolerant'
+          ? $_ eq ".\n" || $_ eq ".\r" || ($prev_bare && $_ eq ".\r\n")
+          : $bare;
+        if ($reject) {
+            $self->respond(554,
+                'Bare LF in message data not permitted, see RFC 5321 2.3.8 (#5.6.0)');
             $self->disconnect;
             return 1;
         }
+        $prev_bare = $bare;
 
         if ($_ eq ".\r\n") {
             $complete++;

t/qpsmtpd-smtp.t

@@ -175,7 +175,7 @@
     $smtpd->mock_hook('queue', sub { $ran = 1; return DONE });
     $smtpd->{_response} = undef;
     no warnings 'redefine';
     local *Test::Qpsmtpd::check_socket = sub { 0 };
     $smtpd->queue($smtpd->transaction);
     ok( !$ran, "queue hooks skipped when client disconnected before queue" );
     is( $smtpd->{_response}, undef, "no response sent to a disconnected client" );
@@ -360,17 +360,48 @@
     };
 
     my $code = $drive->("From: a\@example.com\r\n", "Date: now\r\n", "\r\n", "body\r\n", ".\r\n");
-    isnt( $code, 421, 'well-formed message is not rejected as bare-LF' );
+    isnt( $code, 554, 'well-formed message is not rejected as bare-LF' );
     isnt( $smtpd->connection->notes('disconnected'), 1,
         'well-formed message does not disconnect' );
 
     $code = $drive->("From: a\@example.com\r\n", "bare\n", ".\r\n");
-    is( $code, 421, 'bare LF in body is rejected' );
+    is( $code, 554, 'bare LF in body is rejected' );
     is( $smtpd->connection->notes('disconnected'), 1, 'bare LF in body disconnects' );
 
     $code = $drive->("From: a\@example.com\r\n", "\r\n", "body\r\n", ".\n");
-    is( $code, 421, 'bare LF terminator is rejected' );
+    is( $code, 554, 'bare LF terminator is rejected' );
     is( $smtpd->connection->notes('disconnected'), 1, 'bare LF terminator disconnects' );
+
+    # the 'barelf' config relaxes the check
+    my $orig_config = Test::Qpsmtpd->can('config');
+    my $with_mode = sub {
+        my ($mode, @lines) = @_;
+        no warnings 'redefine';
+        local *Test::Qpsmtpd::config = sub {
+            my ($self, $key) = @_;
+            return ($mode) if $key eq 'barelf';
+            return $orig_config->(@_);
+        };
+        my $c = $drive->(@lines);
+        return ($c, $smtpd->connection->notes('disconnected'));
+    };
+    my @body     = ("From: a\@example.com\r\n", "\r\n", "bare\n", "body\r\n", ".\r\n");
+    my @bare_dot = ("From: a\@example.com\r\n", "\r\n", "body\r\n", ".\n");
+    my @smuggle  = ("From: a\@example.com\r\n", "\r\n", "body\n", ".\r\n");
+    my $disc;
+
+    ($code) = $with_mode->('strict', @body);
+    is( $code, 554, 'strict: bare LF in body is rejected' );
+    ($code) = $with_mode->('bogus', @body);
+    is( $code, 554, 'unknown mode falls back to strict' );
+
+    ($code, $disc) = $with_mode->('tolerant', @body);
+    isnt( $code, 554, 'tolerant: bare LF in body is accepted' );
+    isnt( $disc, 1, 'tolerant: bare LF in body does not disconnect' );
+    ($code) = $with_mode->('tolerant', @bare_dot);
+    is( $code, 554, 'tolerant: bare LF terminator is rejected' );
+    ($code) = $with_mode->('tolerant', @smuggle);
+    is( $code, 554, 'tolerant: <LF>.<CRLF> is rejected (SMTP smuggling)' );
 }
 
 sub __clean_authentication_results {