fix: close the read end of piped logs in every child

apache2/apache2.h

@@ -95,5 +95,7 @@ char DSOLOCAL *format_error_log_message(apr_pool_t *mp, error_message_t *em);
 const DSOLOCAL char *get_response_protocol(request_rec *r);
 
 
+void DSOLOCAL modsec_close_piped_log_read_fds(void);
+
 #endif
 

apache2/apache2_config.c

@@ -214,7 +214,7 @@
                             char *my_error_msg = NULL;
 
                             int rc = msc_regexec(exceptions[j]->param_data,
                                     rule->actionset->msg, strlen(rule->actionset->msg),
                                     &my_error_msg);
                             if (rc >= 0) copy--;
                         }
@@ -234,7 +234,7 @@
                                 if(strcmp("tag", action->metadata->name) == 0)  {
 
                                     int rc = msc_regexec(exceptions[j]->param_data,
                                             action->param, strlen(action->param),
                                             &my_error_msg);
                                     if (rc >= 0) copy--;
                                 }
@@ -754,7 +754,7 @@
     /* Hash */
     if (dcfg->hash_is_enabled == HASH_ENABLED) {
         if (dcfg->crypto_key == NOT_SET_P) dcfg->crypto_key = getkey(dcfg->mp);
         if (dcfg->crypto_key_len == NOT_SET) dcfg->crypto_key_len = strlen(dcfg->crypto_key);
     } else {
         if (dcfg->crypto_key == NOT_SET_P) dcfg->crypto_key = "";
         if (dcfg->crypto_key_len == NOT_SET) dcfg->crypto_key_len = 0;
@@ -1222,11 +1222,11 @@
     assert(p1 != NULL);
     directory_config *dcfg = _dcfg;
 
     if (strcasecmp(p1, "On") == 0) dcfg->auditlog_flag = AUDITLOG_ON;
     else
         if (strcasecmp(p1, "Off") == 0) dcfg->auditlog_flag = AUDITLOG_OFF;
         else
             if (strcasecmp(p1, "RelevantOnly") == 0) dcfg->auditlog_flag = AUDITLOG_RELEVANT;
             else
                 return (const char *)apr_psprintf(cmd->pool,
                         "ModSecurity: Unrecognised parameter value for SecAuditEngine: %s", p1);
@@ -1234,6 +1234,47 @@
     return NULL;
 }
 
+/**
+ * Piped logs are opened while the configuration is read, so unlike the
+ * ones httpd opens in its open_logs hook their read ends are not on the
+ * list httpd closes in every child. A child that keeps the read end of a
+ * pipe can never see EPIPE when the logger dies, and blocks in write()
+ * for ever after a graceful restart (#2822). Remember the read ends here
+ * and close them from the child_init hook.
+ */
+static apr_array_header_t *piped_log_read_fds = NULL;
+
+static apr_status_t piped_log_read_fds_cleanup(void *data) {
+    piped_log_read_fds = NULL;
+    return APR_SUCCESS;
+}
+
+static void piped_log_remember_read_fd(apr_pool_t *p, piped_log *pl) {
+    apr_file_t *fd = ap_piped_log_read_fd(pl);
+
+    if (fd == NULL) return;
+
+    if (piped_log_read_fds == NULL) {
+        piped_log_read_fds = apr_array_make(p, 4, sizeof(apr_file_t *));
+        apr_pool_cleanup_register(p, NULL, piped_log_read_fds_cleanup,
+            apr_pool_cleanup_null);
+    }
+    *(apr_file_t **)apr_array_push(piped_log_read_fds) = fd;
+}
+
+void modsec_close_piped_log_read_fds(void) {
+    int i;
+
+    if (piped_log_read_fds == NULL) return;
+
+    for (i = 0; i < piped_log_read_fds->nelts; i++) {
+        apr_file_t *fd = ((apr_file_t **)piped_log_read_fds->elts)[i];
+        if (fd != NULL) {
+            apr_file_close(fd);
+        }
+    }
+}
+
 static const char *cmd_audit_log(cmd_parms *cmd, void *_dcfg, const char *p1)
 {
     assert(cmd != NULL);
@@ -1253,6 +1294,7 @@
                     pipe_name);
         }
         dcfg->auditlog_fd = ap_piped_log_write_fd(pipe_log);
