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

Allow all plugins to access a drop folder from the log loader #399

Closed
wants to merge 1 commit into from

Conversation

cecinestpasunepipe
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #399 (f30810a) into main (9adde7e) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main     #399      +/-   ##
==========================================
- Coverage   73.28%   73.27%   -0.01%     
==========================================
  Files         245      245              
  Lines       19544    19547       +3     
==========================================
+ Hits        14322    14324       +2     
- Misses       5222     5223       +1     
Flag Coverage Δ
unittests 73.27% <75.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
dissect/target/loaders/log.py 93.33% <75.00%> (-2.97%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Schamper
Copy link
Member

Can you explain what this does or adds?

@cecinestpasunepipe
Copy link
Contributor Author

Can you explain what this does or adds?

You can now easily make a file-based variant of a plugin by checking the drop location.
So for instance, the NotificationPlugin could check for dropped files and then simply process the single file there.

@Schamper
Copy link
Member

And then you'd temporarily edit that plugin to look at that directory?

I'm not sure I agree with this approach for this problem. I was trying to look for a ticket, but it seems like I hadn't written it down yet. An idea I had was to make the plugin system have some kind of generic concept of paths for the kinds of plugins that utilise them. Then you could easily swap out a path, either by command line argument to the plugin (e.g. an alternate .evtx path) or in use with the log plugin.

An internal conflict I have with this idea is that I don't want the plugin system to become too "FizzBuzzEnterpriseEdition", so it'd have to be lean. It should be incredibly easy to list and add paths to a plugin in regular plugin development, as well as specify a custom path when needed.

I'll copy and paste this into a ticket tomorrow. What are your thoughts on it for now?

@cecinestpasunepipe
Copy link
Contributor Author

My idea was to gradually add a fallback mechanism using this path to plugins permanently, not temporary.

Just swapping paths may not work, some plugins also need registry keys, users and perform other logic. This proposal however allows plugins to detect a possible shortcut and just parse the given file(s). I tested this hypothesis with appdb and it seems to work. It is also a very simple mechanism to wrap your head around. Doing magic stuff with paths requires very straight-forward logic in the plugins, unfortunately I believe that is not the case.

An additional benefit of this proposal is that is relatively risk-free. It doesn't touch any other code, it only adds a fallback location for the LogLoader. It offers a very straight-forward path function, just for convenience that can be used by plugins.

Advantages:

  • Simple to understand
  • Risk-free (but see *)
  • Convenient for plugin devs

Drawbacks:

  • No immediate solution, needs to be gradually adopted by plugins
  • Introduces a new code path for every plugin*
  • No way to communicate whether a plugin supports this or not

@Schamper
Copy link
Member

Schamper commented Sep 26, 2023

Doesn't this approach make the line between "fake" paths and "real" paths very thin? There would need to be a very clear distinction between the "fake" paths and the "real" paths. The currently chosen directory could also conflict with a real directory, so ideally any method for this completely bypasses the real filesystems. In my opinion you quickly move towards something like my proposal.

Plugins that require stuff like registry/config file parsing could of course be supported. For example, I imagine a plugin that requires such a thing would simply do that parsing in a _get_paths() method or equivalent. You could roll everything into a single method that yields tuples of (user, path), or split the methods into a _get_paths() and _get_user_paths().

Underwater, plugins could use something like get_paths() (that in turn calls the implemented methods of that plugin) but also allows getting "extra" paths from the command line, loader logic, or whatever other API. I can imagine a system like this also making the log loader obsolete, since you would simply be able to execute e.g. the evtx plugin directly on some paths you pass in from the CLI.

Again, I'm not sure if my proposal is in fact the best approach, but I very much do not want to mix "fake" paths and files in with real ones. Having to edit every plugin to include a fake path, vs a system where the plugin only lists real paths itself, and "fake paths" are inserted someplace else, seems cleaner to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants