Re: [PATCH] tweak QP's header handling for messages with no body

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:[email protected]>
>>> 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:[email protected]>
>>> 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:[email protected]>
>>> 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:[email protected]>
>>> 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
>>
>
>
>