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

2023-02-09 Thread Jan Kara
On Wed 08-02-23 10:03:24, Paul Moore wrote:
> On Wed, Feb 8, 2023 at 7:08 AM Jan Kara  wrote:
> > On Tue 07-02-23 09:54:11, Paul Moore wrote:
> > > On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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.

Ok, thanks for letting me now. Next time I'll wait for an explicit ack from
you. This time, since everybody is fine with the actual patch, let's just
move on ;).

Honza
-- 
Jan Kara 
SUSE Labs, CR

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



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

2023-02-08 Thread Paul Moore
On Wed, Feb 8, 2023 at 12:37 PM Richard Guy Briggs  wrote:
> On 2023-02-08 11:24, Paul Moore wrote:
> > On Wed, Feb 8, 2023 at 10:27 AM Steve Grubb  wrote:
> > > On Wednesday, February 8, 2023 10:03:24 AM EST Paul Moore wrote:
> > > > On Wed, Feb 8, 2023 at 7:08 AM Jan Kara  wrote:
> > > > > On Tue 07-02-23 09:54:11, Paul Moore wrote:
> > > > > > On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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
> > >
> > > Paul,
> > >
> > > I reread the thread. You only had a request to change if/else to a switch
> > > construct only if there was a respin for the 3F. You otherwise said get
> > > Steve's input and the 3F borders on being overly clever. Both were 
> > > addressed.
> > > If you had other questions that needed answers on, please restate them to
> > > expedite approval of this set of patche

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

2023-02-08 Thread Richard Guy Briggs
On 2023-02-08 11:24, Paul Moore wrote:
> On Wed, Feb 8, 2023 at 10:27 AM Steve Grubb  wrote:
> > On Wednesday, February 8, 2023 10:03:24 AM EST Paul Moore wrote:
> > > On Wed, Feb 8, 2023 at 7:08 AM Jan Kara  wrote:
> > > > On Tue 07-02-23 09:54:11, Paul Moore wrote:
> > > > > On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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
> >
> > Paul,
> >
> > I reread the thread. You only had a request to change if/else to a switch
> > construct only if there was a respin for the 3F. You otherwise said get
> > Steve's input and the 3F borders on being overly clever. Both were 
> > addressed.
> > If you had other questions that needed answers on, please restate them to
> > expedite approval of this set of patches. As far as I can tell, all comments
> > are addressed.
> 
> Steve,
> 
> It might be helpful to reread my reply below:
> 
> https://lore.kernel.org/linux-audit/cahc9vhrwdd6tk6aemgoobbkcvkrybvote7-f0tgjd2drk7n...@mail.gmail.com/
> 
> You'll see that I made a comment in that email about not following
> Richard's explanation about "encoding the 

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

2023-02-08 Thread Paul Moore
On Wed, Feb 8, 2023 at 10:27 AM Steve Grubb  wrote:
> On Wednesday, February 8, 2023 10:03:24 AM EST Paul Moore wrote:
> > On Wed, Feb 8, 2023 at 7:08 AM Jan Kara  wrote:
> > > On Tue 07-02-23 09:54:11, Paul Moore wrote:
> > > > On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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
>
> Paul,
>
> I reread the thread. You only had a request to change if/else to a switch
> construct only if there was a respin for the 3F. You otherwise said get
> Steve's input and the 3F borders on being overly clever. Both were addressed.
> If you had other questions that needed answers on, please restate them to
> expedite approval of this set of patches. As far as I can tell, all comments
> are addressed.

Steve,

It might be helpful to reread my reply below:

https://lore.kernel.org/linux-audit/cahc9vhrwdd6tk6aemgoobbkcvkrybvote7-f0tgjd2drk7n...@mail.gmail.com/

You'll see that I made a comment in that email about not following
Richard's explanation about "encoding the zero" (the patch was
encoding a "?" to the best I could tell).  I was hoping for some
clarification from Richard on his comments, and I never saw anything
in my inbox.  I just checked the archives on lore and I don't see
anything there either.

