[PATCH 0/2] ima: extending IMA policy to support interpreters

[PATCH 0/2] ima: extending IMA policy to support interpreters

[Mimi Zohar]

On file open, the kernel has no way of differentiating between files
containing data and those with code that will be executed.  Only the
interpreter knows how the file will be used.  To bridge this gap, this
patch set extends the IMA policy language:

- to identify files with the executable mode bit set
- to support the new file open flag MAY_OPENEXEC introduced by Mickael
  Salaun's "[PATCH v3 0/5] Add support for RESOLVE_MAYEXEC" patch set.

Mimi

Mimi Zohar (2):
  ima: add policy support for identifying file execute mode bit
  ima: add policy support for the new file open MAY_OPENEXEC flag

 Documentation/ABI/testing/ima_policy |  7 ++++---
 security/integrity/ima/ima_main.c    |  3 ++-
 security/integrity/ima/ima_policy.c  | 33 +++++++++++++++++++++++++++------
 3 files changed, 33 insertions(+), 10 deletions(-)

-- 
2.7.5

[PATCH 1/2] ima: add policy support for identifying file execute mode bit

[Mimi Zohar]

Extend the IMA policy language with "mode=IXUGO" to identify files with
the execute mode bit enabled.

Examples:
measure func=FILE_CHECK mode=IXUGO
appraise func=FILE_CHECK appraise_type=imasig mode=IXUGO

Suggested-by: Steve Grubb <sgrubb@redhat.com> (based on execute mode bit)
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/ABI/testing/ima_policy |  5 +++--
 security/integrity/ima/ima_policy.c  | 18 ++++++++++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index cd572912c593..a12e784cee31 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -20,8 +20,8 @@ Description:
 		action: measure | dont_measure | appraise | dont_appraise |
 			audit | hash | dont_hash
 		condition:= base | lsm  [option]
