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

Windows: Add filtering by type to handles. #1072

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

AJohnson24
Copy link

In volatility2, the windows.handles plugin allows filtering by type of handle. This MR adds the corresponding functionality to volatility3.

Like for volatility2, specified types are treated as case-insensitive and they are not checked against a list of valid handle types. For example, a user-inputted type of value "Foo" is not a valid type, but it won't trigger an error and will simply yield no handles. The following volatility2 function contains the functionality that this MR replicates: https://github.com/volatilityfoundation/volatility/blob/a438e768194a9e05eb4d9ee9338b881c0fa25937/volatility/plugins/handles.py#L41

Manual testing was performed on a windows memory image to verify the expected behavior. The windows test suite was also run.

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, this looks good. Just one minor bit about the name of the flag, and then happy to merge. 5:)

volatility3/framework/plugins/windows/handles.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/windows/handles.py Outdated Show resolved Hide resolved
@ikelos
Copy link
Member

ikelos commented Feb 6, 2024

I realize what's being done here is actually quite similar to #1077 , so we should have a think if there's a more generic way of doing this or whether it is best done as an additional flag to the plugin. It looks like it uses data from the plugin (information from the symbol table that you'd need to grab), but I'm not sure that significantly speeds up the plugin, and just outputing lines of data or not, is something that can be done by the UI for the display (and wouldn't require rerunning the plugin to change what data is displayed), so this might not be the best way to achieve what we're after? I'll park this for moment whilst we try to figure out the best solution to #1077 and then whether that applies to this or not...

@ikelos
Copy link
Member

ikelos commented Feb 18, 2024

This should now be superseded by #1098, please test that branch and see if it resolves the same problem this was trying to resolve, and if so this PR can be closed. Since this doesn't look like it does the filtering early, I believe #1098 will be just as fast, in which case this PR can be closed off. If you feel it could be faster by doing the filtering at an earlier point, please make the changes to this PR and let me know, otherwise we'll close this one off when #1098 lands. Thanks! 5:)

@AJohnson24
Copy link
Author

I tested the develop branch since #1098 went in and it does indeed solve the aim of my branch.

I did do some benchmarking and it does seem like this branch, because it filters out handle types before they reach the UI layer, is noticeably faster. I can do some more extensive benchmarking and come back with a number to see if it justifies a merge.

I also think it has an advantage when specifying multiple types to filter by, as on this branch we can write:
--type File Semaphore KeyedEvent

whereas on develop you'd need to specify three separate arguments
--filters Type,KeyedEvent --filters Type,Semaphore --filters Type.File

Lmk if I misunderstood something about the --filters feature. Thanks for the time you spent reviewing this PR, and introducing the more generic fix. I'd be happy to close it depending on your thoughts :)

@ikelos
Copy link
Member

ikelos commented Feb 23, 2024

Ok, I'm happy with a noticable speed improvement, you're welcome to do the benchmarks but if you noticed a difference and the code no longer does any heavy lifting as it goes searching, I'm still happy to add it (and you're right, it is less of a pain to do the filtering with a single flag rather than the cumbersome column selection). 5:)

This change adds the '--types' option to the windows.handles plugin,
whereby a user may specify one or more types of handles to filter the
output by.  Any handles not of the type(s) specified are not returned.
Specified types are treated as case-insensitive.

Because the object type index map is generated dynamically,
user-inputted types are not validated against the map of possible object
types.  For example, a user-inputted type of value "Foo" does not raise
an error and will return no results.
@AJohnson24
Copy link
Author

Sorry for the late response, I got around to doing some quick benchmarking to demonstrate the speedup.

I averaged five runs for both the develop branch and ajohnson24/windows-handles branch, rebased onto the tip of develop. I specified three handle types to filter on: KeyedEvent, Semaphore, and File using the syntaxes described above. I used a windows XP image, I'm happy to share it if you'd like to reproduce.

For develop, I got an average runtime of 11.17 seconds. For ajohnson24/windows-handles, I got an average runtime of 8.31 seconds for an overall speedup of 1.34x. I believe the explanation is that this branch performs the filtering at an earlier stage, meaning less data is carried through the program only to be filtered immediately before rendering.

To confirm this, I ran another set of experiments filtering on a single handle type that was relatively rare: KeyedEvent. I hypothesized that since ajohnson24/windows-handles filters at the earliest step, and there'd be less data to pass along, the speedup would be more significant.

For develop, I got an average runtime of 10.67 seconds. For ajohnson24/windows-handles, I got an average runtime of 6.90 seconds for an overall speedup of 1.55x. This tracked with my expectation and shows that the speedup grows more significant as more handles are filtered out.

Thanks for taking the time to review this! Lmk if you consider the speedup enough to justify merging the PR, otherwise feel free to close.

@ikelos
Copy link
Member

ikelos commented Mar 13, 2024

Yeah, it makes perfect sense. What we're looking at doing is exposing the filter list to the plugins through the configuration tree, and therefore letting them do the filtering mid-run instead (same filtering hopefully) so that the UI doesn't need to change, but plugins can save time in certain cases. Just needs a bit of thinking about to ensure how best to transfer the information.

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.

None yet

2 participants