diff mbox series

[v2,02/13] LSM: Use lsmblob in security_audit_rule_match

Message ID 20240830003411.16818-3-casey@schaufler-ca.com (mailing list archive)
State Changes Requested
Delegated to: Paul Moore
Headers show
Series LSM: Move away from secids | expand

Commit Message

Casey Schaufler Aug. 30, 2024, 12:34 a.m. UTC
Change the secid parameter of security_audit_rule_match
to a lsmblob structure pointer. Pass the entry from the
lsmblob structure for the approprite slot to the LSM hook.

Change the users of security_audit_rule_match to use the
lsmblob instead of a u32. The scaffolding function lsmblob_init()
fills the blob with the value of the old secid, ensuring that
it is available to the appropriate module hook. The sources of
the secid, security_task_getsecid() and security_inode_getsecid(),
will be converted to use the blob structure later in the series.
At the point the use of lsmblob_init() is dropped.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hook_defs.h       |  3 ++-
 include/linux/security.h            |  7 ++++---
 kernel/auditfilter.c                | 11 +++++++----
 kernel/auditsc.c                    | 18 ++++++++++++++----
 security/apparmor/audit.c           |  8 ++++++--
 security/apparmor/include/audit.h   |  2 +-
 security/integrity/ima/ima_policy.c | 11 +++++++----
 security/security.c                 |  7 ++++---
 security/selinux/include/audit.h    |  5 +++--
 security/selinux/ss/services.c      | 11 ++++++++---
 security/smack/smack_lsm.c          | 11 ++++++++---
 11 files changed, 64 insertions(+), 30 deletions(-)

Comments