-			base:	[[func=] [mask=] [fsmagic=] [fsuuid=] [uid=]
-				[euid=] [fowner=] [fsname=]]
+			base:	[[func=] [mask=] [mode=] [fsmagic=] [fsuuid=]
+				[uid=] [euid=] [fowner=] [fsname=]]
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [template=] [permit_directio]
@@ -32,6 +32,7 @@ Description:
 				[KEXEC_CMDLINE] [KEY_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
+			mode:= [IXUGO]
 			fsmagic:= hex value
 			fsuuid:= file system UUID (e.g 8bcbe394-4f13-4144-be8e-5aa9ea2ce2f6)
 			uid:= decimal value
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index ef7f68cc935e..28b68e076638 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -33,6 +33,7 @@
 #define IMA_PCR		0x0100
 #define IMA_FSNAME	0x0200
 #define IMA_KEYRINGS	0x0400
+#define IMA_IXUGO	0x0800
 
 #define UNKNOWN		0
 #define MEASURE		0x0001	/* same as IMA_MEASURE */
@@ -435,6 +436,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 	if ((rule->flags & IMA_INMASK) &&
 	    (!(rule->mask & mask) && func != POST_SETATTR))
 		return false;
+	if ((rule->flags & IMA_IXUGO) && !(inode->i_mode & S_IXUGO))
+		return false;
 	if ((rule->flags & IMA_FSMAGIC)
 	    && rule->fsmagic != inode->i_sb->s_magic)
 		return false;
@@ -459,6 +462,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 	if ((rule->flags & IMA_FOWNER) &&
 	    !rule->fowner_op(inode->i_uid, rule->fowner))
 		return false;
+
 	for (i = 0; i < MAX_LSM_RULES; i++) {
 		int rc = 0;
 		u32 osid;
@@ -822,7 +826,7 @@ enum {
 	Opt_audit, Opt_hash, Opt_dont_hash,
 	Opt_obj_user, Opt_obj_role, Opt_obj_type,
 	Opt_subj_user, Opt_subj_role, Opt_subj_type,
-	Opt_func, Opt_mask, Opt_fsmagic, Opt_fsname,
+	Opt_func, Opt_mask, Opt_mode, Opt_fsmagic, Opt_fsname,
 	Opt_fsuuid, Opt_uid_eq, Opt_euid_eq, Opt_fowner_eq,
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
@@ -847,6 +851,7 @@ static const match_table_t policy_tokens = {
 	{Opt_subj_type, "subj_type=%s"},
 	{Opt_func, "func=%s"},
 	{Opt_mask, "mask=%s"},
+	{Opt_mode, "mode=%s"},
 	{Opt_fsmagic, "fsmagic=%s"},
 	{Opt_fsname, "fsname=%s"},
 	{Opt_fsuuid, "fsuuid=%s"},
@@ -1098,6 +1103,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->flags |= (*args[0].from == '^')
 				     ? IMA_INMASK : IMA_MASK;
 			break;
+		case Opt_mode:
+			ima_log_string(ab, "mode", args[0].from);
+			if ((strcmp(args[0].from, "IXUGO")) == 0)
+				entry->flags |= IMA_IXUGO;
+			else
+				result = -EINVAL;
+			break;
 		case Opt_fsmagic:
 			ima_log_string(ab, "fsmagic", args[0].from);
 
@@ -1185,7 +1197,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			uid_token = (token == Opt_uid_eq) ||
 				    (token == Opt_uid_gt) ||
 				    (token == Opt_uid_lt);
-
 			ima_log_string_op(ab, uid_token ? "uid" : "euid",
 					  args[0].from, entry->uid_op);
 
@@ -1522,6 +1533,9 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, " ");
 	}
 
+	if (entry->flags & IMA_IXUGO)
+		seq_puts(m, "mode=IXUGO ");
+
 	if (entry->flags & IMA_FSMAGIC) {
 		snprintf(tbuf, sizeof(tbuf), "0x%lx", entry->fsmagic);
 		seq_printf(m, pt(Opt_fsmagic), tbuf);
-- 
2.7.5

Re: [PATCH 1/2] ima: add policy support for identifying file execute mode bit

[Lakshmi Ramasubramanian]

On 4/29/20 6:38 AM, Mimi Zohar wrote:

> Extend the IMA policy language with "mode=IXUGO" to identify files with
> the execute mode bit enabled.
> 
> Examples:
> measure func=FILE_CHECK mode=IXUGO
> appraise func=FILE_CHECK appraise_type=imasig mode=IXUGO
> 
> Suggested-by: Steve Grubb <sgrubb@redhat.com> (based on execute mode bit)
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

Reviewed.

[PATCH 2/2] ima: add policy support for the new file open MAY_OPENEXEC flag

[Mimi Zohar]

The kernel has no way of differentiating between a file containing data
or code being opened by an interpreter.  The proposed RESOLVE_MAYEXEC
openat2(2) flag bridges this gap by defining and enabling the MAY_OPENEXEC
flag.

This patch adds IMA policy support for the new MAY_OPENEXEC flag.

Example:
measure func=FILE_CHECK mask=^MAY_OPENEXEC
appraise func=FILE_CHECK appraise_type=imasig mask=^MAY_OPENEXEC

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/ABI/testing/ima_policy |  2 +-
 security/integrity/ima/ima_main.c    |  3 ++-
 security/integrity/ima/ima_policy.c  | 15 +++++++++++----
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index a12e784cee31..aa8e4b6181e0 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -31,7 +31,7 @@ Description:
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
 				[KEXEC_CMDLINE] [KEY_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
-			       [[^]MAY_EXEC]
+			       [[^]MAY_EXEC] [[^]MAY_OPENEXEC]
 			mode:= [IXUGO]
 			fsmagic:= hex value
 			fsuuid:= file system UUID (e.g 8bcbe394-4f13-4144-be8e-5aa9ea2ce2f6)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index f96f151294e6..b644eda68e9e 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -438,7 +438,8 @@ int ima_file_check(struct file *file, int mask)
 
 	security_task_getsecid(current, &secid);
 	return process_measurement(file, current_cred(), secid, NULL, 0,
-				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
+				   mask & (MAY_READ | MAY_WRITE |
+					   MAY_EXEC | MAY_OPENEXEC |
 					   MAY_APPEND), FILE_CHECK);
 }
 EXPORT_SYMBOL_GPL(ima_file_check);
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 28b68e076638..8c29d1b01964 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -407,7 +407,8 @@ static bool ima_match_keyring(struct ima_rule_entry *rule,
  * @cred: a pointer to a credentials structure for user validation
  * @secid: the secid of the task to be validated
  * @func: LIM hook identifier
- * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
+ * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC |
+ *			    MAY_OPENEXEC)
  * @keyring: keyring name to check in policy for KEY_CHECK func
  *
  * Returns true on rule match, false on failure.
@@ -531,7 +532,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  *        being made
  * @secid: LSM secid of the task to be validated
  * @func: IMA hook identifier
- * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
+ * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC |
+ *			    MAY_OPENEXEC)
  * @pcr: set the pcr to extend
  * @template_desc: the template that should be used for this rule
  * @keyring: the keyring name, if given, to be used to check in the policy.
@@ -1097,6 +1099,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->mask = MAY_READ;
 			else if (strcmp(from, "MAY_APPEND") == 0)
 				entry->mask = MAY_APPEND;
+			else if (strcmp(from, "MAY_OPENEXEC") == 0)
+				entry->mask = MAY_OPENEXEC;
 			else
 				result = -EINVAL;
 			if (!result)
@@ -1434,14 +1438,15 @@ const char *const func_tokens[] = {
 
 #ifdef	CONFIG_IMA_READ_POLICY
 enum {
-	mask_exec = 0, mask_write, mask_read, mask_append
+	mask_exec = 0, mask_write, mask_read, mask_append, mask_openexec
 };
 
 static const char *const mask_tokens[] = {
 	"^MAY_EXEC",
 	"^MAY_WRITE",
 	"^MAY_READ",
-	"^MAY_APPEND"
+	"^MAY_APPEND",
+	"^MAY_OPENEXEC"
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)
@@ -1530,6 +1535,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 			seq_printf(m, pt(Opt_mask), mt(mask_read) + offset);
 		if (entry->mask & MAY_APPEND)
 			seq_printf(m, pt(Opt_mask), mt(mask_append) + offset);
+		if (entry->mask & MAY_OPENEXEC)
+			seq_printf(m, pt(Opt_mask), mt(mask_openexec) + offset);
 		seq_puts(m, " ");
 	}
 
-- 
2.7.5

Re: [PATCH 2/2] ima: add policy support for the new file open MAY_OPENEXEC flag

[Lakshmi Ramasubramanian]

On 4/29/20 6:38 AM, Mimi Zohar wrote:
> The kernel has no way of differentiating between a file containing data
> or code being opened by an interpreter.  The proposed RESOLVE_MAYEXEC
> openat2(2) flag bridges this gap by defining and enabling the MAY_OPENEXEC
> flag.
> 
> This patch adds IMA policy support for the new MAY_OPENEXEC flag.
> 
> Example:
> measure func=FILE_CHECK mask=^MAY_OPENEXEC
> appraise func=FILE_CHECK appraise_type=imasig mask=^MAY_OPENEXEC
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

Reviewed.

Re: [PATCH 2/2] ima: add policy support for the new file open MAY_OPENEXEC flag

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]

Hi Mimi,

I love your patch! Yet something to improve:

[auto build test ERROR on integrity/next-integrity]
[also build test ERROR on linus/master v5.7-rc3 next-20200429]
[cannot apply to security/next-testing]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-extending-IMA-policy-to-support-interpreters/20200430-030608
base:   https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
config: arc-allyesconfig (attached as .config)
compiler: arc-elf-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   security/integrity/ima/ima_main.c: In function 'ima_file_check':
>> security/integrity/ima/ima_main.c:442:20: error: 'MAY_OPENEXEC' undeclared (first use in this function); did you mean 'MAY_OPEN'?
     442 |         MAY_EXEC | MAY_OPENEXEC |
         |                    ^~~~~~~~~~~~
         |                    MAY_OPEN
   security/integrity/ima/ima_main.c:442:20: note: each undeclared identifier is reported only once for each function it appears in
>> security/integrity/ima/ima_main.c:444:1: warning: control reaches end of non-void function [-Wreturn-type]
     444 | }
         | ^
