Insufficient protection of secret information?

[yahesh]

I sifted through the code of gocryptfs to find out how the secrets are handled that the tool needs to work properly. However, unfortunately I didn't find all the anwers I were looking for.

Protection of the masterkey

I have seen that the masterkey is actually nulled when it is not needed anymore as can be seen in mount.go. However, when OpenSSL is used as a backend, a stupidgcm structure is created in cryptocore.go that contains a copy of the masterkey. The creation of the masterkey copy is even described in a comment. But this copy of the masterkey does not seem to be nulled upon termination.

Furthermore, the masterkey is used in the changePassword() function in main.go but it is never nulled.

Protection of derived keys

Deriving cryptographic keys seems to mainly happen in the cryptocore which are then passed back as a structure in cryptocore.go. However, all those keys never seem to be nulled after use.

Protection of the password

Reading of the passwords seams to happen in read.go which are then returned as a string. The password is passed to a call of configfile.LoadConfigFile() in main.go. However, the pw variable never seems to be nulled.

Furthermore, the password is used in the changePassword() function in main.go but it is never nulled.

Program Flow

If I understand the program flow correctly, main.go calls doMount().

doMount() calls loadConfig() in main.go which is the function that actually reads the password.

doMount() also calls initFuseFrontend(). Within that function either fusefrontend.New() in fs.go or fusefrontend_reverse.New() in rfs.go is called (depending on whether the forward or reverse mode is used). These now call cryptocore.New() in cryptocore.go where the actual keys are derived from the masterkey.

At the end, initFuseFrontend() returns a Fuse server. doMount() calls the Fuse server's srv.Serve() method that handles the actual file system actions until the kernel encounters an unmount.

When that happens, doMount() returns 0 and the main() function just quits.

Expectations

I'd expect that variables that hold secret information (password, masterkey, derived keys) are properly nulled when they are not needed anymore - a process which is actually done for one occurence of a secret information (namely the masterkey). In cases where this is difficult to achieve because a data type is used that is difficult to handle properly (e.g. because it is passed by value so that hidden copies are created), an alternative data type like []byte should be used that's easier to handle.

changePassword()

changePassword() in main.go should null the masterkey and the newPw variable

loadConfig()

loadConfig() in main.go should null the pw variable

readpassword.Twice()

readpassword.Twice() in read.go should null the p2 variable

doMount()

doMount() should null the derived keys which are encapsulated in the cryptocore structure which is encapsulated in the ContentEnc structure which in turn is encapsulated in the FS/ReverseFS structure which then again is encapsulated in the PathNodeFs structure (of the go-fuse library) which might be encapsulated in some other structure down the line.

[rfjakob]

Zeroing memory is somewhat unreliable in garbage-collected languages ( see https://cryptolosophy.org/memory-security-go/ ), but I agree that we could overwrite more things than we do now.

About the derived keys, you are aware that the encryption functions must have an internal copy?

[yahesh]

Yes, I'm aware that the derived keys are required during execution. However, they can and should be nulled when srv.Serve() returns. Otherwise those keys may be readable from memory after the process has exited. Concerning the difficulties imposed by the GC: I've seen that the use of memguard (https://github.com/awnumar/memguard) has been proposed in another issue already which is meant to mitigate this problem. Memguard also provides a feature to null the allocated memory.

[yahesh]

I'm currently trying to find out whether the Golang crypto library handles keys securely (which it doesn't in my understanding). However: It seems as if the key that's provided to the crypto library is expanded. Cryptocore calls aes.NewCipher() which calls newCipher() which by default calls newCipherGeneric(). THERE expandKeyGo() is called but only the expanded keys are returned by newCipherGeneric(). See: https://golang.org/src/crypto/aes/cipher.go So it might actually be possible to wipe the derived key after the call to aes.NewCipher(). I haven't checked the other core ciphers yet to verify this further. If this is the case for all core ciphers then gocryptfs could null all derived keys directly after initialization. From there on it would be Golang's turn to implement secure key handling.

[rfjakob]

Yes i think we can overwrite the keys. The library will still hold some key equivalent in memory, but i guess it's still an improvement.

[rfjakob]

Thanks for your detailed report.

I believe I got them all. Keys and password are wiped once they are not longer needed, and the actual encryption keys are wiped on unmount via Wipe():

gocryptfs/internal/cryptocore/cryptocore.go

Line 157 in 5ad9bda

func (c *CryptoCore) Wipe() {

called from

gocryptfs/mount.go

Line 131 in 5ad9bda

defer wipeKeys()

Please reopen if you find I missed something!