| Message ID | 79fcf72ea442eeede53ed5e6de567f8df8ef7d83.1670606054.git.rgb@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fanotify: Allow user space to pass back additional audit info | expand |
On Mon, Dec 12, 2022 at 9:06 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > This patch passes the full response so that the audit function can use all > of it. The audit function was updated to log the additional information in > the AUDIT_FANOTIFY record. > > Currently the only type of fanotify info that is defined is an audit > rule number, but convert it to hex encoding to future-proof the field. > Hex encoding suggested by Paul Moore <paul@paul-moore.com>. > > Sample records: > 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=3F subj_trust=2 obj_trust=2 > > Suggested-by: Steve Grubb <sgrubb@redhat.com> > Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > fs/notify/fanotify/fanotify.c | 3 ++- > include/linux/audit.h | 9 +++++---- > kernel/auditsc.c | 25 ++++++++++++++++++++++--- > 3 files changed, 29 insertions(+), 8 deletions(-) ... > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index d1fb821de104..8d523066d81f 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -64,6 +64,7 @@ > #include <uapi/linux/limits.h> > #include <uapi/linux/netfilter/nf_tables.h> > #include <uapi/linux/openat2.h> // struct open_how > +#include <uapi/linux/fanotify.h> > > #include "audit.h" > > @@ -2877,10 +2878,28 @@ void __audit_log_kern_module(char *name) > context->type = AUDIT_KERN_MODULE; > } > > -void __audit_fanotify(u32 response) > +void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar) > { > - audit_log(audit_context(), GFP_KERNEL, > - AUDIT_FANOTIFY, "resp=%u", response); > + struct audit_context *ctx = audit_context(); > + struct audit_buffer *ab; > + char numbuf[12]; > + > + if (friar->hdr.type == FAN_RESPONSE_INFO_NONE) { > + audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, > + "resp=%u fan_type=%u fan_info=3F subj_trust=2 obj_trust=2", > + response, FAN_RESPONSE_INFO_NONE); The fan_info, subj_trust, and obj_trust constant values used here are awfully magic-numbery and not the usual sentinel values one might expect for a "none" operation, e.g. zeros/INT_MAX/etc. I believe a comment here explaining the values would be a good idea. > + return; > + } > + ab = audit_log_start(ctx, GFP_KERNEL, AUDIT_FANOTIFY); > + if (ab) { > + audit_log_format(ab, "resp=%u fan_type=%u fan_info=", > + response, friar->hdr.type); > + snprintf(numbuf, sizeof(numbuf), "%u", friar->rule_number); > + audit_log_n_hex(ab, numbuf, sizeof(numbuf)); It looks like the kernel's printf format string parsing supports %X so why not just use that for now, we can always complicate it later if needed. It would probably also remove the need for the @ab, @numbuf, and @ctx variables. For example: audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, "resp=%u fan_type=%u fan_info=%X subj_trust=%u obj_trust=%u", response, friar->hdr.type, friar->rule_number, friar->subj_trust, friar->obj_trust); Am I missing something? > + audit_log_format(ab, " subj_trust=%u obj_trust=%u", > + friar->subj_trust, friar->obj_trust); > + audit_log_end(ab); > + } > } > > void __audit_tk_injoffset(struct timespec64 offset) > -- > 2.27.0
On 2022-12-20 18:31, Paul Moore wrote: > On Mon, Dec 12, 2022 at 9:06 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > This patch passes the full response so that the audit function can use all > > of it. The audit function was updated to log the additional information in > > the AUDIT_FANOTIFY record. > > > > Currently the only type of fanotify info that is defined is an audit > > rule number, but convert it to hex encoding to future-proof the field. > > Hex encoding suggested by Paul Moore <paul@paul-moore.com>. > > > > Sample records: > > 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=3F subj_trust=2 obj_trust=2 > > > > Suggested-by: Steve Grubb <sgrubb@redhat.com> > > Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > fs/notify/fanotify/fanotify.c | 3 ++- > > include/linux/audit.h | 9 +++++---- > > kernel/auditsc.c | 25 ++++++++++++++++++++++--- > > 3 files changed, 29 insertions(+), 8 deletions(-) > > ... > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > index d1fb821de104..8d523066d81f 100644 > > --- a/kernel/auditsc.c > > +++ b/kernel/auditsc.c > > @@ -64,6 +64,7 @@ > > #include <uapi/linux/limits.h> > > #include <uapi/linux/netfilter/nf_tables.h> > > #include <uapi/linux/openat2.h> // struct open_how > > +#include <uapi/linux/fanotify.h> > > > > #include "audit.h" > > > > @@ -2877,10 +2878,28 @@ void __audit_log_kern_module(char *name) > > context->type = AUDIT_KERN_MODULE; > > } > > > > -void __audit_fanotify(u32 response) > > +void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar) > > { > > - audit_log(audit_context(), GFP_KERNEL, > > - AUDIT_FANOTIFY, "resp=%u", response); > > + struct audit_context *ctx = audit_context(); > > + struct audit_buffer *ab; > > + char numbuf[12]; > > + > > + if (friar->hdr.type == FAN_RESPONSE_INFO_NONE) { > > + audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, > > + "resp=%u fan_type=%u fan_info=3F subj_trust=2 obj_trust=2", > > + response, FAN_RESPONSE_INFO_NONE); > > The fan_info, subj_trust, and obj_trust constant values used here are > awfully magic-numbery and not the usual sentinel values one might > expect for a "none" operation, e.g. zeros/INT_MAX/etc. I believe a > comment here explaining the values would be a good idea. Ack. I'll add a comment. I would have preferred zero for default of unset, but Steve requested 0/1/2 no/yes/unknown. > > + return; > > + } > > + ab = audit_log_start(ctx, GFP_KERNEL, AUDIT_FANOTIFY); > > + if (ab) { > > + audit_log_format(ab, "resp=%u fan_type=%u fan_info=", > > + response, friar->hdr.type); > > + snprintf(numbuf, sizeof(numbuf), "%u", friar->rule_number); > > + audit_log_n_hex(ab, numbuf, sizeof(numbuf)); > > It looks like the kernel's printf format string parsing supports %X so > why not just use that for now, we can always complicate it later if > needed. It would probably also remove the need for the @ab, @numbuf, > and @ctx variables. For example: > > audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, > "resp=%u fan_type=%u fan_info=%X subj_trust=%u obj_trust=%u", > response, friar->hdr.type, friar->rule_number, > friar->subj_trust, friar->obj_trust); > > Am I missing something? No, I am. Thank you, that's much cleaner. > > + audit_log_format(ab, " subj_trust=%u obj_trust=%u", > > + friar->subj_trust, friar->obj_trust); > > + audit_log_end(ab); > > + } > > } > > > > void __audit_tk_injoffset(struct timespec64 offset) > > -- > > 2.27.0 > > -- > paul-moore.com > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://listman.redhat.com/mailman/listinfo/linux-audit > - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
On Thu, Dec 22, 2022 at 3:42 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2022-12-20 18:31, Paul Moore wrote: > > On Mon, Dec 12, 2022 at 9:06 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > > > This patch passes the full response so that the audit function can use all > > > of it. The audit function was updated to log the additional information in > > > the AUDIT_FANOTIFY record. > > > > > > Currently the only type of fanotify info that is defined is an audit > > > rule number, but convert it to hex encoding to future-proof the field. > > > Hex encoding suggested by Paul Moore <paul@paul-moore.com>. > > > > > > Sample records: > > > 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=3F subj_trust=2 obj_trust=2 > > > > > > Suggested-by: Steve Grubb <sgrubb@redhat.com> > > > Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > --- > > > fs/notify/fanotify/fanotify.c | 3 ++- > > > include/linux/audit.h | 9 +++++---- > > > kernel/auditsc.c | 25 ++++++++++++++++++++++--- > > > 3 files changed, 29 insertions(+), 8 deletions(-) > > > > ... > > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > > index d1fb821de104..8d523066d81f 100644 > > > --- a/kernel/auditsc.c > > > +++ b/kernel/auditsc.c > > > @@ -64,6 +64,7 @@ > > > #include <uapi/linux/limits.h> > > > #include <uapi/linux/netfilter/nf_tables.h> > > > #include <uapi/linux/openat2.h> // struct open_how > > > +#include <uapi/linux/fanotify.h> > > > > > > #include "audit.h" > > > > > > @@ -2877,10 +2878,28 @@ void __audit_log_kern_module(char *name) > > > context->type = AUDIT_KERN_MODULE; > > > } > > > > > > -void __audit_fanotify(u32 response) > > > +void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar) > > > { > > > - audit_log(audit_context(), GFP_KERNEL, > > > - AUDIT_FANOTIFY, "resp=%u", response); > > > + struct audit_context *ctx = audit_context(); > > > + struct audit_buffer *ab; > > > + char numbuf[12]; > > > + > > > + if (friar->hdr.type == FAN_RESPONSE_INFO_NONE) { > > > + audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, > > > + "resp=%u fan_type=%u fan_info=3F subj_trust=2 obj_trust=2", > > > + response, FAN_RESPONSE_INFO_NONE); > > > > The fan_info, subj_trust, and obj_trust constant values used here are > > awfully magic-numbery and not the usual sentinel values one might > > expect for a "none" operation, e.g. zeros/INT_MAX/etc. I believe a > > comment here explaining the values would be a good idea. > > Ack. I'll add a comment. I would have preferred zero for default of > unset, but Steve requested 0/1/2 no/yes/unknown. Yeah, if they were zeros I don't think we would need to comment on them as zeros for unset/unknown/invalid is rather common, 2 ... not so much.
Hello,
Sorry to take so long. Holidays and kernel build problems. However, I have
built a kernel with these patches. I only have 2 comments. When I use an
application that expected the old API, meaning it simply does:
response.fd = metadata->fd;
response.response = reply;
close(metadata->fd);
write(fd, &response, sizeof(struct fanotify_response));
I get access denials. Every time. If the program is using the new API and
sets FAN_INFO, then it works as expected. I'll do some more testing but I
think there is something wrong in the compatibility path.
On Monday, December 12, 2022 9:06:11 AM EST Richard Guy Briggs wrote:
> This patch passes the full response so that the audit function can use all
> of it. The audit function was updated to log the additional information in
> the AUDIT_FANOTIFY record.
What I'm seeing is:
type=FANOTIFY msg=audit(01/09/2023 18:43:16.306:366) : resp=deny fan_type=1
fan_info=313300000000000000000000 subj_trust=0 obj_trust=0
Where fan_info was supposed to be 13 decimal. More below...
> Currently the only type of fanotify info that is defined is an audit
> rule number, but convert it to hex encoding to future-proof the field.
> Hex encoding suggested by Paul Moore <paul@paul-moore.com>.
>
> Sample records:
> 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=3F subj_trust=2
> obj_trust=2
>
> Suggested-by: Steve Grubb <sgrubb@redhat.com>
> Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> fs/notify/fanotify/fanotify.c | 3 ++-
> include/linux/audit.h | 9 +++++----
> kernel/auditsc.c | 25 ++++++++++++++++++++++---
> 3 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 24ec1d66d5a8..29bdd99b29fa 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -273,7 +273,8 @@ static int fanotify_get_response(struct fsnotify_group
> *group,
>
> /* Check if the response should be audited */
> if (event->response & FAN_AUDIT)
> - audit_fanotify(event->response & ~FAN_AUDIT);
> + audit_fanotify(event->response & ~FAN_AUDIT,
> + &event->audit_rule);
>
> pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__,
> group, event, ret);
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index d6b7d0c7ce43..31086a72e32a 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -14,6 +14,7 @@
> #include <linux/audit_arch.h>
> #include <uapi/linux/audit.h>
> #include <uapi/linux/netfilter/nf_tables.h>
> +#include <uapi/linux/fanotify.h>
>
> #define AUDIT_INO_UNSET ((unsigned long)-1)
> #define AUDIT_DEV_UNSET ((dev_t)-1)
> @@ -416,7 +417,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);
> +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, @@ -523,10 +524,10 @@ static inline void
> audit_log_kern_module(char *name) __audit_log_kern_module(name);
> }
>
> -static inline void audit_fanotify(u32 response)
> +static inline void audit_fanotify(u32 response, struct
> fanotify_response_info_audit_rule *friar) {
> if (!audit_dummy_context())
> - __audit_fanotify(response);
> + __audit_fanotify(response, friar);
> }
>
> static inline void audit_tk_injoffset(struct timespec64 offset)
> @@ -679,7 +680,7 @@ static inline void audit_log_kern_module(char *name)
> {
> }
>
> -static inline void audit_fanotify(u32 response)
> +static inline void audit_fanotify(u32 response, struct
> fanotify_response_info_audit_rule *friar) { }
>
> static inline void audit_tk_injoffset(struct timespec64 offset)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index d1fb821de104..8d523066d81f 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -64,6 +64,7 @@
> #include <uapi/linux/limits.h>
> #include <uapi/linux/netfilter/nf_tables.h>
> #include <uapi/linux/openat2.h> // struct open_how
> +#include <uapi/linux/fanotify.h>
>
> #include "audit.h"
>
> @@ -2877,10 +2878,28 @@ void __audit_log_kern_module(char *name)
> context->type = AUDIT_KERN_MODULE;
> }
>
> -void __audit_fanotify(u32 response)
> +void __audit_fanotify(u32 response, struct
> fanotify_response_info_audit_rule *friar) {
> - audit_log(audit_context(), GFP_KERNEL,
> - AUDIT_FANOTIFY, "resp=%u", response);
> + struct audit_context *ctx = audit_context();
> + struct audit_buffer *ab;
> + char numbuf[12];
> +
> + if (friar->hdr.type == FAN_RESPONSE_INFO_NONE) {
> + audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY,
> + "resp=%u fan_type=%u fan_info=3F subj_trust=2
obj_trust=2",
> + response, FAN_RESPONSE_INFO_NONE);
> + return;
> + }
> + ab = audit_log_start(ctx, GFP_KERNEL, AUDIT_FANOTIFY);
> + if (ab) {
> + audit_log_format(ab, "resp=%u fan_type=%u fan_info=",
> + response, friar->hdr.type);
> + snprintf(numbuf, sizeof(numbuf), "%u", friar->rule_number);
> + audit_log_n_hex(ab, numbuf, sizeof(numbuf));
I don't think it needs to be converted to ascii and then hexencoded. As Paul
said, probably %X is all we need here.
-Steve
> + audit_log_format(ab, " subj_trust=%u obj_trust=%u",
> + friar->subj_trust, friar->obj_trust);
> + audit_log_end(ab);
> + }
> }
>
> void __audit_tk_injoffset(struct timespec64 offset)
On 2023-01-09 19:06, Steve Grubb wrote: > Hello, > > Sorry to take so long. Holidays and kernel build problems. However, I have > built a kernel with these patches. I only have 2 comments. When I use an > application that expected the old API, meaning it simply does: > > response.fd = metadata->fd; > response.response = reply; > close(metadata->fd); > write(fd, &response, sizeof(struct fanotify_response)); > > I get access denials. Every time. If the program is using the new API and > sets FAN_INFO, then it works as expected. I'll do some more testing but I > think there is something wrong in the compatibility path. I'll have a closer look, because this wasn't the intended behaviour. > On Monday, December 12, 2022 9:06:11 AM EST Richard Guy Briggs wrote: > > This patch passes the full response so that the audit function can use all > > of it. The audit function was updated to log the additional information in > > the AUDIT_FANOTIFY record. > > What I'm seeing is: > > type=FANOTIFY msg=audit(01/09/2023 18:43:16.306:366) : resp=deny fan_type=1 > fan_info=313300000000000000000000 subj_trust=0 obj_trust=0 > > Where fan_info was supposed to be 13 decimal. More below... Well, it *is* 13 decimal, expressed as a hex-encoded string as was requested for future-proofing, albeit longer than necessary... > > Currently the only type of fanotify info that is defined is an audit > > rule number, but convert it to hex encoding to future-proof the field. > > Hex encoding suggested by Paul Moore <paul@paul-moore.com>. > > > > Sample records: > > 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=3F subj_trust=2 > > obj_trust=2 > > > > Suggested-by: Steve Grubb <sgrubb@redhat.com> > > Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > fs/notify/fanotify/fanotify.c | 3 ++- > > include/linux/audit.h | 9 +++++---- > > kernel/auditsc.c | 25 ++++++++++++++++++++++--- > > 3 files changed, 29 insertions(+), 8 deletions(-) > > > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > > index 24ec1d66d5a8..29bdd99b29fa 100644 > > --- a/fs/notify/fanotify/fanotify.c > > +++ b/fs/notify/fanotify/fanotify.c > > @@ -273,7 +273,8 @@ static int fanotify_get_response(struct fsnotify_group > > *group, > > > > /* Check if the response should be audited */ > > if (event->response & FAN_AUDIT) > > - audit_fanotify(event->response & ~FAN_AUDIT); > > + audit_fanotify(event->response & ~FAN_AUDIT, > > + &event->audit_rule); > > > > pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__, > > group, event, ret); > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > index d6b7d0c7ce43..31086a72e32a 100644 > > --- a/include/linux/audit.h > > +++ b/include/linux/audit.h > > @@ -14,6 +14,7 @@ > > #include <linux/audit_arch.h> > > #include <uapi/linux/audit.h> > > #include <uapi/linux/netfilter/nf_tables.h> > > +#include <uapi/linux/fanotify.h> > > > > #define AUDIT_INO_UNSET ((unsigned long)-1) > > #define AUDIT_DEV_UNSET ((dev_t)-1) > > @@ -416,7 +417,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); > > +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, @@ -523,10 +524,10 @@ static inline void > > audit_log_kern_module(char *name) __audit_log_kern_module(name); > > } > > > > -static inline void audit_fanotify(u32 response) > > +static inline void audit_fanotify(u32 response, struct > > fanotify_response_info_audit_rule *friar) { > > if (!audit_dummy_context()) > > - __audit_fanotify(response); > > + __audit_fanotify(response, friar); > > } > > > > static inline void audit_tk_injoffset(struct timespec64 offset) > > @@ -679,7 +680,7 @@ static inline void audit_log_kern_module(char *name) > > { > > } > > > > -static inline void audit_fanotify(u32 response) > > +static inline void audit_fanotify(u32 response, struct > > fanotify_response_info_audit_rule *friar) { } > > > > static inline void audit_tk_injoffset(struct timespec64 offset) > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > index d1fb821de104..8d523066d81f 100644 > > --- a/kernel/auditsc.c > > +++ b/kernel/auditsc.c > > @@ -64,6 +64,7 @@ > > #include <uapi/linux/limits.h> > > #include <uapi/linux/netfilter/nf_tables.h> > > #include <uapi/linux/openat2.h> // struct open_how > > +#include <uapi/linux/fanotify.h> > > > > #include "audit.h" > > > > @@ -2877,10 +2878,28 @@ void __audit_log_kern_module(char *name) > > context->type = AUDIT_KERN_MODULE; > > } > > > > -void __audit_fanotify(u32 response) > > +void __audit_fanotify(u32 response, struct > > fanotify_response_info_audit_rule *friar) { > > - audit_log(audit_context(), GFP_KERNEL, > > - AUDIT_FANOTIFY, "resp=%u", response); > > + struct audit_context *ctx = audit_context(); > > + struct audit_buffer *ab; > > + char numbuf[12]; > > + > > + if (friar->hdr.type == FAN_RESPONSE_INFO_NONE) { > > + audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, > > + "resp=%u fan_type=%u fan_info=3F subj_trust=2 > obj_trust=2", > > + response, FAN_RESPONSE_INFO_NONE); > > + return; > > + } > > + ab = audit_log_start(ctx, GFP_KERNEL, AUDIT_FANOTIFY); > > + if (ab) { > > + audit_log_format(ab, "resp=%u fan_type=%u fan_info=", > > + response, friar->hdr.type); > > + snprintf(numbuf, sizeof(numbuf), "%u", friar->rule_number); > > + audit_log_n_hex(ab, numbuf, sizeof(numbuf)); > > I don't think it needs to be converted to ascii and then hexencoded. As Paul > said, probably %X is all we need here. > > -Steve > > > > + audit_log_format(ab, " subj_trust=%u obj_trust=%u", > > + friar->subj_trust, friar->obj_trust); > > + audit_log_end(ab); > > + } > > } > > > > void __audit_tk_injoffset(struct timespec64 offset) > > > > - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
Hello Richard, On Monday, January 9, 2023 10:08:04 PM EST Richard Guy Briggs wrote: > When I use an application that expected the old API, meaning it simply > does: > > > > response.fd = metadata->fd; > > response.response = reply; > > close(metadata->fd); > > write(fd, &response, sizeof(struct fanotify_response)); > > > > I get access denials. Every time. If the program is using the new API and > > sets FAN_INFO, then it works as expected. I'll do some more testing but I > > think there is something wrong in the compatibility path. > > I'll have a closer look, because this wasn't the intended behaviour. I have done more testing. I think what I saw might have been caused by a stale selinux label (label exists, policy is deleted). With selinux in permissive mode it's all working as expected - both old and new API. -Steve
On 2023-01-10 10:26, Steve Grubb wrote: > Hello Richard, > > On Monday, January 9, 2023 10:08:04 PM EST Richard Guy Briggs wrote: > > When I use an application that expected the old API, meaning it simply > > does: > > > > > > response.fd = metadata->fd; > > > response.response = reply; > > > close(metadata->fd); > > > write(fd, &response, sizeof(struct fanotify_response)); > > > > > > I get access denials. Every time. If the program is using the new API and > > > sets FAN_INFO, then it works as expected. I'll do some more testing but I > > > think there is something wrong in the compatibility path. > > > > I'll have a closer look, because this wasn't the intended behaviour. > > I have done more testing. I think what I saw might have been caused by a > stale selinux label (label exists, policy is deleted). With selinux in > permissive mode it's all working as expected - both old and new API. Ah good, thank you. > -Steve - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 24ec1d66d5a8..29bdd99b29fa 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -273,7 +273,8 @@ static int fanotify_get_response(struct fsnotify_group *group, /* Check if the response should be audited */ if (event->response & FAN_AUDIT) - audit_fanotify(event->response & ~FAN_AUDIT); + audit_fanotify(event->response & ~FAN_AUDIT, + &event->audit_rule); pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__, group, event, ret); diff --git a/include/linux/audit.h b/include/linux/audit.h index d6b7d0c7ce43..31086a72e32a 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -14,6 +14,7 @@ #include <linux/audit_arch.h> #include <uapi/linux/audit.h> #include <uapi/linux/netfilter/nf_tables.h> +#include <uapi/linux/fanotify.h> #define AUDIT_INO_UNSET ((unsigned long)-1) #define AUDIT_DEV_UNSET ((dev_t)-1) @@ -416,7 +417,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); +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, @@ -523,10 +524,10 @@ static inline void audit_log_kern_module(char *name) __audit_log_kern_module(name); } -static inline void audit_fanotify(u32 response) +static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar) { if (!audit_dummy_context()) - __audit_fanotify(response); + __audit_fanotify(response, friar); } static inline void audit_tk_injoffset(struct timespec64 offset) @@ -679,7 +680,7 @@ static inline void audit_log_kern_module(char *name) { } -static inline void audit_fanotify(u32 response) +static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar) { } static inline void audit_tk_injoffset(struct timespec64 offset) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index d1fb821de104..8d523066d81f 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -64,6 +64,7 @@ #include <uapi/linux/limits.h> #include <uapi/linux/netfilter/nf_tables.h> #include <uapi/linux/openat2.h> // struct open_how +#include <uapi/linux/fanotify.h> #include "audit.h" @@ -2877,10 +2878,28 @@ void __audit_log_kern_module(char *name) context->type = AUDIT_KERN_MODULE; } -void __audit_fanotify(u32 response) +void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar) { - audit_log(audit_context(), GFP_KERNEL, - AUDIT_FANOTIFY, "resp=%u", response); + struct audit_context *ctx = audit_context(); + struct audit_buffer *ab; + char numbuf[12]; + + if (friar->hdr.type == FAN_RESPONSE_INFO_NONE) { + audit_log(audit_context(), GFP_KERNEL, AUDIT_FANOTIFY, + "resp=%u fan_type=%u fan_info=3F subj_trust=2 obj_trust=2", + response, FAN_RESPONSE_INFO_NONE); + return; + } + ab = audit_log_start(ctx, GFP_KERNEL, AUDIT_FANOTIFY); + if (ab) { + audit_log_format(ab, "resp=%u fan_type=%u fan_info=", + response, friar->hdr.type); + snprintf(numbuf, sizeof(numbuf), "%u", friar->rule_number); + audit_log_n_hex(ab, numbuf, sizeof(numbuf)); + audit_log_format(ab, " subj_trust=%u obj_trust=%u", + friar->subj_trust, friar->obj_trust); + audit_log_end(ab); + } } void __audit_tk_injoffset(struct timespec64 offset)
This patch passes the full response so that the audit function can use all of it. The audit function was updated to log the additional information in the AUDIT_FANOTIFY record. Currently the only type of fanotify info that is defined is an audit rule number, but convert it to hex encoding to future-proof the field. Hex encoding suggested by Paul Moore <paul@paul-moore.com>. Sample records: 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=3F subj_trust=2 obj_trust=2 Suggested-by: Steve Grubb <sgrubb@redhat.com> Link: https://lore.kernel.org/r/3075502.aeNJFYEL58@x2 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- fs/notify/fanotify/fanotify.c | 3 ++- include/linux/audit.h | 9 +++++---- kernel/auditsc.c | 25 ++++++++++++++++++++++--- 3 files changed, 29 insertions(+), 8 deletions(-)