Re: [PATCH 2/2] ima_evm_utils: limit duplicate "Failed to open keyfile" messages

[Mimi Zohar <zohar@linux.ibm.com>]

On Wed, 2019-07-17 at 00:49 +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Tue, Jul 16, 2019 at 10:30:17AM -0400, Mimi Zohar wrote:
> > Unlike the user provided list of public keys, we don't know which
> > default public key file to use until verify_hash().  As a result, the
> > "Failed to open keyfile" message may be repeated multiple times.
> > 
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> >  src/libimaevm.c | 33 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index 472ab53c7b42..793643331f4b 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -296,18 +296,49 @@ err:
> >  	return err;
> >  }
> >  
> > +/*
> > + * Keep track of missing keyfile names.
> > + *
> > + * Return 1 for found, return 0 for not found.
> > + */
> > +static int lookup_keyfile_name(const char *keyfile_name)
> > +{
> > +	struct keyfile_name_entry {
> > +		struct keyfile_name_entry *next;
> > +		char name[];
> > +	} *entry;
> > +	static struct keyfile_name_entry *keyfile_names = NULL;
> > +
> > +	for (entry = keyfile_names; entry != NULL; entry = entry->next) {
> > +		if (strcmp(entry->name, keyfile_name) == 0)
> > +			return 1;
> > +	}
> > +
> > +	entry = malloc(sizeof(struct keyfile_name_entry) +
> > +			strlen(keyfile_name) + 1);
> > +	if (entry) {
> > +		strcpy(entry->name, keyfile_name);
> > +		entry->next = keyfile_names;
> > +		keyfile_names = entry;
> > +	}
> > +	return 0;
> > +}
> > +
> >  EVP_PKEY *read_pub_pkey(const char *keyfile, int x509)
> >  {
> >  	FILE *fp;
> >  	X509 *crt = NULL;
> >  	EVP_PKEY *pkey = NULL;
> > +	int found;
> >  
> >  	if (!keyfile)
> >  		return NULL;
> >  
> >  	fp = fopen(keyfile, "r");
> >  	if (!fp) {
> > -		log_err("Failed to open keyfile: %s\n", keyfile);
> > +		found = lookup_keyfile_name(keyfile);
> > +		if (!found)
> > +			log_err("Failed to open keyfile: %s\n", keyfile);
> >  		return NULL;
> 
> 
> Now filename list is decoupled from keys themselves. Also we have key
> list creation in init_public_keys(). Maybe we should just always call
> init_public_keys for verify operations?

Initially, I tried that.  The code snippet, below, would be called
once.  The problem is that only during verify_hash() do we know which
default public key file to use.

if (params.keyfile)
    params.keyfile = <default key file>
		
init_public_keys(params.keyfile);

Mimi

> 
> Thanks,
> 
> >  	}
> >  
> > -- 
> > 2.7.5
>