[PATCH v12 03/25] LSM: Use lsmblob in security_audit_rule_match

[Casey Schaufler]

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:
>> 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
>
>>  							rule->lsm[i].type,
>>  							Audit_equal,
>>  							rule->lsm[i].rule);
>
>