--
   security/integrity/ima/ima_policy.c: In function 'ima_parse_rule':
>> security/integrity/ima/ima_policy.c:1100:19: error: 'MAY_OPENEXEC' undeclared (first use in this function); did you mean 'MAY_OPEN'?
    1100 |     entry->mask = MAY_OPENEXEC;
         |                   ^~~~~~~~~~~~
         |                   MAY_OPEN
   security/integrity/ima/ima_policy.c:1100:19: note: each undeclared identifier is reported only once for each function it appears in
   security/integrity/ima/ima_policy.c: In function 'ima_policy_show':
   security/integrity/ima/ima_policy.c:1535:21: error: 'MAY_OPENEXEC' undeclared (first use in this function); did you mean 'MAY_OPEN'?
    1535 |   if (entry->mask & MAY_OPENEXEC)
         |                     ^~~~~~~~~~~~
         |                     MAY_OPEN

vim +442 security/integrity/ima/ima_main.c

   424	
   425	/**
   426	 * ima_path_check - based on policy, collect/store measurement.
   427	 * @file: pointer to the file to be measured
   428	 * @mask: contains MAY_READ, MAY_WRITE, MAY_EXEC or MAY_APPEND
   429	 *
   430	 * Measure files based on the ima_must_measure() policy decision.
   431	 *
   432	 * On success return 0.  On integrity appraisal error, assuming the file
   433	 * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
   434	 */
   435	int ima_file_check(struct file *file, int mask)
   436	{
   437		u32 secid;
   438	
   439		security_task_getsecid(current, &secid);
   440		return process_measurement(file, current_cred(), secid, NULL, 0,
   441					   mask & (MAY_READ | MAY_WRITE |
 > 442						   MAY_EXEC | MAY_OPENEXEC |
   443						   MAY_APPEND), FILE_CHECK);
 > 444	}
   445	EXPORT_SYMBOL_GPL(ima_file_check);
   446	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61946 bytes --]

