Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Lock memory to avoid key/cleartext compromise #137

Closed
mcorb opened this issue Aug 11, 2017 · 4 comments
Closed

Suggestion: Lock memory to avoid key/cleartext compromise #137

mcorb opened this issue Aug 11, 2017 · 4 comments

Comments

@mcorb
Copy link

mcorb commented Aug 11, 2017

When a long-running gocryptfs process is paged out of memory the plain decrypted key stored to disk. With modern SSD drives and wear levelling the key could remain forensically recoverable for months or years.

This can be mitigated using an mlock facility like memguard (the go equivalent of libsodium), which manages memory slices that are guaranteed not to page out.

it looks like memguard could be dropped into the bPool interface without much work. You'd just need to make sure that the user password and decrypted key are only ever stored in memguard-managed memory.

(For bonus points it might be interesting to use the facility for cleartext content buffers too, ensuring that the gocryptfs process doesn't compromise confidential user data either.)

@rfjakob
Copy link
Owner

rfjakob commented Aug 11, 2017

Good point, and the memguard library looks pretty good! However, most of the times, we don't store the key, the standard library does, like here:

goGcmBlockCipher, err := aes.NewCipher(gcmKey)

In this case there is not much we can do about it.

rfjakob added a commit that referenced this issue Aug 11, 2017
Remove the "Masterkey" field from fusefrontend.Args because it
should not be stored longer than neccessary. Instead pass the
masterkey as a separate argument to the filesystem initializers.

Then overwrite it with zeros immediately so we don't have
to wait for garbage collection.

Note that the crypto implementation still stores at least a
masterkey-derived value, so this change makes it harder, but not
impossible, to extract the encryption keys from memory.

Suggested at #137
@rfjakob
Copy link
Owner

rfjakob commented Aug 11, 2017

But due to your suggestion I have just pushed 0c52084 to clear the masterkey as soon as we can. We kept an unneccessary copy around.

@mcorb
Copy link
Author

mcorb commented Aug 11, 2017

Thanks @rfjakob. This mitigates a little and at least makes it explicit. If the standard library is using the standard allocator I guess it's inevitable bits of key are going to get swapped - pity.

@rfjakob
Copy link
Owner

rfjakob commented Aug 12, 2017

The only solution to this, at the moment, is to either disable swap or use an ecrypted partition.

@rfjakob rfjakob closed this as completed Aug 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants