[PATCH v1 0/2] override audit silence norule for fs cases

[PATCH v1 0/2] override audit silence norule for fs cases

[Richard Guy Briggs]

The audit subsystem normally suppresses output when there are no rules
present to avoid overwhelming the user with unwanted messages.  It could
be argued that another security subsystem would generally want to
override that default.  Allow them through for fsnotify and filesystem
security violations.

Richard Guy Briggs (2):
  audit: record fanotify event regardless of presence of rules
  audit: record AUDIT_ANOM_* events regardless of presence of rules

 include/linux/audit.h | 8 +-------
 kernel/audit.c        | 2 +-
 kernel/auditsc.c      | 2 +-
 3 files changed, 3 insertions(+), 9 deletions(-)

-- 
2.43.5

[PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Richard Guy Briggs]

When no audit rules are in place, fanotify event results are
unconditionally dropped due to an explicit check for the existence of
any audit rules.  Given this is a report from another security
sub-system, allow it to be recorded regardless of the existence of any
audit rules.

To test, install and run the fapolicyd daemon with default config.  Then
as an unprivileged user, create and run a very simple binary that should
be denied.  Then check for an event with
	ausearch -m FANOTIFY -ts recent

Link: https://issues.redhat.com/browse/RHEL-1367
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h | 8 +-------
 kernel/auditsc.c      | 2 +-
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 0050ef288ab3..d0c6f23503a1 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
 extern void __audit_mmap_fd(int fd, int flags);
 extern void __audit_openat2_how(struct open_how *how);
 extern void __audit_log_kern_module(char *name);
-extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
+extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
 extern void __audit_tk_injoffset(struct timespec64 offset);
 extern void __audit_ntp_log(const struct audit_ntp_data *ad);
 extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
@@ -525,12 +525,6 @@ static inline void audit_log_kern_module(char *name)
 		__audit_log_kern_module(name);
 }
 
-static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
-{
-	if (!audit_dummy_context())
-		__audit_fanotify(response, friar);
-}
-
 static inline void audit_tk_injoffset(struct timespec64 offset)
 {
 	/* ignore no-op events */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 0627e74585ce..936825114bae 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2880,7 +2880,7 @@ void __audit_log_kern_module(char *name)
 	context->type = AUDIT_KERN_MODULE;
 }
 
-void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
+void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
 {
 	/* {subj,obj}_trust values are {0,1,2}: no,yes,unknown */
 	switch (friar->hdr.type) {
-- 
2.43.5

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Jan Kara]

On Wed 05-03-25 16:33:19, Richard Guy Briggs wrote:
> When no audit rules are in place, fanotify event results are
> unconditionally dropped due to an explicit check for the existence of
> any audit rules.  Given this is a report from another security
> sub-system, allow it to be recorded regardless of the existence of any
> audit rules.
> 
> To test, install and run the fapolicyd daemon with default config.  Then
> as an unprivileged user, create and run a very simple binary that should
> be denied.  Then check for an event with
> 	ausearch -m FANOTIFY -ts recent
> 
> Link: https://issues.redhat.com/browse/RHEL-1367
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>

I don't know enough about security modules to tell whether this is what
admins want or not so that's up to you but:

> -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> -{
> -	if (!audit_dummy_context())
> -		__audit_fanotify(response, friar);
> -}
> -

I think this is going to break compilation with !CONFIG_AUDITSYSCALL &&
CONFIG_FANOTIFY?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Richard Guy Briggs]

On 2025-03-06 16:06, Jan Kara wrote:
> On Wed 05-03-25 16:33:19, Richard Guy Briggs wrote:
> > When no audit rules are in place, fanotify event results are
> > unconditionally dropped due to an explicit check for the existence of
> > any audit rules.  Given this is a report from another security
> > sub-system, allow it to be recorded regardless of the existence of any
> > audit rules.
> > 
> > To test, install and run the fapolicyd daemon with default config.  Then
> > as an unprivileged user, create and run a very simple binary that should
> > be denied.  Then check for an event with
> > 	ausearch -m FANOTIFY -ts recent
> > 
> > Link: https://issues.redhat.com/browse/RHEL-1367
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> 
> I don't know enough about security modules to tell whether this is what
> admins want or not so that's up to you but:
> 
> > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > -{
> > -	if (!audit_dummy_context())
> > -		__audit_fanotify(response, friar);
> > -}
> > -
> 
> I think this is going to break compilation with !CONFIG_AUDITSYSCALL &&
> CONFIG_FANOTIFY?

