[PATCH] integrity: Allow ima_appraise bootparam to be set when SB is enabled

[PATCH] integrity: Allow ima_appraise bootparam to be set when SB is enabled

[Eric Snowberg]

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>
---
 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);
-- 
2.27.0

Re: [PATCH] integrity: Allow ima_appraise bootparam to be set when SB is enabled

[Mimi Zohar]

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);

Re: [PATCH] integrity: Allow ima_appraise bootparam to be set when SB is enabled

[Eric Snowberg]

> On Apr 26, 2022, at 12:18 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> 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.  

I don’t see code preventing this and just created a config with both of them
enabled.  Is this an assumption everyone is supposed to understand?

> 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.

I will fix this in the next round

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

and add a more thorough explanation.  Thanks.

Re: [PATCH] integrity: Allow ima_appraise bootparam to be set when SB is enabled

[Mimi Zohar]

On Wed, 2022-04-27 at 16:12 +0000, Eric Snowberg wrote:
> 
> > On Apr 26, 2022, at 12:18 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > 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.  
> 
> I don’t see code preventing this and just created a config with both of them
> enabled.  Is this an assumption everyone is supposed to understand?

This was very clear in the original patch upstreamed.  Refer to the
IMA_APPRAISE_BOOTPRAM in commit d958083a8f64 ("x86/ima: define
arch_get_ima_policy() for x86").  This subsequently changed to be based
on  the secureboot runtime state.  Refer to commit 311aa6aafea4 ("ima:
move APPRAISE_BOOTPARAM dependency on ARCH_POLICY to runtime").

> 
> > 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.
> 
> I will fix this in the next round.

Either secureboot is or isn't enabled.  When it is enabled, then IMA
must be in enforcing mode.

> > A fuller, more complete explanation for needing "log" mode when secure
> > boot is enabled is required.
> 
> and add a more thorough explanation.  Thanks.

Normally "log" mode is needed during development.

thanks,

Mimi

Re: [PATCH] integrity: Allow ima_appraise bootparam to be set when SB is enabled

[Nayna]

On 4/25/22 18:21, 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.

Tianocore(UEFI Reference Implementation) defines four secure boot modes, 
one of which is Audit Mode. Refer to last few lines of Feature Summary 
section in Readme.MD 
(https://github.com/tianocore/edk2-staging/blob/Customized-Secure-Boot/Readme.MD#3-feature-summary). 
Based on the reference, IMA appraise_mode="log" should probably work in 
coordination with AuditMode.

Thanks & Regards,

     - Nayna