Re: [PATCH 0/2] support to read and tune appraise mode in runtime

[Mimi Zohar]

On Tue, 2020-04-14 at 11:36 +0800, Tianjia Zhang wrote:
> 
> On 2020/4/14 5:55, Mimi Zohar wrote:
> > On Thu, 2020-04-09 at 11:39 +0800, Tianjia Zhang wrote:
> >> Support the read and write operations of ima_appraise by adding a
> >> securifyfs file 'appraise_mode'.
> >>
> >> In order to tune appraise mode in runtime, writing a PKCS#7 signature
> >> corresponding the signed content is required. The content should be off,
> >> enforce, log or fix. Given a simple way to archive this:
> >>
> >> $ echo -n off > mode
> >> $ openssl smime -sign -nocerts -noattr -binary \
> >>      -in mode -inkey <system_trusted_key> \
> >>      -signer <cert> -outform der -out mode.p7s
> >> $ sudo cat mode.p7s \
> >>      > /sys/kernel/security/ima/appraise_mode
> >>
> >> Note that the signing key must be a trust key located in
> >> system trusted keyring. So even the root privilege cannot
> >> simply disable the enforcement.
> > 
> > There are major problems with disabling IMA appraisal. ÂThis patch set
> > proposes disabling IMA appraisal without even providing the motivation
> > for such support.
> > 
> > A lot of effort went into preventing custom IMA policies from
> > disabling appraising the kexec or kernel module signatures. ÂIn
> > addition, the "lockdown" patch set was upstreamed permitting IMA
> > signature verification. ÂThis patch set would break both of these
> > features.
> > 
> > IMA relies on its own keyring for verifying file signatures, not the
> > builtin or secondary trusted kernel keyrings.
> > 
> > Two methods already exist - xattr and appended signatures - for
> > verifying file signatures. ÂThis patch set assumes creating and
> > signing a file, which is then written to a securityfs file. ÂLike for
> > loading a custom IMA policy, instead of cat'ing the file, write the
> > pathname to the securityfs file.
> > 
> > If you must define a new IMA method for verifying file signatures,
> > then it needs to be generic and added to ima_appraise_measurement().
> >  Â(Refer to the new IMA appended signature support.)
> > 
> > Mimi
> > 
> >>
> >> Tianjia Zhang (2):
> >>    ima: support to read appraise mode
> >>    ima: support to tune appraise mode in runtime
> >>
> >>   security/integrity/ima/ima_fs.c | 134 +++++++++++++++++++++++++++++++-
> >>   1 file changed, 133 insertions(+), 1 deletion(-)
> >>
> 
> Thanks for your suggestion, the way to close the appraise mode here is 
> indeed a bit rude, I will reconsider again according to your suggestions.
> 
> In addition, [PATCH 1/2] ima: support to read appraise mode, by the way, 
> see if this patch is acceptable.

My comments were not meant as suggestions, but as an explanation as to
how IMA works. ÂMore details follow.

IMA is based on policy. ÂThat decision was made a long time ago. ÂIt
allowed distros to configure IMA, allowing customers to experiment
with it. ÂYou have one opportunity to totally change the boot time
policy rules, by loading a custom policy. ÂAfter that, rules may only
be added.

There is no valid reason for "turning off" the policy once it has been
enabled. ÂIt breaks existing expectations.

Mimi