Why would that break it?  The part of the patch you (prematurely)
deleted takes care of that.

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 0050ef288ab3..d0c6f23503a1 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
 extern void __audit_mmap_fd(int fd, int flags);
 extern void __audit_openat2_how(struct open_how *how);
 extern void __audit_log_kern_module(char *name);
-extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
+extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
 extern void __audit_tk_injoffset(struct timespec64 offset);
 extern void __audit_ntp_log(const struct audit_ntp_data *ad);
 extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,

> 								Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Jan Kara]

On Thu 06-03-25 20:12:23, Richard Guy Briggs wrote:
> On 2025-03-06 16:06, Jan Kara wrote:
> > On Wed 05-03-25 16:33:19, Richard Guy Briggs wrote:
> > > When no audit rules are in place, fanotify event results are
> > > unconditionally dropped due to an explicit check for the existence of
> > > any audit rules.  Given this is a report from another security
> > > sub-system, allow it to be recorded regardless of the existence of any
> > > audit rules.
> > > 
> > > To test, install and run the fapolicyd daemon with default config.  Then
> > > as an unprivileged user, create and run a very simple binary that should
> > > be denied.  Then check for an event with
> > > 	ausearch -m FANOTIFY -ts recent
> > > 
> > > Link: https://issues.redhat.com/browse/RHEL-1367
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > 
> > I don't know enough about security modules to tell whether this is what
> > admins want or not so that's up to you but:
> > 
> > > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > > -{
> > > -	if (!audit_dummy_context())
> > > -		__audit_fanotify(response, friar);
> > > -}
> > > -
> > 
> > I think this is going to break compilation with !CONFIG_AUDITSYSCALL &&
> > CONFIG_FANOTIFY?
> 
> Why would that break it?  The part of the patch you (prematurely)
> deleted takes care of that.

So I'm failing to see how it takes care of that when with
!CONFIG_AUDITSYSCALL kernel/auditsc.c does not get compiled into the kernel.
So what does provide the implementation of audit_fanotify() in that case?
I think you need to provide empty audit_fanotify() inline wrapper for that
case...

								Honza

> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 0050ef288ab3..d0c6f23503a1 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
>  extern void __audit_mmap_fd(int fd, int flags);
>  extern void __audit_openat2_how(struct open_how *how);
>  extern void __audit_log_kern_module(char *name);
> -extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> +extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
>  extern void __audit_tk_injoffset(struct timespec64 offset);
>  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
>  extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> 
> > 								Honza
> > -- 
> > Jan Kara <jack@suse.com>
> > SUSE Labs, CR
> 
> - RGB
> 
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> Upstream IRC: SunRaycer
> Voice: +1.613.860 2354 SMS: +1.613.518.6570
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Richard Guy Briggs]

On 2025-03-07 15:52, Jan Kara wrote:
> On Thu 06-03-25 20:12:23, Richard Guy Briggs wrote:
> > On 2025-03-06 16:06, Jan Kara wrote:
> > > On Wed 05-03-25 16:33:19, Richard Guy Briggs wrote:
> > > > When no audit rules are in place, fanotify event results are
> > > > unconditionally dropped due to an explicit check for the existence of
> > > > any audit rules.  Given this is a report from another security
> > > > sub-system, allow it to be recorded regardless of the existence of any
> > > > audit rules.
> > > > 
> > > > To test, install and run the fapolicyd daemon with default config.  Then
> > > > as an unprivileged user, create and run a very simple binary that should
> > > > be denied.  Then check for an event with
> > > > 	ausearch -m FANOTIFY -ts recent
> > > > 
> > > > Link: https://issues.redhat.com/browse/RHEL-1367
> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > 
> > > I don't know enough about security modules to tell whether this is what
> > > admins want or not so that's up to you but:
> > > 
> > > > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > > > -{
> > > > -	if (!audit_dummy_context())
> > > > -		__audit_fanotify(response, friar);
> > > > -}
> > > > -
> > > 
> > > I think this is going to break compilation with !CONFIG_AUDITSYSCALL &&
> > > CONFIG_FANOTIFY?
> > 
> > Why would that break it?  The part of the patch you (prematurely)
> > deleted takes care of that.
> 
> So I'm failing to see how it takes care of that when with
> !CONFIG_AUDITSYSCALL kernel/auditsc.c does not get compiled into the kernel.
> So what does provide the implementation of audit_fanotify() in that case?
> I think you need to provide empty audit_fanotify() inline wrapper for that
> case...

