Yup. I should check if Haraka does the right thing with this too :)
On Tue, 16 Aug 2011, Jared Johnson wrote:
True, I was led astray by the comment that seemed to indicate it was all
about headers. if the intention of the comment is correct, then it should
probably if ( $in_header and ... ), whether or not the new regex is added.
That block would also need to be moved to after the block that sets
$in_header = 0 if we've reached the end, to avoid catching the blank line
that separates headers and newlines.
At any rate, Chris has a point -- I was actually thinking the other day
that code like that original deferral would be better of sitting in a
plugin, so I could muck around with it in a more straightforward manner in
order to, say, log what we did to our own database, or change it to a
rejection, or whatever :)
Anyway, this is all kind of minutia compared to the original patch, which
I feel I should remind everyone has to do with accepting legitimate mail
from Lotus software without inadvertently stripping the headers :)
-Jared
That line of code doesn't look at the headers though, just at the final
dot at the end-of-data.
------------------------------------------------------------------------
Jared Johnson <mailto:jjohn...@efolder.net>
August 16, 2011 3:00 PM
There's already a special case for something similar to this:
# Reject messages that have either bare LF or CR. rjkaes noticed a
# lot of spam that is malformed in the header.
($_ eq ".\n" or $_ eq ".\r")
and $self->respond(421, "See
http://smtpd.develooper.com/barelf.html";)
and return $self->disconnect;
you could just make it ( /\r\r\n$/ or $_ eq ".\n" or $_ eq ".\r" )
maybe?
and update the link to point to a URL that explains both?
-Jared
------------------------------------------------------------------------
Matt Sergeant <mailto:m...@sergeant.org>
August 16, 2011 11:28 AM
Yup there's a lot of this going around right now. Just to be explicit
though, the header lines end in \r\r\n. Worth rejecting the bloody
lot, frankly :)
------------------------------------------------------------------------
Chris Lewis <mailto:cle...@nortel.com>
August 15, 2011 4:21 PM
As a FYI, I've been seeing bot-emitted spam that appears to have extra
\r at the end of _all_ header lines, and the qpsmtpd parser seems to
be treating all of it as part of the _body_. IOW: except for the
received line inserted by qpsmtpd, qpsmtpd doesn't see _any_ headers.
This implementation is backrev (0.80 I think), and as it's only spam
from one particular bot, we don't care about that particular wierdness
enough to investigate further. But it's worth being aware of.
------------------------------------------------------------------------
Jared Johnson <mailto:jjohn...@efolder.net>
August 15, 2011 3:39 PM
Hi,
We got a bug report from someone using IBM's Lotus suite (I think for
both
their MUA and MTA). Their users would often send messages where all the
content was in the subject and they didn't bother sending any message
content. I'm not sure if it's due to an apparently uncommon behavior for
their particular MUA or their MTA, but every one of these messages was
coming through with data in a form that looked like this:
"Subject: howdy\r\n.\r\n"
Rather than including the blank line that one might expect to follow
headers, since it's required in the event that a message body is
present:
"Subject: howdy\r\n\r\n.\r\n"
The customer reported these messages were having their subjects
stripped;
additional testing indicted all existing headers were being stripped. It
looks like this is because the loop that processes message data in
Qpsmtpd::SMTP::data_respond() and creates a Mail::Header object which is
later used to write out the header in delivery, only works if a blank
line
exists after the header, e.g. the second form above. The following is
all
I could find in RFC 5322 that elaborated on this blank line, which
obviously must exist if a message body is included:
"A message consists of header fields (collectively called "the header
section of the message") followed, optionally, by a body. The header
section is a sequence of lines of characters with special syntax as
defined in this specification. The body is simply a sequence of
characters that follows the header section and is separated from the
header section by an empty line (i.e., a line with nothing preceding the
CRLF)."
I read this as implicitly allowing the exclusion of this blank line if
there is no message body: the specification for the blank line is only
mentioned in the description of the body, which is itself described as
optional. Considering we haven't run into this bug in years of usage, I
assume it's unconventional to exclude the blank line, but it looks like
it
is legitimate syntax.
At any rate this was effecting multiple legitimate end users so we put
together the attached patch, which pulls the header building into its
own
sub which is then called inside the loop if we reach the blank line
indicating the header section is complete; otherwise, it's called
outside
of the loop if we have no more message data, indicating the header
section
is complete. Sorry I'm not putting this on a github fork, I still don't
have my git stuff together, I may never get around to it but I thought
you
guys might find this useful.
-Jared