+        piped_log_remember_read_fd(cmd->pool, pipe_log);
     }
     else {
         const char *file_name = ap_server_root_relative(cmd->pool, dcfg->auditlog_name);
@@ -1297,6 +1339,7 @@
                     pipe_name);
         }
         dcfg->auditlog2_fd = ap_piped_log_write_fd(pipe_log);
+        piped_log_remember_read_fd(cmd->pool, pipe_log);
     }
     else {
         const char *file_name = ap_server_root_relative(cmd->pool, dcfg->auditlog2_name);
@@ -1362,9 +1405,9 @@
     assert(p1 != NULL);
     directory_config *dcfg = _dcfg;
 
     if (strcasecmp(p1, "Serial") == 0) dcfg->auditlog_type = AUDITLOG_SERIAL;
     else
         if (strcasecmp(p1, "Concurrent") == 0) dcfg->auditlog_type = AUDITLOG_CONCURRENT;
         else
             return (const char *)apr_psprintf(cmd->pool,
                     "ModSecurity: Unrecognised parameter value for SecAuditLogType: %s", p1);
@@ -1381,9 +1424,9 @@
     assert(p1 != NULL);
     directory_config *dcfg = _dcfg;
 
     if (strcasecmp(p1, "JSON") == 0) dcfg->auditlog_format = AUDITLOGFORMAT_JSON;
     else
         if (strcasecmp(p1, "Native") == 0) dcfg->auditlog_format = AUDITLOGFORMAT_NATIVE;
         else
             return (const char *)apr_psprintf(cmd->pool,
                     "ModSecurity: Unrecognised parameter value for SecAuditLogFormat: %s", p1);
@@ -1405,7 +1448,7 @@
     }
     directory_config *dcfg = (directory_config *)_dcfg;
 
     if (strcasecmp(p1, "default") == 0) {
         dcfg->auditlog_dirperms = NOT_SET;
     }
     else {
@@ -1433,7 +1476,7 @@
     }
     directory_config *dcfg = (directory_config *)_dcfg;
 
     if (strcasecmp(p1, "default") == 0) {
         dcfg->auditlog_fileperms = NOT_SET;
     }
     else {
@@ -1491,7 +1534,7 @@
 
     chroot_dir = (char *)p1;
 
     if (getcwd(cwd, 1024) == NULL) {
         return "ModSecurity: Failed to get the current working directory";
     }
 
@@ -1746,6 +1789,7 @@
                 pipe_name);
         }
         guardianlog_fd = ap_piped_log_write_fd(pipe_log);
+        piped_log_remember_read_fd(cmd->pool, pipe_log);
     }
     else {
         const char *file_name = ap_server_root_relative(cmd->pool, guardianlog_name);

apache2/mod_security2.c

@@ -196,7 +196,7 @@
             msc_string *var = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));
 
             var->value = (char *)actionset->intercept_pause;
             var->value_len = strlen(actionset->intercept_pause);
             expand_macros(msr, var, NULL, msr->mp);
 
             pause = atoi(var->value);
@@ -343,7 +343,7 @@
                 msc_string *var = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));
 
                 var->value = (char *)actionset->intercept_uri;
                 var->value_len = strlen(actionset->intercept_uri);
                 expand_macros(msr, var, NULL, msr->mp);
 
                 apr_table_setn(msr->r->headers_out, "Location", var->value);
@@ -844,6 +844,8 @@
  * Initialisation performed for every new child process.
  */
 static void hook_child_init(apr_pool_t *mp, server_rec *s) {
+    /* Drop the read ends of our piped logs, as httpd does for its own. */
+    modsec_close_piped_log_read_fds();
     modsecurity_child_init(modsecurity);
 }
 
@@ -1217,7 +1219,7 @@
         }
     }
 
     limit = limit - strlen(str2) - 5;
     if (limit <= 0) {
         msr_log(msr, 1, "Audit Log: Atomic PIPE write buffer too small: %d", PIPE_BUF);
         return;