-
Notifications
You must be signed in to change notification settings - Fork 529
New linux plugin: modxview #1330
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
New linux plugin: modxview #1330
Conversation
@Abyss-W4tcher it looks good to me.
|
Alright, I thought about a Anyway, the code can indeed be shared somewhere (maybe LinuxUtilities), while keeping the convenient and self-contained APIs inside the Putting the taint flags in other plugins outputs should be en easy task, maybe in a 3-for-1 PR if that's ok for ikelos. |
Sure, this is ultimately up to @ikelos, but I personally lean toward the Unix philosophy: small, specialized tools, each dedicated to a single purpose. This approach doesn't mean each plugin must detect malicious behavior on its own. Then, we can have let's say 'macro' plugins, like the one in this PR, that aggregates various indicators to help identify anomalies. This way, the small plugins can be reused in other "macro" plugins. The kernel tainted flags plugin code will be minimal since we will reuse the code you have already written for modules, but the insights it could provide would be highly valuable and unique. For instance:
The output could be just in a single line, or we could opt for a more detailed, explanatory format such as:
|
Yeah, this is one is for @ikelos. IMO if it's just one function it could be in LinuxUtilities. Alternatively, if there are more related functions, I think it could be more convenient to have a separated class containing that subsystem API .. like https://github.com/volatilityfoundation/volatility3/pull/1332/files#diff-8456da6d20fc84f0d63dedc5bc816ffde418ff41c83b1828da7c614252bda150R840 with its own version, but let's see if ikelos agrees with this idea, since that PR is still pending review. On a related note, I think it would be a good idea to reorganize LinuxUtilities in the future, grouping functions by subsystems such as mount, modules, etc. and only keep in LinuxUtilities the general or framework helpers like
Correct, and call the LinuxUtilities or modules API function with |
Alright, let's wait for ikelos to give its point of view on all these comments, then I'll unify the API depending on where we want to put it. The Globally, it sounds really good to me :) |
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.
Generally looks really nice, next to no comments (which is great work, you're making it harder and harder for me to nit-pick!) 5:P Sadly, the ugly need for version numbers raises its head, so just get that bumped appropriately and sort out the other couple of bits and it should be good to go... 5:)
Hi, this is currently stale, we are waiting for a review regarding the LinuxUtilities stuff. Do we try moving and re-thinking this bit now, or do I just unify the taints function there as discussed ? |
@ikelos: In LinuxUtilities or as a separate/versioned class? I think using a separate class is a better approach, as adding more functionality to LinuxUtilities can lead to issues. Any version change in LinuxUtilities would require version updates in all dependent plugins and APIs. To improve maintainability, we should move to versioned classes dedicated to each subsystem or API. For reference, see the approach taken in this PR. |
That sounds like a reasonable plan @gcmoreira . @Abyss-W4tcher, would you be able to pull the additions out of LinuxUtilities and put them into a separate object in the same vein as the VMCore example Gus provided please? |
@gcmoreira On the more general note, I'm happy to have the submodules split out into versioning subcomponents, but we need to make it slick and fast for people to both check that the submodule exists (so we can add news ones) and that it's the right version. I figure some kind of a check function in LinuxUtilities itself, but then how to do the requirement dependencies gets harder. I suppose we could do some kind of magic where if a module doesn't exist, it instead imports a dummy class successfully but whose version will always fail all version requirements? Starting to sound a little hacky, but the other option would be a specific |
So just to recap, that'd be:
|
I made the requested changes, which lays the ground work for further tainting parsing capabilities as well: (layer_name) >>> from volatility3.framework.symbols.linux import Tainting
(layer_name) >>> kernel = context.modules[self.kernel.name]
(layer_name) >>> tainted_mask = kernel.object_from_symbol("tainted_mask")
(layer_name) >>>
(layer_name) >>> Tainting(self.context, kernel.name).get_taints_parsed(tainted_mask)
['OOT_MODULE', 'UNSIGNED_MODULE'] @gcmoreira the Happy to enhance those capabilities in other plugins once this gets merged, as discussed I might open a dedicated issue to centralize the area of research and the benefits investigators could get from it. |
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.
Thanks for the changes you made. It's close to what I was suggesting, but I'll try and come up with a more formed example tomorrow for you to go off. This is close but it's not quite how I envisaged it...
if self.taint_flags_list: | ||
return self._module_flags_taints_post_4_10_rc1() | ||
return self._module_flags_taints_pre_4_10_rc1() | ||
return linux.Tainting( |
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.
This isn't quite what I had in mind, because it requires LinuxUtilities
to import each of the supported modules, and doesn't allow for plugins to determine whether their requested function will exist or not. I'll have a go at mocking something up tomorrow that's more what I had in mind. I think the separate class stuff is all good, we just need to tweak how we find the function we want to run...
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.
Ok, so we will have to tweak the module
class, to include versioned requirements of the new Tainting
object ?
class module(generic.GenericIntelProcess): |
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.
Errr, there isn't really a way of versioning module
, so it reverts to the version of the framework. If it's purely an additive change (which it looks like it is) then we can just bump the MINOR version (and nothing else needs to change), but if we ever want to change it in the future, then the MAJOR version of the whole framework needs to change. That's why I'm so protective of things like this (and splitting them out into distinct versioned modules is good). I think I thought these were all applying to LinuxUtilities. I hadn't realized they were all being tacked onto module
. I haven't thought about how we version that separately yet. The problem with every thing you version is that it then needs checking before use (which is painful in itself). The requirements let us do that slightly more gracefully and finer grained, but we need to be really careful about what we add to major components of the framework and overriding the linux module
class is one (at least at the moment)... 5:S
Hopefully, all issues should have been addressed or correctly relocated to the appropriate place. In the end, only a minor bump is needed, for the new |
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.
Ok generally looks good, happy with the tainting code being pulled out. I find the intermediate data structure easy to understand, but then we go through all kinds of loops to produce the output, which I think would be easier with a different intermediate data structure. If you genuinely think the current method is the best way then I'll go along with it, but I'd probably then ask for better documentation/variable names to say what's going on in all the loops. Otherwise I think we could improve run_module_scanners
to process the results before returning them.
abc33a9
to
cd8690a
Compare
Sorry for the force-push message, I had an awkward git moment on my local installation '/ |
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.
Ok, I'm happier with this, it's easier to follow, thanks for making the updates. 5:)
Thank you for your time in reviewing this PR. 👍 |
Hello,
This new plugin centralizes the module detection capabilities, and allows to quickly spot and list all the modules. This way, it is easier to detect copycat malicious modules, trying to mimic kernel modules names (ex: https://attack.mitre.org/techniques/T1036/006/). To keep the name familiar for long-term users, it follows the
psxview
naming convention.In addition, this PR introduces the capability to parse taint flags, which embeds additional data to help debug a kernel or a module (as it is in the end, quite similar).
Here are some sample (stripped) outputs :
"/proc/modules" hidden module :
"/proc/modules" hidden module (plain taints string) :
Hidden module :
Note: kovid cleans out the taints attributes, but the UNSIGNED_MODULE one will be there nonetheless. This is still a great piece of information to spot LKM modules.
Happy to read your reviews about this !