awnumar / memguard

Secure software enclave for storage of sensitive information in memory.
Apache License 2.0
2.49k stars 124 forks source link

Reason for having `drop` field in `LockedBuffer`? #153

Closed DmitriyMV closed 4 months ago

DmitriyMV commented 7 months ago

Why does it set finalizer in https://github.com/awnumar/memguard/blame/v0.22.3/buffer.go#L31 to drop and not on b which newBuffer will return? It doesn't access b in closure inside SetFinalizer, and LockedBuffer contains only pointers, so if variable of type LockedBuffer goes out of scope, GC should collect it and call the finalizer?

Pinging @CAFxX because of https://github.com/awnumar/memguard/pull/27/files

awnumar commented 6 months ago

It's because we keep a list of Buffers in the core package to be able to free them all at program termination. If we set the finalizer on the whole LockedBuffer then it would never trigger because of the reference in the global list.

CAFxX commented 6 months ago

As an additional argument, if we added it on the *LockedBuffer, and then the user tried to set a new finalizer, it would panic. If instead the user removed the finalizer (e.g. because they want to add a new one without causing a panic) it could lead to the data not being scrubbed. By adding the finalizer to a different object that is only referenced by an unexported field of LockedBuffer, both of these scenarios become virtually impossible without resorting to unsafe/reflect shenanigans. This is also one of the reasons why the standard library does the same thing e.g. for os.File.