linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ima: Fall back to default kernel module signature verification
@ 2025-09-28  3:03 Coiby Xu
  2025-09-30 13:57 ` Mimi Zohar
  2025-10-02 17:17 ` kernel test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Coiby Xu @ 2025-09-28  3:03 UTC (permalink / raw)
  To: linux-integrity
  Cc: Dmitry Torokhov, Karel Srot, Mimi Zohar, Roberto Sassu,
	Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
	Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list

Currently, for any IMA policy that requires appraisal for kernel modules
e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
booting will fail because IMA will reject a kernel module which will
be decompressed in the kernel space and then have its signature
verified.

This happens because when in-kernel module decompression
(CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
syscall instead of init_module to load a module. And IMA mandates IMA
xattr verification for finit_module unless appraise_type=imasig|modsig
is specified in the rule.  However currently initramfs doesn't support
xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
doesn't work either because IMA will treat to-be-decompressed kernel
module as not having module signature as it can't decompress kernel
module to check if signature exists.

So fall back to default kernel module signature verification when we have
no way to verify IMA xattr.

Reported-by: Karel Srot <ksrot@redhat.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
Another approach will be to make IMA decompress the kernel module to
check the signature. This requires refactoring kernel module code to
make the in-kernel module decompressing feature modular and seemingly
more efforts are needed. A second disadvantage is it feels
counter-intuitive to verify the same kernel module signature twice. And
we still need to make ima_policy=secure_boot allow verifying appended
module signature.

Anyways, I'm open to suggestions and can try the latter approach if
there are some benefits I'm not aware of or a better approach.

 security/integrity/ima/ima_appraise.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f435eff4667f..fcc75dd1486f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -502,9 +502,10 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	int rc = xattr_len;
 	bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
+	bool enforce_module_sig = iint->flags & IMA_DIGSIG_REQUIRED && func == MODULE_CHECK;
 
-	/* If not appraising a modsig, we need an xattr. */
-	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig)
+	/* If not appraising a modsig or using default module verification, we need an xattr. */
+	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !enforce_module_sig)
 		return INTEGRITY_UNKNOWN;
 
 	/*
@@ -517,8 +518,8 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	if (is_bprm_creds_for_exec(func, file))
 		audit_msgno = AUDIT_INTEGRITY_USERSPACE;
 
-	/* If reading the xattr failed and there's no modsig, error out. */
-	if (rc <= 0 && !try_modsig) {
+	/* If reading the xattr failed and there's no modsig or module verification, error out. */
+	if (rc <= 0 && !try_modsig && !enforce_module_sig) {
 		if (rc && rc != -ENODATA)
 			goto out;
 
@@ -549,8 +550,8 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	case INTEGRITY_UNKNOWN:
 		break;
 	case INTEGRITY_NOXATTRS:	/* No EVM protected xattrs. */
-		/* It's fine not to have xattrs when using a modsig. */
-		if (try_modsig)
+		/* Fine to not have xattrs when using a modsig or default module verification. */
+		if (try_modsig || enforce_module_sig)
 			break;
 		fallthrough;
 	case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
@@ -580,6 +581,18 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	     rc == -ENOKEY))
 		rc = modsig_verify(func, modsig, &status, &cause);
 
