TouchID security: encrypted passphrase should be stored in the keychain

[michaelk83]

Expected Behavior

The passphrase should not be kept in regular memory, even if encrypted. It should be saved to the keychain.

1. On TouchID.mm line 112 change from:
   QByteArray dataBytes = (randomKey + randomIV).toHex();
   to:
   QByteArray dataBytes = (randomKey + encryptedMasterKey + randomIV).toHex();

2. Do not store encryptedMasterKey in m_encryptedMasterKeys on line 78.

Actual Behavior

encryptedMasterKey is stored in m_encryptedMasterKeys in regular memory.

Context

See #488 (comment):

Ideally we would use the key stores available in each OS to store the encrypted master key.

Also related to #3337 (comment) and #3467.

KeePassXC - develop branch (2.7.0)
Operating System: macOS

[michaelk83]

Or maybe safer to keep the encryption key and encrypted passphrase in separate entries?

[droidmonkey]

Storing the decryption key with the encrypted data is nonsense, just store the unencrypted data in that case.

[michaelk83]

Yes, I realized that later, hence my previous comment. Not sure if using separate entries is better. Maybe at least flip it to save the encrypted passphrase in the keychain, and the encryption key in memory?

For regular QuickUnlock, the encryption key shouldn't be saved at all, since it can be derived from the user input. With the fingerprint, maybe it can be derived from the fingerprint data? Currently it's just a random key, so it has to be kept somewhere.

[droidmonkey]

That is correct, your biometrics act as the quick unlock password in this case. The current implementation is totally sufficient and correct imo.

[michaelk83]

You may still to need to flip it for the general QuickUnlock, because you'll want to provide a custom derived encryption key, and not save it. In that case, the encrypted passphrase should be saved to the keychain instead of the encryption key, so may as well do that in all cases. But I suppose it doesn't add much at the moment, and it can be done as part of #488.