-- 
paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listm

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

2023-02-08 Thread Steve Grubb
On Wednesday, February 8, 2023 10:03:24 AM EST Paul Moore wrote:
> On Wed, Feb 8, 2023 at 7:08 AM Jan Kara  wrote:
> > On Tue 07-02-23 09:54:11, Paul Moore wrote:
> > > On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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

Paul,

I reread the thread. You only had a request to change if/else to a switch 
construct only if there was a respin for the 3F. You otherwise said get 
Steve's input and the 3F borders on being overly clever. Both were addressed. 
If you had other questions that needed answers on, please restate them to 
expedite approval of this set of patches. As far as I can tell, all comments 
are addressed.

Best,
-Steve



--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



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

2023-02-08 Thread Paul Moore
On Wed, Feb 8, 2023 at 7:08 AM Jan Kara  wrote:
> On Tue 07-02-23 09:54:11, Paul Moore wrote:
> > On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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

-- 
paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



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

2023-02-08 Thread Jan Kara
On Tue 07-02-23 09:54:11, Paul Moore wrote:
> On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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.

Honza
-- 
Jan Kara 
SUSE Labs, CR

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



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

2023-02-07 Thread Paul Moore
On Tue, Feb 7, 2023 at 7:09 AM Jan Kara  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.

-- 
paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



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

2023-02-07 Thread Jan Kara
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.

Honza

> 
> changelog:
> v1:
> - first version by Steve Grubb 
> Link: https://lore.kernel.org/r/2042449.irdbgypaU6@x2
> 
> v2:
> - enhancements suggested by Jan Kara 
> - 1/3 change %d to %u in pr_debug
> - 2/3 change response from __u32 to __u16
> - mod struct fanotify_response and fanotify_perm_event add extra_info_type, 
> extra_info_buf
> - extra_info_buf size max FANOTIFY_MAX_RESPONSE_EXTRA_LEN, add struct 
> fanotify_response_audit_rule
> - extend debug statements
> - remove unneeded macros
> - [internal] change interface to finish_permission_event() and 
> process_access_response()
> - 3/3 update format of extra information
> - [internal] change interface to audit_fanotify()
> - change ctx_type= to fan_type=
> Link: https://lore.kernel.org/r/cover.1651174324.git@redhat.com
> 
> v3:
> - 1/3 switch {,__}audit_fanotify() from uint to u32
> - 2/3 re-add fanotify_get_response switch case FAN_DENY: to avoid unnecessary 
> churn
> - add FAN_EXTRA flag to indicate more info and break with old kernel
> - change response from u16 to u32 to avoid endian issues
> - change extra_info_buf to union
> - move low-cost fd check earlier
> - change FAN_RESPONSE_INFO_AUDIT_NONE to FAN_RESPONSE_INFO_NONE
> - switch to u32 for internal and __u32 for uapi
> Link: https://lore.kernel.org/all/cover.1652730821.git@redhat.com
> 
> v4:
> - scrap FAN_INVALID_RESPONSE_MASK in favour of original to catch invalid 
> response == 0
> - introduce FANOTIFY_RESPONSE_* macros
> - uapi: remove union
> - keep original struct fanotify_response, add fan_info infra starting with 
> audit reason
> - uapi add struct fanotify_response_info_header{type/pad/len} and struct 
> fanotify_response_info_audit_rule{hdr/rule}
> - rename fan_ctx= to fan_info=, FAN_EXTRA to FAN_INFO
> - change event struct from type/buf to len/buf
> - enable multiple info extensions in one message
> - hex encode fan_info in __audit_fanotify()
> - record type FANOTIFY extended to "type=FANOTIFY 
> msg=audit(1659730979.839:284): resp=1 fan_type=0 fan_info=3F" 
>   
>   
> Link: https://lore.kernel.org/all/cover.1659996830.git@redhat.com
> 
> v5:
> - fixed warnings in p2/4 and p3/4 found by 
> - restore original behaviour for !FAN_INFO case and fanotify_get_response()
> - rename member audit_rule to rule_number
> - eliminate memory leak of info_buf on failure (no longer dynamic)
> - rename buf:info, count:info_len, c:remain, ib:infop
> - fix pr_debug
> - return -ENOENT on FAN_INFO and fd==FAN_NOFD to signal new kernel
> - fanotify_write() remove redundant size check
> - add u32 subj_trust obj_trust fields with unknown value "2"
> - split out to helper process_access_response_info()
> - restore finish_permission_event() response_struct to u32
> - assume and enforce one rule to audit, pass struct directly to 
> __audit_fanotify()
> - change fanotify_perm_event struct to union hdr/audir_rule
> - add vspace to fanotify_write() and process_access_response_info()
> - squash 3/4 with 4/4
> 

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

