linux-kernel - Re: [PATCH 2/2] ima: Support appraise_type=imasig

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 13 Feb 2013 16:45:23 -0500
From:	Mimi Zohar <zohar@...ux.vnet.ibm.com>
To:	"Kasatkin, Dmitry" <dmitry.kasatkin@...el.com>
Cc:	Vivek Goyal <vgoyal@...hat.com>,
	linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] ima: Support appraise_type=imasig_optional

On Wed, 2013-02-13 at 19:33 +0200, Kasatkin, Dmitry wrote:
> On Wed, Feb 13, 2013 at 3:44 PM, Mimi Zohar <zohar@...ux.vnet.ibm.com> wrote:
> > On Wed, 2013-02-13 at 15:13 +0200, Kasatkin, Dmitry wrote:
> >> On Wed, Feb 13, 2013 at 2:56 PM, Mimi Zohar <zohar@...ux.vnet.ibm.com> wrote:
> >> > On Wed, 2013-02-13 at 14:31 +0200, Kasatkin, Dmitry wrote:
> >> >> On Mon, Feb 11, 2013 at 10:11 PM, Vivek Goyal <vgoyal@...hat.com> wrote:
> >> >
> >> >> > @@ -158,7 +165,8 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
> >> >> >         }
> >> >> >         switch (xattr_value->type) {
> >> >> >         case IMA_XATTR_DIGEST:
> >> >> > -               if (iint->flags & IMA_DIGSIG_REQUIRED) {
> >> >> > +               if (iint->flags & IMA_DIGSIG_REQUIRED ||
> >> >> > +                   iint->flags & IMA_DIGSIG_OPTIONAL) {
> >> >> >                         cause = "IMA signature required";
> >> >> >                         status = INTEGRITY_FAIL;
> >> >> >                         break;
> >> >>
> >> >> This looks a bit odd... If "optional" signature is missing  - we fail..
> >> >> It is optional... Why we should fail?
> >> >
> >> > 'imasig_optional' does not only mean that the signature is optional, but
> >> > also implies that it has to be a digital signature, not a hash.  This
> >> > latter part is what IMA_DIGSIG_REQUIRED means.
> >> >
> >> > If 'imasig_optional' set both 'IMA_DIGSIG_OPTIONAL' and
> >> > 'IMA_DIGSIG_REQUIRED', then this change wouldn't be necessary.
> >> >
> >> > IMA_DIGSIG_REQUIRED would enforce that it is a signature.
> >> > IMA_DIGSIG_OPTIONAL would fail only for files with invalid signatures.
> >> >
> >>
> >> It sounds odd that optional signature is actually required.
> >> Optional for me means that it may be there or may be not.
> >> If it is not there, then it may be hash or nothing.
> >>
> >> I see it is more logical if it is "appraise_type=optional",
> >> which means that we might have no xattr value, hash or signature.
> >> It if happens to be a signature, then IMA_DIGSIG flag will be set.
> >
> > Right, 'appraise_type=' could have been defined either as a comma
> > separated list of options (eg. appraise_type=imassig,optional) or, as
> > Vivek implemented, a new option.  Eventually, we will need to extend
> > 'appraise_type=' (eg. required algorithm), but for now I don't have a
> > problem with the new option.
> >
> 
> It is not exactly what I meant. IOW, I do not want
> appraise_type=imasig,optional.
> 
> Optional for me is that xattr value is optional. It might be nothing,
> hash or imasig...

To summarize:

Option 1: appraise_type:= [[imasig] | [imahash]]  [optional]

sample rules:
# require either hash or signature
func=bprm
 
# require signature
func=bprm appraise_type=imasig

# permit nothing, hash, or signature
func=bprm appraise_type=optional

# permit nothing or hash
func=bprm appraise_type=imahash, optional

# permit nothing or signature
func=bprm appraise_type=imasig, optional


Option 2: appraise_type:= [imasig] | [imasig_optional]

As I don't see a use case for either just 'optional' or 'imahash,
optional', this leaves 'imasig,optional', which could be expressed as
'imasig_optional'.


Option 3: appraise_type:= [imasig] | [imahash] | [optional]

Dmitry is recommending this syntax, as IMA_DIGSIG will be set in the
iint flags.


Any of these options should work.

> If it would happen that it contains signature, then IMA_DIGSIG flag
> would be set,
> and process could get needed capability as Vivek wants.

With the 'optional' condition, both unsigned and validly signed files
will succeed.  One way of making this information accessible to an LSM,
would be to define a new integrity capability and set it here.  The new
integrity capability would indicate the file was validly signed. 

thanks,

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/