I'm sorry, I responded too quickly without thinking about your question,
my mistake.  It isn't the prototype that was changed in the
CONFIG_SYSCALL case that is relevant in that case.

There was already in existance in the !CONFIG_AUDITSYSCALL case the
inline wrapper to do that job:

	static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
	{ }

Did I understand correctly this time and does this answer your question?

But you do cause me to notice the case that these notifications will be
dropped when CONFIG_AUDIT && !CONFIG_AUDITSYSCALL.

Thanks for persisting.

> 								Honza
> 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 0050ef288ab3..d0c6f23503a1 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
> >  extern void __audit_mmap_fd(int fd, int flags);
> >  extern void __audit_openat2_how(struct open_how *how);
> >  extern void __audit_log_kern_module(char *name);
> > -extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> > +extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> >  extern void __audit_tk_injoffset(struct timespec64 offset);
> >  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
> >  extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> > 
> > > -- 
> > > Jan Kara <jack@suse.com>
> > > SUSE Labs, CR
> > 
> > - RGB
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Jan Kara]

On Fri 07-03-25 14:19:38, Richard Guy Briggs wrote:
> On 2025-03-07 15:52, Jan Kara wrote:
> > On Thu 06-03-25 20:12:23, Richard Guy Briggs wrote:
> > > On 2025-03-06 16:06, Jan Kara wrote:
> > > > On Wed 05-03-25 16:33:19, Richard Guy Briggs wrote:
> > > > > When no audit rules are in place, fanotify event results are
> > > > > unconditionally dropped due to an explicit check for the existence of
> > > > > any audit rules.  Given this is a report from another security
> > > > > sub-system, allow it to be recorded regardless of the existence of any
> > > > > audit rules.
> > > > > 
> > > > > To test, install and run the fapolicyd daemon with default config.  Then
> > > > > as an unprivileged user, create and run a very simple binary that should
> > > > > be denied.  Then check for an event with
> > > > > 	ausearch -m FANOTIFY -ts recent
> > > > > 
> > > > > Link: https://issues.redhat.com/browse/RHEL-1367
> > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > 
> > > > I don't know enough about security modules to tell whether this is what
> > > > admins want or not so that's up to you but:
> > > > 
> > > > > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > > > > -{
> > > > > -	if (!audit_dummy_context())
> > > > > -		__audit_fanotify(response, friar);
> > > > > -}
> > > > > -
> > > > 
> > > > I think this is going to break compilation with !CONFIG_AUDITSYSCALL &&
> > > > CONFIG_FANOTIFY?
> > > 
> > > Why would that break it?  The part of the patch you (prematurely)
> > > deleted takes care of that.
> > 
> > So I'm failing to see how it takes care of that when with
> > !CONFIG_AUDITSYSCALL kernel/auditsc.c does not get compiled into the kernel.
> > So what does provide the implementation of audit_fanotify() in that case?
> > I think you need to provide empty audit_fanotify() inline wrapper for that
> > case...
> 
> I'm sorry, I responded too quickly without thinking about your question,
> my mistake.  It isn't the prototype that was changed in the
> CONFIG_SYSCALL case that is relevant in that case.
> 
> There was already in existance in the !CONFIG_AUDITSYSCALL case the
> inline wrapper to do that job:
> 
> 	static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> 	{ }
> 
> Did I understand correctly this time and does this answer your question?

Yes, thanks for explanation and sorry for not noticing the second
audit_fanotify() implementation. Somehow I've hasted to a conclusion (based
on customs of parts of kernel I maintain ;)) that you rely on
audit_dummy_context() being constant 0 for !CONFIG_AUDITSYSCALL and thus
__audit_fanotify() call getting compiled out (which would not be the case
after your changes).

Anyway, for the patch feel free to add:

Acked-by: Jan Kara <jack@suse.cz>

> But you do cause me to notice the case that these notifications will be
> dropped when CONFIG_AUDIT && !CONFIG_AUDITSYSCALL.

Glad my blindness helped something ;)

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Paul Moore]