2023-02-06 Thread Steve Grubb
Hello Richard,

On Friday, February 3, 2023 4:35:13 PM EST 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.

I have taken the patches and built a new kernel. Everything looks good to me 
on the audit side. It's acting as expected. You can put me down for an ACK or 
tested-by or whatever is appropriate.

-Steve

> 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
> 
> changelog:
> v1:
> - first version by Steve Grubb 
> Link: https://lore.kernel.org/r/2042449.irdbgypaU6@x2
> 
> v2:
> - enhancements suggested by Jan Kara 
> - 1/3 change %d to %u in pr_debug
> - 2/3 change response from __u32 to __u16
> - mod struct fanotify_response and fanotify_perm_event add extra_info_type,
> extra_info_buf - extra_info_buf size max FANOTIFY_MAX_RESPONSE_EXTRA_LEN,
> add struct fanotify_response_audit_rule - extend debug statements
> - remove unneeded macros
> - [internal] change interface to finish_permission_event() and
> process_access_response() - 3/3 update format of extra information
> - [internal] change interface to audit_fanotify()
> - change ctx_type= to fan_type=
> Link: https://lore.kernel.org/r/cover.1651174324.git@redhat.com
> 
> v3:
> - 1/3 switch {,__}audit_fanotify() from uint to u32
> - 2/3 re-add fanotify_get_response switch case FAN_DENY: to avoid
> unnecessary churn - add FAN_EXTRA flag to indicate more info and break
> with old kernel - change response from u16 to u32 to avoid endian issues
> - change extra_info_buf to union
> - move low-cost fd check earlier
> - change FAN_RESPONSE_INFO_AUDIT_NONE to FAN_RESPONSE_INFO_NONE
> - switch to u32 for internal and __u32 for uapi
> Link: https://lore.kernel.org/all/cover.1652730821.git@redhat.com
> 
> v4:
> - scrap FAN_INVALID_RESPONSE_MASK in favour of original to catch invalid
> response == 0 - introduce FANOTIFY_RESPONSE_* macros
> - uapi: remove union
> - keep original struct fanotify_response, add fan_info infra starting with
> audit reason - uapi add struct fanotify_response_info_header{type/pad/len}
> and struct fanotify_response_info_audit_rule{hdr/rule} - rename fan_ctx=
> to fan_info=, FAN_EXTRA to FAN_INFO
> - change event struct from type/buf to len/buf
> - enable multiple info extensions in one message
> - hex encode fan_info in __audit_fanotify()
> - record type FANOTIFY extended to "type=FANOTIFY
> msg=audit(1659730979.839:284): resp=1 fan_type=0 fan_info=3F" Link:
> https://lore.kernel.org/all/cover.1659996830.git@redhat.com
> 
> v5:
> - fixed warnings in p2/4 and p3/4 found by 
> - restore original behaviour for !FAN_INFO case and fanotify_get_response()
> - rename member audit_rule to rule_number
> - eliminate memory leak of info_buf on failure (no longer dynamic)
> - rename buf:info, count:info_len, c:remain, ib:infop
> - fix pr_debug
> - return -ENOENT on FAN_INFO and fd==FAN_NOFD to signal new kernel
> - fanotify_write() remove redundant size check
> - add u32 subj_trust obj_trust fields with unknown value "2"
> - split out to helper process_access_response_info()
> - restore finish_permission_event() response_struct to u32
> - assume and enforce one rule to audit, pass struct directly to
> __audit_fanotify() - change fanotify_perm_event struct to union
> hdr/audir_rule
> - add vspace to fanotify_write() and process_access_response_info()
> - squash 3/4 with 4/4
> - fix v3 and v4 links
> Link: htt

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

