11:30 a.m.
The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same thing
as the event type and is missing the uid and results field. The bigger issue
is that in some failure cases no event is emitted. This patch fixes the noted
problems.
Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
---
--- vanilla-4.9-rc5.orig/security/selinux/selinuxfs.c 2016-11-16 15:16:34.738723900 -0500
+++ linux-4.9.0-0.rc5.git0.1.fc24.x86_64/security/selinux/selinuxfs.c 2016-11-21
12:16:08.046787604 -0500
@@ -494,6 +494,7 @@ static ssize_t sel_write_load(struct fil
{
ssize_t length;
void *data = NULL;
+ unsigned int result = 0;
mutex_lock(&sel_mutex);
@@ -525,24 +526,26 @@ static ssize_t sel_write_load(struct fil
length = sel_make_bools();
if (length)
- goto out1;
+ goto out;
length = sel_make_classes();
if (length)
- goto out1;
+ goto out;
length = sel_make_policycap();
if (length)
- goto out1;
+ goto out;
length = count;
+ result = 1;
-out1:
+out:
audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_POLICY_LOAD,
- "policy loaded auid=%u ses=%u",
+ "uid=%u auid=%u ses=%u res=%u",
+ from_kuid(&init_user_ns, task_uid(current)),
from_kuid(&init_user_ns, audit_get_loginuid(current)),
- audit_get_sessionid(current));
-out:
+ audit_get_sessionid(current), result);
+
mutex_unlock(&sel_mutex);
vfree(data);
return length;
3:50 p.m.
On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb <sgrubb(a)redhat.com> wrote:
The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same
thing
as the event type and is missing the uid and results field. The bigger issue
is that in some failure cases no event is emitted. This patch fixes the noted
problems.
Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
First off, for patches such as these, I think it is good to CC the
affected subsystem, SELinux in this case (fixed).
Beyond that, I'm a little concerned that you adding fields to record
in the middle. In the past you've warned against inserting fields in
the middle of the record, or reordering fields in general ("you'll
break the world") due to some poor userspace practices, yet you do
these exact things when it suits you.
We need a consistent message when it comes to userspace record
processing so we know what we can do in the kernel without causing
massive failure.
--- vanilla-4.9-rc5.orig/security/selinux/selinuxfs.c 2016-11-16
15:16:34.738723900 -0500
+++ linux-4.9.0-0.rc5.git0.1.fc24.x86_64/security/selinux/selinuxfs.c 2016-11-21
12:16:08.046787604 -0500
@@ -494,6 +494,7 @@ static ssize_t sel_write_load(struct fil
{
ssize_t length;
void *data = NULL;
+ unsigned int result = 0;
mutex_lock(&sel_mutex);
@@ -525,24 +526,26 @@ static ssize_t sel_write_load(struct fil
length = sel_make_bools();
if (length)
- goto out1;
+ goto out;
length = sel_make_classes();
if (length)
- goto out1;
+ goto out;
length = sel_make_policycap();
if (length)
- goto out1;
+ goto out;
length = count;
+ result = 1;
-out1:
+out:
audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_POLICY_LOAD,
- "policy loaded auid=%u ses=%u",
+ "uid=%u auid=%u ses=%u res=%u",
+ from_kuid(&init_user_ns, task_uid(current)),
from_kuid(&init_user_ns, audit_get_loginuid(current)),
- audit_get_sessionid(current));
-out:
+ audit_get_sessionid(current), result);
+
mutex_unlock(&sel_mutex);
vfree(data);
return length;
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
4:51 p.m.
On Monday, November 21, 2016 4:50:03 PM EST Paul Moore wrote:
On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb
<sgrubb(a)redhat.com> wrote:
> The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same
> thing as the event type and is missing the uid and results field. The
> bigger issue is that in some failure cases no event is emitted. This patch
> fixes the noted problems.
>
> Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
First off, for patches such as these, I think it is good to CC the
affected subsystem, SELinux in this case (fixed).
OK.
Beyond that, I'm a little concerned that you adding fields to
record
in the middle. In the past you've warned against inserting fields in
the middle of the record, or reordering fields in general ("you'll
break the world") due to some poor userspace practices, yet you do
these exact things when it suits you.
Its not when its suits me, its when it makes sense. This aligns
AUDIT_MAC_POLICY_LOAD with more event patterns. What I think the solution is
is to have a logging function that takes care of the sequence so that people
do not have to hand code it like this is.
We need a consistent message when it comes to userspace record
processing so we know what we can do in the kernel without causing
massive failure.
I'd like to finish aligning all simple events to a standard for the next kernel
release.
-Steve
> --- vanilla-4.9-rc5.orig/security/selinux/selinuxfs.c
2016-11-16
> 15:16:34.738723900 -0500 +++
> linux-4.9.0-0.rc5.git0.1.fc24.x86_64/security/selinux/selinuxfs.c
> 2016-11-21 12:16:08.046787604 -0500 @@ -494,6 +494,7 @@ static ssize_t
> sel_write_load(struct fil
>
> {
>
> ssize_t length;
> void *data = NULL;
>
> + unsigned int result = 0;
>
> mutex_lock(&sel_mutex);
>
> @@ -525,24 +526,26 @@ static ssize_t sel_write_load(struct fil
>
> length = sel_make_bools();
> if (length)
>
> - goto out1;
> + goto out;
>
> length = sel_make_classes();
> if (length)
>
> - goto out1;
> + goto out;
>
> length = sel_make_policycap();
> if (length)
>
> - goto out1;
> + goto out;
>
> length = count;
>
> + result = 1;
>
> -out1:
>
> +out:
> audit_log(current->audit_context, GFP_KERNEL,
> AUDIT_MAC_POLICY_LOAD,
>
> - "policy loaded auid=%u ses=%u",
> + "uid=%u auid=%u ses=%u res=%u",
> + from_kuid(&init_user_ns, task_uid(current)),
>
> from_kuid(&init_user_ns, audit_get_loginuid(current)),
>
> - audit_get_sessionid(current));
> -out:
> + audit_get_sessionid(current), result);
> +
>
> mutex_unlock(&sel_mutex);
> vfree(data);
> return length;
>
> --
> Linux-audit mailing list
> Linux-audit(a)redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
7:56 a.m.
On 11/21/2016 04:50 PM, Paul Moore wrote:
On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb
<sgrubb(a)redhat.com> wrote:
> The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same thing
> as the event type and is missing the uid and results field. The bigger issue
> is that in some failure cases no event is emitted. This patch fixes the noted
> problems.
A potential problem with this patch is that it changes the semantic
meaning of this audit record, from meaning "a policy was loaded into the
kernel" to "there was an attempt to load a policy, check the res= field
to determine whether it succeeded". So anything in userspace that used
the presence of this audit record to determine whether or not policy was
successfully loaded (e.g. audit2allow -l) will be confused. While there
were failure cases that would still generate the audit record
previously, those were all selinuxfs node creation failures; the policy
would nonetheless have been loaded into the kernel and would be active
at that point, so saying res=0 is somewhat misleading. This overlaps
with https://github.com/SELinuxProject/selinux-kernel/issues/1, which
highlights the fact that we can end up in an intermediate state where
policy is loaded but selinuxfs (particularly booleans, class/*, and
policy_capabilities/*) has not been regenerated.
>
> Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
First off, for patches such as these, I think it is good to CC the
affected subsystem, SELinux in this case (fixed).
Beyond that, I'm a little concerned that you adding fields to record
in the middle. In the past you've warned against inserting fields in
the middle of the record, or reordering fields in general ("you'll
break the world") due to some poor userspace practices, yet you do
these exact things when it suits you.
We need a consistent message when it comes to userspace record
processing so we know what we can do in the kernel without causing
massive failure.
> --- vanilla-4.9-rc5.orig/security/selinux/selinuxfs.c 2016-11-16 15:16:34.738723900
-0500
> +++ linux-4.9.0-0.rc5.git0.1.fc24.x86_64/security/selinux/selinuxfs.c 2016-11-21
12:16:08.046787604 -0500
> @@ -494,6 +494,7 @@ static ssize_t sel_write_load(struct fil
> {
> ssize_t length;
> void *data = NULL;
> + unsigned int result = 0;
>
> mutex_lock(&sel_mutex);
>
> @@ -525,24 +526,26 @@ static ssize_t sel_write_load(struct fil
>
> length = sel_make_bools();
> if (length)
> - goto out1;
> + goto out;
>
> length = sel_make_classes();
> if (length)
> - goto out1;
> + goto out;
>
> length = sel_make_policycap();
> if (length)
> - goto out1;
> + goto out;
>
> length = count;
> + result = 1;
>
> -out1:
> +out:
> audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_POLICY_LOAD,
> - "policy loaded auid=%u ses=%u",
> + "uid=%u auid=%u ses=%u res=%u",
> + from_kuid(&init_user_ns, task_uid(current)),
> from_kuid(&init_user_ns, audit_get_loginuid(current)),
> - audit_get_sessionid(current));
> -out:
> + audit_get_sessionid(current), result);
> +
> mutex_unlock(&sel_mutex);
> vfree(data);
> return length;
>
> --
> Linux-audit mailing list
> Linux-audit(a)redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
8:28 a.m.
On Tuesday, November 22, 2016 8:56:57 AM EST Stephen Smalley wrote:
On 11/21/2016 04:50 PM, Paul Moore wrote:
> On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb <sgrubb(a)redhat.com> wrote:
>> The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same
>> thing
>> as the event type and is missing the uid and results field. The bigger
>> issue is that in some failure cases no event is emitted. This patch
>> fixes the noted problems.
A potential problem with this patch is that it changes the semantic
meaning of this audit record, from meaning "a policy was loaded into the
kernel" to "there was an attempt to load a policy, check the res= field
to determine whether it succeeded". So anything in userspace that used
the presence of this audit record to determine whether or not policy was
successfully loaded (e.g. audit2allow -l) will be confused.
I really can't have implicit success. I need to have a field to point to that
says yes/no. It can be hard coded to res=1 (success), but it needs to be
there.
While there were failure cases that would still generate the audit
record
previously, those were all selinuxfs node creation failures; the policy
would nonetheless have been loaded into the kernel and would be active
at that point, so saying res=0 is somewhat misleading.
OK. We can move the point where res=1 is set. But I would think that its a
requirement to have an audit record that states that policy failed to load.
FMT_MSA.3 Static Attribute Initialization. Auditable events: All modifications
of the initial value of security attributes. I would think this means changes
such as booleans, modifying labels, loading a new policy, or failure to load a
policy.
This overlapswith
https://github.com/SELinuxProject/selinux-kernel/issues/1,
which highlights the fact that we can end up in an intermediate state where
policy is loaded but selinuxfs (particularly booleans, class/*, and
policy_capabilities/*) has not been regenerated.
I see. That should be an audited event. If you have a datacenter with a
thousand machines, its best to get this in the audit trail so it can be
alerted on at a central collector.
So, what should we do about the patch? I'm willing to modify it.
-Steve
>> Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
>
> First off, for patches such as these, I think it is good to CC the
> affected subsystem, SELinux in this case (fixed).
>
> Beyond that, I'm a little concerned that you adding fields to record
> in the middle. In the past you've warned against inserting fields in
> the middle of the record, or reordering fields in general ("you'll
> break the world") due to some poor userspace practices, yet you do
> these exact things when it suits you.
>
> We need a consistent message when it comes to userspace record
> processing so we know what we can do in the kernel without causing
> massive failure.
>
>> --- vanilla-4.9-rc5.orig/security/selinux/selinuxfs.c 2016-11-16
>> 15:16:34.738723900 -0500 +++
>> linux-4.9.0-0.rc5.git0.1.fc24.x86_64/security/selinux/selinuxfs.c
>> 2016-11-21 12:16:08.046787604 -0500 @@ -494,6 +494,7 @@ static ssize_t
>> sel_write_load(struct fil
>>
>> {
>>
>> ssize_t length;
>> void *data = NULL;
>>
>> + unsigned int result = 0;
>>
>> mutex_lock(&sel_mutex);
>>
>> @@ -525,24 +526,26 @@ static ssize_t sel_write_load(struct fil
>>
>> length = sel_make_bools();
>> if (length)
>>
>> - goto out1;
>> + goto out;
>>
>> length = sel_make_classes();
>> if (length)
>>
>> - goto out1;
>> + goto out;
>>
>> length = sel_make_policycap();
>> if (length)
>>
>> - goto out1;
>> + goto out;
>>
>> length = count;
>>
>> + result = 1;
>>
>> -out1:
>>
>> +out:
>> audit_log(current->audit_context, GFP_KERNEL,
>> AUDIT_MAC_POLICY_LOAD,
>>
>> - "policy loaded auid=%u ses=%u",
>> + "uid=%u auid=%u ses=%u res=%u",
>> + from_kuid(&init_user_ns, task_uid(current)),
>>
>> from_kuid(&init_user_ns, audit_get_loginuid(current)),
>>
>> - audit_get_sessionid(current));
>> -out:
>> + audit_get_sessionid(current), result);
>> +
>>
>> mutex_unlock(&sel_mutex);
>> vfree(data);
>> return length;
>>
>> --
>> Linux-audit mailing list
>> Linux-audit(a)redhat.com
>> https://www.redhat.com/mailman/listinfo/linux-audit
8:55 a.m.
On 11/22/2016 09:28 AM, Steve Grubb wrote:
On Tuesday, November 22, 2016 8:56:57 AM EST Stephen Smalley wrote:
> On 11/21/2016 04:50 PM, Paul Moore wrote:
>> On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb <sgrubb(a)redhat.com> wrote:
>>> The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same
>>> thing
>>> as the event type and is missing the uid and results field. The bigger
>>> issue is that in some failure cases no event is emitted. This patch
>>> fixes the noted problems.
>
> A potential problem with this patch is that it changes the semantic
> meaning of this audit record, from meaning "a policy was loaded into the
> kernel" to "there was an attempt to load a policy, check the res= field
> to determine whether it succeeded". So anything in userspace that used
> the presence of this audit record to determine whether or not policy was
> successfully loaded (e.g. audit2allow -l) will be confused.
I really can't have implicit success. I need to have a field to point to that
says yes/no. It can be hard coded to res=1 (success), but it needs to be
there.
Ok. Why is it you use res=1|0 in these records but success=yes|no in
syscall records?
> While there were failure cases that would still generate the
audit record
> previously, those were all selinuxfs node creation failures; the policy
> would nonetheless have been loaded into the kernel and would be active
> at that point, so saying res=0 is somewhat misleading.
OK. We can move the point where res=1 is set. But I would think that its a
requirement to have an audit record that states that policy failed to load.
FMT_MSA.3 Static Attribute Initialization. Auditable events: All modifications
of the initial value of security attributes. I would think this means changes
such as booleans, modifying labels, loading a new policy, or failure to load a
policy.
Failure to load a policy is not a modification to the initial value of
the security attribute, is it?
> This overlapswith https://github.com/SELinuxProject/selinux-kernel/issues/1,
> which highlights the fact that we can end up in an intermediate state where
> policy is loaded but selinuxfs (particularly booleans, class/*, and
> policy_capabilities/*) has not been regenerated.
I see. That should be an audited event. If you have a datacenter with a
thousand machines, its best to get this in the audit trail so it can be
alerted on at a central collector.
So, what should we do about the patch? I'm willing to modify it.
At present, we only generate AUDIT_MAC_STATUS, AUDIT_MAC_LOAD, and
AUDIT_MAC_CONFIG_CHANGE on success (or at least partial success). If
you truly need to audit failures, then it seems like you either need to
a) do it through syscall audit filters, which already provide a success=
field or b) add new audit message types for this purpose (e.g.
AUDIT_MAC_STATUS_FAIL, AUDIT_MAC_LOAD_FAIL, ...). To just add a res=
field to the existing ones and change them to always be generated is a
user-visible semantic change.
12:53 p.m.
On 11/22/2016 08:55 AM, Stephen Smalley wrote:
> >OK. We can move the point where res=1 is set. But I would
think that its a
> >requirement to have an audit record that states that policy failed to load.
> >FMT_MSA.3 Static Attribute Initialization. Auditable events: All modifications
> >of the initial value of security attributes. I would think this means changes
> >such as booleans, modifying labels, loading a new policy, or failure to load a
> >policy.
Failure to load a policy is not a modification to the initial value of
the security attribute, is it?
It is definitely relevant, if it falls under another category.
Either a failed malicious intent or a failed supervisory function.
LCB
1:39 p.m.
On Tuesday, November 22, 2016 9:55:11 AM EST Stephen Smalley wrote:
On 11/22/2016 09:28 AM, Steve Grubb wrote:
> On Tuesday, November 22, 2016 8:56:57 AM EST Stephen Smalley wrote:
>> On 11/21/2016 04:50 PM, Paul Moore wrote:
>>> On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb <sgrubb(a)redhat.com>
wrote:
>>>> The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same
>>>> thing as the event type and is missing the uid and results field. The
>>>> bigger issue is that in some failure cases no event is emitted. This
>>>> patch fixes the noted problems.
>>
>> A potential problem with this patch is that it changes the semantic
>> meaning of this audit record, from meaning "a policy was loaded into the
>> kernel" to "there was an attempt to load a policy, check the res=
field
>> to determine whether it succeeded". So anything in userspace that used
>> the presence of this audit record to determine whether or not policy was
>> successfully loaded (e.g. audit2allow -l) will be confused.
>
> I really can't have implicit success. I need to have a field to point to
> that says yes/no. It can be hard coded to res=1 (success), but it needs
> to be there.
Ok. Why is it you use res=1|0 in these records but success=yes|no in
syscall records?
Success is only used in syscall records. It was created to disambiguate the
exit field which can look like a failure on some arches but is actually OK.
Originally the exit field was all that existed. We found that one or two arches
actually have 2 return codes. The whole rest of the audit system uses res= to
mean a crisp yes/no this did or didn't happen. This predates the creation of
the success field. It could be argued success should have been res, but
ausearch/report will use either to mean the same thing.
The new auparse field classifier work will also map both to mean the same thing.
That is how I found AUDIT_MAC_POLICY_LOAD missing any kind of success/fail
indication.
>> While there were failure cases that would still generate the
audit record
>> previously, those were all selinuxfs node creation failures; the policy
>> would nonetheless have been loaded into the kernel and would be active
>> at that point, so saying res=0 is somewhat misleading.
>
> OK. We can move the point where res=1 is set. But I would think that its a
> requirement to have an audit record that states that policy failed to
> load.
> FMT_MSA.3 Static Attribute Initialization. Auditable events: All
> modifications of the initial value of security attributes. I would think
> this means changes such as booleans, modifying labels, loading a new
> policy, or failure to load a policy.
Failure to load a policy is not a modification to the initial value of
the security attribute, is it?
Sure. If the state is intended to enabled and enforcing and its not, that
would be a surprising result that there was no indication in the audit trail.
Also, if for some reason it booted fine and a new policy was loaded at some
point in the future and it failed, then we have a modification to initial
state.
>> This overlapswith
>> https://github.com/SELinuxProject/selinux-kernel/issues/1, which
>> highlights the fact that we can end up in an intermediate state where
>> policy is loaded but selinuxfs (particularly booleans, class/*, and
>> policy_capabilities/*) has not been regenerated.
>
> I see. That should be an audited event. If you have a datacenter with a
> thousand machines, its best to get this in the audit trail so it can be
> alerted on at a central collector.
>
> So, what should we do about the patch? I'm willing to modify it.
At present, we only generate AUDIT_MAC_STATUS, AUDIT_MAC_LOAD, and
AUDIT_MAC_CONFIG_CHANGE on success (or at least partial success). If
you truly need to audit failures, then it seems like you either need to
a) do it through syscall audit filters, which already provide a success=
field
I can't imagine what to audit on. There is an open syscall that has a path.
But I suspect that does not fail because policy has not be written. There is a
write syscall but triggering on that is pretty generic. This is not ideal.
or b) add new audit message types for this purpose (e.g.
AUDIT_MAC_STATUS_FAIL, AUDIT_MAC_LOAD_FAIL, ...). To just add a res=
field to the existing ones and change them to always be generated is a
user-visible semantic change.
OK. This is do-able. I'll hard code the 'res' field for each of them.
-Steve
1:47 p.m.
On 11/22/2016 02:39 PM, Steve Grubb wrote:
On Tuesday, November 22, 2016 9:55:11 AM EST Stephen Smalley wrote:
> On 11/22/2016 09:28 AM, Steve Grubb wrote:
>> On Tuesday, November 22, 2016 8:56:57 AM EST Stephen Smalley wrote:
>>> On 11/21/2016 04:50 PM, Paul Moore wrote:
>>>> On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb <sgrubb(a)redhat.com>
wrote:
>>>>> The AUDIT_MAC_POLICY_LOAD event has dangling text that means the
same
>>>>> thing as the event type and is missing the uid and results field.
The
>>>>> bigger issue is that in some failure cases no event is emitted. This
>>>>> patch fixes the noted problems.
>>>
>>> A potential problem with this patch is that it changes the semantic
>>> meaning of this audit record, from meaning "a policy was loaded into
the
>>> kernel" to "there was an attempt to load a policy, check the res=
field
>>> to determine whether it succeeded". So anything in userspace that used
>>> the presence of this audit record to determine whether or not policy was
>>> successfully loaded (e.g. audit2allow -l) will be confused.
>>
>> I really can't have implicit success. I need to have a field to point to
>> that says yes/no. It can be hard coded to res=1 (success), but it needs
>> to be there.
>
> Ok. Why is it you use res=1|0 in these records but success=yes|no in
> syscall records?
Success is only used in syscall records. It was created to disambiguate the
exit field which can look like a failure on some arches but is actually OK.
Originally the exit field was all that existed. We found that one or two arches
actually have 2 return codes. The whole rest of the audit system uses res= to
mean a crisp yes/no this did or didn't happen. This predates the creation of
the success field. It could be argued success should have been res, but
ausearch/report will use either to mean the same thing.
The new auparse field classifier work will also map both to mean the same thing.
That is how I found AUDIT_MAC_POLICY_LOAD missing any kind of success/fail
indication.
>>> While there were failure cases that would still generate the audit record
>>> previously, those were all selinuxfs node creation failures; the policy
>>> would nonetheless have been loaded into the kernel and would be active
>>> at that point, so saying res=0 is somewhat misleading.
>>
>> OK. We can move the point where res=1 is set. But I would think that its a
>> requirement to have an audit record that states that policy failed to
>> load.
>> FMT_MSA.3 Static Attribute Initialization. Auditable events: All
>> modifications of the initial value of security attributes. I would think
>> this means changes such as booleans, modifying labels, loading a new
>> policy, or failure to load a policy.
>
> Failure to load a policy is not a modification to the initial value of
> the security attribute, is it?
Sure. If the state is intended to enabled and enforcing and its not, that
would be a surprising result that there was no indication in the audit trail.
Also, if for some reason it booted fine and a new policy was loaded at some
point in the future and it failed, then we have a modification to initial
state.
>>> This overlapswith
>>> https://github.com/SELinuxProject/selinux-kernel/issues/1, which
>>> highlights the fact that we can end up in an intermediate state where
>>> policy is loaded but selinuxfs (particularly booleans, class/*, and
>>> policy_capabilities/*) has not been regenerated.
>>
>> I see. That should be an audited event. If you have a datacenter with a
>> thousand machines, its best to get this in the audit trail so it can be
>> alerted on at a central collector.
>>
>> So, what should we do about the patch? I'm willing to modify it.
>
> At present, we only generate AUDIT_MAC_STATUS, AUDIT_MAC_LOAD, and
> AUDIT_MAC_CONFIG_CHANGE on success (or at least partial success). If
> you truly need to audit failures, then it seems like you either need to
> a) do it through syscall audit filters, which already provide a success=
> field
I can't imagine what to audit on. There is an open syscall that has a path.
But I suspect that does not fail because policy has not be written. There is a
write syscall but triggering on that is pretty generic. This is not ideal.
Can't you write an audit syscall filter or watch on
/sys/fs/selinux/load? Ditto for /sys/fs/selinux/enforce,
/sys/fs/selinux/commit_pending_bools, etc.
> or b) add new audit message types for this purpose (e.g.
> AUDIT_MAC_STATUS_FAIL, AUDIT_MAC_LOAD_FAIL, ...). To just add a res=
> field to the existing ones and change them to always be generated is a
> user-visible semantic change.
OK. This is do-able. I'll hard code the 'res' field for each of them.
-Steve
2:13 p.m.
On Tuesday, November 22, 2016 2:47:15 PM EST Stephen Smalley wrote:
>> At present, we only generate AUDIT_MAC_STATUS,
AUDIT_MAC_LOAD, and
>> AUDIT_MAC_CONFIG_CHANGE on success (or at least partial success). If
>> you truly need to audit failures, then it seems like you either need to
>> a) do it through syscall audit filters, which already provide a success=
>> field
>
> I can't imagine what to audit on. There is an open syscall that has a
> path. But I suspect that does not fail because policy has not be written.
> There is a write syscall but triggering on that is pretty generic. This is
> not ideal.
Can't you write an audit syscall filter or watch on
/sys/fs/selinux/load? Ditto for /sys/fs/selinux/enforce,
/sys/fs/selinux/commit_pending_bools, etc.
Yes, you can. But this is for the open syscall. sel_write_load() is the
function where the auditing is done but its mapped to the .write member of
sel_load_ops. Auditing on write is not a good thing.
So, if AUDIT_MAC_POLICY_LOAD must only appear when there is success, then its
best to create a second event for failure and hard code the 'res' fields for
both.
-Steve
2952
days inactive
2953
days old
linux-audit@lists.linux-audit.osci.io
9 comments
4 participants
participants (4)
-
Lenny Bruzenak -
Paul Moore -
Stephen Smalley -
Steve Grubb