race in audit_log_untrusted_string for task_struct::comm
by Richard Guy Briggs
Hi,
I'm investigating a race in audit_log_untrusted_string() in the case of
task_struct::comm.
Originally from commit 0a4ff8c2 audit_log_task() currently hands
task_struct::comm directly to audit_log_untrusted_string() which can
race if another task/thread on another CPU modifies it between the time
strlen() is called on it and the time it is processed in
audit_log_n_untrustedstring(). I originally thought it was just a
matter of the value being truncated or mixed between old and new values,
but it appears it is worse than that in that it causes following labels
to be dropped. If it just affected the value of comm that was printed,
I am guessing this wouldn't be a big deal since the user can modify this
value anyway and we never did trust it. however, since it affects the
rest of the line, it has to be addressed.
In commit 219f0817 from 2005, Stephen Smalley used get_task_comm() to
get the value of comm to log to audit_log_untrusted_string() which calls
the task_lock. This is quite safe, but incurs overhead that may not be
found acceptable by some.
Eric subsequently used memcpy() in c2a7780e in 2008 in another area of the
code that stores the value for later rather than use it immediately, so
the race issue was not present there, but there is still the danger of
incomplete or mixed text in that field.
Both are safe in terms of avoiding losing other fields. One might have
half-inconsistent information in it, the other incurs locking costs.
I'm inclined to go get_task_comm() in all 5 locations, but if we care
more about locking overhead, I'll switch to memcpy().
Steve, do we care about the integrity of the comm field?
Eric, is the overhead of task_lock unacceptable?
Linked in this thread (should I be able to convince git send-email to
work with me) please find the get_task_comm() patch and the alternate
memcpy() patch.
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
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
11 years, 1 month
audit 2.3.5 released
by Steve Grubb
Hi,
I've just released a new version of the audit daemon. It can be downloaded
from http://people.redhat.com/sgrubb/audit. It will also be in rawhide
soon. The ChangeLog is:
- In CRYPTO_KEY_USER events, do not interpret the 'fp' field
- Change formatting of rules listing in auditctl to look like audit.rules
- Change auditctl to do all netlink comm and then print rules
- Add a debug option to ausearch to find skipped events
- Parse subject, auid, and ses in LOGIN events (3.14 kernel changed format)
- In auditd, when shifting logs, ignore the num_logs setting (#950158)
- Allow passing a directory as the input file for ausearch/report (LC Bruzenak)
- Interpret syscall fields in SECCOMP events
- Increase a couple buffers to handle longer input
This release cleans up parsing and interpreting a couple events. There is now
a --debug option to ausearch which will output to stderr any events that
ausearch cannot parse correctly. If anyone finds events using this option, I'd
really like to hear about it.
A big change in this release is that listing rules with auditctl now formats
the output to look like audit.rules.
One other feature to call out is that when aureport/search is passed a file
using the -if option, you can now pass a directory which will allow searching
multiple files. The files still need to have the same numbering scheme that is
currently expected. Thanks goes to Lenny Bruzenak for this.
Please let me know if you run across any problems with this release.
-Steve
11 years, 1 month
[PATCH 00/12] RFC: steps to make audit pid namespace-safe
by Richard Guy Briggs
This patchset is a revival of some of Eric Biederman's work to make audit
pid-namespace-safe.
In a couple of places, audit was printing PIDs in the task's pid namespace
rather than relative to the audit daemon's pid namespace, which currently is
init_pid_ns.
It also allows processes to log audit user messages in their own pid
namespaces, which was not previously permitted. Please see:
https://bugzilla.redhat.com/show_bug.cgi?id=947530
https://bugs.launchpad.net/ubuntu/+source/vsftpd/+bug/1160372
https://bugzilla.novell.com/show_bug.cgi?id=786024
Part of the cleanup here involves deprecating task->pid and task->tgid, which
are error-prone duplicates of the task->pids structure
The next step which I hope to add to this patchset will be to purge task->pid
and task->tgid from the rest of the kernel if possible. Once that is done,
task_pid_nr_init_ns() and task_tgid_nr_init_ns() that were introduced in patch
05/12 and used in patches 06/12 and 08/12 could be replaced with task_pid_nr()
and task_tgid_nr(). Eric B. did take a stab at that, but checking all the
subtleties will be non-trivial.
Does anyone have any opinions or better yet hard data on cache line misses
between pid_nr(struct pid*) and pid_nr_ns(struct pid*, &init_pid_ns)? I'd
like to see pid_nr() use pid_nr_ns(struct pid*, &init_pid_ns), or
pid_nr_init_ns() eliminated in favour of the original pid_nr(). pid_nr()
currently accesses the first level of the pid structure without having to
dereference the level number. If there is an actual speed difference, it could
be worth keeping, otherwise, I'd prefer to simplify that code.
Eric also had a patch to add a printk option to format a struct pid pointer
which was PID namespace-aware. I don't see the point, but I'll let him explain
it.
Discuss.
Eric W. Biederman (5):
audit: Kill the unused struct audit_aux_data_capset
audit: Simplify and correct audit_log_capset
Richard Guy Briggs (7):
audit: fix netlink portid naming and types
pid: get ppid pid_t of task in init_pid_ns safely
audit: convert PPIDs to the inital PID namespace.
pid: get pid_t of task in init_pid_ns correctly
audit: store audit_pid as a struct pid pointer
audit: anchor all pid references in the initial pid namespace
pid: modify task_pid_nr to work without task->pid.
pid: modify task_tgid_nr to work without task->tgid.
pid: rewrite task helper functions avoiding task->pid and task->tgid
pid: mark struct task const in helper functions
drivers/tty/tty_audit.c | 3 +-
include/linux/audit.h | 8 ++--
include/linux/pid.h | 6 +++
include/linux/sched.h | 81 ++++++++++++++++++++++++----------
kernel/audit.c | 76 +++++++++++++++++++------------
kernel/audit.h | 12 +++---
kernel/auditfilter.c | 35 +++++++++++----
kernel/auditsc.c | 36 ++++++---------
kernel/capability.c | 2 +-
kernel/pid.c | 4 +-
security/apparmor/audit.c | 7 +--
security/integrity/integrity_audit.c | 2 +-
security/lsm_audit.c | 11 +++--
security/tomoyo/audit.c | 2 +-
14 files changed, 177 insertions(+), 108 deletions(-)
11 years, 1 month
[RFC PATCH] audit: generic compat system call support
by AKASHI Takahiro
Arm64 supports 32-bit mode(AArch32) and 64-bit mode(AArch64).
To enable audit support, we want to avoid duplicating lib/audit.c
as other arch's do, and instead to use lib/audit.c and extend/re-work it
in order to support compat system calls as well.
Changes are nothing fancy, just copying lib/audit.c and adding hooks
for compat system calls as done in other arch's.
Once this patch is accepted, my aarch64 patch will be rebased on top
of this.
(If you want, I can submit it immediately because it is already working.)
AKASHI Takahiro (1):
audit: Add generic compat syscall support
include/linux/audit.h | 3 +++
lib/Makefile | 3 +++
lib/audit.c | 10 ++++++++
lib/compat_audit.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 80 insertions(+)
create mode 100644 lib/compat_audit.c
--
1.7.9.5
11 years, 1 month
[PATCH] audit: Use struct net not pid_t to remember the network namespce to reply in
by Eric W. Biederman
While reading through 3.14-rc1 I found a pretty siginficant mishandling
of network namespaces in the recent audit changes.
In struct audit_netlink_list and audit_reply add a reference to the
network namespace of the caller and remove the userspace pid of the
caller. This cleanly remembers the callers network namespace, and
removes a huge class of races and nasty failure modes that can occur
when attempting to relook up the callers network namespace from a pid_t
(including the caller's network namespace changing, pid wraparound, and
the pid simply not being present).
Signed-off-by: "Eric W. Biederman" <ebiederm(a)xmission.com>
---
kernel/audit.c | 10 ++++++----
kernel/audit.h | 2 +-
kernel/auditfilter.c | 3 ++-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 34c5a2310fbf..1e5756f16f6f 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -182,7 +182,7 @@ struct audit_buffer {
struct audit_reply {
__u32 portid;
- pid_t pid;
+ struct net *net;
struct sk_buff *skb;
};
@@ -500,7 +500,7 @@ int audit_send_list(void *_dest)
{
struct audit_netlink_list *dest = _dest;
struct sk_buff *skb;
- struct net *net = get_net_ns_by_pid(dest->pid);
+ struct net *net = dest->net;
struct audit_net *aunet = net_generic(net, audit_net_id);
/* wait for parent to finish and send an ACK */
@@ -510,6 +510,7 @@ int audit_send_list(void *_dest)
while ((skb = __skb_dequeue(&dest->q)) != NULL)
netlink_unicast(aunet->nlsk, skb, dest->portid, 0);
+ put_net(net);
kfree(dest);
return 0;
@@ -543,7 +544,7 @@ out_kfree_skb:
static int audit_send_reply_thread(void *arg)
{
struct audit_reply *reply = (struct audit_reply *)arg;
- struct net *net = get_net_ns_by_pid(reply->pid);
+ struct net *net = reply->net;
struct audit_net *aunet = net_generic(net, audit_net_id);
mutex_lock(&audit_cmd_mutex);
@@ -552,6 +553,7 @@ static int audit_send_reply_thread(void *arg)
/* Ignore failure. It'll only happen if the sender goes away,
because our timeout is set to infinite. */
netlink_unicast(aunet->nlsk , reply->skb, reply->portid, 0);
+ put_net(net);
kfree(reply);
return 0;
}
@@ -583,8 +585,8 @@ static void audit_send_reply(__u32 portid, int seq, int type, int done,
if (!skb)
goto out;
+ reply->net = get_net(current->nsproxy->net_ns);
reply->portid = portid;
- reply->pid = task_pid_vnr(current);
reply->skb = skb;
tsk = kthread_run(audit_send_reply_thread, reply, "audit_send_reply");
diff --git a/kernel/audit.h b/kernel/audit.h
index 57cc64d67718..8df132214606 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -247,7 +247,7 @@ extern void audit_panic(const char *message);
struct audit_netlink_list {
__u32 portid;
- pid_t pid;
+ struct net *net;
struct sk_buff_head q;
};
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 14a78cca384e..a5e3d73d73e4 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -29,6 +29,7 @@
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/security.h>
+#include <net/net_namespace.h>
#include "audit.h"
/*
@@ -1083,8 +1084,8 @@ int audit_list_rules_send(__u32 portid, int seq)
dest = kmalloc(sizeof(struct audit_netlink_list), GFP_KERNEL);
if (!dest)
return -ENOMEM;
+ dest->net = get_net(current->nsproxy->net_ns);
dest->portid = portid;
- dest->pid = task_pid_vnr(current);
skb_queue_head_init(&dest->q);
mutex_lock(&audit_filter_mutex);
--
1.7.5.4
11 years, 1 month
scary syslog message (from audit ?)
by Toralf Förster
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Today I observed this in /var/log/messages with kernel 3.13.6 at a 32 bit Gentoo Linux :
Mar 12 21:20:01 n22 crond[26813]: pam_unix(crond:session): session opened for user root by (uid=0)
Mar 12 21:20:01 n22 kernel: type=1006 audit(1394655601.295:160): pid=26813 uid=0 old auid=4294967295 new auid=0 old ses=4294967295 new ses=159 res=1
Mar 12 21:20:01 n22 CROND[26816]: (root) CMD (test -x /usr/sbin/run-crons && /usr/sbin/run-crons )
Mar 12 21:20:01 n22 CROND[26813]: pam_unix(crond:session): session closed for user root
Mar 12 21:29:01 n22 CROND[25166]: pam_unix(crond:session): session closed for user root
Mar 12 21:30:01 n22 crond[30053]: pam_unix(crond:session): session opened for user root by (uid=0)
Mar 12 21:30:01 n22 CROND[30055]: (root) CMD (test -x /usr/sbin/run-crons && /usr/sbin/run-crons )
Mar 12 21:30:01 n22 kernel: audit: audit_lost=1 audit_rate_limit=0 audit_backlog_limit=64
Mar 12 21:30:01 n22 kernel: type=1006 audit(1394656201.313:161): pid=30053 uid=0 old auid=4294967295 new auid=0 old ses=4294967295 new ses=161 res=1
Mar 12 21:30:01 n22 kernel: audit: printk limit exceeded
Mar 12 21:30:01 n22 kernel: new ses=149 res=1
1
1
@ 40000 KHz), (N/A, 2000 mBm)
<6>cfg80211: (5250000 KHz - 5350000 KHz @ 40000 KHz), (N/A, 2000 mBm)
<6>cfg80211: (5470000 KHz - 5725000 KHz @ 40000 KHz), (N/A, 2698 mBm)
<6>cfg80211: (57240000 KHz - 65880000 KHz @ 2160000 KHz), (N/A, 4000 mBm)
00 mBm)
<6>cfg80211: Calling CRDA for country: DE
ulatory domain
<6>PM: freeze of devices complete after 342.951 msecs
<6>PM: late freeze of devices complete after 0.286 msecs
<6>PM: noirq freeze of devices complete after 1.715 msecs
<6>ACPI: Preparing to enter system sleep state S4
<6>PM: Saving platform NVS memory
<4>Disabling non-boot CPUs ...
<6>kvm: disabling virtualization on CPU1
<6>smpboot: CPU 1 is now offline
<6>kvm: disabling virtualization on CPU2
<6>smpboot: CPU 2 is now offline
<6>kvm: disabling virtualization on CPU3
<6>smpboot: CPU 3 is now offline
<6>PM: Creating hibernation image:
<6>PM: Need to copy 152202 pages
<6>PM: Restoring platform NVS memory
<6>Enabling non-boot CPUs ...
<6>x86: Booting SMP configuration:
<6>smpboot: Booting Node 0 Processor 1 APIC 0x1
<6>Initializing CPU#1
<6>Disabled fast string operations
<6>kvm: enabling virtualization on CPU1
<6>CPU1 is up
<6>smpboot: Booting Node 0 Processor 2 APIC 0x2
<6>Initializing CPU#2
<6>Disabled fast string operations
<6>kvm: enabling virtualization on CPU2
<6>CPU2 is up
<6>smpboot: Booting Node 0 Processor 3 APIC 0x3
<6>Initializing CPU#3
<6>Disabled fast string operations
<6>kvm: enabling virtualization on CPU3
<6>CPU3 is up
<6>ACPI: Waking up from system sleep state S4
<6>thinkpad_acpi: EC reports that Thermal Table has changed
<6>PM: noirq restore of devices complete after 23.354 msecs
<6>PM: early restore of devices complete after 0.211 msecs
<4>usb usb1: root hub lost power or was reset
<7>e1000e 0000:00:19.0: irq 41 for MSI/MSI-X
<4>usb usb2: root hub lost power or was reset
<7>snd_hda_intel 0000:00:1b.0: irq 44 for MSI/MSI-X
<7>ehci-pci 0000:00:1a.0: cache line size of 64 is not supported
<7>ehci-pci 0000:00:1d.0: cache line size of 64 is not supported
<6>[drm] Wrong MCH_SSKPD value: 0x16040307
<6>[drm] This can cause pipe underruns and display issues.
<6>[drm] Please upgrade your BIOS to fix this.
<6>ata5: SATA link down (SStatus 0 SControl 300)
<6>ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
<6>ata4: SATA link down (SStatus 0 SControl 300)
<6>ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
<7>ata1.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
<6>ata1.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
<6>ata1.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
<7>ata2.00: ACPI cmd e3/00:1f:00:00:00:a0 (IDLE) succeeded
<6>usb 1-1: reset high-speed USB device number 2 using ehci-pci
<7>ata2.00: ACPI cmd e3/00:02:00:00:00:a0 (IDLE) succeeded
<6>ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
<7>ata1.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
<6>ata1.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) filtered out
<6>ata1.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
<6>ata1.00: configured for UDMA/100
<7>ata2.00: ACPI cmd e3/00:1f:00:00:00:a0 (IDLE) succeeded
<7>ata2.00: ACPI cmd e3/00:02:00:00:00:a0 (IDLE) succeeded
<6>ata2.00: ACPI cmd ef/10:03:00:00:00:a0 (SET FEATURES) filtered out
<6>ata2.00: configured for UDMA/33
<5>sd 0:0:0:0: [sda] Starting disk
<6>usb 2-1: reset high-speed USB device number 2 using ehci-pci
<6>usb 1-1.1: reset high-speed USB device number 3 using ehci-pci
<6>usb 1-1.6: reset high-speed USB device number 5 using ehci-pci
<6>usb 1-1.4: reset full-speed USB device number 4 using ehci-pci
<6>usb 2-1.2: reset high-speed USB device number 3 using ehci-pci
<6>usb 2-1.5: reset full-speed USB device number 4 using ehci-pci
<6>usb 2-1.2.1: reset low-speed USB device number 5 using ehci-pci
<6>[drm] Enabling RC6 states: RC6 on, RC6p on, RC6pp on
<6>usb 2-1.2.3: reset low-speed USB device number 7 using ehci-pci
<6>iwlwifi 0000:03:00.0: L1 Enabled; Disabling L0S
<6>iwlwifi 0000:03:00.0: Radio type=0x1-0x2-0x0
<6>usb 2-1.2.2: reset full-speed USB device number 6 using ehci-pci
<6>usblp0: removed
<6>PM: restore of devices complete after 2649.424 msecs
<6>usblp 2-1.2.2:1.0: usblp0: USB Bidirectional printer dev 6 if 0 alt 0 proto 2 vid 0x043D pid 0x0078
<4>Restarting tasks ... done.
<6>video LNXVIDEO:00: Restoring backlight state
<6>wlp3s0: authenticate with 08:96:d7:05:f9:2a
<6>wlp3s0: send auth to 08:96:d7:05:f9:2a (try 1/3)
<6>wlp3s0: authenticated
<6>wlp3s0: associate with 08:96:d7:05:f9:2a (try 1/3)
<6>wlp3s0: RX AssocResp from 08:96:d7:05:f9:2a (capab=0x431 status=0 aid=1)
<6>wlp3s0: associated
:
Mar 12 21:30:01 n22 crond[30054]: pam_unix(crond:session): session opened for user root by (uid=0)
Mar 12 21:30:01 n22 CROND[30060]: (root) CMD (/usr/lib/sa/sa1 60 15 )
Mar 12 21:30:01 n22 CROND[30053]: pam_unix(crond:session): session closed for user root
Mar 12 21:37:04 n22 su[32414]: Successful su for root by root
Mar 12 21:37:04 n22 su[32414]: + /dev/pts/9 root:root
Mar 12 21:37:04 n22 su[32414]: pam_unix(su:session): session opened for user root by tfoerste(uid=0)
- --
MfG/Sincerely
Toralf Förster
pgp finger print:1A37 6F99 4A9D 026F 13E2 4DCF C4EA CDDE 0076 E94E
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iF4EAREIAAYFAlMgxp8ACgkQxOrN3gB26U5bkAD/Y3QuDUvzyFSNH15MzbRaAeMZ
+jBeoy2MlW3olxEcp68A/1pG4NeNhNm0vzSNL1BRaLQnUSTrPgnTaHziqqJOrXwh
=8UJV
-----END PGP SIGNATURE-----
11 years, 1 month
Is zero a valid value for the pid member of the AUDIT_SIGNAL_INFO message?
by Richard Guy Briggs
Steve,
Subject says it all...
Is zero a valid value for the pid member of the AUDIT_SIGNAL_INFO message?
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
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
11 years, 1 month
[PATCH 0/5] audit: add restricted capability read-only netlink multicast socket
by Richard Guy Briggs
Hi,
This patch set adds a restricted capability read-only netlink multicast socket
to kaudit to enable userspace clients such as systemd to consume audit logs, in
addition to the existing bidirectional auditd userspace client.
Currently, auditd has the CAP_AUDIT_CONTROL and CAP_AUDIT_WRITE capabilities
(both use CAP_NET_ADMIN). The CAP_AUDIT_READ capability will be added for use
by read-only AUDIT_NLGRP_READLOG multicast group clients to the kaudit
subsystem.
This is accomplished by modifying the optional netlink per-protocol bind
function to return an error code.
https://bugzilla.redhat.com/show_bug.cgi?id=887992
It needs a bit of massage to get past checkpatch.pl...
First posted: https://www.redhat.com/archives/linux-audit/2013-January/msg00008.html
https://lkml.org/lkml/2013/1/27/279
Richard Guy Briggs (5):
audit: move kaudit thread start from auditd registration to kaudit
init
netlink: have netlink per-protocol bind function return an error
code.
audit: add netlink audit protocol bind to check capabilities on
multicast join
audit: add netlink multicast group for log read
audit: send multicast messages only if there are listeners
include/linux/netlink.h | 2 +-
include/uapi/linux/audit.h | 8 ++++
include/uapi/linux/capability.h | 7 +++-
kernel/audit.c | 66 +++++++++++++++++++++++++++++-----
net/netfilter/nfnetlink.c | 6 ++-
net/netlink/af_netlink.c | 30 +++++++++-------
net/netlink/af_netlink.h | 4 +-
security/selinux/include/classmap.h | 2 +-
8 files changed, 95 insertions(+), 30 deletions(-)
11 years, 1 month
[PATCH 0/2] kernel AUDIT_LOGIN event is missing subject label
by Richard Guy Briggs
This was fixed in RHEL6 as BZ 670328, but never upstreamed.
Eric Paris (1):
audit: include subject in login records
Richard Guy Briggs (1):
audit: remove superfluous new- prefix in AUDIT_LOGIN messages
kernel/auditsc.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
11 years, 1 month
