Closed amerameen closed 3 years ago
@ec2 Is this related to the keystore.json file? I don't think that's kept in the db. If it is, that should probably change, since it's already on-disk as a separate file.
Oh nice. I thought it was stored in the DB for some reason...
Just had a conversation with @f8-ptrk and he'd really like this feature prioritized. For his use-case, any degree of encryption is helpful, even if it's just AES-GCM-256 with a simple symmetric secret that decrypts the key in-memory while keeping the data encrypted at-rest. That way the keys cannot be exfiltrated from the data center from their miner workers.
Also, I might add one additional point I think the security auditors might bring up: The HTTP JSON-RPC API should have all requests over encrypted (HTTPS) connections, especially when sending a symmetric secret. This should technically be managed by the HTTP gateway on the machine, such as Nginx, or perhaps other network infrastructure, but regardless, I think we should check the X-Forwarded-Proto header to check for HTTPS when an RPC keystore decrypt method is called. This behavior could also be disabled for local testing with a command-line option when running the daemon.
I think this would be a good first issue to throw @connormullett at once he joins, what does everyone think?
After speaking with @f8-ptrk, I added a requirement that the keystore be portable. That'd make it easier to backup and recover, especially if the user loses track of their hostname.
I've moved the following ACs to issue #1083:
Over the weekend I refactored the Encrypted Key Store quite a bit. We're only using sodiumoxide now, instead of mixing cryptosystems. I removed ring from the key_management crate, and we're now using the sodiumoxide Argon2id hash KDF. The salt is then prepended to the keystore when encrypted.
I just tested this out, writing and reading works great. I verified with hexyl that the first 16 bytes were the same (as a salt should be), but the remainder of the file is completely different, which means that secretbox is using a different nonce while flushing bytes to disk, which is more secure. The file also grows when running lotus wallet new
and lotus wallet new bls
, and I can check the balance of an existing wallet with lotus wallet balance [wallet address]
. I'm not able to list, unfortunately.
Remaining items I'll leave to @connormullett to implement, which are the final remaining ACs:
ACs
encrypted_keystore: bool
keystore
file for when using an encrypted keystoreOption<&[u8; 32]>
it can use if thekeystore
file already exists. We'll just prepend the salt to the beginning of the file, and pass a byte slice to derive the same key. This way the keystore is portable between hosts. That's important for backups. The keystore is used for wallet keys, and it'd really suck to lose access to FIL if a user lost track of their hostname.keystore.unlock(passphrase: String)
method that will call thegenerate_key
method, but also manage daemon init blocking, and will be called by the encryption RPC API.Notes