kernel test robot Aug. 30, 2024, 10:48 p.m. UTC | #1
Hi Casey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pcmoore-audit/next]
[also build test WARNING on pcmoore-selinux/next zohar-integrity/next-integrity linus/master v6.11-rc5 next-20240830]
[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/Casey-Schaufler/LSM-Add-the-lsmblob-data-structure/20240830-085050
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
patch link:    https://lore.kernel.org/r/20240830003411.16818-3-casey%40schaufler-ca.com
patch subject: [PATCH v2 02/13] LSM: Use lsmblob in security_audit_rule_match
config: i386-randconfig-061-20240830 (https://download.01.org/0day-ci/archive/20240831/202408310649.X413mMQP-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/20240831/202408310649.X413mMQP-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/202408310649.X413mMQP-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> security/integrity/ima/ima_policy.c:654:53: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] secid @@     got struct lsmblob * @@
   security/integrity/ima/ima_policy.c:654:53: sparse:     expected unsigned int [usertype] secid
   security/integrity/ima/ima_policy.c:654:53: sparse:     got struct lsmblob *
   security/integrity/ima/ima_policy.c:663:53: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] secid @@     got struct lsmblob * @@
   security/integrity/ima/ima_policy.c:663:53: sparse:     expected unsigned int [usertype] secid
   security/integrity/ima/ima_policy.c:663:53: sparse:     got struct lsmblob *
   security/integrity/ima/ima_policy.c: note: in included file:
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   security/integrity/ima/ima_policy.c:1666:52: sparse: sparse: self-comparison always evaluates to false
   security/integrity/ima/ima_policy.c:1701:55: sparse: sparse: self-comparison always evaluates to false
   security/integrity/ima/ima_policy.c:1728:55: sparse: sparse: self-comparison always evaluates to false
   security/integrity/ima/ima_policy.c:1754:55: sparse: sparse: self-comparison always evaluates to false
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true

vim +654 security/integrity/ima/ima_policy.c

   553	
   554	/**
   555	 * ima_match_rules - determine whether an inode matches the policy rule.
   556	 * @rule: a pointer to a rule
   557	 * @idmap: idmap of the mount the inode was found from
   558	 * @inode: a pointer to an inode
   559	 * @cred: a pointer to a credentials structure for user validation
   560	 * @secid: the secid of the task to be validated
   561	 * @func: LIM hook identifier
   562	 * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
   563	 * @func_data: func specific data, may be NULL
   564	 *
   565	 * Returns true on rule match, false on failure.
   566	 */
   567	static bool ima_match_rules(struct ima_rule_entry *rule,
   568				    struct mnt_idmap *idmap,
   569				    struct inode *inode, const struct cred *cred,
   570				    u32 secid, enum ima_hooks func, int mask,
   571				    const char *func_data)
   572	{
   573		int i;
   574		bool result = false;
   575		struct ima_rule_entry *lsm_rule = rule;
   576		bool rule_reinitialized = false;
   577	
   578		if ((rule->flags & IMA_FUNC) &&
   579		    (rule->func != func && func != POST_SETATTR))
   580			return false;
   581	
   582		switch (func) {
   583		case KEY_CHECK:
   584		case CRITICAL_DATA:
   585			return ((rule->func == func) &&
   586				ima_match_rule_data(rule, func_data, cred));
   587		default:
   588			break;
   589		}
   590	
   591		if ((rule->flags & IMA_MASK) &&
   592		    (rule->mask != mask && func != POST_SETATTR))
   593			return false;
   594		if ((rule->flags & IMA_INMASK) &&
   595		    (!(rule->mask & mask) && func != POST_SETATTR))
   596			return false;
   597		if ((rule->flags & IMA_FSMAGIC)
   598		    && rule->fsmagic != inode->i_sb->s_magic)
   599			return false;
   600		if ((rule->flags & IMA_FSNAME)
   601		    && strcmp(rule->fsname, inode->i_sb->s_type->name))
   602			return false;
   603		if ((rule->flags & IMA_FSUUID) &&
   604		    !uuid_equal(&rule->fsuuid, &inode->i_sb->s_uuid))
   605			return false;
   606		if ((rule->flags & IMA_UID) && !rule->uid_op(cred->uid, rule->uid))
   607			return false;
   608		if (rule->flags & IMA_EUID) {
   609			if (has_capability_noaudit(current, CAP_SETUID)) {
   610				if (!rule->uid_op(cred->euid, rule->uid)
   611				    && !rule->uid_op(cred->suid, rule->uid)
   612				    && !rule->uid_op(cred->uid, rule->uid))
   613					return false;
   614			} else if (!rule->uid_op(cred->euid, rule->uid))
   615				return false;
   616		}
   617		if ((rule->flags & IMA_GID) && !rule->gid_op(cred->gid, rule->gid))
   618			return false;
   619		if (rule->flags & IMA_EGID) {
   620			if (has_capability_noaudit(current, CAP_SETGID)) {
   621				if (!rule->gid_op(cred->egid, rule->gid)
   622				    && !rule->gid_op(cred->sgid, rule->gid)
   623				    && !rule->gid_op(cred->gid, rule->gid))
   624					return false;
   625			} else if (!rule->gid_op(cred->egid, rule->gid))
   626				return false;
   627		}
   628		if ((rule->flags & IMA_FOWNER) &&
   629		    !rule->fowner_op(i_uid_into_vfsuid(idmap, inode),
   630				     rule->fowner))
   631			return false;
   632		if ((rule->flags & IMA_FGROUP) &&
   633		    !rule->fgroup_op(i_gid_into_vfsgid(idmap, inode),
   634				     rule->fgroup))
   635			return false;
   636		for (i = 0; i < MAX_LSM_RULES; i++) {
   637			int rc = 0;
   638			struct lsmblob blob = { };
   639	
   640			if (!lsm_rule->lsm[i].rule) {
   641				if (!lsm_rule->lsm[i].args_p)
   642					continue;
   643				else
   644					return false;
   645			}
   646	
   647	retry:
   648			switch (i) {
   649			case LSM_OBJ_USER:
   650			case LSM_OBJ_ROLE:
   651			case LSM_OBJ_TYPE:
   652				/* scaffolding */
   653				security_inode_getsecid(inode, &blob.scaffold.secid);
 > 654				rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type,
   655							   Audit_equal,
   656							   lsm_rule->lsm[i].rule);
   657				break;
   658			case LSM_SUBJ_USER:
   659			case LSM_SUBJ_ROLE:
   660			case LSM_SUBJ_TYPE:
   661				/* scaffolding */
   662				blob.scaffold.secid = secid;
   663				rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type,
   664							   Audit_equal,
   665							   lsm_rule->lsm[i].rule);
   666				break;
   667			default:
   668				break;
   669			}
   670	
   671			if (rc == -ESTALE && !rule_reinitialized) {
   672				lsm_rule = ima_lsm_copy_rule(rule, GFP_ATOMIC);
   673				if (lsm_rule) {
   674					rule_reinitialized = true;
   675					goto retry;
   676				}
   677			}
   678			if (!rc) {
   679				result = false;
   680				goto out;
   681			}
   682		}
   683		result = true;
   684	
   685	out:
   686		if (rule_reinitialized) {
   687			for (i = 0; i < MAX_LSM_RULES; i++)
   688				ima_filter_rule_free(lsm_rule->lsm[i].rule);
   689			kfree(lsm_rule);
   690		}
   691		return result;
   692	}
   693
