[RFC PATCH ghak9 0/3] audit: Record the path of FDs passed to *at(2) syscalls

[Ondrej Mosnacek]

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

[Ondrej Mosnacek]

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 <***@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 */

[Richard Guy Briggs]

Post by Ondrej Mosnacek
This new record type is used to log the full path corresponding to some
important file descriptor used in a syscall.
---
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.

Post by Ondrej Mosnacek
#define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */

- RGB

--
Richard Guy Briggs <***@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]

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
This new record type is used to log the full path corresponding to some
important file descriptor used in a syscall.
---
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.

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
#define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */

- RGB
--
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]

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 <***@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;

[Richard Guy Briggs]

Post by Ondrej Mosnacek
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).
type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
---
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?

Post by Ondrej Mosnacek
+ __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.

Post by Ondrej Mosnacek
+ 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);
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;

- RGB

--
Richard Guy Briggs <***@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]

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
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).
type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
---
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.

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
+ __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.

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
+ 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);
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;

- RGB
--
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

[Richard Guy Briggs]

Post by Ondrej Mosnacek

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
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).
type=FD_PATH msg=audit(...): fd=<file descriptor> path=<file path>
---
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.

Post by Ondrej Mosnacek

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
+ __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.

Post by Richard Guy Briggs

Post by Ondrej Mosnacek
+ 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);
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;

- RGB
--
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 <***@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

[Steve Grubb]

Post by Ondrej Mosnacek
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).
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

Post by Ondrej Mosnacek
---
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);
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;

[Ondrej Mosnacek]

Post by Steve Grubb

Post by Ondrej Mosnacek
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).
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).

Post by Steve Grubb
-Steve

Post by Ondrej Mosnacek
---
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);
+ kfree(buf);
+}
+
static void audit_log_task(struct audit_buffer *ab)
{
kuid_t auid, uid;

[Ondrej Mosnacek]

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 <***@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);
}

[Richard Guy Briggs]

Post by Ondrej Mosnacek
So far only add one to openat(2) for testing purposes.
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
----

This patchset looks encouraging.

Post by Ondrej Mosnacek
---
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 <***@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

[Paul Moore]

Post by Ondrej Mosnacek
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.

[Ondrej Mosnacek]

Post by Paul Moore

Post by Ondrej Mosnacek
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?

Post by Paul Moore
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.

[Paul Moore]

Post by Ondrej Mosnacek

Post by Paul Moore

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
If so, how do we indicate the association between the two PATH records?

Look at how the "nametype" field is used.

Post by Ondrej Mosnacek

Post by Paul Moore
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.

[Ondrej Mosnacek]

Post by Paul Moore

Post by Ondrej Mosnacek

Post by Paul Moore

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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).

Post by Paul Moore

Post by Ondrej Mosnacek

Post by Paul Moore
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.

[Paul Moore]

Post by Ondrej Mosnacek
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.
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
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"

...

Post by Ondrej Mosnacek
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

...

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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.

[Steve Grubb]

Post by Paul Moore

Post by Ondrej Mosnacek
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.
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
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"

...

Post by Ondrej Mosnacek
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

...

Post by Ondrej Mosnacek
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.

Post by Ondrej Mosnacek
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

Post by Paul Moore

Post by Ondrej Mosnacek
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.

[Ondrej Mosnacek]

Post by Steve Grubb

Post by Paul Moore

Post by Ondrej Mosnacek
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.

[Steve Grubb]

Post by Ondrej Mosnacek

Post by Steve Grubb

Post by Paul Moore

Post by Ondrej Mosnacek
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

[Ondrej Mosnacek]

Post by Steve Grubb

Post by Ondrej Mosnacek

Post by Steve Grubb

Post by Paul Moore

Post by Ondrej Mosnacek
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?

[Steve Grubb]

Post by Ondrej Mosnacek

Post by Steve Grubb

Post by Ondrej Mosnacek

Post by Steve Grubb

On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek

Post by Ondrej Mosnacek
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
<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

[Ondrej Mosnacek]

Post by Steve Grubb

Post by Ondrej Mosnacek

Post by Steve Grubb

Post by Ondrej Mosnacek

Post by Steve Grubb

On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek

Post by Ondrej Mosnacek
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
<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.

[Ondrej Mosnacek]

Post by Ondrej Mosnacek
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)

[Paul Moore]

Post by Ondrej Mosnacek

Post by Ondrej Mosnacek
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.
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?

Post by Ondrej Mosnacek
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?

[Ondrej Mosnacek]

Post by Paul Moore

Post by Ondrej Mosnacek

Post by Ondrej Mosnacek
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.
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).

Post by Paul Moore
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).

Post by Paul Moore

Post by Ondrej Mosnacek
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).

Post by Paul Moore
I imagine this would also help the traditional rename() syscall?

I'm not sure how, given the facts above.

[Paul Moore]

Post by Ondrej Mosnacek

Post by Steve Grubb

Post by Ondrej Mosnacek

Post by Steve Grubb

Post by Ondrej Mosnacek

Post by Steve Grubb

On Tue, Jul 24, 2018 at 10:12 AM Ondrej Mosnacek

Post by Ondrej Mosnacek
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
<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 ;)