Skip to contentSkip to conversation

audit: correctly record file names with different path name types

RFC PATCH In review Paul Moore 3 participants 8 messages Dec 1, 2014 – Dec 4, 2014 Audit

lore
Share & tools
lore

Paul Moore

author
#0 lore raw

RFC PATCH 1/1 audit: correctly record file names with different path name types

There is a problem with the audit system when multiple audit records are created for the same path, each with a different path name type. The root cause of the problem is in __audit_inode() when an exact match (both the path name and path name type) is not found for a path name record; the existing code creates a new path name record, but it never sets the path name in this record, leaving it NULL. This patch corrects this problem by assigning the path name to these newly created records.

There are many ways to reproduce this problem, but one of the easiest is the following (assuming auditd is running):

  # mkdir /root/tmp/test
  # touch /root/tmp/test/567
  # auditctl -a always,exit -F dir=/root/tmp/test
  # touch /root/tmp/test/567

Afterwards, or while the commands above are running, check the audit log and pay special attention to the PATH records. A faulty kernel will display something like the following for the file creation:

type=SYSCALL msg=audit(1416957442.025:93): arch=c000003e syscall=2

    success=yes exit=3 ... comm="touch" exe="/usr/bin/touch"
  type=CWD msg=audit(1416957442.025:93):  cwd="/root/tmp"
  type=PATH msg=audit(1416957442.025:93): item=0 name="test/"
    inode=401409 ... nametype=PARENT
  type=PATH msg=audit(1416957442.025:93): item=1 name=(null)
    inode=393804 ... nametype=NORMAL
  type=PATH msg=audit(1416957442.025:93): item=2 name=(null)
    inode=393804 ... nametype=NORMAL

While a patched kernel will show the following:

type=SYSCALL msg=audit(1416955786.566:89): arch=c000003e syscall=2

    success=yes exit=3 ... comm="touch" exe="/usr/bin/touch"
  type=CWD msg=audit(1416955786.566:89):  cwd="/root/tmp"
  type=PATH msg=audit(1416955786.566:89): item=0 name="test/"
    inode=401409 ... nametype=PARENT
  type=PATH msg=audit(1416955786.566:89): item=1 name="test/567"
    inode=393804 ... nametype=NORMAL

This issue was brought up by a number of people, but special credit should go to [email protected] for reporting the problem along with an explanation of the problem and a patch. While the original patch did have some problems (see the archive link below), it did demonstrate the problem and helped kickstart the fix presented here.

* https://lkml.org/lkml/2014/9/5/66

---
 kernel/auditsc.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
kernel/auditsc.c +10 -4
index 21eae3c..ff99c05 100644
@@ -1886,12 +1886,18 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 	}
 
 out_alloc:
-	/* unable to find the name from a previous getname(). Allocate a new
-	 * anonymous entry.
-	 */
-	n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
+	/* unable to find an entry with both a matching name and type */
+	n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
 	if (!n)
 		return;
