Re: [PATCH v2 3/4] ima: limit secure boot feedback scope for appraise

[Bruno Meneguele <bmeneg@redhat.com>]

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

On Fri, Sep 04, 2020 at 05:07:08PM -0400, Mimi Zohar wrote:
> Hi Bruno,
> 
> > +	bool sb_state = arch_ima_get_secureboot();
> > +	int appraisal_state = ima_appraise;
> >  
> >  	if (strncmp(str, "off", 3) == 0)
> > -		ima_appraise = 0;
> > +		appraisal_state = 0;
> >  	else if (strncmp(str, "log", 3) == 0)
> > -		ima_appraise = IMA_APPRAISE_LOG;
> > +		appraisal_state = IMA_APPRAISE_LOG;
> >  	else if (strncmp(str, "fix", 3) == 0)
> > -		ima_appraise = IMA_APPRAISE_FIX;
> > +		appraisal_state = IMA_APPRAISE_FIX;
> >  	else if (strncmp(str, "enforce", 7) == 0)
> > -		ima_appraise = IMA_APPRAISE_ENFORCE;
> > +		appraisal_state = IMA_APPRAISE_ENFORCE;
> >  	else
> >  		pr_err("invalid \"%s\" appraise option", str);
> > +
> > +	/* If appraisal state was changed, but secure boot is enabled,
> > +	 * keep its default */
> > +	if (sb_state) {
> > +		if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> > +			pr_info("Secure boot enabled: ignoring ima_appraise=%s option",
> > +				str);
> > +		else
> > +			ima_appraise = appraisal_state;
> > +	}
> 
> Shouldn't the "else" clause be here.   No need to re-post the entire
> patch set.

Yes, of course it should.
Sorry. Sending the v3 for this patch.

> 
> thanks,
> 
> Mimi
> 
> >  #endif
> >  	return 1;
> >  }
> 
> 

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

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