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

[Steve Grubb]

On Friday, September 9, 2022 7:09:44 AM EDT Jan Kara wrote:
> 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.

Yes. Thank you.

Richard,  add subj_trust and obj_trust. These can be 0|1|2 for no, yes, 
unknown.

-Steve