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

Improve driveletter selection for windows disks #498

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

pyrco
Copy link
Contributor

@pyrco pyrco commented Jan 8, 2024

As of c97c344 filesystems are always mounted. In the case of "fake" NTFS filesystems, as used by e.g. the dir and tar loaders, this means such a filesystem can be present under up to three different mount points, e.g.: sysvol, C: and $fs$\fs0

When a plugin does not use the root filesystem but the bare ntfs filesystem to parse ntfs specific properties, e.g. the mft plugin, it extends paths with a "driveletter" (read: mountpoint) to construct a path that is reachable from the root filesystem.

The old setup always returned the last mount point a drive was mounted under. The new setup changes this to return the first mount point present in the following order:

  1. the first mount point with a letter and colon, e.g. C:,
  2. the first mount point not starting with $fs$, e.g. sysvol,
  3. the first mount point starting with $fs$, e.g. $fs$\fs0.

@pyrco pyrco requested a review from Schamper January 8, 2024 16:38
Copy link
Member

@Schamper Schamper left a comment

Choose a reason for hiding this comment

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

Unrelated to the changes, but a small improvement on the type check for NTFS filesystems.

# "fake" ntfs filesystem is mounted.
# The precedence for drives is first the drive letter drives (e.g. c:),
# second the "normally" named drives (e.g. sysvol) and finally the anonymous
# drives (e.g. /$fs/fs0).
mount_items = (item for item in target.fs.mounts.items() if hasattr(item[1], "ntfs"))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mount_items = (item for item in target.fs.mounts.items() if hasattr(item[1], "ntfs"))
mount_items = (item for item in target.fs.mounts.items() if item[1].__type__ == "ntfs")

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 will break dir and tar filesystems with a fake ntfs part though.

If the mft plugin walks the actual NtfsFilesystem (which checks on __type__ == "ntfs"). The paths generated from this will get an anonymous fs mountpoint (e.g. $fs$\fs0), but these paths are not reachable through things like target-shell, as the directory structures etc are not available, just the mft.

Copy link
Member

Choose a reason for hiding this comment

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

Pff right, forgot about that and thought we'd already fixed that.

@pyrco pyrco requested a review from Schamper January 9, 2024 13:49
Schamper
Schamper previously approved these changes Jan 9, 2024
@Schamper
Copy link
Member

Schamper commented Jan 9, 2024

Can you maybe add a note about $fs$/fs0 etc to the MFT and other plugin documentation that this touches?

@pyrco pyrco force-pushed the feature/dis-2912_improve-drive-letter-selection branch from 3d9e6c0 to 15fecd0 Compare January 10, 2024 13:14
@pyrco pyrco requested a review from Schamper January 10, 2024 13:14
Copy link

codecov bot commented Jan 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6a8257e) 74.77% compared to head (289e99c) 74.79%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #498      +/-   ##
==========================================
+ Coverage   74.77%   74.79%   +0.01%     
==========================================
  Files         269      269              
  Lines       22073    22085      +12     
==========================================
+ Hits        16506    16518      +12     
  Misses       5567     5567              
Flag Coverage Δ
unittests 74.79% <100.00%> (+0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pyrco pyrco force-pushed the feature/dis-2912_improve-drive-letter-selection branch from 15fecd0 to 78130f3 Compare January 10, 2024 13:47
Copy link
Member

@Schamper Schamper left a comment

Choose a reason for hiding this comment

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

I meant moreso in the docstring of those plugins. This comment is also nice, but it'd also nice to see an explanation of where those $fs$ paths are coming from if you're looking up the documentation for those plugins.

@pyrco pyrco force-pushed the feature/dis-2912_improve-drive-letter-selection branch from 78130f3 to 79061db Compare January 11, 2024 08:44
@pyrco pyrco requested a review from Schamper January 11, 2024 08:44
As of c97c344 filesystems are always mounted. In the case of "fake" NTFS
filesystems, as used by e.g. the dir and tar loaders, this means such a
filesystem can be present under up to three different mount points,
e.g.: sysvol, C: and $fs$\fs0

When a plugin does not use the root filesystem but the bare ntfs
filesystem to parse ntfs specific properties, e.g. the mft plugin, it
extends paths with a "driveletter" (read: mountpoint) to construct a
path that is reachable from the root filesystem.

The old setup always returned the last mount point a drive was mounted
under. The new setup changes this to return the first mount point present
in the following order:

1. the first mount point with a letter and colon, e.g. C:,
2. the first mount point not starting with $fs$, e.g. sysvol,
3. the first mount point starting with $fs$, e.g. $fs$\fs0.
@pyrco pyrco force-pushed the feature/dis-2912_improve-drive-letter-selection branch from 79061db to 4e03ed0 Compare January 11, 2024 14:42
@pyrco pyrco requested a review from Schamper January 11, 2024 14:42
@pyrco pyrco merged commit 079a077 into main Jan 11, 2024
@pyrco pyrco deleted the feature/dis-2912_improve-drive-letter-selection branch January 11, 2024 16:53
Poeloe pushed a commit that referenced this pull request Feb 29, 2024
As of c97c344 filesystems are always mounted. In the case of "fake" NTFS
filesystems, as used by e.g. the dir and tar loaders, this means such a
filesystem can be present under up to three different mount points,
e.g.: sysvol, C: and $fs$\fs0

When a plugin does not use the root filesystem but the bare ntfs
filesystem to parse ntfs specific properties, e.g. the mft plugin, it
extends paths with a "driveletter" (read: mountpoint) to construct a
path that is reachable from the root filesystem.

The old setup always returned the last mount point a drive was mounted
under. The new setup changes this to return the first mount point present
in the following order:

1. the first mount point with a letter and colon, e.g. C:,
2. the first mount point not starting with $fs$, e.g. sysvol,
3. the first mount point starting with $fs$, e.g. $fs$\fs0.
Zawadidone pushed a commit to Zawadidone/dissect.target that referenced this pull request Apr 5, 2024
As of c97c344 filesystems are always mounted. In the case of "fake" NTFS
filesystems, as used by e.g. the dir and tar loaders, this means such a
filesystem can be present under up to three different mount points,
e.g.: sysvol, C: and $fs$\fs0

When a plugin does not use the root filesystem but the bare ntfs
filesystem to parse ntfs specific properties, e.g. the mft plugin, it
extends paths with a "driveletter" (read: mountpoint) to construct a
path that is reachable from the root filesystem.

The old setup always returned the last mount point a drive was mounted
under. The new setup changes this to return the first mount point present
in the following order:

1. the first mount point with a letter and colon, e.g. C:,
2. the first mount point not starting with $fs$, e.g. sysvol,
3. the first mount point starting with $fs$, e.g. $fs$\fs0.
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.

2 participants