[RFC PATCH 0/2] ima: uncompressed module appraisal support

[RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

When booting with either "ima_policy=secure_boot module.sig_enforce=1"
or building a kernel with CONFIG_IMA_ARCH_POLICY and booting with
"ima_policy=secure_boot", module loading behaves differently based on if
the module is compressed or not.  Originally when appraising a module
with ima it had to be uncompressed and ima signed.  Recent changes in 5.4 
have allowed internally signed modules to load [1].  But this only works 
if the internally signed module is compressed.  The uncompressed module
that is internally signed must still be ima signed. This patch series
tries to bring the two in line.

I'm sending this as an RFC in case this was done intentionally.  Or
maybe there is another way around this problem?  I also realize the 
uncompressed module will be verified again with module_sig_check.  I'm 
open to suggestions on improvement if this is seen as a problem.

[1] https://patchwork.kernel.org/cover/10986023

Eric Snowberg (2):
  ima: Implement support for uncompressed module appended signatures
  ima: Change default secure_boot policy to include appended signatures

 security/integrity/digsig.c           | 9 +++++++--
 security/integrity/ima/ima_appraise.c | 3 +++
 security/integrity/ima/ima_policy.c   | 4 ++--
 security/integrity/integrity.h        | 3 ++-
 4 files changed, 14 insertions(+), 5 deletions(-)

-- 
2.18.1

[RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures

[Eric Snowberg]

Currently IMA can validate compressed modules containing appended
signatures.  This adds the ability to also validate uncompressed
modules when appraise_type=imasig|modsig.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/digsig.c           | 9 +++++++--
 security/integrity/ima/ima_appraise.c | 3 +++
 security/integrity/integrity.h        | 3 ++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index ea1aae3d07b3..5e0c4d04ab9d 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -15,6 +15,7 @@
 #include <linux/key-type.h>
 #include <linux/digsig.h>
 #include <linux/vmalloc.h>
+#include <linux/verification.h>
 #include <crypto/public_key.h>
 #include <keys/system_keyring.h>
 
@@ -31,6 +32,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
 	".ima",
 #endif
 	".platform",
+	".builtin_trusted_keys",
 };
 
 #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
@@ -45,8 +47,11 @@ static struct key *integrity_keyring_from_id(const unsigned int id)
 		return ERR_PTR(-EINVAL);
 
 	if (!keyring[id]) {
-		keyring[id] =
-			request_key(&key_type_keyring, keyring_name[id], NULL);
+		if (id == INTEGRITY_KEYRING_KERNEL)
+			keyring[id] = VERIFY_USE_SECONDARY_KEYRING;
+		else
+			keyring[id] = request_key(&key_type_keyring,
+						  keyring_name[id], NULL);
 		if (IS_ERR(keyring[id])) {
 			int err = PTR_ERR(keyring[id]);
 			pr_err("no %s keyring: %d\n", keyring_name[id], err);
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 300c8d2943c5..4c009c55d620 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -294,6 +294,9 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
 	    func == KEXEC_KERNEL_CHECK)
 		rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM,
 					     modsig);
+	if (rc && func == MODULE_CHECK)
+		rc = integrity_modsig_verify(INTEGRITY_KEYRING_KERNEL, modsig);
+
 	if (rc) {
 		*cause = "invalid-signature";
 		*status = INTEGRITY_FAIL;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 73fc286834d7..63f0e6bff0e0 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -145,7 +145,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
 #define INTEGRITY_KEYRING_EVM		0
 #define INTEGRITY_KEYRING_IMA		1
 #define INTEGRITY_KEYRING_PLATFORM	2
-#define INTEGRITY_KEYRING_MAX		3
+#define INTEGRITY_KEYRING_KERNEL	3
+#define INTEGRITY_KEYRING_MAX		4
 
 extern struct dentry *integrity_dir;
 
-- 
2.18.1

Re: [RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures

[Lakshmi Ramasubramanian]

On 2/6/2020 8:42 AM, Eric Snowberg wrote:

>   
> @@ -31,6 +32,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
>   	".ima",
>   #endif
>   	".platform",
> +	".builtin_trusted_keys",
>   };
>   
>   #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> @@ -45,8 +47,11 @@ static struct key *integrity_keyring_from_id(const unsigned int id)
>   		return ERR_PTR(-EINVAL);
>   
>   	if (!keyring[id]) {
> -		keyring[id] =
> -			request_key(&key_type_keyring, keyring_name[id], NULL);
> +		if (id == INTEGRITY_KEYRING_KERNEL)
> +			keyring[id] = VERIFY_USE_SECONDARY_KEYRING;

Since "Built-In Trusted Keyring" or "Secondary Trusted Keyring" is used, 
would it be more appropriate to name this identifier 
INTEGRITY_KEYRING_BUILTIN_OR_SECONDARY?

> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 73fc286834d7..63f0e6bff0e0 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -145,7 +145,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>   #define INTEGRITY_KEYRING_EVM		0
>   #define INTEGRITY_KEYRING_IMA		1
>   #define INTEGRITY_KEYRING_PLATFORM	2
> -#define INTEGRITY_KEYRING_MAX		3
> +#define INTEGRITY_KEYRING_KERNEL	3
> +#define INTEGRITY_KEYRING_MAX		4


  -lakshmi

Re: [RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures

[Eric Snowberg]

> On Feb 6, 2020, at 10:07 AM, Lakshmi Ramasubramanian <nramas@linux.microsoft.com> wrote:
> 
> On 2/6/2020 8:42 AM, Eric Snowberg wrote:
> 
>>  @@ -31,6 +32,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
>>  	".ima",
>>  #endif
>>  	".platform",
>> +	".builtin_trusted_keys",
>>  };
>>    #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
>> @@ -45,8 +47,11 @@ static struct key *integrity_keyring_from_id(const unsigned int id)
>>  		return ERR_PTR(-EINVAL);
>>    	if (!keyring[id]) {
>> -		keyring[id] =
>> -			request_key(&key_type_keyring, keyring_name[id], NULL);
>> +		if (id == INTEGRITY_KEYRING_KERNEL)
>> +			keyring[id] = VERIFY_USE_SECONDARY_KEYRING;
> 
> Since "Built-In Trusted Keyring" or "Secondary Trusted Keyring" is used, would it be more appropriate to name this identifier INTEGRITY_KEYRING_BUILTIN_OR_SECONDARY?

I’m open to changing INTEGRITY_KEYRING_KERNEL to INTEGRITY_KEYRING_BUILTIN_OR_SECONDARY if that seems more appropriate.

Re: [RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures

[Mimi Zohar]

Hi Eric,

On Thu, 2020-02-06 at 11:42 -0500, Eric Snowberg wrote:
> Currently IMA can validate compressed modules containing appended
> signatures.  This adds the ability to also validate uncompressed
> modules when appraise_type=imasig|modsig.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>

Your patch description in no way matches the code.

Mimi

> ---
>  security/integrity/digsig.c           | 9 +++++++--
>  security/integrity/ima/ima_appraise.c | 3 +++
>  security/integrity/integrity.h        | 3 ++-
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index ea1aae3d07b3..5e0c4d04ab9d 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -15,6 +15,7 @@
>  #include <linux/key-type.h>
>  #include <linux/digsig.h>
>  #include <linux/vmalloc.h>
> +#include <linux/verification.h>
>  #include <crypto/public_key.h>
>  #include <keys/system_keyring.h>
>  
> @@ -31,6 +32,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
>  	".ima",
>  #endif
>  	".platform",
> +	".builtin_trusted_keys",
>  };
>  
>  #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> @@ -45,8 +47,11 @@ static struct key *integrity_keyring_from_id(const unsigned int id)
>  		return ERR_PTR(-EINVAL);
>  
>  	if (!keyring[id]) {
> -		keyring[id] =
> -			request_key(&key_type_keyring, keyring_name[id], NULL);
> +		if (id == INTEGRITY_KEYRING_KERNEL)
> +			keyring[id] = VERIFY_USE_SECONDARY_KEYRING;
> +		else
> +			keyring[id] = request_key(&key_type_keyring,
> +						  keyring_name[id], NULL);
>  		if (IS_ERR(keyring[id])) {
>  			int err = PTR_ERR(keyring[id]);
>  			pr_err("no %s keyring: %d\n", keyring_name[id], err);
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 300c8d2943c5..4c009c55d620 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -294,6 +294,9 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
>  	    func == KEXEC_KERNEL_CHECK)
>  		rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM,
>  					     modsig);
> +	if (rc && func == MODULE_CHECK)
> +		rc = integrity_modsig_verify(INTEGRITY_KEYRING_KERNEL, modsig);
> +
>  	if (rc) {
>  		*cause = "invalid-signature";
>  		*status = INTEGRITY_FAIL;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 73fc286834d7..63f0e6bff0e0 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -145,7 +145,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>  #define INTEGRITY_KEYRING_EVM		0
>  #define INTEGRITY_KEYRING_IMA		1
>  #define INTEGRITY_KEYRING_PLATFORM	2
> -#define INTEGRITY_KEYRING_MAX		3
> +#define INTEGRITY_KEYRING_KERNEL	3
> +#define INTEGRITY_KEYRING_MAX		4
>  
>  extern struct dentry *integrity_dir;
>  

Re: [RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures

[Eric Snowberg]

> On Feb 6, 2020, at 11:05 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Thu, 2020-02-06 at 11:42 -0500, Eric Snowberg wrote:
>> Currently IMA can validate compressed modules containing appended
>> signatures.  This adds the ability to also validate uncompressed
>> modules when appraise_type=imasig|modsig.
>> 
>> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> 
> Your patch description in no way matches the code.
> 

How about if I changed the description to the following:

Currently IMA can only validate compressed modules containing appended
signatures when appraise_type=imasig|modsig.  An uncompressed module that 
is internally signed must still be ima signed.  

Add the ability to validate the uncompress module by validating it against
keys contained within the .builtin_trusted_keys keyring. Now when using a
policy such as:

appraise func=MODULE_CHECK appraise_type=imasig|modsig

It will load modules containing an appended signature when either compressed
or uncompressed.


>> ---
>> security/integrity/digsig.c           | 9 +++++++--
>> security/integrity/ima/ima_appraise.c | 3 +++
>> security/integrity/integrity.h        | 3 ++-
>> 3 files changed, 12 insertions(+), 3 deletions(-)
>> 
>> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
>> index ea1aae3d07b3..5e0c4d04ab9d 100644
>> --- a/security/integrity/digsig.c
>> +++ b/security/integrity/digsig.c
>> @@ -15,6 +15,7 @@
>> #include <linux/key-type.h>
>> #include <linux/digsig.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/verification.h>
>> #include <crypto/public_key.h>
>> #include <keys/system_keyring.h>
>> 
>> @@ -31,6 +32,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
>> 	".ima",
>> #endif
>> 	".platform",
>> +	".builtin_trusted_keys",
>> };
>> 
>> #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
>> @@ -45,8 +47,11 @@ static struct key *integrity_keyring_from_id(const unsigned int id)
>> 		return ERR_PTR(-EINVAL);
>> 
>> 	if (!keyring[id]) {
>> -		keyring[id] =
>> -			request_key(&key_type_keyring, keyring_name[id], NULL);
>> +		if (id == INTEGRITY_KEYRING_KERNEL)
>> +			keyring[id] = VERIFY_USE_SECONDARY_KEYRING;
>> +		else
>> +			keyring[id] = request_key(&key_type_keyring,
>> +						  keyring_name[id], NULL);
>> 		if (IS_ERR(keyring[id])) {
>> 			int err = PTR_ERR(keyring[id]);
>> 			pr_err("no %s keyring: %d\n", keyring_name[id], err);
>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> index 300c8d2943c5..4c009c55d620 100644
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -294,6 +294,9 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
>> 	    func == KEXEC_KERNEL_CHECK)
>> 		rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM,
>> 					     modsig);
>> +	if (rc && func == MODULE_CHECK)
>> +		rc = integrity_modsig_verify(INTEGRITY_KEYRING_KERNEL, modsig);
>> +
>> 	if (rc) {
>> 		*cause = "invalid-signature";
>> 		*status = INTEGRITY_FAIL;
>> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
>> index 73fc286834d7..63f0e6bff0e0 100644
>> --- a/security/integrity/integrity.h
>> +++ b/security/integrity/integrity.h
>> @@ -145,7 +145,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>> #define INTEGRITY_KEYRING_EVM		0
>> #define INTEGRITY_KEYRING_IMA		1
>> #define INTEGRITY_KEYRING_PLATFORM	2
>> -#define INTEGRITY_KEYRING_MAX		3
>> +#define INTEGRITY_KEYRING_KERNEL	3
>> +#define INTEGRITY_KEYRING_MAX		4
>> 
>> extern struct dentry *integrity_dir;
>> 
> 

Re: [RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures

[Mimi Zohar]

On Thu, 2020-02-06 at 12:01 -0700, Eric Snowberg wrote:
> > On Feb 6, 2020, at 11:05 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Thu, 2020-02-06 at 11:42 -0500, Eric Snowberg wrote:
> >> Currently IMA can validate compressed modules containing appended
> >> signatures.  This adds the ability to also validate uncompressed
> >> modules when appraise_type=imasig|modsig.
> >> 
> >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> > 
> > Your patch description in no way matches the code.
> > 
> 
> How about if I changed the description to the following:
> 
> Currently IMA can only validate compressed modules containing appended
> signatures when appraise_type=imasig|modsig.  An uncompressed module that 
> is internally signed must still be ima signed.  
> 
> Add the ability to validate the uncompress module by validating it against
> keys contained within the .builtin_trusted_keys keyring. Now when using a
> policy such as:
> 
> appraise func=MODULE_CHECK appraise_type=imasig|modsig
> 
> It will load modules containing an appended signature when either compressed
> or uncompressed.

We - Nayna and I - will be commenting on the cover letter shortly.  I
think that will help clarify the problem(s).

Mimi

[RFC PATCH 2/2] ima: Change default secure_boot policy to include appended signatures

[Eric Snowberg]

Change the default secure_boot policy from:

appraise func=MODULE_CHECK appraise_type=imasig
appraise func=FIRMWARE_CHECK appraise_type=imasig
appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig
appraise func=POLICY_CHECK appraise_type=imasig

to

appraise func=MODULE_CHECK appraise_type=imasig|modsig
appraise func=FIRMWARE_CHECK appraise_type=imasig
appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
appraise func=POLICY_CHECK appraise_type=imasig

This will allow appended signatures to work with the default
secure_boot policy.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/ima/ima_policy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index ef8dfd47c7e3..5d835715b472 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -189,11 +189,11 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
-	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED},
 	{.action = APPRAISE, .func = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
-	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED},
 	{.action = APPRAISE, .func = POLICY_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 };
-- 
2.18.1

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Nayna]

On 2/6/20 11:42 AM, Eric Snowberg wrote:
> When booting with either "ima_policy=secure_boot module.sig_enforce=1"
> or building a kernel with CONFIG_IMA_ARCH_POLICY and booting with
> "ima_policy=secure_boot", module loading behaves differently based on if
> the module is compressed or not.  Originally when appraising a module
> with ima it had to be uncompressed and ima signed.  Recent changes in 5.4
> have allowed internally signed modules to load [1].  But this only works
> if the internally signed module is compressed.  The uncompressed module
> that is internally signed must still be ima signed. This patch series
> tries to bring the two in line.

We (Mimi and I) have been trying to understand the cover letter. It 
seems "by internally signed" you are referring to modules signed with 
build time generated keys.

Our interpretation of the cover letter is that IMA originally did not 
support appended signatures and now does. Since the modules are signed 
with build time generated keys, the signature verification still fails, 
as the keys are only available on the .builtin keyring and not the .ima 
keyring.

Lastly, there is nothing in these patches that indicate that the kernel 
modules being compressed/uncompressed is related to the signature 
verification.

Thanks & Regards,

      - Nayna

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 6, 2020, at 1:22 PM, Nayna <nayna@linux.vnet.ibm.com> wrote:
> 
> 
> On 2/6/20 11:42 AM, Eric Snowberg wrote:
>> When booting with either "ima_policy=secure_boot module.sig_enforce=1"
>> or building a kernel with CONFIG_IMA_ARCH_POLICY and booting with
>> "ima_policy=secure_boot", module loading behaves differently based on if
>> the module is compressed or not.  Originally when appraising a module
>> with ima it had to be uncompressed and ima signed.  Recent changes in 5.4
>> have allowed internally signed modules to load [1].  But this only works
>> if the internally signed module is compressed.  The uncompressed module
>> that is internally signed must still be ima signed. This patch series
>> tries to bring the two in line.
> 
> We (Mimi and I) have been trying to understand the cover letter. It seems "by internally signed" you are referring to modules signed with build time generated keys.

I am referring to any module that includes an appended signature.  They could be signed at build time or anytime afterwards using /usr/src/kernels/$(uname -r)/scripts/sign-file.  As long as the public key is contained in the builtin kernel trusted keyring.


> Our interpretation of the cover letter is that IMA originally did not support appended signatures and now does.

Correct, before the changes added to 5.4 [1], it was not possible to have a digital signature based appraisal policy that worked with a compressed module.  This is because you can’t ima sign a compressed module, since the signature would be lost by the time it gets to the init_module syscall.  With the changes in [1] you can, if you include “modsig” to your policy, which allows the appended module to be checked instead.


> Since the modules are signed with build time generated keys, the signature verification still fails, as the keys are only available on the .builtin keyring and not the .ima keyring.

Currently the upstream code will fail if the module is uncompressed.  If you compress the same module it will load with the current upstream code.

> Lastly, there is nothing in these patches that indicate that the kernel modules being compressed/uncompressed is related to the signature verification.
> 

Basically if you have the following setup:

Kernel built with CONFIG_IMA_ARCH_POLICY or kernel booted with module.sig_enforce=1 along with the following ima policy:

appraise func=MODULE_CHECK appraise_type=imasig|modsig

If you have a module foo.ko that contains a valid appended signature but is not ima signed, it will fail to load.  Now if the enduser simply compresses the same foo.ko, making it foo.ko.xz.  The module will load.

Modules can be loaded thru two different syscalls, finit_module and init_module.  The changes added in [1] work if you use the init_module syscall.  My change adds support when the finit_module syscall gets used instead.


[1] https://patchwork.kernel.org/cover/10986023

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Mimi Zohar]

On Thu, 2020-02-06 at 14:40 -0700, Eric Snowberg wrote:

<snip>

> Currently the upstream code will fail if the module is uncompressed.
>  If you compress the same module it will load with the current
> upstream code.
> 
> > Lastly, there is nothing in these patches that indicate that the
> kernel modules being compressed/uncompressed is related to the
> signature verification.
> > 
> 
> Basically if you have the following setup:
> 
> Kernel built with CONFIG_IMA_ARCH_POLICY or kernel booted with
> module.sig_enforce=1 along with the following ima policy:
> 
> appraise func=MODULE_CHECK appraise_type=imasig|modsig

Enabling CONFIG_IMA_ARCH_POLICY or module.sig_enforce=1 behave totally
differently.  CONFIG_IMA_ARCH_POLICY coordinates between the IMA
signature verification and the original module_sig_check()
verification.  Either one signature verification method is enabled or
the other, but not both.

The existing IMA x86 arch policy has not been updated to support
appended signatures.

To understand what is happening, we need to analyze each scenario
separately.

- If CONFIG_MODULE_SIG is configured or enabled on the boot command
line ("module.sig_enforce = 1"), then the IMA arch x86 policy WILL NOT
require an IMA signature.

- If CONFIG_MODULE_SIG is NOT configured or enabled on the boot
command line, then the IMA arch x86 policy WILL require an IMA
signature.

- If CONFIG_MODULE_SIG is configured or enabled on the boot command
line, the IMA arch x86 policy is not configured, and the above policy
rule is defined, an appended signature will be verified by both IMA
and module_sig_check().
  
> 
> If you have a module foo.ko that contains a valid appended signature
> but is not ima signed, it will fail to load.

That would only happen in the second scenario or in the last scenario
if the key is not found.

> Now if the end user simply compresses the same foo.ko, making it
> foo.ko.xz.  The module will load.

This implies that CONFIG_MODULE_SIG is configured or enabled on the
boot command line, like the first scenario described above, or in the
last scenario and the key is found.

> 
> Modules can be loaded thru two different syscalls, finit_module and
> init_module.  The changes added in [1] work if you use the
> init_module syscall.  My change adds support when the finit_module
> syscall gets used instead.

With the IMA arch x86 policy, without CONFIG_MODULE_SIG configured or
enabled on the boot command line, IMA will prevent the init_module()
syscall.  This is intentional.

Your second patch (2/2) changes the arch x86 policy rule to allow
appended signatures.  The reason for any other changes needs to be
clearer.  I suggest you look at the audit log and kernel messages, as
well as the kexec selftests, to better understand what is happening.

Mimi

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 7, 2020, at 7:51 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Thu, 2020-02-06 at 14:40 -0700, Eric Snowberg wrote:
> 
> <snip>
> 
>> Currently the upstream code will fail if the module is uncompressed.
>> If you compress the same module it will load with the current
>> upstream code.
>> 
>>> Lastly, there is nothing in these patches that indicate that the
>> kernel modules being compressed/uncompressed is related to the
>> signature verification.
>>>  
>> 
>> Basically if you have the following setup:
>> 
>> Kernel built with CONFIG_IMA_ARCH_POLICY or kernel booted with
>> module.sig_enforce=1 along with the following ima policy:
>> 
>> appraise func=MODULE_CHECK appraise_type=imasig|modsig
> 
> Enabling CONFIG_IMA_ARCH_POLICY or module.sig_enforce=1 behave totally
> differently.  CONFIG_IMA_ARCH_POLICY coordinates between the IMA
> signature verification and the original module_sig_check()
> verification.  Either one signature verification method is enabled or
> the other, but not both.
> 
> The existing IMA x86 arch policy has not been updated to support
> appended signatures.

That is not what I’m seeing.  Appended signatures mostly work.  They just
don’t work thru the finit_module system call.

> To understand what is happening, we need to analyze each scenario
> separately.
> 
> - If CONFIG_MODULE_SIG is configured or enabled on the boot command
> line ("module.sig_enforce = 1"), then the IMA arch x86 policy WILL NOT
> require an IMA signature.

All tests below are without my change
x86 booted with module.sig_enforce=1

empty ima policy
$ cat /sys/kernel/security/ima/policy
$ insmod ./foo.ko.xz   <— loads ok
$ rmmod foo
$ unxz ./foo.ko.xz
$ insmod ./foo.ko      <— loads ok
$ rmmod foo

add in module appraisal 
$ echo "appraise func=MODULE_CHECK appraise_type=imasig|modsig" > /sys/kernel/security/ima/policy

$ insmod ./foo.ko.xz   <— loads ok
$ rmmod foo

$ insmod ./foo.ko
insmod: ERROR: could not insert module ./foo.ko: Permission denied

last entry from audit log:
type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0 auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod" name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365 res=0^]UID="root" AUID=“root"

This is because modsig_verify() will be called from within ima_appraise_measurement(), 
since try_modsig is true.  Then modsig_verify() will return INTEGRITY_FAIL.

If I build with CONFIG_IMA_ARCH_POLICY & CONFIG_MODULE_SIG all tests work the same above,
I just don’t have to add module.sig_enforce=1 when I boot.

Adding my change will allow foo.ko to load above when “|modsig” is added, since it will now evaluate 
the module. Without my change the “imsig|modsig” is true for compressed, but the policy is really 
“imasig&modsig” for uncompressed.


> - If CONFIG_MODULE_SIG is NOT configured or enabled on the boot
> command line, then the IMA arch x86 policy WILL require an IMA
> signature.

Agreed

> - If CONFIG_MODULE_SIG is configured or enabled on the boot command
> line, the IMA arch x86 policy is not configured, and the above policy
> rule is defined, an appended signature will be verified by both IMA
> and module_sig_check().

I think this is the same as what I have done above?


>> If you have a module foo.ko that contains a valid appended signature
>> but is not ima signed, it will fail to load.
> 
> That would only happen in the second scenario or in the last scenario
> if the key is not found.
> 
>> Now if the end user simply compresses the same foo.ko, making it
>> foo.ko.xz.  The module will load.
> 
> This implies that CONFIG_MODULE_SIG is configured or enabled on the
> boot command line, like the first scenario described above, or in the
> last scenario and the key is found.
>> Modules can be loaded thru two different syscalls, finit_module and
>> init_module.  The changes added in [1] work if you use the
>> init_module syscall.  My change adds support when the finit_module
>> syscall gets used instead.
> 
> With the IMA arch x86 policy, without CONFIG_MODULE_SIG configured or
> enabled on the boot command line, IMA will prevent the init_module()
> syscall.  This is intentional.

Agreed

> Your second patch (2/2) changes the arch x86 policy rule to allow
> appended signatures.  The reason for any other changes needs to be
> clearer.  I suggest you look at the audit log and kernel messages, as
> well as the kexec selftests, to better understand what is happening.
> 

I can add more details.  I’m just trying to make it so the end user has the same 
experience when using the default secure_boot ima policy. I don’t see a point in
forcing someone to compress a module to get around security, especially when they
have a policy that contains “|modsig”.  

Let me know how you would like me to move forward.  Are you ok with the actual code in 
my patches, assuming I add a lot more details? Or do you want more analysis here first?  

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Mimi Zohar]

On Fri, 2020-02-07 at 09:57 -0700, Eric Snowberg wrote:
> > On Feb 7, 2020, at 7:51 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Thu, 2020-02-06 at 14:40 -0700, Eric Snowberg wrote:
> > 
> > <snip>
> > 
> >> Currently the upstream code will fail if the module is uncompressed.
> >> If you compress the same module it will load with the current
> >> upstream code.
> >> 
> >>> Lastly, there is nothing in these patches that indicate that the
> >> kernel modules being compressed/uncompressed is related to the
> >> signature verification.
> >>>  
> >> 
> >> Basically if you have the following setup:
> >> 
> >> Kernel built with CONFIG_IMA_ARCH_POLICY or kernel booted with
> >> module.sig_enforce=1 along with the following ima policy:
> >> 
> >> appraise func=MODULE_CHECK appraise_type=imasig|modsig
> > 
> > Enabling CONFIG_IMA_ARCH_POLICY or module.sig_enforce=1 behave totally
> > differently.  CONFIG_IMA_ARCH_POLICY coordinates between the IMA
> > signature verification and the original module_sig_check()
> > verification.  Either one signature verification method is enabled or
> > the other, but not both.
> > 
> > The existing IMA x86 arch policy has not been updated to support
> > appended signatures.
> 
> That is not what I’m seeing.  Appended signatures mostly work.  They just
> don’t work thru the finit_module system call.
> 
> > To understand what is happening, we need to analyze each scenario
> > separately.
> > 
> > - If CONFIG_MODULE_SIG is configured or enabled on the boot command
> > line ("module.sig_enforce = 1"), then the IMA arch x86 policy WILL NOT
> > require an IMA signature.
> 
> All tests below are without my change
> x86 booted with module.sig_enforce=1
> 
> empty ima policy

Sure, in this example the IMA arch x86 policy is not configured and
there is no custom IMA policy - no IMA.

> $ cat /sys/kernel/security/ima/policy

On a real system, you would want to require a signed IMA policy.

> $ insmod ./foo.ko.xz   <— loads ok
> $ rmmod foo
> $ unxz ./foo.ko.xz
> $ insmod ./foo.ko      <— loads ok
> $ rmmod foo
> 
> add in module appraisal 

Sure, the current system 

> $ echo "appraise func=MODULE_CHECK appraise_type=imasig|modsig" >
> /sys/kernel/security/ima/policy
> 
> $ insmod ./foo.ko.xz   <— loads ok
> $ rmmod foo

Sure, CONFIG_MODULE_SIG is configured or enabled on the boot command
line ("module.sig_enforce = 1").  IMA won't prevent the init_module()
syscall.

> 
> $ insmod ./foo.ko
> insmod: ERROR: could not insert module ./foo.ko: Permission denied
> 
> last entry from audit log:
> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
> res=0^]UID="root" AUID=“root"
> 
> This is because modsig_verify() will be called from within
> ima_appraise_measurement(), 
> since try_modsig is true.  Then modsig_verify() will return
> INTEGRITY_FAIL.

Why is it an "invalid signature"?  For that you need to look at the
kernel messages.  Most likely it can't find the public key on the .ima
keyring to verify the signature.

Mimi

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Fri, 2020-02-07 at 09:57 -0700, Eric Snowberg wrote:
>>> On Feb 7, 2020, at 7:51 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Thu, 2020-02-06 at 14:40 -0700, Eric Snowberg wrote:
>>> 
>>> <snip>
>>> 
>>>> Currently the upstream code will fail if the module is uncompressed.
>>>> If you compress the same module it will load with the current
>>>> upstream code.
>>>> 
>>>>> Lastly, there is nothing in these patches that indicate that the
>>>> kernel modules being compressed/uncompressed is related to the
>>>> signature verification.
>>>>> 
>>>> 
>>>> Basically if you have the following setup:
>>>> 
>>>> Kernel built with CONFIG_IMA_ARCH_POLICY or kernel booted with
>>>> module.sig_enforce=1 along with the following ima policy:
>>>> 
>>>> appraise func=MODULE_CHECK appraise_type=imasig|modsig
>>> 
>>> Enabling CONFIG_IMA_ARCH_POLICY or module.sig_enforce=1 behave totally
>>> differently.  CONFIG_IMA_ARCH_POLICY coordinates between the IMA
>>> signature verification and the original module_sig_check()
>>> verification.  Either one signature verification method is enabled or
>>> the other, but not both.
>>> 
>>> The existing IMA x86 arch policy has not been updated to support
>>> appended signatures.
>> 
>> That is not what I’m seeing.  Appended signatures mostly work.  They just
>> don’t work thru the finit_module system call.
>> 
>>> To understand what is happening, we need to analyze each scenario
>>> separately.
>>> 
>>> - If CONFIG_MODULE_SIG is configured or enabled on the boot command
>>> line ("module.sig_enforce = 1"), then the IMA arch x86 policy WILL NOT
>>> require an IMA signature.
>> 
>> All tests below are without my change
>> x86 booted with module.sig_enforce=1
>> 
>> empty ima policy
> 
> Sure, in this example the IMA arch x86 policy is not configured and
> there is no custom IMA policy - no IMA.
> 
>> $ cat /sys/kernel/security/ima/policy
> 
> On a real system, you would want to require a signed IMA policy.
> 
>> $ insmod ./foo.ko.xz   <— loads ok
>> $ rmmod foo
>> $ unxz ./foo.ko.xz
>> $ insmod ./foo.ko      <— loads ok
>> $ rmmod foo
>> 
>> add in module appraisal 
> 
> Sure, the current system 
> 
>> $ echo "appraise func=MODULE_CHECK appraise_type=imasig|modsig" >
>> /sys/kernel/security/ima/policy
>> 
>> $ insmod ./foo.ko.xz   <— loads ok
>> $ rmmod foo
> 
> Sure, CONFIG_MODULE_SIG is configured or enabled on the boot command
> line ("module.sig_enforce = 1").  IMA won't prevent the init_module()
> syscall.
> 
>> 
>> $ insmod ./foo.ko
>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
>> 
>> last entry from audit log:
>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
>> res=0^]UID="root" AUID=“root"
>> 
>> This is because modsig_verify() will be called from within
>> ima_appraise_measurement(), 
>> since try_modsig is true.  Then modsig_verify() will return
>> INTEGRITY_FAIL.
> 
> Why is it an "invalid signature"?  For that you need to look at the
> kernel messages.  Most likely it can't find the public key on the .ima
> keyring to verify the signature.

It is invalid because the module has not been ima signed. 

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Mimi Zohar]