+	if (name)
+		/* since name is not NULL we know there is already a matching
+		 * name record, see audit_getname(), so there must be a type
+		 * mismatch; reuse the string path since the original name
+		 * record will keep the string valid until we free it in
+		 * audit_free_names() */
+		n->name = name;
+
 out:
 	if (parent) {
 		n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;
Acked-by on v1 1/1
#1 lore raw
Quoted 47 lines · Paul Moore

There is a problem with the audit system when multiple audit records are created for the same path, each with a different path name type. The root cause of the problem is in __audit_inode() when an exact match (both the path name and path name type) is not found for a path name record; the existing code creates a new path name record, but it never sets the path name in this record, leaving it NULL. This patch corrects this problem by assigning the path name to these newly created records.

There are many ways to reproduce this problem, but one of the easiest is the following (assuming auditd is running):

  # mkdir /root/tmp/test
  # touch /root/tmp/test/567
  # auditctl -a always,exit -F dir=/root/tmp/test
  # touch /root/tmp/test/567

Afterwards, or while the commands above are running, check the audit log and pay special attention to the PATH records. A faulty kernel will display something like the following for the file creation:

type=SYSCALL msg=audit(1416957442.025:93): arch=c000003e syscall=2

    success=yes exit=3 ... comm="touch" exe="/usr/bin/touch"
  type=CWD msg=audit(1416957442.025:93):  cwd="/root/tmp"
  type=PATH msg=audit(1416957442.025:93): item=0 name="test/"
    inode=401409 ... nametype=PARENT
  type=PATH msg=audit(1416957442.025:93): item=1 name=(null)
    inode=393804 ... nametype=NORMAL
  type=PATH msg=audit(1416957442.025:93): item=2 name=(null)
    inode=393804 ... nametype=NORMAL

While a patched kernel will show the following:

type=SYSCALL msg=audit(1416955786.566:89): arch=c000003e syscall=2

    success=yes exit=3 ... comm="touch" exe="/usr/bin/touch"
  type=CWD msg=audit(1416955786.566:89):  cwd="/root/tmp"
  type=PATH msg=audit(1416955786.566:89): item=0 name="test/"
    inode=401409 ... nametype=PARENT
  type=PATH msg=audit(1416955786.566:89): item=1 name="test/567"
    inode=393804 ... nametype=NORMAL

This issue was brought up by a number of people, but special credit should go to [email protected] for reporting the problem along with an explanation of the problem and a patch. While the original patch did have some problems (see the archive link below), it did demonstrate the problem and helped kickstart the fix presented here.

Quoted 5 lines

* https://lkml.org/lkml/2014/9/5/66

Quoted 21 lines
---
 kernel/auditsc.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
kernel/auditsc.c +3 -4
index 21eae3c..ff99c05 100644
@@ -1886,12 +1886,18 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 	}
 
 out_alloc:
-	/* unable to find the name from a previous getname(). Allocate a new
-	 * anonymous entry.
-	 */
-	n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
+	/* unable to find an entry with both a matching name and type */
+	n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
 	if (!n)
 		return;
+	if (name)

