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

Expose landlock API #11

Merged
merged 6 commits into from
Feb 21, 2024
Merged

Expose landlock API #11

merged 6 commits into from
Feb 21, 2024

Conversation

vlaci
Copy link
Contributor

@vlaci vlaci commented Jun 2, 2023

There is also a landlock package on PyPI implementing this, however I already implemented it by the time I found it

@vlaci vlaci force-pushed the landlock branch 5 times, most recently from fa11ecc to 08972b7 Compare June 2, 2023 17:42
@qkaiser qkaiser self-requested a review June 2, 2023 17:44
@qkaiser qkaiser added the enhancement New feature or request label Jun 2, 2023
@vlaci vlaci force-pushed the landlock branch 5 times, most recently from 3acdf72 to e060492 Compare June 2, 2023 18:26
src/sandbox/mod.rs Outdated Show resolved Hide resolved
@vlaci vlaci force-pushed the landlock branch 4 times, most recently from 4fecf16 to a6b718f Compare June 7, 2023 21:13
@qkaiser qkaiser force-pushed the landlock branch 2 times, most recently from 3aee5fa to f288785 Compare January 8, 2024 09:36
@qkaiser
Copy link
Contributor

qkaiser commented Jan 8, 2024

Made it working with the landlock branch in unblob (see onekey-sec/unblob#597).

Had to use the v2 ABI otherwise we get into these kinds of issues due to the absence of LANDLOCK_ACCESS_FS_REFER :

Traceback (most recent call last):
 File "/home/quentin/unblob/unblob/processing.py", line 639, in _extract_chunk
   if result := chunk.extract(inpath, extract_dir):
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/quentin/unblob/unblob/models.py", line 115, in extract
   return self.handler.extract(inpath, outdir)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/quentin/unblob/unblob/models.py", line 452, in extract
   return self.EXTRACTOR.extract(inpath, outdir)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/quentin/unblob/unblob/handlers/filesystem/romfs.py", line 312, in extract
   header.dump_fs()
 File "/home/quentin/unblob/unblob/handlers/filesystem/romfs.py", line 299, in dump_fs
   self.create_inode(inode)
 File "/home/quentin/unblob/unblob/handlers/filesystem/romfs.py", line 272, in create_inode
   self.create_hardlink(output_path, inode)
 File "/home/quentin/unblob/unblob/handlers/filesystem/romfs.py", line 263, in create_hardlink
   self.fs.create_hardlink(dst=output_path, src=target_path)
 File "/home/quentin/unblob/unblob/file_utils.py", line 594, in create_hardlink
   os.link(src, dst)
OSError: [Errno 18] Invalid cross-device link: '/tmp/pytest-of-quentin/pytest-2/test_all_handlers_filesystem_r0/fruits.romfs_extract/hlink_1_1' -> '/tmp/pytest-of-quentin/pytest-2/test_all_handlers_filesystem_r0/fruits.romfs_extract/dir_1/file_1.txt'

@qkaiser
Copy link
Contributor

qkaiser commented Jan 8, 2024

Two things to do before tagging as ready for review:

  • solve the ModuleNotFoundError: No module named 'unblob_native.sandbox' error by properly exposing it
  • add unit testing (unsupported platforms, different access checks)

@vlaci
Copy link
Contributor Author

vlaci commented Jan 9, 2024

* [ ]  solve the `ModuleNotFoundError: No module named 'unblob_native.sandbox'` error by properly exposing it

I currently don't have any examples of it, but it worked for me by manually adding the submodule in sys.modules
This issue contains that workaround suggestion together with suggesting just adding an __init__.py
PyO3/pyo3#759

@qkaiser qkaiser force-pushed the landlock branch 3 times, most recently from 041e497 to 21db054 Compare January 9, 2024 16:26
@qkaiser
Copy link
Contributor

qkaiser commented Jan 9, 2024

The module not found behavior is observed by pylance and pyright, but I confirmed that the module is actually in sys.module.

@qkaiser
Copy link
Contributor

qkaiser commented Jan 9, 2024

I have added a simple unit test that only runs on supported platforms (Linux x86-64). There is only one test because pytest runs all the test under the same process so calling restrict_access in one unit test would affect the behavior of all subsequent tests.

@qkaiser qkaiser marked this pull request as ready for review January 9, 2024 16:30
@vlaci
Copy link
Contributor Author

vlaci commented Jan 9, 2024

The module not found behavior is observed by pylance and pyright, but I confirmed that the module is actually in sys.module.

To solve that, we can add a pyi type stub: https://www.maturin.rs/project_layout.html#adding-python-type-information

I have added a simple unit test that only runs on supported platforms (Linux x86-64). There is only one test because pytest runs all the test under the same process so calling restrict_access in one unit test would affect the behavior of all subsequent tests.

yeah, for separation a multiprocessing.Process or something alike is needed.

@qkaiser
Copy link
Contributor

qkaiser commented Jan 9, 2024

To solve that, we can add a pyi type stub: https://www.maturin.rs/project_layout.html#adding-python-type-information

I leave that to you, this is alien to me.

yeah, for separation a multiprocessing.Process or something alike is needed.

Do you think it's worth doing it ? I'd say the landlock Rust crate is probably well tested. Although we could add some tests on the Rust side of things.

@vlaci vlaci force-pushed the landlock branch 2 times, most recently from 127c729 to 03889ce Compare January 10, 2024 15:10
tests/test_sandbox.py Outdated Show resolved Hide resolved
src/sandbox/mod.rs Show resolved Hide resolved
@qkaiser qkaiser force-pushed the landlock branch 6 times, most recently from 03630c7 to cb1eb89 Compare January 11, 2024 15:48
@qkaiser
Copy link
Contributor

qkaiser commented Jan 15, 2024

@vlaci do you see anything blocking the merge of this PR ? Otherwise I'll approve it.

@vlaci
Copy link
Contributor Author

vlaci commented Feb 20, 2024

@vlaci do you see anything blocking the merge of this PR ? Otherwise I'll approve it.

Looks fine by me

vlaci and others added 6 commits February 20, 2024 17:20
This code makes it possible to import math as a Python
submodule. Extension modules are not packages, so their submodules
are not affected by the usual rules from import machinery.
The default output doesn't contain enough information to diagnose issues.
Landlock is a kernel API for unprivileged access control. We take
advantage of it to limit where unblob can write to and read from on the
filesystem. This is a Linux-only feature that won't be enabled on OSX.

For more information, see https://docs.kernel.org/userspace-api/landlock.html

We use Landlock ABI version 2 since it introduced the
LANDLOCK_ACCESS_FS_REFER permission that's required to create hardlinks.

Co-authored-by: Quentin Kaiser <quentin.kaiser@onekey.com>
@qkaiser qkaiser merged commit 95bfa0f into main Feb 21, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants