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

[Vitaly Chikunov <vt@altlinux.org>]

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(-)
> 
> @@ -2596,15 +2637,41 @@ static struct option opts[] = {
>  
>  };
>  
> +/*
> + * Copy password from optarg into secure heap, so it could be
> + * freed in the same way as a result of get_password().
> + */
> +static char *optarg_password(char *optarg)
> +{
> +	size_t len;
> +	char *keypass;
> +
> +	if (!optarg)
> +		return NULL;
> +	len = strlen(optarg);
> +	keypass = OPENSSL_secure_malloc(len + 1);
> +	if (keypass)
> +		memcpy(keypass, optarg, len + 1);
> +	else
> +		perror("OPENSSL_secure_malloc");

I also realized that OPENSSL_secure_malloc does not (intentionally)
set errno, so using perror is perhaps wrong. Better method should be
thanked out.

> +	/*
> +	 * This memset does not add real security, just increases
> +	 * the chance of password being obscured in ps output.
> +	 */
> +	memset(optarg, 'X', len);
> +	return keypass;
> +}
> +
> +/* Read from TTY into secure heap. */
>  static char *get_password(void)
>  {
>  	struct termios flags, tmp_flags;
>  	char *password, *pwd;
> -	int passlen = 64;
> +	const int passlen = 64;
>  
> -	password = malloc(passlen);
> +	password = OPENSSL_secure_malloc(passlen);
>  	if (!password) {
> -		perror("malloc");
> +		perror("OPENSSL_secure_malloc");

Thanks,

>  		return NULL;
>  	}
>