On Mar  5, 2025 Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> When no audit rules are in place, fanotify event results are
> unconditionally dropped due to an explicit check for the existence of
> any audit rules.  Given this is a report from another security
> sub-system, allow it to be recorded regardless of the existence of any
> audit rules.
> 
> To test, install and run the fapolicyd daemon with default config.  Then
> as an unprivileged user, create and run a very simple binary that should
> be denied.  Then check for an event with
> 	ausearch -m FANOTIFY -ts recent
> 
> Link: https://issues.redhat.com/browse/RHEL-1367
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> Acked-by: Jan Kara <jack@suse.cz>
> ---
>  include/linux/audit.h | 8 +-------
>  kernel/auditsc.c      | 2 +-
>  2 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 0050ef288ab3..d0c6f23503a1 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
>  extern void __audit_mmap_fd(int fd, int flags);
>  extern void __audit_openat2_how(struct open_how *how);
>  extern void __audit_log_kern_module(char *name);
> -extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> +extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
>  extern void __audit_tk_injoffset(struct timespec64 offset);
>  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
>  extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> @@ -525,12 +525,6 @@ static inline void audit_log_kern_module(char *name)
>  		__audit_log_kern_module(name);
>  }
>  
> -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> -{
> -	if (!audit_dummy_context())
> -		__audit_fanotify(response, friar);
> -}

It seems like we should at least have an audit_enabled() check, yes?
We've had people complain about audit events being generated when audit
is disabled, any while we don't currently have such a check in place
here, I believe the dummy context check is doing that for us.

  static inline void audit_fanotify(...)
  {
    if (!audit_enabled)
      return;
    __audit_fanotify(...);
  }

>  static inline void audit_tk_injoffset(struct timespec64 offset)
>  {
>  	/* ignore no-op events */
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 0627e74585ce..936825114bae 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2880,7 +2880,7 @@ void __audit_log_kern_module(char *name)
>  	context->type = AUDIT_KERN_MODULE;
>  }
>  
> -void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> +void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
>  {
>  	/* {subj,obj}_trust values are {0,1,2}: no,yes,unknown */
>  	switch (friar->hdr.type) {
> -- 
> 2.43.5

--
paul-moore.com

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Richard Guy Briggs]

On 2025-04-11 14:14, Paul Moore wrote:
> On Mar  5, 2025 Richard Guy Briggs <rgb@redhat.com> wrote:
> > 
> > When no audit rules are in place, fanotify event results are
> > unconditionally dropped due to an explicit check for the existence of
> > any audit rules.  Given this is a report from another security
> > sub-system, allow it to be recorded regardless of the existence of any
> > audit rules.
> > 
> > To test, install and run the fapolicyd daemon with default config.  Then
> > as an unprivileged user, create and run a very simple binary that should
> > be denied.  Then check for an event with
> > 	ausearch -m FANOTIFY -ts recent
> > 
> > Link: https://issues.redhat.com/browse/RHEL-1367
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > Acked-by: Jan Kara <jack@suse.cz>
> > ---
> >  include/linux/audit.h | 8 +-------
> >  kernel/auditsc.c      | 2 +-
> >  2 files changed, 2 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 0050ef288ab3..d0c6f23503a1 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
> >  extern void __audit_mmap_fd(int fd, int flags);
> >  extern void __audit_openat2_how(struct open_how *how);
> >  extern void __audit_log_kern_module(char *name);
> > -extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> > +extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> >  extern void __audit_tk_injoffset(struct timespec64 offset);
> >  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
> >  extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> > @@ -525,12 +525,6 @@ static inline void audit_log_kern_module(char *name)
> >  		__audit_log_kern_module(name);
> >  }
> >  
> > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > -{
> > -	if (!audit_dummy_context())
> > -		__audit_fanotify(response, friar);
> > -}
> 
> It seems like we should at least have an audit_enabled() check, yes?
> We've had people complain about audit events being generated when audit
> is disabled, any while we don't currently have such a check in place
> here, I believe the dummy context check is doing that for us.
> 
>   static inline void audit_fanotify(...)
>   {
>     if (!audit_enabled)
>       return;
>     __audit_fanotify(...);
>   }

That would be consistent with other security events messages.  I was
going through the selinux code to see what it does and I am missing it
if selinux checks with audit_enabled().  Are selinux messages somehow
exempt from audit_enabled()?

