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

Feature: Remote analysis on cloud object-storage. #1037

Merged
merged 6 commits into from
Dec 1, 2023

Conversation

forensicxlab
Copy link
Contributor

@forensicxlab forensicxlab commented Nov 12, 2023

Hello,

  • Adding a new ResourceAccessor to the FileLayer and updated the cli/shell in order to be able to perform memory analysis on remote s3 storage (Amazon/MinIO/Google).

More information about the steps taken : https://www.forensicxlab.com/posts/vols3/

Best regards.

@eve-mem
Copy link
Contributor

eve-mem commented Nov 13, 2023

@forensicxlab - this is a really neat idea! Please wait for a review from a core dev before you waste any time trying to 'fix' my comments - I'm just a random internet vol user - but I thought I'd at least give you a view from someone else. I hope it helps.

The main thing I thought when looking at this is how it would break using vol with only the minimal requirements (e.g. just pefile) so even if you were on an offline laptop or something you'd need the packages to read from s3 even though you never would need to.

I think it might be a better idea to add this in much like the leechcore layer, if the packages are there you can use it - and if they are not it just means that leechcore layer is not useable.

If you look at the leechcore layer (https://github.com/volatilityfoundation/volatility3/blob/develop/volatility3/framework/layers/leechcore.py) you'll see how it adds the leechcore: handler - I imagine you could do something similar with s3: meaning you wouldn't need to mess with the physical layer or uri parsing parts - just register your handler and vol will come to your layer if the user supplied the right kind of path.

I think you might be able to use a get_requirements to handle the checks for access keys etc - which would then mean the 'normal' flow of vol would catch that error if none was supplied and raise it that way (which is useful for none vol.py and volshell.py uses of the framework)

Really cool work - I can see this kind of thing being useful in an enterprise where lots of memory is being pulled back quickly into s3 or similar that needs processing quickly by some machine in AWS or similar.

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 very much for the contribution, it looks like it'll be awesome! Sadly it's tinkering a lot in parts of the core and the CLI, and it would be much more modular and have less impact on the existing running if it plugged into the python URL handler. There's already an example of doing this in the leechcore layer.

It tests whether the leechcore modules can import before it defines the classes, then it defines a VolatilityHandler class. This will automatically get loaded if it's defined and it should read the URL, and if it can be handled, return the appropriate handler, and if not return None. The mechanism for this can be ready about at the python docs for OpenDirectors. You can provide a hint as to whether volatility should try to cache the contents of the URL or not (with full memory images, almost certainly not) by overriding the non_cached_schemes method to provide the URL scheme that shouldn't be cached. Otherwise the class is just a normal BaseHandler.

I won't be merging this in its current form, but if you get it ported over the Handler model, then I'll definitely get it merged! Please shout if you need a hand with anything, feel free to do updates in this PR rather than starting a new one... 5:)

volatility3/cli/__init__.py Outdated Show resolved Hide resolved
volatility3/cli/__init__.py Outdated Show resolved Hide resolved
volatility3/cli/__init__.py Outdated Show resolved Hide resolved
volatility3/cli/volshell/__init__.py Outdated Show resolved Hide resolved
volatility3/framework/layers/physical.py Outdated Show resolved Hide resolved
volatility3/framework/layers/resources.py Outdated Show resolved Hide resolved
@forensicxlab
Copy link
Contributor Author

Hello @ikelos, @eve-mem,

Thank you a lot for your feedback!
I will get right into the implementation of the necessary changes and update this pull request ;)

@forensicxlab
Copy link
Contributor Author

forensicxlab commented Nov 18, 2023

Hello @eve-mem, @ikelos,

I think I got it right this time.
Tested both the feature with success. Raised a "LayerException" if the requirement is missing.

I have reset to commit 581c493 to undo my previous modifications and pushed the new implementation.

Let me know what you think :) Thank you both again, I have learned a lot !

Best regards.

S3 buckets

./vol.py -f s3://5c00720b-15a6-4f0a-9bb0-bba614ff2462/Investigation-2.raw windows.pslist
Volatility 3 Framework 2.5.0
Progress:  100.00               PDB scanning finished                        
PID     PPID    ImageFileName   Offset(V)       Threads Handles SessionId       Wow64   CreateTime      ExitTime        File output

