subspacecommunity / subspace

A fork of the simple WireGuard VPN server GUI community maintained
MIT License
1.8k stars 131 forks source link

[Fix #127] Data dir permission is too wide #128

Closed ssiuhk closed 3 years ago

ssiuhk commented 4 years ago

to: cc: @subspacecommunity/subspace-maintainers related to: resolves: #127

Background

Data directory default permission is too wide that allows anyone from the system to read the configuration files

Changes

Testing

Using the updated entrypoint.sh

[root@localhost subspace]# rm -rf data [root@localhost subspace]# docker-compose up -d Creating subspace ... done [root@localhost subspace]# ls -alh data total 8.0K drwxr-xr-x. 3 root root 42 Jul 26 02:29 . drwxr-xr-x. 8 root root 156 Jul 26 02:28 .. -rw-r--r--. 1 root root 4.3K Jul 26 02:29 config.json <- It was generated at later stage by the subspace process drwx------. 4 root root 96 Jul 26 02:28 wireguard <- Permission tightened [root@localhost subspace]# docker-compose restart Restarting subspace ... done [root@localhost subspace]# ls -alh data total 8.0K drwxr-xr-x. 3 root root 42 Jul 26 02:29 . drwxr-xr-x. 8 root root 156 Jul 26 02:28 .. -rw-------. 1 root root 4.3K Jul 26 02:29 config.json <- Permission tightened upon 2nd run drwx------. 4 root root 96 Jul 26 02:28 wireguard

sonarcloud[bot] commented 4 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

ssiuhk commented 4 years ago

Yeah, for that probably need to change default umask when starting the subspace application, but not sure about impact of that. Anyway, upper level folder locked up should be good enough

jacksgt commented 4 years ago

Yeah, for that probably need to change default umask when starting the subspace application, but not sure about impact of that. Anyway, upper level folder locked up should be good enough

Thank you for this patch! I really appreciate someone talking a look at the small security details.

I don't think changing the umask will have any negative impact, since it only affects newly created files (i.e. there should be no regressions for people upgrading).