Re: [PATCH ghak122 v1] audit: store event sockaddr in case of no rules
On 2020-07-13 10:55, Casey Schaufler wrote:
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(a)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.
The first thing I notice is audit_stamp_context() looks pretty similar
to audit_alloc_local() in
[PATCH ghak90 V9 07/13] audit: add support for non-syscall auxiliary records
which creates a local context for limited use by the primary record and
local auxiliary records (contid) and is immediately tossed after those
records. One use case is network namespace events that are not (yet)
tied to any user context ("[PATCH ghak90 V9 10/13] audit: add support
for containerid to network namespaces"). The other is for user records
that don't need to be tied to the userspace caller ("[PATCH ghak90 V9
08/13] audit: add containerid support for user records").
How does this tie in with this situation? I would ideally like to
retroactively capture the sockaddr information in the audit context
struct (to avoid the overhead of always capturing sockaddr) to trigger a
syscall record on exit along with a sockaddr record once a manadatory
record has been triggered (such as audit or netfilter config, LSM event,
etc...), depending only on audit_enabled. There could be other
substructures of audit context that would need filling out after it is
determined that an audit event has happenned and information needs to be
collected to support its reporting.
>>> Please see the upstream issue
>>> https://github.com/linux-audit/audit-kernel/issues/122
>>>
>>> Signed-off-by: Richard Guy Briggs <rgb(a)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
- RGB
--
Richard Guy Briggs <rgb(a)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