kernel/auditsc.c void __audit_inode(struct filename *name, const struct dentry *dentry, · line 1903

Quoted 10 lines
kernel/auditsc.c +7 -0
+		/* since name is not NULL we know there is already a matching
+		 * name record, see audit_getname(), so there must be a type
+		 * mismatch; reuse the string path since the original name
+		 * record will keep the string valid until we free it in
+		 * audit_free_names() */
+		n->name = name;
+
 out:
 	if (parent) {
 		n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

#2 lore raw
In reply to Paul Moore
Quoted 33 lines · Paul Moore
---
 kernel/auditsc.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
kernel/auditsc.c +10 -4
index 21eae3c..ff99c05 100644
@@ -1886,12 +1886,18 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 	}
 
 out_alloc:
-	/* unable to find the name from a previous getname(). Allocate a new
-	 * anonymous entry.
-	 */
-	n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
+	/* unable to find an entry with both a matching name and type */
+	n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
 	if (!n)
 		return;
+	if (name)
+		/* since name is not NULL we know there is already a matching
+		 * name record, see audit_getname(), so there must be a type
+		 * mismatch; reuse the string path since the original name
+		 * record will keep the string valid until we free it in
+		 * audit_free_names() */
+		n->name = name;
+
 out:
 	if (parent) {
 		n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;
Quoted 2 lines

.

Hi Paul,

Thanks for your work~! But I'm sorry to say I've tested this patch with a kernel 3.10.53 and met a panic while booting. I think it's caused by this patch.

Could you please take some time to look at this? Did I do something wrong?

Thanks~!

Hu

INIT: Entering runlevel: 3
Starting OpenBSD Secure Shell server: sshd
done.
Starting audit daemon auditd

[   25.257694] type=1305 audit(1417530900.169:2): audit_pid=1348 old=0 auid=4294967295 ses=4294967295
[   25.257694]  res=1

Starting domain name service: namedwrote key file "/etc/bind/rndc.key" .
hwclock: can't open '/dev/misc/rtc': No such file or directory Starting ntpd: done
Starting syslog-ng:[ 25.623155] Unable to handle kernel NULL pointer dereference at virtual address 00000001

[   25.631287] pgd = c5a1c000
[   25.633994] [00000001] *pgd=85880831, *pte=00000000, *ppte=00000000
[   25.640295] Internal error: Oops: 17 [#1] SMP ARM
[   25.644993] Modules linked in: ipv6
[   25.648507] CPU: 0 PID: 1375 Comm: syslog-ng Not tainted 3.10.53 #1
[   25.655286] task: ef34ac00 ti: c5ae6000 task.ti: c5ae6000
[   25.660681] PC is at strlen+0xc/0x20
[   25.664264] LR is at audit_compare_dname_path+0x20/0x68
[   25.669484] pc : [<c01906f0>]    lr : [<c007fe30>]    psr: 600f0013
[   25.669484] sp : c5ae7e58  ip : 00000000  fp : ef349c44
[   25.680944] r10: 0000c1ed  r9 : ef26c1a8  r8 : ee74ef0c
[   25.686162] r7 : ee74eee0  r6 : 00000003  r5 : 00000001  r4 : 00000005
[   25.692679] r3 : 00000002  r2 : 00000001  r1 : 00000000  r0 : 00000001
[   25.699198] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[   25.706323] Control: 18c53c7d  Table: 85a1c04a  DAC: 00000015
[   25.712061] Process syslog-ng (pid: 1375, stack limit = 0xc5ae6238)
[   25.718319] Stack: (0xc5ae7e58 to 0xc5ae8000)
[   25.722672] 7e40:                                                       ef349c00 00000000
[   25.730841] 7e60: ef349dd8 ee74eee0 ee74ef0c c0080504 ef26c1a8 00000004 00000004 ef26c1a8
[   25.739009] 7e80: c5815680 ee74eee0 0000c1ed 00000000 00000001 0000c1ed 0000000b c00fa2c4
[   25.747178] 7ea0: ef26c1a8 ee74eee0 dd79fc00 c5815680 00000000 ee74eee0 c581581c c02b6550
[   25.755346] 7ec0: c5bfd015 c5bfd010 00000000 c048e000 ef26c1a8 00000001 00000002 c5ae6000
[   25.763514] 7ee0: dd9b96d0 ee71ac38 c5ae7f18 eec45800 0000000b 01357070 0000011a c000e1e4
[   25.771682] 7f00: c5ae6000 00000200 00000000 c022fcf4 00000000 00000000 642f0001 6c2f7665
[   25.779850] 7f20: 0000676f dd7eb400 ef34ac00 c04a6270 c5ae7f48 c04a6368 00000001 c0081d14
[   25.788016] 7f40: c5ae7f48 000000c3 ef349c00 ef349c00 00000001 0000011a ef349c00 00000001
[   25.796183] 7f60: c5ae7f68 c0082108 547dce14 202fbeff 00000008 c5ae7f88 c5ae6000 0000011a
[   25.804351] 7f80: 0000011a c001037c 0000000b 01357060 0000000b 01357060 01357060 00000008
[   25.812520] 7fa0: beaf8a2c c000e1c8 01357060 00000008 00000008 01357070 0000000b 01357060
[   25.820687] 7fc0: 01357060 00000008 beaf8a2c 0000011a 01350ba8 00000000 4fa97000 00000000
[   25.828855] 7fe0: b6d8e870 beaf88ec b6f43ee0 b6d8e87c 600f0010 00000008 af7fd821 af7fdc21
[   25.837031] [<c01906f0>] (strlen+0xc/0x20) from [<c007fe30>] (audit_compare_dname_path+0x20/0x68)
[   25.845899] [<c007fe30>] (audit_compare_dname_path+0x20/0x68) from [<c0080504>] (__audit_inode_child+0x124/0x26c)
[   25.856153] [<c0080504>] (__audit_inode_child+0x124/0x26c) from [<c00fa2c4>] (vfs_mknod+0x138/0x158)
[   25.865285] [<c00fa2c4>] (vfs_mknod+0x138/0x158) from [<c02b6550>] (unix_bind+0x114/0x2b8)
[   25.873552] [<c02b6550>] (unix_bind+0x114/0x2b8) from [<c022fcf4>] (SyS_bind+0x5c/0x80)
[   25.881556] [<c022fcf4>] (SyS_bind+0x5c/0x80) from [<c000e1c8>] (__sys_trace_return+0x0/0x18)
[   25.890072] Code: c02f1948 e1a03000 e1a02003 e2833001 (e5d21000)
[   25.896176] ---[ end trace 2f04133705b763f6 ]---
[   25.900790] Kernel panic - not syncing: Fatal exception
#3 lore raw

This is configure options in my environment. I hope it would help you~!

# 5.2 audit configuration
# 5.2.1

# 5.2.2 Stop system when log is full
configuration modify "/etc/audit/auditd.conf@space_left_action = SYSLOG@space_left_action = SYSLOG" #configuration modify "/etc/audit/auditd.conf@admin_space_left_action = SUSPEND@admin_space_left_action = HALT" configuration modify "/etc/audit/auditd.conf@space_left = 75@space_left = 2"
configuration modify "/etc/audit/auditd.conf@admin_space_left = 50@admin_space_left = 1"

# 5.2.3
configuration modify "/etc/audit/auditd.conf@max_log_file_action = ROTATE@max_log_file_action = ROTATE" configuration modify "/etc/audit/auditd.conf@max_log_file = 6@max_log_file = 5"

# 5.2.4 Audit syscall for reset system time
configuration add "/etc/audit/audit.rules@@-w /etc/group -p wa -k identity" configuration add "/etc/audit/audit.rules@@-w /etc/passwd -p wa -k identity" # 5.2.6
configuration add "/etc/audit/audit.rules@@-w /etc/issue -p wa -k system-locale" configuration add "/etc/audit/audit.rules@@-w /etc/issue.net -p wa -k system-locale" # 5.2.7
configuration add "/etc/audit/audit.rules@@-w /etc/selinux/ -p wa -k MAC-policy"

# 5.2.8
configuration add "/etc/audit/audit.rules@@-w /var/log/faillog -p wa -k logins" configuration add "/etc/audit/audit.rules@@-w /var/log/lastlog -p wa -k logins"

# 5.2.9
configuration add "/etc/audit/audit.rules@@-w /var/run/utmp -p wa -k session" configuration add "/etc/audit/audit.rules@@-w /var/log/wtmp -p wa -k session"

# 5.2.10
configuration add "/etc/audit/audit.rules@@-a always,exit -F arch=b32 -S setxattr -S lsetxattr -S fsetxattr -S removexattr -S lremovexattr -S fremovexattr -F auid!=4294967295 -k perm_mod"

# 5.2.11
configuration add "/etc/audit/audit.rules@@-a always,exit -F arch=b32 -S creat -S open -S openat -S truncate -S ftruncate -F exit=-EACCES -F auid!=4294967295 -k access" configuration add "/etc/audit/audit.rules@@-a always,exit -F arch=b32 -S creat -S open -S openat -S truncate -S ftruncate -F exit=-EPERM -F auid!=4294967295 -k access"

# 5.2.12

# 5.2.13
configuration add "/etc/audit/audit.rules@@-a always,exit -F arch=b32 -S unlink -S unlinkat -S rename -S renameat -S rmdir -F auid!=4294967295 -k delete"

# 5.2.14
configuration add "/etc/audit/audit.rules@@-w /etc/sudoers -p wa -k scope"

# 5.2.15
#configuration add "/etc/audit/audit.rules@@-e 2"

author
#4 lore raw
In reply to hujianyang
Quoted 8 lines · hujianyang

Hi Paul,

Thanks for your work~! But I'm sorry to say I've tested this patch with a kernel 3.10.53 and met a panic while booting. I think it's caused by this patch.

Could you please take some time to look at this? Did I do something wrong?

...

Quoted 7 lines · hujianyang

This is configure options in my environment. I hope it would help you~!

# 5.2 audit configuration
# 5.2.1

Quoted 7 lines

# 5.2.2 Stop system when log is full
configuration modify "/etc/audit/auditd.conf@space_left_action = SYSLOG@space_left_action = SYSLOG" #configuration modify "/etc/audit/auditd.conf@admin_space_left_action = SUSPEND@admin_space_left_action = HALT" configuration modify "/etc/audit/auditd.conf@space_left = 75@space_left = 2" configuration modify "/etc/audit/auditd.conf@admin_space_left = 50@admin_space_left = 1"

Thanks for taking the time to test, however, a few things ...

First, could you provide the /etc/audit/auditd.conf and /etc/audit/audit.rules files you used for your testing? I don't understand configuration script/language you used above.

Second, I tested the patch against the audit tree's stable-3.18 branch, could you (re)test against 3.18-rcX instead of 3.10.X? There have been a number of changes to the audit subsystem since 3.10 was released and it would surprise me if the patch I posted has problems on 3.10.X.

* git://git.infradead.org/users/pcmoore/audit stable-3.18

Thanks,
-Paul

Signature · 2 lines
paul moore
security and virtualization @ redhat
#5 lore raw
Quoted 4 lines · Paul Moore

First, could you provide the /etc/audit/auditd.conf and /etc/audit/audit.rules files you used for your testing? I don't understand configuration script/language you used above.

/etc/audit/audit.conf

#
# This file controls the configuration of the audit daemon #

log_file = /var/log/audit/audit.log
log_format = RAW
log_group = root
priority_boost = 4
flush = INCREMENTAL
freq = 20
num_logs = 5
disp_qos = lossy
dispatcher = /sbin/audispd
name_format = NONE
##name = mydomain
max_log_file = 5
max_log_file_action = ROTATE
space_left = 2
space_left_action = SYSLOG
action_mail_acct = root
admin_space_left = 1
admin_space_left_action = SUSPEND
disk_full_action = SUSPEND
disk_error_action = SUSPEND
##tcp_listen_port =
tcp_listen_queue = 5
tcp_max_per_addr = 1
##tcp_client_ports = 1024-65535
tcp_client_max_idle = 0
enable_krb5 = no
krb5_principal = auditd
##krb5_key_file = /etc/audit/audit.key

/etc/audit/audit.rules:

# This file contains the auditctl rules that are loaded

# whenever the audit daemon is started via the initscripts.
# The rules are simply the parameters that would be passed
# to auditctl.

# First rule - delete all
-D

# Increase the buffers to survive stress events.
# Make this bigger for busy systems
-b 320

# Feel free to add below this line. See auditctl man page

-a always,exit -F arch=b32 -S adjtimex -S settimeofday -S stime -k time-change
-a always,exit -F arch=b32 -S clock_settime -k time-change
-w /etc/localtime -p wa -k time-change
-w /etc/group -p wa -k identity
-w /etc/passwd -p wa -k identity
-w /etc/gshadow -p wa -k identity
-w /etc/shadow -p wa -k identity
-w /etc/security/opasswd -p wa -k identity
-a always,exit -F arch=b32 -S sethostname -S setdomainname -k system-locale
-w /etc/issue -p wa -k system-locale
-w /etc/issue.net -p wa -k system-locale
-w /etc/hosts -p wa -k system-locale
-w /etc/selinux/ -p wa -k MAC-policy
-w /var/log/faillog -p wa -k logins
-w /var/log/lastlog -p wa -k logins
-w /var/run/utmp -p wa -k session
-w /var/log/wtmp -p wa -k session
-a always,exit -F arch=b32 -S chmod -S fchmod -S fchmodat -F auid!=4294967295 -k perm_mod
-a always,exit -F arch=b32 -S chown -S fchown -S fchownat -S lchown -S chown32 -S fchown32 -S lchown32 -F auid!=429496 -a always,exit -F arch=b32 -S setxattr -S lsetxattr -S fsetxattr -S removexattr -S lremovexattr -S fremovexattr -F aui -a always,exit -F arch=b32 -S creat -S open -S openat -S truncate -S ftruncate -F exit=-EACCES -F auid!=4294967295 -k -a always,exit -F arch=b32 -S creat -S open -S openat -S truncate -S ftruncate -F exit=-EPERM -F auid!=4294967295 -k a -a always,exit -F arch=b32 -S unlink -S unlinkat -S rename -S renameat -S rmdir -F auid!=4294967295 -k delete -w /etc/sudoers -p wa -k scope

Quoted 8 lines

Second, I tested the patch against the audit tree's stable-3.18 branch, could you (re)test against 3.18-rcX instead of 3.10.X? There have been a number of changes to the audit subsystem since 3.10 was released and it would surprise me if the patch I posted has problems on 3.10.X.

* git://git.infradead.org/users/pcmoore/audit stable-3.18

Sorry, my testing environment is built on a embedded arm device. Changing kernel version need lots of changes for device driver which is beyond my ability.

I wish you could implement my configuration on your environment and test if it's OK. After that, we can list the changes from 3.10 stable to 3.18 stable.

Thank you again~!

Hu

author
#6 lore raw
Quoted 10 lines · hujianyang
Quoted 3 lines · Paul Moore

First, could you provide the /etc/audit/auditd.conf and /etc/audit/audit.rules files you used for your testing? I don't understand configuration script/language you used above.

/etc/audit/audit.conf

#
# This file controls the configuration of the audit daemon #

... {snip} ...

Quoted 6 lines

/etc/audit/audit.rules:

# This file contains the auditctl rules that are loaded

# whenever the audit daemon is started via the initscripts.
# The rules are simply the parameters that would be passed
# to auditctl.

... {snip} ...

I setup my system using your configuration and the system booted and ran the regression test described in the patch description without problem. I know of at least one other person that has tested this patch without problem as well.

Quoted 10 lines
Quoted 6 lines

Second, I tested the patch against the audit tree's stable-3.18 branch, could you (re)test against 3.18-rcX instead of 3.10.X? There have been a number of changes to the audit subsystem since 3.10 was released and it would surprise me if the patch I posted has problems on 3.10.X.

* git://git.infradead.org/users/pcmoore/audit stable-3.18

Sorry, my testing environment is built on a embedded arm device. Changing kernel version need lots of changes for device driver which is beyond my ability.

I know that many embedded systems include several kernel patches that deviate from the upstream sources (device drivers, etc.), is that the case with your system?

Quoted 3 lines

I wish you could implement my configuration on your environment and test if it's OK. After that, we can list the changes from 3.10 stable to 3.18 stable.

I did test your configuration, without problem. I suspect there is some sort of conflict between the patch and one of the kernel patches in your system. Is there any chance you can debug the problem you saw?

I'm going to remove the CC:stable from the patch description to be safe, but as of right now I think it is reasonable to include the patch in the audit next branch.

Signature · 2 lines
paul moore
security and virtualization @ redhat
#7 lore raw
Quoted 5 lines · Paul Moore

I setup my system using your configuration and the system booted and ran the regression test described in the patch description without problem. I know of at least one other person that has tested this patch without problem as well.

OK~

Quoted 5 lines

I know that many embedded systems include several kernel patches that deviate from the upstream sources (device drivers, etc.), is that the case with your system?

I'm not sure, probably not.

Quoted 5 lines

I did test your configuration, without problem. I suspect there is some sort of conflict between the patch and one of the kernel patches in your system. Is there any chance you can debug the problem you saw?

We have a plan to upgrade kernel version from linux 3.10 to linux 3.18. My colleagues are about to testing your patch with new kernel next week. So maybe we can find the cause of the panic in my environment in a few days.

Quoted 4 lines

I'm going to remove the CC:stable from the patch description to be safe, but as of right now I think it is reasonable to include the patch in the audit next branch.

Go ahead~

I will inform you if we fix the panic in my environment as soon as possible.

Thanks.

Hu