From 2e9a35c35861c174f70bbff051e300137ab1012d Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 14 Dec 2016 10:09:53 -0300 Subject: [PATCH] Refactoring on the audit logs implementation Among of other things, it is now supporting shared file locks between different process. --- .../modsecurity}/audit_log.h | 51 +++- headers/modsecurity/rules.h | 1 - headers/modsecurity/rules_properties.h | 32 +-- src/Makefile.am | 7 +- src/audit_log/audit_log.cc | 133 ++++++--- src/audit_log/writer/https.cc | 6 +- src/audit_log/writer/https.h | 23 +- src/audit_log/writer/parallel.cc | 104 ++++--- src/audit_log/writer/parallel.h | 26 +- src/audit_log/writer/serial.cc | 25 +- src/audit_log/writer/serial.h | 32 +-- src/audit_log/{ => writer}/writer.cc | 24 +- src/audit_log/{ => writer}/writer.h | 50 ++-- src/parser/driver.cc | 11 +- src/parser/driver.h | 2 +- src/parser/seclang-parser.yy | 2 +- src/rules.cc | 26 +- src/transaction.cc | 5 +- src/utils/shared_files.cc | 253 ++++++++++++++++++ src/utils/shared_files.h | 83 ++++++ src/utils/system.cc | 15 +- src/utils/system.h | 2 +- .../regression/config-secdefaultaction.json | 2 +- test/test-cases/regression/issue-394.json | 6 +- 24 files changed, 661 insertions(+), 260 deletions(-) rename {src/audit_log => headers/modsecurity}/audit_log.h (83%) rename src/audit_log/{ => writer}/writer.cc (56%) rename src/audit_log/{ => writer}/writer.h (54%) create mode 100644 src/utils/shared_files.cc create mode 100644 src/utils/shared_files.h diff --git a/src/audit_log/audit_log.h b/headers/modsecurity/audit_log.h similarity index 83% rename from src/audit_log/audit_log.h rename to headers/modsecurity/audit_log.h index c2405146..e74e3c9f 100644 --- a/src/audit_log/audit_log.h +++ b/headers/modsecurity/audit_log.h @@ -19,16 +19,19 @@ #include #endif -#ifndef SRC_AUDIT_LOG_AUDIT_LOG_H_ -#define SRC_AUDIT_LOG_AUDIT_LOG_H_ +#ifndef HEADERS_MODSECURITY_AUDIT_LOG_H_ +#define HEADERS_MODSECURITY_AUDIT_LOG_H_ #include "modsecurity/transaction.h" -#include "src/audit_log/writer.h" + #ifdef __cplusplus namespace modsecurity { namespace audit_log { +namespace writer { +class Writer; +} /** @ingroup ModSecurity_CPP_API */ class AuditLog { @@ -36,16 +39,15 @@ class AuditLog { AuditLog(); ~AuditLog(); - void refCountIncrease(); - void refCountDecreaseAndCheck(); - enum AuditLogType { + NotSetAuditLogType, SerialAuditLogType, ParallelAuditLogType, HttpsAuditLogType }; enum AuditLogStatus { + NotSetLogStatus, OnAuditLogStatus, OffAuditLogStatus, RelevantOnlyAuditLogStatus @@ -149,10 +151,14 @@ class AuditLog { bool setFilePath2(const std::basic_string& path); bool setStorageDir(const std::basic_string& path); + int getDirectoryPermission(); + int getFilePermission(); + int getParts(); + bool setParts(const std::basic_string& new_parts); bool setType(AuditLogType audit_type); - bool init(); + bool init(std::string *error); bool close(); bool saveIfRelevant(Transaction *transaction); @@ -162,28 +168,49 @@ class AuditLog { int addParts(int parts, const std::string& new_parts); int removeParts(int parts, const std::string& new_parts); + bool merge(AuditLog *from, std::string *error); + std::string m_path1; std::string m_path2; std::string m_storage_dir; - int m_filePermission; - int m_directoryPermission; + void refCountIncrease() { + m_refereceCount++; + } + bool refCountDecreaseAndCheck() { + m_refereceCount--; + if (m_refereceCount == 0) { + delete this; + return true; + } + return false; + } + + protected: int m_parts; + int m_defaultParts = AAuditLogPart | BAuditLogPart | CAuditLogPart + | FAuditLogPart | HAuditLogPart | ZAuditLogPart; + + int m_filePermission; + int m_defaultFilePermission = 0600; + + int m_directoryPermission; + int m_defaultDirectoryPermission = 0766; private: AuditLogStatus m_status; - AuditLogType m_type; std::string m_relevant; - audit_log::Writer *m_writer; + audit_log::writer::Writer *m_writer; int m_refereceCount; }; + } // namespace audit_log } // namespace modsecurity #endif -#endif // SRC_AUDIT_LOG_AUDIT_LOG_H_ +#endif // HEADERS_MODSECURITY_AUDIT_LOG_H_ diff --git a/headers/modsecurity/rules.h b/headers/modsecurity/rules.h index 8b81a2f8..6d01e0ea 100644 --- a/headers/modsecurity/rules.h +++ b/headers/modsecurity/rules.h @@ -36,7 +36,6 @@ namespace modsecurity { class Rule; -class AuditLog; namespace Parser { class Driver; } diff --git a/headers/modsecurity/rules_properties.h b/headers/modsecurity/rules_properties.h index e8fee266..ce91cc40 100644 --- a/headers/modsecurity/rules_properties.h +++ b/headers/modsecurity/rules_properties.h @@ -32,20 +32,19 @@ #include "modsecurity/rule.h" #include "modsecurity/rules_exceptions.h" #include "modsecurity/actions/action.h" +#include "modsecurity/audit_log.h" #ifdef __cplusplus namespace modsecurity { class RulesExceptions; -namespace audit_log { -class AuditLog; -} namespace Parser { class Driver; } using modsecurity::debug_log::DebugLog; +using modsecurity::audit_log::AuditLog; /** @ingroup ModSecurity_CPP_API */ class ConfigInt { @@ -74,8 +73,9 @@ class ConfigString { class RulesProperties { public: - RulesProperties() : m_auditLog(NULL), + RulesProperties() : m_debugLog(new DebugLog()), + m_auditLog(new AuditLog()), m_remoteRulesActionOnFailed(PropertyNotSetRemoteRulesAction), m_secRequestBodyAccess(PropertyNotSetConfigBoolean), m_secResponseBodyAccess(PropertyNotSetConfigBoolean), @@ -87,8 +87,9 @@ class RulesProperties { m_tmpSaveUploadedFiles(PropertyNotSetConfigBoolean) { } - explicit RulesProperties(DebugLog *debugLog) : m_auditLog(NULL), + explicit RulesProperties(DebugLog *debugLog) : m_debugLog(debugLog), + m_auditLog(new AuditLog()), m_remoteRulesActionOnFailed(PropertyNotSetRemoteRulesAction), m_secRequestBodyAccess(PropertyNotSetConfigBoolean), m_secResponseBodyAccess(PropertyNotSetConfigBoolean), @@ -102,6 +103,7 @@ class RulesProperties { ~RulesProperties() { delete m_debugLog; + delete m_auditLog; } @@ -298,14 +300,6 @@ class RulesProperties { to->m_httpblKey.m_value = from->m_httpblKey.m_value; } - if (from->m_auditLogPath.m_set == true) { - to->m_auditLogPath.m_value = from->m_auditLogPath.m_value; - } - - if (from->m_auditLogParts.m_set == true) { - to->m_auditLogParts.m_value = from->m_auditLogParts.m_value; - } - to->m_exceptions.merge(from->m_exceptions); to->m_components.insert(to->m_components.end(), @@ -328,6 +322,16 @@ class RulesProperties { } } + + if (to->m_auditLog) { + std::string error; + to->m_auditLog->merge(from->m_auditLog, &error); + if (error.size() > 0) { + *err << error; + return -1; + } + } + if (from->m_debugLog && to->m_debugLog && from->m_debugLog->isLogFileSet()) { std::string error; @@ -412,8 +416,6 @@ class RulesProperties { std::list m_components; std::ostringstream m_parserError; std::set m_responseBodyTypeToBeInspected; - ConfigString m_auditLogParts; - ConfigString m_auditLogPath; ConfigString m_httpblKey; ConfigString m_uploadDirectory; ConfigString m_uploadTmpDirectory; diff --git a/src/Makefile.am b/src/Makefile.am index e15e2ef9..ed63d057 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -26,6 +26,7 @@ MAINTAINERCLEANFILES = \ pkginclude_HEADERS = \ ../headers/modsecurity/transaction.h \ ../headers/modsecurity/debug_log.h \ + ../headers/modsecurity/audit_log.h \ ../headers/modsecurity/intervention.h \ ../headers/modsecurity/modsecurity.h \ ../headers/modsecurity/rule.h \ @@ -53,7 +54,6 @@ noinst_HEADERS = \ actions/disruptive/*.h \ actions/transformations/*.h \ debug_log/*.h \ - audit_log/*.h \ audit_log/writer/*.h \ collection/backend/*.h \ operators/*.h \ @@ -220,7 +220,8 @@ UTILS = \ utils/regex.cc \ utils/sha1.cc \ utils/string.cc \ - utils/system.cc + utils/system.cc \ + utils/shared_files.cc @@ -242,7 +243,7 @@ libmodsecurity_la_SOURCES = \ parser/driver.cc \ transaction.cc \ audit_log/audit_log.cc \ - audit_log/writer.cc \ + audit_log/writer/writer.cc \ audit_log/writer/https.cc \ audit_log/writer/serial.cc \ audit_log/writer/parallel.cc \ diff --git a/src/audit_log/audit_log.cc b/src/audit_log/audit_log.cc index 523e9b4d..c26f7280 100644 --- a/src/audit_log/audit_log.cc +++ b/src/audit_log/audit_log.cc @@ -13,7 +13,7 @@ * */ -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" #include #include @@ -24,6 +24,7 @@ #include "src/audit_log/writer/https.h" #include "src/audit_log/writer/parallel.h" #include "src/audit_log/writer/serial.h" +#include "src/audit_log/writer/writer.h" #include "src/utils/regex.h" #define PARTS_CONSTAINS(a, c) \ @@ -38,40 +39,37 @@ parts = parts & ~c; \ } +#define AL_MERGE_STRING_CONF(a, c) \ + if (a.empty() == false) { \ + c = a; \ + } + + namespace modsecurity { namespace audit_log { + AuditLog::AuditLog() : m_path1(""), m_path2(""), m_storage_dir(""), - m_filePermission(0600), - m_directoryPermission(0766), - m_parts(AAuditLogPart | BAuditLogPart | CAuditLogPart | FAuditLogPart - | HAuditLogPart | ZAuditLogPart), - m_status(OffAuditLogStatus), - m_type(ParallelAuditLogType), + m_filePermission(-1), + m_directoryPermission(-1), + m_parts(-1), + m_status(NotSetLogStatus), + m_type(NotSetAuditLogType), m_relevant(""), m_writer(NULL), - m_refereceCount(0) { } + m_refereceCount(1) { } + AuditLog::~AuditLog() { if (m_writer) { - m_writer->refCountDecreaseAndCheck(); + delete m_writer; + m_writer = NULL; } } -void AuditLog::refCountIncrease() { - m_refereceCount++; -} - - -void AuditLog::refCountDecreaseAndCheck() { - m_refereceCount--; - if (m_refereceCount == 0) { - delete this; - } -} bool AuditLog::setStorageDirMode(int permission) { this->m_directoryPermission = permission; @@ -85,8 +83,24 @@ bool AuditLog::setFileMode(int permission) { } -bool AuditLog::setStatus(AuditLogStatus new_status) { - this->m_status = new_status; +int AuditLog::getFilePermission() { + if (m_filePermission == -1) { + return m_defaultFilePermission; + } + + return m_filePermission; +} + +int AuditLog::getDirectoryPermission() { + if (m_directoryPermission == -1) { + return m_defaultDirectoryPermission; + } + + return m_directoryPermission; +} + +bool AuditLog::setStatus(AuditLogStatus status) { + this->m_status = status; return true; } @@ -172,34 +186,46 @@ bool AuditLog::setParts(const std::basic_string& new_parts) { } +int AuditLog::getParts() { + if (m_parts == -1) { + return m_defaultParts; + } + + return m_parts; +} + + bool AuditLog::setType(AuditLogType audit_type) { this->m_type = audit_type; return true; } -bool AuditLog::init() { - if (m_type == ParallelAuditLogType) { - m_writer = new audit_log::writer::Parallel(this); - } + +bool AuditLog::init(std::string *error) { if (m_type == SerialAuditLogType) { m_writer = new audit_log::writer::Serial(this); - } - if (m_type == HttpsAuditLogType) { + } else if (m_type == HttpsAuditLogType) { m_writer = new audit_log::writer::Https(this); + } else { + /* + * if (m_type == ParallelAuditLogType + * || m_type == NotSetAuditLogType) + * + */ + m_writer = new audit_log::writer::Parallel(this); } - m_writer->refCountIncrease(); - if (m_writer == NULL || m_writer->init() == false) { - std::cout << "not able to open the log for write." << std::endl; + + if (m_writer == NULL || m_writer->init(error) == false) { return false; } /* Sanity check */ if (m_status == RelevantOnlyAuditLogStatus) { if (m_relevant.empty()) { - std::cout << "m_relevant cannot be null while status is " << \ - "RelevantOnly" << std::endl; + error->assign("m_relevant cannot be null while status is set to " \ + "RelevantOnly"); return false; } } @@ -256,7 +282,16 @@ bool AuditLog::saveIfRelevant(Transaction *transaction, int parts) { } transaction->debug(5, "Saving this request as part " \ "of the audit logs."); - m_writer->write(transaction, parts); + if (m_writer == NULL) { + transaction->debug(1, "Internal error, audit log writer is null"); + } else { + std::string error; + bool a = m_writer->write(transaction, parts, &error); + if (a == false) { + transaction->debug(1, "Cannot save the audit log: " + error); + return false; + } + } return true; } @@ -267,5 +302,35 @@ bool AuditLog::close() { } +bool AuditLog::merge(AuditLog *from, std::string *error) { + AL_MERGE_STRING_CONF(from->m_path1, m_path1); + AL_MERGE_STRING_CONF(from->m_path2, m_path2); + AL_MERGE_STRING_CONF(from->m_storage_dir, m_storage_dir); + AL_MERGE_STRING_CONF(from->m_relevant, m_relevant); + + if (from->m_filePermission != -1) { + m_filePermission = from->m_filePermission; + } + + if (from->m_directoryPermission != -1) { + m_directoryPermission = from->m_directoryPermission; + } + + if (from->m_type != NotSetAuditLogType) { + m_type = from->m_type; + } + + if (from->m_status != NotSetLogStatus) { + m_status = from->m_status; + } + + if (from->m_parts != -1) { + m_parts = from->m_parts; + } + + return init(error); +} + + } // namespace audit_log } // namespace modsecurity diff --git a/src/audit_log/writer/https.cc b/src/audit_log/writer/https.cc index 42c9b22f..7ceda404 100644 --- a/src/audit_log/writer/https.cc +++ b/src/audit_log/writer/https.cc @@ -25,7 +25,7 @@ #include #include -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" #include "modsecurity/transaction.h" #include "src/utils/md5.h" #include "src/utils/https_client.h" @@ -40,12 +40,12 @@ Https::~Https() { } -bool Https::init() { +bool Https::init(std::string *error) { return true; } -bool Https::write(Transaction *transaction, int parts) { +bool Https::write(Transaction *transaction, int parts, std::string *error) { Utils::HttpsClient m_http_client; transaction->debug(7, "Sending logs to: " + m_audit->m_path1); diff --git a/src/audit_log/writer/https.h b/src/audit_log/writer/https.h index 71a4fe4f..f802377e 100644 --- a/src/audit_log/writer/https.h +++ b/src/audit_log/writer/https.h @@ -22,7 +22,7 @@ #ifndef SRC_AUDIT_LOG_WRITER_HTTPS_H_ #define SRC_AUDIT_LOG_WRITER_HTTPS_H_ -#include "src/audit_log/writer.h" +#include "src/audit_log/writer/writer.h" #include "modsecurity/transaction.h" #ifdef __cplusplus @@ -32,27 +32,16 @@ namespace audit_log { namespace writer { /** @ingroup ModSecurity_CPP_API */ -class Https : public audit_log::Writer { +class Https : public Writer { public: explicit Https(audit_log::AuditLog *audit) - : audit_log::Writer(audit) { } + : audit_log::writer::Writer(audit) { } ~Https() override; - void refCountIncrease() override { - m_refereceCount++; - } - - - void refCountDecreaseAndCheck() override { - m_refereceCount--; - if (m_refereceCount == 0) { - delete this; - } - } - - bool init() override; - bool write(Transaction *transaction, int parts) override; + bool init(std::string *error) override; + bool write(Transaction *transaction, int parts, + std::string *error) override; }; } // namespace writer diff --git a/src/audit_log/writer/parallel.cc b/src/audit_log/writer/parallel.cc index c3391684..a1d177c3 100644 --- a/src/audit_log/writer/parallel.cc +++ b/src/audit_log/writer/parallel.cc @@ -25,7 +25,7 @@ #include #include -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" #include "modsecurity/transaction.h" #include "src/utils/system.h" #include "src/utils/md5.h" @@ -36,17 +36,9 @@ namespace audit_log { namespace writer { -std::mutex g_writeMutex; - - Parallel::~Parallel() { - if (log1.is_open()) { - log1.close(); - } - - if (log2.is_open()) { - log2.close(); - } + utils::SharedFiles::getInstance().close(m_audit->m_path1); + utils::SharedFiles::getInstance().close(m_audit->m_path2); } @@ -80,68 +72,106 @@ inline std::string Parallel::logFilePath(time_t *t, } -bool Parallel::init() { - /** TODO:: Check if the directory exists. */ - /** TODO:: Checking if we have permission to write in the target dir */ - +bool Parallel::init(std::string *error) { + bool ret = true; if (!m_audit->m_path1.empty()) { - log1.open(m_audit->m_path1, std::fstream::out | std::fstream::app); + ret = utils::SharedFiles::getInstance().open(m_audit->m_path1, error); + if (!ret) { + return false; + } } if (!m_audit->m_path2.empty()) { - log2.open(m_audit->m_path2, std::fstream::out | std::fstream::app); + ret = utils::SharedFiles::getInstance().open(m_audit->m_path2, error); + if (!ret) { + return false; + } + } + + if (m_audit->m_storage_dir.empty() == false) { + if (utils::createDir(m_audit->m_storage_dir, + m_audit->getDirectoryPermission(), error) == false) { + return false; + } } return true; } -bool Parallel::write(Transaction *transaction, int parts) { - std::lock_guard guard(g_writeMutex); +bool Parallel::write(Transaction *transaction, int parts, std::string *error) { FILE *fp; int fd; std::string log = transaction->toJSON(parts); std::string fileName = logFilePath(&transaction->m_timeStamp, YearMonthDayDirectory | YearMonthDayAndTimeDirectory | YearMonthDayAndTimeFileName); + bool ret; std::string logPath = m_audit->m_storage_dir; fileName = logPath + fileName + "-" + transaction->m_id; if (logPath.empty()) { - return false; + error->assign("Log path is not valid."); + return false; } - utils::createDir((logPath + + ret = utils::createDir((logPath + logFilePath(&transaction->m_timeStamp, YearMonthDayDirectory)).c_str(), - m_audit->m_directoryPermission); - utils::createDir((logPath + + m_audit->getDirectoryPermission(), + error); + if (ret == false) { + return false; + } + ret = utils::createDir((logPath + logFilePath(&transaction->m_timeStamp, YearMonthDayDirectory | YearMonthDayAndTimeDirectory)).c_str(), - m_audit->m_directoryPermission); + m_audit->getDirectoryPermission(), + error); + if (ret == false) { + return false; + } - fd = open(fileName.c_str(), O_CREAT | O_WRONLY, m_audit->m_filePermission); + fd = open(fileName.c_str(), O_CREAT | O_WRONLY, + m_audit->getFilePermission()); if (fd < 0) { + error->assign("Not able to open: " + fileName + ". " \ + + strerror(errno)); return false; } fp = fdopen(fd, "w"); fwrite(log.c_str(), log.length(), 1, fp); fclose(fp); - if (log1.is_open() && log2.is_open()) { - log2 << transaction->toOldAuditLogFormatIndex(fileName, log.length(), - Utils::Md5::hexdigest(log)); - log2.flush(); + if (m_audit->m_path1.empty() == false + && m_audit->m_path2.empty() == false) { + std::string msg = transaction->toOldAuditLogFormatIndex(fileName, + log.length(), Utils::Md5::hexdigest(log)); + ret = utils::SharedFiles::getInstance().write(m_audit->m_path2, msg, + error); + if (ret == false) { + return false; + } } - if (log1.is_open() && !log2.is_open()) { - log1 << transaction->toOldAuditLogFormatIndex(fileName, log.length(), - Utils::Md5::hexdigest(log)); - log1.flush(); + if (m_audit->m_path1.empty() == false + && m_audit->m_path2.empty() == true) { + std::string msg = transaction->toOldAuditLogFormatIndex(fileName, + log.length(), Utils::Md5::hexdigest(log)); + ret = utils::SharedFiles::getInstance().write(m_audit->m_path1, msg, + error); + if (ret == false) { + return false; + } } - if (!log1.is_open() && log2.is_open()) { - log2 << transaction->toOldAuditLogFormatIndex(fileName, log.length(), - Utils::Md5::hexdigest(log)); - log2.flush(); + if (m_audit->m_path1.empty() == true + && m_audit->m_path2.empty() == false) { + std::string msg = transaction->toOldAuditLogFormatIndex(fileName, + log.length(), Utils::Md5::hexdigest(log)); + ret = utils::SharedFiles::getInstance().write(m_audit->m_path2, msg, + error); + if (ret == false) { + return false; + } } return true; diff --git a/src/audit_log/writer/parallel.h b/src/audit_log/writer/parallel.h index d8fd8b42..38c6aee4 100644 --- a/src/audit_log/writer/parallel.h +++ b/src/audit_log/writer/parallel.h @@ -18,8 +18,10 @@ #ifndef SRC_AUDIT_LOG_WRITER_PARALLEL_H_ #define SRC_AUDIT_LOG_WRITER_PARALLEL_H_ -#include "src/audit_log/writer.h" +#include "src/audit_log/writer/writer.h" #include "modsecurity/transaction.h" +#include "modsecurity/audit_log.h" +#include "src/utils/shared_files.h" #ifdef __cplusplus @@ -28,26 +30,16 @@ namespace audit_log { namespace writer { /** @ingroup ModSecurity_CPP_API */ -class Parallel : public audit_log::Writer { +class Parallel : public Writer { public: explicit Parallel(AuditLog *audit) - : audit_log::Writer(audit) { } + : audit_log::writer::Writer(audit) { } ~Parallel() override; - bool init() override; - bool write(Transaction *transaction, int parts) override; + bool init(std::string *error) override; + bool write(Transaction *transaction, int parts, + std::string *error) override; - void refCountIncrease() override { - m_refereceCount++; - } - - - void refCountDecreaseAndCheck() override { - m_refereceCount--; - if (m_refereceCount == 0) { - delete this; - } - } /** * @@ -72,8 +64,6 @@ class Parallel : public audit_log::Writer { YearMonthDayAndTimeFileName = 8, }; - std::ofstream log1; - std::ofstream log2; inline std::string logFilePath(time_t *t, int part); }; diff --git a/src/audit_log/writer/serial.cc b/src/audit_log/writer/serial.cc index 556ed4c7..c066ae6c 100644 --- a/src/audit_log/writer/serial.cc +++ b/src/audit_log/writer/serial.cc @@ -15,9 +15,7 @@ #include "src/audit_log/writer/serial.h" -// #include - -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" namespace modsecurity { namespace audit_log { @@ -26,7 +24,7 @@ namespace writer { Serial::~Serial() { - m_log.close(); + utils::SharedFiles::getInstance().close(m_audit->m_path1); } @@ -42,25 +40,20 @@ void Serial::generateBoundary(std::string *boundary) { } -bool Serial::init() { - m_log.open(m_audit->m_path1, std::fstream::out | std::fstream::app); - return true; +bool Serial::init(std::string *error) { + return utils::SharedFiles::getInstance().open(m_audit->m_path1, error); } -bool Serial::write(Transaction *transaction, int parts) { +bool Serial::write(Transaction *transaction, int parts, std::string *error) { std::string boundary; + std::string msg; generateBoundary(&boundary); + msg = transaction->toOldAuditLogFormat(parts, "-" + boundary + "--"); - // serialLoggingMutex.lock(); - - m_log << transaction->toOldAuditLogFormat(parts, "-" + boundary + "--"); - m_log.flush(); - - // serialLoggingMutex.unlock(); - - return true; + return utils::SharedFiles::getInstance().write(m_audit->m_path1, msg, + error); } } // namespace writer diff --git a/src/audit_log/writer/serial.h b/src/audit_log/writer/serial.h index 28d2df3e..2ee075cf 100644 --- a/src/audit_log/writer/serial.h +++ b/src/audit_log/writer/serial.h @@ -22,8 +22,10 @@ #ifndef SRC_AUDIT_LOG_WRITER_SERIAL_H_ #define SRC_AUDIT_LOG_WRITER_SERIAL_H_ -#include "src/audit_log/writer.h" +#include "src/audit_log/writer/writer.h" +#include "src/utils/shared_files.h" #include "modsecurity/transaction.h" +#include "modsecurity/audit_log.h" #ifdef __cplusplus @@ -33,37 +35,21 @@ namespace writer { #define SERIAL_AUDIT_LOG_BOUNDARY_LENGTH 8 + /** @ingroup ModSecurity_CPP_API */ -class Serial : public audit_log::Writer { +class Serial : public Writer { public: explicit Serial(audit_log::AuditLog *audit) - : audit_log::Writer(audit) { } + : audit_log::writer::Writer(audit) { } ~Serial() override; - void refCountIncrease() override { - m_refereceCount++; - } - - void refCountDecreaseAndCheck() override { - /* - m_refereceCount--; - - - if (m_refereceCount == 0) { - */ - delete this; - /* - /} - */ - } - - bool init() override;; - bool write(Transaction *transaction, int parts) override; + bool init(std::string *error) override; + bool write(Transaction *transaction, int parts, + std::string *error) override; private: - std::ofstream m_log; void generateBoundary(std::string *boundary); }; diff --git a/src/audit_log/writer.cc b/src/audit_log/writer/writer.cc similarity index 56% rename from src/audit_log/writer.cc rename to src/audit_log/writer/writer.cc index f17e25b9..0606f162 100644 --- a/src/audit_log/writer.cc +++ b/src/audit_log/writer/writer.cc @@ -13,32 +13,18 @@ * */ -#include "src/audit_log/writer.h" +#include "src/audit_log/writer/writer.h" #include -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" namespace modsecurity { namespace audit_log { - -std::string Writer::file_name(const std::string& unique_id) { - time_t timer; - time(&timer); - - /** TODO: return file with time stamp and etc. */ - return std::string("/tmp/temp_audit_log_file.txt"); -} -/** - * - * Temporary print the log into the std::cout to debug purposes. - * - */ -bool Writer::write(Transaction *transaction, int parts) { - std::cout << transaction->toJSON(parts) << std::endl; - return true; -} +namespace writer { + +} // namespace writer } // namespace audit_log } // namespace modsecurity diff --git a/src/audit_log/writer.h b/src/audit_log/writer/writer.h similarity index 54% rename from src/audit_log/writer.h rename to src/audit_log/writer/writer.h index 48600b8f..381d9252 100644 --- a/src/audit_log/writer.h +++ b/src/audit_log/writer/writer.h @@ -13,47 +13,67 @@ * */ -#ifndef SRC_AUDIT_LOG_WRITER_H_ -#define SRC_AUDIT_LOG_WRITER_H_ +#ifndef SRC_AUDIT_LOG_WRITER_WRITER_H_ +#define SRC_AUDIT_LOG_WRITER_WRITER_H_ -#ifdef __cplusplus + +#include +#include +#include +#include + +#include #include -#endif +#include +#include + #include "modsecurity/transaction.h" +#include "modsecurity/audit_log.h" -#ifdef __cplusplus - namespace modsecurity { namespace audit_log { +namespace writer { + -class AuditLog; /** @ingroup ModSecurity_CPP_API */ class Writer { public: explicit Writer(AuditLog *audit) : m_audit(audit), - m_refereceCount(0) { } + m_refereceCount(1) { } virtual ~Writer() { } - virtual void refCountIncrease() = 0; - virtual void refCountDecreaseAndCheck() = 0; + virtual bool init(std::string *error) = 0; + virtual bool write(Transaction *transaction, int parts, + std::string *error) = 0; - virtual bool init() { return true; } - virtual bool write(Transaction *transaction, int parts); - std::string file_name(const std::string& unique_id); + void refCountIncrease() { + m_refereceCount++; + } + + + bool refCountDecreaseAndCheck() { + m_refereceCount--; + if (m_refereceCount == 0) { + delete this; + return true; + } + return false; + } protected: AuditLog *m_audit; int m_refereceCount; }; + +} // namespace writer } // namespace audit_log } // namespace modsecurity -#endif -#endif // SRC_AUDIT_LOG_WRITER_H_ +#endif // SRC_AUDIT_LOG_WRITER_WRITER_H_ diff --git a/src/parser/driver.cc b/src/parser/driver.cc index 274b2db4..f021de8b 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -16,7 +16,7 @@ #include "src/parser/driver.h" #include "src/parser/seclang-parser.hh" -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" #include "modsecurity/rules_properties.h" using modsecurity::audit_log::AuditLog; @@ -116,6 +116,7 @@ int Driver::addSecRule(Rule *rule) { int Driver::parse(const std::string &f, const std::string &ref) { + std::string error; lastRule = NULL; loc.push_back(new yy::location()); if (ref.empty()) { @@ -131,11 +132,13 @@ int Driver::parse(const std::string &f, const std::string &ref) { int res = parser.parse(); scan_end(); - if (m_auditLog->init() == false) { - m_parserError << "Problems while initializing the audit logs" \ - << std::endl; + /* + if (m_auditLog->init(&error) == false) { + m_parserError << "Problems while initializing the audit logs: " \ + << error << std::endl; return false; } + */ return res == 0; } diff --git a/src/parser/driver.h b/src/parser/driver.h index 04e2d6aa..73801062 100644 --- a/src/parser/driver.h +++ b/src/parser/driver.h @@ -27,7 +27,7 @@ #include "modsecurity/modsecurity.h" #include "modsecurity/rules.h" #include "modsecurity/rules_properties.h" -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" #include "src/parser/seclang-parser.hh" diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index 7edeb03b..72dddb43 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -49,7 +49,7 @@ class Driver; #include "src/actions/transformations/transformation.h" #include "src/actions/ver.h" #include "src/actions/xmlns.h" -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" #include "modsecurity/modsecurity.h" #include "modsecurity/rules_properties.h" #include "modsecurity/rule.h" diff --git a/src/rules.cc b/src/rules.cc index d0489434..50335497 100644 --- a/src/rules.cc +++ b/src/rules.cc @@ -97,10 +97,7 @@ Rules::~Rules() { tmp->pop_back(); } } - /** Cleanup audit log */ - if (m_auditLog) { - m_auditLog->refCountDecreaseAndCheck(); - } + free(unicode_map_table); } @@ -250,17 +247,6 @@ int Rules::merge(Driver *from) { dynamic_cast(this), &m_parserError); - if (from->m_auditLog != NULL && this->m_auditLog != NULL) { - this->m_auditLog->refCountDecreaseAndCheck(); - } - - if (from->m_auditLog) { - this->m_auditLog = from->m_auditLog; - } - if (this->m_auditLog != NULL) { - this->m_auditLog->refCountIncrease(); - } - return amount_of_rules; } @@ -272,16 +258,6 @@ int Rules::merge(Rules *from) { dynamic_cast(this), &m_parserError); - if (from->m_auditLog != NULL && this->m_auditLog != NULL) { - this->m_auditLog->refCountDecreaseAndCheck(); - } - if (from->m_auditLog) { - this->m_auditLog = from->m_auditLog; - } - if (this->m_auditLog != NULL) { - this->m_auditLog->refCountIncrease(); - } - return amount_of_rules; } diff --git a/src/transaction.cc b/src/transaction.cc index 463d7fe5..5b7ce8e1 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -39,7 +39,7 @@ #include "src/request_body_processor/multipart.h" #include "src/request_body_processor/xml.h" #include "src/request_body_processor/json.h" -#include "src/audit_log/audit_log.h" +#include "modsecurity/audit_log.h" #include "src/unique_id.h" #include "src/utils/string.h" #include "src/utils/system.h" @@ -1239,7 +1239,7 @@ int Transaction::processLogging() { /* If relevant, save this transaction information at the audit_logs */ if (m_rules != NULL && m_rules->m_auditLog != NULL) { - int parts = -1; + int parts = this->m_rules->m_auditLog->getParts(); #ifndef NO_LOGS debug(8, "Checking if this request is suitable to be " \ "saved as an audit log."); @@ -1250,7 +1250,6 @@ int Transaction::processLogging() { debug(4, "There was an audit log modifier for this transaction."); #endif std::list>::iterator it; - parts = this->m_rules->m_auditLog->m_parts; debug(7, "AuditLog parts before modification(s): " + std::to_string(parts) + "."); for (it = m_auditLogModifier.begin(); diff --git a/src/utils/shared_files.cc b/src/utils/shared_files.cc new file mode 100644 index 00000000..ef66aaa5 --- /dev/null +++ b/src/utils/shared_files.cc @@ -0,0 +1,253 @@ +/* + * ModSecurity, http://www.modsecurity.org/ + * Copyright (c) 2015 Trustwave Holdings, Inc. (http://www.trustwave.com/) + * + * You may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * If any of the files related to licensing are missing or if you have any + * other questions related to licensing please contact Trustwave Holdings, Inc. + * directly using the email address security@modsecurity.org. + * + */ + +#include "src/utils/shared_files.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +#include +#include + +namespace modsecurity { +namespace utils { + + +msc_file_handler_t *SharedFiles::find_handler( + const std::string &fileName) { + msc_file_handler_t *current = m_first; + while (current != NULL) { + if (current->file_name == fileName) { + return current; + } + current = reinterpret_cast(current->next); + } + + return NULL; +} + + +msc_file_handler_t *SharedFiles::add_new_handler( + const std::string &fileName, std::string *error) { + msc_file_handler_t *current = m_first; + int shm_id; + key_t mem_key_structure; + key_t mem_key_file_name; + msc_file_handler_t *new_debug_log; + char *shm_ptr2; + FILE *fp; + + fp = fopen(fileName.c_str(), "a"); + if (fp == 0) { + error->assign("Failed to open file: " + fileName); + goto err_fh; + } + + mem_key_structure = ftok(fileName.c_str(), 1); + if (mem_key_structure < 0) { + error->assign("Failed to select key for the shared memory (1): "); + error->append(strerror(errno)); + goto err_mem_key; + } + + mem_key_file_name = ftok(fileName.c_str(), 2); + if (mem_key_file_name < 0) { + error->assign("Failed to select key for the shared memory (2): "); + error->append(strerror(errno)); + goto err_mem_key; + } + + shm_id = shmget(mem_key_structure, sizeof (msc_file_handler_t), + IPC_CREAT | 0666); + if (shm_id < 0) { + error->assign("Failed to allocate shared memory (1): "); + error->append(strerror(errno)); + goto err_shmget1; + } + + new_debug_log = reinterpret_cast( + shmat(shm_id, NULL, 0)); + if ((reinterpret_cast(new_debug_log)[0]) == -1) { + error->assign("Failed to attach shared memory (1): "); + error->append(strerror(errno)); + goto err_shmat1; + } + memset(new_debug_log, '\0', sizeof(msc_file_handler_t)); + + pthread_mutex_init(&new_debug_log->lock, NULL); + new_debug_log->fp = fp; + new_debug_log->file_handler = fileno(new_debug_log->fp); + new_debug_log->next = NULL; + new_debug_log->previous = NULL; + new_debug_log->shm_id_structure = shm_id; + shm_id = shmget(mem_key_file_name, (fileName.size() + 1 * sizeof(char)), + IPC_CREAT | 0666); + if (shm_id < 0) { + error->assign("Failed to allocate shared memory (2): "); + error->append(strerror(errno)); + goto err_shmget2; + } + new_debug_log->shm_id_file_name = shm_id; + shm_ptr2 = reinterpret_cast(shmat(shm_id, NULL, 0)); + if (shm_ptr2[0] == -1) { + error->assign("Failed to attach shared memory (2): "); + error->append(strerror(errno)); + goto err_shmat2; + } + memcpy(shm_ptr2, fileName.c_str(), fileName.size()); + shm_ptr2[fileName.size()] = '\0'; + + new_debug_log->file_name = shm_ptr2; + + if (m_first == NULL) { + m_first = new_debug_log; + } else { + current = m_first; + while (current != NULL) { + if (current->next == NULL) { + current->next = new_debug_log; + new_debug_log->previous = current; + new_debug_log->next = NULL; + current = NULL; + } else { + current = reinterpret_cast( + current->next); + } + } + } + + return new_debug_log; +err_shmget2: +err_shmat2: + shmdt(shm_ptr2); + fclose(new_debug_log->fp); +err_shmget1: +err_shmat1: + shmdt(new_debug_log); +err_mem_key: +err_fh: + return NULL; +} + + +bool SharedFiles::open(const std::string& fileName, std::string *error) { + msc_file_handler_t *a = find_handler(fileName); + if (a == NULL) { + a = add_new_handler(fileName, error); + if (error->size() > 0) { + return false; + } + } + if (a == NULL) { + error->assign("Not able to open: " + fileName); + return false; + } + + a->using_it++; + + return true; +} + + +void SharedFiles::close(const std::string& fileName) { + msc_file_handler_t *a; + + if (fileName.empty()) { + return; + } + + a = find_handler(fileName); + if (a == NULL) { + return; + } + + a->using_it--; + + if (a->using_it == 0) { + bool first = false; + int shm_id1 = a->shm_id_structure; + int shm_id2 = a->shm_id_file_name; + msc_file_handler_t *p , *n; + pthread_mutex_lock(&a->lock); + fclose(a->fp); + + p = reinterpret_cast(a->previous); + n = reinterpret_cast(a->next); + if (p != NULL) { + p->next = reinterpret_cast(n); + } + if (n != NULL) { + n->previous = reinterpret_cast(p); + } + a->previous = NULL; + a->next = NULL; + pthread_mutex_unlock(&a->lock); + pthread_mutex_destroy(&a->lock); + + if (a->file_name == m_first->file_name) { + first = true; + } + + shmdt(a->file_name); + shmdt(a); + + shmctl(shm_id1, IPC_RMID, NULL); + shmctl(shm_id2, IPC_RMID, NULL); + + if (first) { + m_first = NULL; + } + a = NULL; + } +} + + +bool SharedFiles::write(const std::string& fileName, + const std::string &msg, std::string *error) { + std::string lmsg = msg; + size_t wrote; + bool ret = true; + + msc_file_handler_t *a = find_handler(fileName); + if (a == NULL) { + error->assign("file is not open: " + fileName); + return false; + } + + pthread_mutex_lock(&a->lock); + wrote = fwrite(reinterpret_cast(lmsg.c_str()), 1, + lmsg.size(), a->fp); + if (wrote < msg.size()) { + error->assign("failed to write: " + fileName); + ret = false; + } + pthread_mutex_unlock(&a->lock); + + return ret; +} + + +} // namespace utils +} // namespace modsecurity diff --git a/src/utils/shared_files.h b/src/utils/shared_files.h new file mode 100644 index 00000000..7d71089c --- /dev/null +++ b/src/utils/shared_files.h @@ -0,0 +1,83 @@ +/* + * ModSecurity, http://www.modsecurity.org/ + * Copyright (c) 2015 Trustwave Holdings, Inc. (http://www.trustwave.com/) + * + * You may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * If any of the files related to licensing are missing or if you have any + * other questions related to licensing please contact Trustwave Holdings, Inc. + * directly using the email address security@modsecurity.org. + * + */ + +#ifndef SRC_UTILS_SHARED_FILES_H_ +#define SRC_UTILS_SHARED_FILES_H_ + + +#include +#include +#include +#include + +#include + +#include "modsecurity/transaction.h" +#include "modsecurity/audit_log.h" + + +namespace modsecurity { +namespace utils { + + +typedef struct msc_file_handler { + char *file_name; + FILE *fp; + int file_handler; + int shm_id_file_name; + int shm_id_structure; + int using_it; + pthread_mutex_t lock; + void *next; + void *previous; +} msc_file_handler_t; + + +class SharedFiles { + public: + bool open(const std::string& fileName, std::string *error); + void close(const std::string& fileName); + bool write(const std::string& fileName, const std::string &msg, + std::string *error); + static SharedFiles& getInstance() { + static SharedFiles instance; + return instance; + } + + protected: + msc_file_handler_t *find_handler(const std::string &fileName); + msc_file_handler_t *add_new_handler(const std::string &fileName, + std::string *error); + + private: + SharedFiles() : m_first(NULL) { } + ~SharedFiles() { } + + // C++ 03 + // ======== + // Dont forget to declare these two. You want to make sure they + // are unacceptable otherwise you may accidentally get copies of + // your singleton appearing. + SharedFiles(SharedFiles const&); + void operator=(SharedFiles const&); + + msc_file_handler_t *m_first; +}; + + +} // namespace utils +} // namespace modsecurity + +#endif // SRC_UTILS_SHARED_FILES_H_ diff --git a/src/utils/system.cc b/src/utils/system.cc index 1e07d628..30381405 100644 --- a/src/utils/system.cc +++ b/src/utils/system.cc @@ -122,12 +122,15 @@ std::list expandEnv(const std::string& var, int flags) { } -void createDir(std::string dir, int mode) { -#if defined _MSC_VER - _mkdir(dir.data()); -#elif defined __GNUC__ - mkdir(dir.data(), mode); -#endif +bool createDir(std::string dir, int mode, std::string *error) { + int ret = mkdir(dir.data(), mode); + if (ret != 0 && errno != EEXIST) { + error->assign("Not able to create directory: " + dir + ": " \ + + strerror(errno) + "."); + return false; + } + + return true; } diff --git a/src/utils/system.h b/src/utils/system.h index e6366427..fd03b35d 100644 --- a/src/utils/system.h +++ b/src/utils/system.h @@ -32,7 +32,7 @@ double cpu_seconds(void); std::string find_resource(const std::string& file, const std::string& param); std::string get_path(const std::string& file); std::list expandEnv(const std::string& var, int flags); -void createDir(std::string dir, int mode); +bool createDir(std::string dir, int mode, std::string *error); } // namespace utils diff --git a/test/test-cases/regression/config-secdefaultaction.json b/test/test-cases/regression/config-secdefaultaction.json index fa11c456..45bbdec6 100644 --- a/test/test-cases/regression/config-secdefaultaction.json +++ b/test/test-cases/regression/config-secdefaultaction.json @@ -272,7 +272,7 @@ }, "expected":{ "audit_log":"", - "debug_log":"Request was relevant to be saved.", + "debug_log":"Saving this request as part of the audit log.", "http_code": 302 }, "rules":[ diff --git a/test/test-cases/regression/issue-394.json b/test/test-cases/regression/issue-394.json index 177792a9..82827ac8 100644 --- a/test/test-cases/regression/issue-394.json +++ b/test/test-cases/regression/issue-394.json @@ -32,11 +32,7 @@ "rules": [ "SecRuleEngine On", "SecRequestBodyAccess On", - "SecResponseBodyAccess On", - "SecAuditEngine On", - "SecAuditLogType Serial", - "SecAuditLog logs\/modsec_audit.log", - "SecAuditLogParts ABCDEFHJKZ" + "SecResponseBodyAccess On" ] } ]