On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
> 
> > On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> >> $ insmod ./foo.ko
> >> insmod: ERROR: could not insert module ./foo.ko: Permission denied
> >> 
> >> last entry from audit log:
> >> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
> >> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
> >> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
> >> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
> >> res=0^]UID="root" AUID=“root"
> >> 
> >> This is because modsig_verify() will be called from within
> >> ima_appraise_measurement(), 
> >> since try_modsig is true.  Then modsig_verify() will return
> >> INTEGRITY_FAIL.
> > 
> > Why is it an "invalid signature"?  For that you need to look at the
> > kernel messages.  Most likely it can't find the public key on the .ima
> > keyring to verify the signature.
> 
> It is invalid because the module has not been ima signed. 

With the IMA policy rule "appraise func=MODULE_CHECK
appraise_type=imasig|modsig", IMA first tries to verify the IMA
signature stored as an xattr and on failure then attempts to verify
the appended signatures.

The audit message above indicates that there was a signature, but the
signature validation failed.

Mimi

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
>> 
>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>>> $ insmod ./foo.ko
>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
>>>> 
>>>> last entry from audit log:
>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
>>>> res=0^]UID="root" AUID=“root"
>>>> 
>>>> This is because modsig_verify() will be called from within
>>>> ima_appraise_measurement(), 
>>>> since try_modsig is true.  Then modsig_verify() will return
>>>> INTEGRITY_FAIL.
>>> 
>>> Why is it an "invalid signature"?  For that you need to look at the
>>> kernel messages.  Most likely it can't find the public key on the .ima
>>> keyring to verify the signature.
>> 
>> It is invalid because the module has not been ima signed. 
> 
> With the IMA policy rule "appraise func=MODULE_CHECK
> appraise_type=imasig|modsig", IMA first tries to verify the IMA
> signature stored as an xattr and on failure then attempts to verify
> the appended signatures.
> 
> The audit message above indicates that there was a signature, but the
> signature validation failed.
> 

