dissect.target icon indicating copy to clipboard operation
dissect.target copied to clipboard

Add target-diff

Open JSCU-CNI opened this issue 1 year ago • 5 comments

This PR adds the command target-diff, which can be used to compare two or more targets against one another:

$ target-diff --help

target-diff

positional arguments:
  {shell,fs,query}      Mode for differentiating targets
    shell               Open an interactive shell to compare two or more targets.
    fs                  Yield records about differences between target filesystems.
    query               Differentiate plugin outputs between two or more targets.

options:
  -d, --deep            Compare file contents even if metadata suggests they have been left unchanged (default: False)
  -l LIMIT, --limit LIMIT
                        How many bytes to compare before assuming a file is left unchanged (0 for no limit) (default:
                        32768)

fs mode outputs records denoting filesystem changes from one target to the other:

$ target-diff --deep fs src.tar dst.tar

<differential/file/created hostname=None domain=None src_target='src.tar' dst_target='dst.tar' path='/changes/only_on_dst'>
<differential/file/deleted hostname=None domain=None src_target='src.tar' dst_target='dst.tar' path='/changes/only_on_src'>
<differential/file/modified hostname=None domain=None src_target='src.tar' dst_target='dst.tar' path='/changes/changed' diff=[b'--- \n', b'+++ \n', b'@@ -1 +1 @@\n', b'-SRC', b'+DST']>

Using query mode, you can compare plugin outputs from one target to the other:

$ target-diff query -f users src.tar dst.tar

<differential/record/unchanged hostname=None domain=None src_target='src.tar' dst_target='dst.tar' record=<unix/user hostname='dst_target' domain=None name='root' passwd='x' uid=0 gid=0 gecos='root' home='/root' shell='/bin/bash' source='/etc/passwd'>>
<differential/record/unchanged hostname=None domain=None src_target='src.tar' dst_target='dst.tar' record=<unix/user hostname='dst_target' domain=None name='user' passwd='x' uid=1000 gid=1000 gecos='user' home='/home/user' shell='/bin/bash' source='/etc/passwd'>>
<differential/record/created hostname=None domain=None src_target='src.tar' dst_target='dst.tar' record=<unix/user hostname='dst_target' domain=None name='dst_user' passwd='x' uid=1001 gid=1001 gecos='dst_user' home='/home/dst_user' shell='/bin/bash' source='/etc/passwd'>>
<differential/record/deleted hostname=None domain=None src_target='src.tar' dst_target='dst.tar' record=<unix/user hostname='src_target' domain=None name='src_user' passwd='x' uid=1001 gid=1001 gecos='src_user' home='/home/src_user' shell='/bin/bash' source='/etc/passwd'>>

In shell mode, you can browse the target filesystems like in target-shell, where directory listings will show which files / directories have been changed, added or deleted. Using the plugin command, plugin outputs can be compared from within the shell context.

$ target-diff shell src.tar dst.tar

(dst_target/src_target)/diff />help

Target Diff
==========


Documented commands (type help <topic>):
=================================================================
cat  clear  diff   exit  help  ls    plugin  previous  set
cd   cyber  enter  find  list  next  prev    python  

(dst_target/src_target)/diff />cd changes
(dst_target/src_target)/diff /changes>ls
changed
only_on_dst (deleted)
only_on_src (created)
subdirectory_both
subdirectory_dst (deleted)
subdirectory_src (created)
unchanged

target-diff depends on https://github.com/fox-it/flow.record/pull/107. To allow tests to run for this PR we've temporarily bumped flow.record to 3.15.dev10 in pyproject.toml

When three or more targets are provided, you can choose between treating every target as a 'delta' or compare every target against one 'absolute' target. Treating targets as 'deltas' is useful if you have multiple snapshots of the same target from different points in time. Treating targets as 'absolutes' can be useful in situations where you have a 'golden image' that you want to compare different targets against.

To keep code duplication low between tools/diff.py and tools/shell.py, this PR adds a superclass ExtendedCmd to shell.py that contains most of the functionality that is shared between the two. Both TargetCmd and DifferentialCli inherit from this class.

