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

landlock sandboxing is too permissive in tests #1086

Open
qkaiser opened this issue Jan 22, 2025 · 1 comment
Open

landlock sandboxing is too permissive in tests #1086

qkaiser opened this issue Jan 22, 2025 · 1 comment
Assignees
Labels
bug Something isn't working CI/CD Pull request that updates our Github CI/CD python Pull requests that update Python code

Comments

@qkaiser
Copy link
Contributor

qkaiser commented Jan 22, 2025

We introduced landlock to sandbox the unblob process and limit what it can do on the filesystem.

However, during tests, we enable full R/W permissions with this function:

def is_sandbox_available():
    is_sandbox_available = True

    try:
        restrict_access(AccessFS.read_write("/"))
    except SandboxError:
        is_sandbox_available = False

    if platform.architecture == "x86_64" and platform.system == "linux":
        assert is_sandbox_available, "Sandboxing should work at least on Linux-x86_64"

    return is_sandbox_available

This is used in:

pytestmark = pytest.mark.skipif(
    not is_sandbox_available(), reason="Sandboxing only works on Linux"
)

This leads to a bunch of permission limitations not being caught during testing such as our inability to delete extraction directories (#1085) or handlers using tempfile lacking permissions to do anything under /tmp. These issues - had the sandbox settings during testing reflects the ones in normal usage - would have been caught by our integration tests suite.

is_sandbox_available should call restrict_access with a stricter ruleset, most probably imported from Sandbox.passthrough.

@qkaiser qkaiser added bug Something isn't working CI/CD Pull request that updates our Github CI/CD python Pull requests that update Python code labels Jan 22, 2025
@qkaiser qkaiser self-assigned this Jan 22, 2025
@qkaiser
Copy link
Contributor Author

qkaiser commented Jan 22, 2025

@martonilles made a good point, we should do sandboxing in test_all_handlers like we do when running unblob normally. Something like:

sandbox = Sandbox(config, log_path, report_file)
process_results = sandbox.run(process_file, config, file, report_file)

One issue we'll get into is coverage files generated by pytest, to be investigated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI/CD Pull request that updates our Github CI/CD python Pull requests that update Python code
Projects
None yet
Development

No branches or pull requests

1 participant