[openssl-dev] [openssl.org #4116] [PATCH] Reimplement non-asm OPENSSL_cleanse()

[Kurt Roeckx via RT]

On Sat, Oct 31, 2015 at 09:58:50AM -1000, Brian Smith wrote:
> Alessandro Ghedini via RT <rt at openssl.org> wrote:
> 
> > I was also wondering whether it would make sense to just drop the asm
> > implementations. Does the speed-up justify the added complexity?
> >
> 
> IMO, it should work like this:
> * memset_s when memset_s is available.
> * Otherwise, SecureZeroMemory, when SecureZeroMemory is available.
> * Otherwise, if a flag OPENSSL_REQUIRE_SECURE_ZERO is set, fail.

I guess you mean here that if neither of those two is available,
we really can't guarantee anything.

> * Otherwise, use an assembly language implementation, if available.
> * Otherwise, emit a warning and use the C implementation.
> 
> Note in particular that the C compiler is allowed to completely defeat the
> purpose of the function unless SecureZeroMemory or memset_s is used, even
> if you use "volatile" or other tricks. The primary purpose of the assembly
> language implementations is to reduce the possibility that the C compiler
> will do the weird things that C compilers love to do.

It's my understanding that the compiler can still try to outsmart
us and remove things unless it has explicit support for something
like memset_s().  Which is also why explicit_bzero() can be
optimized away.

This is what BoringSSL currently uses:
void OPENSSL_cleanse(void *ptr, size_t len) {
#if defined(OPENSSL_WINDOWS)
        SecureZeroMemory(ptr, len);
#else
        memset(ptr, 0, len);

#if !defined(OPENSSL_NO_ASM)
  /* As best as we can tell, this is sufficient to break any optimisations that
     might try to eliminate "superfluous" memsets. If there's an easy way to
     detect memset_s, it would be better to use that. */
  __asm__ __volatile__("" : : "r"(ptr) : "memory");
#endif
#endif  /* !OPENSSL_NO_ASM */
}

(The /* !OPENSSL_NO_ASM */ is at the wrong line.)

I find the "#if !defined(OPENSSL_NO_ASM)" a little annoying there,
building with no-asm might suddenly optimize it away.

I'm also not sure we can actually use it like that, this is
probably going to cause issues on some platforms.


Kurt