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

EXT Filesystem Components #337

Merged

Conversation

rbs-jacob
Copy link
Member

@rbs-jacob rbs-jacob commented Jun 26, 2023

One sentence summary of this PR (This should go in the CHANGELOG!)

Add packers and unpackers for EXT filesystems (versions 2 through 4).

Anyone you think should look at this, specifically?

@whyitfor @EdwardLarson

Outstanding tasks:

  • Packer
  • Tests

Copy link
Collaborator

@EdwardLarson EdwardLarson left a comment

Choose a reason for hiding this comment

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

Is there any dependencies that need to be installed? That is, is a change to the Dockerstub missing from this PR?

IIRC this doesn't work for Windows. I feel it's alright that this only works on Linux/Mac systems, if not ideal. The ComponentExternalTool being there can protect against trying to run it on Windows. I do think there should be a TODO comment somewhere in here explicitly saying Windows support is limited.

@rbs-jacob
Copy link
Member Author

rbs-jacob commented Jul 6, 2023

Is there any dependencies that need to be installed? That is, is a change to the Dockerstub missing from this PR?

IIRC this doesn't work for Windows. I feel it's alright that this only works on Linux/Mac systems, if not ideal. The ComponentExternalTool being there can protect against trying to run it on Windows. I do think there should be a TODO comment somewhere in here explicitly saying Windows support is limited.

On macOS, there is a dependency that needs to be installed via brew, and is included as a ComponentExternalTool. There are no such external dependencies on Linux, since I believe Debian bundles the required tools by default.

jacob@blade:~$ docker run --rm -it debian:latest bash
root@25be021f7df1:/# which debugfs
/usr/sbin/debugfs

While it's true that I haven't been able to find a version that works for Windows, I suspect that the 7z command line tool can at least extract EXT filesystems. It may not be able to rebuild them, though. It seems like we agree that not having a Windows version of the tool should block merging this.

@rbs-jacob rbs-jacob marked this pull request as ready for review July 26, 2023 20:41
@rbs-jacob
Copy link
Member Author

In the interest of getting this merged in, we're omitting a packer for now. A packer is considerably more complicated to add because of cross-platform concerns with the normal workflow for making a populated EXT filesystem. Whereas for unpacking, the debugfs utility can easily be used cross-platform.

The workflow for packing usually involves:

  • Making an empty file with some dd incantation
  • Using some variation of mkfs.ext4 to make the file into a filesystem
  • Mounting the filesystem with mount
  • Populating the filesystem
  • Unmounting it

This can all be done fairly easily from a Linux system, but for other OSes, the kernels lack support for EXT filesystems, so they cannot easily be mounted. The packer will likely have to involve some alternative, cross platform way to build and populate them. As such, we're waiting for a separate PR for the packer so that this work doesn't stall in the short-term.

Copy link
Collaborator

@EdwardLarson EdwardLarson left a comment

Choose a reason for hiding this comment

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

The debugf dependency should be "private" so as to not be imported elsewhere.

The test cases could maybe be combined into one class rather than a separate one for each EXT version, maybe with some clever pytest fixtures, but that's arguably harder to read and not important anyway.

ofrak_core/ofrak/core/extfs.py Outdated Show resolved Hide resolved
ofrak_core/ofrak/core/extfs.py Outdated Show resolved Hide resolved
Co-authored-by: Edward Larson <wasabiofip@gmail.com>
@EdwardLarson EdwardLarson merged commit d0dc623 into redballoonsecurity:master Jul 31, 2023
5 checks passed
@EdwardLarson EdwardLarson mentioned this pull request Aug 10, 2023
1 task
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