Re: [PATCH v7 0/3] fanotify: Allow user space to pass back additional audit info

On Wed, Feb 8, 2023 at 7:08 AM Jan Kara <jack(a)suse.cz&gt; wrote: > On Tue 07-02-23 09:54:11, Paul Moore wrote: > > On Tue, Feb 7, 2023 at 7:09 AM Jan Kara <jack(a)suse.cz&gt; wrote: > > > On Fri 03-02-23 16:35:13, Richard Guy Briggs wrote: > > > > The Fanotify API can be used for access control by requesting > > > > permission > > > > event notification. The user space tooling that uses it may have a > > > > complicated policy that inherently contains additional context for > > > > the > > > > decision. If this information were available in the audit trail, > > > > policy > > > > writers can close the loop on debugging policy. Also, if this > > > > additional > > > > information were available, it would enable the creation of tools > > > > that > > > > can suggest changes to the policy similar to how audit2allow can > > > > help > > > > refine labeled security. > > > > > > > > This patchset defines a new flag (FAN_INFO) and new extensions that > > > > define additional information which are appended after the response > > > > structure returned from user space on a permission event. The > > > > appended > > > > information is organized with headers containing a type and size > > > > that > > > > can be delegated to interested subsystems. One new information > > > > type is > > > > defined to audit the triggering rule number. > > > > > > > > A newer kernel will work with an older userspace and an older > > > > kernel > > > > will behave as expected and reject a newer userspace, leaving it up > > > > to > > > > the newer userspace to test appropriately and adapt as necessary. > > > > This > > > > is done by providing a a fully-formed FAN_INFO extension but > > > > setting the > > > > fd to FAN_NOFD. On a capable kernel, it will succeed but issue no > > > > audit > > > > record, whereas on an older kernel it will fail. > > > > > > > > The audit function was updated to log the additional information in > > > > the > > > > AUDIT_FANOTIFY record. The following are examples of the new record > > > > > > > > format: > > > > type=FANOTIFY msg=audit(1600385147.372:590): resp=2 fan_type=1 > > > > fan_info=3137 subj_trust=3 obj_trust=5 type=FANOTIFY > > > > msg=audit(1659730979.839:284): resp=1 fan_type=0 fan_info=0 > > > > subj_trust=2 obj_trust=2> > > > > > Thanks! I've applied this series to my tree. > > > > While I think this version of the patchset is fine, for future > > reference it would have been nice if you had waited for my ACK on > > patch 3/3; while Steve maintains his userspace tools, I'm the one > > responsible for maintaining the Linux Kernel's audit subsystem. > > Aha, I'm sorry for that. I had the impression that on the last version of > the series you've said you don't see anything for which the series should > be respun so once Steve's objections where addressed and you were silent > for a few days, I thought you consider the thing settled... My bad. That's understandable, especially given inconsistencies across subsystems. If it helps, if I'm going to ACK something I make it explicit with a proper 'Acked-by: ...' line in my reply; if I say something looks good but there is no explicit ACK, there is usually something outstanding that needs to be resolved, e.g. questions, additional testing, etc. In this particular case I posed some questions in that thread and never saw a reply with any answers, hence the lack of an ACK. While I think the patches were reasonable, I withheld my ACK until the questions were answered ... which they never were from what I can tell, we just saw a new patchset with changes. /me shrugs