Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules

[Casey Schaufler <casey@schaufler-ca.com>]

On 7/13/2020 10:40 AM, Richard Guy Briggs wrote:
> On 2020-07-08 18:49, Paul Moore wrote:
>> On Fri, Jul 3, 2020 at 1:18 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>>> When there are no rules present, the event SOCKADDR record is not
>>> generated due to audit_dummy_context() generated at syscall entry from
>>> audit_n_rules.  Store this information if there is a context present to
>>> store it so that mandatory events are more complete (startup, LSMs...).

I don't know for sure, but this looks a lot like the issues
I have had to address for LSM stacking in

[PATCH v18 20/23] Audit: Add new record for multiple process LSM attributes

I don't know if the approach I took will help here, or be
acceptable at all for that matter. But it might be worth taking
a look.

>>>
>>> Please see the upstream issue
>>> https://github.com/linux-audit/audit-kernel/issues/122
>>>
>>> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>>> ---
>>> Passes audit-testsuite.
>>>
>>>  include/linux/audit.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> Do we have any certification requirements driving this change?  I ask
>> because if we make this change, why not do the same for PATH records?
> I filed the issue because I noticed the SOCKADDR record missing from
> configuration events required for certification.
>
>> The problem of course is that doing this for both is going to be
>> costly, the PATH records in particular seem like they would raise a
>> performance regression.
> I had a minor concern about performance for SOCKADDR.  I filed SOCKADDR
> because I noticed it missing, but intended to file others as noticed.
>
> I agree the PATH records would have a larger performance concern, but if
> they are required to support event records that are required for
> certification then either we store them when the context is present or
> generate them retroactively once a required event record is generated.
>
> In the case of SOCKADDR it may be possible to store that information
> once the required record has been generated rather than whenever there
> is a valid audit context, but it is currently collected earlier than
> config records are emitted.
>
>> I agree it would be nice to have this information, but I fear that
>> gating this on audit_dummy_context() is the right thing to do unless
>> there is a strong requirement that we always record this information.
> That would have been great feedback when the issue was filed.
> However, there may be a middle ground as descirbed above.
>
>>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>>> index 03c4035a532b..07fecd99741a 100644
>>> --- a/include/linux/audit.h
>>> +++ b/include/linux/audit.h
>>> @@ -448,7 +448,7 @@ static inline int audit_socketcall_compat(int nargs, u32 *args)
>>>
>>>  static inline int audit_sockaddr(int len, void *addr)
>>>  {
>>> -       if (unlikely(!audit_dummy_context()))
>>> +       if (audit_context())
>>>                 return __audit_sockaddr(len, addr);
>>>         return 0;
>>>  }
>> paul moore
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
>

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit