-
Notifications
You must be signed in to change notification settings - Fork 116
Set restrictive file permissions for seed file #741
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
Set restrictive file permissions for seed file #741
Conversation
|
👋 Thanks for assigning @tankyleo as a reviewer! |
src/io/utils.rs
Outdated
| // to protect the sensitive seed material. We use `create_new` to fail if the | ||
| // file already exists, providing defense-in-depth against race conditions. | ||
| #[cfg(unix)] | ||
| let mut f = { OpenOptions::new().write(true).create_new(true).mode(0o600).open(keys_seed_path)? }; |
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.
I'd prefer we set permissions here to 0400 to protect against accidental writes. Once the seed file is created there really shouldn't be a reason to write anything to the file.
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.
Fair:
> git diff-tree -U2 af77366c 7e2f1fa9
diff --git a/src/io/utils.rs b/src/io/utils.rs
index 47b408e2..965831aa 100644
--- a/src/io/utils.rs
+++ b/src/io/utils.rs
@@ -86,5 +86,5 @@ pub(crate) fn read_or_generate_seed_file(
// file already exists, providing defense-in-depth against race conditions.
#[cfg(unix)]
- let mut f = { OpenOptions::new().write(true).create_new(true).mode(0o600).open(keys_seed_path)? };
+ let mut f = { OpenOptions::new().write(true).create_new(true).mode(0o400).open(keys_seed_path)? };
#[cfg(not(unix))]af77366 to
7e2f1fa
Compare
tankyleo
left a comment
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.
LGTM two nits
Previously, seed files were created using `fs::File::create()` which inherits the default umask, potentially making the sensitive seed material world-readable on Unix systems. This change: - Creates seed files with mode 0o400 (owner read only) on Unix - Uses `create_new` instead of `create` to atomically fail if the file already exists, providing defense-in-depth against TOCTOU race conditions Co-Authored-By: Claude AI
7e2f1fa to
e93844a
Compare
Previously, seed files were created using
fs::File::create()which inherits the default umask, potentially making the sensitive seed material world-readable on Unix systems.This change:
create_newinstead ofcreateto atomically fail if the file already exists, providing defense-in-depth against TOCTOU race conditionsCo-Authored-By: Claude AI