Re: [PATCH v4 3/4] fanotify,audit: Allow audit to use the full permission event response

[Jan Kara]

Hello Steve!

On Fri 09-09-22 00:03:53, Steve Grubb wrote:
> On Thursday, September 8, 2022 10:41:44 PM EDT Richard Guy Briggs wrote:
> > > I'm trying to abide by what was suggested by the fs-devel folks. I can
> > > live with it. But if you want to make something non-generic for all
> > > users of fanotify, call the new field "trusted". This would decern when
> > > a decision was made because the file was untrusted or access denied for
> > > another reason.
> >
> > So, "u32 trusted;" ?  How would you like that formatted?
> > "fan_trust={0|1}"
> 
> So how does this play out if there is another user? Do they want a num= and 
> trust=  if not, then the AUDIT_FANOTIFY record will have multiple formats 
> which is not good. I'd rather suggest something generic that can be 
> interpreted based on who's attached to fanotify. IOW we have a fan_type=0 and 
> then followed by info0= info1=  the interpretation of those solely depend on 
> fan_type. If the fan_type does not need both, then any interpretation skips 
> what it doesn't need. If fan_type=1, then it follows what arg0= and arg1= is 
> for that format. But make this pivot on fan_type and not actual names.

So I think there is some misunderstanding so let me maybe spell out in
detail how I see things so that we can get on the same page:

It was a requirement from me (and probably Amir) that there is a generic
way to attach additional info to a response to fanotify permission event.
This is achieved by defining:

struct fanotify_response_info_header {
       __u8 type;
       __u8 pad;
       __u16 len;
};

which is a generic header and kernel can based on 'len' field decide how
large the response structure is (to safely copy it from userspace) and
based on 'type' field it can decide who should be the recipient of this
extra information (or generally what to do with it). So any additional
info needs to start with this header.

Then there is:

struct fanotify_response_info_audit_rule {
       struct fanotify_response_info_header hdr;
       __u32 audit_rule;
};

which properly starts with the header and hdr.type is expected to be
FAN_RESPONSE_INFO_AUDIT_RULE. What happens after the header with type
FAN_RESPONSE_INFO_AUDIT_RULE until length hdr.len is fully within *audit*
subsystem's responsibility. Fanotify code will just pass this as an opaque
blob to the audit subsystem.

So if you know audit subsystem will also need some other field together
with 'audit_rule' now is a good time to add it and it doesn't have to be
useful for anybody else besides audit. If someone else will need other
information passed along with the response, he will append structure with
another header with different 'type' field. In principle, there can be
multiple structures appended to fanotify response like

<hdr> <data> <hdr> <data> ...

and fanotify subsystem will just pass them to different receivers based
on the type in 'hdr' field.

Also if audit needs to pass even more information along with the respose,
we can define a new 'type' for it. But the 'type' space is not infinite so
I'd prefer this does not happen too often...

I hope this clears out things a bit.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR