Hacker News new | ask | show | jobs
by encryptThrow32 3348 days ago
go.secrets uses the libsodium sodium_memzero to clear the bytes, whereas memguard sets each byte to 0.

I feel more comfortable with the first, but can't exactly explain why. memguard seems better organised in the repo like a ready to go package; I think go.secrets would be a better solution if it was organised as well as memguard.

4 comments

Author of `go.secrets` here (it's defunct, btw). Memguard, at first glance, will not actually do anything. Why? Because go's garbage collector can and will move and copy memory as it sees fit. `mlock` on a go-managed buffer just prevents the original memory location from being swapped out to disk, but does nothing to the copies that the go runtime will periodically create.

This is why I used libsodium (I also implemented this same concept, much more maturely for Rust[1]). If you want this approach to work, you have to manage the memory yourself.

In the Rust version, I also use Rust's ownership rules to automatically `mprotect` with `PROT_NONE` when it's not in use, `PROT_READ` when it's being borrowed immutably, and `PROT_WRITE` when it's being borrowed mutably, all with static compilation guarantees. Plus libsodium creates guard pages before and after the allocation (ensuring no underflows or overflows either into or out of the allocated memory space), and also places a canary before the allocated region that panics when the memory is freed if the canary has been modified. It's far, far more than a simple `mlock`.

I have a rewrite half-in-progress[2] that handles stack-allocated secrets with fewer guarantees (`mlock` and zero-on-free) but that's more appropriate for short-lived stack secrets.

[1]: https://github.com/stouset/secrets

[2]: https://github.com/stouset/secrets/tree/stack-secrets

You could evade the runtime relocation by copying the data into a new buffer allocated by mmap.
You could, but then you still have the original data in the original location which needs to be scrubbed. And go does not, to my knowledge, provide semantics that allow for doing this in a way that should be considered cryptographically reliable.

The go runtime might optimize away your memzero, or it could have created other copies that you don't have a handle to.

In the Rust version of my library (and maybe in the go version, it's been ages since I worked on it), I go out of my way to make it difficult to copy data from runtime-managed memory into a secret buffer. You can do this, and it makes a best-effort attempt at zeroing the data when you do, but you lose a lot of hard guarantees when you do.

Not really, you're suppose to use memguard to create a slice then an address is returned; there is no original data.
The GP specifically mentioned copying data into an `mmap`'d buffer. My point is that copying presupposes secrets already in managed memory, and at that point you've already lost.
Because go's garbage collector can and will move and copy memory as it sees fit.

Is there more to this than the fact that the Go language specification doesn't forbid it? Have you seen it happen?

There's a few things here.

First, the fact that go's language specification allows for this should be enough to stop you right there. Even if today it doesn't move memory around, an update very well could. This library is supposed to be used for cryptographic secrets; "it works by accident for now, probably" is not even close to the kind of situation you should be designing an API around. At any point, without serious warning, an update to the golang runtime can render these protections useless.

There are situations today where go will move data on the stack. I am unsure if it will move heap allocations, but when the garbage collector adds compaction support this will absolutely be the case.

Sure, but that seems a minor difference.

There are more important differences: go.secrets calls panic() if it fails to lock, memguard seems to log a warning with Printf. go.secrets protects against buffer overflows and underflows using a canary, etc.

https://github.com/jedisct1/libsodium/blob/be5e5a53b35961e32...

They either do SecureZeroMemory for windows, memset or bzero for linux and in worst cases they manually wipe the array which is what memguard does, so, how is sodium_memzero any better in this case?

It's not specifically about `sodium_memzero`, it's about managing the memory outside of the scope of go's garbage collector which can and will copy and move memory around as it sees fit.

https://news.ycombinator.com/item?id=14174500

I've been meaning to implement libsodium's sodium_memzero instead of zeroing out. It's next on the Agenda.
Hey there! Thanks for building this. I hate to rain on your parade, but your approach, unfortunately, won't work as currently written[1]. You have to manage the memory yourself.

https://news.ycombinator.com/item?id=14174500

Hey there stouset! I've read your comments and I thank you for the tips you've given. I do realise that there are a lot of things that I have missed, and I plan on having them fixed soon.

I definitely will be managing memory myself. The project is in very early stages at this point, just in v0.1.0 right now.

I'm not trying to pick on you or discourage you, but "just v0.1.0" doesn't excuse that it doesn't work. Whether they should or not, people trust projects like this because they, too, don't have the capability to evaluate whether or not this works. I wouldn't expect potential consumers to understand that Go's garbage collector makes this a nonviable approach any more than this project currently does.

Right now, this is a primed hand grenade of a project. You should disclaim its insecurity at the top of the README. Be very, very specific that it is not currently functional and discourage anyone from using this until it is functional.

And to be clear, saying "don't use it in production" isn't the same thing as saying it doesn't work at all. Your change is irresponsible, because it doesn't work at all.
I don't think it's worth it to add a cgo dependency to the package. Maybe write a set of assembly functions? The compiler can't omit calls to them and there isn't much more you can do anyway than to zero the memory.