Added SecAuditLogDirMode and SecAuditLogFileMode (MODSEC-82).

Cleaned up SecUploadFileMode implementation.

CHANGES

@@ -1,6 +1,11 @@
-12 Aug 2009 - 2.5.10-dev3
+24 Aug 2009 - 2.5.10-dev3
 -------------------------
 
+ * Added SecAuditLogDirMode and SecAuditLogFileMode to allow fine tuning
+   auditlog permissions (especially with mpm-itk).
+
+ * Cleaned up SecUploadFileMode implementation.
+
  * Cleanup build scripts.
 
 

apache2/apache2.h

@@ -103,5 +103,6 @@ char DSOLOCAL *format_error_log_message(apr_pool_t *mp, error_message *em);
 
 const DSOLOCAL char *get_response_protocol(request_rec *r);
 
+
 #endif
 

apache2/apache2_config.c

@@ -20,6 +20,7 @@
 
 #include "modsecurity.h"
 #include "msc_logging.h"
+#include "msc_util.h"
 #include "pdf_protect.h"
 #include "http_log.h"
 
@@ -69,6 +70,8 @@ void *create_directory_config(apr_pool_t *mp, char *path) {
     /* audit log variables */
     dcfg->auditlog_flag = NOT_SET;
     dcfg->auditlog_type = NOT_SET;
+    dcfg->auditlog_dirperms = NOT_SET;
+    dcfg->auditlog_fileperms = NOT_SET;
     dcfg->auditlog_name = NOT_SET_P;
     dcfg->auditlog2_name = NOT_SET_P;
     dcfg->auditlog_fd = NOT_SET_P;
@@ -388,6 +391,10 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) {
         ? parent->auditlog_flag : child->auditlog_flag);
     merged->auditlog_type = (child->auditlog_type == NOT_SET
         ? parent->auditlog_type : child->auditlog_type);
