False positives from header_checks

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

False positives from header_checks

Cedric Knight
The documentation for header_checks includes an example to "block
attachments with bad file name extensions", and I expect many
installations have a similar rule to cut down on malware.  This reads:

  /^Content-(Disposition|Type).*name\s*=\s*"?(.*(\.|=2E)(
   ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
   hlp|ht[at]|
   inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
   \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
   ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
   vb[esx]?|vxd|ws[cfh]))(\?=)?"?\s*(;|$)/x
     REJECT Attachment name "$2" may not end with ".$4"

Unfortunately, this can now block valid email from iPhone/iOS/ithing
users.  The second ".*" can span multiple parameters.  This shows up in
logs when searching for "x-apple-part-url" as follows:

postfix/cleanup[1234]: 123412341234: reject: header Content-Type:
application/vnd.ms-publisher;??name="redacted
redacted.pub";??x-apple-part-url="[hidden email]"

What Apple has done seems legal under RFC 2045 but may make some of
their users' email undeliverable.

Rules can be amended to limit to "token" or "quoted-string" versions of
the filename like this:

  /^Content-(Disposition|Type).*name\s*=\s*
   ("(?:[^"]|\\")*|[^();:,\/<>\@\"?=<>\[\]\ ]*)
   ((?:\.|=2E)(
   ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
   hlp|ht[at]|
   inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
   \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
   ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
   vb[esx]?|vxd|ws[cfh])(\?=)?"?)\s*(;|$)/x
     REJECT Attachment name $2$3 may not end with ".$4"

A separate security point is that this doesn't actually block "bad"
extensions if the Content-Type name is base64 encoded and the filename
parameter in the Content-Disposition is percent-encoded.

Hope this is useful to some.

CK


Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Laz C. Peterson
This is great information.

It's very odd ... Apple has been responsible for the foundation of quite a few RFC's but in our experience has actually made it difficult for our software to both comply with the RFC as well as Apple's client software.

Thank you Cedric.

~ Laz Peterson
Paravis, LLC

> On Apr 6, 2016, at 3:28 PM, Cedric Knight <[hidden email]> wrote:
>
> The documentation for header_checks includes an example to "block
> attachments with bad file name extensions", and I expect many
> installations have a similar rule to cut down on malware.  This reads:
>
>  /^Content-(Disposition|Type).*name\s*=\s*"?(.*(\.|=2E)(
>   ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
>   hlp|ht[at]|
>   inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
>   \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
>   ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
>   vb[esx]?|vxd|ws[cfh]))(\?=)?"?\s*(;|$)/x
>     REJECT Attachment name "$2" may not end with ".$4"
>
> Unfortunately, this can now block valid email from iPhone/iOS/ithing
> users.  The second ".*" can span multiple parameters.  This shows up in
> logs when searching for "x-apple-part-url" as follows:
>
> postfix/cleanup[1234]: 123412341234: reject: header Content-Type:
> application/vnd.ms-publisher;??name="redacted
> redacted.pub";??x-apple-part-url="[hidden email]"
>
> What Apple has done seems legal under RFC 2045 but may make some of
> their users' email undeliverable.
>
> Rules can be amended to limit to "token" or "quoted-string" versions of
> the filename like this:
>
>  /^Content-(Disposition|Type).*name\s*=\s*
>   ("(?:[^"]|\\")*|[^();:,\/<>\@\"?=<>\[\]\ ]*)
>   ((?:\.|=2E)(
>   ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
>   hlp|ht[at]|
>   inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
>   \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
>   ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
>   vb[esx]?|vxd|ws[cfh])(\?=)?"?)\s*(;|$)/x
>     REJECT Attachment name $2$3 may not end with ".$4"
>
> A separate security point is that this doesn't actually block "bad"
> extensions if the Content-Type name is base64 encoded and the filename
> parameter in the Content-Disposition is percent-encoded.
>
> Hope this is useful to some.
>
> CK
>
>

Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Curtis Villamizar
Since pcre evaluates in order you could add

/^Content-(Disposition|Type).*;??x-apple-part-url="[^"]+"$/x  DUNNO


before the pcre that does the rejection.

Since "." is commonly "%2E" you could also change the "\." in the RE to
"(\.|%2E)".
That doesn't solve base64 encoding.

Disclaimer: I haven't tried this.

Curtis

On 04/06/16 22:02, Laz C. Peterson wrote:

