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

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