Re: [PATCH v2 1/4] ima: added policy support for 'security.ima' type

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

On Thu, 2013-01-31 at 13:41 -0500, Vivek Goyal wrote:
> On Wed, Jan 30, 2013 at 05:42:39PM -0500, Mimi Zohar wrote:
> > On Wed, 2013-01-30 at 16:53 -0500, Vivek Goyal wrote:
> > > On Tue, Jan 22, 2013 at 05:07:31PM -0500, Mimi Zohar wrote:
> > > 
> > > [..]
> > > >  /* iint cache flags */
> > > > +#define IMA_ACTION_FLAGS	0xff00
> > > >  #define IMA_DIGSIG		0x0100
> > > > +#define IMA_DIGSIG_REQUIRED	0x0200
> > > 
> > > Hi Mimi,
> > > 
> > > IMA_DIGSIG_REQUIRED state does not really have to be stored in iint I guess.
> > > This is needed only if we go on to appraise and then we want to make
> > > sure digital signatures are present. Once appraisal is done, IMG_DIGSIG
> > > will be set and saved in iint->flags but looks like we don't require
> > > IMA_DIGSIG_REQUIRED to be saved.
> > > 
> > > If yes, will it make sense to not save it in iint. There are still 8
> > > bits left unused. We can mark these 8 bits for temporary flags like
> > > IMA_DIGSIG_REQUIRED. If that works, I can use same space for defining
> > > additional temporary flags like IMA_DIGSIG_SIGNED_ONLY to handle the
> > > case of appraise_type=imasig,signed_only. That flag also does not have
> > > to be persistent in iint.
> > 
> > Interesting idea.  My only concern would be that we're not leaving room
> > for additional hooks (eg. firmware).
> 
> Ok. May be we can employ another 32bit variable and put both of them in
> a structure to expand it, when firmware hooks come along.
> 
> Or I can do that right now, if you like it that way.
> 
> I need to move out non-persistent flags from iint flags, otherwise it
> can happen that conflicting flag might be set from different hooks.
> 
> For example, say hypothetically there are too hooks.
> 
> appraise fowner=0 func=mmap appraise_type=imasig
> appraise fowner=0 func=BPRM_CHECK appraise_type=imasig_signed_only
> 
> Now this will lead to setting of both IMA_DISGIG_REQUIRED and
> IMA_DIGSIG_SIGNED_ONLY in iint->flags and that does not mean much in
> the context of iint.
> 
> So by keeping them temporary, these flags become kind of hook specific
> and then two appraise hook don't conflict with each other.

With the following patches, which James pulled earlier this week, each
hook can have a different appraise status.

5a73fcf ima: differentiate appraise status only for hook specific rules
d79d72e ima: per hook cache integrity appraisal status
f578c08 ima: increase iint flag size
0e5a247 ima: added policy support for 'security.ima' type

thanks,

Mimi