Skip to content

Conversation

valoq
Copy link
Contributor

@valoq valoq commented Oct 1, 2024

This PR adds landlock filesystem isolation to ouch as discussed in #722

At the moment this is just a quickly hacked implementation to demonstrate the use of landlock in ouch.
It restricts the entire filesystem to be read only and only permits write actions in the current working directory of the process.

In order to test the isolation feature, use the -d option to write the decompressed files to a path outside of $PWD
A final implementation would address the -d option as well to allow writing to specified output directories, but I left it untouched for now to allow an easy demonstration/test of the landlock feature.

Todo:

  • restrict write permissions to the .tmp-XXXXXX directory, including permissions for said directory to rename itself to the final name LANDLOCK_ACCESS_FS_MAKE_DIR (This should avoid writing to any control files in $HOME like .bashrc)
  • Identify all special cases that require permissions (see failed unit tests)

@valoq
Copy link
Contributor Author

valoq commented Oct 1, 2024

Note that the test failed due to the missing permissions.
This is expected for the current version but also indicates the missing cases to add permissions for before merging.

@valoq
Copy link
Contributor Author

valoq commented May 4, 2025

Updated the branch to align with version 0.6.1 and added pseudo code that I had in mind to finish this feature.
Unfortunately I would need some time to get more familiar with rust to correctly implement this, which I wont have in the foreseeable future. It should not take much to finish the implementation for anyone with basic rust skills though if there is interest.

@marcospb19
Copy link
Member

For the scope of this PR, I think it's OK for this to only be activated for Linux >=5.19, which should be the limitation for the Landlock V2.

I'm also suffering from low free time to put on this, so I 100% comprehend this being stale.

@marcospb19 marcospb19 marked this pull request as draft June 28, 2025 01:22
@valoq valoq force-pushed the main branch 2 times, most recently from 25c4ff9 to 61599df Compare June 28, 2025 18:04
@valoq
Copy link
Contributor Author

valoq commented Jun 28, 2025

Ok this is now a step further and works with -d option as well.

Todo:

  • support arbitrary directories in ouch-compress
  • fix the ugly cli args fix to expose output_dir
  • restrict write permissions to the .tmp-XXXXXX directory instead of current dir
  • ensure CI is working

@valoq
Copy link
Contributor Author

valoq commented Jun 29, 2025

This feature should now work and is ready for some testing by those interested.

At the moment the flowing cases are supported:

  • Listing files in an archive will have write restriction to "/tmp" since some formats create temporary files there
  • Decompressing restricts write access to either the specified directory (-d option) or the current working directory where ouch is executed

TODO:

  • Fix CI test: allow removing archive file, allow overwriting files
  • implement landlock unit test
  • Move tmpfile for rar (and possible other formats) to the global decompress (and list) space to provide to landlock

Future improvements:
The landlock restriction can potentially be more strict when no output directory is specified.
Currently, this results in write permissions for the current working directory, which is can be unsafe if ouch is run in $HOME for example. By allowing write actions only to the temporary directory while allowing LANDLOCK_ACCESS_FS_MAKE_DIR in the current working directory for later renaming of the temporary directory will fix this.
However at the moment ouch does not always use a temporary directory in all cases, which requires this to be changed first for effective isolation. Also see onekey-sec/unblob#597

Add landlock for compression?

Additionally the isolation for the list command can be further restricted to only allow write permissions in the temporary directory created in "/tmp" (Implemented)

@valoq
Copy link
Contributor Author

valoq commented Jul 21, 2025

The CI currently fails due to #852

If we fix the overwrite behavior to avoid the recreation of the target directory, it should work as expected

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