+    merged->auditlog_dirperms = (child->auditlog_dirperms == NOT_SET
+        ? parent->auditlog_dirperms : child->auditlog_dirperms);
+    merged->auditlog_fileperms = (child->auditlog_fileperms == NOT_SET
+        ? parent->auditlog_fileperms : child->auditlog_fileperms);
     if (child->auditlog_fd != NOT_SET_P) {
         merged->auditlog_fd = child->auditlog_fd;
         merged->auditlog_name = child->auditlog_name;
@@ -512,6 +519,8 @@ void init_directory_config(directory_config *dcfg) {
     /* audit log variables */
     if (dcfg->auditlog_flag == NOT_SET) dcfg->auditlog_flag = 0;
     if (dcfg->auditlog_type == NOT_SET) dcfg->auditlog_type = AUDITLOG_SERIAL;
+    if (dcfg->auditlog_dirperms == NOT_SET) dcfg->auditlog_dirperms = CREATEMODE_DIR;
+    if (dcfg->auditlog_fileperms == NOT_SET) dcfg->auditlog_fileperms = CREATEMODE;
     if (dcfg->auditlog_fd == NOT_SET_P) dcfg->auditlog_fd = NULL;
     if (dcfg->auditlog2_fd == NOT_SET_P) dcfg->auditlog2_fd = NULL;
     if (dcfg->auditlog_name == NOT_SET_P) dcfg->auditlog_name = NULL;
@@ -525,7 +534,7 @@ void init_directory_config(directory_config *dcfg) {
     if (dcfg->upload_dir == NOT_SET_P) dcfg->upload_dir = NULL;
     if (dcfg->upload_keep_files == NOT_SET) dcfg->upload_keep_files = KEEP_FILES_OFF;
     if (dcfg->upload_validates_files == NOT_SET) dcfg->upload_validates_files = 0;
-    if (dcfg->upload_filemode == NOT_SET) dcfg->upload_filemode = 0600;
+    if (dcfg->upload_filemode == NOT_SET) dcfg->upload_filemode = mode2fileperms(0600);
 
     /* Misc */
     if (dcfg->data_dir == NOT_SET_P) dcfg->data_dir = NULL;
@@ -1026,6 +1035,46 @@ static const char *cmd_audit_log_type(cmd_parms *cmd, void *_dcfg, const char *p
     return NULL;
 }
 
+static const char *cmd_audit_log_dirmode(cmd_parms *cmd, void *_dcfg, const char *p1) {
+    directory_config *dcfg = (directory_config *)_dcfg;
+
+    if (dcfg == NULL) return NULL;
+
+    if (strcasecmp(p1, "default") == 0) {
+        dcfg->auditlog_dirperms = NOT_SET;
+    }
+    else {
+        long int mode = strtol(p1, NULL, 8); /* expects octal mode */
+        if ((mode == LONG_MAX)||(mode == LONG_MIN)||(mode <= 0)||(mode > 07777)) {
+            return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecAuditLogDirMode: %s", p1);
+        }
+
+        dcfg->auditlog_dirperms = mode2fileperms((mode_t)mode);
+    }
+
+    return NULL;
+}
+
+static const char *cmd_audit_log_filemode(cmd_parms *cmd, void *_dcfg, const char *p1) {
+    directory_config *dcfg = (directory_config *)_dcfg;
+
+    if (dcfg == NULL) return NULL;
+
+    if (strcasecmp(p1, "default") == 0) {
+        dcfg->auditlog_fileperms = NOT_SET;
+    }
+    else {
+        long int mode = strtol(p1, NULL, 8); /* expects octal mode */
+        if ((mode == LONG_MAX)||(mode == LONG_MIN)||(mode <= 0)||(mode > 07777)) {
+            return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecAuditLogFileMode: %s", p1);
+        }
+
+        dcfg->auditlog_fileperms = mode2fileperms((mode_t)mode);
+    }
+
+    return NULL;
+}
+
 static const char *cmd_audit_log_storage_dir(cmd_parms *cmd, void *_dcfg, const char *p1) {
     directory_config *dcfg = _dcfg;
 
@@ -1541,7 +1590,7 @@ static const char *cmd_upload_filemode(cmd_parms *cmd, void *_dcfg, const char *
     }
     else {
         long int mode = strtol(p1, NULL, 8); /* expects octal mode */
-        if ((mode == LONG_MAX)||(mode == LONG_MIN)||(mode <= 0)||(mode > 0777)) {
+        if ((mode == LONG_MAX)||(mode == LONG_MIN)||(mode <= 0)||(mode > 07777)) {
             return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecUploadFileMode: %s", p1);
         }
 
@@ -1854,6 +1903,22 @@ const command_rec module_directives[] = {
         "path to the audit log storage area; absolute, or relative to the root of the server"
     ),
 
+    AP_INIT_TAKE1 (
+        "SecAuditLogDirMode",
+        cmd_audit_log_dirmode,
+        NULL,
+        CMD_SCOPE_ANY,
+        "octal permissions mode for concurrent audit log directories"
+    ),
+
+    AP_INIT_TAKE1 (
+        "SecAuditLogFileMode",
+        cmd_audit_log_filemode,
+        NULL,
+        CMD_SCOPE_ANY,
+        "octal permissions mode for concurrent audit log files"
+    ),
+
     AP_INIT_TAKE12 (
         "SecCacheTransformations",
         cmd_cache_transformations,

apache2/configure

@@ -3812,7 +3812,9 @@ done
 
 
 
-for ac_header in fcntl.h limits.h stdlib.h string.h unistd.h
+
+
+for ac_header in fcntl.h limits.h stdlib.h string.h unistd.h sys/types.h sys/stat.h
 do
 as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
 if { as_var=$as_ac_Header; eval "test \"\${$as_var+set}\" = set"; }; then
@@ -4905,8 +4907,7 @@ esac
 
 
 
-
+for ac_func in atexit getcwd memmove memset strcasecmp strchr strdup strerror strncasecmp strrchr strstr strtol
-for ac_func in atexit fchmod getcwd memmove memset strcasecmp strchr strdup strerror strncasecmp strrchr strstr strtol
 do
 as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 { $as_echo "$as_me:$LINENO: checking for $ac_func" >&5

apache2/configure.in

@@ -26,7 +26,7 @@ AC_PATH_PROGS(ENV_CMD, [env printenv], )
 
 # Checks for header files.
 AC_HEADER_STDC
-AC_CHECK_HEADERS([fcntl.h limits.h stdlib.h string.h unistd.h])
+AC_CHECK_HEADERS([fcntl.h limits.h stdlib.h string.h unistd.h sys/types.h sys/stat.h])
 
 # Checks for typedefs, structures, and compiler characteristics.
 AC_C_CONST
@@ -40,7 +40,7 @@ AC_TYPE_UINT8_T
 # Checks for library functions.
 AC_FUNC_MALLOC
 AC_FUNC_MEMCMP
-AC_CHECK_FUNCS([atexit fchmod getcwd memmove memset strcasecmp strchr strdup strerror strncasecmp strrchr strstr strtol])
+AC_CHECK_FUNCS([atexit getcwd memmove memset strcasecmp strchr strdup strerror strncasecmp strrchr strstr strtol])
 
 # Some directories
 MSC_BASE_DIR=`pwd`

apache2/mod_security2_config.h.in

@@ -3,9 +3,6 @@
 /* Define to 1 if you have the `atexit' function. */
 #undef HAVE_ATEXIT
 
-/* Define to 1 if you have the `fchmod' function. */
-#undef HAVE_FCHMOD
-
 /* Define to 1 if you have the <fcntl.h> header file. */
 #undef HAVE_FCNTL_H
 

apache2/modsecurity.h

@@ -392,6 +392,10 @@ struct directory_config {
     /* AUDITLOG_SERIAL (single file) or AUDITLOG_CONCURRENT (multiple files) */
     int                  auditlog_type;
 
+    /* Mode for audit log directories and files */
+    apr_fileperms_t      auditlog_dirperms;
+    apr_fileperms_t      auditlog_fileperms;
+
     /* The name of the audit log file (for the old type), or the
      * name of the index file (for the new audit log type)
      */
@@ -425,7 +429,7 @@ struct directory_config {
     const char          *upload_dir;
     int                  upload_keep_files;
     int                  upload_validates_files;
-    int                  upload_filemode;
+    int                  upload_filemode; /* int only so NOT_SET works */
 
     /* Used only in the configuration phase. */
     msre_rule           *tmp_chain_starter;

apache2/msc_logging.c

@@ -16,6 +16,9 @@
  * directly using the email address support@breach.com.
  *
  */
+#include <sys/stat.h>
+
+#include "mod_security2_config.h"
 #include "re.h"
 #include "msc_logging.h"
 #include "httpd.h"
@@ -443,7 +446,7 @@ void sec_audit_logger(modsec_rec *msr) {
          * we could cache the time we last checked and don't check if we know
          * the folder is there.
          */
-        rc = apr_dir_make_recursive(entry_basename, CREATEMODE_DIR, msr->mp);
+        rc = apr_dir_make_recursive(entry_basename, msr->txcfg->auditlog_dirperms, msr->mp);
         if (rc != APR_SUCCESS) {
             msr_log(msr, 1, "Audit log: Failed to create subdirectories: %s (%s)",
                 entry_basename, get_apr_error(msr->mp, rc));
@@ -452,7 +455,7 @@ void sec_audit_logger(modsec_rec *msr) {
 
         rc = apr_file_open(&msr->new_auditlog_fd, entry_filename,
             APR_WRITE | APR_TRUNCATE | APR_CREATE | APR_BINARY | APR_FILE_NOCLEANUP,
-            CREATEMODE, msr->mp);
+            msr->txcfg->auditlog_fileperms, msr->mp);
         if (rc != APR_SUCCESS) {
             msr_log(msr, 1, "Audit log: Failed to create file: %s (%s)",
                 entry_filename, get_apr_error(msr->mp, rc));

apache2/msc_multipart.c

@@ -396,7 +396,7 @@ static int multipart_process_part_data(modsec_rec *msr, char **error_msg) {
                 /* construct temporary file name */
                 msr->mpd->mpp->tmp_file_name = apr_psprintf(msr->mp, "%s/%s-%s-file-XXXXXX",
                     msr->txcfg->tmp_dir, current_filetime(msr->mp), msr->txid);
-                msr->mpd->mpp->tmp_file_fd = msc_mkstemp(msr->mpd->mpp->tmp_file_name);
+                msr->mpd->mpp->tmp_file_fd = msc_mkstemp_ex(msr->mpd->mpp->tmp_file_name, msr->txcfg->upload_filemode);
 
                 /* do we have an opened file? */
                 if (msr->mpd->mpp->tmp_file_fd < 0) {
@@ -409,21 +409,6 @@ static int multipart_process_part_data(modsec_rec *msr, char **error_msg) {
                     msr_log(msr, 4, "Multipart: Created temporary file: %s",
                         log_escape_nq(msr->mp, msr->mpd->mpp->tmp_file_name));
                 }
-
-                #ifdef HAVE_FCHMOD
-                if (msr->txcfg->debuglog_level >= 9) {
-                    msr_log(msr, 9, "Multipart: Changing file mode to %04o: %s", msr->txcfg->upload_filemode, log_escape_nq(msr->mp, msr->mpd->mpp->tmp_file_name));
-                }
-                if (fchmod(msr->mpd->mpp->tmp_file_fd, msr->txcfg->upload_filemode) < 0) {
-
-                    char errbuf[256];
-                    if (msr->txcfg->debuglog_level >= 3) {
-                        msr_log(msr, 3, "Multipart: Could not change mode on \"%s\" (%d): %s",
-                            log_escape_nq(msr->mp, msr->mpd->mpp->tmp_file_name),
-                            errno, apr_strerror(APR_FROM_OS_ERROR(errno), errbuf, 256));
-                    }
-                }
-                #endif
             }
 
             /* write the reserve first */

apache2/msc_util.c

@@ -418,17 +418,24 @@ char *current_filetime(apr_pool_t *mp) {
 /**
  *
  */
-int msc_mkstemp(char *template) {
+int msc_mkstemp_ex(char *template, mode_t mode) {
     /* ENH Use apr_file_mktemp instead. */
 
     #if !(defined(WIN32)||defined(NETWARE))
     return mkstemp(template);
     #else
     if (mktemp(template) == NULL) return -1;
-    return open(template, O_WRONLY | O_APPEND | O_CREAT | O_BINARY, CREATEMODE_UNISTD);
+    return open(template, O_WRONLY | O_APPEND | O_CREAT | O_BINARY, mode);
     #endif
 }
 
+/**
+ *
+ */
+int msc_mkstemp(char *template) {
+    return msc_mkstemp_ex(template, CREATEMODE_UNISTD);
+}
+
 /**
  * Converts the input string to lowercase (in-place).
  */
@@ -1351,3 +1358,26 @@ int css_decode_inplace(unsigned char *input, long int input_len) {
 
     return count;
 }
+
+/**
+ * Translate UNIX octal umask/mode to APR apr_fileperms_t
+ */
+apr_fileperms_t mode2fileperms(mode_t mode) {
+    apr_fileperms_t perms = 0;
+
+    if (mode & S_IXOTH) perms |= APR_WEXECUTE;
+    if (mode & S_IWOTH) perms |= APR_WWRITE;
+    if (mode & S_IROTH) perms |= APR_WREAD;
+    if (mode & S_IXGRP) perms |= APR_GEXECUTE;
+    if (mode & S_IWGRP) perms |= APR_GWRITE;
+    if (mode & S_IRGRP) perms |= APR_GREAD;
+    if (mode & S_IXUSR) perms |= APR_UEXECUTE;
+    if (mode & S_IWUSR) perms |= APR_UWRITE;
+    if (mode & S_IRUSR) perms |= APR_UREAD;
+    if (mode & S_ISVTX) perms |= APR_WSTICKY;
+    if (mode & S_ISGID) perms |= APR_GSETID;
+    if (mode & S_ISUID) perms |= APR_USETID;
+
+    return perms;
+}
+

apache2/msc_util.h

@@ -19,6 +19,9 @@
 #ifndef _UTIL_H_
 #define _UTIL_H_
 
+#include <sys/types.h>
+#include <apr_file_info.h>
+
 #include "modsecurity.h"
 
 int DSOLOCAL normalise_path_inplace(unsigned char *input, int len, int win, int *changed);
@@ -53,6 +56,8 @@ char DSOLOCAL *current_logtime(apr_pool_t *mp);
 
 char DSOLOCAL *current_filetime(apr_pool_t *mp);
 
+int DSOLOCAL msc_mkstemp_ex(char *template, mode_t mode);
+
 int DSOLOCAL msc_mkstemp(char *template);
 
 char DSOLOCAL *strtolower_inplace(unsigned char *str);
@@ -94,4 +99,6 @@ char DSOLOCAL *resolve_relative_path(apr_pool_t *pool, const char *parent_filena
 
 int DSOLOCAL css_decode_inplace(unsigned char *input, long int input_len);
 
+apr_fileperms_t DSOLOCAL mode2fileperms(mode_t mode);
+
 #endif

doc/modsecurity2-apache-reference.xml

@@ -6,7 +6,7 @@
   Manual</title>
 
   <articleinfo>
-    <releaseinfo>Version 2.5.10-dev2 (Aug 12, 2009)</releaseinfo>
+    <releaseinfo>Version 2.5.10-dev3 (Aug 24, 2009)</releaseinfo>
 
     <copyright>
       <year>2004-2009</year>
@@ -698,6 +698,79 @@ SecAuditLogStorageDir logs/audit
       audit logging.</para>
     </section>
 
+    <section>
+      <title><literal>SecAuditLogDirMode</literal></title>
+
+      <para><emphasis>Description:</emphasis> Configures the mode
+      (permissions) of any directories created for concurrent audit logs using
+      an octal mode (as used in chmod). See <literal
+      moreinfo="none">SecAuditLogFileMode</literal> for controlling the mode
+      of audit log files.</para>
+
+      <para><emphasis>Syntax:</emphasis> <literal
+      moreinfo="none">SecAuditLogDirMode octal_mode|"default"</literal></para>
+
+      <para><emphasis>Example Usage:</emphasis> <literal
+      moreinfo="none">SecAuditLogDirMode 02750</literal></para>
+
+      <para><emphasis>Processing Phase:</emphasis> N/A</para>
+
+      <para><emphasis>Scope:</emphasis> Any</para>
+
+      <para><emphasis>Version:</emphasis> 2.5.10</para>
+
+      <para><emphasis>Dependencies/Notes:</emphasis> This feature is not
+      available on operating systems not supporting octal file modes. The
+      default mode (0600) only grants read/write access to the account writing
+      the file. If access from another account is needed (using mpm-itk is a
+      good example), then this directive may be required. However, use this
+      directive with caution to avoid exposing potentially sensitive data to
+      unauthorized users. Using the value "default" will revert back to the
+      default setting.</para>
+
+      <note>
+        <para>The process umask may still limit the mode if it is being more
+        restrictive than the mode set using this directive.</para>
+      </note>
+    </section>
+
+    <section>
+      <title><literal>SecAuditLogFileMode</literal></title>
+
+      <para><emphasis>Description:</emphasis> Configures the mode
+      (permissions) of any files created for concurrent audit logs using an
+      octal mode (as used in chmod). See <literal
+      moreinfo="none">SecAuditLogDirMode</literal> for controlling the mode of
+      created audit log directories.</para>
+
+      <para><emphasis>Syntax:</emphasis> <literal
+      moreinfo="none">SecAuditLogFileMode
+      octal_mode|"default"</literal></para>
+
+      <para><emphasis>Example Usage:</emphasis> <literal
+      moreinfo="none">SecAuditLogFileMode 00640</literal></para>
+
+      <para><emphasis>Processing Phase:</emphasis> N/A</para>
+
+      <para><emphasis>Scope:</emphasis> Any</para>
+
+      <para><emphasis>Version:</emphasis> 2.5.10</para>
+
+      <para><emphasis>Dependencies/Notes:</emphasis> This feature is not
+      available on operating systems not supporting octal file modes. The
+      default mode (0600) only grants read/write access to the account writing
+      the file. If access from another account is needed (using mpm-itk is a
+      good example), then this directive may be required. However, use this
+      directive with caution to avoid exposing potentially sensitive data to
+      unauthorized users. Using the value "default" will revert back to the
+      default setting.</para>
+
+      <note>
+        <para>The process umask may still limit the mode if it is being more
+        restrictive than the mode set using this directive.</para>
+      </note>
+    </section>
+
     <section>
       <title><literal>SecAuditLogParts</literal></title>
 
@@ -2400,7 +2473,7 @@ SecRuleUpdateActionById 12345 "t:compressWhitespace,deny,status:403,msg:'A new m
       <title><literal>SecUploadFileMode</literal></title>
 
       <para><emphasis>Description:</emphasis> Configures the mode
-      (permissions) of any uploaded files using an octal number (as used in
+      (permissions) of any uploaded files using an octal mode (as used in
       chmod).</para>
 
       <para><emphasis>Syntax:</emphasis> <literal
@@ -2423,6 +2496,11 @@ SecRuleUpdateActionById 12345 "t:compressWhitespace,deny,status:403,msg:'A new m
       directive with caution to avoid exposing potentially sensitive data to
       unauthorized users. Using the value "default" will revert back to the
       default setting.</para>
+
+      <note>
+        <para>The process umask may still limit the mode if it is being more
+        restrictive than the mode set using this directive.</para>
+      </note>
     </section>
 
     <section>
@@ -4757,9 +4835,9 @@ setvar:session.suspicious=1,<emphasis>expirevar:session.suspicious=3600</emphasi
 
       <para><emphasis>Note</emphasis></para>
 
-      <para>Normally you will want to use <emphasis>phase:1</emphasis>
+      <para>Normally you will want to use <emphasis>phase:1</emphasis> along
-      along with <emphasis>initcol</emphasis> so that the collection is
+      with <emphasis>initcol</emphasis> so that the collection is available in
-      available in all phases.</para>
+      all phases.</para>
 
       <para>Collections are loaded into memory when the initcol action is
       encountered. The collection in storage will be persisted (and the