I’m not sure if my use-case is an unusual one, I need to:

-          Accept large files (100Mb+ , possibly 1Gb in some places)

-          Anticipate flurries of files around deadlines

-          Minimise latency by using tmpfs (similar to ramdisk) for SecTmpDir

-          Maximise ability to store suspicious files (eg. Files blocked based on AV scan) by using NFS SAN storage for SecUploadDir for a long while (at least 6 months)

-          Avoid a DOS scenario by exhausting memory (ramdisk) if someone repeatedly uploads large files containing a virus

-          Only want to keep suspicious files (eg. Files blocked based on AV scan), not all files (should be very small minority)

-          Minimise latency for majority of requests where file will not be permanently stored on WAF (reverse-proxy)

 

Given these requirements, it seems to me highly desirable to have different filesystems for SecTmpDir and SecUploadDir. With SecTmpDir providing very fast performance using a ramdisk (or possibly local SSD drive), and SecUploadDir providing slower long term storage, that would only be used for ‘archiving’ suspicious files.

 

I’m assuming the only reason for these needing to be on the same filesystem currently is the apr_file_rename in msc_multipart.c

 

I’m no C programmer, so the below code is probably far from optimal, and specific to my scenario. But the following modification in apache2/msc_multipart.c , yields the specific behaviour I’m after (and appears to be working for me).

 

                    if (apr_file_copy(parts[i]->tmp_file_name, new_filename, mode2fileperms(msr->txcfg->upload_filemode),

                        msr->msc_reqbody_mp) != APR_SUCCESS)

                    {

                        msr_log(msr, 1, "Input filter: Failed to rename file from \"%s\" to \"%s\".",

                            log_escape(msr->mp, parts[i]->tmp_file_name),

                            log_escape(msr->mp, new_filename));

                        return -1;

                    } else {

                        if (msr->txcfg->debuglog_level >= 4) {

                            msr_log(msr, 4, "Input filter: Moved file from \"%s\" to \"%s\".",

                                log_escape(msr->mp, parts[i]->tmp_file_name),

                                log_escape(msr->mp, new_filename));

                        }

                        if (unlink(parts[i]->tmp_file_name) < 0) {

                            msr_log(msr, 1, "Multipart: Failed to delete file after copy (part) \"%s\" because %d(%s)",

                                log_escape(msr->mp, parts[i]->tmp_file_name), errno, strerror(errno));

                        } else {

                            if (msr->txcfg->debuglog_level >= 4) {

                                msr_log(msr, 4, "Multipart: Deleted file after copy \"%s\"",

                                    log_escape(msr->mp, parts[i]->tmp_file_name));

                            }

                        }

                    }

 

 

I’m extremely conscious that I’m meddling with something I am only just exploring, and that this was originally written by someone far more experienced. If there is a good reason why I shouldn’t do what I’m doing I would be grateful to hear about it. If it is a reasonable thing for me to do, are my requirements too unusual to justify making this a configurable behaviour so that apr_file_rename would be used in on same filesystem, and apr_file_copy (followed by unlink) if not?

 

Thanks,

Paul

 

 

 

 

Diff output below:

*** modsecurity-apache_2.7.2/apache2/msc_multipart.c    2012-12-17 16:09:11.000000000 +0000

--- modsecurity-apache_2.7.2-uea/apache2/msc_multipart.c        2013-01-18 10:53:56.294181447 +0000

***************

*** 1338,1344 ****

                          new_basename);

                      if (new_filename == NULL) return -1;

 

!                     if (apr_file_rename(parts[i]->tmp_file_name, new_filename,

                          msr->msc_reqbody_mp) != APR_SUCCESS)

                      {

                          msr_log(msr, 1, "Input filter: Failed to rename file from \"%s\" to \"%s\".",

--- 1338,1344 ----

                          new_basename);

                      if (new_filename == NULL) return -1;

 

!                     if (apr_file_copy(parts[i]->tmp_file_name, new_filename, mode2fileperms(msr->txcfg->upload_filemode),

                          msr->msc_reqbody_mp) != APR_SUCCESS)

                      {

                          msr_log(msr, 1, "Input filter: Failed to rename file from \"%s\" to \"%s\".",

***************

*** 1351,1357 ****

--- 1351,1368 ----

                                  log_escape(msr->mp, parts[i]->tmp_file_name),

                                  log_escape(msr->mp, new_filename));

                          }

+                         if (unlink(parts[i]->tmp_file_name) < 0) {

+                             msr_log(msr, 1, "Multipart: Failed to delete file after copy (part) \"%s\" because %d(%s)",

+                                 log_escape(msr->mp, parts[i]->tmp_file_name), errno, strerror(errno));

+                         } else {

+                             if (msr->txcfg->debuglog_level >= 4) {

+                                 msr_log(msr, 4, "Multipart: Deleted file after copy \"%s\"",

+                                     log_escape(msr->mp, parts[i]->tmp_file_name));

+                             }

+                         }

                      }

