5:33 p.m.
This patch came from our L3 department. AppArmor LSM is logging using the
common_lsm_audit()
call but the audit userspace parsing code expects to see an SELinux tclass field. This
patch
doesn't address the lack of support for AppArmor in "aureport --avc".
Talking to Seth Arnold,
Canonical apparently has patches for this; if this is true perhaps they can post for
inclusion.
Based-on-work-by: William Preston <wpreston(a)suse.com>
Signed-off-by: Tony Jones <tonyj(a)suse.de>
--- a/src/ausearch-parse.c 2014-05-21 14:45:22.000000000 +0200
+++ b/src/ausearch-parse.c 2014-05-21 14:53:55.000000000 +0200
@@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
// Now get the class...its at the end, so we do things different
str = strstr(term, "tclass=");
- if (str == NULL) {
- rc = 9;
- goto err;
+ if (str) {
+ str += 7;
+ term = strchr(str, ' ');
+ if (term)
+ *term = 0;
+ an.avc_class = strdup(str);
+ if (term)
+ *term = ' ';
}
- str += 7;
- term = strchr(str, ' ');
- if (term)
- *term = 0;
- an.avc_class = strdup(str);
- if (term)
- *term = ' ';
if (audit_avc_init(s) == 0) {
alist_append(s->avc, &an);
3:31 a.m.
On 2014-05-28 15:33:06, Tony Jones wrote:
This patch came from our L3 department. AppArmor LSM is logging
using the common_lsm_audit()
call but the audit userspace parsing code expects to see an SELinux tclass field. This
patch
doesn't address the lack of support for AppArmor in "aureport --avc".
Talking to Seth Arnold,
Canonical apparently has patches for this; if this is true perhaps they can post for
inclusion.
Making the audit tools work with AppArmor generated events has been on
my todo list for quite a while, but no patches exist.
I'm surprised that this patch makes ausearch work correctly for AppArmor
AVC events. The first thing that parse_avc() does is look for the
"avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
include that string, so an.avc_result and an.avc_perm would not be set,
would they?
Tyler
Based-on-work-by: William Preston <wpreston(a)suse.com>
Signed-off-by: Tony Jones <tonyj(a)suse.de>
--- a/src/ausearch-parse.c 2014-05-21 14:45:22.000000000 +0200
+++ b/src/ausearch-parse.c 2014-05-21 14:53:55.000000000 +0200
@@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
// Now get the class...its at the end, so we do things different
str = strstr(term, "tclass=");
- if (str == NULL) {
- rc = 9;
- goto err;
+ if (str) {
+ str += 7;
+ term = strchr(str, ' ');
+ if (term)
+ *term = 0;
+ an.avc_class = strdup(str);
+ if (term)
+ *term = ' ';
}
- str += 7;
- term = strchr(str, ' ');
- if (term)
- *term = 0;
- an.avc_class = strdup(str);
- if (term)
- *term = ' ';
if (audit_avc_init(s) == 0) {
alist_append(s->avc, &an);
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
10:01 a.m.
On Thursday, May 29, 2014 10:31:52 AM Tyler Hicks wrote:
On 2014-05-28 15:33:06, Tony Jones wrote:
> This patch came from our L3 department. AppArmor LSM is logging using the
> common_lsm_audit() call but the audit userspace parsing code expects to
> see an SELinux tclass field. This patch doesn't address the lack of
> support for AppArmor in "aureport --avc". Talking to Seth Arnold,
> Canonical apparently has patches for this; if this is true perhaps they
> can post for inclusion.
Making the audit tools work with AppArmor generated events has been on
my todo list for quite a while, but no patches exist.
I'm surprised that this patch makes ausearch work correctly for AppArmor
AVC events. The first thing that parse_avc() does is look for the
"avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
include that string, so an.avc_result and an.avc_perm would not be set,
would they?
I have a feeling a whole lot of testing is needed for apparmor, smack, tomoyo,
or any other LSM besides SE Linux. (Maybe they work fine? I don't know.)
Ausearch/report, auparse, and auvirt would all need updating. I'd also suggest
sending patches to the ausearch test suite so that it can verify correctness
of finding events. One last area, perhaps the prelude plugin might need some
updating as well....but then again the prelude project kind of died any ways.
-Steve
> Based-on-work-by: William Preston <wpreston(a)suse.com>
> Signed-off-by: Tony Jones <tonyj(a)suse.de>
>
> --- a/src/ausearch-parse.c 2014-05-21 14:45:22.000000000 +0200
> +++ b/src/ausearch-parse.c 2014-05-21 14:53:55.000000000 +0200
> @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
>
> // Now get the class...its at the end, so we do things different
> str = strstr(term, "tclass=");
>
> - if (str == NULL) {
> - rc = 9;
> - goto err;
> + if (str) {
> + str += 7;
> + term = strchr(str, ' ');
> + if (term)
> + *term = 0;
> + an.avc_class = strdup(str);
> + if (term)
> + *term = ' ';
>
> }
>
> - str += 7;
> - term = strchr(str, ' ');
> - if (term)
> - *term = 0;
> - an.avc_class = strdup(str);
> - if (term)
> - *term = ' ';
>
> if (audit_avc_init(s) == 0) {
>
> alist_append(s->avc, &an);
>
> --
> Linux-audit mailing list
> Linux-audit(a)redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
10:15 a.m.
On 2014-05-29 11:01:38, Steve Grubb wrote:
On Thursday, May 29, 2014 10:31:52 AM Tyler Hicks wrote:
> On 2014-05-28 15:33:06, Tony Jones wrote:
> > This patch came from our L3 department. AppArmor LSM is logging using the
> > common_lsm_audit() call but the audit userspace parsing code expects to
> > see an SELinux tclass field. This patch doesn't address the lack of
> > support for AppArmor in "aureport --avc". Talking to Seth Arnold,
> > Canonical apparently has patches for this; if this is true perhaps they
> > can post for inclusion.
> Making the audit tools work with AppArmor generated events has been on
> my todo list for quite a while, but no patches exist.
>
> I'm surprised that this patch makes ausearch work correctly for AppArmor
> AVC events. The first thing that parse_avc() does is look for the
> "avc: " term in the AVCs that SELinux generates. AppArmor's AVCs
don't
> include that string, so an.avc_result and an.avc_perm would not be set,
> would they?
I have a feeling a whole lot of testing is needed for apparmor, smack, tomoyo,
or any other LSM besides SE Linux. (Maybe they work fine? I don't know.)
Ausearch/report, auparse, and auvirt would all need updating. I'd also suggest
sending patches to the ausearch test suite so that it can verify correctness
of finding events.
Agreed. It felt to me like it would be a more work than just updating
parse_avc() to gain full support for other LSMs.
In addition, updating the ausearch test suite is a no-brainer so that
you can easily test with non-SELinux events.
With that said, I don't think these things should be prereqs for
Tony's patch being merged.
One last area, perhaps the prelude plugin might need some updating
as
well....but then again the prelude project kind of died any ways.
I'm not really familiar with prelude, but I'll keep it in mind.
Tyler
-Steve
> > Based-on-work-by: William Preston <wpreston(a)suse.com>
> > Signed-off-by: Tony Jones <tonyj(a)suse.de>
> >
> > --- a/src/ausearch-parse.c 2014-05-21 14:45:22.000000000 +0200
> > +++ b/src/ausearch-parse.c 2014-05-21 14:53:55.000000000 +0200
> > @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
> >
> > // Now get the class...its at the end, so we do things different
> > str = strstr(term, "tclass=");
> >
> > - if (str == NULL) {
> > - rc = 9;
> > - goto err;
> > + if (str) {
> > + str += 7;
> > + term = strchr(str, ' ');
> > + if (term)
> > + *term = 0;
> > + an.avc_class = strdup(str);
> > + if (term)
> > + *term = ' ';
> >
> > }
> >
> > - str += 7;
> > - term = strchr(str, ' ');
> > - if (term)
> > - *term = 0;
> > - an.avc_class = strdup(str);
> > - if (term)
> > - *term = ' ';
> >
> > if (audit_avc_init(s) == 0) {
> >
> > alist_append(s->avc, &an);
> >
> > --
> > Linux-audit mailing list
> > Linux-audit(a)redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
8 p.m.
On 05/29/2014 01:31 AM, Tyler Hicks wrote:
I'm surprised that this patch makes ausearch work correctly for
AppArmor
AVC events. The first thing that parse_avc() does is look for the
"avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
include that string, so an.avc_result and an.avc_perm would not be set,
would they?
That patch does "work" (tested w/ svn trunk). After I read your comment I
looked at the code and I was confused also as 'avc_result == AVC_UNSET' but
find_avc() which checks against UNSET isn't being called, rather the record gets
selected for output by 'n = list_get_cur(l)' [ausearch-match.c:113]. I would
need to spend more time to fully understand what is happening in the code.
$ cat log
type=AVC msg=audit(1390876383.602:15646): apparmor="DENIED"
operation="open" parent=21147 profile="/tmp/ls"
name="/var/log/audit/" pid=21598 comm="ls"
requested_mask="r" denied_mask="r" fsuid=0 ouid=0
type=AVC msg=audit(1390936201.188:15647): apparmor="ALLOWED"
operation="file_lock" parent=7873 profile="/usr/sbin/sshd"
name="/tmp/pam_krb5_tmp_FqhNDa" pid=7875 comm="sshd"
requested_mask="k" denied_mask="k" fsuid=0 ouid=0
$ /tmp/audit/sbin/ausearch -m AVC -if ./log
----
time->Mon Jan 27 18:33:03 2014
type=AVC msg=audit(1390876383.602:15646): apparmor="DENIED"
operation="open" parent=21147 profile="/tmp/ls"
name="/var/log/audit/" pid=21598 comm="ls"
requested_mask="r" denied_mask="r" fsuid=0 ouid=0
----
time->Tue Jan 28 11:10:01 2014
type=AVC msg=audit(1390936201.188:15647): apparmor="ALLOWED"
operation="file_lock" parent=7873 profile="/usr/sbin/sshd"
name="/tmp/pam_krb5_tmp_FqhNDa" pid=7875 comm="sshd"
requested_mask="k" denied_mask="k" fsuid=0 ouid=0
Without patch, ausearch just outputs "<no matches>"
tony
9:47 a.m.
On Monday, June 02, 2014 06:00:54 PM Tony Jones wrote:
On 05/29/2014 01:31 AM, Tyler Hicks wrote:
> I'm surprised that this patch makes ausearch work correctly for AppArmor
> AVC events. The first thing that parse_avc() does is look for the
> "avc: " term in the AVCs that SELinux generates. AppArmor's AVCs
don't
> include that string, so an.avc_result and an.avc_perm would not be set,
> would they?
That patch does "work" (tested w/ svn trunk).
Right. The parsing code aborts the parse if it doesn't find required/expected
fields. So, if a field is missing, it skips the event. The --debug option will
print events that get skipped due to being malformed.
After I read your comment I
looked at the code and I was confused also as 'avc_result == AVC_UNSET' but
find_avc() which checks against UNSET isn't being called, rather the
record gets selected for output by 'n = list_get_cur(l)'
[ausearch-match.c:113]. I would need to spend more time to fully
understand what is happening in the code.
Ausearch is simple. It gathers fields only if they are relevant to the command
line flags. If auid is given, it collects those fields, but the parser expects a
well formed event or it will exclude it from the results.
Without patch, ausearch just outputs "<no matches>"
Yep. So, the question is really how to fix this. Should we have a different
function that is swung in with #ifdef WITH_APPARMOR called parse_aa_avc? Then
it can be tuned exactly for AppArmor's needs? Later, the kernel event number
can be changed and the switch/case can pick that up. Also, are there other AA
events that are missing in action? The ausearch-test should tell you.
-Steve
11:34 a.m.
On 06/03/2014 07:47 AM, Steve Grubb wrote:
Yep. So, the question is really how to fix this. Should we have a
different
function that is swung in with #ifdef WITH_APPARMOR called parse_aa_avc? Then
it can be tuned exactly for AppArmor's needs? Later, the kernel event number
can be changed and the switch/case can pick that up. Also, are there other AA
events that are missing in action? The ausearch-test should tell you.
We'll take the patch (locally) for SLES. Seems to me, since there really isn't
any AppArmor awareness in audit at present that the AppArmor developers
may as well fix the kernel event numbering first, audit userspace after that .... anyhow,
I see no point considering the previous patch for upstreaming.
Thanks
Tony
10:21 a.m.
On 2014-05-28 15:33:06, Tony Jones wrote:
This patch came from our L3 department. AppArmor LSM is logging
using the common_lsm_audit()
call but the audit userspace parsing code expects to see an SELinux tclass field. This
patch
doesn't address the lack of support for AppArmor in "aureport --avc".
Talking to Seth Arnold,
Canonical apparently has patches for this; if this is true perhaps they can post for
inclusion.
Based-on-work-by: William Preston <wpreston(a)suse.com>
Signed-off-by: Tony Jones <tonyj(a)suse.de>
--- a/src/ausearch-parse.c 2014-05-21 14:45:22.000000000 +0200
+++ b/src/ausearch-parse.c 2014-05-21 14:53:55.000000000 +0200
@@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
// Now get the class...its at the end, so we do things different
str = strstr(term, "tclass=");
- if (str == NULL) {
- rc = 9;
It is nitpicky, but all error return codes after this should be
decremented (there is only one to do).
The rest of the patch looks fine to me.
Tyler
- goto err;
+ if (str) {
+ str += 7;
+ term = strchr(str, ' ');
+ if (term)
+ *term = 0;
+ an.avc_class = strdup(str);
+ if (term)
+ *term = ' ';
}
- str += 7;
- term = strchr(str, ' ');
- if (term)
- *term = 0;
- an.avc_class = strdup(str);
- if (term)
- *term = ' ';
if (audit_avc_init(s) == 0) {
alist_append(s->avc, &an);
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
2:53 p.m.
On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
This patch came from our L3 department. AppArmor LSM is logging
using the
common_lsm_audit() call but the audit userspace parsing code expects to see
an SELinux tclass field. This patch doesn't address the lack of support for
AppArmor in "aureport --avc". Talking to Seth Arnold, Canonical apparently
has patches for this; if this is true perhaps they can post for inclusion.
Based-on-work-by: William Preston <wpreston(a)suse.com>
Signed-off-by: Tony Jones <tonyj(a)suse.de>
I was looking at this patch and was wondering something. Does AppArmor produce
AUDIT_AVC events? If not, how does the code even get into parse_avc? IOW, is
there another part of the patch missing in the switch statement that direct
AUDIT_APPARMOR_* events into parse_avc?
-Steve
--- a/src/ausearch-parse.c 2014-05-21 14:45:22.000000000 +0200
+++ b/src/ausearch-parse.c 2014-05-21 14:53:55.000000000 +0200
@@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
// Now get the class...its at the end, so we do things different
str = strstr(term, "tclass=");
- if (str == NULL) {
- rc = 9;
- goto err;
+ if (str) {
+ str += 7;
+ term = strchr(str, ' ');
+ if (term)
+ *term = 0;
+ an.avc_class = strdup(str);
+ if (term)
+ *term = ' ';
}
- str += 7;
- term = strchr(str, ' ');
- if (term)
- *term = 0;
- an.avc_class = strdup(str);
- if (term)
- *term = ' ';
if (audit_avc_init(s) == 0) {
alist_append(s->avc, &an);
3:16 p.m.
On 2014-05-30 15:53:49, Steve Grubb wrote:
On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> This patch came from our L3 department. AppArmor LSM is logging using the
> common_lsm_audit() call but the audit userspace parsing code expects to see
> an SELinux tclass field. This patch doesn't address the lack of support for
> AppArmor in "aureport --avc". Talking to Seth Arnold, Canonical
apparently
> has patches for this; if this is true perhaps they can post for inclusion.
>
> Based-on-work-by: William Preston <wpreston(a)suse.com>
> Signed-off-by: Tony Jones <tonyj(a)suse.de>
I was looking at this patch and was wondering something. Does AppArmor produce
AUDIT_AVC events?
It does. Here's an odd ball that I picked out of my audit log:
type=AVC msg=audit(1400295012.391:11143): apparmor="DENIED"
operation="mount" info="failed type match"
profile="lxc-container-default" name="/sys/fs/cgroup/systemd/"
pid=15761 comm="mount" fstype="cgroup" srcname="systemd"
flags="rw, nosuid, nodev, noexec"
If not, how does the code even get into parse_avc? IOW, is
there another part of the patch missing in the switch statement that direct
AUDIT_APPARMOR_* events into parse_avc?
As you can see above, they already flow through parse_avc().
I spent a little time today getting a patch together to parse the
'apparmor="DENIED" operation="mount"' portion parsed to fill
an.avc_result and an.avc_perm.
It worked well with ausearch and aureport during some quick manual
testing. I'd like to do some more testing next week and, hopefully, add
some automated tests before I send it to the list.
Tyler
-Steve
> --- a/src/ausearch-parse.c 2014-05-21 14:45:22.000000000 +0200
> +++ b/src/ausearch-parse.c 2014-05-21 14:53:55.000000000 +0200
> @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
>
> // Now get the class...its at the end, so we do things different
> str = strstr(term, "tclass=");
> - if (str == NULL) {
> - rc = 9;
> - goto err;
> + if (str) {
> + str += 7;
> + term = strchr(str, ' ');
> + if (term)
> + *term = 0;
> + an.avc_class = strdup(str);
> + if (term)
> + *term = ' ';
> }
> - str += 7;
> - term = strchr(str, ' ');
> - if (term)
> - *term = 0;
> - an.avc_class = strdup(str);
> - if (term)
> - *term = ' ';
>
> if (audit_avc_init(s) == 0) {
> alist_append(s->avc, &an);
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
4 p.m.
On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
On 2014-05-30 15:53:49, Steve Grubb wrote:
> On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> > This patch came from our L3 department. AppArmor LSM is logging using
> > the
> > common_lsm_audit() call but the audit userspace parsing code expects to
> > see
> > an SELinux tclass field. This patch doesn't address the lack of support
> > for
> > AppArmor in "aureport --avc". Talking to Seth Arnold, Canonical
> > apparently
> > has patches for this; if this is true perhaps they can post for
> > inclusion.
> >
> > Based-on-work-by: William Preston <wpreston(a)suse.com>
> > Signed-off-by: Tony Jones <tonyj(a)suse.de>
>
> I was looking at this patch and was wondering something. Does AppArmor
> produce AUDIT_AVC events?
It does. Here's an odd ball that I picked out of my audit log:
Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this
problem would never happen.
libaudit.h:
#define AUDIT_FIRST_SELINUX 1400
#define AUDIT_LAST_SELINUX 1499
#define AUDIT_FIRST_APPARMOR 1500
#define AUDIT_LAST_APPARMOR 1599
When you have an event of the same number, it has to have the same fields in
the same order with same value representation. As you can see the reporting
tools is sensitive to this because they are optimized to handle huge log files.
Hmmm....
type=AVC msg=audit(1400295012.391:11143):
apparmor="DENIED"
operation="mount" info="failed type match"
profile="lxc-container-default"
name="/sys/fs/cgroup/systemd/" pid=15761 comm="mount"
fstype="cgroup"
srcname="systemd" flags="rw, nosuid, nodev, noexec"
> If not, how does the code even get into parse_avc? IOW, is
> there another part of the patch missing in the switch statement that
> direct AUDIT_APPARMOR_* events into parse_avc?
As you can see above, they already flow through parse_avc().
This is a big mistake, IMHO. In theory, this is what should have happened:
An access decisionl event should have been named in the 1500 block. It would
then be free to include the field it needs in the order it needs. The ausearch
would get a function parse_aa_decision. That function would stuff a struct
specially tuned for AA usage. Aureport would gain a new report.
I spent a little time today getting a patch together to parse the
'apparmor="DENIED" operation="mount"' portion parsed to fill
an.avc_result and an.avc_perm.
It worked well with ausearch and aureport during some quick manual
testing. I'd like to do some more testing next week and, hopefully, add
some automated tests before I send it to the list.
Well...I wished it had been clear a long time ago that the 1500 block was free
to do anything with for AA's needs so we don't overload the usage. Not sure
what to do about this.
-Steve
7:01 p.m.
On 05/30/2014 02:00 PM, Steve Grubb wrote:
This is a big mistake, IMHO. In theory, this is what should have
happened:
An access decisionl event should have been named in the 1500 block. It would
then be free to include the field it needs in the order it needs. The ausearch
would get a function parse_aa_decision. That function would stuff a struct
specially tuned for AA usage. Aureport would gain a new report.
The very original AA submission logged everything from the kernel using AUDIT_AA which was
defined in the submission as:
+#define AUDIT_AA 1500 /* AppArmor audit */
I'm not sure when the change was made to call common_lsm_audit() which logs as
AUDIT_AVC. I agree with Steve, doesn't seem a good idea.
tony
1:46 p.m.
On 2014-05-30 17:00:04, Steve Grubb wrote:
On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
> On 2014-05-30 15:53:49, Steve Grubb wrote:
> > On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> > > This patch came from our L3 department. AppArmor LSM is logging using
> > > the
> > > common_lsm_audit() call but the audit userspace parsing code expects to
> > > see
> > > an SELinux tclass field. This patch doesn't address the lack of
support
> > > for
> > > AppArmor in "aureport --avc". Talking to Seth Arnold,
Canonical
> > > apparently
> > > has patches for this; if this is true perhaps they can post for
> > > inclusion.
> > >
> > > Based-on-work-by: William Preston <wpreston(a)suse.com>
> > > Signed-off-by: Tony Jones <tonyj(a)suse.de>
> >
> > I was looking at this patch and was wondering something. Does AppArmor
> > produce AUDIT_AVC events?
>
> It does. Here's an odd ball that I picked out of my audit log:
Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this
problem would never happen.
libaudit.h:
#define AUDIT_FIRST_SELINUX 1400
#define AUDIT_LAST_SELINUX 1499
#define AUDIT_FIRST_APPARMOR 1500
#define AUDIT_LAST_APPARMOR 1599
I wasn't involved with AppArmor when it was going through upstream
acceptance reviews, but I've asked around to get the history.
As Tony mentioned, AppArmor was originally using the 1500-1599 block. At
some point (I couldn't find it in the list archives), it was said that
AppArmor needs to use common_lsm_audit() which unconditionally uses
AUDIT_AVC.
That function is used by SELinux, SMACK, and AppArmor. SMACK was the
first user of that function:
http://git.kernel.org/linus/6e837fb152410e571a81aaadbd9884f0bc46a55e
http://git.kernel.org/linus/ecfcc53fef3c357574bb6143dce6631e6d56295c
AppArmor has used common_lsm_audit() since its upstream acceptance:
http://git.kernel.org/linus/67012e8209df95a8290d135753ff5145431a666e
When you have an event of the same number, it has to have the same fields in
the same order with same value representation. As you can see the reporting
tools is sensitive to this because they are optimized to handle huge log files.
Hmmm....
> type=AVC msg=audit(1400295012.391:11143): apparmor="DENIED"
> operation="mount" info="failed type match"
profile="lxc-container-default"
> name="/sys/fs/cgroup/systemd/" pid=15761 comm="mount"
fstype="cgroup"
> srcname="systemd" flags="rw, nosuid, nodev, noexec"
> > If not, how does the code even get into parse_avc? IOW, is
> > there another part of the patch missing in the switch statement that
> > direct AUDIT_APPARMOR_* events into parse_avc?
>
> As you can see above, they already flow through parse_avc().
This is a big mistake, IMHO. In theory, this is what should have happened:
An access decisionl event should have been named in the 1500 block. It would
then be free to include the field it needs in the order it needs. The ausearch
would get a function parse_aa_decision. That function would stuff a struct
specially tuned for AA usage. Aureport would gain a new report.
> I spent a little time today getting a patch together to parse the
> 'apparmor="DENIED" operation="mount"' portion parsed to
fill
> an.avc_result and an.avc_perm.
>
> It worked well with ausearch and aureport during some quick manual
> testing. I'd like to do some more testing next week and, hopefully, add
> some automated tests before I send it to the list.
Well...I wished it had been clear a long time ago that the 1500 block was free
to do anything with for AA's needs so we don't overload the usage. Not sure
what to do about this.
There must have been a disconnect between what you were envisioning in
the audit tools and the merging of common_lsm_audit().
There are now two options going forward:
1) AppArmor and SMACK keep the current audit values and the audit tools
parse a little deeper.
SELinux audit events are separated from other LSM audit events by the
"avc: denied {" prefix.
AppArmor audit events are separated from other LSM audit events by
the "apparmor=" prefix.
And so on for SMACK and others...
I'm assuming that other audit event types can easily be separated
this way, but I haven't looked into that yet.
2) AppArmor, SMACK, and other non-SELinux LSMs switch over to using
their own block for audit events. This creates kernel work to change
over to the new block. It also creates work in the AppArmor tools, as
they do parsing of audit events, too.
This is the cleanest solution but, as usual, requires the most work.
Tyler
4:10 p.m.
[Added Eric to cc]
On 2014-06-06 13:46:48, Tyler Hicks wrote:
On 2014-05-30 17:00:04, Steve Grubb wrote:
> On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
> > On 2014-05-30 15:53:49, Steve Grubb wrote:
> > > On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> > > > This patch came from our L3 department. AppArmor LSM is logging
using
> > > > the
> > > > common_lsm_audit() call but the audit userspace parsing code expects
to
> > > > see
> > > > an SELinux tclass field. This patch doesn't address the lack of
support
> > > > for
> > > > AppArmor in "aureport --avc". Talking to Seth Arnold,
Canonical
> > > > apparently
> > > > has patches for this; if this is true perhaps they can post for
> > > > inclusion.
> > > >
> > > > Based-on-work-by: William Preston <wpreston(a)suse.com>
> > > > Signed-off-by: Tony Jones <tonyj(a)suse.de>
> > >
> > > I was looking at this patch and was wondering something. Does AppArmor
> > > produce AUDIT_AVC events?
> >
> > It does. Here's an odd ball that I picked out of my audit log:
>
> Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this
> problem would never happen.
>
> libaudit.h:
> #define AUDIT_FIRST_SELINUX 1400
> #define AUDIT_LAST_SELINUX 1499
> #define AUDIT_FIRST_APPARMOR 1500
> #define AUDIT_LAST_APPARMOR 1599
I wasn't involved with AppArmor when it was going through upstream
acceptance reviews, but I've asked around to get the history.
As Tony mentioned, AppArmor was originally using the 1500-1599 block. At
some point (I couldn't find it in the list archives), it was said that
AppArmor needs to use common_lsm_audit() which unconditionally uses
AUDIT_AVC.
I found the review that caused AppArmor to switch to the common LSM
audit function:
https://lkml.org/lkml/2009/11/9/232
That email is almost 5 years old and minds can change over that time,
but Eric seemed to be against adding new audit event types for each LSM.
Instead, he wanted a lsm=<LSM> pair to be included in the message.
AppArmor can accommodate either approach so I think Steve and Eric ought
to come to an agreement on what non-SELinux LSMs should do when
auditing.
Tyler
7:06 p.m.
On 06/06/2014 02:10 PM, Tyler Hicks wrote:
[Added Eric to cc]
You didn't actually add Eric to the Cc: Adding him.
On 2014-06-06 13:46:48, Tyler Hicks wrote:
> On 2014-05-30 17:00:04, Steve Grubb wrote:
>> On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
>>> On 2014-05-30 15:53:49, Steve Grubb wrote:
>>>> On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
>>>>> This patch came from our L3 department. AppArmor LSM is logging
using
>>>>> the
>>>>> common_lsm_audit() call but the audit userspace parsing code expects
to
>>>>> see
>>>>> an SELinux tclass field. This patch doesn't address the lack of
support
>>>>> for
>>>>> AppArmor in "aureport --avc". Talking to Seth Arnold,
Canonical
>>>>> apparently
>>>>> has patches for this; if this is true perhaps they can post for
>>>>> inclusion.
>>>>>
>>>>> Based-on-work-by: William Preston <wpreston(a)suse.com>
>>>>> Signed-off-by: Tony Jones <tonyj(a)suse.de>
>>>>
>>>> I was looking at this patch and was wondering something. Does AppArmor
>>>> produce AUDIT_AVC events?
>>>
>>> It does. Here's an odd ball that I picked out of my audit log:
>>
>> Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this
>> problem would never happen.
>>
>> libaudit.h:
>> #define AUDIT_FIRST_SELINUX 1400
>> #define AUDIT_LAST_SELINUX 1499
>> #define AUDIT_FIRST_APPARMOR 1500
>> #define AUDIT_LAST_APPARMOR 1599
>
> I wasn't involved with AppArmor when it was going through upstream
> acceptance reviews, but I've asked around to get the history.
>
> As Tony mentioned, AppArmor was originally using the 1500-1599 block. At
> some point (I couldn't find it in the list archives), it was said that
> AppArmor needs to use common_lsm_audit() which unconditionally uses
> AUDIT_AVC.
I found the review that caused AppArmor to switch to the common LSM
audit function:
https://lkml.org/lkml/2009/11/9/232
That email is almost 5 years old and minds can change over that time,
but Eric seemed to be against adding new audit event types for each LSM.
Instead, he wanted a lsm=<LSM> pair to be included in the message.
AppArmor can accommodate either approach so I think Steve and Eric ought
to come to an agreement on what non-SELinux LSMs should do when
auditing.
Tyler
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
10:34 a.m.
I'm fine if other LSMs would like to use their own record type. Makes
sense.
-Eric
On Mon, 23 Jun 2014 17:06:55 -0700
Tony Jones <tonyj(a)suse.de> wrote:
On 06/06/2014 02:10 PM, Tyler Hicks wrote:
> [Added Eric to cc]
You didn't actually add Eric to the Cc: Adding him.
>
> On 2014-06-06 13:46:48, Tyler Hicks wrote:
>> On 2014-05-30 17:00:04, Steve Grubb wrote:
>>> On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
>>>> On 2014-05-30 15:53:49, Steve Grubb wrote:
>>>>> On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
>>>>>> This patch came from our L3 department. AppArmor LSM is
>>>>>> logging using the
>>>>>> common_lsm_audit() call but the audit userspace parsing code
>>>>>> expects to see
>>>>>> an SELinux tclass field. This patch doesn't address the
lack
>>>>>> of support for
>>>>>> AppArmor in "aureport --avc". Talking to Seth
Arnold,
>>>>>> Canonical apparently
>>>>>> has patches for this; if this is true perhaps they can post for
>>>>>> inclusion.
>>>>>>
>>>>>> Based-on-work-by: William Preston <wpreston(a)suse.com>
>>>>>> Signed-off-by: Tony Jones <tonyj(a)suse.de>
>>>>>
>>>>> I was looking at this patch and was wondering something. Does
>>>>> AppArmor produce AUDIT_AVC events?
>>>>
>>>> It does. Here's an odd ball that I picked out of my audit log:
>>>
>>> Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so
>>> that this problem would never happen.
>>>
>>> libaudit.h:
>>> #define AUDIT_FIRST_SELINUX 1400
>>> #define AUDIT_LAST_SELINUX 1499
>>> #define AUDIT_FIRST_APPARMOR 1500
>>> #define AUDIT_LAST_APPARMOR 1599
>>
>> I wasn't involved with AppArmor when it was going through upstream
>> acceptance reviews, but I've asked around to get the history.
>>
>> As Tony mentioned, AppArmor was originally using the 1500-1599
>> block. At some point (I couldn't find it in the list archives), it
>> was said that AppArmor needs to use common_lsm_audit() which
>> unconditionally uses AUDIT_AVC.
>
> I found the review that caused AppArmor to switch to the common LSM
> audit function:
>
> https://lkml.org/lkml/2009/11/9/232
>
> That email is almost 5 years old and minds can change over that
> time, but Eric seemed to be against adding new audit event types
> for each LSM. Instead, he wanted a lsm=<LSM> pair to be included in
> the message.
>
> AppArmor can accommodate either approach so I think Steve and Eric
> ought to come to an agreement on what non-SELinux LSMs should do
> when auditing.
>
> Tyler
>
>
>
> --
> Linux-audit mailing list
> Linux-audit(a)redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
>
3833
days inactive
3860
days old
linux-audit@lists.linux-audit.osci.io
15 comments
4 participants
participants (4)
-
Eric Paris -
Steve Grubb -
Tony Jones -
Tyler Hicks