> >  static inline void audit_tk_injoffset(struct timespec64 offset)
> >  {
> >  	/* ignore no-op events */
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 0627e74585ce..936825114bae 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2880,7 +2880,7 @@ void __audit_log_kern_module(char *name)
> >  	context->type = AUDIT_KERN_MODULE;
> >  }
> >  
> > -void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > +void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> >  {
> >  	/* {subj,obj}_trust values are {0,1,2}: no,yes,unknown */
> >  	switch (friar->hdr.type) {
> > -- 
> > 2.43.5
> 
> --
> paul-moore.com
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Paul Moore]

On Tue, May 27, 2025 at 9:10 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2025-04-11 14:14, Paul Moore wrote:
> > On Mar  5, 2025 Richard Guy Briggs <rgb@redhat.com> wrote:
> > >
> > > When no audit rules are in place, fanotify event results are
> > > unconditionally dropped due to an explicit check for the existence of
> > > any audit rules.  Given this is a report from another security
> > > sub-system, allow it to be recorded regardless of the existence of any
> > > audit rules.
> > >
> > > To test, install and run the fapolicyd daemon with default config.  Then
> > > as an unprivileged user, create and run a very simple binary that should
> > > be denied.  Then check for an event with
> > >     ausearch -m FANOTIFY -ts recent
> > >
> > > Link: https://issues.redhat.com/browse/RHEL-1367
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > Acked-by: Jan Kara <jack@suse.cz>
> > > ---
> > >  include/linux/audit.h | 8 +-------
> > >  kernel/auditsc.c      | 2 +-
> > >  2 files changed, 2 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > > index 0050ef288ab3..d0c6f23503a1 100644
> > > --- a/include/linux/audit.h
> > > +++ b/include/linux/audit.h
> > > @@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
> > >  extern void __audit_mmap_fd(int fd, int flags);
> > >  extern void __audit_openat2_how(struct open_how *how);
> > >  extern void __audit_log_kern_module(char *name);
> > > -extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> > > +extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> > >  extern void __audit_tk_injoffset(struct timespec64 offset);
> > >  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
> > >  extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> > > @@ -525,12 +525,6 @@ static inline void audit_log_kern_module(char *name)
> > >             __audit_log_kern_module(name);
> > >  }
> > >
> > > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > > -{
> > > -   if (!audit_dummy_context())
> > > -           __audit_fanotify(response, friar);
> > > -}
> >
> > It seems like we should at least have an audit_enabled() check, yes?
> > We've had people complain about audit events being generated when audit
> > is disabled, any while we don't currently have such a check in place
> > here, I believe the dummy context check is doing that for us.
> >
> >   static inline void audit_fanotify(...)
> >   {
> >     if (!audit_enabled)
> >       return;
> >     __audit_fanotify(...);
> >   }
>
> That would be consistent with other security events messages.  I was
> going through the selinux code to see what it does and I am missing it
> if selinux checks with audit_enabled().  Are selinux messages somehow
> exempt from audit_enabled()?

There are likely a number of callers in the kernel that don't have
audit_enabled() checks, some are probably bugs, others probably
intentional; I wouldn't worry too much about what one subsystem does
when deciding what to do for another.  In the case of fanotify, I
suspect the right thing to do is add an audit_enabled() check since it
is already doing an audit_dummy_context() check.  To be clear, there
may be some cases where we do an audit_dummy_context() check and doing
an audit_enabled() check would be wrong, but I don't believe that is
the case with fanotify.

-- 
paul-moore.com

Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Richard Guy Briggs]

