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: [RFC PATCH v9 00/16] Integrity Policy Enforcement LSM (IPE)

2023-02-08 Thread Fan Wu
On Thu, Feb 02, 2023 at 11:48:18AM +0100, Roberto Sassu wrote:
> On Tue, 2023-01-31 at 16:48 -0800, Fan Wu wrote:
> > On Tue, Jan 31, 2023 at 03:22:05PM +0100, Roberto Sassu wrote:
> > > On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> > > > IPE has two known gaps:
> > > > 
> > > > 1. IPE cannot verify the integrity of anonymous executable memory, such 
> > > > as
> > > >   the trampolines created by gcc closures and libffi (<3.4.2), or JIT'd 
> > > > code.
> > > >   Unfortunately, as this is dynamically generated code, there is no way
> > > >   for IPE to ensure the integrity of this code to form a trust basis. 
> > > > In all
> > > >   cases, the return result for these operations will be whatever the 
> > > > admin
> > > >   configures the DEFAULT action for "EXECUTE".
> > > 
> > > I think it would be useful to handle special cases, for example you
> > > could allow a process that created a file with memfd to use it, at the
> > > condition that nobody else writes it.
> > > 
> > > This would be required during the boot, otherwise services could fail
> > > to start (depending on the policy).
> > > 
> > Thanks for the suggestion. I agree with your opinion and I think supporting
> > memfd is possible but restricting read/write needs more hooks. We would like
> > to avoid adding more complexity to this initial posting as necessary. 
> > We will consider this as a future work and will post follow-on patches
> > in the future.
> 
> Ok, maybe it is necessary to specify better the scope of IPE, why the
> current implementation can be considered as complete.
> 
> If we say, IPE can only allow/deny operations on system components with
> immutable security properties, clearly memfd as a component cannot
> fullfill this goal due to the non-immutability. This would apply to any
> component allowing modifications.
> 
> How to address this? What is the immutable property then?
> 
> In the case of memfd, intuitively, a useful property for integrity
> could be for example that the content can be accessed/modified by only
> one process. No other (possibly malicious) processes can tamper with
> that file.
> 
> So, it is true, to make this property immutable more hooks are needed.
> But should it be something that IPE does? Or it should be done by an
> external component (another LSM) that does the enforcement and reports
> to IPE that the property is true? Theoretically (with a proper policy),
> existing LSMs could be used for that purpose too.
> 
> I would say more the second, it should not be IPE job, so that IPE can
> exclusively focus on evaluating properties, not making sure that the
> properties are immutable.
> 
> Roberto
> 
I think the issue here is not about the scope of IPE but the use cases
of IPE. 

We use IPE on fixed-function devices, which are completely locked down.
In our system, IPE denies all anonymous memory execution so memfd will
not work on our system.

Therefore, to make memfd useable with IPE we must add more properties.

-Fan

--
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: [RFC PATCH v9 13/16] ipe: enable support for fs-verity as a trust provider

2023-02-08 Thread Fan Wu
On Thu, Feb 02, 2023 at 10:51:56AM +0100, Roberto Sassu wrote:
> On Wed, 2023-02-01 at 15:50 -0800, Fan Wu wrote:
> > On Tue, Jan 31, 2023 at 03:00:08PM +0100, Roberto Sassu wrote:
> > > On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> > > > +/**
> > > > + * evaluate_fsv_sig_false - Analyze @ctx against a fsv sig false 
> > > > property.
> > > > + * @ctx: Supplies a pointer to the context being evaluated.
> > > > + * @p: Supplies a pointer to the property being evaluated.
> > > > + *
> > > > + * Return:
> > > > + * * true  - The current @ctx match the @p
> > > > + * * false - The current @ctx doesn't match the @p
> > > > + */
> > > > +static bool evaluate_fsv_sig_false(const struct ipe_eval_ctx *const 
> > > > ctx,
> > > > +  struct ipe_prop *p)
> > > > +{
> > > > +   return !ctx->ino ||
> > > > +  !IS_VERITY(ctx->ino) ||
> > > > +  !ctx->ipe_inode ||
> > > > +  !ctx->ipe_inode->fs_verity_signed;
> > > > +}
> > > > +
> > > > +/**
> > > > + * evaluate_fsv_sig_true - Analyze @ctx against a fsv sig true 
> > > > property.
> > > > + * @ctx: Supplies a pointer to the context being evaluated.
> > > > + * @p: Supplies a pointer to the property being evaluated.
> > > > + *
> > > > + * Return:
> > > > + * * true - The current @ctx match the @p
> > > > + * * false - The current @ctx doesn't match the @p
> > > > + */
> > > > +static bool evaluate_fsv_sig_true(const struct ipe_eval_ctx *const ctx,
> > > > + struct ipe_prop *p)
> > > > +{
> > > > +   return ctx->ino &&
> > > > +  IS_VERITY(ctx->ino) &&
> > > > +  ctx->ipe_inode &&
> > > > +  ctx->ipe_inode->fs_verity_signed;
> > > > +}
> > > 
> > > Isn't better to just define one function and prepend a ! in
> > > evaluate_property()?
> > Yes that's a better way to do it, I will take this idea.
> > 
> > > Not sure about the usefulness of the fsverity_signature= property as it
> > > is. I would at minimum allow to specify which keyring signatures are
> > > verified against, and ensure that the keyring has a restriction.
> > > 
> > > And maybe I would call fsverity_verify_signature() directly, after
> > > extending it to pass the desired keyring.
> > > 
> > Thanks for the suggestion.
> > For the initial version we only have the fsverity_signature property
> > to enable the policy can make decision based on the existence of the
> > signature. In the future we plan to add more properties to leverage
> > the remaining signature information so we can have the restrictions
> > you mentioned.
> 
> Uhm, these boolean properties feel like something is missing. In my
> opinion, one cannot accept just any signature, but should be able to
> specify the approved signers.
> 
> Roberto
> 
It does not accept any signature. For fsverity, the signature must be signed
by a key in the fsverity_keyring and similarly for dmverity the signature
must be signed by a key in the kernel builtin trusted keys or secondary keyring.
Therefore, the root of trust here is the system configured keyrings. 

The Boolean properties dmverity_signature and fsverity_signature are used
to differentiate the existence of signature because the signature is optional.
In a =TRUE case of these two properties, we know the digests are signed
by a key we can trust. And in a =FALSE case we know the file is from a unsigned
dmverity or fsverity, we could use a stricter policy to deny them. 

I agree that having the ability to restrict signers is better, but the feedback
from the last version was asking us to keep the initial version as simple
as possible. We definitely want to add more properties, but we will invest
more time in them once the initial version is accepted. 

-Fan

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