curl
curl copied to clipboard
curl
lib: Add explicit_bzero to zero out sensitive buffers
Sharing this early as a proof-of-concept to hear others thoughts on this. The CMake and Windows parts are untested so getting them onto the CI is a motivator.
This adds cross platform support for zeroing out buffers containing sensitive content without the zeroing being optimized away by the compiler due being a dead-store. In case the platform doesn't have bzero, explicit_bzero (available on Archlinux etc) or memset_s there is a fallback which use a plain memset via a volatile pointer.
A set of buffers are included here, going through and finding all potential buffers is future work.
This is influenced on how OpenSSH and PostgreSQL zeroes out sensitive buffers. It clearly won't move the security needle all that much but every little bit helps.
I would think the secrets are probably still in memory somewhere. I'd want to hear from some security experts to know whether this is really worth it for us and actually does what we expect.
There exists one more variant: explicit_memset:
check_symbol_exists("explicit_memset" "string.h" HAVE_EXPLICIT_MEMSET)
#define _libssh2_explicit_zero(buf, size) (void)explicit_memset(buf, 0, size)
https://man.netbsd.org/explicit_memset.3
It's checked after explicit_bzero and before memset_s in libssh2.
In libssh2 the wrapper for all the variants is a macro. It has the advantage to inline the native Windows call (and maybe others if they have a similar implementation). Throwing it here as an idea to consider.
https://github.com/libssh2/libssh2/blob/d19b61907039554b8261a90898f2f31e1a706f38/src/misc.h#L42-L59
There exists one more variant:
explicit_memset:
Interesting, we even use it in the code for the NetBSD bug (which is active for NetBSD < 9 but explicit_memset is only available for 7.0+). Added support for it.
In libssh2 the wrapper for all the variants is a macro. It has the advantage to inline the native Windows call (and maybe others if they have a similar implementation). Throwing it here as an idea to consider.
Absolutely, that's a possibility for sure. This is unlikely to be used in performance critical codepaths but might still be worth doing?
One more TODO is to move clearing of values to when they're no longer needed and not just before the free call.
In libssh2 the wrapper for all the variants is a macro. It has the advantage to inline the native Windows call (and maybe others if they have a similar implementation). Throwing it here as an idea to consider.
Absolutely, that's a possibility for sure. This is unlikely to be used in performance critical codepaths but might still be worth doing?
You're right. Let's see if this starts to hit something performance critical and deal with it then.
I would think the secrets are probably still in memory somewhere. I'd want to hear from some security experts to know whether this is really worth it for us and actually does what we expect.
This is pretty standard operating procedure in code dealing with sensitive information (OpenSSL, OpenSSH, libssh2 et.al), defined as CWE-14, and provides basic code hygiene. I agree that it won't push the needle all that far, but provides more defense in depth.
With the window open again, any :+1: or :-1: on this? (it needs a rebase of course)
As this has fallen a bit behind I'm closing. Feel free to reopen if you want to rebase and have another go.