I do have  CONFIG_IMA_APPRAISE_MODSIG enabled.  I believe the audit message above 
is coming from modsig_verify in security/integrity/ima/ima_appraise.c.

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Mimi Zohar]

On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
> 
> > On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
> >> 
> >>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>> 
> >>>> $ insmod ./foo.ko
> >>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
> >>>> 
> >>>> last entry from audit log:
> >>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
> >>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
> >>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
> >>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
> >>>> res=0^]UID="root" AUID=“root"
> >>>> 
> >>>> This is because modsig_verify() will be called from within
> >>>> ima_appraise_measurement(), 
> >>>> since try_modsig is true.  Then modsig_verify() will return
> >>>> INTEGRITY_FAIL.
> >>> 
> >>> Why is it an "invalid signature"?  For that you need to look at the
> >>> kernel messages.  Most likely it can't find the public key on the .ima
> >>> keyring to verify the signature.
> >> 
> >> It is invalid because the module has not been ima signed. 
> > 
> > With the IMA policy rule "appraise func=MODULE_CHECK
> > appraise_type=imasig|modsig", IMA first tries to verify the IMA
> > signature stored as an xattr and on failure then attempts to verify
> > the appended signatures.
> > 
> > The audit message above indicates that there was a signature, but the
> > signature validation failed.
> > 
> 
> I do have  CONFIG_IMA_APPRAISE_MODSIG enabled.  I believe the audit message above 
> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.