Re: [PATCH 2/2] ima: add policy support for the new file open MAY_OPENEXEC flag

[Mimi Zohar]

Hi Mickaël,

On Thu, 2020-04-30 at 05:24 +0800, kbuild test robot wrote:
> Hi Mimi,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on integrity/next-integrity]
> [also build test ERROR on linus/master v5.7-rc3 next-20200429]
> [cannot apply to security/next-testing]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

To prevent this sort of message, in the future could you include this
patch (2/2) with your patch set?  Please include the "Reviewed-by:
Lakshmi Ramasubramanian <nramas@linux.microsoft.com>" tag.

thanks,

Mimi

> 
> url:    https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-extending-IMA-policy-to-support-interpreters/20200430-030608
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
> config: arc-allyesconfig (attached as .config)
> compiler: arc-elf-gcc (GCC) 9.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=arc 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> All error/warnings (new ones prefixed by >>):
> 
>    security/integrity/ima/ima_main.c: In function 'ima_file_check':
> >> security/integrity/ima/ima_main.c:442:20: error: 'MAY_OPENEXEC' undeclared (first use in this function); did you mean 'MAY_OPEN'?
>      442 |         MAY_EXEC | MAY_OPENEXEC |
>          |                    ^~~~~~~~~~~~
>          |                    MAY_OPEN

Re: [PATCH 2/2] ima: add policy support for the new file open MAY_OPENEXEC flag

[Mickaël Salaün]

OK, I'll add it to the next series.

On 30/04/2020 15:42, Mimi Zohar wrote:
> Hi Mickaël,
> 
> On Thu, 2020-04-30 at 05:24 +0800, kbuild test robot wrote:
>> Hi Mimi,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on integrity/next-integrity]
>> [also build test ERROR on linus/master v5.7-rc3 next-20200429]
>> [cannot apply to security/next-testing]
>> [if your patch is applied to the wrong git tree, please drop us a note to help
>> improve the system. BTW, we also suggest to use '--base' option to specify the
>> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> 
> To prevent this sort of message, in the future could you include this
> patch (2/2) with your patch set?  Please include the "Reviewed-by:
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com>" tag.
> 
> thanks,
> 
> Mimi
> 
>>
>> url:    https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-extending-IMA-policy-to-support-interpreters/20200430-030608
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
>> config: arc-allyesconfig (attached as .config)
>> compiler: arc-elf-gcc (GCC) 9.3.0
>> reproduce:
>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         # save the attached .config to linux build tree
>>         COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=arc 
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kbuild test robot <lkp@intel.com>
>>
>> All error/warnings (new ones prefixed by >>):
>>
>>    security/integrity/ima/ima_main.c: In function 'ima_file_check':
>>>> security/integrity/ima/ima_main.c:442:20: error: 'MAY_OPENEXEC' undeclared (first use in this function); did you mean 'MAY_OPEN'?
>>      442 |         MAY_EXEC | MAY_OPENEXEC |
>>          |                    ^~~~~~~~~~~~
>>          |                    MAY_OPEN
>