4       0       System  0x823c8830      51      244     N/A     False   N/A     N/A     Disabled
348     4       smss.exe        0x82169020      3       19      N/A     False   2017-05-12 21:21:55.000000      N/A     Disabled
596     348     csrss.exe       0x82161da0      12      352     0       False   2017-05-12 21:22:00.000000      N/A     Disabled
620     348     winlogon.exe    0x8216e020      23      536     0       False   2017-05-12 21:22:01.000000      N/A     Disabled
664     620     services.exe    0x821937f0      15      265     0       False   2017-05-12 21:22:01.000000      N/A     Disabled

GCS buckets

./vol.py -f gs://volweb/Investigation-2.raw windows.dlllist

Volatility 3 Framework 2.5.0
Progress:  100.00               PDB scanning finished                        
PID     Process Base    Size    Name    Path    LoadTime        File output

348     smss.exe        0x48580000      0xf000  smss.exe        \SystemRoot\System32\smss.exe   N/A     Disabled
348     smss.exe        0x7c900000      0xb2000 ntdll.dll       C:\WINDOWS\system32\ntdll.dll   N/A     Disabled
596     csrss.exe       0x4a680000      0x5000  csrss.exe       \??\C:\WINDOWS\system32\csrss.exe       N/A     Disabled
596     csrss.exe       0x7c900000      0xb2000 ntdll.dll       C:\WINDOWS\system32\ntdll.dll   N/A     Disabled
596     csrss.exe       0x75b40000      0xb000  CSRSRV.dll      C:\WINDOWS\system32\CSRSRV.dll  N/A     Disabled
596     csrss.exe       0x75b50000      0x10000 basesrv.dll     C:\WINDOWS\system32\basesrv.dll N/A     Disabled
596     csrss.exe       0x75b60000      0x4b000 winsrv.dll      C:\WINDOWS\system32\winsrv.dll  N/A     Disabled
596     csrss.exe       0x77f10000      0x49000 GDI32.dll       C:\WINDOWS\system32\GDI32.dll   N/A     Disabled
596     csrss.exe       0x7c800000      0xf6000 KERNEL32.dll    C:\WINDOWS\system32\KERNEL32.dll        N/A     Disabled
596     csrss.exe       0x7e410000      0x91000 USER32.dll      C:\WINDOWS\system32\USER32.dll  N/A     Disabled
596     csrss.exe       0x629c0000      0x9000  LPK.DLL C:\WINDOWS\system32\LPK.DLL     N/A     Disabled

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 very much, this looks MUCH cleaner and nicer!

Only a few points:

  • we use __name__ instead of __file__ so to keep the logs consistent, this should use that too.
  • objectstorage.py feels a little generic? Would cloudstorage by more accurate?
  • the URL handling feels like a string mangling rather than a library usage, could we turn it into a library usage, so we get all the free benefits please?

volatility3/framework/layers/objectstorage.py Outdated Show resolved Hide resolved
volatility3/framework/layers/objectstorage.py Outdated Show resolved Hide resolved
volatility3/framework/layers/objectstorage.py Outdated Show resolved Hide resolved
volatility3/framework/layers/objectstorage.py Outdated Show resolved Hide resolved
volatility3/framework/layers/objectstorage.py Outdated Show resolved Hide resolved
@forensicxlab
Copy link
Contributor Author

Hello @ikelos,
Thank you for all your insights!

  • Indeed, the requirement conditions placement is not clean, I have kept things simple in the new commit.
  • Let me know what you think about the new way of parsing the URL.

I think the leechcore plugin might be subject to your comments regarding the URL parsing method and logger used.

@ikelos
Copy link
Member

ikelos commented Nov 29, 2023

Ah, you're right, leechcore does it almost the same way... I'm probably just being overly cautious, and this isn't exactly a URL, it's more a way to pass a kind of a connection string through to the code, so I'll merge it as it. Thanks for all your work on it! 5:)

@ikelos ikelos self-requested a review November 29, 2023 20:54
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.

Looks good, thanks! 5:)

@ikelos
Copy link
Member

ikelos commented Nov 29, 2023

Hmmm, this has shown up some issues during testing. Volatility 3 supports python 3.7 and upwards, but the version depended on (2023.6.0) is only for 3.8 and above. It looks like 2023.1 is the last version that supported 3.7. Do you know if the plugin will work with that version and whether we could change the requirements to support that as the minimum version please?

Also, could you run black over it to correct the formatting (or use the patch generated here to fix it) please?

@forensicxlab
Copy link
Contributor Author

Hello @ikelos, should be ok now :)
Many thanks !

Best regards.

except ImportError:
HAS_GCSFS = False

from volatility3.framework import exceptions

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'exceptions' is not used.
@ikelos ikelos merged commit 42a89cf into volatilityfoundation:develop Dec 1, 2023
14 checks passed
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.

4 participants