Right, and it's calling:

	rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);

It's failing because it is trying to find the public key on the .ima
keyring.  Make sure that the public needed to validate the kernel
module is on the IMA keyring (eg. keyctl show %keyring:.ima).

Mimi

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 7, 2020, at 11:54 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
>> 
>>> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
>>>> 
>>>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>> 
>>>>>> $ insmod ./foo.ko
>>>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
>>>>>> 
>>>>>> last entry from audit log:
>>>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
>>>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
>>>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
>>>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
>>>>>> res=0^]UID="root" AUID=“root"
>>>>>> 
>>>>>> This is because modsig_verify() will be called from within
>>>>>> ima_appraise_measurement(), 
>>>>>> since try_modsig is true.  Then modsig_verify() will return
>>>>>> INTEGRITY_FAIL.
>>>>> 
>>>>> Why is it an "invalid signature"?  For that you need to look at the
>>>>> kernel messages.  Most likely it can't find the public key on the .ima
>>>>> keyring to verify the signature.
>>>> 
>>>> It is invalid because the module has not been ima signed. 
>>> 
>>> With the IMA policy rule "appraise func=MODULE_CHECK
>>> appraise_type=imasig|modsig", IMA first tries to verify the IMA
>>> signature stored as an xattr and on failure then attempts to verify
>>> the appended signatures.
>>> 
>>> The audit message above indicates that there was a signature, but the
>>> signature validation failed.
>>> 
>> 
>> I do have  CONFIG_IMA_APPRAISE_MODSIG enabled.  I believe the audit message above 
>> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
> 
> Right, and it's calling:
> 
> 	rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
> 
> It's failing because it is trying to find the public key on the .ima
> keyring.  Make sure that the public needed to validate the kernel
> module is on the IMA keyring (eg. keyctl show %keyring:.ima).
> 

