Re: [PATCH ima-evm-utils v2] Use secure heap for private keys and passwords

[Bruno Meneguele <bmeneg@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]

On Fri, Aug 20, 2021 at 01:04:45AM +0300, Vitaly Chikunov wrote:
> Bruno,
> 
> On Thu, Aug 19, 2021 at 03:28:17PM -0300, Bruno Meneguele wrote:
> > On Thu, Aug 19, 2021 at 05:11:36AM +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.
> > > 
> > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > ---
> > > Change from v1:
> > > - Do not use setfbuf to disable buffering as this is not proven to be
> > >   meaningful.
> > > - Use secure heap for passwords too as suggested by Mimi Zohar.
> > > - Fallback to OPENSSL_malloc for old OpenSSL as suggested by Mimi Zohar.
> > > - Simplify logic of calling CRYPTO_secure_malloc_init (call it always on
> > >   OpenSSL init.)
> > > - Should be applied after Bruno Meneguele's "evmctl: fix memory leak in
> > >   get_password" patch v2.
> > > 
> > >  src/evmctl.c | 143 ++++++++++++++++++++++++++++++++++++++++++---------
> > >  1 file changed, 118 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/src/evmctl.c b/src/evmctl.c
> > > index 5f7c2b8..a27e0b9 100644
> > > --- a/src/evmctl.c
> > > +++ b/src/evmctl.c
> > > @@ -59,6 +59,7 @@
> > >  #include <assert.h>
> > >  
> > >  #include <openssl/asn1.h>
> > > +#include <openssl/crypto.h>
> > >  #include <openssl/sha.h>
> > >  #include <openssl/pem.h>
> > >  #include <openssl/hmac.h>
> > > @@ -165,6 +166,24 @@ struct tpm_bank_info {
> > >  static char *pcrfile[MAX_PCRFILE];
> > >  static unsigned npcrfile;
> > >  
> > > +#if OPENSSL_VERSION_NUMBER <= 0x10100000
> > > +#warning Your OpenSSL version is too old to have OPENSSL_secure_malloc, \
> > > +	falling back to use plain OPENSSL_malloc.
> > > +#define OPENSSL_secure_malloc	  OPENSSL_malloc
> > > +#define OPENSSL_secure_free	  OPENSSL_free
> > > +/*
> > > + * Secure heap memory automatically cleared on free, but
> > > + * OPENSSL_secure_clear_free will be used in case of fallback
> > 
> > Shouldn't it be OPENSSL_clear_free instead of OPENSLL_secure_clear_free
> > in the setence above?
> 
> No. I meant our code will use OPENSSL_secure_clear_free, so when there
> is no secure heap it will fallback to OPENSSL_clear_free (and not just
> to OPENSSL_free if we used OPENSSL_secure_free).
> 
> Thanks,
> 

Ah ok, I misread the sentence.
Thanks for the clarification.

It seems you'll send a new version of the patch, right?
Will review that as soon as it lands.

Many thanks.

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]