On 5/21/2021 1:19 PM, Paul Moore wrote:
> On Thu, May 13, 2021 at 4:32 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
>> Create a new audit record type to contain the subject information
>> when there are multiple security modules that require such data.
>> This record is linked with the same timestamp and serial number
>> using the audit_alloc_local() mechanism.
> The record is linked with the other associated records into a single
> event, it doesn't matter if it gets the timestamp/serial from
> audit_alloc_local() or an existing audit event, e.g. ongoing syscall.
>
>> The record is produced only in cases where there is more than one
>> security module with a process "context".
>> In cases where this record is produced the subj= fields of
>> other records in the audit event will be set to "subj=?".
>>
>> An example of the MAC_TASK_CONTEXTS (1420) record is:
>>
>> type=UNKNOWN[1420]
>> msg=audit(1600880931.832:113)
>> subj_apparmor==unconfined
> It should be just a single "=" in the line above.
AppArmor provides the 2nd "=" as part of the subject context.
What's here is correct. I won't argue that it won't case confusion
or worse.
>> subj_smack=_
>>
>> There will be a subj_$LSM= entry for each security module
>> LSM that supports the secid_to_secctx and secctx_to_secid
>> hooks. The BPF security module implements secid/secctx
>> translation hooks, so it has to be considered to provide a
>> secctx even though it may not actually do so.
>>
>> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
>> To: paul@xxxxxxxxxxxxxx
>> To: linux-audit@xxxxxxxxxx
>> To: rgb@xxxxxxxxxx
>> Cc: netdev@xxxxxxxxxxxxxxx
>> ---
>> drivers/android/binder.c | 2 +-
>> include/linux/audit.h | 24 ++++++++
>> include/linux/security.h | 16 ++++-
>> include/net/netlabel.h | 3 +-
>> include/net/scm.h | 2 +-
>> include/net/xfrm.h | 13 +++-
>> include/uapi/linux/audit.h | 1 +
>> kernel/audit.c | 80 ++++++++++++++++++-------
>> kernel/audit.h | 3 +
>> kernel/auditfilter.c | 6 +-
>> kernel/auditsc.c | 75 ++++++++++++++++++++---
>> net/ipv4/ip_sockglue.c | 2 +-
>> net/netfilter/nf_conntrack_netlink.c | 4 +-
>> net/netfilter/nf_conntrack_standalone.c | 2 +-
>> net/netfilter/nfnetlink_queue.c | 2 +-
>> net/netlabel/netlabel_domainhash.c | 4 +-
>> net/netlabel/netlabel_unlabeled.c | 24 ++++----
>> net/netlabel/netlabel_user.c | 20 ++++---
>> net/netlabel/netlabel_user.h | 6 +-
>> net/xfrm/xfrm_policy.c | 10 ++--
>> net/xfrm/xfrm_state.c | 20 ++++---
>> security/integrity/ima/ima_api.c | 7 ++-
>> security/integrity/integrity_audit.c | 6 +-
>> security/security.c | 46 +++++++++-----
>> security/smack/smackfs.c | 3 +-
>> 25 files changed, 274 insertions(+), 107 deletions(-)
> ...
>
>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>> index 97cd7471e572..229cd71fbf09 100644
>> --- a/include/linux/audit.h
>> +++ b/include/linux/audit.h
>> @@ -386,6 +395,19 @@ static inline void audit_ptrace(struct task_struct *t)
>> __audit_ptrace(t);
>> }
>>
>> +static inline struct audit_context *audit_alloc_for_lsm(gfp_t gfp)
>> +{
>> + struct audit_context *context = audit_context();
>> +
>> + if (context)
>> + return context;
>> +
>> + if (lsm_multiple_contexts())
>> + return audit_alloc_local(gfp);
>> +
>> + return NULL;
>> +}
> See my other comments, but this seems wrong at face value. The
> additional LSM record should happen as part of the existing audit log
> functions.
I'm good with that. But if you defer calling audit_alloc_local()
until you know you need it you may be in a place where you can't
associate the new context with the event. I think. I will have
another go at it.
<snip>
> I think I was distracted with the local context issue and I've lost
> track of the details here, perhaps it's best to fix the local context
> issue first (that should be a big change to this patch) and then we
> can take another look.
I really need to move forward. I'll give allocation of local contexts
as necessary in audit_log_task_context() another shot.
>
>
> --
> paul moore
> www.paul-moore.com
