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);
>
>