diff mbox series

Patch

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 855db460e08b..1d3bdf71109e 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -416,7 +416,8 @@  LSM_HOOK(void, LSM_RET_VOID, key_post_create_or_update, struct key *keyring,
 LSM_HOOK(int, 0, audit_rule_init, u32 field, u32 op, char *rulestr,
 	 void **lsmrule, gfp_t gfp)
 LSM_HOOK(int, 0, audit_rule_known, struct audit_krule *krule)
-LSM_HOOK(int, 0, audit_rule_match, u32 secid, u32 field, u32 op, void *lsmrule)
+LSM_HOOK(int, 0, audit_rule_match, struct lsmblob *blob, u32 field, u32 op,
+	 void *lsmrule)
 LSM_HOOK(void, LSM_RET_VOID, audit_rule_free, void *lsmrule)
 #endif /* CONFIG_AUDIT */
 
diff --git a/include/linux/security.h b/include/linux/security.h
index 0057a22137e8..c0ed2119a622 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -2071,7 +2071,8 @@  static inline void security_key_post_create_or_update(struct key *keyring,
 int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule,
 			     gfp_t gfp);
 int security_audit_rule_known(struct audit_krule *krule);
-int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule);
+int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
+			      void *lsmrule);
 void security_audit_rule_free(void *lsmrule);
 
 #else
@@ -2087,8 +2088,8 @@  static inline int security_audit_rule_known(struct audit_krule *krule)
 	return 0;
 }
 
