Memguard must manage memory allocation to work

[stouset]

See HN article for more detail. TL;DR, golang does not specify any particular semantics about memory addresses. Calling mlock on a go-managed memory address provide little to nothing in the way of guarantees, because that memory will be copied and moved around as the runtime sees fit.

This is pretty much unavoidable. The address you call mlock on isn't even guaranteed to be the address passed in via memlock.Lock(b).

[awnumar]

I've been reading all of the comments, and I do realise that there's a lot of things that I've missed.

Luckily this is version v0.1.0 and I plan to have all of these issues fixed soon.

[rohansingh]

Might be prudent to post a warning in the README so nobody ends up using this for truly sensitive data.

[eropple]

To be clear: it's not that this isn't ready for use in production. It's that this doesn't work. That's a bigger concern here.

[josephlr]

So I've had to deal with a problem very similar to this (for encryption keys in Go). The other commenters are correct, you need to allocate and release the memory yourself. I took the approach of using a call to unix.Mmap to create the buffer and unix.Munmap in your Cleanup/Wipe/Destroy function (along with zeroing the buffer in a for loop). Specifically, the following call to unix.Mmap:

prot := unix.PROT_READ | unix.PROT_WRITE
flags := unix.MAP_PRIVATE | unix.MAP_ANONYMOUS | unix.MAP_LOCKED
buffer, err := unix.Mmap(-1, 0, length, prot, flags)

According to the MAP_LOCKED section of the mmap() man page this does the same thing as mlock/munlock.

I only did this for Linux, but NetBSD has MAP_WIRED. For Darwin, FreeBSD, OpenBSD, or Solaris you might just have to use mmap + mlock then later munlock + munmap. Darwin also has wired memory, but I don't know how to use it from Go.

Windows is sort of weird there is VirtualAlloc and VirtualFree (along with VirtualLock and VirtualUnlock which you are already using). VirtualAlloc and VirtualFree aren't part of x/sys/windows, so that might be a bug.

[awnumar]

@josephlr Yep, on Unix there's also Mprotect which is available on all the kernels since it seems to be available in the sys package. And on Windows there's VirtualProtect too, in addition to VirtualLock.

However, VirtualProtect isn't available in sys/windows either, so we'll see how that's going to be handled.

[awnumar]

Looking more into it, MAP_PRIVATE is COW, which might cause problems. Also, I'd be wary of using the MAP_LOCKED flag since it's ignored on older kernels and isn't as resilient as using mlock(2) itself. Mmap for allocations with it being strictly PROT_NONE unless being written to or read from sounds like a decent start.

[coderanger]

@libeclipse To spell this out more clearly, none of those calls matter. The issue is you can't use any of them with memory allocated by the Go runtime because it owns those buffers and can move them at will without telling you. For this to work you need to fundamentally rework the API such that "protected" data lives in memory allocated some other way, probably directly by libc's malloc. This is further complicated by the fact that you can't ever return data from a safe buffer to an unsafe one or you lose the game. So every API that consumes a locked buffer will have to understand whatever custom data structure you come up with. This feels like a losing proposition to me but if you want to go for it, then 👍.

[josephlr]

@libeclipse It doesn't seem like mprotect or VirtualProtect deals with preventing data from being paged to disk. You don't need to worry about COW with MAP_PRIVATE as the mapping would be anonymous anyway. Also, you really don't want to use PROT_NONE as that makes everything segfault, for example:

buffer, err := unix.Mmap(-1, 0, 10, unix.PROT_NONE, unix.MAP_PRIVATE|unix.MAP_ANONYMOUS)
if err != nil {
	panic(err)
}
// This will SEGSEGV
buffer[0] = 1

As for using MAP_LOCKED for an anonymous mapping, it does the exact same thing as mmap + mlock without the potential race condition (as the MAP_LOCKED flag is older than the oldest Linux kernel that Go supports, so you don't have to worry about things being ignored). But you're right in that doing mmap + mlock would be simpler across all of the unix platforms.

@coderanger It would probably be easier to just use unix.Mmap (or windows equivalent) instead of spinning up cgo just to make a call into malloc.

It seems like if you simplified the API to just have two functions:

func Make(length int) (data []byte, err error) {}
func Destroy(data []byte) (err error) {}

or perhaps using a containing struct to make it harder to mess up:

type LockedBuffer struct {
	Buffer []byte
}
func Make(length int) (*LockedBuffer, error) {}
func (b *LockedBuffer) Destory() (error) {}

you could get something working.

[awnumar]

@josephlr Having something throw a SIGSEGV is not such a bad thing, imo. It means that if anything tries to modify or access the data that is protected, the program will crash. This is similar to the view libsodium takes on things. Then when the user wishes to read/write, he/she can explicitly unlock it and then lock it again. Mprotect can be used to change the permissions.

And good point about MAP_LOCKED. The flag is available in the golang stdlib so I'm not sure if they handle the cases where it's not available on the kernel, by mlocking specifically maybe, but to just be more explicit (and for ease of implementation) doing mmap + mlock separately is probably best.

For Windows, there is this library by one of the Golang developers that implements VirtualAlloc, VirtualFree, and VirtualProtect.