I know that will validate the module properly, but that is not what I’m 
trying to solve here. I thought the point of adding “|modsig” to the
ima policy was to tell ima it can either validate against an ima keyring OR 
default back to the kernel keyring.  This is what happens with the compressed
module.  There isn’t anything in the ima keyring to validate the compressed
modules and it loads when I add “|modsig”.

The use case I’m trying to solve is when someone boots with ima_policy=secure_boot.
If their initramfs contains compressed modules with appended signatures the
system boots.  If they use the same ima policy with an initramfs that contains
uncompressed modules, it doesn't boot.  I thought the point of adding “|modsig”
was to help with the initramfs problem, since it is difficult to ima sign
things within it.

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Mimi Zohar]

On Fri, 2020-02-07 at 14:38 -0700, Eric Snowberg wrote:
> > On Feb 7, 2020, at 11:54 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
> >> 
> >>> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>> 
> >>> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
> >>>> 
> >>>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>>>> 
> >>>>>> $ insmod ./foo.ko
> >>>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
> >>>>>> 
> >>>>>> last entry from audit log:
> >>>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
> >>>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
> >>>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
> >>>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
> >>>>>> res=0^]UID="root" AUID=“root"
> >>>>>> 
> >>>>>> This is because modsig_verify() will be called from within
> >>>>>> ima_appraise_measurement(), 
> >>>>>> since try_modsig is true.  Then modsig_verify() will return
> >>>>>> INTEGRITY_FAIL.
> >>>>> 
> >>>>> Why is it an "invalid signature"?  For that you need to look at the
> >>>>> kernel messages.  Most likely it can't find the public key on the .ima
> >>>>> keyring to verify the signature.
> >>>> 
> >>>> It is invalid because the module has not been ima signed. 
> >>> 
> >>> With the IMA policy rule "appraise func=MODULE_CHECK
> >>> appraise_type=imasig|modsig", IMA first tries to verify the IMA
> >>> signature stored as an xattr and on failure then attempts to verify
> >>> the appended signatures.
> >>> 
> >>> The audit message above indicates that there was a signature, but the
> >>> signature validation failed.
> >>> 
> >> 
> >> I do have  CONFIG_IMA_APPRAISE_MODSIG enabled.  I believe the audit message above 
> >> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
> > 
> > Right, and it's calling:
> > 
> > 	rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
> > 
> > It's failing because it is trying to find the public key on the .ima
> > keyring.  Make sure that the public needed to validate the kernel
> > module is on the IMA keyring (eg. keyctl show %keyring:.ima).
> > 
> 
> I know that will validate the module properly, but that is not what I’m 
> trying to solve here. I thought the point of adding “|modsig” to the
> ima policy was to tell ima it can either validate against an ima keyring OR 
> default back to the kernel keyring.  This is what happens with the compressed
> module.  There isn’t anything in the ima keyring to validate the compressed
> modules and it loads when I add “|modsig”.

