volatilityfoundation / volatility3

Volatility 3.0 development
http://volatilityfoundation.org/
Other
2.72k stars 463 forks source link

[core, constants] : add OS and framework supported architectures #1247

Closed Abyss-W4tcher closed 2 months ago

Abyss-W4tcher commented 3 months ago

Hi,

To increase general scalability, in the multi-architectures handling task, I would like to propose a way to unify and remove "string hardcoded" values all around the framework. Indeed, if a new architecture is added, most of the plugins will need to be updated to support this new layer. If there are a few cases where a plugin would work on Intel but not an AArch64 (although I do not see right now any reason), the plugin could still use a custom set of supported architectures.

With this PR, the plugins could later be updated from :

https://github.com/volatilityfoundation/volatility3/blob/517f46e833648d1232b410436e53e07da429d6f5/volatility3/framework/plugins/windows/pslist.py#L33

to :

architectures=WINDOWS_ARCHS,

When a new (CPU) architecture is implemented, this should allow to reduce the amount of small changes in each plugin, and unify everything.


This is also a starting point to resolve the hardcoded intel.Intel checks, which are in fact CPU layers checks.

Going from : https://github.com/volatilityfoundation/volatility3/blob/517f46e833648d1232b410436e53e07da429d6f5/volatility3/framework/automagic/linux.py#L44-L47 to :

# This "check" can also be put in an global utility
if any([isinstance(layer, l) for l in LINUX_ARCHS_LAYERS]): 
   return None 

or equivalent. This can be applied easily inside plugins checks.

ps : For this automagic bit, it can be a bit trickier when virtual CPU layers stacking occurs, as discussed here : https://github.com/volatilityfoundation/volatility3/pull/1088#discussion_r1471104621.


I created a new file, as inserting the code in constants.py resulted in circular imports.

Thanks by advance for the reviews :)

gcmoreira commented 2 months ago

@Abyss-W4tcher I like your idea.

Just a minor note from your comment above: isinstance accepts a tuple. So you should be able to do:

# This "check" can also be put in an global utility
if isinstance(layer, tuple(LINUX_ARCHS_LAYERS)):
   return None 

Alternatively, if you want to go a bit further, you could also change all the definitions in volatility3/framework/constants/architectures.py to tuples. The issue with this is that ModuleRequirement and TranslationLayerRequirement require a list. However, this seems over killing to me, as a quick look at the code suggests they aren't using any specific functionality of a list. Maybe it's a good time to update that as well, but it's up to you and ikelos.

ikelos commented 2 months ago

Hiya, thanks for this (and making it small and easily reviewable rather than part of a mammoth pull request!). 5:)

I'm happy with the idea and I think it should work out fine. We will just need to be very careful over the implementation. This area wasn't fully fleshed out when volatility was first designed so it was mostly as a placeholder we knew we'd need to update. @gcmoreira is right, isinstance does accept a tuple (although you can turn a list into a tuple at time of use, so there's not much between them).

I'm just trying to think through issues we might run into, and the main one I can think of would be automagic somehow managing to fulfil the wrong architecture, but that should be pretty difficult. I don't think the IDs between windows system of different architectures would match, and Linux includes the arch in the uname which we use as the unique identifier. So yep, happy to merge this...