| Message ID | 20241127210234.121546-1-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ima: instantiate the bprm_creds_for_exec() hook | expand |
Hi Mimi, kernel test robot noticed the following build errors: [auto build test ERROR on zohar-integrity/next-integrity] [also build test ERROR on linus/master v6.12 next-20241128] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Mimi-Zohar/ima-instantiate-the-bprm_creds_for_exec-hook/20241128-120656 base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity patch link: https://lore.kernel.org/r/20241127210234.121546-1-zohar%40linux.ibm.com patch subject: [PATCH] ima: instantiate the bprm_creds_for_exec() hook config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20241128/202411282320.KuWvLpDS-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241128/202411282320.KuWvLpDS-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/202411282320.KuWvLpDS-lkp@intel.com/ All errors (new ones prefixed by >>): security/integrity/ima/ima_main.c: In function 'ima_bprm_creds_for_exec': >> security/integrity/ima/ima_main.c:572:18: error: 'struct linux_binprm' has no member named 'is_check' 572 | if (!bprm->is_check) | ^~ -- security/integrity/ima/ima_appraise.c: In function 'is_bprm_creds_for_exec': >> security/integrity/ima/ima_appraise.c:492:25: error: 'struct linux_binprm' has no member named 'is_check' 492 | if (bprm->is_check) | ^~ vim +572 security/integrity/ima/ima_main.c 556 557 /** 558 * ima_bprm_creds_for_exec - based on policy, collect/store/appraise measurement. 559 * @bprm: contains the linux_binprm structure 560 * 561 * Based on the IMA policy and the execvat(2) AT_CHECK flag, measure and 562 * appraise the integrity of a file to be executed by script interpreters. 563 * Unlike any of the other LSM hooks where the kernel enforces file integrity, 564 * enforcing file integrity is left up to the discretion of the script 565 * interpreter (userspace). 566 * 567 * On success return 0. On integrity appraisal error, assuming the file 568 * is in policy and IMA-appraisal is in enforcing mode, return -EACCES. 569 */ 570 static int ima_bprm_creds_for_exec(struct linux_binprm *bprm) 571 { > 572 if (!bprm->is_check) 573 return 0; 574 575 return ima_bprm_check(bprm); 576 } 577
Hi Mimi,
kernel test robot noticed the following build errors:
[auto build test ERROR on zohar-integrity/next-integrity]
[also build test ERROR on linus/master v6.12 next-20241128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mimi-Zohar/ima-instantiate-the-bprm_creds_for_exec-hook/20241128-120656
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
patch link: https://lore.kernel.org/r/20241127210234.121546-1-zohar%40linux.ibm.com
patch subject: [PATCH] ima: instantiate the bprm_creds_for_exec() hook
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241129/202411290413.VUC6seTw-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241129/202411290413.VUC6seTw-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/202411290413.VUC6seTw-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from security/integrity/ima/ima_main.c:23:
In file included from include/linux/mman.h:5:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
In file included from security/integrity/ima/ima_main.c:26:
In file included from include/linux/ima.h:12:
In file included from include/linux/security.h:35:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from security/integrity/ima/ima_main.c:26:
In file included from include/linux/ima.h:12:
In file included from include/linux/security.h:35:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from security/integrity/ima/ima_main.c:26:
In file included from include/linux/ima.h:12:
In file included from include/linux/security.h:35:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
>> security/integrity/ima/ima_main.c:572:13: error: no member named 'is_check' in 'struct linux_binprm'
572 | if (!bprm->is_check)
| ~~~~ ^
7 warnings and 1 error generated.
--
In file included from security/integrity/ima/ima_appraise.c:13:
In file included from include/linux/xattr.h:18:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
In file included from security/integrity/ima/ima_appraise.c:15:
In file included from include/linux/ima.h:12:
In file included from include/linux/security.h:35:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from security/integrity/ima/ima_appraise.c:15:
In file included from include/linux/ima.h:12:
In file included from include/linux/security.h:35:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from security/integrity/ima/ima_appraise.c:15:
In file included from include/linux/ima.h:12:
In file included from include/linux/security.h:35:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
>> security/integrity/ima/ima_appraise.c:492:13: error: no member named 'is_check' in 'struct linux_binprm'
492 | if (bprm->is_check)
| ~~~~ ^
7 warnings and 1 error generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for GET_FREE_REGION
Depends on [n]: SPARSEMEM [=n]
Selected by [m]:
- RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
vim +572 security/integrity/ima/ima_main.c
556
557 /**
558 * ima_bprm_creds_for_exec - based on policy, collect/store/appraise measurement.
559 * @bprm: contains the linux_binprm structure
560 *
561 * Based on the IMA policy and the execvat(2) AT_CHECK flag, measure and
562 * appraise the integrity of a file to be executed by script interpreters.
563 * Unlike any of the other LSM hooks where the kernel enforces file integrity,
564 * enforcing file integrity is left up to the discretion of the script
565 * interpreter (userspace).
566 *
567 * On success return 0. On integrity appraisal error, assuming the file
568 * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
569 */
570 static int ima_bprm_creds_for_exec(struct linux_binprm *bprm)
571 {
> 572 if (!bprm->is_check)
573 return 0;
574
575 return ima_bprm_check(bprm);
576 }
577
For reference, here is the base patch series: https://lore.kernel.org/all/20241112191858.162021-1-mic@digikod.net/ CCing audit@ On Wed, Nov 27, 2024 at 04:02:34PM -0500, Mimi Zohar wrote: > Like direct file execution (e.g. ./script.sh), indirect file execution > (e.g. sh script.sh) needs to be measured and appraised. Instantiate > the new security_bprm_creds_for_exec() hook to measure and verify the > indirect file's integrity. Unlike direct file execution, indirect file > execution integrity is optionally enforced by the interpreter. > > Update the audit messages to differentiate between kernel and userspace > enforced integrity. I'm not sure to see the full picture. What is the difference between execveat() calls and execveat() + AT_EXECVE_CHECK calls? Both are from user space, the only difference is that the first can lead to a full execution, but the intent is the same. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > security/integrity/ima/ima_appraise.c | 84 ++++++++++++++++++++------- > security/integrity/ima/ima_main.c | 22 +++++++ > 2 files changed, 86 insertions(+), 20 deletions(-) > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index 656c709b974f..b5f8e49cde9d 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -8,6 +8,7 @@ > #include <linux/module.h> > #include <linux/init.h> > #include <linux/file.h> > +#include <linux/binfmts.h> > #include <linux/fs.h> > #include <linux/xattr.h> > #include <linux/magic.h> > @@ -16,6 +17,7 @@ > #include <linux/fsverity.h> > #include <keys/system_keyring.h> > #include <uapi/linux/fsverity.h> > +#include <linux/securebits.h> > > #include "ima.h" > > @@ -276,7 +278,8 @@ static int calc_file_id_hash(enum evm_ima_xattr_type type, > */ > static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > struct evm_ima_xattr_data *xattr_value, int xattr_len, > - enum integrity_status *status, const char **cause) > + enum integrity_status *status, const char **cause, > + bool is_check) > { > struct ima_max_digest_data hash; > struct signature_v2_hdr *sig; > @@ -292,9 +295,11 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > if (*status != INTEGRITY_PASS_IMMUTABLE) { > if (iint->flags & IMA_DIGSIG_REQUIRED) { > if (iint->flags & IMA_VERITY_REQUIRED) > - *cause = "verity-signature-required"; > + *cause = !is_check ? "verity-signature-required" : > + "verity-signature-required(userspace)"; This looks simpler (same for all following checks): is_check ? "verity-signature-required(userspace)" : "verity-signature-required"; > else > - *cause = "IMA-signature-required"; > + *cause = !is_check ? "IMA-signature-required" : > + "IMA-signature-required(userspace)"; > *status = INTEGRITY_FAIL; > break; > } > @@ -314,7 +319,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > else > rc = -EINVAL; > if (rc) { > - *cause = "invalid-hash"; > + *cause = !is_check ? "invalid-hash" : > + "invalid-hash(userspace)"; > *status = INTEGRITY_FAIL; > break; > } > @@ -325,14 +331,16 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > > mask = IMA_DIGSIG_REQUIRED | IMA_VERITY_REQUIRED; > if ((iint->flags & mask) == mask) { > - *cause = "verity-signature-required"; > + *cause = !is_check ? "verity-signature-required" : > + "verity-signature-required(userspace)"; > *status = INTEGRITY_FAIL; > break; > } > > sig = (typeof(sig))xattr_value; > if (sig->version >= 3) { > - *cause = "invalid-signature-version"; > + *cause = !is_check ? "invalid-signature-version" : > + "invalid-signature-version(userspace)"; > *status = INTEGRITY_FAIL; > break; > } > @@ -353,7 +361,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > iint->ima_hash->digest, > iint->ima_hash->length); > if (rc) { > - *cause = "invalid-signature"; > + *cause = !is_check ? "invalid-signature" : > + "invalid-signature(userspace)"; > *status = INTEGRITY_FAIL; > } else { > *status = INTEGRITY_PASS; > @@ -364,7 +373,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > > if (iint->flags & IMA_DIGSIG_REQUIRED) { > if (!(iint->flags & IMA_VERITY_REQUIRED)) { > - *cause = "IMA-signature-required"; > + *cause = !is_check ? "IMA-signature-required" : > + "IMA-signature-required(userspace)"; > *status = INTEGRITY_FAIL; > break; > } > @@ -372,7 +382,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > > sig = (typeof(sig))xattr_value; > if (sig->version != 3) { > - *cause = "invalid-signature-version"; > + *cause = !is_check ? "invalid-signature-version" : > + "invalid-signature-version(userspace)"; > *status = INTEGRITY_FAIL; > break; > } > @@ -382,7 +393,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > container_of(&hash.hdr, > struct ima_digest_data, hdr)); > if (rc) { > - *cause = "sigv3-hashing-error"; > + *cause = !is_check ? "sigv3-hashing-error" : > + "sigv3-hashing-error(userspace)"; > *status = INTEGRITY_FAIL; > break; > } > @@ -392,7 +404,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > xattr_len, hash.digest, > hash.hdr.length); > if (rc) { > - *cause = "invalid-verity-signature"; > + *cause = !is_check ? "invalid-verity-signature" : > + "invalid-verify-signature(userspace)"; > *status = INTEGRITY_FAIL; > } else { > *status = INTEGRITY_PASS; > @@ -401,7 +414,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, > break; > default: > *status = INTEGRITY_UNKNOWN; > - *cause = "unknown-ima-data"; > + *cause = !is_check ? "unknown-ima-data" : > + "unknown-ima-data(userspace)"; > break; > } > > @@ -469,6 +483,18 @@ int ima_check_blacklist(struct ima_iint_cache *iint, > return rc; > } > > +static int is_bprm_creds_for_exec(enum ima_hooks func, struct file *file) > +{ > + struct linux_binprm *bprm = NULL; > + > + if (func == BPRM_CHECK) { > + bprm = container_of(&file, struct linux_binprm, file); > + if (bprm->is_check) > + return 1; > + } > + return 0; > +} > + > /* > * ima_appraise_measurement - appraise file measurement > * > @@ -489,11 +515,24 @@ 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 is_check = false; > > /* If not appraising a modsig, we need an xattr. */ > if (!(inode->i_opflags & IOP_XATTR) && !try_modsig) > return INTEGRITY_UNKNOWN; > > + /* > + * Unlike any of the other LSM hooks where the kernel enforces file > + * integrity, enforcing file integrity for the bprm_creds_for_exec() > + * LSM hook is left up to the discretion of the script interpreter > + * (userspace). > + * > + * Since the SECBIT_EXEC_RESTRICT_FILE flag is just a hint as to > + * userspace intentions, simply annotate the audit messages indicating > + * a userspace based query. > + */ > + is_check = is_bprm_creds_for_exec(func, file); > + > /* If reading the xattr failed and there's no modsig, error out. */ > if (rc <= 0 && !try_modsig) { > if (rc && rc != -ENODATA) > @@ -501,11 +540,14 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, > > if (iint->flags & IMA_DIGSIG_REQUIRED) { > if (iint->flags & IMA_VERITY_REQUIRED) > - cause = "verity-signature-required"; > + cause = !is_check ? "verity-signature-required" : > + "verity-signature-required(userspace)"; > else > - cause = "IMA-signature-required"; > + cause = !is_check ? "IMA-signature-required" : > + "IMA-signature-required(userspace)"; > } else { > - cause = "missing-hash"; > + cause = !is_check ? "missing-hash" : > + "missing-hash(userspace)"; > } > > status = INTEGRITY_NOLABEL; > @@ -531,14 +573,15 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, > break; > fallthrough; > case INTEGRITY_NOLABEL: /* No security.evm xattr. */ > - cause = "missing-HMAC"; > + cause = !is_check ? "missing-HMAC" : "missing-HMAC(userspace)"; > goto out; > case INTEGRITY_FAIL_IMMUTABLE: > set_bit(IMA_DIGSIG, &iint->atomic_flags); > - cause = "invalid-fail-immutable"; > + cause = !is_check ? "invalid-fail-immutable" : > + "invalid-fail-immutable(userspace)"; > goto out; > case INTEGRITY_FAIL: /* Invalid HMAC/signature. */ > - cause = "invalid-HMAC"; > + cause = !is_check ? "invalid-HMAC" : "invalid-HMAC(userspace)"; > goto out; > default: > WARN_ONCE(true, "Unexpected integrity status %d\n", status); > @@ -546,7 +589,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, > > if (xattr_value) > rc = xattr_verify(func, iint, xattr_value, xattr_len, &status, > - &cause); > + &cause, is_check); > > /* > * If we have a modsig and either no imasig or the imasig's key isn't > @@ -568,7 +611,8 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, > ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) || > (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) { > status = INTEGRITY_FAIL; > - cause = "unverifiable-signature"; > + cause = !is_check ? "unverifiable-signature" : > + "unverifiable-signature(userspace)"; > integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, > op, cause, rc, 0); Instead of adding new causes, another option would be to add a new audit record type (e.g. AUDIT_INTEGRITY_DATA_CHECK). This would help filter these new kind of messages and I guess scale better. Another alternative would be to extend the audit message with a new field (e.g. "check=1"), but that would not help for efficient filtering. > } else if (status != INTEGRITY_PASS) { > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 06132cf47016..2b5d6bae77a4 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -554,6 +554,27 @@ static int ima_bprm_check(struct linux_binprm *bprm) > MAY_EXEC, CREDS_CHECK); > } > > +/** > + * ima_bprm_creds_for_exec - based on policy, collect/store/appraise measurement. > + * @bprm: contains the linux_binprm structure > + * > + * Based on the IMA policy and the execvat(2) AT_CHECK flag, measure and > + * appraise the integrity of a file to be executed by script interpreters. > + * Unlike any of the other LSM hooks where the kernel enforces file integrity, > + * enforcing file integrity is left up to the discretion of the script > + * interpreter (userspace). > + * > + * On success return 0. On integrity appraisal error, assuming the file > + * is in policy and IMA-appraisal is in enforcing mode, return -EACCES. > + */ > +static int ima_bprm_creds_for_exec(struct linux_binprm *bprm) > +{ > + if (!bprm->is_check) > + return 0; > + > + return ima_bprm_check(bprm); > +} > + > /** > * ima_file_check - based on policy, collect/store measurement. > * @file: pointer to the file to be measured > @@ -1177,6 +1198,7 @@ static int __init init_ima(void) > > static struct security_hook_list ima_hooks[] __ro_after_init = { > LSM_HOOK_INIT(bprm_check_security, ima_bprm_check), > + LSM_HOOK_INIT(bprm_creds_for_exec, ima_bprm_creds_for_exec), Why not replace bprm_check_security with bprm_creds_for_exec implementation altogether? > LSM_HOOK_INIT(file_post_open, ima_file_check), > LSM_HOOK_INIT(inode_post_create_tmpfile, ima_post_create_tmpfile), > LSM_HOOK_INIT(file_release, ima_file_free), > -- > 2.47.0 > >
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 656c709b974f..b5f8e49cde9d 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -8,6 +8,7 @@ #include <linux/module.h> #include <linux/init.h> #include <linux/file.h> +#include <linux/binfmts.h> #include <linux/fs.h> #include <linux/xattr.h> #include <linux/magic.h> @@ -16,6 +17,7 @@ #include <linux/fsverity.h> #include <keys/system_keyring.h> #include <uapi/linux/fsverity.h> +#include <linux/securebits.h> #include "ima.h" @@ -276,7 +278,8 @@ static int calc_file_id_hash(enum evm_ima_xattr_type type, */ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, - enum integrity_status *status, const char **cause) + enum integrity_status *status, const char **cause, + bool is_check) { struct ima_max_digest_data hash; struct signature_v2_hdr *sig; @@ -292,9 +295,11 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, if (*status != INTEGRITY_PASS_IMMUTABLE) { if (iint->flags & IMA_DIGSIG_REQUIRED) { if (iint->flags & IMA_VERITY_REQUIRED) - *cause = "verity-signature-required"; + *cause = !is_check ? "verity-signature-required" : + "verity-signature-required(userspace)"; else - *cause = "IMA-signature-required"; + *cause = !is_check ? "IMA-signature-required" : + "IMA-signature-required(userspace)"; *status = INTEGRITY_FAIL; break; } @@ -314,7 +319,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, else rc = -EINVAL; if (rc) { - *cause = "invalid-hash"; + *cause = !is_check ? "invalid-hash" : + "invalid-hash(userspace)"; *status = INTEGRITY_FAIL; break; } @@ -325,14 +331,16 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, mask = IMA_DIGSIG_REQUIRED | IMA_VERITY_REQUIRED; if ((iint->flags & mask) == mask) { - *cause = "verity-signature-required"; + *cause = !is_check ? "verity-signature-required" : + "verity-signature-required(userspace)"; *status = INTEGRITY_FAIL; break; } sig = (typeof(sig))xattr_value; if (sig->version >= 3) { - *cause = "invalid-signature-version"; + *cause = !is_check ? "invalid-signature-version" : + "invalid-signature-version(userspace)"; *status = INTEGRITY_FAIL; break; } @@ -353,7 +361,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, iint->ima_hash->digest, iint->ima_hash->length); if (rc) { - *cause = "invalid-signature"; + *cause = !is_check ? "invalid-signature" : + "invalid-signature(userspace)"; *status = INTEGRITY_FAIL; } else { *status = INTEGRITY_PASS; @@ -364,7 +373,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, if (iint->flags & IMA_DIGSIG_REQUIRED) { if (!(iint->flags & IMA_VERITY_REQUIRED)) { - *cause = "IMA-signature-required"; + *cause = !is_check ? "IMA-signature-required" : + "IMA-signature-required(userspace)"; *status = INTEGRITY_FAIL; break; } @@ -372,7 +382,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, sig = (typeof(sig))xattr_value; if (sig->version != 3) { - *cause = "invalid-signature-version"; + *cause = !is_check ? "invalid-signature-version" : + "invalid-signature-version(userspace)"; *status = INTEGRITY_FAIL; break; } @@ -382,7 +393,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, container_of(&hash.hdr, struct ima_digest_data, hdr)); if (rc) { - *cause = "sigv3-hashing-error"; + *cause = !is_check ? "sigv3-hashing-error" : + "sigv3-hashing-error(userspace)"; *status = INTEGRITY_FAIL; break; } @@ -392,7 +404,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, xattr_len, hash.digest, hash.hdr.length); if (rc) { - *cause = "invalid-verity-signature"; + *cause = !is_check ? "invalid-verity-signature" : + "invalid-verify-signature(userspace)"; *status = INTEGRITY_FAIL; } else { *status = INTEGRITY_PASS; @@ -401,7 +414,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint, break; default: *status = INTEGRITY_UNKNOWN; - *cause = "unknown-ima-data"; + *cause = !is_check ? "unknown-ima-data" : + "unknown-ima-data(userspace)"; break; } @@ -469,6 +483,18 @@ int ima_check_blacklist(struct ima_iint_cache *iint, return rc; } +static int is_bprm_creds_for_exec(enum ima_hooks func, struct file *file) +{ + struct linux_binprm *bprm = NULL; + + if (func == BPRM_CHECK) { + bprm = container_of(&file, struct linux_binprm, file); + if (bprm->is_check) + return 1; + } + return 0; +} + /* * ima_appraise_measurement - appraise file measurement * @@ -489,11 +515,24 @@ 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 is_check = false; /* If not appraising a modsig, we need an xattr. */ if (!(inode->i_opflags & IOP_XATTR) && !try_modsig) return INTEGRITY_UNKNOWN; + /* + * Unlike any of the other LSM hooks where the kernel enforces file + * integrity, enforcing file integrity for the bprm_creds_for_exec() + * LSM hook is left up to the discretion of the script interpreter + * (userspace). + * + * Since the SECBIT_EXEC_RESTRICT_FILE flag is just a hint as to + * userspace intentions, simply annotate the audit messages indicating + * a userspace based query. + */ + is_check = is_bprm_creds_for_exec(func, file); + /* If reading the xattr failed and there's no modsig, error out. */ if (rc <= 0 && !try_modsig) { if (rc && rc != -ENODATA) @@ -501,11 +540,14 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, if (iint->flags & IMA_DIGSIG_REQUIRED) { if (iint->flags & IMA_VERITY_REQUIRED) - cause = "verity-signature-required"; + cause = !is_check ? "verity-signature-required" : + "verity-signature-required(userspace)"; else - cause = "IMA-signature-required"; + cause = !is_check ? "IMA-signature-required" : + "IMA-signature-required(userspace)"; } else { - cause = "missing-hash"; + cause = !is_check ? "missing-hash" : + "missing-hash(userspace)"; } status = INTEGRITY_NOLABEL; @@ -531,14 +573,15 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, break; fallthrough; case INTEGRITY_NOLABEL: /* No security.evm xattr. */ - cause = "missing-HMAC"; + cause = !is_check ? "missing-HMAC" : "missing-HMAC(userspace)"; goto out; case INTEGRITY_FAIL_IMMUTABLE: set_bit(IMA_DIGSIG, &iint->atomic_flags); - cause = "invalid-fail-immutable"; + cause = !is_check ? "invalid-fail-immutable" : + "invalid-fail-immutable(userspace)"; goto out; case INTEGRITY_FAIL: /* Invalid HMAC/signature. */ - cause = "invalid-HMAC"; + cause = !is_check ? "invalid-HMAC" : "invalid-HMAC(userspace)"; goto out; default: WARN_ONCE(true, "Unexpected integrity status %d\n", status); @@ -546,7 +589,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, if (xattr_value) rc = xattr_verify(func, iint, xattr_value, xattr_len, &status, - &cause); + &cause, is_check); /* * If we have a modsig and either no imasig or the imasig's key isn't @@ -568,7 +611,8 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint, ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) || (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) { status = INTEGRITY_FAIL; - cause = "unverifiable-signature"; + cause = !is_check ? "unverifiable-signature" : + "unverifiable-signature(userspace)"; integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, op, cause, rc, 0); } else if (status != INTEGRITY_PASS) { diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 06132cf47016..2b5d6bae77a4 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -554,6 +554,27 @@ static int ima_bprm_check(struct linux_binprm *bprm) MAY_EXEC, CREDS_CHECK); } +/** + * ima_bprm_creds_for_exec - based on policy, collect/store/appraise measurement. + * @bprm: contains the linux_binprm structure + * + * Based on the IMA policy and the execvat(2) AT_CHECK flag, measure and + * appraise the integrity of a file to be executed by script interpreters. + * Unlike any of the other LSM hooks where the kernel enforces file integrity, + * enforcing file integrity is left up to the discretion of the script + * interpreter (userspace). + * + * On success return 0. On integrity appraisal error, assuming the file + * is in policy and IMA-appraisal is in enforcing mode, return -EACCES. + */ +static int ima_bprm_creds_for_exec(struct linux_binprm *bprm) +{ + if (!bprm->is_check) + return 0; + + return ima_bprm_check(bprm); +} + /** * ima_file_check - based on policy, collect/store measurement. * @file: pointer to the file to be measured @@ -1177,6 +1198,7 @@ static int __init init_ima(void) static struct security_hook_list ima_hooks[] __ro_after_init = { LSM_HOOK_INIT(bprm_check_security, ima_bprm_check), + LSM_HOOK_INIT(bprm_creds_for_exec, ima_bprm_creds_for_exec), LSM_HOOK_INIT(file_post_open, ima_file_check), LSM_HOOK_INIT(inode_post_create_tmpfile, ima_post_create_tmpfile), LSM_HOOK_INIT(file_release, ima_file_free),