-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Update Client Keys Permissions to 0700 #3733
Conversation
Seems that setting permission on |
Codecov Report
@@ Coverage Diff @@
## develop #3733 +/- ##
===========================================
+ Coverage 61.18% 61.18% +<.01%
===========================================
Files 190 190
Lines 14042 14044 +2
===========================================
+ Hits 8591 8593 +2
Misses 4913 4913
Partials 538 538 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look fine to me. I'm not very familiar with Unix permission structure - will other processes still be unable to read the files if the parent directory is 0700
(even if they know the file path)?
@cwgoes other processes will not be able to read the files I think. However, this shouldn't be a problem because everything should be run under the user process anyway to which |
@cwgoes, @alexanderbez with 0700 only the file owner has any permissions on it. If a directory has 0700, no one but the owner is permitted to read the contents of the directory. So even with a full file path, any other user will just get the "file not found" |
Thanks @mircea-c! |
Perfect, thanks @mircea-c |
The "short-term" approach to #3716.
I initially attempted to set
chmod -R 0700
on the parent Keybase directory and recursively on all its contents after anyNewGoLevelDB
call (see commit: dfd9dee).However, I am able to use the Keybase just fine, but some tests fail on CI because a
ldb
file is missing? Strange. So, I opted to only set0700
on the parent Keybase directory (keys
). This should be good enough. Correct me if I'm wrong @alessio or @mircea-c.Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (
docs/
)Added entries in
PENDING.md
with issue #rereviewed
Files changed
in the github PR explorerFor Admin Use: