Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support reopening audit logs #2304

Open
wants to merge 2 commits into
base: v3/master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions headers/modsecurity/audit_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ class AuditLog {
bool setType(AuditLogType audit_type);

bool init(std::string *error);
bool reopen(std::string *error);
virtual bool close();

bool saveIfRelevant(Transaction *transaction);
Expand Down
1 change: 1 addition & 0 deletions headers/modsecurity/rules_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ int msc_rules_add_remote(RulesSet *rules, const char *key, const char *uri,
int msc_rules_add_file(RulesSet *rules, const char *file, const char **error);
int msc_rules_add(RulesSet *rules, const char *plain_rules, const char **error);
int msc_rules_cleanup(RulesSet *rules);
int msc_rules_reopen_audit_log(RulesSet *rules, const char **error);

#ifdef __cplusplus
}
Expand Down
7 changes: 7 additions & 0 deletions src/audit_log/audit_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,13 @@ bool AuditLog::merge(AuditLog *from, std::string *error) {
return init(error);
}

bool AuditLog::reopen(std::string *error) {
if (m_writer != NULL) {
return m_writer->reopen(error);
}
return true;
}


} // namespace audit_log
} // namespace modsecurity
3 changes: 3 additions & 0 deletions src/audit_log/writer/https.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ class Https : public Writer {
bool init(std::string *error) override;
bool write(Transaction *transaction, int parts,
std::string *error) override;
bool reopen(std::string *error) override {
return true;
}
};

} // namespace writer
Expand Down
32 changes: 32 additions & 0 deletions src/audit_log/writer/parallel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,38 @@ bool Parallel::write(Transaction *transaction, int parts, std::string *error) {
return true;
}


bool Parallel::reopen(std::string *error) {
bool success1 = true;
bool success2 = true;
std::string error1;
std::string error2;

if (!m_audit->m_path1.empty()) {
success1 = utils::SharedFiles::getInstance().reopen(m_audit->m_path1, &error1);
}
if (!m_audit->m_path2.empty()) {
success2 = utils::SharedFiles::getInstance().reopen(m_audit->m_path2, &error2);
}

std::stringstream errorStream;
if (!success1 || !success2) {
errorStream << "There was an error reopening parallel audit logs.";

if (!success1 && !error1.empty()) {
errorStream << " " << error1;
}
if (!success2 && !error2.empty()) {
errorStream << " " << error2;
}

*error = errorStream.str();
}

return success1 && success2;
}


} // namespace writer
} // namespace audit_log
} // namespace modsecurity
1 change: 1 addition & 0 deletions src/audit_log/writer/parallel.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Parallel : public Writer {
bool init(std::string *error) override;
bool write(Transaction *transaction, int parts,
std::string *error) override;
bool reopen(std::string *error) override;


/**
Expand Down
15 changes: 15 additions & 0 deletions src/audit_log/writer/serial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,21 @@ bool Serial::write(Transaction *transaction, int parts, std::string *error) {
error);
}


bool Serial::reopen(std::string *error) {
bool success;

std::string errorDetail;
success = utils::SharedFiles::getInstance().reopen(m_audit->m_path1, &errorDetail);
if (!success) {
*error = "There was an error reopening the serial audit log. ";
error->append(errorDetail);
}

return success;
}


} // namespace writer
} // namespace audit_log
} // namespace modsecurity
1 change: 1 addition & 0 deletions src/audit_log/writer/serial.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class Serial : public Writer {
bool init(std::string *error) override;
bool write(Transaction *transaction, int parts,
std::string *error) override;
bool reopen(std::string *error) override;
};

} // namespace writer
Expand Down
1 change: 1 addition & 0 deletions src/audit_log/writer/writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class Writer {
virtual bool init(std::string *error) = 0;
virtual bool write(Transaction *transaction, int parts,
std::string *error) = 0;
virtual bool reopen(std::string *error) = 0;

static void generateBoundary(std::string *boundary);

Expand Down
21 changes: 21 additions & 0 deletions src/rules_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,5 +318,26 @@ extern "C" int msc_rules_cleanup(RulesSet *rules) {
}


extern "C" int msc_rules_reopen_audit_log(RulesSet *rules, const char **error) {
bool succeeded = true;
std::string errorStr;

if (rules->m_auditLog != NULL) {
succeeded = rules->m_auditLog->reopen(&errorStr);
}

if (!succeeded) {
if (!errorStr.empty()) {
*error = strdup(errorStr.c_str());
} else {
// Guarantee an error message is always assigned in the event of a failure
*error = strdup("Unknown error reopening audit log");
}
}

return succeeded ? 0 : -1;
}


} // namespace modsecurity

58 changes: 58 additions & 0 deletions src/utils/shared_files.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,64 @@ bool SharedFiles::open(const std::string& fileName, std::string *error) {
}


bool SharedFiles::reopen(const std::string& fileName, std::string *error) {
std::pair<msc_file_handler *, FILE *> *a = NULL;
bool ret = true;
FILE *fp;
struct stat target_stat;
struct stat current_stat;

#if MODSEC_USE_GENERAL_LOCK
pthread_mutex_lock(m_generalLock);
#endif

for (auto &i : m_handlers) {
if (i.first == fileName) {
a = &i.second;
break;
}
}

if (a == NULL || a->first == NULL || a->second == NULL) {
error->assign("Cannot find open file to reopen: " + fileName);
ret = false;
goto out;
}

if (
stat(fileName.c_str(), &target_stat) == 0 &&
fstat(fileno(a->second), &current_stat) == 0 &&
current_stat.st_dev == target_stat.st_dev &&
current_stat.st_ino == target_stat.st_ino
) {
// We already have a file open at the right path. Let's skip reopen.
ret = true;
goto out;
}

// Use fopen() and fclose() instead of freopen() because freopen() closes
// the current file before proving it can open the new file.
// If the open fails, it's better to continue writing to the old file than
// to fail outright.
fp = fopen(fileName.c_str(), "a");
if (fp == NULL) {
error->assign("Failed to open file: " + fileName);
ret = false;
goto out;
}

fclose(a->second);
a->second = fp;

out:
#if MODSEC_USE_GENERAL_LOCK
pthread_mutex_unlock(m_generalLock);
#endif

return ret;
}


void SharedFiles::close(const std::string& fileName) {
std::pair<msc_file_handler *, FILE *> a;
/* int ret; */
Expand Down
1 change: 1 addition & 0 deletions src/utils/shared_files.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ typedef struct msc_file_handler {
class SharedFiles {
public:
bool open(const std::string& fileName, std::string *error);
bool reopen(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);
Expand Down
3 changes: 2 additions & 1 deletion test/cppcheck_suppressions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ invalidScanfArgType_int:src/rules_set_properties.cc:102
// ModSecurity v3 code...
//
unmatchedSuppression:src/utils/geo_lookup.cc:82
useInitializationList:src/utils/shared_files.h:87
functionConst:src/utils/shared_files.h:60
useInitializationList:src/utils/shared_files.h:88
unmatchedSuppression:src/utils/msc_tree.cc
functionStatic:headers/modsecurity/transaction.h:373
duplicateBranch:src/audit_log/audit_log.cc:224
Expand Down