"modsig" has nothing to do with keyrings.  The term "modsig" is
juxtaposed to "imasig".  "modsig" refers to kernel module appended
signature. 

> 
> The use case I’m trying to solve is when someone boots with ima_policy=secure_boot.

As the secure_boot policy rules are replaced once a custom policy is
loaded, the "secure_boot" policy should probably be deprecated.  I
highly recommend using the more recent build time and architecture
specific run time policy rules, which persist after loading a custom
policy. 

> If their initramfs contains compressed modules with appended signatures the
> system boots.  If they use the same ima policy with an initramfs that contains
> uncompressed modules, it doesn't boot.  I thought the point of adding “|modsig”
> was to help with the initramfs problem, since it is difficult to ima sign
> things within it.

There have been a number of attempts to address the CPIO problem of
not being able to include security extended attributes in the
initramfs.  If you're interested in solving that problem, then review
and comment on Roberto Sassu's patches[1].

Mimi

[1] https://lkml.org/lkml/2019/5/23/415

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 8, 2020, at 4:43 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Fri, 2020-02-07 at 14:38 -0700, Eric Snowberg wrote:
>>> On Feb 7, 2020, at 11:54 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
>>>> 
>>>>> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>> 
>>>>> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
>>>>>> 
>>>>>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>>>> 
>>>>>>>> $ insmod ./foo.ko
>>>>>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
>>>>>>>> 
>>>>>>>> last entry from audit log:
>>>>>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
>>>>>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
>>>>>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
>>>>>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
>>>>>>>> res=0^]UID="root" AUID=“root"
>>>>>>>> 
>>>>>>>> This is because modsig_verify() will be called from within
>>>>>>>> ima_appraise_measurement(), 
>>>>>>>> since try_modsig is true.  Then modsig_verify() will return
>>>>>>>> INTEGRITY_FAIL.
>>>>>>> 
>>>>>>> Why is it an "invalid signature"?  For that you need to look at the
>>>>>>> kernel messages.  Most likely it can't find the public key on the .ima
>>>>>>> keyring to verify the signature.
>>>>>> 
>>>>>> It is invalid because the module has not been ima signed. 
>>>>> 
>>>>> With the IMA policy rule "appraise func=MODULE_CHECK
>>>>> appraise_type=imasig|modsig", IMA first tries to verify the IMA
>>>>> signature stored as an xattr and on failure then attempts to verify
>>>>> the appended signatures.
>>>>> 
>>>>> The audit message above indicates that there was a signature, but the
>>>>> signature validation failed.
>>>>> 
>>>> 
>>>> I do have  CONFIG_IMA_APPRAISE_MODSIG enabled.  I believe the audit message above 
>>>> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
>>> 
>>> Right, and it's calling:
>>> 
>>> 	rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
>>> 
>>> It's failing because it is trying to find the public key on the .ima
>>> keyring.  Make sure that the public needed to validate the kernel
>>> module is on the IMA keyring (eg. keyctl show %keyring:.ima).
>>> 
>> 
>> I know that will validate the module properly, but that is not what I’m 
>> trying to solve here. I thought the point of adding “|modsig” to the
>> ima policy was to tell ima it can either validate against an ima keyring OR 
>> default back to the kernel keyring.  This is what happens with the compressed
>> module.  There isn’t anything in the ima keyring to validate the compressed
>> modules and it loads when I add “|modsig”.
> 
> "modsig" has nothing to do with keyrings.  The term "modsig" is
> juxtaposed to "imasig".  "modsig" refers to kernel module appended
> signature. 

Ok, understood, “modsig” refers to strictly kernel module appended signatures
without regard to the keyring that verifies it.  Since there are inconsistencies
here, would you consider something like my first patch?  It will verify an 
uncompressed kernel module containing an appended signature  when the public key
is contained within the kernel keyring instead of the ima keyring.  Why force a 
person to add the same keys into the ima keyring for validation?  Especially when
the kernel keyring is now used to verify appended signatures in the compressed
modules.

> 
>> 
>> The use case I’m trying to solve is when someone boots with ima_policy=secure_boot.
> 
> As the secure_boot policy rules are replaced once a custom policy is
> loaded, the "secure_boot" policy should probably be deprecated.  I
> highly recommend using the more recent build time and architecture
> specific run time policy rules, which persist after loading a custom
> policy. 

I found the secure_boot policy useful, until a custom policy got loaded.  But if it
is targeted to be deprecated, I’ll drop my second patch.  I will look at the run
time policy rules instead. Thanks.

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Mimi Zohar]

On Mon, 2020-02-10 at 09:34 -0700, Eric Snowberg wrote:
> > On Feb 8, 2020, at 4:43 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Fri, 2020-02-07 at 14:38 -0700, Eric Snowberg wrote:
> >>> On Feb 7, 2020, at 11:54 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>> 
> >>> On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
> >>>> 
> >>>>> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>>>> 
> >>>>> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
> >>>>>> 
> >>>>>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>>>>>> 
> >>>>>>>> $ insmod ./foo.ko
> >>>>>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
> >>>>>>>> 
> >>>>>>>> last entry from audit log:
> >>>>>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
> >>>>>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
> >>>>>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
> >>>>>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
> >>>>>>>> res=0^]UID="root" AUID=“root"
> >>>>>>>> 
> >>>>>>>> This is because modsig_verify() will be called from within
> >>>>>>>> ima_appraise_measurement(), 
> >>>>>>>> since try_modsig is true.  Then modsig_verify() will return
> >>>>>>>> INTEGRITY_FAIL.
> >>>>>>> 
> >>>>>>> Why is it an "invalid signature"?  For that you need to look at the
> >>>>>>> kernel messages.  Most likely it can't find the public key on the .ima
> >>>>>>> keyring to verify the signature.
> >>>>>> 
> >>>>>> It is invalid because the module has not been ima signed. 
> >>>>> 
> >>>>> With the IMA policy rule "appraise func=MODULE_CHECK
> >>>>> appraise_type=imasig|modsig", IMA first tries to verify the IMA
> >>>>> signature stored as an xattr and on failure then attempts to verify
> >>>>> the appended signatures.
> >>>>> 
> >>>>> The audit message above indicates that there was a signature, but the
> >>>>> signature validation failed.
> >>>>> 
> >>>> 
> >>>> I do have  CONFIG_IMA_APPRAISE_MODSIG enabled.  I believe the audit message above 
> >>>> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
> >>> 
> >>> Right, and it's calling:
> >>> 
> >>> 	rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
> >>> 
> >>> It's failing because it is trying to find the public key on the .ima
> >>> keyring.  Make sure that the public needed to validate the kernel
> >>> module is on the IMA keyring (eg. keyctl show %keyring:.ima).
> >>> 
> >> 
> >> I know that will validate the module properly, but that is not what I’m 
> >> trying to solve here. I thought the point of adding “|modsig” to the
> >> ima policy was to tell ima it can either validate against an ima keyring OR 
> >> default back to the kernel keyring.  This is what happens with the compressed
> >> module.  There isn’t anything in the ima keyring to validate the compressed
> >> modules and it loads when I add “|modsig”.
> > 
> > "modsig" has nothing to do with keyrings.  The term "modsig" is
> > juxtaposed to "imasig".  "modsig" refers to kernel module appended
> > signature. 
> 
> Ok, understood, “modsig” refers to strictly kernel module appended signatures
> without regard to the keyring that verifies it.  Since there are inconsistencies
> here, would you consider something like my first patch?  It will verify an 
> uncompressed kernel module containing an appended signature  when the public key
> is contained within the kernel keyring instead of the ima keyring.  Why force a 
> person to add the same keys into the ima keyring for validation?  Especially when
> the kernel keyring is now used to verify appended signatures in the compressed
> modules.

