-
Notifications
You must be signed in to change notification settings - Fork 53
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 running target query against single files #950
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #950 +/- ##
==========================================
+ Coverage 77.82% 77.87% +0.05%
==========================================
Files 323 325 +2
Lines 27647 27712 +65
==========================================
+ Hits 21517 21582 +65
Misses 6130 6130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
b05639b
to
f413f6c
Compare
Functionality is not added to existing log loader, because it disributes files according to their extension
ed85564
to
aa305bb
Compare
aa305bb
to
226ccc4
Compare
@@ -29,6 +29,8 @@ | |||
|
|||
FunctionTuple = tuple[plugin.Plugin, Optional[Union[plugin.Plugin, property]]] | |||
|
|||
SINGLE_FILE_DIR = "$drop$" # The directory where user-specified single files are mapped |
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.
In principle I don't agree with this approach for supporting single files. I don't believe we should spend time on this workaround and only work on the proper way to support this feature.
The way this should be done is described in #789.
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.
Hi,
A preliminary look by the team confirms that this PR broadly implements what was discussed during the sprint planning, and what is in the story description.
Compared to the Logfile loader, which creates an implicit coupling to logging plugins though the location of log file directories, the mechanism in this PR is at least more explicit.
To your point, I can relate to classifying this solution as a workaround. Although you are not very specific in your criticsm, my own gripe is that
this solution requires changing the plugin logic in an ad-hoc manner to retrieve the paths to the (log) files, which is not fully offset by the Mixin class.
The plugin architecture is pull-based, meaning that plugins are reponsible for fetching information they need from targets.
For example, the IIS plugin not just maps over log files, but the mapping itself is a function of a configuration file. Similary, the EVTX plugin infers the location of log files form the registry, and from additional command line arguments. In these examples, the mapping defined in the plugin from the target input to the record outputs is not solely determined by the contents of a log file, but is context-dependent, with the context being auxiliary data in the target and/or configuration through command line arguments.
The conditional switching inside the plugins depending on whether single file mode is active could be eliminated by abstracting through a "file locator" interface, and injecting into the plugin either the implementation which returns paths to single files as specified by the user, or an implementation which uses for example the target and command line arguments. This pushes the conditional out of the plugin and into the caller. Note that the caller somehow needs to know which file locator implementation to instantiate, and how to instantiate the implementation by using for example factories.
Alternatively, we could augment the plugin API to support functions which do not pull data from targets, but get file streams pushed into them. When processing the query, the specified files can be iterated over, and the plugin function is invoked with the stream.
But perhaps you have other ideas? In a comment on the ticket, you wrote:
Why declarative? I think that will be needlessly complicated. I suggest a simple _get_paths() and _get_user_paths() for path sources, and something similar for registry.
Perhaps you can expand on the solution you have in mind?
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.
Can you leave this comment under the linked issue?
Regardless of what was planned, I think it's a waste of time to implement a "drop folder" approach since it's suboptimal in many aspects and will be removed in the long term. The need for this functionality isn't that high that we need a "dirty workaround" like the drop folder right this moment.
I don't have time to go into this with much more detail at this time.
Allow running supported plugins against single files on the local system.
Usage:
target-query -f evtx --single-file tests/_data/plugins/os/windows/log/schedlgu/schedlgu.txt
Supported Plugins:
Implementation details:
This feature is implemented using a single file loader, and a companion single file mixin. The single file loader is similar to the log loader, but maps source files to a dedicated folder on the target file system. Plugins indicate they support single file mode by inheriting from the
SingleFileMixin
. Simultaneously, this class provided convenience methods to retrieve the files added by the single file loader. It is expected that the log loader can be deprecated.Issues