beacon_chain::getPersistentNetKeys - harden file permissions for key material

Description

getPersistentNetKeys dumps the node's unprotected privkey in plaintext into the specified --data-dir. When the node is restarted, it attempts to reload the private key from disk. By default, the private key is stored in the --data-dir which on my system (MacOs) resolves to /Users/tintin/Library/Application Support/Nimbus/BeaconNode/privkey.protobuf. The file is written using nim's writeFile() which depending on the system's setting for umask may create world-readable files (e.g. MacOS default: umask 22 -> writeFile() creates a file with 644 (rw-r--r--)).

Under normal circumstances, however, the folder permissions (/User/tintin/Library/...) restrict access to a world-readable file in the subfolder to the currently logged in user.

However, the private key might be exposed on systems where the default is not a directory in userhome or the --data-dir was overridden to be a shared folder. Now, this is arguably insecure on its own but it is nevertheless recommended, to add another layer of protection for the privkey material to prevent it from accidentally being leaked.

8 -rw-r--r--  1 tintin  staff  36 14 Jul 12:33 /Users/tintin/Library/Application Support/Nimbus/BeaconNode/privkey.protobuf

How to reproduce

Manually override the datadir to be in a shared folder:

⇒  ./beacon_node --web3-url=https://mainnet.infura.com --deposit-contract=0x92c506d3dd51a37650cc8e352a7551c26e2c607d --deposit-contract-block=0x8491ec6dfea11adc920bd33ff66a0523de1da24f913bca6f6ef71d5b298c6c9c --data-dir=/tmp

Check file permissions:

⇒  ls -lsat /tmp/
total 8
0 drwxrwxrwt  8 root    wheel  256 14 Jul 15:33 .
0 drwxr-xr-x  3 tintin  wheel   96 14 Jul 15:33 db
8 -rw-r--r--  1 tintin  wheel   36 14 Jul 15:33 privkey.protobuf

Details

https://github.com/status-im/nim-beacon-chain/blob/b0470d7318d05901d4cd6fd2fd635965480817c8/beacon_chain/eth2_network.nim#L1069-L1083

Recommendation

consider enforcing strict folder permission (+u+g-o) for the datadir (if it does not exist yet) (not world-readable)
consider enforcing strict file permissions for privkey material (when creating it) (note: a potential race between writeFile() and setFilepermissions())
consider encrypting cached key material with a user unlock/or application specific key. Scenario: other process running as the user accesses privkey (generally hard to prevent this scenario and may arguably always be bypassed unless some level of interaction is required)
check and warn if file permissions are set insecurely
dblcheck uses of writeFile as uses might not be aware that it may create world-readable files.

status-im / nimbus-eth2