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

New linux plugin: modxview #1330

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

Abyss-W4tcher
Copy link
Contributor

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 :

$ python3 vol.py -f sample1.bin linux.modxview
  |                Name |        Address | In /proc/modules | In /sys/module/ | Hidden |                      Taints
* |                lime | 0xffffc0bd2040 |             True |            True |  False | OOT_MODULE,UNSIGNED_MODULE
* |       rootkit_ports | 0xffffc0a9b180 |             True |            True |  False | OOT_MODULE,UNSIGNED_MODULE
* |        rootkit_char | 0xffffc0a8f2c0 |             True |            True |  False | OOT_MODULE,UNSIGNED_MODULE
* |              vboxsf | 0xffffc0bf8800 |             True |            True |  False | OOT_MODULE,UNSIGNED_MODULE
* |      rootkit_sysfsh | 0xffffc0aa0180 |            False |            True |  False | OOT_MODULE,UNSIGNED_MODULE

"/proc/modules" hidden module (plain taints string) :

$ python3 vol.py -f sample1.bin linux.modxview --plain-taints
  |                Name |        Address | In /proc/modules | In /sys/module/ | Hidden | Taints
* |                lime | 0xffffc0bd2040 |             True |            True |  False |   GOE
* |       rootkit_ports | 0xffffc0a9b180 |             True |            True |  False |   GOE
* |        rootkit_char | 0xffffc0a8f2c0 |             True |            True |  False |   GOE
* |              vboxsf | 0xffffc0bf8800 |             True |            True |  False |   GOE
* |      rootkit_sysfsh | 0xffffc0aa0180 |            False |            True |  False |   GOE

Hidden module :

$ python3 vol.py -f sample2.bin linux.modxview
  |                Name |        Address | In /proc/modules | In /sys/module/ | Hidden |                      Taints
* |                 drm | 0xffffc03809c0 |             True |            True |  False |                          -
* |               kovid | 0xffffc09ed4c0 |            False |           False |   True |            UNSIGNED_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 !

@Abyss-W4tcher Abyss-W4tcher changed the title New plugin: modxview New linux plugin: modxview Oct 31, 2024
@gcmoreira
Copy link
Contributor

gcmoreira commented Nov 4, 2024

@Abyss-W4tcher it looks good to me.
There are some potential improvements that could be included in this or a future PR, but they would impact the current design.

  • It would be amazing to also have a Linux Kernel Tainted flags plugin that checks the (tainted_mask) flags when TaintFlags.module == False. That's what you get when read /proc/sys/kernel/tainted and it's implemented here. This indicates that the tainted flags parsing code should be relocated to a shared/common location. How about consolidating to a single function, i.e. parse_tainted_mask(tainted_mask, is_module), that can be used for both the module tainted mask and the kernel tainted mask?
  • It would be ideal if the module tainted flags were reported consistently across the three other plugins you are using here, and not just in this one.

@Abyss-W4tcher
Copy link
Contributor Author

@Abyss-W4tcher it looks good to me. There are some potential improvements that could be included in this or a future PR, but they would impact the current design.

  • It would be amazing to also have a Linux Kernel Tainted flags plugin that checks the (tainted_mask) flags when TaintFlags.module == False. That's what you get when read /proc/sys/kernel/tainted and it's implemented here. This indicates that the tainted flags parsing code should be relocated to a shared/common location. How about consolidating to a single function, i.e. parse_tainted_mask(tainted_mask, is_module), that can be used for both the module tainted mask and the kernel tainted mask?
  • It would be ideal if the module tainted flags were reported consistently across the three other plugins you are using here, and not just in this one.

Alright, I thought about a linux.kernel_taints plugin, but wasn't sure about the relevance of such a small plugin, thinking it would be better integrated in a windows.info equivalent for Linux ? However, I'm definitely ok for parsing the tainted_mask.

Anyway, the code can indeed be shared somewhere (maybe LinuxUtilities), while keeping the convenient and self-contained APIs inside the extensions.module as a wrapper for them ?

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.

