On 12/31/2019 9:14 AM, Mimi Zohar wrote:

[Cc'ing Matthew Garret based on the additional bprm call to
process_measurement() - commit d906c10d8a31 ("IMA: Support using new
creds in appraisal policy")]

On Tue, 2019-12-24 at 15:18 -0800, Casey Schaufler wrote:

quoted

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f19a895ad7cd..193ddd55420b 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 	for (i = 0; i < MAX_LSM_RULES; i++) {
 		int rc = 0;
 		u32 osid;
+		struct lsmblob blob;

 		if (!rule->lsm[i].rule)
 			continue;

@@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 		case LSM_OBJ_ROLE:
 		case LSM_OBJ_TYPE:
 			security_inode_getsecid(inode, &osid);
-			rc = security_filter_rule_match(osid,
+			lsmblob_init(&blob, osid);
+			rc = security_filter_rule_match(&blob,
 							rule->lsm[i].type,
 							Audit_equal,
 							rule->lsm[i].rule);

@@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 		case LSM_SUBJ_USER:
 		case LSM_SUBJ_ROLE:
 		case LSM_SUBJ_TYPE:
-			rc = security_filter_rule_match(secid,
+			lsmblob_init(&blob, secid);
+			rc = security_filter_rule_match(&blob,

On the bprm hook, IMA calls process_measurement() twice.  The first
time the secid is passed as an argument based on a call to
security_task_getsecid(), while the second time it is based on
security_cred_getsecid().  process_measurement() passes the correct
secid converted to a blob, but instead of using the passed variable,
this code uses the locally defined blob field.  A later patch removes
the the lsmblob_init(), leaving the local blob uninitialized.
 Something is terribly wrong here.

I can see that there's significant work required on audit rule
filtering. security_audit_rule_init() isn't going to work correctly
the way it is. 

I'll admit that the aliasing of audit_rule to filter_rule had me
very confused for some time.

Mimi

quoted

 							rule->lsm[i].type,
 							Audit_equal,
 							rule->lsm[i].rule);