1:54 p.m.
Hi,
A new version of auditd has been released. You can download it from:
http://people.redhat.com/sgrubb/audit
Some of the changes:
- Add R option to auditctl to allow reading rules from file.
- Do not allow task creation list to have syscall auditing
- Add D option to allow deleting all rules with 1 command
- Added pam_audit man page & sample.rules
- Mod initscript to call auditctl to load rules at start-up
- Write message to log file for daemon start up
- Write message that daemon is shutting down
- Modify auditd shutdown to wait until logger thread is finished
- Fix bug where extra info was appended to some messages
This version now supports reading a set of rules when the daemon is started.
Edit the file: /etc/audit.rules and place the audit ctl commands. There is a
sample audit rules file included. Look for sample.rules.
Compiled versions will be available in rawhide tomorrow morning.
-Steve Grubb
2:19 p.m.
* Steve Grubb (sgrubb(a)redhat.com) wrote:
Some of the changes:
[snip]
- Fix bug where extra info was appended to some messages
Is this the string termination fix from Junji (that was likely causing
the corrupted messages reported by selinux folks, peter m, and myself)?
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
2:29 p.m.
On Friday 11 February 2005 15:19, you wrote:
Is this the string termination fix from Junji
Yes.
(that was likely causing the corrupted messages reported by selinux
folks,
peter m, and myself)?
Sort of. It fixes the one you saw. However, the corruption Peter was chasing
is probably not related. This was a userspace fix. I think there is a
separate kernel side one that's been discussed in the SE Linux mail list.
-Steve Grubb
2:22 p.m.
On Fri, 2005-02-11 at 15:29, Steve Grubb wrote:
Sort of. It fixes the one you saw. However, the corruption Peter was
chasing
is probably not related. This was a userspace fix. I think there is a
separate kernel side one that's been discussed in the SE Linux mail list.
Yes, we saw corruption in the SELinux avc messages prior to any use of
auditd at all, when everything was still being handled by klogd.
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
2:31 p.m.
* Stephen Smalley (sds(a)epoch.ncsc.mil) wrote:
On Fri, 2005-02-11 at 15:29, Steve Grubb wrote:
> Sort of. It fixes the one you saw. However, the corruption Peter was chasing
> is probably not related. This was a userspace fix. I think there is a
> separate kernel side one that's been discussed in the SE Linux mail list.
Yes, we saw corruption in the SELinux avc messages prior to any use of
auditd at all, when everything was still being handled by klogd.
Ah, I missed that, thanks.
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
5:10 a.m.
Stephen Smalley wrote:
>Sort of. It fixes the one you saw. However, the corruption Peter
was chasing
>is probably not related. This was a userspace fix. I think there is a
>separate kernel side one that's been discussed in the SE Linux mail list.
Yes, we saw corruption in the SELinux avc messages prior to any use of
auditd at all, when everything was still being handled by klogd.
This is my guess though line 356 and 372 in audit.c looks suspicious.
audit_log_format(ab, "login pid=%d uid=%d loginuid=%d"
" length=%d msg='%.1024s'",
pid, uid,
login->loginuid,
login->msglen,
login->msg);
It assumes msg is C string but guess if it is not. It tries to print 1024 byes
in worst case. It is probably safer change this line to:
audit_log_format(ab, "login pid=%d uid=%d loginuid=%d"
" length=%d msg='%.*s'",
pid, uid,
login->loginuid,
login->msglen,
login->msglen,
login->msg);
It won't be overhead since either way it passes length.
I noticed it a while back ago but didn't report it 'cuz I'm not 100%
sure if msg string null termination is always guaranteed or not.
If so then there could be some other kernel thread is stomping its tail...
Hope this helps.
-- Junji
1:09 p.m.
Hello,
On Fri, 2005-02-11 at 15:22, Stephen Smalley wrote:
On Fri, 2005-02-11 at 15:29, Steve Grubb wrote:
> Sort of. It fixes the one you saw. However, the corruption Peter was chasing
> is probably not related. This was a userspace fix. I think there is a
> separate kernel side one that's been discussed in the SE Linux mail list.
Yes, we saw corruption in the SELinux avc messages prior to any use of
auditd at all, when everything was still being handled by klogd.
The corruption I
was able to track down was in the kernel, this patch
will fix the problem, (the problem I saw was corrupted avc messages
which always started with 118 leading white spaces). This is not a
final patch as I still need to determine what the issue is with printk.
The "%*.*s" format specifier is not popular in the kernel, iirc it is
only used in a few routines, which include the audit and avc routines.
So yes there is a separate kernel side issue dealing with corrupted avc
messages.
Regards,
Peter
10:34 a.m.
New subject: kernel-2.6.9-5.EL.audit.6 in yum repository.
On Wed, 2005-02-16 at 14:09 -0500, Peter Martuccelli wrote:
The corruption I was able to track down was in the kernel, this
patch
will fix the problem, (the problem I saw was corrupted avc messages
which always started with 118 leading white spaces). This is not a
final patch as I still need to determine what the issue is with
printk.
The "%*.*s" format specifier is not popular in the kernel, iirc it is
only used in a few routines, which include the audit and avc routines.
So yes there is a separate kernel side issue dealing with corrupted
avc messages.
I've included this patch, and some fixes for bugs in the IPC code
pointed out by Mounir, in kernel-2.6.9-5.EL.audit.6
ftp://ftp.uk.linux.org/pub/people/dwmw2/audit/
--
dwmw2
8:51 a.m.
On Wed, 2005-02-16 at 14:09 -0500, Peter Martuccelli wrote:
The corruption I was able to track down was in the kernel, this
patch
will fix the problem, (the problem I saw was corrupted avc messages
which always started with 118 leading white spaces). This is not a
final patch as I still need to determine what the issue is with
printk.
This patch is still lurking in the RPM. Is there any sign of a real fix,
or should I add this to the list of stuff to investigate myself?
--
dwmw2
11:03 a.m.
On Thursday 17 March 2005 09:51, David Woodhouse wrote:
This patch is still lurking in the RPM. Is there any sign of a real
fix,
or should I add this to the list of stuff to investigate myself?
I think this is the real fix. There's no reason to use the %*.*s notation. I'd
say send this upstream so SE Linux avc messages will be fixed.
-Steve
2:26 a.m.
* Peter Martuccelli (peterm(a)redhat.com) wrote:
Hello,
On Fri, 2005-02-11 at 15:22, Stephen Smalley wrote:
> On Fri, 2005-02-11 at 15:29, Steve Grubb wrote:
> > Sort of. It fixes the one you saw. However, the corruption Peter was chasing
> > is probably not related. This was a userspace fix. I think there is a
> > separate kernel side one that's been discussed in the SE Linux mail list.
>
> Yes, we saw corruption in the SELinux avc messages prior to any use of
> auditd at all, when everything was still being handled by klogd.
The corruption I was able to track down was in the kernel, this patch
will fix the problem, (the problem I saw was corrupted avc messages
which always started with 118 leading white spaces). This is not a
final patch as I still need to determine what the issue is with printk.
The "%*.*s" format specifier is not popular in the kernel, iirc it is
only used in a few routines, which include the audit and avc routines.
So yes there is a separate kernel side issue dealing with corrupted avc
messages.
--- linux/kernel/audit.c.orig 2005-02-16 13:49:28.839925080 -0500
+++ linux/kernel/audit.c 2005-02-16 13:53:24.757060224 -0500
@@ -513,8 +513,8 @@
if (!audit_pid) { /* No daemon */
int offset = ab->nlh ? NLMSG_SPACE(0) : 0;
int len = skb->len - offset;
- printk(KERN_ERR "%*.*s\n",
- len, len, skb->data + offset);
+ skb->data[offset + len] = '\0';
+ printk(KERN_ERR "%s\n", skb->data + offset);
}
kfree_skb(skb);
ab->nlh = NULL;
What was your test case? This patch will potentially corrupt data in
skb->data[offset + len]. Typically there's extra space so it's ok, but
at the most that's pointing at the first member for skb_shared_info
(dataref), which could cause a memory leak.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
3:32 p.m.
* Steve Grubb (sgrubb(a)redhat.com) wrote:
On Wednesday 27 April 2005 03:26, Chris Wright wrote:
> What was your test case? This patch will potentially corrupt data in
> skb->data[offset + len].
Chris,
It may be more expedient to just submit a corrected patch to the mail list.
I don't have a good patch yet. NULL terminating the buffer doesn't look
safe. I was beginning to suspect audit_log_vformat buffer size
handling, but can't make it break (nor figure when ab->len could become
suspect. I'll keep digging, but a way to trigger would sure help.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
3:41 p.m.
On Wednesday 27 April 2005 16:32, Chris Wright wrote:
NULL terminating the buffer doesn't look safe.
It should be. Surely we know how long the buffer is. Admittedly, there's
another bug that's in printk's handling of %*.* stuff. But even with that
fixed, it would be more efficient (timewise) to terminate the string so
printk doesn't have to interpret the *.* stuff.
-Steve
4:51 p.m.
* Steve Grubb (sgrubb(a)redhat.com) wrote:
On Wednesday 27 April 2005 16:32, Chris Wright wrote:
> NULL terminating the buffer doesn't look safe.
It should be. Surely we know how long the buffer is. Admittedly, there's
another bug that's in printk's handling of %*.* stuff. But even with that
fixed, it would be more efficient (timewise) to terminate the string so
printk doesn't have to interpret the *.* stuff.
We know how long the buffer is, but the NULL byte is not in the buffer.
So we either overwrite the last byte of the buffer, or the first byte of
the next thing in memory. Most of the time, this will happen to work
out fine because the skb payload is padded with some alignment, etc, but
on the rare case that there is no extra space, this is silent data
corruption of data that the kernel uses. I _think_ we could add the
NULL byte, but my concern is that something else in the buffer lenght
accounting is broken. In the worst case, that brokeness will copy
random kernel data to userspace as part of audit messages.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
5:02 p.m.
On Wednesday 27 April 2005 17:51, Chris Wright wrote:
We know how long the buffer is, but the NULL byte is not in the
buffer.
So we either overwrite the last byte of the buffer, or the first byte of
the next thing in memory.
I think the intent was to overwrite the last thing in the buffer. One of my
concerns has been that legally, paths can be 4096 bytes. There is a note in
the audit.c file that says we are limiting ourselves to 1024 bytes because of
printk limitations. So it we've accepted that we can't printk full file
names, what's wrong with losing 1 byte?
-Steve
5:12 p.m.
* Steve Grubb (sgrubb(a)redhat.com) wrote:
On Wednesday 27 April 2005 17:51, Chris Wright wrote:
> We know how long the buffer is, but the NULL byte is not in the buffer.
> So we either overwrite the last byte of the buffer, or the first byte of
> the next thing in memory.
I think the intent was to overwrite the last thing in the buffer. One of my
concerns has been that legally, paths can be 4096 bytes. There is a note in
the audit.c file that says we are limiting ourselves to 1024 bytes because of
printk limitations. So it we've accepted that we can't printk full file
names, what's wrong with losing 1 byte?
I had hoped to find the actual bug (esp. since I'm not convinced it's
a vsprintf bug in kernel). Short of that, I agree, chopping off last
byte is doable.
thanks,
-chris
2:31 p.m.
* Steve Grubb (sgrubb(a)redhat.com) wrote:
On Friday 11 February 2005 15:19, you wrote:
> Is this the string termination fix from Junji
Yes.
Great, thanks.
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
3:45 p.m.
Thanks Steve!
Looks like you are making good progress. Any trouble spots?
Merry
On Fri, 2005-02-11 at 14:54, Steve Grubb wrote:
Hi,
A new version of auditd has been released. You can download it from:
http://people.redhat.com/sgrubb/audit
Some of the changes:
- Add R option to auditctl to allow reading rules from file.
- Do not allow task creation list to have syscall auditing
- Add D option to allow deleting all rules with 1 command
- Added pam_audit man page & sample.rules
- Mod initscript to call auditctl to load rules at start-up
- Write message to log file for daemon start up
- Write message that daemon is shutting down
- Modify auditd shutdown to wait until logger thread is finished
- Fix bug where extra info was appended to some messages
This version now supports reading a set of rules when the daemon is started.
Edit the file: /etc/audit.rules and place the audit ctl commands. There is a
sample audit rules file included. Look for sample.rules.
Compiled versions will be available in rawhide tomorrow morning.
-Steve Grubb
--
Linux-audit mailing list
Linux-audit(a)redhat.com
http://www.redhat.com/mailman/listinfo/linux-audit
--
Merry Beekman
Sr. Program Marketing Manager
Red Hat
1608 Spring Hill Rd. Suite 350
Vienna, VA 22182
703.356.2803 x62401
4:38 p.m.
On Friday 11 February 2005 16:45, Merry Beekman wrote:
Looks like you are making good progress. Any trouble spots?
The only challenge is the kernel abi for FC4. If that were to pick up some of
the kernel patches floating around, I could probably do a 0.6.3 to get FC4
closer to something usable. That would mean picking up the loginuid stuff.
Everything else would fall into 0.7 as planned.
-Steve
7285
days inactive
7360
days old
linux-audit@lists.linux-audit.osci.io
19 comments
7 participants
participants (7)
-
Chris Wright -
David Woodhouse -
Junji Kanemaru -
Merry Beekman -
Peter Martuccelli -
Stephen Smalley -
Steve Grubb