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

Fix to allow non-root users access to storage volumes. #3714

Merged
merged 5 commits into from
Jul 28, 2021

Conversation

dereknola
Copy link
Member

@dereknola dereknola commented Jul 26, 2021

Proposed Changes

Non root users can now access local-storage volumes, while making it difficult to view/access all storage volumes. The parent directory /var/lib/rancher/k3s/storage is now 701, allowing non-root users access to sub-directories if they know the exact name of the sub-directory. All created local-storage volumes are reverted back to 777.

Types of Changes

Bugfix

Verification

  • Start k3s server
  • Create local storage by following rancher guide https://rancher.com/docs/k3s/latest/en/storage/
  • Verify that storage directory at /var/lib/rancher/k3s/storage/ has 0701 permissions
  • Verify that the create sub-directory for the local volume has 777 permissions.
  • As a non-root user, cd into the created subdirectory directly.

Linked Issues

#3704

User-Facing Change

NONE

Further Comments

Security thru obscurity: The solution of having the /var/lib/rancher/k3s/storage as 701 will not prevent other users from potentially accessing other storage pods. It just prevents easy access via tools like ls and such. If someone knows the name of the subdirectory, or can guess it, as the subdir is 777 there is nothing stopping them from accessing it.

… allowing non-root users access to subdirectories.

Signed-off-by: dereknola <[email protected]>
@dereknola dereknola requested a review from brandond July 26, 2021 19:29
@dereknola dereknola self-assigned this Jul 26, 2021
@dereknola dereknola requested a review from a team as a code owner July 26, 2021 19:29
@codecov-commenter
Copy link

Codecov Report

Merging #3714 (006350b) into master (21c8a33) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3714   +/-   ##
=======================================
  Coverage   11.59%   11.59%           
=======================================
  Files         135      135           
  Lines        8778     8778           
=======================================
  Hits         1018     1018           
  Misses       7539     7539           
  Partials      221      221           
Flag Coverage Δ
inttests 0.62% <ø> (ø)
unittests 11.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/deploy/zz_generated_bindata.go 0.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21c8a33...006350b. Read the comment docs.

Signed-off-by: dereknola <[email protected]>
Signed-off-by: dereknola <[email protected]>
Signed-off-by: dereknola <[email protected]>
@dereknola dereknola merged commit a1d7a62 into k3s-io:master Jul 28, 2021
dereknola added a commit to dereknola/k3s that referenced this pull request Jul 28, 2021
* Fix to prevent non-root users from accessing storage directory, while allowing non-root users access to subdirectories.

Signed-off-by: dereknola <[email protected]>

* Added integration test

Signed-off-by: dereknola <[email protected]>
dereknola added a commit that referenced this pull request Jul 28, 2021
* Fix to allow non-root users access to storage volumes. (#3714)

* Fix to prevent non-root users from accessing storage directory, while allowing non-root users access to subdirectories.

Signed-off-by: dereknola <[email protected]>
@dereknola dereknola deleted the localstorage_777_fix branch August 9, 2021 18:04
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.

5 participants