@gcmoreira
Copy link
Contributor

gcmoreira commented Nov 4, 2024

Alright, I thought about a linux.kernel_taints plugin, but wasn't sure about the relevance of such a small plugin, thinking it would be better integrated in a windows.info equivalent for Linux ? However, I'm definitely ok for parsing the tainted_mask.

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:

Flag  Description
----- -----------
A     desc A
B     desc B
C     desc C

@gcmoreira
Copy link
Contributor

gcmoreira commented Nov 5, 2024

Anyway, the code can indeed be shared somewhere (maybe LinuxUtilities)

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 container_of(), get_module_from_volobj_type(), etc. So, if everyone agree with this, it may be a good opportunity to start differentiating components from LinuxUtilities. This will also help minimize the impact of modifying a specific subsystem API and reduce the scope of its corresponding testing.

while keeping the convenient and self-contained APIs inside the extensions.module as a wrapper for them ?

Correct, and call the LinuxUtilities or modules API function with parse_tainted_mask(self.taints_value, is_module=True) and from the the Linux kernel tainted plugin parse_tainted_mask(tainted_mask_value, is_module=False).

@Abyss-W4tcher
Copy link
Contributor Author

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 kernel_taints plugin might end up calling modxview and then check for a LKM flag etc., so it might be worth opening a dedicated issue to centralize what this plugin should check and get a clear picture of what it leverages and gives insights about.

Globally, it sounds really good to me :)

@gcmoreira
Copy link
Contributor

Hey @ikelos any chance you could take a look at this PR? We need it sorted before moving on to #1286. Thanks

Copy link
Member

@ikelos ikelos left a 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:)


return taints_string

def get_taints_as_plain_string(self) -> str:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are externally visible additions to the API, which means a MINOR version number somewhere, needs to go up. I suspect it may be the framework itself because I don't think anything further down is versioned?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, we want to include this API in a more generic place, but keep these convenient self-contained functions. So right now I'm not sure of how many bumps are needed.

volatility3/framework/symbols/linux/extensions/__init__.py Outdated Show resolved Hide resolved
volatility3/framework/symbols/linux/extensions/__init__.py Outdated Show resolved Hide resolved
@Abyss-W4tcher
Copy link
Contributor Author

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 ?

@gcmoreira
Copy link
Contributor

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.

@ikelos
Copy link
Member

ikelos commented Dec 22, 2024

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?

@ikelos
Copy link
Member

ikelos commented Dec 22, 2024

@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 LinuxUtilityRequirement where you specify the name of the module and the version, and then it does the import attempt and fails. Except then how does the plugin get the imported module if it exists? 5:S We can't populate the config with it because it's not a simple value and I don't want to go wading into the config system to butcher something in place. Otherwise there's no way for the config system to report back success or failure of a requirement (and we'd have to start littering the config system with the requirements). So you'd need a LinuxUtilities lookup function and a LinuxUtilitiesRequirement which would allow the requirement to fail if it couldn't be fulfilled and otherwise the lookup to return the module? I think that could work? We'd need to leave the old ones in place to avoid having to change every plugin. But they could probably be turned into shims that just do the module check themselves and then proxy the call? Big PR, but I'm up for reviewing it if someone wants to write it... 5;)

@ikelos
Copy link
Member

ikelos commented Dec 22, 2024

So just to recap, that'd be:

LinuxUtilities:
    get_module('module_name') -> Module:
    // Probably this is a generic wrapper that we can parameterize so just old_function = smart_wrapper('module', 'function_name')
    old_function = smart_wrapper('module', 'function_name')
    smart_wrapper(args):
        modobj = get_module(module)
        return getattr(module, function_name)(args)
    
LinuxUtilitiesModule(Versionable):
    _version
    <previous functions>

LinuxUtilitiesRequirement // with params (module, version)

@Abyss-W4tcher
Copy link
Contributor Author

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 is_module flag seems to only strictly apply to modules, in the sense that tainted_mask parsing does not account for this flag: https://elixir.bootlin.com/linux/v6.5/source/kernel/panic.c#L492 vs https://elixir.bootlin.com/linux/v6.5/source/kernel/module/main.c#L879.

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.

Copy link
Member

@ikelos ikelos left a 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...

@@ -376,13 +329,6 @@ def section_strtab(self):
return self.strtab
raise AttributeError("Unable to get strtab")

@property
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property needs to be exposed to avoid a bump too....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property was removed from the module class, not sure if you meant to write the comment here ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing it is a non-additive API change. That's why I said it needs to still be exposed there for anything that had been using it, otherwise a caller is going to call it expecting it to work and they'll get an ugly AttributeError. The point of the version numbers is to avoid situations like that, so happy for you to bump the version number, or put the property back, but you need to do one of them...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, I see, module isn't versioned at all! 5:S That gets trickier... 5:\

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still needs to be exposed then, otherwise any code calling module_instance.taint_flags_list is going to throw an error. This is why I like people to be careful about what they throw into classes like this. Changing them in the future is tricky... 5:P

Copy link
Contributor Author

@Abyss-W4tcher Abyss-W4tcher Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property was added in this PR :), it was not present before. Changes might be confusing as we are moving things.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it was only to be used internally, it needed to be called _taint_flags_list...

