Banning .docm gives misleading error message

[Kai Risku]

I *do* think the solution is to check both leaf and non-leaf nodes as my proposed fix does.
Then the administrator can freely apply banning rules on leaf or non-leaf nodes just as he prefers.

You are however correct when saying this is more of a reporting issue, as the banning rule for .docm did indeed block the attachment because checking the leaf nodes also saw the non-leaf node that the rule targeted. However I think I spent the better part of a working day trying all kinds of reporting macros in an effort to actually get the report telling me what was the reason for blocking. Spewing regular expressions or mime paths at an end-user never resulted in anything that I could dare put in front of the user trying to send his .docm -file. In the end, my removing of one line of code from amavisd immediately gave me exactly the result I was looking for with the default reporting template.

Frankly I do not see why amavisd should ignore non-leaf nodes in banning checks, because the more things checked the better!

With humble regards and deeply thankful for all efforts put into amavisd,

    Kai

--
Kai.Risku at arrak.fi     GSM  +358-40-767 8282
Oy Arrak Software Ab   http://www.arrak.fi

-----Original Message-----
From: amavis-users [mailto:amavis-users-bounces+kai.risku=arrak.fi at amavis.org] On Behalf Of Mark Martinec
Sent: Tuesday, April 26, 2016 18:22
To: amavis-users at amavis.org
Subject: RE: Banning .docm gives misleading error message

On 2016-04-05 10:21, Kai Risku wrote:
> We also have ClamAV blocking all files containing OLE2 Macros, so I am 
> going for a belt-and-suspenders type of approach…
> 
> Regardless of the effectivity of blocking certain types of files or 
> not, my main point for posting was the inability of amavis to clearly 
> report which file was blocked when the blocked file is being detected 
> as a container (e.g. a zip-file) containing other files.


[...]
> In order to guard against malicious macros, we have banned all 
> macro-enabled Office document formats, i.e. added the following to
> $banned_filename_re:
> 
> # block macro-enabled office files
> qr'.\.(xlsm|xltm|xlam|docm|dotm|pptm|potm|ppam|ppsm|sldm)$'i,
> 
> Since modern Office documents are technically zip-files, amavisd-new 
> opens and processes the zip archive. For originating (outgoing) 
> messages we bounce the banned emails so the poor sender can understand 
> why his emails are not delivered, but in this case amavisd-new does 
> not report the actual office document being banned but instead blames 
> the first file inside the zip-archive. This results in very cryptic 
> error messages, like:
> 
>                              Subject: BANNED contents from you
> (.txt,[Content_Types].xml)
> 
> Our content checker found
>     banned name: .txt,[Content_Types].xml
>                              in email presumable from you
> 
> It seems amavisd has a small “optimization” that skips banned checks 
> for non-leaf nodes. I propose removing that so the actual office 
> documents can be directly banned and correctly reported:
> 
> @@ -9912,7 +9912,9 @@
>        my(@path) = @{$part->path};
>        next  if @path <= 1;
>        shift(@path);  # ignore place-holder root node
> -      next  if @{$part->children};  # ignore non-leaf nodes
> +      # also process non-leaf nodes or we cannot block office
> documents
> +      # without alert about wrong parts (blames the innocent zip
> member)
> +      # next  if @{$part->children};  # ignore non-leaf nodes
>        my(@descr_trad);  # a part path: list of predecessors of a 
> message part
>        my(@descr);  # same, but in form suitable for check on 
> banned_namepath_re
>        for my $p (@path) {



I don't think the solution is not to ignore non-leaf nodes.
The non-leaf nodes information will be visible and checked during processing of each leaf node - as their parent nodes.
This provides more information to checker, putting each leaf node in its context.

The issue here is reporting, not checking. The %F macro expands to whatever a method banning_reason_short() povides.
Perhaps this method should enhanced, or the notification template should use more informative macros instead of %F (in $notify_sender_templ ) :

   BANNED contents from you (%F)


The following macros are available (README.customize) :
(or a new macro could be devised)

   banning_rule_key  a lookup key (a regexp) of a banning rule which matched,
                 e.g.:  (?-xism:^\\.(exe-ms|dll)$(?# rule #9))
   banning_rule_comment  a comment from a regexp in banning_rule_key, or
                 the whole banning_rule_key if it does not contain a comment,
                 e.g.:  rule #9
   banning_rule_rhs  right-hand side of a banning rule which matched, often
                 just a '1', but can be any string which evaluates to true
   banned_parts  a list of banned parts, with their full path in a MIME/archive
      e.g.:  multipart/mixed |
application/octet-stream,.exe,.exe-ms,videos.exe

   banned_parts_as_attr  similar to banned_parts, but uses a different syntax
                 using attribute/value pairs; currently known attributes
are:
      P: part's base name, i.e. a file name in a ./parts/ temporary directory
      L: part's location (path) in a mail tree
      M: MIME type as declared in MIME header fields of a message
      T: short part's content type according to a file(1) utility
      N: declared part names (none, one or more), as declared in MIME header
         fields or in an archive (tar, zip, ...)
      A: part's attributes as follows:
         U=undecodable, C=crypted, D=directory, S=special(device), L=link
      e.g.:
        P=p003,L=1,M=multipart/mixed |
        P=p002,L=1/2,M=application/octet-stream,T=rar,N=Setup1.1.rar |
        P=p007,L=1/2/4,T=exe,T=exe-ms,N=setup.exe

   F  just the first leaf node from banned_parts, with a prepended rule comment
      (if any), e.g.:  rule
#9:application/octet-stream,.exe,.exe-ms,videos.exe


Mark