JSCU-CNI avatar Apr 04 '24 16:04 JSCU-CNI

Thanks for your suggestions on the flow.record context manager addition. We were wondering if there is an ETA on a review of the PR as a whole, considering it's been over 2 months.

JSCU-CNI avatar Jun 12 '24 10:06 JSCU-CNI

We understand this PR will take some time to review, but we do think it might be worthwhile to incorporate the changes to shell.py (the ExtendedCmd class, some fixes to target-shell, moving some functions outside of Cmd classes so they can be re-used) into main earlier. In its current state, this PR being left open for a long time causes it to increasingly divert from target-shell.

What we could do is move the shell.py changes into a seperate PR that you can then review first. We don't mind making this PR, and while we're at it we can also pick up some open issues along the way, such as #623, #624, #625 and #585. These improvements can be used for target-fs as well, as it only makes sense to re-use as much as possible between target-shell and target-fs when it comes to outputting target filesystem information. This will likely create a merge conflict with #716 if that is not yet merged, but if that ends up happening we'll incorporate those changes as well.

Do you agree with this approach?

JSCU-CNI avatar Aug 01 '24 09:08 JSCU-CNI

I am working slowly through the backlog of long outstanding PRs but had not gotten to this yet. Your proposal of splitting it up sounds like a good idea though, it's always nice if we can split of large PRs into separate smaller ones. I won't complain either if you pick up some of the mentioned improvements along the way :smile:.

I've just merged #716 for your convenience so feel free to do with that as you please :wink:.

Schamper avatar Aug 01 '24 12:08 Schamper

Target-diff should be more or less on par with the changes made in #812 and should be ready for review @Schamper .

JSCU-CNI avatar Sep 16 '24 09:09 JSCU-CNI

I will check if someone from the dissect team can review this.

yunzheng avatar Oct 07 '24 10:10 yunzheng

Codecov Report

Attention: Patch coverage is 69.23077% with 176 lines in your changes missing coverage. Please review.

Project coverage is 77.69%. Comparing base (e728a2c) to head (99b7177). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
dissect/target/tools/diff.py 69.23% 176 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #664      +/-   ##
==========================================
- Coverage   77.85%   77.69%   -0.17%     
==========================================
  Files         325      326       +1     
  Lines       27909    28481     +572     
==========================================
+ Hits        21729    22127     +398     
- Misses       6180     6354     +174     
Flag Coverage Δ
unittests 77.69% <69.23%> (-0.17%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 28 '24 10:10 codecov[bot]

Thanks for the review @cecinestpasunepipe, I've applied your suggestions in 0a522e9.

JSCU-CNI avatar Nov 11 '24 11:11 JSCU-CNI

@JSCU-CNI looks good to me, can you just fix the remaining tests? Apart from that, I have no further comments. I also want to add that we really appreciate this feature, nice work!

cecinestpasunepipe avatar Nov 11 '24 16:11 cecinestpasunepipe

Thanks @cecinestpasunepipe! The tests should be fixed in c3266b9.

JSCU-CNI avatar Nov 11 '24 16:11 JSCU-CNI

LGTM, I have no further requests.

cecinestpasunepipe avatar Nov 12 '24 15:11 cecinestpasunepipe

The linked issue #795 was closed but the PR is still open. Did something go wrong there?

JSCU-CNI avatar Nov 13 '24 09:11 JSCU-CNI

@JSCU-CNI Can you rebase the branch?

The linked issue #795 was closed but the PR is still open. Did something go wrong there?

@EinatFox do you know why this issue was closed?

yunzheng avatar Nov 13 '24 13:11 yunzheng

@JSCU-CNI Can you rebase the branch?

The linked issue #795 was closed but the PR is still open. Did something go wrong there?

@EinatFox do you know why this issue was closed?

I see this was wrongly marked as done, reopened the issue.

EinatFox avatar Nov 18 '24 14:11 EinatFox

What is the status of this PR?

JSCU-CNI avatar Nov 21 '24 13:11 JSCU-CNI

:partying_face:

JSCU-CNI avatar Dec 04 '24 13:12 JSCU-CNI