Different use case scenarios have different requirements.  Suppose for
example that the group creating the kernel image is not the same as
using it.  The group using the kernel image could sign all files,
including kernel modules (imasig), with their own private key. Only
files that they signed would be permitted.  Your proposal would break
the current expectations, allowing kernel modules signed by someone
else to be loaded.

Mimi

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 10, 2020, at 10:09 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Mon, 2020-02-10 at 09:34 -0700, Eric Snowberg wrote:
>>> On Feb 8, 2020, at 4:43 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Fri, 2020-02-07 at 14:38 -0700, Eric Snowberg wrote:
>>>>> On Feb 7, 2020, at 11:54 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>> 
>>>>> On Fri, 2020-02-07 at 11:45 -0700, Eric Snowberg wrote:
>>>>>> 
>>>>>>> On Feb 7, 2020, at 11:28 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>>>> 
>>>>>>> On Fri, 2020-02-07 at 10:49 -0700, Eric Snowberg wrote:
>>>>>>>> 
>>>>>>>>> On Feb 7, 2020, at 10:40 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>>>>>> 
>>>>>>>>>> $ insmod ./foo.ko
>>>>>>>>>> insmod: ERROR: could not insert module ./foo.ko: Permission denied
>>>>>>>>>> 
>>>>>>>>>> last entry from audit log:
>>>>>>>>>> type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0
>>>>>>>>>> auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-
>>>>>>>>>> s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod"
>>>>>>>>>> name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365
>>>>>>>>>> res=0^]UID="root" AUID=“root"
>>>>>>>>>> 
>>>>>>>>>> This is because modsig_verify() will be called from within
>>>>>>>>>> ima_appraise_measurement(), 
>>>>>>>>>> since try_modsig is true.  Then modsig_verify() will return
>>>>>>>>>> INTEGRITY_FAIL.
>>>>>>>>> 
>>>>>>>>> Why is it an "invalid signature"?  For that you need to look at the
>>>>>>>>> kernel messages.  Most likely it can't find the public key on the .ima
>>>>>>>>> keyring to verify the signature.
>>>>>>>> 
>>>>>>>> It is invalid because the module has not been ima signed. 
>>>>>>> 
>>>>>>> With the IMA policy rule "appraise func=MODULE_CHECK
>>>>>>> appraise_type=imasig|modsig", IMA first tries to verify the IMA
>>>>>>> signature stored as an xattr and on failure then attempts to verify
>>>>>>> the appended signatures.
>>>>>>> 
>>>>>>> The audit message above indicates that there was a signature, but the
>>>>>>> signature validation failed.
>>>>>>> 
>>>>>> 
>>>>>> I do have  CONFIG_IMA_APPRAISE_MODSIG enabled.  I believe the audit message above 
>>>>>> is coming from modsig_verify in security/integrity/ima/ima_appraise.c.
>>>>> 
>>>>> Right, and it's calling:
>>>>> 
>>>>> 	rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
>>>>> 
>>>>> It's failing because it is trying to find the public key on the .ima
>>>>> keyring.  Make sure that the public needed to validate the kernel
>>>>> module is on the IMA keyring (eg. keyctl show %keyring:.ima).
>>>>> 
>>>> 
>>>> I know that will validate the module properly, but that is not what I’m 
>>>> trying to solve here. I thought the point of adding “|modsig” to the
>>>> ima policy was to tell ima it can either validate against an ima keyring OR 
>>>> default back to the kernel keyring.  This is what happens with the compressed
>>>> module.  There isn’t anything in the ima keyring to validate the compressed
>>>> modules and it loads when I add “|modsig”.
>>> 
>>> "modsig" has nothing to do with keyrings.  The term "modsig" is
>>> juxtaposed to "imasig".  "modsig" refers to kernel module appended
>>> signature. 
>> 
>> Ok, understood, “modsig” refers to strictly kernel module appended signatures
>> without regard to the keyring that verifies it.  Since there are inconsistencies
>> here, would you consider something like my first patch?  It will verify an 
>> uncompressed kernel module containing an appended signature  when the public key
>> is contained within the kernel keyring instead of the ima keyring.  Why force a 
>> person to add the same keys into the ima keyring for validation?  Especially when
>> the kernel keyring is now used to verify appended signatures in the compressed
>> modules.
> 
> Different use case scenarios have different requirements.  Suppose for
> example that the group creating the kernel image is not the same as
> using it.  The group using the kernel image could sign all files,
> including kernel modules (imasig), with their own private key. Only
> files that they signed would be permitted.  Your proposal would break
> the current expectations, allowing kernel modules signed by someone
> else to be loaded.
> 

All the end user needs to do is compress any module created by the group that built
the original kernel image to work around the scenario above.  Then the appended 
signature in the compressed module will be verified by the kernel keyring. Does 
this mean there is a security problem that should be fixed, if this is a concern?

For the use case above, wouldn’t it be better to use a module policy like:

appraise func=MODULE_CHECK appraise_type=imasig

Obviously the downside is it disables appended signatures. It would prevent 
compressed modules from loading, and only allow ima signed modules to load.

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Mimi Zohar]

On Mon, 2020-02-10 at 12:24 -0700, Eric Snowberg wrote:
> > On Feb 10, 2020, at 10:09 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:

> >> 
> >> Ok, understood, “modsig” refers to strictly kernel module appended signatures
> >> without regard to the keyring that verifies it.  Since there are inconsistencies
> >> here, would you consider something like my first patch?  It will verify an 
> >> uncompressed kernel module containing an appended signature  when the public key
> >> is contained within the kernel keyring instead of the ima keyring.  Why force a 
> >> person to add the same keys into the ima keyring for validation?  Especially when
> >> the kernel keyring is now used to verify appended signatures in the compressed
> >> modules.
> > 
> > Different use case scenarios have different requirements.  Suppose for
> > example that the group creating the kernel image is not the same as
> > using it.  The group using the kernel image could sign all files,
> > including kernel modules (imasig), with their own private key. Only
> > files that they signed would be permitted.  Your proposal would break
> > the current expectations, allowing kernel modules signed by someone
> > else to be loaded.
> > 
> 
> All the end user needs to do is compress any module created by the group that built
> the original kernel image to work around the scenario above.  Then the appended 
> signature in the compressed module will be verified by the kernel keyring. Does 
> this mean there is a security problem that should be fixed, if this is a concern?

Again, the issue isn't compressed/uncompressed kernel modules, but the
syscall used to load the kernel module.  IMA can prevent using the the
init_module syscall.  Refer to the ima_load_data() LOADING_MODULE
case.

Mimi

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 10, 2020, at 1:33 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Mon, 2020-02-10 at 12:24 -0700, Eric Snowberg wrote:
>>> On Feb 10, 2020, at 10:09 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
>>>> 
>>>> Ok, understood, “modsig” refers to strictly kernel module appended signatures
>>>> without regard to the keyring that verifies it.  Since there are inconsistencies
>>>> here, would you consider something like my first patch?  It will verify an 
>>>> uncompressed kernel module containing an appended signature  when the public key
>>>> is contained within the kernel keyring instead of the ima keyring.  Why force a 
>>>> person to add the same keys into the ima keyring for validation?  Especially when
>>>> the kernel keyring is now used to verify appended signatures in the compressed
>>>> modules.
>>> 
>>> Different use case scenarios have different requirements.  Suppose for
>>> example that the group creating the kernel image is not the same as
>>> using it.  The group using the kernel image could sign all files,
>>> including kernel modules (imasig), with their own private key. Only
>>> files that they signed would be permitted.  Your proposal would break
>>> the current expectations, allowing kernel modules signed by someone
>>> else to be loaded.
>>> 
>> 
>> All the end user needs to do is compress any module created by the group that built
>> the original kernel image to work around the scenario above.  Then the appended 
>> signature in the compressed module will be verified by the kernel keyring. Does 
>> this mean there is a security problem that should be fixed, if this is a concern?
> 
> Again, the issue isn't compressed/uncompressed kernel modules, but the
> syscall used to load the kernel module.  IMA can prevent using the the
> init_module syscall.  Refer to the ima_load_data() LOADING_MODULE
> case.