-static inline int security_audit_rule_match(u32 secid, u32 field, u32 op,
-					    void *lsmrule)
+static inline int security_audit_rule_match(struct lsmblob *blob, u32 field,
+					    u32 op, void *lsmrule)
 {
 	return 0;
 }
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index d6ef4f4f9cba..c4c7cda3b846 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1339,8 +1339,8 @@  int audit_filter(int msgtype, unsigned int listtype)
 
 		for (i = 0; i < e->rule.field_count; i++) {
 			struct audit_field *f = &e->rule.fields[i];
+			struct lsmblob blob = { };
 			pid_t pid;
-			u32 sid;
 
 			switch (f->type) {
 			case AUDIT_PID:
@@ -1370,9 +1370,12 @@  int audit_filter(int msgtype, unsigned int listtype)
 			case AUDIT_SUBJ_SEN:
 			case AUDIT_SUBJ_CLR:
 				if (f->lsm_rule) {
-					security_current_getsecid_subj(&sid);
-					result = security_audit_rule_match(sid,
-						   f->type, f->op, f->lsm_rule);
+					/* scaffolding */
+					security_current_getsecid_subj(
+							&blob.scaffold.secid);
+					result = security_audit_rule_match(
+						   &blob, f->type, f->op,
+						   f->lsm_rule);
 				}
 				break;
 			case AUDIT_EXE:
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6f0d6fb6523f..23adb15cae43 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -471,6 +471,7 @@  static int audit_filter_rules(struct task_struct *tsk,
 	const struct cred *cred;
 	int i, need_sid = 1;
 	u32 sid;
+	struct lsmblob blob = { };
 	unsigned int sessionid;
 
 	if (ctx && rule->prio <= ctx->prio)
@@ -681,7 +682,10 @@  static int audit_filter_rules(struct task_struct *tsk,
 					security_current_getsecid_subj(&sid);
 					need_sid = 0;
 				}
-				result = security_audit_rule_match(sid, f->type,
+				/* scaffolding */
+				blob.scaffold.secid = sid;
+				result = security_audit_rule_match(&blob,
+								   f->type,
 								   f->op,
 								   f->lsm_rule);
 			}
@@ -696,15 +700,19 @@  static int audit_filter_rules(struct task_struct *tsk,
 			if (f->lsm_rule) {
 				/* Find files that match */
 				if (name) {
+					/* scaffolding */
+					blob.scaffold.secid = name->osid;
 					result = security_audit_rule_match(
-								name->osid,
+								&blob,
 								f->type,
 								f->op,
 								f->lsm_rule);
 				} else if (ctx) {
 					list_for_each_entry(n, &ctx->names_list, list) {
+						/* scaffolding */
+						blob.scaffold.secid = n->osid;
 						if (security_audit_rule_match(
-								n->osid,
+								&blob,
 								f->type,
 								f->op,
 								f->lsm_rule)) {
@@ -716,7 +724,9 @@  static int audit_filter_rules(struct task_struct *tsk,
 				/* Find ipc objects that match */
 				if (!ctx || ctx->type != AUDIT_IPC)
 					break;
-				if (security_audit_rule_match(ctx->ipc.osid,
+				/* scaffolding */
+				blob.scaffold.secid = ctx->ipc.osid;
+				if (security_audit_rule_match(&blob,
 							      f->type, f->op,
 							      f->lsm_rule))
 					++result;
diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 6b5181c668b5..758b75a9c1c5 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -264,13 +264,17 @@  int aa_audit_rule_known(struct audit_krule *rule)
 	return 0;
 }
 
-int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
+int aa_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, void *vrule)
 {
 	struct aa_audit_rule *rule = vrule;
 	struct aa_label *label;
 	int found = 0;
 
-	label = aa_secid_to_label(sid);
+	/* scaffolding */
+	if (!blob->apparmor.label && blob->scaffold.secid)
+		label = aa_secid_to_label(blob->scaffold.secid);
+	else
+		label = blob->apparmor.label;
 
 	if (!label)
 		return -ENOENT;
diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h
index 0c8cc86b417b..c5a516e61318 100644
--- a/security/apparmor/include/audit.h
+++ b/security/apparmor/include/audit.h
@@ -202,6 +202,6 @@  static inline int complain_error(int error)
 void aa_audit_rule_free(void *vrule);
 int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule, gfp_t gfp);
 int aa_audit_rule_known(struct audit_krule *rule);
-int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule);
+int aa_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, void *vrule);
 
 #endif /* __AA_AUDIT_H */
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 09da8e639239..40119816b848 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -635,7 +635,7 @@  static bool ima_match_rules(struct ima_rule_entry *rule,
 		return false;
 	for (i = 0; i < MAX_LSM_RULES; i++) {
 		int rc = 0;
-		u32 osid;
+		struct lsmblob blob = { };
 
 		if (!lsm_rule->lsm[i].rule) {
 			if (!lsm_rule->lsm[i].args_p)
@@ -649,15 +649,18 @@  static bool ima_match_rules(struct ima_rule_entry *rule,
 		case LSM_OBJ_USER:
 		case LSM_OBJ_ROLE:
 		case LSM_OBJ_TYPE:
-			security_inode_getsecid(inode, &osid);
-			rc = ima_filter_rule_match(osid, lsm_rule->lsm[i].type,
+			/* scaffolding */
+			security_inode_getsecid(inode, &blob.scaffold.secid);
+			rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type,
 						   Audit_equal,
 						   lsm_rule->lsm[i].rule);
 			break;
 		case LSM_SUBJ_USER:
 		case LSM_SUBJ_ROLE:
 		case LSM_SUBJ_TYPE:
-			rc = ima_filter_rule_match(secid, lsm_rule->lsm[i].type,
+			/* scaffolding */
+			blob.scaffold.secid = secid;
+			rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type,
 						   Audit_equal,
 						   lsm_rule->lsm[i].rule);
 			break;
diff --git a/security/security.c b/security/security.c
index 8cee5b6c6e6d..64a6d6bbd1f4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5399,7 +5399,7 @@  void security_audit_rule_free(void *lsmrule)
 
 /**
  * security_audit_rule_match() - Check if a label matches an audit rule
- * @secid: security label
+ * @lsmblob: security label
  * @field: LSM audit field
  * @op: matching operator
  * @lsmrule: audit rule
@@ -5410,9 +5410,10 @@  void security_audit_rule_free(void *lsmrule)
  * Return: Returns 1 if secid matches the rule, 0 if it does not, -ERRNO on
  *         failure.
  */
-int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule)
+int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
+			      void *lsmrule)
 {
-	return call_int_hook(audit_rule_match, secid, field, op, lsmrule);
+	return call_int_hook(audit_rule_match, blob, field, op, lsmrule);
 }
 #endif /* CONFIG_AUDIT */
 
diff --git a/security/selinux/include/audit.h b/security/selinux/include/audit.h
index 29c7d4c86f6d..104165e4c931 100644
--- a/security/selinux/include/audit.h
+++ b/security/selinux/include/audit.h
@@ -41,7 +41,7 @@  void selinux_audit_rule_free(void *rule);
 
 /**
  *	selinux_audit_rule_match - determine if a context ID matches a rule.
- *	@sid: the context ID to check
+ *	@blob: includes the context ID to check
  *	@field: the field this rule refers to
  *	@op: the operator the rule uses
  *	@rule: pointer to the audit rule to check against
@@ -49,7 +49,8 @@  void selinux_audit_rule_free(void *rule);
  *	Returns 1 if the context id matches the rule, 0 if it does not, and
  *	-errno on failure.
  */
-int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *rule);
+int selinux_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
+			     void *rule);
 
 /**
  *	selinux_audit_rule_known - check to see if rule contains selinux fields.
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e33e55384b75..43eb1d46942c 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -3633,7 +3633,8 @@  int selinux_audit_rule_known(struct audit_krule *rule)
 	return 0;
 }
 
-int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
+int selinux_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
+			     void *vrule)
 {
 	struct selinux_state *state = &selinux_state;
 	struct selinux_policy *policy;
@@ -3659,10 +3660,14 @@  int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
 		goto out;
 	}
 
-	ctxt = sidtab_search(policy->sidtab, sid);
+	/* scaffolding */
+	if (!blob->selinux.secid && blob->scaffold.secid)
+		blob->selinux.secid = blob->scaffold.secid;
+
+	ctxt = sidtab_search(policy->sidtab, blob->selinux.secid);
 	if (unlikely(!ctxt)) {
 		WARN_ONCE(1, "selinux_audit_rule_match: unrecognized SID %d\n",
-			  sid);
+			  blob->selinux.secid);
 		match = -ENOENT;
 		goto out;
 	}
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 4164699cd4f6..52d5ef986db8 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4776,7 +4776,7 @@  static int smack_audit_rule_known(struct audit_krule *krule)
 
 /**
  * smack_audit_rule_match - Audit given object ?
- * @secid: security id for identifying the object to test
+ * @blob: security id for identifying the object to test
  * @field: audit rule flags given from user-space
  * @op: required testing operator
  * @vrule: smack internal rule presentation
@@ -4784,7 +4784,8 @@  static int smack_audit_rule_known(struct audit_krule *krule)
  * The core Audit hook. It's used to take the decision of
  * whether to audit or not to audit a given object.
  */
-static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule)
+static int smack_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
+				  void *vrule)
 {
 	struct smack_known *skp;
 	char *rule = vrule;
@@ -4797,7 +4798,11 @@  static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule)
 	if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
 		return 0;
 
-	skp = smack_from_secid(secid);
+	/* scaffolding */
+	if (!blob->smack.skp && blob->scaffold.secid)
+		skp = smack_from_secid(blob->scaffold.secid);
+	else
+		skp = blob->smack.skp;
 
 	/*
 	 * No need to do string comparisons. If a match occurs,