> This is great information.
>
> It's very odd ... Apple has been responsible for the foundation of quite a few RFC's but in our experience has actually made it difficult for our software to both comply with the RFC as well as Apple's client software.
>
> Thank you Cedric.
>
> ~ Laz Peterson
> Paravis, LLC
>
>> On Apr 6, 2016, at 3:28 PM, Cedric Knight <[hidden email]> wrote:
>>
>> The documentation for header_checks includes an example to "block
>> attachments with bad file name extensions", and I expect many
>> installations have a similar rule to cut down on malware.  This reads:
>>
>>   /^Content-(Disposition|Type).*name\s*=\s*"?(.*(\.|=2E)(
>>    ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
>>    hlp|ht[at]|
>>    inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
>>    \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
>>    ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
>>    vb[esx]?|vxd|ws[cfh]))(\?=)?"?\s*(;|$)/x
>>      REJECT Attachment name "$2" may not end with ".$4"
>>
>> Unfortunately, this can now block valid email from iPhone/iOS/ithing
>> users.  The second ".*" can span multiple parameters.  This shows up in
>> logs when searching for "x-apple-part-url" as follows:
>>
>> postfix/cleanup[1234]: 123412341234: reject: header Content-Type:
>> application/vnd.ms-publisher;??name="redacted
>> redacted.pub";??x-apple-part-url="[hidden email]"
>>
>> What Apple has done seems legal under RFC 2045 but may make some of
>> their users' email undeliverable.
>>
>> Rules can be amended to limit to "token" or "quoted-string" versions of
>> the filename like this:
>>
>>   /^Content-(Disposition|Type).*name\s*=\s*
>>    ("(?:[^"]|\\")*|[^();:,\/<>\@\"?=<>\[\]\ ]*)
>>    ((?:\.|=2E)(
>>    ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
>>    hlp|ht[at]|
>>    inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
>>    \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
>>    ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
>>    vb[esx]?|vxd|ws[cfh])(\?=)?"?)\s*(;|$)/x
>>      REJECT Attachment name $2$3 may not end with ".$4"
>>
>> A separate security point is that this doesn't actually block "bad"
>> extensions if the Content-Type name is base64 encoded and the filename
>> parameter in the Content-Disposition is percent-encoded.
>>
>> Hope this is useful to some.
>>
>> CK
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Cedric Knight
In reply to this post by Cedric Knight
Curtis Villamizar wrote:
> Since pcre evaluates in order you could add>
> /^Content-(Disposition|Type).*;??x-apple-part-url="[^"]+"$/x  DUNNO
>
> before the pcre that does the rejection.

That's one possibility, but:

(a) you probably won't want the '??' qualifying the ';'.  '??' in the
Postfix log sample I mentioned just symbolises a CRLF line break.
Postfix/cleanup concatenates header lines, so either '\s*' or '\r?\n\s+'
will match such a 'fold' in the headers in a PCRE.

(b) there may be alternative or additional parameters to look out for,
not just x-apple-part-url;

(c) that table entry allows malware to pass any block, if it adds some
'x-apple-part-url=' string after or as part of an executable filename.

I still prefer the solution below, which specifies the character classes
for Content-* parameters and so prevents the false match.

Is there anywhere else I should raise this as a documentation bug?

In general, wherever you see a '.*', IMHO you should ask what exactly
the syntax of the string is you're matching against, or if it is free
text at least limit it to a '.{0,n}' quantifier.  This avoids lack of
specificity, possible combinatorial explosion and unnecessary resource
usage.

> Since "." is commonly "%2E" you could also change the "\." in the RE to
> "(\.|%2E)".

'=2E' (quoted-printable) was already in the examples in the existing
documentation and below, so maybe that should be '(\.|=2E|\%2E)'.
Probably '=2E' helps with encoded words in headers (RFC 2047), and maybe
also with MIME headers that are enclosed within another MIME entity that
specifies quoted-printable encoding.

Actually I'm not sure if any MUAs will regard '%2E' as a '.' unless the
parameter is a full RFC 6266 'filename*=' (not 'filename=') followed by
a character encoding name - in which case the file extension is quite
likely to be percent-encoded too.

> That doesn't solve base64 encoding.

No, it doesn't.  As I understand it, postfix/cleanup is designed to be
fast and lightweight.  man pages for body_checks and header_checks say
"they do not decode attachments", so presumably shouldn't be expected to
apply the various decoding mechanisms to the various header fields and
parameters including those indicating filenames.

So I would suggest, unless there's a real flood of .exe attachments,
moving these type of restrictions to something like SpamAssassin
'mimeheader' rules anyway, which does decoding first.  These rules
should also not use a '.*' between the 'name=' and the extension, but
instead something like
  (?:"(?:[^"]|\\")*|[^();:,\/<>\@\"?=<>\[\]\ ]*)

