On Thu, Aug 2, 2018 at 1:45 PM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
When a relative path has just a single component and we want to emit
a
nametype=PARENT record, the current implementation just reports the full
CWD path (which is alrady available in the audit context).
This is wrong for three reasons:
1. Wasting log space for redundant data (CWD path is already in the CWD
record).
2. Inconsistency with other PATH records (if a relative PARENT directory
path contains at least one component, only the verbatim relative path
is logged).
3. In some syscalls (e.g. openat(2)) the relative path may not even be
relative to the CWD, but to another directory specified as a file
descriptor. In that case the logged path is simply plain wrong.
This patch modifies this behavior to simply report "." in the
aforementioned case, which is equivalent to an "empty" directory path
and can be concatenated with the actual base directory path (CWD or
dirfd from openat(2)-like syscall) once support for its logging is added
later. In the meantime, defaulting to CWD as base directory on relative
paths (as already done by the userspace tools) will be enough to achieve
results equivalent to the current behavior.
I tested this patch a little bit with the libauparse library (it has
some functions for normalizing paths extracted from the records) and
it seems to behave as intended. The userspace functions seem to take a
rather crude approach and I think they won't always properly normalize
some paths, but that is a separate issue and this patch at least
definitely doesn't make it worse.
See:
https://github.com/linux-audit/audit-kernel/issues/95
Fixes: 9c937dcc7102 ("[PATCH] log more info for directory entry change
events")
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
kernel/audit.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 2a8058764aa6..4f18bd48eb4b 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2127,28 +2127,27 @@ void audit_log_name(struct audit_context *context, struct
audit_names *n,
audit_log_format(ab, "item=%d", record_num);
+ audit_log_format(ab, " name=");
if (path)
- audit_log_d_path(ab, " name=", path);
+ audit_log_d_path(ab, NULL, path);
else if (n->name) {
switch (n->name_len) {
case AUDIT_NAME_FULL:
/* log the full path */
- audit_log_format(ab, " name=");
audit_log_untrustedstring(ab, n->name->name);
break;
case 0:
/* name was specified as a relative path and the
* directory component is the cwd */
- audit_log_d_path(ab, " name=", &context->pwd);
+ audit_log_untrustedstring(ab, ".");
break;
default:
/* log the name's directory component */
- audit_log_format(ab, " name=");
audit_log_n_untrustedstring(ab, n->name->name,
n->name_len);
}
} else
- audit_log_format(ab, " name=(null)");
+ audit_log_format(ab, "(null)");
if (n->ino != AUDIT_INO_UNSET)
audit_log_format(ab, " inode=%lu"
--
2.17.1
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.