Copy link
Contributor Author

@Abyss-W4tcher Abyss-W4tcher Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point, however there is no need for bumping as it hasn't made it into the framework yet. I'll make sure to make it "private" though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, sorry, I know it's tricky to get right (and I'm getting lost looking at commit diffs inside the PR, so I thought it had already gone in). Yeah, anything that doesn't need to be accessed from the outside would should be marked with a leading _, otherwise it becomes part of the API and you've seen the mess of versioning that comes from that... 5;)

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(
Copy link
Member

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...

Copy link
Contributor Author

@Abyss-W4tcher Abyss-W4tcher Dec 23, 2024

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):

Copy link
Member

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

@@ -830,3 +831,123 @@ def get_cached_pages(self) -> Iterator[interfaces.objects.ObjectInterface]:
page = self.vmlinux.object("page", offset=page_addr, absolute=True)
if page:
yield page


class Tainting:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might want to go in a tainting.py module, so it would become linux.tainting.Tainting (and so that the init doesn't get too full).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also would need its own version number, and a required_framework_version for anything it made use of (and the __init__ would need to check those...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, see my branch for an example.

- kernel: print_tainted
"""

def __init__(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also expected these just to be function containers, rather than carrying state themselves. We can have them carry state I guess, I'm just not sure the extra complexity/separation of parameters is worth it?

@Abyss-W4tcher
Copy link
Contributor Author

Alright, no problem. I took reference from https://github.com/volatilityfoundation/volatility3/pull/1332/files#diff-8456da6d20fc84f0d63dedc5bc816ffde418ff41c83b1828da7c614252bda150R840 as indicated, but it might not have been exactly what you had in mind for this topic.

@ikelos
Copy link
Member

ikelos commented Dec 23, 2024

What I'd been thinking of was a little more along these lines?

https://github.com/volatilityfoundation/volatility3/tree/feature/linux-utilities-split

It allows for all the old code to keep working, but new code should be to import the separate modules. If a new plugin is used with an old framework, the import will fail and the plugin won't load (kinda gracefully), if an old plugin is used with a new framework, it should raise a deprecation warning and eventually we can bump LinuxUtilities MAJOR version and drop all the proxied functions. That will probably have to happen after a MAJOR bump to any of the modules (otherwise the proxied calls would change without anyway of telling from the deprecated perspective). Slowly we should be able to empty it down to just general linux functions and have all the others off using separate modules. Hopefully that all makes sense, naming of the imported modules may require some work (because we typically only keep the module name, not several modules below, so linux and utilities will get lost (hence my as linux_paths in the example). I'm not intended to push the whole example, probably just the deprecation capability, just I wanted to show how I was envisaging it all unfolding....

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

Successfully merging this pull request may close these issues.

4 participants