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
Wildcard support in Windows registers #15852
Conversation
fa50cd0
to
762b37f
Compare
cf065cc
to
3c8db66
Compare
3c8db66
to
412ce24
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the changes, please also check the UTs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
d99e0fe
to
fac4e09
Compare
Fix: Memory leaks errors Add: 64-bit register support Fix: Improve code quality
Del: Debug message
Fix: Code style
fac4e09
to
4be5239
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GabrielEValenzuela @MarcelKemp I've found the following defect in the code:
** CID 321729: (RESOURCE_LEAK)
/config/syscheck-config.c: 639 in read_reg()
/config/syscheck-config.c: 653 in read_reg()
________________________________________________________________________________________________________
*** CID 321729: (RESOURCE_LEAK)
/config/syscheck-config.c: 639 in read_reg()
633 dump_syscheck_registry(syscheck, *paths_wildcard, opts, restrict_key, restrict_value, recursion_level, tag, ARCH_32BIT, tmp_diff_size);
634 } else {
635 dump_syscheck_registry(syscheck, *paths_wildcard, opts, restrict_key, restrict_value, recursion_level, tag, arch, tmp_diff_size);
636 }
637 paths_wildcard++;
638 }
>>> CID 321729: (RESOURCE_LEAK)
>>> Overwriting "paths_wildcard" in "paths_wildcard = start_vector" leaks the storage that "paths_wildcard" points to.
639 paths_wildcard = start_vector;
640 free_strarray(paths_wildcard);
641 mdebug1(FIM_WILDCARDS_REGISTERS_FINALIZE);
642 } else {
643 /* Add new entry */
644 if (arch == ARCH_BOTH) {
/config/syscheck-config.c: 653 in read_reg()
647 } else {
648 dump_syscheck_registry(syscheck, tmp_entry, opts, restrict_key, restrict_value, recursion_level, tag, arch, tmp_diff_size);
649 }
650 }
651 /* Next entry */
652 free(entry[i]);
>>> CID 321729: (RESOURCE_LEAK)
>>> Variable "paths_wildcard" going out of scope leaks the storage it points to.
653 }
654 free(entry);
655
656 retval = 1;
657 clean_reg:
658 os_free(tag);
Please review it and fix that if applicable.
Cheers!
This PR aims to add a new feature within the Syscheck module which is the possibility to use wildcard (also called metacharacters) to specify many windows registers at once. The asterisk
*
is replaced by any number of characters in a registry name, and the question mark?
is replaced by any single character.Description of the new feature
When parsing the path composition provided in the
osse.conf
file, the different parts of the path are split using the two wildcard options*
or?
as delimiters.Subsequently, the Windows API is used, where the records of a root key and a (possibly null) subkey are listed and the complete new paths are assembled, which will then be monitored by FIM. It is important to clarify that this solution is **case sensitive.
Important note
According with this code section
wazuh/src/syscheckd/src/registry/registry.c
Lines 364 to 380 in 78bbd9f
Only the root keys:
HKEY_LOCAL_MACHINE
,HKEY_CLASSES_ROOT
,HKEY_CURRENT_CONFIG
,HKEY_USERS
are valid root keys for monitoring and will work well with the current feature.Example of configuration and tests
🟢 Combine three possibles cases
Configuration
🟢 Start agent with new feature
🟢 Add new registry
🟢 Modify an existenting value
Software Design Document
SDD Report
Introduction
Wildcards provide a shorthand for specifying sets of files with similar names.
An asterisk
*
is replaced by any number of characters in a filename. For example,ae*
would matchaegis
,aerie
,aeon
, etc. if those files were in the same directory.And, a question
?
mark is replaced by any single character (soh?p
matcheshop
andhip
, but nothelp
).This support was already implemented for directories but not for Windows registers. This solution has been
designed, in an effort to create a necessary feature to FIM that is open for extensions; this document describes the implementation of this one.
Not only does this document describe the software already created, it is also intended to enforce compatibility of future modifications or add–ons.
Algortihm Overview
The algorithm consists in the input of a path provided by the ossec.conf file which is checked to see if it has a wildcard or not.
Subsequently, a copy of this path is created so as not to alter the original and three cases are analyzed:
Combination case: Both wildcards are present in the path, expanding first the asterisks and then the question marks.
Asterisk case: All possible records are analyzed.
Question mark case: All possible records matching the original path are analyzed.
Once the copy of the original path is created, it is divided into two parts taking the wildcard as the inflection point. The first part extracts the root key and possible sub key. The second part is concatenated with the result of the query that uses the Windows API to obtain all possible values.
For the case of the question mark, the Windows API is also used which has support for finding the matches of two paths.
This can be summarized in the following sequence diagram:
Y deterministic finite state machine (FSM) diagram.
Conclusión
The proposed solution is based on the use of API functions previously used in the project, testing with 90% code coverage and analyzing with CLANG with positive results.
It is available for future improvements, fixes and/or comments from the management team and the community.
542569
Coverage and ScanBuild reports