Set permissions on generated keys to be user-only on Unix systems #225
Conversation
protochron
requested review from
autodidaddict,
brooksmtownsend,
stevelr and
thomastaylor312
as code owners
protochron
force-pushed
the
unix_permissions_keys
branch
from
21d8bce
to
b870c10
Compare
|
I can leave this as is and add in Windows as a follow-up PR or leave this up until that work is finished. Let me know what you'd prefer! |
There was a problem hiding this comment.
~/.wash ➜ ll
Permissions Size User Date Modified Name
drwxr-xr-x - brooks 29 Nov 2021 contexts/
drwx------ - brooks 6 Jan 09:36 keys/
drwxr-xr-x - brooks 6 Jan 09:29 prekeys/
.rw-r--r-- 634 brooks 5 Jan 13:00 host_config.json
~/.wash ➜ ll prekeys
Permissions Size User Date Modified Name
.rw-r--r-- 58 brooks 29 Nov 2021 brooks_account.nk
.rw-r--r-- 58 brooks 30 Nov 2021 clinicapi_module.nk
...
~/.wash ➜ ll
Permissions Size User Date Modified Name
drwxr-xr-x - brooks 29 Nov 2021 contexts/
drwx------ - brooks 6 Jan 09:36 keys/
drwxr-xr-x - brooks 6 Jan 09:29 prekeys/
.rw-r--r-- 634 brooks 5 Jan 13:00 host_config.json
~/.wash ➜ ll keys
Permissions Size User Date Modified Name
.rw------- 58 brooks 6 Jan 09:36 brooks_account.nk
.rw------- 58 brooks 6 Jan 09:36 echo_module.nk
LGTM! Cool thing is that this will also retroactively fix the permissions of the keys directory too (not existing keys) so this will improve security regardless.
I don't have any reservation with accepting the partial fix and accepting Windows later, especially for something that's non-breaking
|
@protochron looks like you might just need a rebase since we merged another PR |
| fs::create_dir_all(Path::new(&path).parent().unwrap())?; | ||
| let mut f = File::create(path)?; | ||
| let key_path = Path::new(&path).parent().unwrap(); | ||
| fs::create_dir_all(key_path)?; |
There was a problem hiding this comment.
Nit: I feel like this might be a bit cleaner if we create an ensure_key_dir function that makes sure the directory exists with the correct permissions and a write_key function that takes the bytes and does everything to create the file with the right permissions. To be clear, this is only a nit though
There was a problem hiding this comment.
Agreed, but I might tackle that in a follow-up PR, since I may end up reworking that function anyway
protochron
dismissed stale reviews from thomastaylor312 and brooksmtownsend
via
0181eb5
protochron
force-pushed
the
unix_permissions_keys
branch
from
b870c10
to
0181eb5
Compare
|
@brooksmtownsend rebased! I think someone needs to approve running the other github actions. |
protochron
force-pushed
the
unix_permissions_keys
branch
from
0181eb5
to
e10aedb
Compare
Set the permissions on generated keys to `0600` and the directory to `0700` on Unix systems. Windows permissions are a no-op due to having to call the Windows APIs directly which is not very straightforward in Rust. Partial fix for wasmCloud#218. Signed-off-by: Dan Norris <protochron@users.noreply.github.com>
protochron
force-pushed
the
unix_permissions_keys
branch
from
e10aedb
to
0b7853f
Compare
Set the permissions on generated keys to
0600and the directory to0700on Unix systems. Windows permissions are a no-op due to having tocall the Windows APIs directly which is not very straightforward in
Rust.
Partial fix for #218.