Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <d7ba004bd2ce2a8ce2ff0601b4bca921a5301ece.camel@linux.ibm.com>
Subject: Re: [PATCH] integrity: Allow ima_appraise bootparam to be set when
 SB is enabled
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Eric Snowberg <eric.snowberg@oracle.com>
Cc:     dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Tue, 26 Apr 2022 14:18:15 -0400
In-Reply-To: <20220425222120.1998888-1-eric.snowberg@oracle.com>
References: <20220425222120.1998888-1-eric.snowberg@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 2022-04-25 at 18:21 -0400, Eric Snowberg wrote:
> The IMA_APPRAISE_BOOTPARM config allows enabling different "ima_appraise="
> modes (log, fix, enforce) to be configured at boot time.  When booting
> with Secure Boot enabled, all modes are ignored except enforce.  To use
> log or fix, Secure Boot must be disabled.
> 
> With a policy such as:
> 
> appraise func=BPRM_CHECK appraise_type=imasig
> 
> A user may just want to audit signature validation. Not all users
> are interested in full enforcement and find the audit log appropriate
> for their use case.
> 
> Add a new IMA_APPRAISE_SB_BOOTPARAM config allowing "ima_appraise="
> to work when Secure Boot is enabled.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>

Since the IMA architecture specific policy rules were first
upstreamed, either enabling IMA_APPRAISE_BOOTPARAM or IMA_ARCH_POLICY
was permitted, but not both.  This Kconfig negates the assumptions on
which the CONFIG_IMA_ARCH_POLICY and the ima_appraise_signature() are
based without any indication of the ramifications.   This impacts the
kexec file syscall lockdown LSM assumptions as well.

A fuller, more complete explanation for needing "log" mode when secure
boot is enabled is required.

thanks,

Mimi

> ---
>  security/integrity/ima/Kconfig        | 9 +++++++++
>  security/integrity/ima/ima_appraise.c | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index f3a9cc201c8c..66d25345e478 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -237,6 +237,15 @@ config IMA_APPRAISE_BOOTPARAM
>  	  This option enables the different "ima_appraise=" modes
>  	  (eg. fix, log) from the boot command line.
>  
> +config IMA_APPRAISE_SB_BOOTPARAM
> +	bool "ima_appraise secure boot parameter"
> +	depends on IMA_APPRAISE_BOOTPARAM
> +	default n
> +	help
> +	  This option enables the different "ima_appraise=" modes
> +	  (eg. fix, log) from the boot command line when booting
> +	  with Secure Boot enabled.
> +
>  config IMA_APPRAISE_MODSIG
>  	bool "Support module-style signatures for appraisal"
>  	depends on IMA_APPRAISE
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 17232bbfb9f9..a66b1e271806 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -43,7 +43,7 @@ void __init ima_appraise_parse_cmdline(void)
>  
>  	/* If appraisal state was changed, but secure boot is enabled,
>  	 * keep its default */
> -	if (sb_state) {
> +	if (sb_state && !IS_ENABLED(CONFIG_IMA_APPRAISE_SB_BOOTPARAM)) {
>  		if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
>  			pr_info("Secure boot enabled: ignoring ima_appraise=%s option",
>  				str);