* [PATCH] alpha_table.h: Syscall 511 is getrandom, not renameat2
From: James Clarke @ 2016-07-23 19:15 UTC (permalink / raw)
To: linux-audit; +Cc: James Clarke
---
This fixes gen_alpha_tables_h aborting due to renameat2 being duplicated.
lib/alpha_table.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/alpha_table.h b/lib/alpha_table.h
index 08171cc..c43744f 100644
--- a/lib/alpha_table.h
+++ b/lib/alpha_table.h
@@ -451,6 +451,6 @@ _S(507, "finit_module")
_S(508, "sched_setattr")
_S(509, "sched_getattr")
_S(510, "renameat2")
-_S(511, "renameat2")
+_S(511, "getrandom")
_S(512, "memfd_create")
_S(513, "execveat")
--
2.9.1
^ permalink raw reply related
* Re: [PATCH] audit: fix a double fetch in audit_log_single_execve_arg()
From: Paul Moore @ 2016-07-22 14:10 UTC (permalink / raw)
To: linux-audit; +Cc: Pengfei Wang
In-Reply-To: <CAHC9VhTcFM0YQ-LS=dUZLcYFm=110WZie+BKnGNjHWmJcGc7Dg@mail.gmail.com>
On Wed, Jul 20, 2016 at 2:56 PM, Paul Moore <paul@paul-moore.com> wrote:
> Generally I would take a patch this late for the upcoming merge
> window, but considering the nature of this patch I'm going to make an
> exception. I've added it to the audit#next branch and I've built a
> temporary kernel for testing at the link below for anyone who may be
> interested; I'm also building a new pcmoore/kernel-secnext COPR kernel
> with the patch right now, it should be available in a few hours.
>
> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/394059
Unfortunately there was an issue with the COPR build and the link
above failed, the link below should lead you to a working kernel.
* https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-secnext/build/402631
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] Add .gitignore file for git-svn and github.
From: Steve Grubb @ 2016-07-21 18:56 UTC (permalink / raw)
To: linux-audit; +Cc: Richard Guy Briggs
In-Reply-To: <20160721000625.GA1304@madcap2.tricolour.ca>
On Wednesday, July 20, 2016 8:06:25 PM EDT Richard Guy Briggs wrote:
> On 2016-07-20 19:19, Richard Guy Briggs wrote:
> > Ignore generated files if using git.
Thanks. Applied.
> I should add this this is based on audit-2.6.5.
It applies fine against trunk.
-Steve
^ permalink raw reply
* Re: /var/log/audit ownership/permissions
From: Steve Grubb @ 2016-07-21 14:31 UTC (permalink / raw)
To: Ondrej Moris; +Cc: linux-audit
In-Reply-To: <5790D860.8060508@redhat.com>
On Thursday, July 21, 2016 4:12:48 PM EDT Ondrej Moris wrote:
> On 07/21/2016 03:55 PM, Steve Grubb wrote:
> >> I am fine with that but while I see the motivation [1], I
> >> just cannot find where is that happening in the code.
> >
> > https://fedorahosted.org/audit/browser/trunk/src/auditd-event.c#L886
>
> Thanks, now it is clear. You one thing - line 903 suggests that it is
> either 0700 or 0770 which I can confirm by testing:
>
> # # log_group = root
> # ls -ld /var/log/audit/
> drwx------. 2 root root 4096 Jul 21 09:56 /var/log/audit/
>
> # # log_group = input
> # ls -ld /var/log/audit/
> drwxrwx---. 2 root input 4096 Jul 21 09:56 /var/log/audit/
Fixed in commit 1360.
> >> Besides, specfile
> >> still contains:
> >>
> >> %attr(750,root,root) %dir %{_var}/log/audit
> >
> > Maybe I should take the attr away or modify it to (-,root,-). The group
> > can
> > change. For example, I have wheel allowed to run audit reports on my
> > system.>
> >> and hence 'rpm -V audit' obviously fails.
> >
> > Yeah. Hmm.
>
> Yes, change you mentioned would solve 'rpm -V' problem. It sounds very
> reasonable since both group ownership and permission are configurable
> via auditd.conf.
Also fixed in the same commit.
-Steve
^ permalink raw reply
* Re: /var/log/audit ownership/permissions
From: Ondrej Moris @ 2016-07-21 14:12 UTC (permalink / raw)
To: Steve Grubb, linux-audit
In-Reply-To: <12890758.RtUGNIL9cO@x2>
On 07/21/2016 03:55 PM, Steve Grubb wrote:
> On Thursday, July 21, 2016 11:48:04 AM EDT Ondrej Moris wrote:
>> Hi, I noticed that in 2.6.5 /var/log/audit permission were dropped from
>> 750 to 600.
>
> The directory should be 0750 or 0700 depending on your config. 0600 would be a
> mistake.
Sorry, it was a typo - it should be 0700 (not 0600).
>
>
>> I am fine with that but while I see the motivation [1], I
>> just cannot find where is that happening in the code.
>
> https://fedorahosted.org/audit/browser/trunk/src/auditd-event.c#L886
Thanks, now it is clear. You one thing - line 903 suggests that it is
either 0700 or 0770 which I can confirm by testing:
# # log_group = root
# ls -ld /var/log/audit/
drwx------. 2 root root 4096 Jul 21 09:56 /var/log/audit/
# # log_group = input
# ls -ld /var/log/audit/
drwxrwx---. 2 root input 4096 Jul 21 09:56 /var/log/audit/
>
>> Besides, specfile
>> still contains:
>>
>> %attr(750,root,root) %dir %{_var}/log/audit
>
> Maybe I should take the attr away or modify it to (-,root,-). The group can
> change. For example, I have wheel allowed to run audit reports on my system.
>
>> and hence 'rpm -V audit' obviously fails.
>
> Yeah. Hmm.
Yes, change you mentioned would solve 'rpm -V' problem. It sounds very
reasonable since both group ownership and permission are configurable
via auditd.conf.
>
> -Steve
>
>> [1]
>> http://post-office.corp.redhat.com/archives/tech-list/2016-May/msg00468.html
>>
>> --
>> Ondrej
>>
>> --
>> Linux-audit mailing list
>> Linux-audit@redhat.com
>> https://www.redhat.com/mailman/listinfo/linux-audit
>
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
>
^ permalink raw reply
* Re: /var/log/audit ownership/permissions
From: Steve Grubb @ 2016-07-21 13:55 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <1d3522ae-ff55-5a91-5e8d-b64fac67e84b@redhat.com>
On Thursday, July 21, 2016 11:48:04 AM EDT Ondrej Moris wrote:
> Hi, I noticed that in 2.6.5 /var/log/audit permission were dropped from
> 750 to 600.
The directory should be 0750 or 0700 depending on your config. 0600 would be a
mistake.
> I am fine with that but while I see the motivation [1], I
> just cannot find where is that happening in the code.
https://fedorahosted.org/audit/browser/trunk/src/auditd-event.c#L886
> Besides, specfile
> still contains:
>
> %attr(750,root,root) %dir %{_var}/log/audit
Maybe I should take the attr away or modify it to (-,root,-). The group can
change. For example, I have wheel allowed to run audit reports on my system.
> and hence 'rpm -V audit' obviously fails.
Yeah. Hmm.
-Steve
> [1]
> http://post-office.corp.redhat.com/archives/tech-list/2016-May/msg00468.html
>
> --
> Ondrej
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Re: /var/log/audit ownership/permissions
From: Ondrej Moris @ 2016-07-21 9:48 UTC (permalink / raw)
To: linux-audit
Hi, I noticed that in 2.6.5 /var/log/audit permission were dropped from
750 to 600. I am fine with that but while I see the motivation [1], I
just cannot find where is that happening in the code. Besides, specfile
still contains:
%attr(750,root,root) %dir %{_var}/log/audit
and hence 'rpm -V audit' obviously fails.
[1]
http://post-office.corp.redhat.com/archives/tech-list/2016-May/msg00468.html
--
Ondrej
^ permalink raw reply
* Re: [PATCH] Add .gitignore file for git-svn and github.
From: Richard Guy Briggs @ 2016-07-21 0:06 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <1469056797-23741-1-git-send-email-rgb@redhat.com>
On 2016-07-20 19:19, Richard Guy Briggs wrote:
> Ignore generated files if using git.
I should add this this is based on audit-2.6.5.
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> trunk/.gitignore | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 54 insertions(+), 0 deletions(-)
> create mode 100644 trunk/.gitignore
>
> diff --git a/trunk/.gitignore b/trunk/.gitignore
> new file mode 100644
> index 0000000..a328f80
> --- /dev/null
> +++ b/trunk/.gitignore
> @@ -0,0 +1,54 @@
> +*~
> +*.[oa]
> +*.lo
> +*.la
> +*.pc
> +gen_*_h
> +INSTALL
> +Makefile
> +Makefile.in
> +TAGS
> +/aclocal.m4
> +/autom4te.cache
> +/audit*.tar.gz
> +/audit-rhel?.spec
> +/config.guess
> +/config.sub
> +/config.status
> +/config.log
> +/config.h
> +/config.h.in
> +/configure
> +/compile
> +/depcomp
> +/install-sh
> +/libtool
> +/missing
> +/py-compile
> +.libs/
> +.deps/
> +audisp/audispd
> +audisp/plugins/remote/audisp-remote
> +audisp/plugins/zos-remote/audispd-zos-remote
> +auparse/*tabs.h
> +auparse/epoll_ctls.h
> +auparse/strsplit.c
> +bindings/swig/python/audit.py
> +bindings/swig/python/audit_wrap.c
> +bindings/swig/python3/audit.py
> +bindings/swig/python3/audit_wrap.c
> +lib/*tabs.h
> +lib/*tables.h
> +m4/libtool.m4
> +m4/lt*.m4
> +src/auditctl
> +src/auditd
> +src/aureport
> +src/ausearch
> +src/autrace
> +stamp-h1
> +test-driver
> +tools/aulast/aulast
> +tools/aulastlog/aulastlog
> +tools/ausyscall/ausyscall
> +tools/auvirt/auvirt
> --
> 1.7.1
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* [PATCH] Add .gitignore file for git-svn and github.
From: Richard Guy Briggs @ 2016-07-20 23:19 UTC (permalink / raw)
To: linux-audit; +Cc: Richard Guy Briggs
Ignore generated files if using git.
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
trunk/.gitignore | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 54 insertions(+), 0 deletions(-)
create mode 100644 trunk/.gitignore
diff --git a/trunk/.gitignore b/trunk/.gitignore
new file mode 100644
index 0000000..a328f80
--- /dev/null
+++ b/trunk/.gitignore
@@ -0,0 +1,54 @@
+*~
+*.[oa]
+*.lo
+*.la
+*.pc
+gen_*_h
+INSTALL
+Makefile
+Makefile.in
+TAGS
+/aclocal.m4
+/autom4te.cache
+/audit*.tar.gz
+/audit-rhel?.spec
+/config.guess
+/config.sub
+/config.status
+/config.log
+/config.h
+/config.h.in
+/configure
+/compile
+/depcomp
+/install-sh
+/libtool
+/missing
+/py-compile
+.libs/
+.deps/
+audisp/audispd
+audisp/plugins/remote/audisp-remote
+audisp/plugins/zos-remote/audispd-zos-remote
+auparse/*tabs.h
+auparse/epoll_ctls.h
+auparse/strsplit.c
+bindings/swig/python/audit.py
+bindings/swig/python/audit_wrap.c
+bindings/swig/python3/audit.py
+bindings/swig/python3/audit_wrap.c
+lib/*tabs.h
+lib/*tables.h
+m4/libtool.m4
+m4/lt*.m4
+src/auditctl
+src/auditd
+src/aureport
+src/ausearch
+src/autrace
+stamp-h1
+test-driver
+tools/aulast/aulast
+tools/aulastlog/aulastlog
+tools/ausyscall/ausyscall
+tools/auvirt/auvirt
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] audit: fix a double fetch in audit_log_single_execve_arg()
From: Paul Moore @ 2016-07-20 18:56 UTC (permalink / raw)
To: linux-audit; +Cc: Pengfei Wang
In-Reply-To: <146904022501.25455.10908779521910965267.stgit@localhost>
On Wed, Jul 20, 2016 at 2:43 PM, Paul Moore <pmoore@redhat.com> wrote:
> From: Paul Moore <paul@paul-moore.com>
>
> There is a double fetch problem in audit_log_single_execve_arg()
> where we first check the execve(2) argumnets for any "bad" characters
> which would require hex encoding and then re-fetch the arguments for
> logging in the audit record[1]. Of course this leaves a window of
> opportunity for an unsavory application to munge with the data.
>
> This patch reworks things by only fetching the argument data once[2]
> into a buffer where it is scanned and logged into the audit
> records(s). In addition to fixing the double fetch, this patch
> improves on the original code in a few other ways: better handling
> of large arguments which require encoding, stricter record length
> checking, and some performance improvements (completely unverified,
> but we got rid of some strlen() calls, that's got to be a good
> thing).
>
> As part of the development of this patch, I've also created a basic
> regression test for the audit-testsuite, the test can be tracked on
> GitHub at the following link:
>
> * https://github.com/linux-audit/audit-testsuite/issues/25
>
> [1] If you pay careful attention, there is actually a triple fetch
> problem due to a strnlen_user() call at the top of the function.
>
> [2] This is a tiny white lie, we do make a call to strnlen_user()
> prior to fetching the argument data. I don't like it, but due to the
> way the audit record is structured we really have no choice unless we
> copy the entire argument at once (which would require a rather
> wasteful allocation). The good news is that with this patch the
> kernel no longer relies on this strnlen_user() value for anything
> beyond recording it in the log, we also update it with a trustworthy
> value whenever possible.
>
> Reported-by: Pengfei Wang <wpengfeinudt@gmail.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> kernel/auditsc.c | 332 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 164 insertions(+), 168 deletions(-)
Generally I would take a patch this late for the upcoming merge
window, but considering the nature of this patch I'm going to make an
exception. I've added it to the audit#next branch and I've built a
temporary kernel for testing at the link below for anyone who may be
interested; I'm also building a new pcmoore/kernel-secnext COPR kernel
with the patch right now, it should be available in a few hours.
* https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/394059
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [audit-testsuite PATCH] exec_execve: add some simple tests for the EXECVE record
From: Paul Moore @ 2016-07-20 18:44 UTC (permalink / raw)
To: linux-audit
Beyond the simple existence of the EXECVE record, we test three basic
things:
* large number of arguments spanning multiple records
* single large argument spanning multiple records
* single argument that requires hex encoding
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
tests/Makefile | 1
tests/exec_execve/.gitignore | 1
tests/exec_execve/Makefile | 11 +++
tests/exec_execve/execve_arg_gen.c | 108 +++++++++++++++++++++++++++++++
tests/exec_execve/test | 124 ++++++++++++++++++++++++++++++++++++
5 files changed, 245 insertions(+)
create mode 100644 tests/exec_execve/.gitignore
create mode 100644 tests/exec_execve/Makefile
create mode 100644 tests/exec_execve/execve_arg_gen.c
create mode 100755 tests/exec_execve/test
diff --git a/tests/Makefile b/tests/Makefile
index decb6b2..48a8307 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -5,6 +5,7 @@ DISTRO = $(shell ./os_detect)
SUBDIRS := \
exec_name \
+ exec_execve \
file_create \
file_delete \
file_rename \
diff --git a/tests/exec_execve/.gitignore b/tests/exec_execve/.gitignore
new file mode 100644
index 0000000..fb9f5a6
--- /dev/null
+++ b/tests/exec_execve/.gitignore
@@ -0,0 +1 @@
+execve_arg_gen
diff --git a/tests/exec_execve/Makefile b/tests/exec_execve/Makefile
new file mode 100644
index 0000000..97a4ce4
--- /dev/null
+++ b/tests/exec_execve/Makefile
@@ -0,0 +1,11 @@
+TARGETS=$(patsubst %.c,%,$(wildcard *.c))
+
+LDLIBS += -lpthread
+
+all: $(TARGETS)
+
+execve_arg_gen: execve_arg_gen.c
+ $(CC) $(CFLAGS) -o $@ $^
+
+clean:
+ rm -f $(TARGETS)
diff --git a/tests/exec_execve/execve_arg_gen.c b/tests/exec_execve/execve_arg_gen.c
new file mode 100644
index 0000000..a6da4d5
--- /dev/null
+++ b/tests/exec_execve/execve_arg_gen.c
@@ -0,0 +1,108 @@
+/**
+ * audit-testsuite EXECVE test tool
+ *
+ * Copyright (c) 2016 Red Hat <pmoore@redhat.com>
+ * Author: Paul Moore <paul@paul-moore.com>
+ */
+
+/*
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this library; if not, see <http://www.gnu.org/licenses>.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+
+#define BADCHAR 0x20
+
+char **arg_setup(const char *name, unsigned int size)
+{
+ char **args = NULL;
+
+ args = malloc(sizeof(char *) * (size + 3));
+ if (!args)
+ exit(1);
+
+ args[0] = strdup("testing");
+ if (!args[0])
+ exit(1);
+ args[1] = strdup(name);
+ if (!args[1])
+ exit(1);
+
+ return args;
+}
+
+char *arg_gen(unsigned int length, char insert)
+{
+ int iter;
+ char val;
+ char *buf;
+
+ buf = malloc(length + 1);
+ if (!buf)
+ exit(1);
+
+ for (iter = 0; iter < length; iter++) {
+ if (insert && iter % 2) {
+ buf[iter] = insert;
+ } else {
+ /* ascii: 0x0..0xF */
+ val = iter % 0x10;
+ buf[iter] = (val > 9 ? 55 + val : 48 + val);
+ }
+ }
+ buf[length] = '\0';
+
+ return buf;
+}
+
+int main(int argc, char *argv[])
+{
+ int rc;
+ int iter;
+ int test_cfg;
+ char **exec_argv = NULL;
+
+ /* check if we are calling ourselves for testing purposes */
+ if ((argc >= 1) && (strcmp(argv[0], "testing") == 0))
+ return 0;
+
+ /* run a specific test? */
+ if (argc == 3) {
+ test_cfg = atoi(argv[2]);
+
+ if (strcmp(argv[1], "count") == 0) {
+ exec_argv = arg_setup("count", test_cfg);
+ for (iter = 0; iter < test_cfg; iter++)
+ exec_argv[iter + 2] = arg_gen(1, 0);
+ exec_argv[test_cfg + 2] = NULL;
+ } else if (strcmp(argv[1], "size") == 0) {
+ exec_argv = arg_setup("size", 1);
+ exec_argv[2] = arg_gen(test_cfg, 0);
+ exec_argv[3] = NULL;
+ } else if (strcmp(argv[1], "hex") == 0) {
+ exec_argv = arg_setup("hex", 1);
+ exec_argv[2] = arg_gen(test_cfg, BADCHAR);
+ exec_argv[3] = NULL;
+ }
+
+ rc = execve(argv[0], exec_argv, NULL);
+ return (rc < 0 ? errno : 0);
+ }
+
+ /* no idea what we were supposed to do */
+ return 2;
+}
diff --git a/tests/exec_execve/test b/tests/exec_execve/test
new file mode 100755
index 0000000..b16b0cd
--- /dev/null
+++ b/tests/exec_execve/test
@@ -0,0 +1,124 @@
+#!/usr/bin/perl
+
+use strict;
+
+use Test;
+BEGIN { plan tests => 4 }
+
+use File::Temp qw/ tempfile /;
+
+my $basedir = $0;
+$basedir =~ s|(.*)/[^/]*|$1|;
+
+###
+# functions
+
+sub key_gen {
+ my @chars = ("A".."Z", "a".."z");
+ my $key = "testsuite-" . time . "-";
+ $key .= $chars[rand @chars] for 1..8;
+ return $key;
+}
+
+###
+# setup
+
+# reset audit
+system("auditctl -D >& /dev/null");
+
+# create stdout/stderr sinks
+(my $fh_out, my $stdout) = tempfile(TEMPLATE => '/tmp/audit-testsuite-out-XXXX',
+ UNLINK => 1);
+(my $fh_err, my $stderr) = tempfile(TEMPLATE => '/tmp/audit-testsuite-err-XXXX',
+ UNLINK => 1);
+(my $fh_out2, my $stdout2) = tempfile(
+ TEMPLATE => '/tmp/audit-testsuite-out-XXXX',
+ UNLINK => 1);
+(my $fh_err2, my $stderr2) = tempfile(
+ TEMPLATE => '/tmp/audit-testsuite-err-XXXX',
+ UNLINK => 1);
+
+###
+# tests
+
+# set the audit-by-executable filter
+my $key = key_gen();
+system("auditctl -a always,exit -F arch=b64 -S execve -k $key");
+system("auditctl -a always,exit -F arch=b32 -S execve -k $key");
+
+# test parameters
+my $test_count = 2048;
+my $test_size = 7499;
+my $test_hex = 6144;
+
+# run the tests
+system("$basedir/execve_arg_gen");
+system("$basedir/execve_arg_gen count $test_count");
+system("$basedir/execve_arg_gen size $test_size");
+system("$basedir/execve_arg_gen hex $test_hex");
+
+# test if we generate any audit records from the filter rule
+my $result = system("ausearch -i -k $key > $stdout 2> $stderr");
+ok($result, 0);
+
+# test if we generate the EXECVE records correctly
+my $line;
+my $line2;
+my $id;
+my $found_count = 0;
+my $found_size = 0;
+my $found_hex = 0;
+while ($line = <$fh_out>) {
+ if ($line =~ /^type=EXECVE /) {
+ if ($line =~ / a0=testing a1=count / ) {
+ ($id) = ($line =~ / msg=audit\(.*:([0-9]*)\).* /);
+ seek($fh_out2, 0, 0);
+ system("ausearch -i -k $key -a $id" .
+ " > $stdout2 2> $stderr2");
+ while ($line2 = <$fh_out2>) {
+ if ($line2 =~ /^type=EXECVE /) {
+ $found_count += () =
+ $line2 =~ /a([0-9])*=0/g;
+ }
+ }
+ } elsif ($line =~ / a0=testing a1=size / ) {
+ ($id) = ($line =~ / msg=audit\(.*:([0-9]*)\).* /);
+ seek($fh_out2, 0, 0);
+ system("ausearch -i -k $key -a $id" .
+ " > $stdout2 2> $stderr2");
+ while ($line2 = <$fh_out2>) {
+ if ($line2 =~ /^type=EXECVE / &&
+ $line2 =~ / a2(\[.*\])?=.* /) {
+ my $arg_iter;
+ my $arg_val;
+ ($arg_iter,$arg_val) = ($line2 =~
+ / a2(\[.*\])?=(.*) /);
+ $found_size += length $arg_val;
+ }
+ }
+ } elsif ($line =~ / a0=testing a1=hex / ) {
+ ($id) = ($line =~ / msg=audit\(.*:([0-9]*)\).* /);
+ seek($fh_out2, 0, 0);
+ system("ausearch -i -k $key -a $id" .
+ " > $stdout2 2> $stderr2");
+ while ($line2 = <$fh_out2>) {
+ if ($line2 =~ /^type=EXECVE / &&
+ $line2 =~ / a2(\[.*\])?=.* /) {
+ my $arg_iter;
+ my $arg_val;
+ ($arg_iter,$arg_val) = ($line2 =~
+ / a2(\[.*\])?=(.*) /);
+ $found_hex += length $arg_val;
+ }
+ }
+ }
+ }
+}
+ok($found_count == $test_count);
+ok($found_size == $test_size);
+ok($found_hex == $test_hex);
+
+###
+# cleanup
+
+system("auditctl -D >& /dev/null");
^ permalink raw reply related
* [PATCH] audit: fix a double fetch in audit_log_single_execve_arg()
From: Paul Moore @ 2016-07-20 18:43 UTC (permalink / raw)
To: linux-audit; +Cc: Pengfei Wang
From: Paul Moore <paul@paul-moore.com>
There is a double fetch problem in audit_log_single_execve_arg()
where we first check the execve(2) argumnets for any "bad" characters
which would require hex encoding and then re-fetch the arguments for
logging in the audit record[1]. Of course this leaves a window of
opportunity for an unsavory application to munge with the data.
This patch reworks things by only fetching the argument data once[2]
into a buffer where it is scanned and logged into the audit
records(s). In addition to fixing the double fetch, this patch
improves on the original code in a few other ways: better handling
of large arguments which require encoding, stricter record length
checking, and some performance improvements (completely unverified,
but we got rid of some strlen() calls, that's got to be a good
thing).
As part of the development of this patch, I've also created a basic
regression test for the audit-testsuite, the test can be tracked on
GitHub at the following link:
* https://github.com/linux-audit/audit-testsuite/issues/25
[1] If you pay careful attention, there is actually a triple fetch
problem due to a strnlen_user() call at the top of the function.
[2] This is a tiny white lie, we do make a call to strnlen_user()
prior to fetching the argument data. I don't like it, but due to the
way the audit record is structured we really have no choice unless we
copy the entire argument at once (which would require a rather
wasteful allocation). The good news is that with this patch the
kernel no longer relies on this strnlen_user() value for anything
beyond recording it in the log, we also update it with a trustworthy
value whenever possible.
Reported-by: Pengfei Wang <wpengfeinudt@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
kernel/auditsc.c | 332 +++++++++++++++++++++++++++---------------------------
1 file changed, 164 insertions(+), 168 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index aa3feec..c65af21 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -73,6 +73,7 @@
#include <linux/compat.h>
#include <linux/ctype.h>
#include <linux/string.h>
+#include <linux/uaccess.h>
#include <uapi/linux/limits.h>
#include "audit.h"
@@ -82,7 +83,8 @@
#define AUDITSC_SUCCESS 1
#define AUDITSC_FAILURE 2
-/* no execve audit message should be longer than this (userspace limits) */
+/* no execve audit message should be longer than this (userspace limits),
+ * see the note near the top of audit_log_execve_info() about this value */
#define MAX_EXECVE_AUDIT_LEN 7500
/* max length to print of cmdline/proctitle value during audit */
@@ -992,184 +994,178 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
return rc;
}
-/*
- * to_send and len_sent accounting are very loose estimates. We aren't
- * really worried about a hard cap to MAX_EXECVE_AUDIT_LEN so much as being
- * within about 500 bytes (next page boundary)
- *
- * why snprintf? an int is up to 12 digits long. if we just assumed when
- * logging that a[%d]= was going to be 16 characters long we would be wasting
- * space in every audit message. In one 7500 byte message we can log up to
- * about 1000 min size arguments. That comes down to about 50% waste of space
- * if we didn't do the snprintf to find out how long arg_num_len was.
- */
-static int audit_log_single_execve_arg(struct audit_context *context,
- struct audit_buffer **ab,
- int arg_num,
- size_t *len_sent,
- const char __user *p,
- char *buf)
+static void audit_log_execve_info(struct audit_context *context,
+ struct audit_buffer **ab)
{
- char arg_num_len_buf[12];
- const char __user *tmp_p = p;
- /* how many digits are in arg_num? 5 is the length of ' a=""' */
- size_t arg_num_len = snprintf(arg_num_len_buf, 12, "%d", arg_num) + 5;
- size_t len, len_left, to_send;
- size_t max_execve_audit_len = MAX_EXECVE_AUDIT_LEN;
- unsigned int i, has_cntl = 0, too_long = 0;
- int ret;
-
- /* strnlen_user includes the null we don't want to send */
- len_left = len = strnlen_user(p, MAX_ARG_STRLEN) - 1;
-
- /*
- * We just created this mm, if we can't find the strings
- * we just copied into it something is _very_ wrong. Similar
- * for strings that are too long, we should not have created
- * any.
- */
- if (WARN_ON_ONCE(len < 0 || len > MAX_ARG_STRLEN - 1)) {
- send_sig(SIGKILL, current, 0);
- return -1;
+ long len_max;
+ long len_rem;
+ long len_full;
+ long len_buf;
+ long len_abuf;
+ long len_tmp;
+ bool require_data;
+ bool encode;
+ unsigned int iter;
+ unsigned int arg;
+ char *buf_head;
+ char *buf;
+ const char __user *p = (const char __user *)current->mm->arg_start;
+
+ /* NOTE: this buffer needs to be large enough to hold all the non-arg
+ * data we put in the audit record for this argument (see the
+ * code below) ... at this point in time 96 is plenty */
+ char abuf[96];
+
+ /* NOTE: we set MAX_EXECVE_AUDIT_LEN to a rather arbitrary limit, the
+ * current value of 7500 is not as important as the fact that it
+ * is less than 8k, a setting of 7500 gives us plenty of wiggle
+ * room if we go over a little bit in the logging below */
+ WARN_ON_ONCE(MAX_EXECVE_AUDIT_LEN > 7500);
+ len_max = MAX_EXECVE_AUDIT_LEN;
+
+ /* scratch buffer to hold the userspace args */
+ buf_head = kmalloc(MAX_EXECVE_AUDIT_LEN + 1, GFP_KERNEL);
+ if (!buf_head) {
+ audit_panic("out of memory for argv string");
+ return;
}
+ buf = buf_head;
- /* walk the whole argument looking for non-ascii chars */
+ audit_log_format(*ab, "argc=%d", context->execve.argc);
+
+ len_rem = len_max;
+ len_buf = 0;
+ len_full = 0;
+ require_data = true;
+ encode = false;
+ iter = 0;
+ arg = 0;
do {
- if (len_left > MAX_EXECVE_AUDIT_LEN)
- to_send = MAX_EXECVE_AUDIT_LEN;
- else
- to_send = len_left;
- ret = copy_from_user(buf, tmp_p, to_send);
- /*
- * There is no reason for this copy to be short. We just
- * copied them here, and the mm hasn't been exposed to user-
- * space yet.
- */
- if (ret) {
- WARN_ON(1);
- send_sig(SIGKILL, current, 0);
- return -1;
- }
- buf[to_send] = '\0';
- has_cntl = audit_string_contains_control(buf, to_send);
- if (has_cntl) {
- /*
- * hex messages get logged as 2 bytes, so we can only
- * send half as much in each message
- */
- max_execve_audit_len = MAX_EXECVE_AUDIT_LEN / 2;
- break;
- }
- len_left -= to_send;
- tmp_p += to_send;
- } while (len_left > 0);
-
- len_left = len;
-
- if (len > max_execve_audit_len)
- too_long = 1;
-
- /* rewalk the argument actually logging the message */
- for (i = 0; len_left > 0; i++) {
- int room_left;
-
- if (len_left > max_execve_audit_len)
- to_send = max_execve_audit_len;
- else
- to_send = len_left;
-
- /* do we have space left to send this argument in this ab? */
- room_left = MAX_EXECVE_AUDIT_LEN - arg_num_len - *len_sent;
- if (has_cntl)
- room_left -= (to_send * 2);
- else
- room_left -= to_send;
- if (room_left < 0) {
- *len_sent = 0;
- audit_log_end(*ab);
- *ab = audit_log_start(context, GFP_KERNEL, AUDIT_EXECVE);
- if (!*ab)
- return 0;
- }
+ /* NOTE: we don't ever want to trust this value for anything
+ * serious, but the audit record format insists we
+ * provide an argument length for really long arguments,
+ * e.g. > MAX_EXECVE_AUDIT_LEN, so we have no choice but
+ * to use strncpy_from_user() to obtain this value for
+ * recording in the log, although we don't use it
+ * anywhere here to avoid a double-fetch problem */
+ if (len_full == 0)
+ len_full = strnlen_user(p, MAX_ARG_STRLEN) - 1;
+
+ /* read more data from userspace */
+ if (require_data) {
+ /* can we make more room in the buffer? */
+ if (buf != buf_head) {
+ memmove(buf_head, buf, len_buf);
+ buf = buf_head;
+ }
+
+ /* fetch as much as we can of the argument */
+ len_tmp = strncpy_from_user(&buf_head[len_buf], p,
+ len_max - len_buf);
+ if (len_tmp == -EFAULT) {
+ /* unable to copy from userspace */
+ send_sig(SIGKILL, current, 0);
+ goto out;
+ } else if (len_tmp == (len_max - len_buf)) {
+ /* buffer is not large enough */
+ require_data = true;
+ /* NOTE: if we are going to span multiple
+ * buffers force the encoding so we stand
+ * a chance at a sane len_full value and
+ * consistent record encoding */
+ encode = true;
+ len_full = len_full * 2;
+ p += len_tmp;
+ } else {
+ require_data = false;
+ if (!encode)
+ encode = audit_string_contains_control(
+ buf, len_tmp);
+ /* try to use a trusted value for len_full */
+ if (len_full < len_max)
+ len_full = (encode ?
+ len_tmp * 2 : len_tmp);
+ p += len_tmp + 1;
+ }
+ len_buf += len_tmp;
+ buf_head[len_buf] = '\0';
- /*
- * first record needs to say how long the original string was
- * so we can be sure nothing was lost.
- */
- if ((i == 0) && (too_long))
- audit_log_format(*ab, " a%d_len=%zu", arg_num,
- has_cntl ? 2*len : len);
-
- /*
- * normally arguments are small enough to fit and we already
- * filled buf above when we checked for control characters
- * so don't bother with another copy_from_user
- */
- if (len >= max_execve_audit_len)
- ret = copy_from_user(buf, p, to_send);
- else
- ret = 0;
- if (ret) {
- WARN_ON(1);
- send_sig(SIGKILL, current, 0);
- return -1;
+ /* length of the buffer in the audit record? */
+ len_abuf = (encode ? len_buf * 2 : len_buf + 2);
}
- buf[to_send] = '\0';
-
- /* actually log it */
- audit_log_format(*ab, " a%d", arg_num);
- if (too_long)
- audit_log_format(*ab, "[%d]", i);
- audit_log_format(*ab, "=");
- if (has_cntl)
- audit_log_n_hex(*ab, buf, to_send);
- else
- audit_log_string(*ab, buf);
-
- p += to_send;
- len_left -= to_send;
- *len_sent += arg_num_len;
- if (has_cntl)
- *len_sent += to_send * 2;
- else
- *len_sent += to_send;
- }
- /* include the null we didn't log */
- return len + 1;
-}
-static void audit_log_execve_info(struct audit_context *context,
- struct audit_buffer **ab)
-{
- int i, len;
- size_t len_sent = 0;
- const char __user *p;
- char *buf;
+ /* write as much as we can to the audit log */
+ if (len_buf > 0) {
+ /* NOTE: some magic numbers here - basically if we
+ * can't fit a reasonable amount of data into the
+ * existing audit buffer, flush it and start with
+ * a new buffer */
+ if ((sizeof(abuf) + 8) > len_rem) {
+ len_rem = len_max;
+ audit_log_end(*ab);
+ *ab = audit_log_start(context,
+ GFP_KERNEL, AUDIT_EXECVE);
+ if (!*ab)
+ goto out;
+ }
- p = (const char __user *)current->mm->arg_start;
+ /* create the non-arg portion of the arg record */
+ len_tmp = 0;
+ if (require_data || (iter > 0) ||
+ ((len_abuf + sizeof(abuf)) > len_rem)) {
+ if (iter == 0) {
+ len_tmp += snprintf(&abuf[len_tmp],
+ sizeof(abuf) - len_tmp,
+ " a%d_len=%lu",
+ arg, len_full);
+ }
+ len_tmp += snprintf(&abuf[len_tmp],
+ sizeof(abuf) - len_tmp,
+ " a%d[%d]=", arg, iter++);
+ } else
+ len_tmp += snprintf(&abuf[len_tmp],
+ sizeof(abuf) - len_tmp,
+ " a%d=", arg);
+ WARN_ON(len_tmp >= sizeof(abuf));
+ abuf[sizeof(abuf) - 1] = '\0';
+
+ /* log the arg in the audit record */
+ audit_log_format(*ab, "%s", abuf);
+ len_rem -= len_tmp;
+ len_tmp = len_buf;
+ if (encode) {
+ if (len_abuf > len_rem)
+ len_tmp = len_rem / 2; /* encoding */
+ audit_log_n_hex(*ab, buf, len_tmp);
+ len_rem -= len_tmp * 2;
+ len_abuf -= len_tmp * 2;
+ } else {
+ if (len_abuf > len_rem)
+ len_tmp = len_rem - 2; /* quotes */
+ audit_log_n_string(*ab, buf, len_tmp);
+ len_rem -= len_tmp + 2;
+ /* don't subtract the "2" because we still need
+ * to add quotes to the remaining string */
+ len_abuf -= len_tmp;
+ }
+ len_buf -= len_tmp;
+ buf += len_tmp;
+ }
- audit_log_format(*ab, "argc=%d", context->execve.argc);
+ /* ready to move to the next argument? */
+ if ((len_buf == 0) && !require_data) {
+ arg++;
+ iter = 0;
+ len_full = 0;
+ require_data = true;
+ encode = false;
+ }
+ } while (arg < context->execve.argc);
- /*
- * we need some kernel buffer to hold the userspace args. Just
- * allocate one big one rather than allocating one of the right size
- * for every single argument inside audit_log_single_execve_arg()
- * should be <8k allocation so should be pretty safe.
- */
- buf = kmalloc(MAX_EXECVE_AUDIT_LEN + 1, GFP_KERNEL);
- if (!buf) {
- audit_panic("out of memory for argv string");
- return;
- }
+ /* NOTE: the caller handles the final audit_log_end() call */
- for (i = 0; i < context->execve.argc; i++) {
- len = audit_log_single_execve_arg(context, ab, i,
- &len_sent, p, buf);
- if (len <= 0)
- break;
- p += len;
- }
- kfree(buf);
+out:
+ kfree(buf_head);
}
static void show_special(struct audit_context *context, int *call_panic)
^ permalink raw reply related
* Re: The res field has a value of 1 instead of either success or fail
From: Steve Grubb @ 2016-07-20 13:17 UTC (permalink / raw)
To: linux-audit; +Cc: Konrad Witaszczyk
In-Reply-To: <68967373-F5EC-4790-B7F0-DFD35220B0A8@FreeBSD.org>
On Wednesday, July 20, 2016 11:25:19 AM EDT Mateusz Piotrowski wrote:
> Hello,
>
> > On 19 Jul 2016, at 12:28, Mateusz Piotrowski <0mp@freebsd.org> wrote:
> >
> > type=CONFIG_CHANGE msg=audit(1464013671.541:406): auid=1000 ses=7 op="add
> > rule" key=(null) list=4 res=1 As you can see, there is a res field which
> > value is 1.
> >
> > Is it because my auditd is outdated? Is there a missing res field which is
> > purely numeric (just like the fields called fp [3])?
No. There is inconsistency because different people do it their way without
regard for anyone who is trying to make sense of the audit trail. This is why
I have published so many specifications. I want to point to the docs and say
you have to conform. And this is also why I want to write a validation suite.
We need to find all the outliers and fix them.
-Steve
> > As Steve said in previous emails, it is possible and it might be fixed
> > already. I’ll try to find out if I get similar logs with the latest
> > auditd (2.6.5) on CentOS 6.8-i386 later.
>
> I confirm that it is possible to generate a type=CONFIG_CHANGE record with a
> res=1 field on CentOS 6.8 with auditd v2.6.5.
>
> Cheers
>
> -m
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Re: The res field has a value of 1 instead of either success or fail
From: Mateusz Piotrowski @ 2016-07-20 9:25 UTC (permalink / raw)
To: linux-audit; +Cc: Konrad Witaszczyk
In-Reply-To: <1DCCD2B1-2986-49D0-A204-C9246F3E1F12@FreeBSD.org>
Hello,
> On 19 Jul 2016, at 12:28, Mateusz Piotrowski <0mp@freebsd.org> wrote:
>
> type=CONFIG_CHANGE msg=audit(1464013671.541:406): auid=1000 ses=7 op="add rule" key=(null) list=4 res=1
> As you can see, there is a res field which value is 1.
>
> Is it because my auditd is outdated? Is there a missing res field which is purely numeric (just like the fields called fp [3])?
>
> As Steve said in previous emails, it is possible and it might be fixed already. I’ll try to find out if I get similar logs with the latest auditd (2.6.5) on CentOS 6.8-i386 later.
I confirm that it is possible to generate a type=CONFIG_CHANGE record with a res=1 field on CentOS 6.8 with auditd v2.6.5.
Cheers
-m
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* The res field has a value of 1 instead of either success or fail
From: Mateusz Piotrowski @ 2016-07-19 10:28 UTC (permalink / raw)
To: linux-audit; +Cc: Konrad Witaszczyk
[-- Attachment #1.1: Type: text/plain, Size: 1734 bytes --]
Hello,
According to this [1] and the definition of the res field here [2], the res field should have a value of either success or fail.
Here are some logs I generated on Debian:
type=USER_START msg=audit(1464013671.525:405): pid=3569 uid=0 auid=1000 ses=7 msg='op=PAM:session_open acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/1 res=success'
type=CONFIG_CHANGE msg=audit(1464013671.541:406): auid=1000 ses=7 op="add rule" key=(null) list=4 res=1
type=USER_END msg=audit(1464013671.549:407): pid=3569 uid=0 auid=1000 ses=7 msg='op=PAM:session_close acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/1 res=success’
As you can see, there is a res field which value is 1.
Is it because my auditd is outdated? Is there a missing res field which is purely numeric (just like the fields called fp [3])?
As Steve said in previous emails, it is possible and it might be fixed already. I’ll try to find out if I get similar logs with the latest auditd (2.6.5) on CentOS 6.8-i386 later.
Cheers!
-m
[1]: https://github.com/linux-audit/audit-userspace/blob/ac9384a884841ef66b4cae42884d9e63d0b6a438/auparse/typetab.h#L79-L80 <https://github.com/linux-audit/audit-userspace/blob/ac9384a884841ef66b4cae42884d9e63d0b6a438/auparse/typetab.h#L79-L80>
[2]: https://github.com/linux-audit/audit-documentation/blob/master/specs/fields/field-dictionary.csv#L186 <https://github.com/linux-audit/audit-documentation/blob/master/specs/fields/field-dictionary.csv#L186>
[3]: https://github.com/linux-audit/audit-documentation/blob/master/specs/fields/field-dictionary.csv#L62-L63 <https://github.com/linux-audit/audit-documentation/blob/master/specs/fields/field-dictionary.csv#L62-L63>
[-- Attachment #1.2: Type: text/html, Size: 2539 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: How can I install the latest version of Linux Audit on CentOS 6.8?
From: Mateusz Piotrowski @ 2016-07-18 15:39 UTC (permalink / raw)
To: linux-audit; +Cc: Konrad Witaszczyk
In-Reply-To: <2326978.VhGrlLPiTR@x2>
[-- Attachment #1.1: Type: text/plain, Size: 1151 bytes --]
Hello,
> On 15 Jul 2016, at 15:17, Steve Grubb <sgrubb@redhat.com> wrote:
> On Friday, July 15, 2016 2:52:02 PM EDT Mateusz Piotrowski wrote:
>> Is there an easier way to get the latest Linux Audit version on my system?
> Why wouldn't you start with the distribution tar file?
> http://people.redhat.com/sgrubb/audit/audit-2.6.5.tar.gz <http://people.redhat.com/sgrubb/audit/audit-2.6.5.tar.gz>
I was able to install the latest version from the tar file using the following commands. Thanks Steve!
curl -O http://people.redhat.com/sgrubb/audit/audit-2.6.5.tar.gz <http://people.redhat.com/sgrubb/audit/audit-2.6.5.tar.gz>
gzip -d audit-2.6.5.tar.gz
tar xf audit-2.6.5.tar
cd audit-2.6.5
yum install autoconf automake libtool tcp_wrappers-devel openldap-devel
# Python and libcap-ng didn't work for me.
# This is why I set --with-python and --with-libcap-ng to no instead to yes.
./configure --sbindir=/sbin --with-python=no --with-libwrap --enable-gssapi-krb5=yes --with-libcap-ng=no
make
make install
My CentOS is now running auditd version 2.6.5 (according to information within /var/log/audit/audit.log).
Cheers!
-m
[-- Attachment #1.2: Type: text/html, Size: 2040 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH] field-dictionary: add ioctlcmd
From: Paul Moore @ 2016-07-17 13:19 UTC (permalink / raw)
To: william.c.roberts; +Cc: seandroid-list, selinux, linux-audit
In-Reply-To: <1468612513-28042-1-git-send-email-william.c.roberts@intel.com>
On Fri, Jul 15, 2016 at 3:55 PM, <william.c.roberts@intel.com> wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> Per-request ioctlcmd controls were added to SE Linux, however
> the audit field dictionary was not updated.
>
> This patch updates that dictionary.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
> specs/fields/field-dictionary.csv | 1 +
> 1 file changed, 1 insertion(+)
Applied, thanks!
> diff --git a/specs/fields/field-dictionary.csv b/specs/fields/field-dictionary.csv
> index a645085..5a922e5 100644
> --- a/specs/fields/field-dictionary.csv
> +++ b/specs/fields/field-dictionary.csv
> @@ -82,6 +82,7 @@ inode,numeric,inode number,
> inode_gid,numeric,group id of the inode's owner,
> inode_uid,numeric,user id of the inode's owner,
> invalid_context,encoded,SELinux context,
> +ioctlcmd,numeric,The request argument to the ioctl syscall,
> ipx-net,numeric,IPX network number,
> item,numeric,which item is being recorded,
> items,numeric,the number of path records in the event,
> --
> 1.9.1
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
From: Steve Grubb @ 2016-07-15 20:12 UTC (permalink / raw)
To: Roberts, William C
Cc: seandroid-list@tycho.nsa.gov, selinux@tycho.nsa.gov,
linux-audit@redhat.com
In-Reply-To: <476DC76E7D1DF2438D32BFADF679FC5601258605@ORSMSX103.amr.corp.intel.com>
On Friday, July 15, 2016 7:49:22 PM EDT Roberts, William C wrote:
> > I also asked some other questions. Is this the ioctl number? As in
> > syscall arg a1? I need to know if its the same thing so that I can hook
> > up its translation if so.
>
> Yes, per man ioctl, it's the "request number". Assuming a0 is the file
> descriptor, then a1 is the Ioctlcmd value.
OK, great. I hooked this field up to the translator so that the ioctl name can
be printed (if known).
Thanks,
-Steve
^ permalink raw reply
* [PATCH] field-dictionary: add ioctlcmd
From: william.c.roberts-ral2JQCrhuEAvxtiuMwx3w @ 2016-07-15 19:55 UTC (permalink / raw)
To: sgrubb-H+wXaHxf7aLQT0dZR+AlfA, linux-audit-H+wXaHxf7aLQT0dZR+AlfA
Cc: seandroid-list-+05T5uksL2qpZYMLLGbcSA,
selinux-+05T5uksL2qpZYMLLGbcSA
From: William Roberts <william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Per-request ioctlcmd controls were added to SE Linux, however
the audit field dictionary was not updated.
This patch updates that dictionary.
Signed-off-by: William Roberts <william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
specs/fields/field-dictionary.csv | 1 +
1 file changed, 1 insertion(+)
diff --git a/specs/fields/field-dictionary.csv b/specs/fields/field-dictionary.csv
index a645085..5a922e5 100644
--- a/specs/fields/field-dictionary.csv
+++ b/specs/fields/field-dictionary.csv
@@ -82,6 +82,7 @@ inode,numeric,inode number,
inode_gid,numeric,group id of the inode's owner,
inode_uid,numeric,user id of the inode's owner,
invalid_context,encoded,SELinux context,
+ioctlcmd,numeric,The request argument to the ioctl syscall,
ipx-net,numeric,IPX network number,
item,numeric,which item is being recorded,
items,numeric,the number of path records in the event,
--
1.9.1
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply related
* RE: [PATCH] selinux: print leading 0x on ioctlcmd audits
From: Roberts, William C @ 2016-07-15 19:49 UTC (permalink / raw)
To: Steve Grubb
Cc: seandroid-list-+05T5uksL2qpZYMLLGbcSA@public.gmane.org,
selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <3333447.I3SejC0nv6@x2>
> -----Original Message-----
> From: Steve Grubb [mailto:sgrubb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
> Sent: Friday, July 15, 2016 12:42 PM
> To: Roberts, William C <william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Paul Moore <pmoore-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; William Roberts
> <bill.c.roberts-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; seandroid-list-+05T5uksL2qpZYMLLGbcSA@public.gmane.org;
> selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org; linux-audit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Subject: Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
>
> On Friday, July 15, 2016 7:33:09 PM EDT Roberts, William C wrote:
> > <snip>
> >
> > > > This is important so that people don't make up new ones that do
> > > > the same thing. The ioctlcmd field name should be recorded. Are
> > > > there more that need documenting?
> > >
> > > Steve/William, one of you want to send a patch/PR for the field
> > > dictionary?
> >
> > I'll send it over.
>
> I also asked some other questions. Is this the ioctl number? As in syscall arg a1? I
> need to know if its the same thing so that I can hook up its translation if so.
Yes, per man ioctl, it's the "request number". Assuming a0 is the file descriptor, then a1 is the
Ioctlcmd value.
>
> Thanks,
> -Steve
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply
* Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
From: Steve Grubb @ 2016-07-15 19:41 UTC (permalink / raw)
To: Roberts, William C
Cc: seandroid-list@tycho.nsa.gov, selinux@tycho.nsa.gov,
linux-audit@redhat.com
In-Reply-To: <476DC76E7D1DF2438D32BFADF679FC56012585BF@ORSMSX103.amr.corp.intel.com>
On Friday, July 15, 2016 7:33:09 PM EDT Roberts, William C wrote:
> <snip>
>
> > > This is important so that people don't make up new ones that do the
> > > same thing. The ioctlcmd field name should be recorded. Are there more
> > > that need documenting?
> >
> > Steve/William, one of you want to send a patch/PR for the field
> > dictionary?
>
> I'll send it over.
I also asked some other questions. Is this the ioctl number? As in syscall
arg a1? I need to know if its the same thing so that I can hook up its
translation if so.
Thanks,
-Steve
^ permalink raw reply
* Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
From: Paul Moore @ 2016-07-15 19:35 UTC (permalink / raw)
To: Roberts, William C
Cc: Steve Grubb,
seandroid-list-+05T5uksL2qpZYMLLGbcSA@public.gmane.org,
selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <476DC76E7D1DF2438D32BFADF679FC56012585A7-P5GAC/sN6hlQxe9IK+vIArfspsVTdybXVpNB7YpNyf8@public.gmane.org>
On Fri, Jul 15, 2016 at 3:31 PM, Roberts, William C
<william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> Does this mean then the patch will be applied?
As I mentioned earlier, I added it to the SELinux next queue, as soon
as the merge window closes (approx two weeks from this weekend) I will
rotate the patch into the SELinux next branch.
--
paul moore
security @ redhat
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply
* RE: [PATCH] selinux: print leading 0x on ioctlcmd audits
From: Roberts, William C @ 2016-07-15 19:33 UTC (permalink / raw)
To: Paul Moore, Steve Grubb, William Roberts
Cc: seandroid-list@tycho.nsa.gov, selinux@tycho.nsa.gov,
linux-audit@redhat.com
In-Reply-To: <CAGH-Kgsx9a21PbypubWH0yYNkRq3fPb6mxB1fyV1yeTb-=yDVg@mail.gmail.com>
<snip>
> > This is important so that people don't make up new ones that do the
> > same thing. The ioctlcmd field name should be recorded. Are there more
> > that need documenting?
>
> Steve/William, one of you want to send a patch/PR for the field dictionary?
I'll send it over.
^ permalink raw reply
* RE: [PATCH] selinux: print leading 0x on ioctlcmd audits
From: Roberts, William C @ 2016-07-15 19:31 UTC (permalink / raw)
To: Steve Grubb, Paul Moore
Cc: seandroid-list@tycho.nsa.gov, selinux@tycho.nsa.gov,
linux-audit@redhat.com
In-Reply-To: <1613578.6STCXkp1CU@x2>
> -----Original Message-----
> From: Steve Grubb [mailto:sgrubb@redhat.com]
> Sent: Friday, July 15, 2016 11:54 AM
> To: Paul Moore <paul@paul-moore.com>
> Cc: Roberts, William C <william.c.roberts@intel.com>; selinux@tycho.nsa.gov;
> seandroid-list@tycho.nsa.gov; Stephen Smalley <sds@tycho.nsa.gov>; linux-
> audit@redhat.com
> Subject: Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
>
> On Thursday, July 14, 2016 6:17:32 PM EDT Paul Moore wrote:
> > Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
> > From: Paul Moore <paul@paul-moore.com>
> > To: william.c.roberts@intel.com
> > CC: selinux@tycho.nsa.gov, seandroid-list@tycho.nsa.gov, Stephen Smalley
> > <sds@tycho.nsa.gov>, Me, linux-audit@redhat.com Date: Yesterday 6:17
> PM
> >
> > On Thu, Jul 14, 2016 at 3:29 PM, <william.c.roberts@intel.com> wrote:
> > > From: William Roberts <william.c.roberts@intel.com>
> > >
> > > ioctlcmd is currently printing hex numbers, but their is no leading
> > > 0x. Thus things like ioctlcmd=1234 are misleading, as the base is
> > > not evident.
> > >
> > > Correct this by adding 0x as a prefix, so ioctlcmd=1234 becomes
> > > ioctlcmd=0x1234.
> > >
> > > Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > > ---
> > > security/lsm_audit.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > NOTE: adding Steve Grubb and the audit mailing list to the CC line
> >
> > Like it or not, I believe the general standard/convention when it
> > comes to things like this is to leave off the "0x" prefix; the idea
> > being that is saves precious space in the audit logs and the value is
> > only ever going to be in hex anyway.
>
> We normally like the 0x prefix on anything that is hex so that stroul can figure it
> out itself. And since AVC's should in theory be rare or occassional, log space is not
> a concern.
Does this mean then the patch will be applied?
>
> That said, what is this ioctlcmd field name? Is this the ioctl number? As in syscall
> arg a1? If so, it should be hooked up to the interpretation for that.
>
> Also, we have a field dictionary with some basic info about each field used in
> audit events:
>
> http://people.redhat.com/sgrubb/audit/field-dictionary.txt
>
> This is important so that people don't make up new ones that do the same thing.
> The ioctlcmd field name should be recorded. Are there more that need
> documenting?
>
> -Steve
^ permalink raw reply
* Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
From: Paul Moore @ 2016-07-15 19:02 UTC (permalink / raw)
To: Steve Grubb, William Roberts
Cc: seandroid-list-+05T5uksL2qpZYMLLGbcSA,
selinux-+05T5uksL2qpZYMLLGbcSA,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1613578.6STCXkp1CU@x2>
On Fri, Jul 15, 2016 at 2:54 PM, Steve Grubb <sgrubb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Thursday, July 14, 2016 6:17:32 PM EDT Paul Moore wrote:
>> Re: [PATCH] selinux: print leading 0x on ioctlcmd audits
>> From: Paul Moore <paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org>
>> To: william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
>> CC: selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org, seandroid-list-+05T5uksL2qpZYMLLGbcSA@public.gmane.org, Stephen Smalley
>> <sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>, Me, linux-audit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org Date: Yesterday 6:17 PM
>>
>> On Thu, Jul 14, 2016 at 3:29 PM, <william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> > From: William Roberts <william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> >
>> > ioctlcmd is currently printing hex numbers, but their is no leading
>> > 0x. Thus things like ioctlcmd=1234 are misleading, as the base is
>> > not evident.
>> >
>> > Correct this by adding 0x as a prefix, so ioctlcmd=1234 becomes
>> > ioctlcmd=0x1234.
>> >
>> > Signed-off-by: William Roberts <william.c.roberts-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> > ---
>> > security/lsm_audit.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> NOTE: adding Steve Grubb and the audit mailing list to the CC line
>>
>> Like it or not, I believe the general standard/convention when it
>> comes to things like this is to leave off the "0x" prefix; the idea
>> being that is saves precious space in the audit logs and the value is
>> only ever going to be in hex anyway.
>
> We normally like the 0x prefix on anything that is hex so that stroul can figure
> it out itself. And since AVC's should in theory be rare or occassional, log
> space is not a concern.
>
> That said, what is this ioctlcmd field name? Is this the ioctl number? As in
> syscall arg a1? If so, it should be hooked up to the interpretation for that.
>
> Also, we have a field dictionary with some basic info about each field used in
> audit events:
>
> http://people.redhat.com/sgrubb/audit/field-dictionary.txt
Correction, that file now lives at the link below, the file on Steve's
people page is deprecated.
https://github.com/linux-audit/audit-documentation/blob/master/specs/fields/field-dictionary.csv
> This is important so that people don't make up new ones that do the same
> thing. The ioctlcmd field name should be recorded. Are there more that need
> documenting?
Steve/William, one of you want to send a patch/PR for the field dictionary?
--
paul moore
security @ redhat
_______________________________________________
Selinux mailing list
Selinux-+05T5uksL2qpZYMLLGbcSA@public.gmane.org
To unsubscribe, send email to Selinux-leave-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
To get help, send an email containing "help" to Selinux-request-+05T5uksL2pAGbPMOrvdOA@public.gmane.org
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox