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-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: Upstream kernel development and the linux-audit mailing list

2023-02-07 Thread Paul Moore
On Fri, Feb 3, 2023 at 11:39 AM Paul Moore  wrote:
> On Tue, Jan 31, 2023 at 2:44 PM Paul Moore  wrote:
> >
> > Hello all,
>
> ...
>
> > I'll hold off on list creation for a couple of days in case anyone has
> > a well reasoned argument against moving upstream kernel development to
> > a new list.  However, I want to underscore that any argument to keep
> > upstream discussions on a moderated list will need to be strong enough
> > to counter potentially excluding other subsystems from the discussion.
>
> Seeing no comments, I just sent a request off to the vger
> postmaster(s) to create a new list; I'll send another update when it
> is up and running.

The new list, au...@vger.kernel.org, is up and running and is CC'd on
this email.  While I'll continue to monitor linux-audit@redhat for
upstream Linux Kernel patch submissions for a period of time to ease
the transition, I do ask that everyone start submitting their upstream
kernel patches and bug reports to audit@vger; at some point in the
future I plan to stop monitoring linux-audit@redhat for patch
submissions, bug reports, etc.

Information on subscribing to the audit@vger list can be found below:

* http://vger.kernel.org/vger-lists.html#audit
* http://vger.kernel.org/majordomo-info.html

I'll submit a patch to update the MAINTAINERS file with the new list
address later today.

-- 
paul-moore.com

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



[PATCH] audit: update the mailing list in MAINTAINERS

2023-02-07 Thread Paul Moore
We've moved the upstream Linux Kernel audit subsystem discussions to
a new mailing list, this patch updates the MAINTAINERS info with the
new list address.

Marking this for stable inclusion to help speed uptake of the new
list across all of the supported kernel releases.  This is a doc only
patch so the risk should be close to nil.

Cc: sta...@vger.kernel.org
Signed-off-by: Paul Moore 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f86d02cb427..b686c3af313f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3511,7 +3511,7 @@ F:drivers/net/ieee802154/atusb.h
 AUDIT SUBSYSTEM
 M: Paul Moore 
 M: Eric Paris 
-L: linux-audit@redhat.com (moderated for non-subscribers)
+L: au...@vger.kernel.org
 S: Supported
 W: https://github.com/linux-audit
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
-- 
2.39.1

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



Re: [PATCH] audit: update the mailing list in MAINTAINERS

2023-02-07 Thread Paul Moore
On Tue, Feb 7, 2023 at 1:44 PM Paul Moore  wrote:
>
> We've moved the upstream Linux Kernel audit subsystem discussions to
> a new mailing list, this patch updates the MAINTAINERS info with the
> new list address.
>
> Marking this for stable inclusion to help speed uptake of the new
> list across all of the supported kernel releases.  This is a doc only
> patch so the risk should be close to nil.
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Paul Moore 
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I've just merged this into audit/next.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f86d02cb427..b686c3af313f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3511,7 +3511,7 @@ F:drivers/net/ieee802154/atusb.h
>  AUDIT SUBSYSTEM
>  M: Paul Moore 
>  M: Eric Paris 
> -L: linux-audit@redhat.com (moderated for non-subscribers)
> +L: au...@vger.kernel.org
>  S: Supported
>  W: https://github.com/linux-audit
>  T: git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
> --
> 2.39.1

-- 
paul-moore.com

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



Re: [RFC PATCH v9 10/16] dm-verity: consume root hash digest and signature data via LSM hook

2023-02-07 Thread Fan Wu
On Thu, Feb 02, 2023 at 09:21:24AM +0100, Roberto Sassu wrote:
> On Wed, 2023-02-01 at 15:26 -0800, Fan Wu wrote:
> > On Tue, Jan 31, 2023 at 02:22:01PM +0100, Roberto Sassu wrote:
> > > On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> > > > From: Deven Bowers 
> > > > 
> > > > dm-verity provides a strong guarantee of a block device's integrity. As
> > > > a generic way to check the integrity of a block device, it provides
> > > > those integrity guarantees to its higher layers, including the 
> > > > filesystem
> > > > level.
> > > 
> > > I think you could reuse most of is_trusted_verity_target(), in
> > > particular dm_verity_get_root_digest().
> > > 
> > > And probably, the previous patch is not necessary.
> > > 
> > > Roberto
> > > 
> > Thanks for the info. This function seems could be used to get the roothash
> > but for saving the signature we still need the hook function in the previous
> > patch.
> 
> Uhm, look at the LoadPin case. It does not need to temporarily store
> the root digest in a security blob. It evaluates it directly.
> 
> Well, ok, dm_verity_loadpin_is_bdev_trusted() looks for trusted digests
> in the dm_verity_loadpin_trusted_root_digests list. So, something
> equivalent needs to be made for IPE (or you just get the digest).
> However, I find not introducing new hooks and evaluating the
> information directly more efficient.
> 
> Roberto

Thanks for the input. I did a deeper dive into the source code and did some
experiements, my conclusion is the hook is still the preferred way for us.

For the root digest part, dm_verity_loadpin_is_bdev_trusted is able to query
the root digest is because the root digest is saved in struct dm_verity.
Specifically it will call dm_verity_get_root_digest to kmemdup the digest.
If every binary execution will trigger a kmemdup to copy a digest,
the overhead will be noticeable. 
Using a hook can let us copy the root digest exactly once when
the block device is created and free the copied digest when 
the block device is unmounted. 

For the signature, it is currently an optional parameter and it is not
saved in struct dm_verity. But even if we let struct dm_verity saves
the signature it will still have the kmemdup problem above. 
So using the hook will be the cleanest way.

-Fan

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