fix(security): salt device hash for telemetry#883
Conversation
pszymkowiak
left a comment
There was a problem hiding this comment.
Good security improvement @aeppling — salting the device hash is the right call. The OnceLock cache, getrandom choice, and fallback path are all solid.
P1 — Backward compatibility: all device hashes change
Every existing user gets a new hash after upgrade (salt didn't exist before). The backend telemetry will see all users as "new devices" on the day they upgrade. Is this expected? If RTK Cloud tracks device count trends, there will be a one-time spike. Not a blocker but the backend should be aware.
P1 — Salt file permissions
.device_salt contains a crypto secret. On shared machines, another user could read it and reconstruct the device hash. Set 0600 after creation:
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let _ = std::fs::set_permissions(&salt_path, std::fs::Permissions::from_mode(0o600));
}P2 — Dotfile naming
.device_salt starts with . (hidden) but lives in ~/.local/share/rtk/ where files are normally not hidden. Minor — consider device_salt (no dot) or leave as-is.
P2 — Init telemetry message
The config::telemetry_enabled() check in init.rs is a nice touch but it's a functional change hidden in a security PR. Consider noting it in the PR description.
Overall solid — fix the file permissions and we're good.
|
@pszymkowiak resolved, except for hidden file which is fine |
Add per-user random salt to generate_device_hash() in telemetry.
Salt stored in user machine.