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

keep toplevel dir modes and symlinks #138

Merged
merged 19 commits into from
Mar 8, 2024
Merged

Conversation

ezrizhu
Copy link
Collaborator

@ezrizhu ezrizhu commented Jan 10, 2024

In this pull request, we are ensuring the dirs and files in the / root path of the temproot is as close as the 'real' one as possible.

$SANDBOX_DIR/temproot is now being created with the same permission as the host, and every other directories are created with the same mode as the real one.
Symlinks are now also created in the unshare, and removed after unshare finishes.
Tests are created to check the mode, ownership, and symlink of the files in the / directory.

Known issues
In the test, we're ignoring files with the name swap.
And also /proc, our current mount -t proc proc /proc invocation are creating the /proc dir with nobody and nogroup ownership.

This PR currently assumes there are no other files in the root dir besides the swap.img.

Fixes #80
Fixes #139

@ezrizhu ezrizhu self-assigned this Jan 10, 2024
@ezrizhu ezrizhu changed the title keep toplevel dir perms [WIP] keep toplevel dir perms Feb 17, 2024
@ezrizhu ezrizhu marked this pull request as ready for review February 17, 2024 11:25
@ezrizhu ezrizhu changed the title [WIP] keep toplevel dir perms keep toplevel dir perms Feb 20, 2024
@ezrizhu ezrizhu changed the title keep toplevel dir perms keep toplevel dir modes and symlinks Feb 20, 2024
Copy link
Contributor

@mgree mgree left a comment

Choose a reason for hiding this comment

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

Looks good! A few questions/nits.

try Outdated Show resolved Hide resolved
try Outdated Show resolved Hide resolved
test/reuse_problematic_sandbox.sh Show resolved Hide resolved
@ezrizhu ezrizhu requested a review from mgree February 22, 2024 21:06
Copy link
Contributor

@mgree mgree left a comment

Choose a reason for hiding this comment

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

Nice! Just a few questions.

test/reuse_problematic_sandbox.sh Show resolved Hide resolved
try Outdated Show resolved Hide resolved
try Outdated Show resolved Hide resolved
try Show resolved Hide resolved
@ezrizhu ezrizhu requested a review from mgree March 6, 2024 22:56
This was referenced Mar 8, 2024
Copy link
Contributor

@mgree mgree left a comment

Choose a reason for hiding this comment

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

Looks good to me (but maybe fix the typo, no need for re-review).

try Outdated Show resolved Hide resolved
@ezrizhu ezrizhu merged commit 55a7619 into main Mar 8, 2024
17 checks passed
@ezrizhu ezrizhu deleted the ezri/keep-toplevel-perms branch March 8, 2024 20:01
ezrizhu added a commit that referenced this pull request Jun 12, 2024
In this commit, we are ensuring the dirs in the `/` root path of the temproot is as close as the 'real' one as possible. 

`$SANDBOX_DIR/temproot` is now being created with the same permission as the host, and every other directories on the top level are created with the same mode as the real one.
Symlinks are now also created in the unshare, and removed after unshare finishes. 
Tests are created to check the mode, ownership, and symlink of the files in the `/` directory. 

Known issues
In the test, we're ignoring files with the name swap.
And also /proc, our current `mount -t proc proc /proc` invocation are creating the /proc dir with nobody and nogroup ownership.  We're tracking this in #151
This PR currently assumes there are no regular files in the root dir besides the swap.img. We're tracking this in issue #150 

* feat: keep toplevel dir perms in temproot - fixes #80

* feat: recreate symlinks in temproot - fixes #139

* feat: set correct permission for root dir, and remove symlink after unshare

* feat: set temproot to be writible before removing symlinks

* test: add new test to verify consistency of root dir (see known issues)

* test(reuse_problematic_sandbox): set test to use a non-symblink dir

* test(toplevel-perms): ignore acl bit, user&group ownership
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.

top level symlink handled incorrectly sync top level dir permissions
2 participants