- The previous approach would create a std::unique_ptr and store it in
a std::list in VariableValue (Origins)
- The new approach now stores Origins in a std::vector and constructs
VariableOrigin elements in-place on insertion.
- Instead of having two heap-allocations for every added VariableOrigin
instance, this performs only one.
- If multiple origins are added, std::vector's growth strategy may even
prevent a heap-allocation. There's a cost on growing the size of the
vector, because a copy of current elements will be necessary.
- Introduced reserveOrigin method to notify that multiple insertions
will be made, so that we can use std::vector's reserve and do a
single allocation (and copy of previous elements), and then just
initialize the new elements in-place.
- Added support to build 32-bit versions of libModSecurity on Linux
- Added support to build libModSecurity using clang on Linux (both
64-bit and 32-bit versions)
- Fixed macOS dependencies to include yajl, not only because it is
a required dependency, but because tests were not being run on
macOS builds without it.
- Added build 'without libxml' to Linux & macOS configurations.
- Added build 'without ssdeep' to Linux configurations (already in macOS
configuration)
- Added build 'with lmdb' to Linux & macOS configurations, replacing the
existing one 'without lmdb' because by default LMDB is disabled if not
explicitly turn on in configure.
- Removed 'without yajl' build because it's a required 3rd party
dependency.
- Added bison & flex dependencies to enable parser generation.
- The parser is not used interactively so we can avoid including
unistd.h, which is not available on Windows MSVC C++ compiler.
- The #ifdef WIN32 introduced in PR #3132 would probably be overwritten
when the parser is updated.
- By default, all the 3rd party dependencies are enabled.
- A dependency can be turned off by adding the "-DWITHOUT_xxx=ON" to
the call of vcbuild.bat
- List of 3rd party dependencies and associated option to turn them off:
- LMDB: WITHOUT_LMDB
- LUA: WITHOUT_LUA
- LibXML2: WITHOUT_LIBXML2
- MaxMind: WITHOUT_MAXMIND
- cURL: WITHOUT_CURL
- Added new test/test_suite.in with list of regression and unit tests
previously in Makefile.am, to be shared between Unix and Windows
builds.
- Updated regression.cc & unit.cc to return the number of failed tests
to indicate to CTest that the test failed. Similarly, a crash or
unhandled exception terminates the process with a non-zero exit code.
- This change doesn't affect running the tests with autotest in Unix
builds because this processes test output from custom-test-driver &
test-suite.sh, and ignores the exit code of the test runner.
- Removed comment in test/test-cases/regression-offset-variable.json as
this is not supported by JSON and prevents strict parsers to read and
process the file.
- Minor change in regression.cc's clearAuditLog to replace std::ifstream
with std::ofstream as the mode to open the flag applies to an output
stream.
- Minor change in unit.cc to simplify code that deletes tests.
- Minor changes to test/custom-test-driver to correct usage information.
- Addresses SonarCloud issue cpp:S5025 (Memory should not be managed manually)
- This function was not changed for the Windows port, but a similar
change to the one suggested was done in expandEnv in the same file.
- The first stream is not destructed at the exact same point it was in
the previous code (but rather when the second stream replaces it on
assignment to the same variable). An arbitrary scope could have been
introduced to destruct the object at the same place, but it doesn't
seem to be necessary and would make the code a bit strange.
- Address SonarCloud cpp:S3806 issues ("#include" paths should be portable)
- This is not an actual issue in this case, because WinSock2.h and
WS2tcpip.h are Windows only.
- 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.
- Env::evaluate
- Environment variable names in Windows are case-insensitive, so in
the Windows build we use strcasecmp to ignore case when matching
variables in transaction->m_variableEnvs.
- If the variable is found, we use the expected variable name to
create the VariableValue instance, as further rule processing will
look for the variable using case-sensitive comparisons.
- This code is not limited to Windows to avoid another #ifdef block
because for other platforms, because the env variable names are
case-sensitive the value from either x.first and m_name will be the
same.
- In Windows build, avoid redefining environ, already defined by
including stdlib.h.
- Use after free issue detected with Address Sanitizer while running
the reading_logs_with_offset example.
- Keeps reference to last element in vars vector with vars.back(). Then
it removes the element from vars calling vars.pop_back() which
invalidates the reference, but it's accessed later in the function.
- RuleWithActions::evaluate(Transaction *transaction)
- Removed temporary rm local variable used to immediately create
std::shared_ptr<RuleMessage>.
- Leverage std::make_shared & auto to simplify code.
- expandEnv on Windows uses POCO C++ Libraries implementation of Glob
- Paths of matched files are adjusted to preserve UNIX path
separators for consistency with the rest of the code.
- Minor change to code shared with other platforms that removes
allocation of std::ifstream on the heap to check whether the file can
be opened, which can be done with local stack variable that closes
the file when out of scope.
- createDir uses _mkdir on Windows, which doesn't support configuring
the new directory's mode.
- added public domain implementation of clock_gettime for clock_id
CLOCK_PROCESS_CPUTIME_ID from mingw-w64's winpthreads to support
cpu_seconds on Windows.
- Updated included headers to support compilation on Windows (using
Visual C++)
- Updated included headers to support compilation on Windows (using
Visual C++)
- Minor change to use C++ default (zero) initialization instead of
calling memset.
- most of posix related functions and constants in unistd.h can be
found in io.h in Visual C++
- introduced src/compat/msvc.h to adjust for compiler differences (and
avoid updating code with #ifdef blocks for Windows support)
- removed some included headers that are not needed (both on Unix and
Windows builds)