+	/* Fall back to default kernel module signature verification */
+	if (rc && enforce_module_sig) {
+		rc = 0;
+		set_module_sig_enforced();
+		/* CONFIG_MODULE_SIG may be disabled */
+		if (is_module_sig_enforced()) {
+			rc = 0;
+			status = INTEGRITY_PASS;
+			pr_debug("Fall back to default kernel module verification for %s\n", filename);
+		}
+	}
+
 out:
 	/*
 	 * File signatures on some filesystems can not be properly verified.

base-commit: cec1e6e5d1ab33403b809f79cd20d6aff124ccfe
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ima: Fall back to default kernel module signature verification
  2025-09-28  3:03 [PATCH] ima: Fall back to default kernel module signature verification Coiby Xu
@ 2025-09-30 13:57 ` Mimi Zohar
  2025-09-30 20:28   ` Mimi Zohar
  2025-10-02 17:17 ` kernel test robot
  1 sibling, 1 reply; 4+ messages in thread
From: Mimi Zohar @ 2025-09-30 13:57 UTC (permalink / raw)
  To: Coiby Xu, linux-integrity
  Cc: Dmitry Torokhov, Karel Srot, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
	open list:SECURITY SUBSYSTEM, open list

On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
> Currently, for any IMA policy that requires appraisal for kernel modules
> e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
> booting will fail because IMA will reject a kernel module which will
> be decompressed in the kernel space and then have its signature
> verified.
> 
> This happens because when in-kernel module decompression
> (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
> syscall instead of init_module to load a module. And IMA mandates IMA
> xattr verification for finit_module unless appraise_type=imasig|modsig
> is specified in the rule.  However currently initramfs doesn't support
> xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
> doesn't work either because IMA will treat to-be-decompressed kernel
> module as not having module signature as it can't decompress kernel
> module to check if signature exists.
> 
> So fall back to default kernel module signature verification when we have
> no way to verify IMA xattr.
> 
> Reported-by: Karel Srot <ksrot@redhat.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
> Another approach will be to make IMA decompress the kernel module to
> check the signature. This requires refactoring kernel module code to
> make the in-kernel module decompressing feature modular and seemingly
> more efforts are needed. A second disadvantage is it feels
> counter-intuitive to verify the same kernel module signature twice. And
> we still need to make ima_policy=secure_boot allow verifying appended
> module signature.
> 
> Anyways, I'm open to suggestions and can try the latter approach if
> there are some benefits I'm not aware of or a better approach.

Coiby, there are multiple issues being discussed here.  Before deciding on an
appropriate solution, let's frame the issues(s) properly.

1. The finit_module syscall eventually calls init_module_from_file() to read the
module into memory and then decompress it.  The problem is that the kernel
module signature verification occurs during the kernel_read_file(), before the
kernel module is decompressed.  Thus, the appended kernel module signature
cannot be verified.

2. CPIO doesn't have xattr support. There were multiple attempts at including
xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
security.ima were available in the initramfs, then finit_module() could verify
them, as opposed to the appended kernel module signature.

3. The issues described above are generic, not limited to Power.  When
CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
include an "appraise func=MODULE_CHECK".

4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
policy, specified on the boot command line as "ima_policy=secure_boot", always
enforces the IMA signature stored in security.ima.

Partial solutions without kernel changes:
- Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
- Disable kernel module compression.

Complete solution:
- Pick up and upstream Roberto Sassu's last version of initramfs support [1].
- Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
enumeration is READING_MODULE and the kernel module is compressed.  The change
might be limited to ima_post_read_file().

thanks,

Mimi

[1] [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
https://lore.kernel.org/linux-fsdevel/20190523121803.21638-1-roberto.sassu@huawei.com/



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ima: Fall back to default kernel module signature verification
  2025-09-30 13:57 ` Mimi Zohar
@ 2025-09-30 20:28   ` Mimi Zohar
  0 siblings, 0 replies; 4+ messages in thread
From: Mimi Zohar @ 2025-09-30 20:28 UTC (permalink / raw)
  To: Coiby Xu, linux-integrity
  Cc: Dmitry Torokhov, Karel Srot, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
	open list:SECURITY SUBSYSTEM, open list

On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
> On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
> > Currently, for any IMA policy that requires appraisal for kernel modules
> > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
> > booting will fail because IMA will reject a kernel module which will
> > be decompressed in the kernel space and then have its signature
> > verified.
> > 
> > This happens because when in-kernel module decompression
> > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
> > syscall instead of init_module to load a module. And IMA mandates IMA
> > xattr verification for finit_module unless appraise_type=imasig|modsig
> > is specified in the rule.  However currently initramfs doesn't support
> > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
> > doesn't work either because IMA will treat to-be-decompressed kernel
> > module as not having module signature as it can't decompress kernel
> > module to check if signature exists.
> > 
> > So fall back to default kernel module signature verification when we have
> > no way to verify IMA xattr.
> > 
> > Reported-by: Karel Srot <ksrot@redhat.com>
> > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > ---
> > Another approach will be to make IMA decompress the kernel module to
> > check the signature. This requires refactoring kernel module code to
> > make the in-kernel module decompressing feature modular and seemingly
> > more efforts are needed. A second disadvantage is it feels
> > counter-intuitive to verify the same kernel module signature twice. And
> > we still need to make ima_policy=secure_boot allow verifying appended
> > module signature.
> > 
> > Anyways, I'm open to suggestions and can try the latter approach if
> > there are some benefits I'm not aware of or a better approach.
> 
> Coiby, there are multiple issues being discussed here.  Before deciding on an
> appropriate solution, let's frame the issues(s) properly.
> 
> 1. The finit_module syscall eventually calls init_module_from_file() to read the
> module into memory and then decompress it.  The problem is that the kernel
> module signature verification occurs during the kernel_read_file(), before the
> kernel module is decompressed.  Thus, the appended kernel module signature
> cannot be verified.
> 
> 2. CPIO doesn't have xattr support. There were multiple attempts at including
> xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
> security.ima were available in the initramfs, then finit_module() could verify
> them, as opposed to the appended kernel module signature.
> 
> 3. The issues described above are generic, not limited to Power.  When
> CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
> include an "appraise func=MODULE_CHECK".
> 
> 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
> policy, specified on the boot command line as "ima_policy=secure_boot", always
> enforces the IMA signature stored in security.ima.
> 
> Partial solutions without kernel changes:
> - Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
> - Disable kernel module compression.
> 
> Complete solution:
> - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
> - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
> enumeration is READING_MODULE and the kernel module is compressed.  The change
> might be limited to ima_post_read_file().

or perhaps not totally.

init_module_from_file() doesn't pass the flags variable to kernel_read_file(). 
You might want to consider defining a new kernel_read_file_id enumeration named
READING_COMPRESSED_MODULE.

Mimi

> 
> [1] [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
> https://lore.kernel.org/linux-fsdevel/20190523121803.21638-1-roberto.sassu@huawei.com/
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ima: Fall back to default kernel module signature verification
  2025-09-28  3:03 [PATCH] ima: Fall back to default kernel module signature verification Coiby Xu
  2025-09-30 13:57 ` Mimi Zohar
@ 2025-10-02 17:17 ` kernel test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2025-10-02 17:17 UTC (permalink / raw)
  To: Coiby Xu, linux-integrity
  Cc: oe-kbuild-all, Dmitry Torokhov, Karel Srot, Mimi Zohar,
	Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
	James Morris, Serge E. Hallyn, linux-security-module,
	linux-kernel

Hi Coiby,

kernel test robot noticed the following build errors:

[auto build test ERROR on cec1e6e5d1ab33403b809f79cd20d6aff124ccfe]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/ima-Fall-back-to-default-kernel-module-signature-verification/20250928-110501
base:   cec1e6e5d1ab33403b809f79cd20d6aff124ccfe
patch link:    https://lore.kernel.org/r/20250928030358.3873311-1-coxu%40redhat.com
patch subject: [PATCH] ima: Fall back to default kernel module signature verification
config: i386-randconfig-012-20251002 (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>> security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'


vim +587 security/integrity/ima/ima_appraise.c

   483	
   484	/*
   485	 * ima_appraise_measurement - appraise file measurement
   486	 *
   487	 * Call evm_verifyxattr() to verify the integrity of 'security.ima'.
   488	 * Assuming success, compare the xattr hash with the collected measurement.
   489	 *
   490	 * Return 0 on success, error code otherwise
   491	 */
   492	int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
   493				     struct file *file, const unsigned char *filename,
   494				     struct evm_ima_xattr_data *xattr_value,
   495				     int xattr_len, const struct modsig *modsig)
   496	{
   497		static const char op[] = "appraise_data";
   498		int audit_msgno = AUDIT_INTEGRITY_DATA;
   499		const char *cause = "unknown";
   500		struct dentry *dentry = file_dentry(file);
   501		struct inode *inode = d_backing_inode(dentry);
   502		enum integrity_status status = INTEGRITY_UNKNOWN;
   503		int rc = xattr_len;
   504		bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
   505		bool enforce_module_sig = iint->flags & IMA_DIGSIG_REQUIRED && func == MODULE_CHECK;
   506	
   507		/* If not appraising a modsig or using default module verification, we need an xattr. */
   508		if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !enforce_module_sig)
   509			return INTEGRITY_UNKNOWN;
   510	
   511		/*
   512		 * Unlike any of the other LSM hooks where the kernel enforces file
   513		 * integrity, enforcing file integrity for the bprm_creds_for_exec()
   514		 * LSM hook with the AT_EXECVE_CHECK flag is left up to the discretion
   515		 * of the script interpreter(userspace). Differentiate kernel and
   516		 * userspace enforced integrity audit messages.
   517		 */
   518		if (is_bprm_creds_for_exec(func, file))
   519			audit_msgno = AUDIT_INTEGRITY_USERSPACE;
   520	
   521		/* If reading the xattr failed and there's no modsig or module verification, error out. */
   522		if (rc <= 0 && !try_modsig && !enforce_module_sig) {
   523			if (rc && rc != -ENODATA)
   524				goto out;
   525	
   526			if (iint->flags & IMA_DIGSIG_REQUIRED) {
   527				if (iint->flags & IMA_VERITY_REQUIRED)
   528					cause = "verity-signature-required";
   529				else
   530					cause = "IMA-signature-required";
   531			} else {
   532				cause = "missing-hash";
   533			}
   534	
   535			status = INTEGRITY_NOLABEL;
   536			if (file->f_mode & FMODE_CREATED)
   537				iint->flags |= IMA_NEW_FILE;
   538			if ((iint->flags & IMA_NEW_FILE) &&
   539			    (!(iint->flags & IMA_DIGSIG_REQUIRED) ||
   540			     (inode->i_size == 0)))
   541				status = INTEGRITY_PASS;
   542			goto out;
   543		}
   544	
   545		status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value,
   546					 rc < 0 ? 0 : rc);
   547		switch (status) {
   548		case INTEGRITY_PASS:
   549		case INTEGRITY_PASS_IMMUTABLE:
   550		case INTEGRITY_UNKNOWN:
   551			break;
   552		case INTEGRITY_NOXATTRS:	/* No EVM protected xattrs. */
   553			/* Fine to not have xattrs when using a modsig or default module verification. */
   554			if (try_modsig || enforce_module_sig)
   555				break;
   556			fallthrough;
   557		case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
   558			cause = "missing-HMAC";
   559			goto out;
   560		case INTEGRITY_FAIL_IMMUTABLE:
   561			set_bit(IMA_DIGSIG, &iint->atomic_flags);
   562			cause = "invalid-fail-immutable";
   563			goto out;
   564		case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
   565			cause = "invalid-HMAC";
   566			goto out;
   567		default:
   568			WARN_ONCE(true, "Unexpected integrity status %d\n", status);
   569		}
   570	
   571		if (xattr_value)
   572			rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
   573					  &cause);
   574	
   575		/*
   576		 * If we have a modsig and either no imasig or the imasig's key isn't
   577		 * known, then try verifying the modsig.
   578		 */
   579		if (try_modsig &&
   580		    (!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
   581		     rc == -ENOKEY))
   582			rc = modsig_verify(func, modsig, &status, &cause);
   583	
   584		/* Fall back to default kernel module signature verification */
   585		if (rc && enforce_module_sig) {
   586			rc = 0;
 > 587			set_module_sig_enforced();
   588			/* CONFIG_MODULE_SIG may be disabled */
   589			if (is_module_sig_enforced()) {
   590				rc = 0;
   591				status = INTEGRITY_PASS;
   592				pr_debug("Fall back to default kernel module verification for %s\n", filename);
   593			}
   594		}
   595	
   596	out:
   597		/*
   598		 * File signatures on some filesystems can not be properly verified.
   599		 * When such filesystems are mounted by an untrusted mounter or on a
   600		 * system not willing to accept such a risk, fail the file signature
   601		 * verification.
   602		 */
   603		if ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
   604		    ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) ||
   605		     (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) {
   606			status = INTEGRITY_FAIL;
   607			cause = "unverifiable-signature";
   608			integrity_audit_msg(audit_msgno, inode, filename,
   609					    op, cause, rc, 0);
   610		} else if (status != INTEGRITY_PASS) {
   611			/* Fix mode, but don't replace file signatures. */
   612			if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
   613			    (!xattr_value ||
   614			     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
   615				if (!ima_fix_xattr(dentry, iint))
   616					status = INTEGRITY_PASS;
   617			}
   618	
   619			/*
   620			 * Permit new files with file/EVM portable signatures, but
   621			 * without data.
   622			 */
   623			if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
   624			    test_bit(IMA_DIGSIG, &iint->atomic_flags)) {
   625				status = INTEGRITY_PASS;
   626			}
   627	
   628			integrity_audit_msg(audit_msgno, inode, filename,
   629					    op, cause, rc, 0);
   630		} else {
   631			ima_cache_flags(iint, func);
   632		}
   633	
   634		ima_set_cache_status(iint, func, status);
   635		return status;
   636	}
   637	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-02 17:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-28  3:03 [PATCH] ima: Fall back to default kernel module signature verification Coiby Xu
2025-09-30 13:57 ` Mimi Zohar
2025-09-30 20:28   ` Mimi Zohar
2025-10-02 17:17 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).