Within the ima_load_data() LOADING_MODULE case, to prevent IMA from using
the init_module syscall, is_module_sig_enforced() must return false. Currently
when is_module_sig_enforced() returns true, the kernel keyring is always used
for verification.

What if I change this part of my patch from

+       if (rc && func == MODULE_CHECK)

to

+       sig_enforce = is_module_sig_enforced();
+       if (sig_enforce && rc && func == MODULE_CHECK)

Now when the init_module syscall is available, finit_module syscall will use
both the ima keyring and kernel keyring for verification.  When the
init_module syscall is blocked from use, the finit_module syscall will only use
the ima keyring for validation.  I believe this would satisfy both your use
case and mine.

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Nayna]

On 2/11/20 12:33 PM, Eric Snowberg wrote:
>> On Feb 10, 2020, at 1:33 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>
>> On Mon, 2020-02-10 at 12:24 -0700, Eric Snowberg wrote:
>>>> On Feb 10, 2020, at 10:09 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>> Ok, understood, “modsig” refers to strictly kernel module appended signatures
>>>>> without regard to the keyring that verifies it.  Since there are inconsistencies
>>>>> here, would you consider something like my first patch?  It will verify an
>>>>> uncompressed kernel module containing an appended signature  when the public key
>>>>> is contained within the kernel keyring instead of the ima keyring.  Why force a
>>>>> person to add the same keys into the ima keyring for validation?  Especially when
>>>>> the kernel keyring is now used to verify appended signatures in the compressed
>>>>> modules.
>>>> Different use case scenarios have different requirements.  Suppose for
>>>> example that the group creating the kernel image is not the same as
>>>> using it.  The group using the kernel image could sign all files,
>>>> including kernel modules (imasig), with their own private key. Only
>>>> files that they signed would be permitted.  Your proposal would break
>>>> the current expectations, allowing kernel modules signed by someone
>>>> else to be loaded.
>>>>
>>> All the end user needs to do is compress any module created by the group that built
>>> the original kernel image to work around the scenario above.  Then the appended
>>> signature in the compressed module will be verified by the kernel keyring. Does
>>> this mean there is a security problem that should be fixed, if this is a concern?
>> Again, the issue isn't compressed/uncompressed kernel modules, but the
>> syscall used to load the kernel module.  IMA can prevent using the the
>> init_module syscall.  Refer to the ima_load_data() LOADING_MODULE
>> case.
> Within the ima_load_data() LOADING_MODULE case, to prevent IMA from using
> the init_module syscall, is_module_sig_enforced() must return false. Currently
> when is_module_sig_enforced() returns true, the kernel keyring is always used
> for verification.
>
> What if I change this part of my patch from
>
> +       if (rc && func == MODULE_CHECK)
>
> to
>
> +       sig_enforce = is_module_sig_enforced();
> +       if (sig_enforce && rc && func == MODULE_CHECK)
>
> Now when the init_module syscall is available, finit_module syscall will use
> both the ima keyring and kernel keyring for verification.  When the
> init_module syscall is blocked from use, the finit_module syscall will only use
> the ima keyring for validation.  I believe this would satisfy both your use
> case and mine.
>
There are two syscalls - init_module, finit_module - and two signature 
verification methods. The problem you are trying to address is the 
finit_module syscall, using both signature verification methods. Why 
enable both signature verification methods ?

If the modules are signed with build time generated keys, use module 
signature verification. If the keys are generated by you and can be 
added to .ima keyring, use IMA policy. The IMA_ARCH_POLICY defines 
secure boot policies at runtime, based on the secure boot state of the 
system. The arch-specific policies defines IMA policy only if the module 
verification is not enabled.

Thanks & Regards,

     - Nayna

Re: [RFC PATCH 0/2] ima: uncompressed module appraisal support

[Eric Snowberg]

> On Feb 12, 2020, at 7:04 AM, Nayna <nayna@linux.vnet.ibm.com> wrote:
> 
> 
> On 2/11/20 12:33 PM, Eric Snowberg wrote:
>>> On Feb 10, 2020, at 1:33 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Mon, 2020-02-10 at 12:24 -0700, Eric Snowberg wrote:
>>>>> On Feb 10, 2020, at 10:09 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>>> Ok, understood, “modsig” refers to strictly kernel module appended signatures
>>>>>> without regard to the keyring that verifies it.  Since there are inconsistencies
>>>>>> here, would you consider something like my first patch?  It will verify an
>>>>>> uncompressed kernel module containing an appended signature  when the public key
>>>>>> is contained within the kernel keyring instead of the ima keyring.  Why force a
>>>>>> person to add the same keys into the ima keyring for validation?  Especially when
>>>>>> the kernel keyring is now used to verify appended signatures in the compressed
>>>>>> modules.
>>>>> Different use case scenarios have different requirements.  Suppose for
>>>>> example that the group creating the kernel image is not the same as
>>>>> using it.  The group using the kernel image could sign all files,
>>>>> including kernel modules (imasig), with their own private key. Only
>>>>> files that they signed would be permitted.  Your proposal would break
>>>>> the current expectations, allowing kernel modules signed by someone
>>>>> else to be loaded.
>>>>> 
>>>> All the end user needs to do is compress any module created by the group that built
>>>> the original kernel image to work around the scenario above.  Then the appended
>>>> signature in the compressed module will be verified by the kernel keyring. Does
>>>> this mean there is a security problem that should be fixed, if this is a concern?
>>> Again, the issue isn't compressed/uncompressed kernel modules, but the
>>> syscall used to load the kernel module.  IMA can prevent using the the
>>> init_module syscall.  Refer to the ima_load_data() LOADING_MODULE
>>> case.
>> Within the ima_load_data() LOADING_MODULE case, to prevent IMA from using
>> the init_module syscall, is_module_sig_enforced() must return false. Currently
>> when is_module_sig_enforced() returns true, the kernel keyring is always used
>> for verification.
>> 
>> What if I change this part of my patch from
>> 
>> +       if (rc && func == MODULE_CHECK)
>> 
>> to
>> 
>> +       sig_enforce = is_module_sig_enforced();
>> +       if (sig_enforce && rc && func == MODULE_CHECK)
>> 
>> Now when the init_module syscall is available, finit_module syscall will use
>> both the ima keyring and kernel keyring for verification.  When the
>> init_module syscall is blocked from use, the finit_module syscall will only use
>> the ima keyring for validation.  I believe this would satisfy both your use
>> case and mine.
>> 
> There are two syscalls - init_module, finit_module - and two signature verification methods. The problem you are trying to address is the finit_module syscall, using both signature verification methods. Why enable both signature verification methods ?

I am enabling both in my patch since a person can turn around and use the other syscall by 
simply compressing their module.  Now their module is verified by a different keyring. 
Other than completely disabling the init_module syscall, which we don’t do, there is nothing 
preventing them from doing that.  We have one kernel config per architecture. We build
and sign the modules with an appended signature.

I can not predict all the ways someone will use a kernel built from this single config.  
I do believe if someone has IMA working with module verification and appended signatures,
some are not going to understand why their module that was compressed and loading 
(via syscall init_module) suddenly fails to load (via syscall finit_module) once they 
uncompress it.