Re: [PATCH] x86/ima: require signed kernel modules

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

[Cc'ing Bruno E. O. Meneguele]

On Fri, 2019-02-08 at 13:21 -0600, Seth Forshee wrote:
> On Tue, Feb 05, 2019 at 01:52:21PM -0500, Mimi Zohar wrote:
> > On Tue, 2019-02-05 at 12:32 -0600, Seth Forshee wrote:
> > > On Tue, Feb 05, 2019 at 11:47:24AM -0500, Mimi Zohar wrote:
> > > > Hi Seth,
> > > > 
> > > > On Tue, 2019-02-05 at 09:18 -0600, Seth Forshee wrote:
> > > > > On Thu, Jan 31, 2019 at 02:18:59PM -0500, Mimi Zohar wrote:
> > > > > > Require signed kernel modules on systems with secure boot mode enabled.
> > > > > > 
> > > > > > To coordinate between appended kernel module signatures and IMA
> > > > > > signatures, only define an IMA MODULE_CHECK policy rule if
> > > > > > CONFIG_MODULE_SIG is not enabled.
> > > > > > 
> > > > > > This patch defines a function named set_module_sig_required() and renames
> > > > > > is_module_sig_enforced() to is_module_sig_enforced_or_required().  The
> > > > > > call to set_module_sig_required() is dependent on CONFIG_IMA_ARCH_POLICY
> > > > > > being enabled.
> > > > > > 
> > > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > > 
> > > > > With respect to interactions with the kernel lockdown patches, this
> > > > > looks better than the patches I saw previously. I don't feel like I know
> > > > > enough about what's going on with IMA to ack the patch, but I feel
> > > > > confident that it's at least not going to break signature enforcement
> > > > > for us.
> > > > 
> > > > Thank you for testing!  Could this be translated into a "tested-by"
> > > > "(for w/lockdown patches)"?
> > > 
> > > Yeah, that's fine. To be clear about what I tested, I've confirmed that
> > > it doesn't interfere with requiring signed modules under lockdown with
> > > CONFIG_IMA_ARCH_POLICY=n and IMA appraisal enabled.
> > > 
> > > Tested-by: Seth Forshee <seth.forshee@canonical.com>
> > 
> > Oh!  You've disabled the coordination of the two signature
> > verification methods.  Any chance you could test with
> > "CONFIG_IMA_ARCH_POLICY" enabled?
> 
> Ok, I've tested this now and it also seems to be working. However it
> does seem redundant to have CONFIG_IMA_ARCH_POLICY alongside lockdown,
> as it doesn't enforce anything not already being enforced by lockdown.

Ok.  Based on Luis' and your comments, I'll defer this discussion to
after IMA appended signature support is upstreamed.  At that point,
for the finit_module syscall, IMA-appraisal at least won't fail the
signature verification.  The same appended signature will be verified
not once, but twice - once on the LSM security_kernel_read_file() hook
and then again by module_sig_check().

For the init_module() syscall, the only coordination needed will be
updating is_module_sig_enforced(), based on either the "lockdown" or
some other flag.

Mimi