Besides catching evasions using various encodings, using SpamAssassin
may also have the advantage that specific malware can be picked up first
by an AV, before the slower SpamAssassin rules look for message
attributes and headers associated with generic malware.

In any case, be careful if trying to block filenames in header_checks.

> Disclaimer: I haven't tried this.
>
> Curtis
>
> On 04/06/16 22:02, Laz C. Peterson wrote:
>> This is great information.
>>
>> It's very odd ... Apple has been responsible for the foundation of
quite a few RFC's but in our experience has actually made it difficult
for our software to both comply with the RFC as well as Apple's client
software.

Hmm.  Is that because Apple software doesn't conform, or the new RFCs
are cumbersome or overcomplicated?

CK

>>
>> Thank you Cedric.

:)

On 06/04/16 23:28, Cedric Knight wrote:

> The documentation for header_checks includes an example to "block
> attachments with bad file name extensions", and I expect many
> installations have a similar rule to cut down on malware.  This reads:
>
>   /^Content-(Disposition|Type).*name\s*=\s*"?(.*(\.|=2E)(
>    ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
>    hlp|ht[at]|
>    inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
>    \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
>    ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
>    vb[esx]?|vxd|ws[cfh]))(\?=)?"?\s*(;|$)/x
>      REJECT Attachment name "$2" may not end with ".$4"
>
> Unfortunately, this can now block valid email from iPhone/iOS/ithing
> users.  The second ".*" can span multiple parameters.  This shows up in
> logs when searching for "x-apple-part-url" as follows:
>
> postfix/cleanup[1234]: 123412341234: reject: header Content-Type:
> application/vnd.ms-publisher;??name="redacted
> redacted.pub";??x-apple-part-url="[hidden email]"
>
> What Apple has done seems legal under RFC 2045 but may make some of
> their users' email undeliverable.
>
> Rules can be amended to limit to "token" or "quoted-string" versions of
> the filename like this:
>
>   /^Content-(Disposition|Type).*name\s*=\s*
>    ("(?:[^"]|\\")*|[^();:,\/<>\@\"?=<>\[\]\ ]*)
>    ((?:\.|=2E)(
>    ade|adp|asp|bas|bat|chm|cmd|com|cpl|crt|dll|exe|
>    hlp|ht[at]|
>    inf|ins|isp|jse?|lnk|md[betw]|ms[cipt]|nws|
>    \{[[:xdigit:]]{8}(?:-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}\}|
>    ops|pcd|pif|prf|reg|sc[frt]|sh[bsm]|swf|
>    vb[esx]?|vxd|ws[cfh])(\?=)?"?)\s*(;|$)/x
>      REJECT Attachment name $2$3 may not end with ".$4"
>
> A separate security point is that this doesn't actually block "bad"
> extensions if the Content-Type name is base64 encoded and the filename
> parameter in the Content-Disposition is percent-encoded.
Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Viktor Dukhovni
In reply to this post by Laz C. Peterson

> On Apr 6, 2016, at 10:02 PM, Laz C. Peterson <[hidden email]> wrote:
>
> It's very odd ... Apple has been responsible for the foundation of quite a few RFC's but in our experience has actually made it difficult for our software to both comply with the RFC as well as Apple's client software.

The fault here is no Apple's.  It is a sin to parse MIME with regexps.
A proper MIME parser is required to do the job right.

The Postfix header checks are a not a complete solution, just a cheap
filter that can stop the most common junk.  It is a bug in the (sample)
filter that it admits false positives.  Any regexp filter will inevitably
have false negatives.

--
        Viktor.

Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Wietse Venema
In reply to this post by Cedric Knight
Unfortunately, I don't have time to decode this discussion. Can
someone post a tested diff, someone maybe post a revised version,
and when there is agreement, then I can adopt it.

        Wietse
Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Noel Jones-2
On 4/9/2016 8:00 AM, Wietse Venema wrote:
> Unfortunately, I don't have time to decode this discussion. Can
> someone post a tested diff, someone maybe post a revised version,
> and when there is agreement, then I can adopt it.
>
> Wietse
>

Does someone have a full, unmodified offending header to post so
this can be tested?


Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Bill Cole-3
In reply to this post by Wietse Venema
On 9 Apr 2016, at 9:00, Wietse Venema wrote:

> Unfortunately, I don't have time to decode this discussion. Can
> someone post a tested diff, someone maybe post a revised version,
> and when there is agreement, then I can adopt it.


Simplest fix: prevent *that* class of false positives by narrowing the
check to a single attribute, rather than including all attributes in the
header following one which includes 'name' in its name:

-  /^Content-(Disposition|Type).*name\s*=\s*"?(.*(\.|=2E)(
+  /^Content-(Disposition|Type).*name\s*=\s*"?([^;]*(\.|=2E)(

As Viktor noted: regular expressions are the wrong toolkit for MIME
parsing. A proper mature MIME parser is available for Postfix in the
MIMEDefang milter, which also is a fine tool for hooking in SpamAssassin
and AV tools.
Reply | Threaded
Open this post in threaded view
|

Re: False positives from header_checks

Wietse Venema
Bill Cole:

> On 9 Apr 2016, at 9:00, Wietse Venema wrote:
>
> > Unfortunately, I don't have time to decode this discussion. Can
> > someone post a tested diff, someone maybe post a revised version,
> > and when there is agreement, then I can adopt it.
>
>
> Simplest fix: prevent *that* class of false positives by narrowing the
> check to a single attribute, rather than including all attributes in the
> header following one which includes 'name' in its name:
>
> -  /^Content-(Disposition|Type).*name\s*=\s*"?(.*(\.|=2E)(
> +  /^Content-(Disposition|Type).*name\s*=\s*"?([^;]*(\.|=2E)(

Thanks, got it.

> As Viktor noted: regular expressions are the wrong toolkit for MIME
> parsing. A proper mature MIME parser is available for Postfix in the
> MIMEDefang milter, which also is a fine tool for hooking in SpamAssassin
> and AV tools.

Absolutely. But this class of false positives is easy to remove.

        Wietse
Reply | Threaded
Open this post in threaded view
|

Postfix 3.0.3 TCP interface Panic error condition

Charles Orth
In reply to this post by Bill Cole-3
Hi Folks,

I have compiled postfix 3.0.3. Using the ldap interface for the
following maps everything works as designed.
However, if I use theTCP interface, I am getting a panic error
condition. Config eg
virtual_alias_maps = tcp:removed-host-name-here:9871
virtual_mailbox_domains = tcp:removed-host-name-here:9872
transport_maps = tcp:removed-host-name-here:9873
check_sender_access tcp:removed-host-name-here:9876,
smtpd_sender_login_maps = tcp:removed-host-name-here:9875


May  2 14:58:27 postfix-dev1-m01 postfix/trivial-rewrite[31448]: panic:
VSTREAM_CTL_READ_FD requires double buffering
May  2 14:58:28 postfix-dev1-m01 postfix/master[30248]: warning: process
/opt/zimbra/pf.mtaout.xclient/libexec/trivial-rewrite pid 31448 killed
by signal 6
May  2 14:58:28 postfix-dev1-m01 postfix/master[30248]: warning:
/opt/zimbra/pf.mtaout.xclient/libexec/trivial-rewrite: bad command
startup -- throttling

Has anyone used the TCP interface with postfix 3.0.3?

Charles


Reply | Threaded
Open this post in threaded view
|

Re: Postfix 3.0.3 TCP interface Panic error condition

Viktor Dukhovni
On Fri, May 06, 2016 at 12:12:14PM -0400, Charles Orth wrote:

> Hi Folks,
>
> I have compiled postfix 3.0.3. Using the ldap interface for the following
> maps everything works as designed.
> However, if I use theTCP interface, I am getting a panic error condition.
> Config eg
> virtual_alias_maps = tcp:removed-host-name-here:9871
> virtual_mailbox_domains = tcp:removed-host-name-here:9872
> transport_maps = tcp:removed-host-name-here:9873
> check_sender_access tcp:removed-host-name-here:9876,
> smtpd_sender_login_maps = tcp:removed-host-name-here:9875
>
>
> May  2 14:58:27 postfix-dev1-m01 postfix/trivial-rewrite[31448]: panic:
> VSTREAM_CTL_READ_FD requires double buffering

Your Postfix source or binaries are likely corrupted.  Postfix
3.0.3 defines, but not use VSTREAM_CTL_READ_FD.

The TCP table code does use VSTREAM_CTL_TIMEOUT.

    vstream_control(dict_tcp->fp,
                    CA_VSTREAM_CTL_TIMEOUT(DICT_TCP_TMOUT),
                    CA_VSTREAM_CTL_END);

Something's changed that code or the vstream.h header.  My sources
show

        #define VSTREAM_CTL_END 0
        #define VSTREAM_CTL_READ_FD 5
        #define VSTREAM_CTL_TIMEOUT 8

If either the 8 or 0 changed to 5, you're likely to get the reported
error.

[ Both tcp_table support and vstream support are from the same
  library module, so version mismatch between executable programs
  and the installed shared libraries seems an unlikely cause. ]

--
        Viktor.