2023-02-03 Thread Richard Guy Briggs
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

changelog:
v1:
- first version by Steve Grubb 
Link: https://lore.kernel.org/r/2042449.irdbgypaU6@x2

v2:
- enhancements suggested by Jan Kara 
- 1/3 change %d to %u in pr_debug
- 2/3 change response from __u32 to __u16
- mod struct fanotify_response and fanotify_perm_event add extra_info_type, 
extra_info_buf
- extra_info_buf size max FANOTIFY_MAX_RESPONSE_EXTRA_LEN, add struct 
fanotify_response_audit_rule
- extend debug statements
- remove unneeded macros
- [internal] change interface to finish_permission_event() and 
process_access_response()
- 3/3 update format of extra information
- [internal] change interface to audit_fanotify()
- change ctx_type= to fan_type=
Link: https://lore.kernel.org/r/cover.1651174324.git@redhat.com

v3:
- 1/3 switch {,__}audit_fanotify() from uint to u32
- 2/3 re-add fanotify_get_response switch case FAN_DENY: to avoid unnecessary 
churn
- add FAN_EXTRA flag to indicate more info and break with old kernel
- change response from u16 to u32 to avoid endian issues
- change extra_info_buf to union
- move low-cost fd check earlier
- change FAN_RESPONSE_INFO_AUDIT_NONE to FAN_RESPONSE_INFO_NONE
- switch to u32 for internal and __u32 for uapi
Link: https://lore.kernel.org/all/cover.1652730821.git@redhat.com

v4:
- scrap FAN_INVALID_RESPONSE_MASK in favour of original to catch invalid 
response == 0
- introduce FANOTIFY_RESPONSE_* macros
- uapi: remove union
- keep original struct fanotify_response, add fan_info infra starting with 
audit reason
- uapi add struct fanotify_response_info_header{type/pad/len} and struct 
fanotify_response_info_audit_rule{hdr/rule}
- rename fan_ctx= to fan_info=, FAN_EXTRA to FAN_INFO
- change event struct from type/buf to len/buf
- enable multiple info extensions in one message
- hex encode fan_info in __audit_fanotify()
- record type FANOTIFY extended to "type=FANOTIFY 
msg=audit(1659730979.839:284): resp=1 fan_type=0 fan_info=3F"   

  
Link: https://lore.kernel.org/all/cover.1659996830.git@redhat.com

v5:
- fixed warnings in p2/4 and p3/4 found by 
- restore original behaviour for !FAN_INFO case and fanotify_get_response()
- rename member audit_rule to rule_number
- eliminate memory leak of info_buf on failure (no longer dynamic)
- rename buf:info, count:info_len, c:remain, ib:infop
- fix pr_debug
- return -ENOENT on FAN_INFO and fd==FAN_NOFD to signal new kernel
- fanotify_write() remove redundant size check
- add u32 subj_trust obj_trust fields with unknown value "2"
- split out to helper process_access_response_info()
- restore finish_permission_event() response_struct to u32
- assume and enforce one rule to audit, pass struct directly to 
__audit_fanotify()
- change fanotify_perm_event struct to union hdr/audir_rule
- add vspace to fanotify_write() and process_access_response_info()
- squash 3/4 with 4/4
- fix v3 and v4 links
Link: https://lore.kernel.org/all/cover.1670606054.git@redhat.com

v6:
- simplify __audit_fanotify() from audit_log_format/audit_log_n_hex to 
audit_log/%X
- add comment to clarify {subj,obj}_trust values
- remove fd processing from process_access_response_info()
- return info_len immediately from process_access_response() on FAN_NOFD after 
proc