mirror of
https://github.com/owasp-modsecurity/ModSecurity.git
synced 2025-08-13 21:36:00 +03:00
Added support to lock files on Windows and major rewrite to reintroduce reference counting and remove unused code.
- In Windows build, replaced usage of fcntl with cmd F_SETLKW with Win32 APIs to do file locking (LockFileEx & UnlockFileEx). - Reintroduced the reference counting initially present in the class which is necessary to correctly handle merging of rules. This allows for correctly closing the file and removing the associated entry from m_handlers when the file is no longer used. - The need for reference counting can be seen in the example simple_example_using_c, where rules are initially loaded locally and then further rules are loaded remotely. This will initially open a shared file for a log, then in order to merge rules, the shared file is opened again for the new configuration. Then, the previous configuration closes the shared file on destruction. That is, two consecutive opens are done on a shared file, which is followed by a close. If the shared file is not reference counted, the shared file will be closed while there is still a reference active. The current version works because closing of the file has been disabled after reference counting was removed. - Replaced `std::vector` data structure with `std::unordered_map` to improve lookup/update times, and simplify code. - Removed unused code - Shared memory to store msc_file_handler structure - Initially SharedFiles used shared memory to store information about each shared file, including its file pointer and a mutex to synchronize access to the file on write. See code at commit 01c13da, in particular, usage of lock & fp fields in the msc_file_handler_t structure. - At that time, msc_file_handler_t included reference counting too with the using_it field, which was incremented when a file was opened and decremented on close. If the reference count reached zero, the shared file would be closed, the lock destroyed and the file handler entry removed from m_handlers. - Reference counting was removed in commit 7f9cd76, which introduced the following issues in SharedFiles::close: - No longer closes the file pointer. - The file pointer appears to be reset when a.second = 0, but this is a local copy of the data pair obtained from m_handlers, so this is essentially a nop (updating a local variable that is not referenced later in the function). - NOTE: The file pointer was moved out of the shared memory in this commit too, and stored alongside the msc_file_handler_t instance in the m_handlers entry associated to the shared file. - The lock is no longer destroyed. - The shared memory is marked to be destroyed in the call to: shmctl(a.first->shm_id_structure, IPC_RMID, NULL); - The shared file entry is not removed from m_handlers, so: - the file pointer is still valid, which is how writing to the file continues to work, - the reference to the shared memory is also present and will be marked to be destroyed whenever close is called again on the shared file. - File locking using the mutex in msc_file_handler_t was replaced in commit 3d20304 with usage of fcntl with cmd F_SETLKW. - At this time, it appears that the shared memory is no longer used, as the file pointer and locking no longer depend on it. - MODSEC_USE_GENERAL_LOCK - This code is introduced commit 7f9cd76 and is enabled if MODSEC_USE_GENERAL_LOCK` is defined. - The define is commented out in the source code since the original commit and is not present in the build configuration either. - In commit ff9152e, in the SharedFiles constructor, the initialization of the local variable toBeCreated is removed. This means that in this version, if MODSEC_USE_GENERAL_LOCK is enabled, execution of the code that checks on toBeCreated is undefined. - Then, in commit 9b40a04, the variable toBeCreated is initialized again, but is now set to false, which means that if MODSEC_USE_GENERAL_LOCK is enabled, the shared memory and lock it uses will *not* be initialized and thus doesn't work (execution of the current version will result in trying to acquire a lock that will be null). - I conclude that the feature is not used and can be removed. - Additionally, if it were working, I think the lock should be used in SharedFiles::write as well, which is a reader of the underlying data structures protected by this lock when they're modified in SharedFiles::open & SharedFiles::close.
This commit is contained in:
parent
50e78331b1
commit
d7c49ed590
@ -15,236 +15,103 @@
|
||||
|
||||
#include "src/utils/shared_files.h"
|
||||
|
||||
#include <errno.h>
|
||||
#include <fcntl.h>
|
||||
#include <pthread.h>
|
||||
#include <stdio.h>
|
||||
#include <sys/ipc.h>
|
||||
#include <sys/shm.h>
|
||||
#include <sys/stat.h>
|
||||
#include <sys/types.h>
|
||||
#include <unistd.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#ifdef WIN32
|
||||
#include <windows.h>
|
||||
#include <io.h>
|
||||
#endif
|
||||
|
||||
#include <utility>
|
||||
#include <fstream>
|
||||
#include <string>
|
||||
|
||||
namespace modsecurity {
|
||||
namespace utils {
|
||||
|
||||
|
||||
std::pair<msc_file_handler *, FILE *> SharedFiles::find_handler(
|
||||
const std::string &fileName) {
|
||||
for (const auto &i : m_handlers) {
|
||||
if (i.first == fileName) {
|
||||
return i.second;
|
||||
}
|
||||
}
|
||||
return std::pair<modsecurity::utils::msc_file_handler *,
|
||||
FILE *>(NULL, NULL);
|
||||
}
|
||||
|
||||
|
||||
std::pair<msc_file_handler *, FILE *> SharedFiles::add_new_handler(
|
||||
SharedFiles::handlers_map::iterator SharedFiles::add_new_handler(
|
||||
const std::string &fileName, std::string *error) {
|
||||
int shm_id;
|
||||
int ret;
|
||||
key_t mem_key_structure;
|
||||
msc_file_handler_t *new_debug_log = NULL;
|
||||
struct shmid_ds shared_mem_info;
|
||||
FILE *fp;
|
||||
bool toBeCreated = true;
|
||||
|
||||
fp = fopen(fileName.c_str(), "a");
|
||||
FILE *fp = fopen(fileName.c_str(), "a");
|
||||
if (fp == 0) {
|
||||
error->assign("Failed to open file: " + fileName);
|
||||
goto err_fh;
|
||||
return m_handlers.end();
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
shm_id = shmget(mem_key_structure, sizeof (msc_file_handler_t) \
|
||||
+ fileName.size() + 1, IPC_CREAT | IPC_EXCL | 0666);
|
||||
if (shm_id < 0) {
|
||||
shm_id = shmget(mem_key_structure, sizeof (msc_file_handler_t)
|
||||
+ fileName.size() + 1, IPC_CREAT | 0666);
|
||||
toBeCreated = false;
|
||||
if (shm_id < 0) {
|
||||
error->assign("Failed to allocate shared memory (1): ");
|
||||
error->append(strerror(errno));
|
||||
goto err_shmget1;
|
||||
}
|
||||
}
|
||||
|
||||
ret = shmctl(shm_id, IPC_STAT, &shared_mem_info);
|
||||
if (ret < 0) {
|
||||
error->assign("Failed to get information on shared memory (1): ");
|
||||
error->append(strerror(errno));
|
||||
goto err_shmctl1;
|
||||
}
|
||||
|
||||
new_debug_log = reinterpret_cast<msc_file_handler_t *>(
|
||||
shmat(shm_id, NULL, 0));
|
||||
if ((reinterpret_cast<char *>(new_debug_log)[0]) == -1) {
|
||||
error->assign("Failed to attach shared memory (1): ");
|
||||
error->append(strerror(errno));
|
||||
goto err_shmat1;
|
||||
}
|
||||
|
||||
if (toBeCreated == false && shared_mem_info.shm_nattch == 0) {
|
||||
toBeCreated = true;
|
||||
}
|
||||
|
||||
if (toBeCreated) {
|
||||
memset(new_debug_log, '\0', sizeof(msc_file_handler_t));
|
||||
new_debug_log->shm_id_structure = shm_id;
|
||||
memcpy(new_debug_log->file_name, fileName.c_str(), fileName.size());
|
||||
new_debug_log->file_name[fileName.size()] = '\0';
|
||||
}
|
||||
m_handlers.push_back(std::make_pair(fileName,
|
||||
std::make_pair(new_debug_log, fp)));
|
||||
|
||||
return std::make_pair(new_debug_log, fp);
|
||||
err_shmat1:
|
||||
shmdt(new_debug_log);
|
||||
err_shmctl1:
|
||||
err_shmget1:
|
||||
err_mem_key:
|
||||
fclose(fp);
|
||||
err_fh:
|
||||
return std::pair<modsecurity::utils::msc_file_handler *,
|
||||
FILE *>(NULL, NULL);
|
||||
return m_handlers.insert({ fileName, {fp, 0} }).first;
|
||||
}
|
||||
|
||||
|
||||
bool SharedFiles::open(const std::string& fileName, std::string *error) {
|
||||
std::pair<msc_file_handler *, FILE *> a;
|
||||
bool ret = true;
|
||||
|
||||
#if MODSEC_USE_GENERAL_LOCK
|
||||
pthread_mutex_lock(m_generalLock);
|
||||
#endif
|
||||
|
||||
a = find_handler(fileName);
|
||||
if (a.first == NULL) {
|
||||
a = add_new_handler(fileName, error);
|
||||
if (error->size() > 0) {
|
||||
ret = false;
|
||||
goto out;
|
||||
}
|
||||
auto it = m_handlers.find(fileName);
|
||||
if (it == m_handlers.end()) {
|
||||
it = add_new_handler(fileName, error);
|
||||
if (error->size() > 0)
|
||||
return false;
|
||||
}
|
||||
if (a.first == NULL) {
|
||||
|
||||
if (it == m_handlers.end()) {
|
||||
error->assign("Not able to open: " + fileName);
|
||||
ret = false;
|
||||
goto out;
|
||||
return false;
|
||||
}
|
||||
|
||||
out:
|
||||
#if MODSEC_USE_GENERAL_LOCK
|
||||
pthread_mutex_unlock(m_generalLock);
|
||||
#endif
|
||||
it->second.cnt++;
|
||||
|
||||
return ret;
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
void SharedFiles::close(const std::string& fileName) {
|
||||
std::pair<msc_file_handler *, FILE *> a;
|
||||
/* int ret; */
|
||||
/* int shm_id; */
|
||||
/* struct shmid_ds shared_mem_info; */
|
||||
/* int j = 0; */
|
||||
if (fileName.empty())
|
||||
return;
|
||||
|
||||
#if MODSEC_USE_GENERAL_LOCK
|
||||
pthread_mutex_lock(m_generalLock);
|
||||
#endif
|
||||
auto it = m_handlers.find(fileName);
|
||||
if (it == m_handlers.end())
|
||||
return;
|
||||
|
||||
if (fileName.empty()) {
|
||||
goto out;
|
||||
it->second.cnt--;
|
||||
if (it->second.cnt == 0)
|
||||
{
|
||||
fclose(it->second.fp);
|
||||
|
||||
m_handlers.erase(it);
|
||||
}
|
||||
|
||||
a = find_handler(fileName);
|
||||
if (a.first == NULL || a.second == NULL) {
|
||||
goto out;
|
||||
}
|
||||
|
||||
/* fclose(a.second); */
|
||||
a.second = 0;
|
||||
|
||||
/*
|
||||
* Delete the file structure will be welcomed, but we cannot delay
|
||||
* while the process is being killed.
|
||||
*
|
||||
for (std::pair<std::string,
|
||||
std::pair<msc_file_handler *, FILE *>> i : m_handlers) {
|
||||
if (i.first == fileName) {
|
||||
j++;
|
||||
}
|
||||
}
|
||||
|
||||
m_handlers.erase(m_handlers.begin()+j);
|
||||
*/
|
||||
|
||||
/* hmdt(a.second); */
|
||||
shmctl(a.first->shm_id_structure, IPC_RMID, NULL);
|
||||
|
||||
/*
|
||||
*
|
||||
* We could check to see how many process attached to the shared memory
|
||||
* we have, prior to the deletion of the shared memory.
|
||||
*
|
||||
ret = shmctl(a.first->shm_id_structure, IPC_STAT, &shared_mem_info);
|
||||
if (ret < 0) {
|
||||
goto out;
|
||||
}
|
||||
ret = shared_mem_info.shm_nattch;
|
||||
shm_id = a.first->shm_id_structure;
|
||||
*/
|
||||
|
||||
out:
|
||||
#if MODSEC_USE_GENERAL_LOCK
|
||||
pthread_mutex_unlock(m_generalLock);
|
||||
#endif
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
bool SharedFiles::write(const std::string& fileName,
|
||||
const std::string &msg, std::string *error) {
|
||||
std::pair<msc_file_handler *, FILE *> a;
|
||||
std::string lmsg = msg;
|
||||
size_t wrote;
|
||||
struct flock lock{};
|
||||
bool ret = true;
|
||||
|
||||
a = find_handler(fileName);
|
||||
if (a.first == NULL) {
|
||||
auto it = m_handlers.find(fileName);
|
||||
if (it == m_handlers.end()) {
|
||||
error->assign("file is not open: " + fileName);
|
||||
return false;
|
||||
}
|
||||
|
||||
//Exclusively lock whole file
|
||||
#ifndef WIN32
|
||||
struct flock lock {};
|
||||
lock.l_start = lock.l_len = lock.l_whence = 0;
|
||||
lock.l_type = F_WRLCK;
|
||||
fcntl(fileno(a.second), F_SETLKW, &lock);
|
||||
fcntl(fileno(it->second.fp), F_SETLKW, &lock);
|
||||
#else
|
||||
auto handle = reinterpret_cast<HANDLE>(_get_osfhandle(fileno(it->second.fp)));
|
||||
OVERLAPPED overlapped = { 0 };
|
||||
::LockFileEx(handle, LOCKFILE_EXCLUSIVE_LOCK, 0, MAXDWORD, MAXDWORD, &overlapped);
|
||||
#endif
|
||||
|
||||
wrote = fwrite(lmsg.c_str(), 1, lmsg.size(), a.second);
|
||||
auto wrote = fwrite(msg.c_str(), 1, msg.size(), it->second.fp);
|
||||
if (wrote < msg.size()) {
|
||||
error->assign("failed to write: " + fileName);
|
||||
ret = false;
|
||||
}
|
||||
fflush(a.second);
|
||||
fflush(it->second.fp);
|
||||
|
||||
//Remove exclusive lock
|
||||
#ifndef WIN32
|
||||
lock.l_type = F_UNLCK;
|
||||
fcntl(fileno(a.second), F_SETLKW, &lock);
|
||||
fcntl(fileno(it->second.fp), F_SETLKW, &lock);
|
||||
#else
|
||||
overlapped = { 0 };
|
||||
::UnlockFileEx(handle, 0, MAXDWORD, MAXDWORD, &overlapped);
|
||||
#endif
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
@ -17,45 +17,18 @@
|
||||
#define SRC_UTILS_SHARED_FILES_H_
|
||||
|
||||
|
||||
#include <errno.h>
|
||||
#include <fcntl.h>
|
||||
#include <pthread.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <sys/ipc.h>
|
||||
#include <sys/shm.h>
|
||||
#include <sys/stat.h>
|
||||
#include <sys/types.h>
|
||||
#include <unistd.h>
|
||||
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
#include <fstream>
|
||||
#include <unordered_map>
|
||||
#include <string>
|
||||
|
||||
|
||||
#include "modsecurity/transaction.h"
|
||||
#include "modsecurity/audit_log.h"
|
||||
|
||||
/**
|
||||
* Not using this critical section yet.
|
||||
*
|
||||
*/
|
||||
/* #define MODSEC_USE_GENERAL_LOCK */
|
||||
|
||||
namespace modsecurity {
|
||||
namespace utils {
|
||||
|
||||
|
||||
typedef struct msc_file_handler {
|
||||
int shm_id_structure;
|
||||
char file_name[];
|
||||
} msc_file_handler_t;
|
||||
|
||||
|
||||
class SharedFiles {
|
||||
public:
|
||||
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,
|
||||
@ -66,86 +39,28 @@ class SharedFiles {
|
||||
return instance;
|
||||
}
|
||||
|
||||
protected:
|
||||
std::pair<msc_file_handler *, FILE *> find_handler(
|
||||
const std::string &fileName);
|
||||
std::pair<msc_file_handler *, FILE *> add_new_handler(
|
||||
const std::string &fileName, std::string *error);
|
||||
|
||||
private:
|
||||
SharedFiles()
|
||||
#ifdef MODSEC_USE_GENERAL_LOCK
|
||||
: m_generalLock(NULL),
|
||||
m_memKeyStructure(0)
|
||||
#endif
|
||||
{
|
||||
#ifdef MODSEC_USE_GENERAL_LOCK
|
||||
int shm_id;
|
||||
bool toBeCreated(false);
|
||||
bool err = false;
|
||||
|
||||
m_memKeyStructure = ftok(".", 1); // cppcheck-suppress useInitializationList
|
||||
if (m_memKeyStructure < 0) {
|
||||
err = true;
|
||||
goto err_mem_key;
|
||||
}
|
||||
|
||||
shm_id = shmget(m_memKeyStructure, sizeof(pthread_mutex_t),
|
||||
IPC_CREAT | IPC_EXCL | 0666);
|
||||
if (shm_id < 0) {
|
||||
shm_id = shmget(m_memKeyStructure, sizeof(pthread_mutex_t),
|
||||
IPC_CREAT | 0666);
|
||||
toBeCreated = false;
|
||||
if (shm_id < 0) {
|
||||
err = true;
|
||||
goto err_shmget1;
|
||||
}
|
||||
}
|
||||
|
||||
m_generalLock = reinterpret_cast<pthread_mutex_t *>(
|
||||
shmat(shm_id, NULL, 0));
|
||||
if ((reinterpret_cast<char *>(m_generalLock)[0]) == -1) {
|
||||
err = true;
|
||||
goto err_shmat1;
|
||||
}
|
||||
|
||||
if (toBeCreated) {
|
||||
memset(m_generalLock, '\0', sizeof(pthread_mutex_t));
|
||||
pthread_mutex_init(m_generalLock, NULL);
|
||||
pthread_mutex_unlock(m_generalLock);
|
||||
}
|
||||
|
||||
if (err) {
|
||||
err_mem_key:
|
||||
std::cerr << strerror(errno) << std::endl;
|
||||
err_shmget1:
|
||||
std::cerr << "err_shmget1" << std::endl;
|
||||
err_shmat1:
|
||||
std::cerr << "err_shmat1" << std::endl;
|
||||
}
|
||||
#endif
|
||||
}
|
||||
~SharedFiles() {
|
||||
#if MODSEC_USE_GENERAL_LOCK
|
||||
shmdt(m_generalLock);
|
||||
shmctl(m_memKeyStructure, IPC_RMID, NULL);
|
||||
#endif
|
||||
}
|
||||
private:
|
||||
SharedFiles() = default;
|
||||
~SharedFiles() = default;
|
||||
|
||||
// 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&);
|
||||
SharedFiles(SharedFiles const&) = delete;
|
||||
void operator=(SharedFiles const&) = delete;
|
||||
|
||||
std::vector<std::pair<std::string,
|
||||
std::pair<msc_file_handler *, FILE *>>> m_handlers;
|
||||
#if MODSEC_USE_GENERAL_LOCK
|
||||
pthread_mutex_t *m_generalLock;
|
||||
key_t m_memKeyStructure;
|
||||
#endif
|
||||
struct handler_info {
|
||||
FILE* fp;
|
||||
unsigned int cnt;
|
||||
};
|
||||
|
||||
using handlers_map = std::unordered_map<std::string, handler_info>;
|
||||
handlers_map m_handlers;
|
||||
|
||||
handlers_map::iterator add_new_handler(
|
||||
const std::string &fileName, std::string *error);
|
||||
};
|
||||
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user