+

+

                  }

              }

          }

 

 

 

 

From: Paul Beckett (ITCS) [mailto:P.Beckett@uea.ac.uk]
Sent: Thursday, January 17, 2013 8:44 PM
To: Ryan Barnett; mod-security-users@lists.sourceforge.net
Subject: Re: [mod-security-users] AV scanning : lua or c

 

Thanks for the response. I'm hoping to benchmark both approaches.... when I get the time to figure out how-to get Apachebench (or something else) to post a file to an upload script I'll post the results back here.

 

Because I need to accept large files (100Mb+), I thought I might be better using tmpfs rather than a true ramdisk, so that it could be swapped out to disk if required (hopefully wouldn't occur to often). However doing this I hit a problem. The file move from the SecTmpDir to the SecUploadDir directory fails with:

Input filter: Moved file from "/usr/local/apache/modsecurity/tmp/20130117-200140-UPhYpIveeK0AAC5e6y0AAABk-file-HIhlke" to "/usr/local/apache/modsecurity/upload/20130117-200140-UPhYpIveeK0AAC5e6y0AAABk-file-HIhlke".

 

I now see that the docs at https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual#wiki-SecUploadDir do say that SecUploadDir needs to be in the same filesystem as SecTmpDir. I presume this is because the  apr_file_rename in apache2/msc_multipart.c fails when not on the same filesystem.



I guess there is a good reason why the code in apache2/msc_multipart.c, doesn't do something like:

   status = apr_file_rename(parts[i]->tmp_file_name, new_filename, msr->msc_reqbody_mp);
   if (APR_STATUS_IS_EXDEV(status)) {
       (status = apr_file_copy(from_path_apr, to_path_apr, APR_FILE_SOURCE_PERMS, pool)) &&
       (status = apr_file_remove(from_path_apr, pool));
   }
   if status != APR_SUCCESS {
         msr_log(msr, 1, "Input filter: Failed to rename file from \"%s\" to \"%s\".",
                 log_escape(msr->mp, parts[i]->tmp_file_name),
                 log_escape(msr->mp, new_filename));
                 return -1;
   else {
         if (msr->txcfg->debuglog_level >= 4) {
                 msr_log(msr, 4, "Input filter: Moved file from \"%s\" to \"%s\".",
                 log_escape(msr->mp, parts[i]->tmp_file_name),
                 log_escape(msr->mp, new_filename));
         }
   }
 
And therefore a good reason why I shouldn't customise it to do that (or in this case as I know the rename will always fail, simply replace it with a copy and delete).

 

Thanks again,

Paul

 


From: Ryan Barnett [RBarnett@trustwave.com]
Sent: 15 January 2013 14:03
To: Paul Beckett (ITCS); mod-security-users@lists.sourceforge.net
Subject: Re: [mod-security-users] AV scanning : lua or c

 

From: "Paul Beckett (ITCS)" <P.Beckett@uea.ac.uk>
Date: Tuesday, January 15, 2013 6:14 AM
To: "mod-security-users@lists.sourceforge.net" <mod-security-users@lists.sourceforge.net>
Subject: [mod-security-users] AV scanning : lua or c

 

I was wanting to implement AV scanning with clamd. Having seen a perl script to do this. I figured a solution using lua would provide better performance, and shouldn’t be too hard to write. Some googling revealed Josh Zlatin had already done this J

http://www.purehacking.com/blogs/josh-zlatin/virus-detection-in-modsecurity

 

I had to make a small tweak to the script, as I couldn’t get the rex pcre library installed on my rhel servers (replaced rex.match with string.find), tested it and sat back feeling happy until I noticed CRS comes with a C program: runAV-clamd, doing essentially the same thing.

 

Now I’m wondering which would provide the best performance (lua or C), and if there are any other considerations. Any advice would be much appreciated.

 

Thanks,

Paul

 

 

Hey Paul,

Not sure what the performance difference would be between running the runAV binary program via exec action vs. running the Lua script via exec.

 

The only other recommendation that I have would be to consider creating a RAM disk for use by the SecUploadDir directive - https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual#wiki-SecUploadDir.  In previous testing that we did, we found that the IO of writing files to disk (for FILES_TMPNAMES) caused the greatest amount of latency.  By creating a RAM disk, this virtual swapping is much faster.

 

As a side note – using RAM disks are also great for performance for any persistent storage usage with inicol, setsid, etc…

 

-Ryan

 



This transmission may contain information that is privileged, confidential, and/or exempt from disclosure under applicable law. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein (including any reliance thereon) is STRICTLY PROHIBITED. If you received this transmission in error, please immediately contact the sender and destroy the material in its entirety, whether in electronic or hard copy format.