lkml.org 
[lkml]   [2019]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v6 3/9] powerpc: add support to initialize ima policy rules
    From
    Date
    On Tue, 2019-10-01 at 12:07 -0400, Nayna wrote:
    >
    > On 09/30/2019 09:04 PM, Thiago Jung Bauermann wrote:
    > > Hello,
    >
    > Hi,
    >
    > >
    > >> diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
    > >> new file mode 100644
    > >> index 000000000000..39401b67f19e
    > >> --- /dev/null
    > >> +++ b/arch/powerpc/kernel/ima_arch.c
    > >> @@ -0,0 +1,33 @@
    > >> +// SPDX-License-Identifier: GPL-2.0
    > >> +/*
    > >> + * Copyright (C) 2019 IBM Corporation
    > >> + * Author: Nayna Jain
    > >> + */
    > >> +
    > >> +#include <linux/ima.h>
    > >> +#include <asm/secure_boot.h>
    > >> +
    > >> +bool arch_ima_get_secureboot(void)
    > >> +{
    > >> + return is_powerpc_os_secureboot_enabled();
    > >> +}
    > >> +
    > >> +/* Defines IMA appraise rules for secureboot */
    > >> +static const char *const arch_rules[] = {
    > >> + "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
    > >> +#if !IS_ENABLED(CONFIG_MODULE_SIG)
    > >> + "appraise func=MODULE_CHECK appraise_type=imasig|modsig",
    > >> +#endif
    > >> + NULL
    > >> +};
    > >> +
    > >> +/*
    > >> + * Returns the relevant IMA arch policies based on the system secureboot state.
    > >> + */
    > >> +const char *const *arch_get_ima_policy(void)
    > >> +{
    > >> + if (is_powerpc_os_secureboot_enabled())
    > >> + return arch_rules;
    > >> +
    > >> + return NULL;
    > >> +}
    > > If CONFIG_MODULE_SIG is enabled but module signatures aren't enforced,
    > > then IMA won't enforce module signature either. x86's
    > > arch_get_ima_policy() calls set_module_sig_enforced(). Doesn't the
    > > powerpc version need to do that as well?
    > >
    > > On the flip side, if module signatures are enforced by the module
    > > subsystem then IMA will verify the signature a second time since there's
    > > no sharing of signature verification results between the module
    > > subsystem and IMA (this was observed by Mimi).
    > >
    > > IMHO this is a minor issue, since module loading isn't a hot path and
    > > the duplicate work shouldn't impact anything. But it could be avoided by
    > > having a NULL entry in arch_rules, which arch_get_ima_policy() would
    > > dynamically update with the "appraise func=MODULE_CHECK" rule if
    > > is_module_sig_enforced() is true.
    >
    > Thanks Thiago for reviewing.  I am wondering that this will give two
    > meanings for NULL. Can we do something like below, there are possibly
    > two options ?
    >
    > 1. Set IMA_APPRAISED in the iint->flags if is_module_sig_enforced().
    >
    > OR
    >
    > 2. Let ima_get_action() check for is_module_sig_enforced() when policy
    > is appraise and func is MODULE_CHECK.

    I'm a bit hesitant about mixing the module subsystem signature
    verification method with the IMA measure "template=ima-modsig" rules.
     Does it actually work?

    We can at least limit verifying the same appended signature twice to
    when "module.sig_enforce" is specified on the boot command line, by
    changing "!IS_ENABLED(CONFIG_MODULE_SIG)" to test
    "CONFIG_MODULE_SIG_FORCE".

    Mimi

    \
     
     \ /
      Last update: 2019-10-02 23:49    [W:5.587 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site