6:36 a.m.
This patchset is a prototype implementation of the feature requested in GHAK issue #9 [1].
I decided for a simple auxiliary record with just 2 fields (fd and path) that is emitted
whenever we want to record the full path for a file descriptor passed to a syscall (e.g.
the dirfd argument of openat(2)). I choose this approach because for some syscalls there
is more than one file descriptor we might be interested in (a good example is the
renameat(2) syscall).
The motivation for this feature (as I understand it) is to avoid the need to reconstruct
the paths corresponding to the file descriptors passed to syscalls, as this might be
difficult and time consuming or even impossible in case not all of the right sycalls are
being logged. Note that it is always possible to disable these records by simply adding an
exclude filter rule matching all records of type FD_PATH.
At this moment I only implement logging for a single syscall (openat(2)) to keep it
simple. In the final version I plan to add support for other similar syscalls ()mkdirat,
mknodeat, fchownat, ...).
Please let me know if the general approach and the proposed record format make sense to
you so I can improve/complete the solution.
Thanks,
Ondrej
[1] https://github.com/linux-audit/audit-kernel/issues/9
6:36 a.m.
New subject: [RFC PATCH ghak9 1/3] audit: Add AUDIT_FD_PATH auxiliary record type
This new record type is used to log the full path corresponding to some
important file descriptor used in a syscall.
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
include/uapi/linux/audit.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e3eaba84175..d60041ae34a8 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -114,6 +114,7 @@
#define AUDIT_REPLACE 1329 /* Replace auditd if this packet unanswerd */
#define AUDIT_KERN_MODULE 1330 /* Kernel Module events */
#define AUDIT_FANOTIFY 1331 /* Fanotify access decision */
+#define AUDIT_FD_PATH 1334 /* File descriptor path info */
#define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
--
2.17.1
9:51 a.m.
New subject: [RFC PATCH ghak9 1/3] audit: Add AUDIT_FD_PATH auxiliary record type
On 2018-07-12 13:36, Ondrej Mosnacek wrote:
This new record type is used to log the full path corresponding to
some
important file descriptor used in a syscall.
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
include/uapi/linux/audit.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e3eaba84175..d60041ae34a8 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -114,6 +114,7 @@
#define AUDIT_REPLACE 1329 /* Replace auditd if this packet unanswerd */
#define AUDIT_KERN_MODULE 1330 /* Kernel Module events */
#define AUDIT_FANOTIFY 1331 /* Fanotify access decision */
+#define AUDIT_FD_PATH 1334 /* File descriptor path info */
The final message type number depends on other work in flight which may
or may not be accepted first, so don't count on this one being the
final. Having said that, we usually use the next number in sequence
unless there is a hard dependence on another patchset.
This will be the maintainer's job to juggle all these when they are
merged upstream. Unfortunately, that will make more work for the
corresponding user library patches that help identify this record type.
#define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
- RGB
--
Richard Guy Briggs <rgb(a)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
3:19 a.m.
New subject: [RFC PATCH ghak9 1/3] audit: Add AUDIT_FD_PATH auxiliary record type
On Fri, Jul 13, 2018 at 4:53 PM Richard Guy Briggs <rgb(a)redhat.com> wrote:
On 2018-07-12 13:36, Ondrej Mosnacek wrote:
> This new record type is used to log the full path corresponding to some
> important file descriptor used in a syscall.
>
> Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
> ---
> include/uapi/linux/audit.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 4e3eaba84175..d60041ae34a8 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -114,6 +114,7 @@
> #define AUDIT_REPLACE 1329 /* Replace auditd if this packet
unanswerd */
> #define AUDIT_KERN_MODULE 1330 /* Kernel Module events */
> #define AUDIT_FANOTIFY 1331 /* Fanotify access decision */
> +#define AUDIT_FD_PATH 1334 /* File descriptor path info */
The final message type number depends on other work in flight which may
or may not be accepted first, so don't count on this one being the
final. Having said that, we usually use the next number in sequence
unless there is a hard dependence on another patchset.
This will be the maintainer's job to juggle all these when they are
merged upstream. Unfortunately, that will make more work for the
corresponding user library patches that help identify this record type.
Of course, I set it to a different number mainly for easier testing on
my side, I can set it to (previous+1) in the later "production-ready"
patchsets.
> #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
> #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
- RGB
--
Richard Guy Briggs <rgb(a)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
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
6:36 a.m.
New subject: [RFC PATCH ghak9 2/3] audit: Add a function to log the path of an fd
The function logs an FD_PATH record that is associated with the current
syscall. The record associates the given file descriptor with the
current path of the file under it (if it is possible to retrieve such
path). The reader of the log can then logically connect this information
to the syscall arguments from the SYSCALL record (based on the syscall
type).
Record format:
type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
include/linux/audit.h | 10 ++++++++++
kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 9334fbef7bae..95d338bb603a 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -356,6 +356,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_log_kern_module(char *name);
extern void __audit_fanotify(unsigned int response);
+extern void __audit_fd_path(int fd);
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
@@ -458,6 +459,12 @@ static inline void audit_fanotify(unsigned int response)
__audit_fanotify(response);
}
+static inline void audit_fd_path(int fd)
+{
+ if (fd >= 0 && !audit_dummy_context())
+ __audit_fd_path(fd);
+}
+
extern int audit_n_rules;
extern int audit_signals;
#else /* CONFIG_AUDITSYSCALL */
@@ -584,6 +591,9 @@ static inline void audit_log_kern_module(char *name)
static inline void audit_fanotify(unsigned int response)
{ }
+static inline void audit_fd_path(int fd)
+{ }
+
static inline void audit_ptrace(struct task_struct *t)
{ }
#define audit_n_rules 0
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index d762e0b8160e..82dad69213a2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -74,6 +74,8 @@
#include <linux/string.h>
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
+#include <linux/file.h>
+#include <linux/dcache.h>
#include <uapi/linux/limits.h>
#include "audit.h"
@@ -2422,6 +2424,40 @@ void __audit_fanotify(unsigned int response)
AUDIT_FANOTIFY, "resp=%u", response);
}
+void __audit_fd_path(int fd)
+{
+ struct audit_buffer *ab;
+ struct file *file;
+ char *buf, *path;
+
+ if (!audit_enabled)
+ return;
+
+ file = fget_raw(fd);
+ if (!file)
+ return;
+
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (!buf)
+ return;
+
+ path_get(&file->f_path);
+ path = d_absolute_path(&file->f_path, buf, PATH_MAX);
+ path_put(&file->f_path);
+ fput(file);
+ if (!path || IS_ERR(path))
+ goto free_buf;
+
+ ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FD_PATH);
+ if (unlikely(!ab))
+ goto free_buf;
+ audit_log_format(ab, "fd=%i path=", fd);
+ audit_log_untrustedstring(ab, path);
+ audit_log_end(ab);
+free_buf:
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;
--
2.17.1
10:15 a.m.
New subject: [RFC PATCH ghak9 2/3] audit: Add a function to log the path of an fd
On 2018-07-12 13:36, Ondrej Mosnacek wrote:
The function logs an FD_PATH record that is associated with the
current
syscall. The record associates the given file descriptor with the
current path of the file under it (if it is possible to retrieve such
path). The reader of the log can then logically connect this information
to the syscall arguments from the SYSCALL record (based on the syscall
type).
Record format:
type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
include/linux/audit.h | 10 ++++++++++
kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 9334fbef7bae..95d338bb603a 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -356,6 +356,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_log_kern_module(char *name);
extern void __audit_fanotify(unsigned int response);
+extern void __audit_fd_path(int fd);
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
@@ -458,6 +459,12 @@ static inline void audit_fanotify(unsigned int response)
__audit_fanotify(response);
}
+static inline void audit_fd_path(int fd)
+{
+ if (fd >= 0 && !audit_dummy_context())
Isn't an fd of 0 valid?
+ __audit_fd_path(fd);
+}
+
extern int audit_n_rules;
extern int audit_signals;
#else /* CONFIG_AUDITSYSCALL */
@@ -584,6 +591,9 @@ static inline void audit_log_kern_module(char *name)
static inline void audit_fanotify(unsigned int response)
{ }
+static inline void audit_fd_path(int fd)
+{ }
+
static inline void audit_ptrace(struct task_struct *t)
{ }
#define audit_n_rules 0
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index d762e0b8160e..82dad69213a2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -74,6 +74,8 @@
#include <linux/string.h>
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
+#include <linux/file.h>
+#include <linux/dcache.h>
#include <uapi/linux/limits.h>
#include "audit.h"
@@ -2422,6 +2424,40 @@ void __audit_fanotify(unsigned int response)
AUDIT_FANOTIFY, "resp=%u", response);
}
+void __audit_fd_path(int fd)
+{
+ struct audit_buffer *ab;
+ struct file *file;
+ char *buf, *path;
+
+ if (!audit_enabled)
+ return;
+
+ file = fget_raw(fd);
+ if (!file)
+ return;
+
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (!buf)
I think we need an fput(file) here.
+ return;
+
+ path_get(&file->f_path);
+ path = d_absolute_path(&file->f_path, buf, PATH_MAX);
+ path_put(&file->f_path);
+ fput(file);
+ if (!path || IS_ERR(path))
+ goto free_buf;
+
+ ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FD_PATH);
+ if (unlikely(!ab))
+ goto free_buf;
+ audit_log_format(ab, "fd=%i path=", fd);
+ audit_log_untrustedstring(ab, path);
+ audit_log_end(ab);
+free_buf:
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;
- RGB
--
Richard Guy Briggs <rgb(a)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
3:29 a.m.
New subject: [RFC PATCH ghak9 2/3] audit: Add a function to log the path of an fd
On Fri, Jul 13, 2018 at 5:17 PM Richard Guy Briggs <rgb(a)redhat.com> wrote:
On 2018-07-12 13:36, Ondrej Mosnacek wrote:
> The function logs an FD_PATH record that is associated with the current
> syscall. The record associates the given file descriptor with the
> current path of the file under it (if it is possible to retrieve such
> path). The reader of the log can then logically connect this information
> to the syscall arguments from the SYSCALL record (based on the syscall
> type).
>
> Record format:
> type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
>
> Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
> ---
> include/linux/audit.h | 10 ++++++++++
> kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 9334fbef7bae..95d338bb603a 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -356,6 +356,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_log_kern_module(char *name);
> extern void __audit_fanotify(unsigned int response);
> +extern void __audit_fd_path(int fd);
>
> static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> {
> @@ -458,6 +459,12 @@ static inline void audit_fanotify(unsigned int response)
> __audit_fanotify(response);
> }
>
> +static inline void audit_fd_path(int fd)
> +{
> + if (fd >= 0 && !audit_dummy_context())
Isn't an fd of 0 valid?
It is treated as valid by the above condition (it only rejects
negative values), so I'm not sure if you mean "valid" or
"invalid"...
I suppose an fd of 0 is unlikely to be used as dirfd in openat(2) et
al., but in general it is a valid fd and I don't think we should
explicitly exclude it here. The corresponding syscalls' input checks
will already filter out values that are invalid for them.
> + __audit_fd_path(fd);
> +}
> +
> extern int audit_n_rules;
> extern int audit_signals;
> #else /* CONFIG_AUDITSYSCALL */
> @@ -584,6 +591,9 @@ static inline void audit_log_kern_module(char *name)
> static inline void audit_fanotify(unsigned int response)
> { }
>
> +static inline void audit_fd_path(int fd)
> +{ }
> +
> static inline void audit_ptrace(struct task_struct *t)
> { }
> #define audit_n_rules 0
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index d762e0b8160e..82dad69213a2 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -74,6 +74,8 @@
> #include <linux/string.h>
> #include <linux/uaccess.h>
> #include <linux/fsnotify_backend.h>
> +#include <linux/file.h>
> +#include <linux/dcache.h>
> #include <uapi/linux/limits.h>
>
> #include "audit.h"
> @@ -2422,6 +2424,40 @@ void __audit_fanotify(unsigned int response)
> AUDIT_FANOTIFY, "resp=%u", response);
> }
>
> +void __audit_fd_path(int fd)
> +{
> + struct audit_buffer *ab;
> + struct file *file;
> + char *buf, *path;
> +
> + if (!audit_enabled)
> + return;
> +
> + file = fget_raw(fd);
> + if (!file)
> + return;
> +
> + buf = kmalloc(PATH_MAX, GFP_KERNEL);
> + if (!buf)
I think we need an fput(file) here.
Indeed we do, will fix in next revision.
> + return;
> +
> + path_get(&file->f_path);
> + path = d_absolute_path(&file->f_path, buf, PATH_MAX);
> + path_put(&file->f_path);
> + fput(file);
> + if (!path || IS_ERR(path))
> + goto free_buf;
> +
> + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FD_PATH);
> + if (unlikely(!ab))
> + goto free_buf;
> + audit_log_format(ab, "fd=%i path=", fd);
> + audit_log_untrustedstring(ab, path);
> + audit_log_end(ab);
> +free_buf:
> + kfree(buf);
> +}
> +
> static void audit_log_task(struct audit_buffer *ab)
> {
> kuid_t auid, uid;
- RGB
--
Richard Guy Briggs <rgb(a)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
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
12:30 p.m.
New subject: [RFC PATCH ghak9 2/3] audit: Add a function to log the path of an fd
On 2018-07-16 10:29, Ondrej Mosnacek wrote:
On Fri, Jul 13, 2018 at 5:17 PM Richard Guy Briggs
<rgb(a)redhat.com> wrote:
> On 2018-07-12 13:36, Ondrej Mosnacek wrote:
> > The function logs an FD_PATH record that is associated with the current
> > syscall. The record associates the given file descriptor with the
> > current path of the file under it (if it is possible to retrieve such
> > path). The reader of the log can then logically connect this information
> > to the syscall arguments from the SYSCALL record (based on the syscall
> > type).
> >
> > Record format:
> > type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
> > ---
> > include/linux/audit.h | 10 ++++++++++
> > kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 46 insertions(+)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 9334fbef7bae..95d338bb603a 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -356,6 +356,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_log_kern_module(char *name);
> > extern void __audit_fanotify(unsigned int response);
> > +extern void __audit_fd_path(int fd);
> >
> > static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> > {
> > @@ -458,6 +459,12 @@ static inline void audit_fanotify(unsigned int response)
> > __audit_fanotify(response);
> > }
> >
> > +static inline void audit_fd_path(int fd)
> > +{
> > + if (fd >= 0 && !audit_dummy_context())
>
> Isn't an fd of 0 valid?
It is treated as valid by the above condition (it only rejects
negative values), so I'm not sure if you mean "valid" or
"invalid"...
I suppose an fd of 0 is unlikely to be used as dirfd in openat(2) et
al., but in general it is a valid fd and I don't think we should
explicitly exclude it here. The corresponding syscalls' input checks
will already filter out values that are invalid for them.
Sorry, that must have been a brain fart on my part. I must have seen
">" when I first reviewed it, then later saw ">="... so that
should be
fine.
> > + __audit_fd_path(fd);
> > +}
> > +
> > extern int audit_n_rules;
> > extern int audit_signals;
> > #else /* CONFIG_AUDITSYSCALL */
> > @@ -584,6 +591,9 @@ static inline void audit_log_kern_module(char *name)
> > static inline void audit_fanotify(unsigned int response)
> > { }
> >
> > +static inline void audit_fd_path(int fd)
> > +{ }
> > +
> > static inline void audit_ptrace(struct task_struct *t)
> > { }
> > #define audit_n_rules 0
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index d762e0b8160e..82dad69213a2 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -74,6 +74,8 @@
> > #include <linux/string.h>
> > #include <linux/uaccess.h>
> > #include <linux/fsnotify_backend.h>
> > +#include <linux/file.h>
> > +#include <linux/dcache.h>
> > #include <uapi/linux/limits.h>
> >
> > #include "audit.h"
> > @@ -2422,6 +2424,40 @@ void __audit_fanotify(unsigned int response)
> > AUDIT_FANOTIFY, "resp=%u", response);
> > }
> >
> > +void __audit_fd_path(int fd)
> > +{
> > + struct audit_buffer *ab;
> > + struct file *file;
> > + char *buf, *path;
> > +
> > + if (!audit_enabled)
> > + return;
> > +
> > + file = fget_raw(fd);
> > + if (!file)
> > + return;
> > +
> > + buf = kmalloc(PATH_MAX, GFP_KERNEL);
> > + if (!buf)
>
> I think we need an fput(file) here.
Indeed we do, will fix in next revision.
>
> > + return;
> > +
> > + path_get(&file->f_path);
> > + path = d_absolute_path(&file->f_path, buf, PATH_MAX);
> > + path_put(&file->f_path);
> > + fput(file);
> > + if (!path || IS_ERR(path))
> > + goto free_buf;
> > +
> > + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FD_PATH);
> > + if (unlikely(!ab))
> > + goto free_buf;
> > + audit_log_format(ab, "fd=%i path=", fd);
> > + audit_log_untrustedstring(ab, path);
> > + audit_log_end(ab);
> > +free_buf:
> > + kfree(buf);
> > +}
> > +
> > static void audit_log_task(struct audit_buffer *ab)
> > {
> > kuid_t auid, uid;
>
> - RGB
>
> --
> Richard Guy Briggs <rgb(a)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
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
- RGB
--
Richard Guy Briggs <rgb(a)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
11:26 a.m.
New subject: [RFC PATCH ghak9 2/3] audit: Add a function to log the path of an fd
On Thursday, July 12, 2018 7:36:32 AM EDT Ondrej Mosnacek wrote:
The function logs an FD_PATH record that is associated with the
current
syscall. The record associates the given file descriptor with the
current path of the file under it (if it is possible to retrieve such
path). The reader of the log can then logically connect this information
to the syscall arguments from the SYSCALL record (based on the syscall
type).
Record format:
type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
Event looks OK to me. However, do you check for AT_FDCWD? If so, should we
skip generating this record?
-Steve
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
include/linux/audit.h | 10 ++++++++++
kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 9334fbef7bae..95d338bb603a 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -356,6 +356,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_log_kern_module(char *name);
extern void __audit_fanotify(unsigned int response);
+extern void __audit_fd_path(int fd);
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
@@ -458,6 +459,12 @@ static inline void audit_fanotify(unsigned int
response) __audit_fanotify(response);
}
+static inline void audit_fd_path(int fd)
+{
+ if (fd >= 0 && !audit_dummy_context())
+ __audit_fd_path(fd);
+}
+
extern int audit_n_rules;
extern int audit_signals;
#else /* CONFIG_AUDITSYSCALL */
@@ -584,6 +591,9 @@ static inline void audit_log_kern_module(char *name)
static inline void audit_fanotify(unsigned int response)
{ }
+static inline void audit_fd_path(int fd)
+{ }
+
static inline void audit_ptrace(struct task_struct *t)
{ }
#define audit_n_rules 0
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index d762e0b8160e..82dad69213a2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -74,6 +74,8 @@
#include <linux/string.h>
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
+#include <linux/file.h>
+#include <linux/dcache.h>
#include <uapi/linux/limits.h>
#include "audit.h"
@@ -2422,6 +2424,40 @@ void __audit_fanotify(unsigned int response)
AUDIT_FANOTIFY, "resp=%u", response);
}
+void __audit_fd_path(int fd)
+{
+ struct audit_buffer *ab;
+ struct file *file;
+ char *buf, *path;
+
+ if (!audit_enabled)
+ return;
+
+ file = fget_raw(fd);
+ if (!file)
+ return;
+
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (!buf)
+ return;
+
+ path_get(&file->f_path);
+ path = d_absolute_path(&file->f_path, buf, PATH_MAX);
+ path_put(&file->f_path);
+ fput(file);
+ if (!path || IS_ERR(path))
+ goto free_buf;
+
+ ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FD_PATH);
+ if (unlikely(!ab))
+ goto free_buf;
+ audit_log_format(ab, "fd=%i path=", fd);
+ audit_log_untrustedstring(ab, path);
+ audit_log_end(ab);
+free_buf:
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;
3:31 a.m.
New subject: [RFC PATCH ghak9 2/3] audit: Add a function to log the path of an fd
On Sat, Jul 14, 2018 at 6:26 PM Steve Grubb <sgrubb(a)redhat.com> wrote:
On Thursday, July 12, 2018 7:36:32 AM EDT Ondrej Mosnacek wrote:
> The function logs an FD_PATH record that is associated with the current
> syscall. The record associates the given file descriptor with the
> current path of the file under it (if it is possible to retrieve such
> path). The reader of the log can then logically connect this information
> to the syscall arguments from the SYSCALL record (based on the syscall
> type).
>
> Record format:
> type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
Event looks OK to me. However, do you check for AT_FDCWD? If so, should we
skip generating this record?
Yes, AT_FDCWD is defined as -100 and I explicitly reject all negative
file descriptors (I'll add a comment clarifying that).
-Steve
> Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
> ---
> include/linux/audit.h | 10 ++++++++++
> kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 9334fbef7bae..95d338bb603a 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -356,6 +356,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_log_kern_module(char *name);
> extern void __audit_fanotify(unsigned int response);
> +extern void __audit_fd_path(int fd);
>
> static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> {
> @@ -458,6 +459,12 @@ static inline void audit_fanotify(unsigned int
> response) __audit_fanotify(response);
> }
>
> +static inline void audit_fd_path(int fd)
> +{
> + if (fd >= 0 && !audit_dummy_context())
> + __audit_fd_path(fd);
> +}
> +
> extern int audit_n_rules;
> extern int audit_signals;
> #else /* CONFIG_AUDITSYSCALL */
> @@ -584,6 +591,9 @@ static inline void audit_log_kern_module(char *name)
> static inline void audit_fanotify(unsigned int response)
> { }
>
> +static inline void audit_fd_path(int fd)
> +{ }
> +
> static inline void audit_ptrace(struct task_struct *t)
> { }
> #define audit_n_rules 0
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index d762e0b8160e..82dad69213a2 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -74,6 +74,8 @@
> #include <linux/string.h>
> #include <linux/uaccess.h>
> #include <linux/fsnotify_backend.h>
> +#include <linux/file.h>
> +#include <linux/dcache.h>
> #include <uapi/linux/limits.h>
>
> #include "audit.h"
> @@ -2422,6 +2424,40 @@ void __audit_fanotify(unsigned int response)
> AUDIT_FANOTIFY, "resp=%u", response);
> }
>
> +void __audit_fd_path(int fd)
> +{
> + struct audit_buffer *ab;
> + struct file *file;
> + char *buf, *path;
> +
> + if (!audit_enabled)
> + return;
> +
> + file = fget_raw(fd);
> + if (!file)
> + return;
> +
> + buf = kmalloc(PATH_MAX, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + path_get(&file->f_path);
> + path = d_absolute_path(&file->f_path, buf, PATH_MAX);
> + path_put(&file->f_path);
> + fput(file);
> + if (!path || IS_ERR(path))
> + goto free_buf;
> +
> + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FD_PATH);
> + if (unlikely(!ab))
> + goto free_buf;
> + audit_log_format(ab, "fd=%i path=", fd);
> + audit_log_untrustedstring(ab, path);
> + audit_log_end(ab);
> +free_buf:
> + kfree(buf);
> +}
> +
> static void audit_log_task(struct audit_buffer *ab)
> {
> kuid_t auid, uid;
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
6:36 a.m.
New subject: [RFC PATCH ghak9 3/3] [WIP] fs: Add audit_fd_path() calls to syscall handlers
So far only add one to openat(2) for testing purposes.
Example records:
type=PROCTITLE msg=audit(07/04/18 15:28:41.808:76) : proctitle=/bin/bash
./openat-audit-test.sh
type=PATH msg=audit(07/04/18 15:28:41.808:76) : item=0 name=b inode=2154 dev=00:1a
mode=dir,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:tmpfs_t:s0
nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
type=CWD msg=audit(07/04/18 15:28:41.808:76) : cwd=/root/Dokumenty/Kernel
type=SYSCALL msg=audit(07/04/18 15:28:41.808:76) : arch=x86_64 syscall=openat success=yes
exit=6 a0=0x5 a1=0x55bad2616348
a2=O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC a3=0x0 items=1 ppid=594
pid=630 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
fsgid=root tty=(none) ses=1 comm=find exe=/usr/bin/find subj=system_u:system_r:kernel_t:s0
key=(null)
type=FD_PATH msg=audit(07/04/18 15:28:41.808:76) : fd=5 path=/tmp/a
----
type=PROCTITLE msg=audit(07/04/18 15:28:41.808:77) : proctitle=/bin/bash
./openat-audit-test.sh
type=PATH msg=audit(07/04/18 15:28:41.808:77) : item=0 name=c inode=2155 dev=00:1a
mode=dir,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:tmpfs_t:s0
nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
type=CWD msg=audit(07/04/18 15:28:41.808:77) : cwd=/root/Dokumenty/Kernel
type=SYSCALL msg=audit(07/04/18 15:28:41.808:77) : arch=x86_64 syscall=openat success=yes
exit=6 a0=0x7 a1=0x55bad260e328
a2=O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC a3=0x0 items=1 ppid=594
pid=630 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
fsgid=root tty=(none) ses=1 comm=find exe=/usr/bin/find subj=system_u:system_r:kernel_t:s0
key=(null)
type=FD_PATH msg=audit(07/04/18 15:28:41.808:77) : fd=7 path=/tmp/a/b
----
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
fs/open.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/open.c b/fs/open.c
index d0e955b558ad..7fa326fc025d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1125,6 +1125,8 @@ SYSCALL_DEFINE4(openat, int, dfd, const char __user *, filename,
int, flags,
if (force_o_largefile())
flags |= O_LARGEFILE;
+ audit_fd_path(dfd);
+
return do_sys_open(dfd, filename, flags, mode);
}
--
2.17.1
10:20 a.m.
New subject: [RFC PATCH ghak9 3/3] [WIP] fs: Add audit_fd_path() calls to syscall handlers
On 2018-07-12 13:36, Ondrej Mosnacek wrote:
So far only add one to openat(2) for testing purposes.
Example records:
type=PROCTITLE msg=audit(07/04/18 15:28:41.808:76) : proctitle=/bin/bash
./openat-audit-test.sh
type=PATH msg=audit(07/04/18 15:28:41.808:76) : item=0 name=b inode=2154 dev=00:1a
mode=dir,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:tmpfs_t:s0
nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
type=CWD msg=audit(07/04/18 15:28:41.808:76) : cwd=/root/Dokumenty/Kernel
type=SYSCALL msg=audit(07/04/18 15:28:41.808:76) : arch=x86_64 syscall=openat success=yes
exit=6 a0=0x5 a1=0x55bad2616348
a2=O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC a3=0x0 items=1 ppid=594
pid=630 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
fsgid=root tty=(none) ses=1 comm=find exe=/usr/bin/find subj=system_u:system_r:kernel_t:s0
key=(null)
type=FD_PATH msg=audit(07/04/18 15:28:41.808:76) : fd=5 path=/tmp/a
----
type=PROCTITLE msg=audit(07/04/18 15:28:41.808:77) : proctitle=/bin/bash
./openat-audit-test.sh
type=PATH msg=audit(07/04/18 15:28:41.808:77) : item=0 name=c inode=2155 dev=00:1a
mode=dir,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:tmpfs_t:s0
nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
type=CWD msg=audit(07/04/18 15:28:41.808:77) : cwd=/root/Dokumenty/Kernel
type=SYSCALL msg=audit(07/04/18 15:28:41.808:77) : arch=x86_64 syscall=openat success=yes
exit=6 a0=0x7 a1=0x55bad260e328
a2=O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC a3=0x0 items=1 ppid=594
pid=630 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
fsgid=root tty=(none) ses=1 comm=find exe=/usr/bin/find subj=system_u:system_r:kernel_t:s0
key=(null)
type=FD_PATH msg=audit(07/04/18 15:28:41.808:77) : fd=7 path=/tmp/a/b
----
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
This patchset looks encouraging.
---
fs/open.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/open.c b/fs/open.c
index d0e955b558ad..7fa326fc025d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1125,6 +1125,8 @@ SYSCALL_DEFINE4(openat, int, dfd, const char __user *, filename,
int, flags,
if (force_o_largefile())
flags |= O_LARGEFILE;
+ audit_fd_path(dfd);
+
return do_sys_open(dfd, filename, flags, mode);
}
- RGB
--
Richard Guy Briggs <rgb(a)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
3:41 p.m.
On Thu, Jul 12, 2018 at 7:36 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
This patchset is a prototype implementation of the feature requested
in GHAK issue #9 [1]. I decided for a simple auxiliary record with just 2 fields (fd and
path) that is emitted whenever we want to record the full path for a file descriptor
passed to a syscall (e.g. the dirfd argument of openat(2)). I choose this approach because
for some syscalls there is more than one file descriptor we might be interested in (a good
example is the renameat(2) syscall).
The motivation for this feature (as I understand it) is to avoid the need to reconstruct
the paths corresponding to the file descriptors passed to syscalls, as this might be
difficult and time consuming or even impossible in case not all of the right sycalls are
being logged. Note that it is always possible to disable these records by simply adding an
exclude filter rule matching all records of type FD_PATH.
At this moment I only implement logging for a single syscall (openat(2)) to keep it
simple. In the final version I plan to add support for other similar syscalls ()mkdirat,
mknodeat, fchownat, ...).
Please let me know if the general approach and the proposed record format make sense to
you so I can improve/complete the solution.
Thanks,
Ondrej
[1] https://github.com/linux-audit/audit-kernel/issues/9
While I recognize that the GH issue did raise the idea of possibly
creating a new record type, looking at these patches I'm not sure a
new record type is justified, I think reusing the existing PATH record
type would be more beneficial. I recognize that this proposed FD_PATH
record also contains the file descriptor number, but that information
should also be contained in the associated SYSCALL record and arguably
the fd number is only useful if you are logging the SYSCALL
information.
Can you explain what advantage the FS_PATH record type has over
reusing the existing PATH record? I know you mention multiple
fd/paths as in the case of renameat(2), but we already have to deal
with this in the non-at rename(2) case.
--
paul moore
www.paul-moore.com
5:11 a.m.
On Wed, Jul 18, 2018 at 10:41 PM Paul Moore <paul(a)paul-moore.com> wrote:
On Thu, Jul 12, 2018 at 7:36 AM Ondrej Mosnacek
<omosnace(a)redhat.com> wrote:
> This patchset is a prototype implementation of the feature requested in GHAK issue
#9 [1]. I decided for a simple auxiliary record with just 2 fields (fd and path) that is
emitted whenever we want to record the full path for a file descriptor passed to a syscall
(e.g. the dirfd argument of openat(2)). I choose this approach because for some syscalls
there is more than one file descriptor we might be interested in (a good example is the
renameat(2) syscall).
>
> The motivation for this feature (as I understand it) is to avoid the need to
reconstruct the paths corresponding to the file descriptors passed to syscalls, as this
might be difficult and time consuming or even impossible in case not all of the right
sycalls are being logged. Note that it is always possible to disable these records by
simply adding an exclude filter rule matching all records of type FD_PATH.
>
> At this moment I only implement logging for a single syscall (openat(2)) to keep it
simple. In the final version I plan to add support for other similar syscalls ()mkdirat,
mknodeat, fchownat, ...).
>
> Please let me know if the general approach and the proposed record format make sense
to you so I can improve/complete the solution.
>
> Thanks,
> Ondrej
>
> [1] https://github.com/linux-audit/audit-kernel/issues/9
While I recognize that the GH issue did raise the idea of possibly
creating a new record type, looking at these patches I'm not sure a
new record type is justified, I think reusing the existing PATH record
type would be more beneficial. I recognize that this proposed FD_PATH
record also contains the file descriptor number, but that information
should also be contained in the associated SYSCALL record and arguably
the fd number is only useful if you are logging the SYSCALL
information.
To be honest, I'm not sure what is the exact semantic of the PATH
record... Is it simply "log information about files that the syscall
somehow touched"? Or "log information about any string syscall
argument that represents a file path"? In the PATH record samples I
have seen, the name=... field sometimes contains just the last segment
of the path, other times it contains the full path (huh?). When we log
the full path in PATH, then I guess the FD_PATH record would be
(almost) redundant, but it seems that whenever openat(2) is called
with dirfd != AT_FDCWD, PATH name=... contains just the relative path
supplied to the syscall (thus FD_PATH actually provides the missing
directory path).
So... assuming we would want to provide the missing information in the
existing PATH record, how should it look like? Should the name=...
field simply always contain the full path? Should there be another
PATH record for the directory? If so, how do we indicate the
association between the two PATH records?
Can you explain what advantage the FS_PATH record type has over
reusing the existing PATH record? I know you mention multiple
fd/paths as in the case of renameat(2), but we already have to deal
with this in the non-at rename(2) case.
As I said above, I don't fully understand the PATH record so I can't
compare them right now. Multiple fd arguments was meant as more of a
justification for the presence of the fd=... field in the new FD_PATH
record.
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
3:49 p.m.
On Fri, Jul 20, 2018 at 6:12 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
On Wed, Jul 18, 2018 at 10:41 PM Paul Moore
<paul(a)paul-moore.com> wrote:
> On Thu, Jul 12, 2018 at 7:36 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
> > This patchset is a prototype implementation of the feature requested in GHAK
issue #9 [1]. I decided for a simple auxiliary record with just 2 fields (fd and path)
that is emitted whenever we want to record the full path for a file descriptor passed to a
syscall (e.g. the dirfd argument of openat(2)). I choose this approach because for some
syscalls there is more than one file descriptor we might be interested in (a good example
is the renameat(2) syscall).
> >
> > The motivation for this feature (as I understand it) is to avoid the need to
reconstruct the paths corresponding to the file descriptors passed to syscalls, as this
might be difficult and time consuming or even impossible in case not all of the right
sycalls are being logged. Note that it is always possible to disable these records by
simply adding an exclude filter rule matching all records of type FD_PATH.
> >
> > At this moment I only implement logging for a single syscall (openat(2)) to
keep it simple. In the final version I plan to add support for other similar syscalls
()mkdirat, mknodeat, fchownat, ...).
> >
> > Please let me know if the general approach and the proposed record format make
sense to you so I can improve/complete the solution.
> >
> > Thanks,
> > Ondrej
> >
> > [1] https://github.com/linux-audit/audit-kernel/issues/9
>
> While I recognize that the GH issue did raise the idea of possibly
> creating a new record type, looking at these patches I'm not sure a
> new record type is justified, I think reusing the existing PATH record
> type would be more beneficial. I recognize that this proposed FD_PATH
> record also contains the file descriptor number, but that information
> should also be contained in the associated SYSCALL record and arguably
> the fd number is only useful if you are logging the SYSCALL
> information.
To be honest, I'm not sure what is the exact semantic of the PATH
record... Is it simply "log information about files that the syscall
somehow touched"? Or "log information about any string syscall
argument that represents a file path"?
The PATH record exists to record path name information about files
related to the audit event, depending on the circumstances, both
relative and absolute paths could be expected. Look at the functions,
and callers, that add to the audit_context->names_list to get a better
understanding of where the path name information is collected. I
doubt you will be surprised at this, but fair warning, it's a bit ugly
in places.
In the PATH record samples I
have seen, the name=... field sometimes contains just the last segment
of the path, other times it contains the full path (huh?).
Depending on the syscall, the syscall arguments, and the "nametype" of
the PATH record you could see either a full or partial path. In the
cases of a partial path, there *should* be additional PATH records
which can help recreate a full path to the file for that particular
event instance.
When we log
the full path in PATH, then I guess the FD_PATH record would be
(almost) redundant, but it seems that whenever openat(2) is called
with dirfd != AT_FDCWD, PATH name=... contains just the relative path
supplied to the syscall (thus FD_PATH actually provides the missing
directory path).
So... assuming we would want to provide the missing information in the
existing PATH record, how should it look like? Should the name=...
field simply always contain the full path? Should there be another
PATH record for the directory?
The only real hard requirement is that there be enough to information
to determine the file full path(s) for any given audit event using
one, or a combination, of PATH records associated with the event.
If so, how do we indicate the association between the two PATH
records?
Look at how the "nametype" field is used.
> Can you explain what advantage the FS_PATH record type has over
> reusing the existing PATH record? I know you mention multiple
> fd/paths as in the case of renameat(2), but we already have to deal
> with this in the non-at rename(2) case.
As I said above, I don't fully understand the PATH record so I can't
compare them right now. Multiple fd arguments was meant as more of a
justification for the presence of the fd=... field in the new FD_PATH
record.
I suggest taking some time to better understand how the current file
path auditing works and then revisiting this patchset.
--
paul moore
www.paul-moore.com
9:12 a.m.
On Mon, Jul 23, 2018 at 10:49 PM Paul Moore <paul(a)paul-moore.com> wrote:
On Fri, Jul 20, 2018 at 6:12 AM Ondrej Mosnacek
<omosnace(a)redhat.com> wrote:
> On Wed, Jul 18, 2018 at 10:41 PM Paul Moore <paul(a)paul-moore.com> wrote:
> > On Thu, Jul 12, 2018 at 7:36 AM Ondrej Mosnacek <omosnace(a)redhat.com>
wrote:
> > > This patchset is a prototype implementation of the feature requested in
GHAK issue #9 [1]. I decided for a simple auxiliary record with just 2 fields (fd and
path) that is emitted whenever we want to record the full path for a file descriptor
passed to a syscall (e.g. the dirfd argument of openat(2)). I choose this approach because
for some syscalls there is more than one file descriptor we might be interested in (a good
example is the renameat(2) syscall).
> > >
> > > The motivation for this feature (as I understand it) is to avoid the need
to reconstruct the paths corresponding to the file descriptors passed to syscalls, as this
might be difficult and time consuming or even impossible in case not all of the right
sycalls are being logged. Note that it is always possible to disable these records by
simply adding an exclude filter rule matching all records of type FD_PATH.
> > >
> > > At this moment I only implement logging for a single syscall (openat(2))
to keep it simple. In the final version I plan to add support for other similar syscalls
()mkdirat, mknodeat, fchownat, ...).
> > >
> > > Please let me know if the general approach and the proposed record format
make sense to you so I can improve/complete the solution.
> > >
> > > Thanks,
> > > Ondrej
> > >
> > > [1] https://github.com/linux-audit/audit-kernel/issues/9
> >
> > While I recognize that the GH issue did raise the idea of possibly
> > creating a new record type, looking at these patches I'm not sure a
> > new record type is justified, I think reusing the existing PATH record
> > type would be more beneficial. I recognize that this proposed FD_PATH
> > record also contains the file descriptor number, but that information
> > should also be contained in the associated SYSCALL record and arguably
> > the fd number is only useful if you are logging the SYSCALL
> > information.
>
> To be honest, I'm not sure what is the exact semantic of the PATH
> record... Is it simply "log information about files that the syscall
> somehow touched"? Or "log information about any string syscall
> argument that represents a file path"?
The PATH record exists to record path name information about files
related to the audit event, depending on the circumstances, both
relative and absolute paths could be expected. Look at the functions,
and callers, that add to the audit_context->names_list to get a better
understanding of where the path name information is collected. I
doubt you will be surprised at this, but fair warning, it's a bit ugly
in places.
> In the PATH record samples I
> have seen, the name=... field sometimes contains just the last segment
> of the path, other times it contains the full path (huh?).
Depending on the syscall, the syscall arguments, and the "nametype" of
the PATH record you could see either a full or partial path. In the
cases of a partial path, there *should* be additional PATH records
which can help recreate a full path to the file for that particular
event instance.
> When we log
> the full path in PATH, then I guess the FD_PATH record would be
> (almost) redundant, but it seems that whenever openat(2) is called
> with dirfd != AT_FDCWD, PATH name=... contains just the relative path
> supplied to the syscall (thus FD_PATH actually provides the missing
> directory path).
>
> So... assuming we would want to provide the missing information in the
> existing PATH record, how should it look like? Should the name=...
> field simply always contain the full path? Should there be another
> PATH record for the directory?
The only real hard requirement is that there be enough to information
to determine the file full path(s) for any given audit event using
one, or a combination, of PATH records associated with the event.
> If so, how do we indicate the association between the two PATH records?
Look at how the "nametype" field is used.
OK, so I just wrote a small script to see what PATH records would be
generated for a renameat(2) syscall with non-AT_FDCWD fd arguments,
and it seems the current implementation is not lacking information,
but actually buggy.
strace output:
openat(AT_FDCWD, "/tmp/tmp.CXtBRafonK/a", O_RDONLY|O_PATH|O_DIRECTORY) = 3
openat(AT_FDCWD, "/tmp/tmp.CXtBRafonK/b", O_RDONLY|O_PATH|O_DIRECTORY) = 4
renameat(3, "f", 4, "g") = 0
close(3) = 0
close(4) = 0
Corresponding audit records for renameat():
type=SYSCALL msg=audit(1532439957.660:5): arch=c000003e syscall=264
success=yes exit=0 a0=3 a1=7ffcc364de2a a2=4 a3=7ffcc364de42 items=4
ppid=594 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
sgid=0 fsgid=0 tty=(none) ses=1 comm="trigger-renamea"
exe="/tmp/tmp.GEfuEtn1II/trigger-renameat"
subj=system_u:system_r:kernel_t:s0 key=(null)
type=CWD msg=audit(1532439957.660:5): cwd="/root/Dokumenty/Kernel"
type=PATH msg=audit(1532439957.660:5): item=0
name="/root/Dokumenty/Kernel" inode=2155 dev=00:1a mode=040755 ouid=0
ogid=0 rdev=00:00 obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1532439957.660:5): item=1
name="/root/Dokumenty/Kernel" inode=2156 dev=00:1a mode=040755 ouid=0
ogid=0 rdev=00:00 obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1532439957.660:5): item=2 name="f" inode=2157
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=DELETE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1532439957.660:5): item=3 name="g" inode=2157
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=CREATE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PROCTITLE msg=audit(1532439957.660:5):
proctitle=2F746D702F746D702E4745667545746E3149492F747269676765722D72656E616D656174002F746D702F746D702E4745667545746E3149492F610066002F746D702F746D702E4745667545746E3149492F620067
The PARENT paths are incorrectly reporting the CWD path instead of the
path of the source/destination directories, even though the inode
numbers seem to be correct.
Beyond that, there is really no information in the records that would
allow reconstructing which PARENT path belongs to which CREATE/DELETE
path... (Intuitively you can guess that src will come before dst, but
that is not very reliable.) I think a "parent inode" field in the PATH
records could fix this, but maybe there is a better solution...
I'll see if/how I can fix these issues (especially the first one) and
then I'll get back to the original issue. renameat() and maybe a few
other syscalls should be OK after the fix, but at least openat() will
need some further work (right now it only emits just one PATH record
with only relative path).
> > Can you explain what advantage the FS_PATH record type has
over
> > reusing the existing PATH record? I know you mention multiple
> > fd/paths as in the case of renameat(2), but we already have to deal
> > with this in the non-at rename(2) case.
>
> As I said above, I don't fully understand the PATH record so I can't
> compare them right now. Multiple fd arguments was meant as more of a
> justification for the presence of the fd=... field in the new FD_PATH
> record.
I suggest taking some time to better understand how the current file
path auditing works and then revisiting this patchset.
--
paul moore
www.paul-moore.com--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
5:15 p.m.
On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
OK, so I just wrote a small script to see what PATH records would be
generated for a renameat(2) syscall with non-AT_FDCWD fd arguments,
and it seems the current implementation is not lacking information,
but actually buggy.
strace output:
openat(AT_FDCWD, "/tmp/tmp.CXtBRafonK/a", O_RDONLY|O_PATH|O_DIRECTORY) = 3
openat(AT_FDCWD, "/tmp/tmp.CXtBRafonK/b", O_RDONLY|O_PATH|O_DIRECTORY) = 4
renameat(3, "f", 4, "g") = 0
close(3) = 0
close(4) = 0
Corresponding audit records for renameat():
type=SYSCALL msg=audit(1532439957.660:5): arch=c000003e syscall=264
success=yes exit=0 a0=3 a1=7ffcc364de2a a2=4 a3=7ffcc364de42 items=4
ppid=594 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
sgid=0 fsgid=0 tty=(none) ses=1 comm="trigger-renamea"
exe="/tmp/tmp.GEfuEtn1II/trigger-renameat"
subj=system_u:system_r:kernel_t:s0 key=(null)
type=CWD msg=audit(1532439957.660:5): cwd="/root/Dokumenty/Kernel"
...
type=PATH msg=audit(1532439957.660:5): item=0
name="/root/Dokumenty/Kernel" inode=2155 dev=00:1a mode=040755 ouid=0
ogid=0 rdev=00:00 obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1532439957.660:5): item=1
name="/root/Dokumenty/Kernel" inode=2156 dev=00:1a mode=040755 ouid=0
ogid=0 rdev=00:00 obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1532439957.660:5): item=2 name="f" inode=2157
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=DELETE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1532439957.660:5): item=3 name="g" inode=2157
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=CREATE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
...
The PARENT paths are incorrectly reporting the CWD path instead of
the
path of the source/destination directories, even though the inode
numbers seem to be correct.
Yes, that's odd, and not desirable.
Beyond that, there is really no information in the records that
would
allow reconstructing which PARENT path belongs to which CREATE/DELETE
path... (Intuitively you can guess that src will come before dst, but
that is not very reliable.) I think a "parent inode" field in the PATH
records could fix this, but maybe there is a better solution...
I have my suspicions, but I would be curious to hear from Steve how
the reconstruction is typically handled.
I'll see if/how I can fix these issues (especially the first one)
and
then I'll get back to the original issue. renameat() and maybe a few
other syscalls should be OK after the fix, but at least openat() will
need some further work (right now it only emits just one PATH record
with only relative path).
Yes, let's fix this first and go from there.
Thanks for looking into this.
--
paul moore
www.paul-moore.com
8:11 p.m.
On Tuesday, July 24, 2018 6:15:54 PM EDT Paul Moore wrote:
On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek
<omosnace(a)redhat.com>
wrote:
> OK, so I just wrote a small script to see what PATH records
would be
> generated for a renameat(2) syscall with non-AT_FDCWD fd arguments,
> and it seems the current implementation is not lacking information,
> but actually buggy.
>
> strace output:
> openat(AT_FDCWD, "/tmp/tmp.CXtBRafonK/a", O_RDONLY|O_PATH|O_DIRECTORY) =
> 3
> openat(AT_FDCWD, "/tmp/tmp.CXtBRafonK/b", O_RDONLY|O_PATH|O_DIRECTORY) =
> 4
> renameat(3, "f", 4, "g") = 0
> close(3) = 0
> close(4) = 0
>
> Corresponding audit records for renameat():
> type=SYSCALL msg=audit(1532439957.660:5): arch=c000003e syscall=264
> success=yes exit=0 a0=3 a1=7ffcc364de2a a2=4 a3=7ffcc364de42 items=4
> ppid=594 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> sgid=0 fsgid=0 tty=(none) ses=1 comm="trigger-renamea"
> exe="/tmp/tmp.GEfuEtn1II/trigger-renameat"
> subj=system_u:system_r:kernel_t:s0 key=(null)
> type=CWD msg=audit(1532439957.660:5): cwd="/root/Dokumenty/Kernel"
...
> type=PATH msg=audit(1532439957.660:5): item=0
> name="/root/Dokumenty/Kernel" inode=2155 dev=00:1a mode=040755 ouid=0
> ogid=0 rdev=00:00 obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
> cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PATH msg=audit(1532439957.660:5): item=1
> name="/root/Dokumenty/Kernel" inode=2156 dev=00:1a mode=040755 ouid=0
> ogid=0 rdev=00:00 obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
> cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PATH msg=audit(1532439957.660:5): item=2 name="f" inode=2157
> dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
> obj=system_u:object_r:tmpfs_t:s0 nametype=DELETE
> cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PATH msg=audit(1532439957.660:5): item=3 name="g" inode=2157
> dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
> obj=system_u:object_r:tmpfs_t:s0 nametype=CREATE
> cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
...
> The PARENT paths are incorrectly reporting the CWD path instead of the
> path of the source/destination directories, even though the inode
> numbers seem to be correct.
Yes, that's odd, and not desirable.
> Beyond that, there is really no information in the records that would
> allow reconstructing which PARENT path belongs to which CREATE/DELETE
> path... (Intuitively you can guess that src will come before dst, but
> that is not very reliable.) I think a "parent inode" field in the PATH
> records could fix this, but maybe there is a better solution...
I have my suspicions, but I would be curious to hear from Steve how
the reconstruction is typically handled.
For any *at function when the dirfd is not AT_FDCWD, it goes badly. If its a
old style syscall without the dirfd, then if the first character is '/' use
that. Otherwise concatonate cwd and path and pass it to realpath to sort out.
-Steve
> I'll see if/how I can fix these issues (especially the first
one) and
> then I'll get back to the original issue. renameat() and maybe a few
> other syscalls should be OK after the fix, but at least openat() will
> need some further work (right now it only emits just one PATH record
> with only relative path).
Yes, let's fix this first and go from there.
Thanks for looking into this.
2:44 a.m.
On Wed, Jul 25, 2018 at 3:11 AM Steve Grubb <sgrubb(a)redhat.com> wrote:
On Tuesday, July 24, 2018 6:15:54 PM EDT Paul Moore wrote:
> On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek <omosnace(a)redhat.com>
> > Beyond that, there is really no information in the records that would
> > allow reconstructing which PARENT path belongs to which CREATE/DELETE
> > path... (Intuitively you can guess that src will come before dst, but
> > that is not very reliable.) I think a "parent inode" field in the
PATH
> > records could fix this, but maybe there is a better solution...
>
> I have my suspicions, but I would be curious to hear from Steve how
> the reconstruction is typically handled.
For any *at function when the dirfd is not AT_FDCWD, it goes badly. If its a
old style syscall without the dirfd, then if the first character is '/' use
that. Otherwise concatonate cwd and path and pass it to realpath to sort out.
In that case it seems the best fix for openat() et al. would be to
somehow always force outputting the full path when dirfd != AT_FDCWD.
Hopefully that won't require too much hacking around...
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
7:48 a.m.
On Wednesday, July 25, 2018 3:44:07 AM EDT Ondrej Mosnacek wrote:
On Wed, Jul 25, 2018 at 3:11 AM Steve Grubb <sgrubb(a)redhat.com>
wrote:
> On Tuesday, July 24, 2018 6:15:54 PM EDT Paul Moore wrote:
> > On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek <omosnace(a)redhat.com>
> >
> > > Beyond that, there is really no information in the records that would
> > > allow reconstructing which PARENT path belongs to which CREATE/DELETE
> > > path... (Intuitively you can guess that src will come before dst, but
> > > that is not very reliable.) I think a "parent inode" field in
the
> > > PATH
> > > records could fix this, but maybe there is a better solution...
> >
> > I have my suspicions, but I would be curious to hear from Steve how
> > the reconstruction is typically handled.
>
> For any *at function when the dirfd is not AT_FDCWD, it goes badly. If
> its a old style syscall without the dirfd, then if the first character
> is '/' use that. Otherwise concatonate cwd and path and pass it to
> realpath to sort out.
In that case it seems the best fix for openat() et al. would be to
somehow always force outputting the full path when dirfd != AT_FDCWD.
Hopefully that won't require too much hacking around...
What is asked for is the full path that dirfd was opened with. I can take
care of everything else.
-Steve
8:02 a.m.
On Wed, Jul 25, 2018 at 2:48 PM Steve Grubb <sgrubb(a)redhat.com> wrote:
On Wednesday, July 25, 2018 3:44:07 AM EDT Ondrej Mosnacek wrote:
> On Wed, Jul 25, 2018 at 3:11 AM Steve Grubb <sgrubb(a)redhat.com> wrote:
> > On Tuesday, July 24, 2018 6:15:54 PM EDT Paul Moore wrote:
> > > On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek
<omosnace(a)redhat.com>
> > >
> > > > Beyond that, there is really no information in the records that
would
> > > > allow reconstructing which PARENT path belongs to which
CREATE/DELETE
> > > > path... (Intuitively you can guess that src will come before dst,
but
> > > > that is not very reliable.) I think a "parent inode" field
in the
> > > > PATH
> > > > records could fix this, but maybe there is a better solution...
> > >
> > > I have my suspicions, but I would be curious to hear from Steve how
> > > the reconstruction is typically handled.
> >
> > For any *at function when the dirfd is not AT_FDCWD, it goes badly. If
> > its a old style syscall without the dirfd, then if the first character
> > is '/' use that. Otherwise concatonate cwd and path and pass it to
> > realpath to sort out.
>
> In that case it seems the best fix for openat() et al. would be to
> somehow always force outputting the full path when dirfd != AT_FDCWD.
> Hopefully that won't require too much hacking around...
What is asked for is the full path that dirfd was opened with. I can take
care of everything else.
But where/how should that path be logged? In case of renameat(), for
example, we have 6 (!) path components:
<src_dir>/<src_parent>/<src_child> and
<dst_dir>/<dst_parent>/<dst_child>
(I am assuming the child paths always represent just the last path
component based on the observed inodes of the parent/child records.)
Current record format can distinguish between PARENT and child
(DELETE/CREATE), but there is no nametype for the dirfd path. That's
why I am leaning towards just logging the full "<*_dir>/<*_parent>"
path in the PARENT record. Or do you prefer that we add a new nametype
for the dirfd path?
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
8:11 a.m.
On Wednesday, July 25, 2018 9:02:50 AM EDT Ondrej Mosnacek wrote:
On Wed, Jul 25, 2018 at 2:48 PM Steve Grubb <sgrubb(a)redhat.com>
wrote:
> On Wednesday, July 25, 2018 3:44:07 AM EDT Ondrej Mosnacek wrote:
> > On Wed, Jul 25, 2018 at 3:11 AM Steve Grubb <sgrubb(a)redhat.com> wrote:
> > > On Tuesday, July 24, 2018 6:15:54 PM EDT Paul Moore wrote:
> > > > On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek
> > > > <omosnace(a)redhat.com>
> > > >
> > > > > Beyond that, there is really no information in the records that
> > > > > would
> > > > > allow reconstructing which PARENT path belongs to which
> > > > > CREATE/DELETE
> > > > > path... (Intuitively you can guess that src will come before
dst,
> > > > > but
> > > > > that is not very reliable.) I think a "parent inode"
field in the
> > > > > PATH
> > > > > records could fix this, but maybe there is a better solution...
> > > >
> > > > I have my suspicions, but I would be curious to hear from Steve how
> > > > the reconstruction is typically handled.
> > >
> > > For any *at function when the dirfd is not AT_FDCWD, it goes badly.
> > > If
> > > its a old style syscall without the dirfd, then if the first
> > > character
> > > is '/' use that. Otherwise concatonate cwd and path and pass it
to
> > > realpath to sort out.
> >
> > In that case it seems the best fix for openat() et al. would be to
> > somehow always force outputting the full path when dirfd != AT_FDCWD.
> > Hopefully that won't require too much hacking around...
>
> What is asked for is the full path that dirfd was opened with. I can take
> care of everything else.
But where/how should that path be logged? In case of renameat(), for
example, we have 6 (!) path components:
<src_dir>/<src_parent>/<src_child> and
<dst_dir>/<dst_parent>/<dst_child>
(I am assuming the child paths always represent just the last path
component based on the observed inodes of the parent/child records.)
Current record format can distinguish between PARENT and child
(DELETE/CREATE), but there is no nametype for the dirfd path. That's
why I am leaning towards just logging the full
"<*_dir>/<*_parent>"
path in the PARENT record. Or do you prefer that we add a new nametype
for the dirfd path?
You could make a new nametype so that we can make sense of it. But do you
have all of the required information for a PATH record? I thought that you
were making a new record type since you have abbreviated information.
-Steve
3:12 a.m.
On Wed, Jul 25, 2018 at 3:11 PM Steve Grubb <sgrubb(a)redhat.com> wrote:
On Wednesday, July 25, 2018 9:02:50 AM EDT Ondrej Mosnacek wrote:
> On Wed, Jul 25, 2018 at 2:48 PM Steve Grubb <sgrubb(a)redhat.com> wrote:
> > On Wednesday, July 25, 2018 3:44:07 AM EDT Ondrej Mosnacek wrote:
> > > On Wed, Jul 25, 2018 at 3:11 AM Steve Grubb <sgrubb(a)redhat.com>
wrote:
> > > > On Tuesday, July 24, 2018 6:15:54 PM EDT Paul Moore wrote:
> > > > > On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek
> > > > > <omosnace(a)redhat.com>
> > > > >
> > > > > > Beyond that, there is really no information in the records
that
> > > > > > would
> > > > > > allow reconstructing which PARENT path belongs to which
> > > > > > CREATE/DELETE
> > > > > > path... (Intuitively you can guess that src will come
before dst,
> > > > > > but
> > > > > > that is not very reliable.) I think a "parent
inode" field in the
> > > > > > PATH
> > > > > > records could fix this, but maybe there is a better
solution...
> > > > >
> > > > > I have my suspicions, but I would be curious to hear from Steve
how
> > > > > the reconstruction is typically handled.
> > > >
> > > > For any *at function when the dirfd is not AT_FDCWD, it goes badly.
> > > > If
> > > > its a old style syscall without the dirfd, then if the first
> > > > character
> > > > is '/' use that. Otherwise concatonate cwd and path and pass
it to
> > > > realpath to sort out.
> > >
> > > In that case it seems the best fix for openat() et al. would be to
> > > somehow always force outputting the full path when dirfd != AT_FDCWD.
> > > Hopefully that won't require too much hacking around...
> >
> > What is asked for is the full path that dirfd was opened with. I can take
> > care of everything else.
>
> But where/how should that path be logged? In case of renameat(), for
> example, we have 6 (!) path components:
> <src_dir>/<src_parent>/<src_child> and
<dst_dir>/<dst_parent>/<dst_child>
>
> (I am assuming the child paths always represent just the last path
> component based on the observed inodes of the parent/child records.)
>
> Current record format can distinguish between PARENT and child
> (DELETE/CREATE), but there is no nametype for the dirfd path. That's
> why I am leaning towards just logging the full
"<*_dir>/<*_parent>"
> path in the PARENT record. Or do you prefer that we add a new nametype
> for the dirfd path?
You could make a new nametype so that we can make sense of it. But do you
have all of the required information for a PATH record? I thought that you
were making a new record type since you have abbreviated information.
I think it should be possible to collect that information by putting
hooks in the right places of the filesystem code (and fixing the
current ones).
To be honest, the reason why I had jumped right to making a new record
type was Paul's wording in the issue description ("We may need a new
auxiliary record type since this is neither the cwd or path." - the
second part of that sentence sounded like using the PATH record
wouldn't be a reasonable choice) and the fact that the purpose of the
PATH records was not clear to me at that time. Now that I spent some
time with them, they don't look that scary anymore :)
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
4:12 a.m.
On Thu, Jul 26, 2018 at 10:12 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
I think it should be possible to collect that information by putting
hooks in the right places of the filesystem code (and fixing the
current ones).
Hm, after closer look, it seems this won't be doable (at least not
easily). The PATH record always logs the original path string from
userspace (and I think we need to preserve this behavior in case
someone relies on it). In case of PARENT records, it truncates away
the last component (because it wants to log inode information also for
the parent directory). If this truncated string ends up empty (i.e.
the original string had just one component), it just smashes in the
absolute path of the CWD (which is known), because it pretends no
*at() syscalls exist and all relative paths are relative to current
CWD.
So to fix this, we need to do one of the following:
1. Add a new field to the PATH records that would specify the path of
the directory that the value of name=... is relative to. If this is
CWD, we can just use some special value
("(null)"/"(none)"/"(cwd)"/...) or omit the field
completely. I prefer
this approach, because it will best solve the case of renameat(),
where different PATH records can have different base directories.
2. If adding fields is considered A Bad Thing, we could alternatively
provide this information in separate records (either PATH with special
nametype or a new record). However in such case we need to somehow
specify to which PATH records each base directory corresponds. For
PATH records this could be guessed from their order, but this is a
fragile thing (changes in filesystem code could change the order).
Let me give an example to illustrate better how it might look like:
=== renameat() with two FD arguments ===
openat(AT_FDCWD, "/tmp/tmp.SIMqhBS0eI/a", O_RDONLY|O_PATH|O_DIRECTORY) = 3
openat(AT_FDCWD, "/tmp/tmp.SIMqhBS0eI/b", O_RDONLY|O_PATH|O_DIRECTORY) = 4
renameat(3, "x/f", 4, "y/g") = 0
close(3) = 0
close(4) = 0
type=SYSCALL msg=audit(1532504483.814:5): [...]
type=CWD msg=audit(1532504483.814:5): cwd="/root/Dokumenty/Kernel"
type=PATH msg=audit(1532504483.814:5): item=0 name="x/" inode=2156
dev=00:1a mode=040755 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir="/tmp/tmp.SIMqhBS0eI/a"
type=PATH msg=audit(1532504483.814:5): item=1 name="y/" inode=2158
dev=00:1a mode=040755 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir="/tmp/tmp.SIMqhBS0eI/b"
type=PATH msg=audit(1532504483.814:5): item=2 name="x/f" inode=2159
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=DELETE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir="/tmp/tmp.SIMqhBS0eI/a"
type=PATH msg=audit(1532504483.814:5): item=3 name="y/g" inode=2159
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=CREATE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir="/tmp/tmp.SIMqhBS0eI/b"
=== renameat() with one CWD and one FD argument ===
openat(AT_FDCWD, "/tmp/tmp.SIMqhBS0eI/a", O_RDONLY|O_PATH|O_DIRECTORY) = 3
renameat(3, "x/f", AT_FDCWD, "y/g") = 0
close(3) = 0
type=SYSCALL msg=audit(1532504483.814:5): [...]
type=CWD msg=audit(1532504483.814:5): cwd="/root/Dokumenty/Kernel"
type=PATH msg=audit(1532504483.814:5): item=0 name="x/" inode=2156
dev=00:1a mode=040755 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir="/tmp/tmp.SIMqhBS0eI/a"
type=PATH msg=audit(1532504483.814:5): item=1 name="y/" inode=2158
dev=00:1a mode=040755 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=PARENT
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir=(cwd)
type=PATH msg=audit(1532504483.814:5): item=2 name="x/f" inode=2159
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=DELETE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir="/tmp/tmp.SIMqhBS0eI/a"
type=PATH msg=audit(1532504483.814:5): item=3 name="y/g" inode=2159
dev=00:1a mode=0100644 ouid=0 ogid=0 rdev=00:00
obj=system_u:object_r:tmpfs_t:s0 nametype=CREATE
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
basedir=(cwd)
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
6:58 p.m.
On Thu, Jul 26, 2018 at 5:13 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
On Thu, Jul 26, 2018 at 10:12 AM Ondrej Mosnacek
<omosnace(a)redhat.com> wrote:
> I think it should be possible to collect that information by putting
> hooks in the right places of the filesystem code (and fixing the
> current ones).
Hm, after closer look, it seems this won't be doable (at least not
easily). The PATH record always logs the original path string from
userspace (and I think we need to preserve this behavior in case
someone relies on it). In case of PARENT records, it truncates away
the last component (because it wants to log inode information also for
the parent directory). If this truncated string ends up empty (i.e.
the original string had just one component), it just smashes in the
absolute path of the CWD (which is known), because it pretends no
*at() syscalls exist and all relative paths are relative to current
CWD.
So to fix this, we need to do one of the following:
1. Add a new field to the PATH records that would specify the path of
the directory that the value of name=... is relative to. If this is
CWD, we can just use some special value
("(null)"/"(none)"/"(cwd)"/...) or omit the field
completely. I prefer
this approach, because it will best solve the case of renameat(),
where different PATH records can have different base directories.
It is worth calling extra attention to the fact that we would now be
effectively recording two paths in a single record. I'm not sure how
much we care about that, but it does increase the chances we blow past
the end of the netlink buffer; although it is worth noting that a
single PATH_MAX entry would do that today.
Also, would this new field remain empty for non-*at syscalls?
2. If adding fields is considered A Bad Thing, we could
alternatively
provide this information in separate records (either PATH with special
nametype or a new record). However in such case we need to somehow
specify to which PATH records each base directory corresponds. For
PATH records this could be guessed from their order, but this is a
fragile thing (changes in filesystem code could change the order).
While this may be a bit more difficult it seems like this is more in
keeping with the current methodology, and would keep the overall audit
logs smaller. In the case of the *at syscalls I presume you would
create PARENT records for the base directory, omitting it if AT_FDCWD
was used?
I imagine this would also help the traditional rename() syscall?
--
paul moore
www.paul-moore.com
4:19 a.m.
On Fri, Aug 3, 2018 at 1:58 AM Paul Moore <paul(a)paul-moore.com> wrote:
On Thu, Jul 26, 2018 at 5:13 AM Ondrej Mosnacek
<omosnace(a)redhat.com> wrote:
> On Thu, Jul 26, 2018 at 10:12 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
> > I think it should be possible to collect that information by putting
> > hooks in the right places of the filesystem code (and fixing the
> > current ones).
>
> Hm, after closer look, it seems this won't be doable (at least not
> easily). The PATH record always logs the original path string from
> userspace (and I think we need to preserve this behavior in case
> someone relies on it). In case of PARENT records, it truncates away
> the last component (because it wants to log inode information also for
> the parent directory). If this truncated string ends up empty (i.e.
> the original string had just one component), it just smashes in the
> absolute path of the CWD (which is known), because it pretends no
> *at() syscalls exist and all relative paths are relative to current
> CWD.
>
> So to fix this, we need to do one of the following:
> 1. Add a new field to the PATH records that would specify the path of
> the directory that the value of name=... is relative to. If this is
> CWD, we can just use some special value
> ("(null)"/"(none)"/"(cwd)"/...) or omit the field
completely. I prefer
> this approach, because it will best solve the case of renameat(),
> where different PATH records can have different base directories.
It is worth calling extra attention to the fact that we would now be
effectively recording two paths in a single record. I'm not sure how
much we care about that, but it does increase the chances we blow past
the end of the netlink buffer; although it is worth noting that a
single PATH_MAX entry would do that today.
Yes, but note that the two "paths" are actually just parts of a single
(full) path. The situation will be no different from when the user
would just specify a full path. But your comment just made me realize
that I will need to make sure that the new field is empty also when
dirfd != AT_FDCWD, but the path supplied to the syscall is absolute
(because then it would be just useless information).
Also, would this new field remain empty for non-*at syscalls?
Yes, the emptiness (or some special value depending on the chosen
implementation) would indicate that dirfd is CWD (which can be looked
up in the CWD record).
> 2. If adding fields is considered A Bad Thing, we could
alternatively
> provide this information in separate records (either PATH with special
> nametype or a new record). However in such case we need to somehow
> specify to which PATH records each base directory corresponds. For
> PATH records this could be guessed from their order, but this is a
> fragile thing (changes in filesystem code could change the order).
While this may be a bit more difficult it seems like this is more in
keeping with the current methodology, and would keep the overall audit
logs smaller. In the case of the *at syscalls I presume you would
create PARENT records for the base directory, omitting it if AT_FDCWD
was used?
Right now, the parent/child records are only emitted when a file is
moved, created, or removed (actually I'm not sure right now about the
latter two, but definitely for the first case). The PARENT record
always contains the (relative or absolute) path as given to the
syscall minus the last element and the corresponding CREATE/DELETE
record contains only the last element. I don't know what is the goal
of this splitting (probably to report details (inode, ...) also about
the containing directory or maybe to deduplicate PATH records if src
and dst directories are the same?), but I'm not sure it is a good idea
to overload the semantics of nametype=PARENT for yet another purpose
(one of the PARENTs would be parent for the other PARENT...). I'm
almost certain this would also confuse the current userspace code
(especially considering that for rename() the kernel generates both
PARENT records before the child records...).
That said, it seems to me this would actually make the logs *bigger*,
because you would also log the usual PATH record details (inode,
fcaps, ...) for the base directory (dirfd).
I imagine this would also help the traditional rename() syscall?
I'm not sure how, given the facts above.
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
6:16 p.m.
On Thu, Jul 26, 2018 at 4:12 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
On Wed, Jul 25, 2018 at 3:11 PM Steve Grubb <sgrubb(a)redhat.com>
wrote:
> On Wednesday, July 25, 2018 9:02:50 AM EDT Ondrej Mosnacek wrote:
> > On Wed, Jul 25, 2018 at 2:48 PM Steve Grubb <sgrubb(a)redhat.com> wrote:
> > > On Wednesday, July 25, 2018 3:44:07 AM EDT Ondrej Mosnacek wrote:
> > > > On Wed, Jul 25, 2018 at 3:11 AM Steve Grubb <sgrubb(a)redhat.com>
wrote:
> > > > > On Tuesday, July 24, 2018 6:15:54 PM EDT Paul Moore wrote:
> > > > > > On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek
> > > > > > <omosnace(a)redhat.com>
> > > > > >
> > > > > > > Beyond that, there is really no information in the
records that
> > > > > > > would
> > > > > > > allow reconstructing which PARENT path belongs to
which
> > > > > > > CREATE/DELETE
> > > > > > > path... (Intuitively you can guess that src will come
before dst,
> > > > > > > but
> > > > > > > that is not very reliable.) I think a "parent
inode" field in the
> > > > > > > PATH
> > > > > > > records could fix this, but maybe there is a better
solution...
> > > > > >
> > > > > > I have my suspicions, but I would be curious to hear from
Steve how
> > > > > > the reconstruction is typically handled.
> > > > >
> > > > > For any *at function when the dirfd is not AT_FDCWD, it goes
badly.
> > > > > If
> > > > > its a old style syscall without the dirfd, then if the first
> > > > > character
> > > > > is '/' use that. Otherwise concatonate cwd and path and
pass it to
> > > > > realpath to sort out.
> > > >
> > > > In that case it seems the best fix for openat() et al. would be to
> > > > somehow always force outputting the full path when dirfd !=
AT_FDCWD.
> > > > Hopefully that won't require too much hacking around...
> > >
> > > What is asked for is the full path that dirfd was opened with. I can take
> > > care of everything else.
> >
> > But where/how should that path be logged? In case of renameat(), for
> > example, we have 6 (!) path components:
> > <src_dir>/<src_parent>/<src_child> and
<dst_dir>/<dst_parent>/<dst_child>
> >
> > (I am assuming the child paths always represent just the last path
> > component based on the observed inodes of the parent/child records.)
> >
> > Current record format can distinguish between PARENT and child
> > (DELETE/CREATE), but there is no nametype for the dirfd path. That's
> > why I am leaning towards just logging the full
"<*_dir>/<*_parent>"
> > path in the PARENT record. Or do you prefer that we add a new nametype
> > for the dirfd path?
>
> You could make a new nametype so that we can make sense of it. But do you
> have all of the required information for a PATH record? I thought that you
> were making a new record type since you have abbreviated information.
I think it should be possible to collect that information by putting
hooks in the right places of the filesystem code (and fixing the
current ones).
To be honest, the reason why I had jumped right to making a new record
type was Paul's wording in the issue description ("We may need a new
auxiliary record type since this is neither the cwd or path." ...
For future reference, when I say "may" I do really mean it; it *may*
be a good idea, but it *may* also be a garbage idea ;)
--
paul moore
www.paul-moore.com
2332
days inactive
2354
days old
linux-audit@lists.linux-audit.osci.io
26 comments
4 participants
participants (4)
-
Ondrej Mosnacek -
Paul Moore -
Richard Guy Briggs -
Steve Grubb