Re: [PATCH ima-evm-utils] Improve memory handling for private keys and passwords

[Vitaly Chikunov <vt@altlinux.org>]

Mimi,

On Wed, Aug 18, 2021 at 05:07:20PM -0400, Mimi Zohar wrote:
> On Fri, 2021-08-13 at 17:31 -0400, Mimi Zohar wrote:
> > On Fri, 2021-08-13 at 00:46 +0300, Vitaly Chikunov wrote:
> > > On Fri, Aug 13, 2021 at 12:21:43AM +0300, Vitaly Chikunov wrote:
> > > > After CRYPTO_secure_malloc_init OpenSSL will store private keys in
> > > > secure heap. This facility is only available since OpenSSL_1_1_0-pre1
> > > > and enabled for 'ima_sign', 'sign', 'sign_hash', and 'hmac'.
> > 
> > From the manpage:
> > CRYPTO_secure_malloc_init() returns 0 on failure, 1 if successful, and
> > 2 if successful but the heap could not be protected by memory mapping.
> > 
> > Not sure what we would do on failure ( 0, 2), but we should at least
> > check the return code.
> > > 
> > > > setvbuf(3) _IONBF is used to hopefully avoid private key and password
> > > > being stored inside of stdio buffers.
> > > 
> > > I should note that usefulness of this method (of avoiding buffering) is
> > > not proven. I don't find other implementations doing it. So, I'm open to
> > > suggestion of removing it.
> > > 
> > Probably would be better to split the patch.
> 
> According to the man page "OPENSSL_secure_malloc() allocates num bytes
> from the heap.  If CRYPTO_secure_malloc_init() is not called, this is
> equivalent to calling OPENSSL_malloc()".   OPENSSL_malloc() is
> supported in the older openssl versions.
> 
> Does it make sense to replace allocating memory for the password via
> malloc() with OPENSSL_secure_malloc()?  For older openssl versions,
> define OPENSSL_secure_malloc() and OPENSSL_secure_free() as
> OPENSSL_malloc() and OPENSSL_free().

It makes sense to use OPENSSL_secure_malloc for passwords, but it will
look strange to define OPENSSL_secure_malloc as OPENSSL_malloc for older
versions.

Maybe we could add a #warning in that case.

Thanks,

> 
> This doesn't solve the memory handling for private keys and passwords
> for older openssl versions, but it is a path forward.
> 
> thanks,
> 
> Mimi