On 2025-05-29 20:01, Paul Moore wrote:
> On Tue, May 27, 2025 at 9:10 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2025-04-11 14:14, Paul Moore wrote:
> > > On Mar  5, 2025 Richard Guy Briggs <rgb@redhat.com> wrote:
> > > >
> > > > When no audit rules are in place, fanotify event results are
> > > > unconditionally dropped due to an explicit check for the existence of
> > > > any audit rules.  Given this is a report from another security
> > > > sub-system, allow it to be recorded regardless of the existence of any
> > > > audit rules.
> > > >
> > > > To test, install and run the fapolicyd daemon with default config.  Then
> > > > as an unprivileged user, create and run a very simple binary that should
> > > > be denied.  Then check for an event with
> > > >     ausearch -m FANOTIFY -ts recent
> > > >
> > > > Link: https://issues.redhat.com/browse/RHEL-1367
> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > Acked-by: Jan Kara <jack@suse.cz>
> > > > ---
> > > >  include/linux/audit.h | 8 +-------
> > > >  kernel/auditsc.c      | 2 +-
> > > >  2 files changed, 2 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > > > index 0050ef288ab3..d0c6f23503a1 100644
> > > > --- a/include/linux/audit.h
> > > > +++ b/include/linux/audit.h
> > > > @@ -418,7 +418,7 @@ extern void __audit_log_capset(const struct cred *new, const struct cred *old);
> > > >  extern void __audit_mmap_fd(int fd, int flags);
> > > >  extern void __audit_openat2_how(struct open_how *how);
> > > >  extern void __audit_log_kern_module(char *name);
> > > > -extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> > > > +extern void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
> > > >  extern void __audit_tk_injoffset(struct timespec64 offset);
> > > >  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
> > > >  extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> > > > @@ -525,12 +525,6 @@ static inline void audit_log_kern_module(char *name)
> > > >             __audit_log_kern_module(name);
> > > >  }
> > > >
> > > > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > > > -{
> > > > -   if (!audit_dummy_context())
> > > > -           __audit_fanotify(response, friar);
> > > > -}
> > >
> > > It seems like we should at least have an audit_enabled() check, yes?
> > > We've had people complain about audit events being generated when audit
> > > is disabled, any while we don't currently have such a check in place
> > > here, I believe the dummy context check is doing that for us.
> > >
> > >   static inline void audit_fanotify(...)
> > >   {
> > >     if (!audit_enabled)
> > >       return;
> > >     __audit_fanotify(...);
> > >   }
> >
> > That would be consistent with other security events messages.  I was
> > going through the selinux code to see what it does and I am missing it
> > if selinux checks with audit_enabled().  Are selinux messages somehow
> > exempt from audit_enabled()?
> 
> There are likely a number of callers in the kernel that don't have
> audit_enabled() checks, some are probably bugs, others probably
> intentional; I wouldn't worry too much about what one subsystem does
> when deciding what to do for another.  In the case of fanotify, I
> suspect the right thing to do is add an audit_enabled() check since it
> is already doing an audit_dummy_context() check.  To be clear, there
> may be some cases where we do an audit_dummy_context() check and doing
> an audit_enabled() check would be wrong, but I don't believe that is
> the case with fanotify.

I totally dropped the ball on this.  I had it respun, tested and
documented, ready to go May 28th and had a note that I'd sent it
complete with Message-ID and I find no evidence it was ever sent.
Re-based, re-tested, sending.

> paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570

[PATCH v1 2/2] audit: record AUDIT_ANOM_* events regardless of presence of rules

[Richard Guy Briggs]

When no audit rules are in place, AUDIT_ANOM_{LINK,CREAT} events
reported in audit_log_path_denied() are unconditionally dropped due to
an explicit check for the existence of any audit rules.  Given this is a
report of a security violation, allow it to be recorded regardless of
the existence of any audit rules.

To test,
	mkdir -p /root/tmp
	chmod 1777 /root/tmp
	touch /root/tmp/test.txt
	useradd test
	chown test /root/tmp/test.txt
	{echo C0644 12 test.txt; printf 'hello\ntest1\n'; printf \\000;} | \
		scp -t /root/tmp
Check with
	ausearch -m ANOM_CREAT -ts recent

Link: https://issues.redhat.com/browse/RHEL-9065
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 53e3bddcc327..0cf2827882fc 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2285,7 +2285,7 @@ void audit_log_path_denied(int type, const char *operation)
 {
 	struct audit_buffer *ab;
 
-	if (!audit_enabled || audit_dummy_context())
+	if (!audit_enabled)
 		return;
 
 	/* Generate log with subject, operation, outcome. */
-- 
2.43.5

Re: [PATCH v1 2/2] audit: record AUDIT_ANOM_* events regardless of presence of rules

[Paul Moore]

On Mar  5, 2025 Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> When no audit rules are in place, AUDIT_ANOM_{LINK,CREAT} events
> reported in audit_log_path_denied() are unconditionally dropped due to
> an explicit check for the existence of any audit rules.  Given this is a
> report of a security violation, allow it to be recorded regardless of
> the existence of any audit rules.
> 
> To test,
> 	mkdir -p /root/tmp
> 	chmod 1777 /root/tmp
> 	touch /root/tmp/test.txt
> 	useradd test
> 	chown test /root/tmp/test.txt
> 	{echo C0644 12 test.txt; printf 'hello\ntest1\n'; printf \\000;} | \
> 		scp -t /root/tmp
> Check with
> 	ausearch -m ANOM_CREAT -ts recent
> 
> Link: https://issues.redhat.com/browse/RHEL-9065
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks okay to me, merged into audit/dev, thanks!

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 53e3bddcc327..0cf2827882fc 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2285,7 +2285,7 @@ void audit_log_path_denied(int type, const char *operation)
>  {
>  	struct audit_buffer *ab;
>  
> -	if (!audit_enabled || audit_dummy_context())
> +	if (!audit_enabled)
>  		return;
>  
>  	/* Generate log with subject, operation, outcome. */
> -- 
> 2.43.5

--
paul-moore.com