Discussion:
[RFC][PATCH] audit: add feature audit_lost reset
(too old to reply)
Richard Guy Briggs
2016-12-05 08:02:37 UTC
Permalink
Add a method to reset the audit_lost value.

An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.

If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.

See: https://github.com/linux-audit/audit-kernel/issues/3

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..5eb2dc2 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -330,10 +330,12 @@ enum {
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004
#define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND 0x00000008
+#define AUDIT_FEATURE_BITMAP_LOST_RESET 0x00000010
#define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \
- AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND)
+ AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
+ AUDIT_FEATURE_BITMAP_LOST_RESET)

/* deprecated: AUDIT_VERSION_* */
#define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL
@@ -440,7 +442,8 @@ struct audit_features {

#define AUDIT_FEATURE_ONLY_UNSET_LOGINUID 0
#define AUDIT_FEATURE_LOGINUID_IMMUTABLE 1
-#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOGINUID_IMMUTABLE
+#define AUDIT_FEATURE_LOST_RESET 2
+#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOST_RESET

#define audit_feature_valid(x) ((x) >= 0 && (x) <= AUDIT_LAST_FEATURE)
#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32 */
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..6b52da6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);

/* The netlink socket. */
static struct sock *audit_sock;
@@ -150,9 +150,10 @@
.features = 0,
.lock = 0,};

-static char *audit_feature_names[2] = {
+static char *audit_feature_names[3] = {
"only_unset_loginuid",
"loginuid_immutable",
+ "lost_reset",
};


@@ -854,7 +855,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
s.pid = audit_pid;
s.rate_limit = audit_rate_limit;
s.backlog_limit = audit_backlog_limit;
- s.lost = atomic_read(&audit_lost);
+ s.lost = is_audit_feature_set(AUDIT_FEATURE_LOST_RESET) ?
+ atomic_xchg(&audit_lost, 0) :
+ atomic_read(&audit_lost);
s.backlog = skb_queue_len(&audit_skb_queue);
s.feature_bitmap = AUDIT_FEATURE_BITMAP_ALL;
s.backlog_wait_time = audit_backlog_wait_time_master;
--
1.7.1
Paul Moore
2016-12-05 16:02:29 UTC
Permalink
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).

I'm mixed on adding this to the feature bitmap, it shouldn't be
strictly necessary as old kernels will simply ignore the
AUDIT_SET/AUDIT_STATUS_LOST bit, but I can understand if userspace
might want it ... I just hate to burn a bit in the bitmap for
something that has no ill effect on behavior.
Post by Richard Guy Briggs
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..5eb2dc2 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -330,10 +330,12 @@ enum {
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004
#define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND 0x00000008
+#define AUDIT_FEATURE_BITMAP_LOST_RESET 0x00000010
#define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \
- AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND)
+ AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
+ AUDIT_FEATURE_BITMAP_LOST_RESET)
/* deprecated: AUDIT_VERSION_* */
#define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL
@@ -440,7 +442,8 @@ struct audit_features {
#define AUDIT_FEATURE_ONLY_UNSET_LOGINUID 0
#define AUDIT_FEATURE_LOGINUID_IMMUTABLE 1
-#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOGINUID_IMMUTABLE
+#define AUDIT_FEATURE_LOST_RESET 2
+#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOST_RESET
#define audit_feature_valid(x) ((x) >= 0 && (x) <= AUDIT_LAST_FEATURE)
#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32 */
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..6b52da6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -150,9 +150,10 @@
.features = 0,
.lock = 0,};
-static char *audit_feature_names[2] = {
+static char *audit_feature_names[3] = {
"only_unset_loginuid",
"loginuid_immutable",
+ "lost_reset",
};
@@ -854,7 +855,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
s.pid = audit_pid;
s.rate_limit = audit_rate_limit;
s.backlog_limit = audit_backlog_limit;
- s.lost = atomic_read(&audit_lost);
+ s.lost = is_audit_feature_set(AUDIT_FEATURE_LOST_RESET) ?
+ atomic_read(&audit_lost);
s.backlog = skb_queue_len(&audit_skb_queue);
s.feature_bitmap = AUDIT_FEATURE_BITMAP_ALL;
s.backlog_wait_time = audit_backlog_wait_time_master;
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2016-12-05 16:52:11 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).
I realized as I was coding it up that we would potentially lose an
accurate count if the read and reset were not atomic. This was the
reason for using atomic_xchg().
Post by Paul Moore
I'm mixed on adding this to the feature bitmap, it shouldn't be
strictly necessary as old kernels will simply ignore the
AUDIT_SET/AUDIT_STATUS_LOST bit, but I can understand if userspace
might want it ... I just hate to burn a bit in the bitmap for
something that has no ill effect on behavior.
As pointed out, we may not need the bitmap addition if we can read the
array of audit_feature_names.
Post by Paul Moore
Post by Richard Guy Briggs
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..5eb2dc2 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -330,10 +330,12 @@ enum {
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004
#define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND 0x00000008
+#define AUDIT_FEATURE_BITMAP_LOST_RESET 0x00000010
#define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \
- AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND)
+ AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
+ AUDIT_FEATURE_BITMAP_LOST_RESET)
/* deprecated: AUDIT_VERSION_* */
#define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL
@@ -440,7 +442,8 @@ struct audit_features {
#define AUDIT_FEATURE_ONLY_UNSET_LOGINUID 0
#define AUDIT_FEATURE_LOGINUID_IMMUTABLE 1
-#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOGINUID_IMMUTABLE
+#define AUDIT_FEATURE_LOST_RESET 2
+#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LOST_RESET
#define audit_feature_valid(x) ((x) >= 0 && (x) <= AUDIT_LAST_FEATURE)
#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32 */
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..6b52da6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -150,9 +150,10 @@
.features = 0,
.lock = 0,};
-static char *audit_feature_names[2] = {
+static char *audit_feature_names[3] = {
"only_unset_loginuid",
"loginuid_immutable",
+ "lost_reset",
};
@@ -854,7 +855,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
s.pid = audit_pid;
s.rate_limit = audit_rate_limit;
s.backlog_limit = audit_backlog_limit;
- s.lost = atomic_read(&audit_lost);
+ s.lost = is_audit_feature_set(AUDIT_FEATURE_LOST_RESET) ?
+ atomic_read(&audit_lost);
s.backlog = skb_queue_len(&audit_skb_queue);
s.feature_bitmap = AUDIT_FEATURE_BITMAP_ALL;
s.backlog_wait_time = audit_backlog_wait_time_master;
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2016-12-05 17:48:12 UTC
Permalink
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).
I realized as I was coding it up that we would potentially lose an
accurate count if the read and reset were not atomic. This was the
reason for using atomic_xchg().
Well, okay, but that isn't what I was talking about ... ? The
audit_cmd_mutex is held for both AUDIT_GET and AUDIT_SET so we should
never process these requests at the same time.
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2016-12-06 05:13:37 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).
I realized as I was coding it up that we would potentially lose an
accurate count if the read and reset were not atomic. This was the
reason for using atomic_xchg().
Well, okay, but that isn't what I was talking about ... ? The
audit_cmd_mutex is held for both AUDIT_GET and AUDIT_SET so we should
never process these requests at the same time.
I guess I still don't follow what you are talking about... I hadn't
thought of including both an AUDIT_GET and an AUDIT_SET in the same skb,
but that would ensure that no other command reads or resets the lost
value. However, the lost value could still be incrementing on another
CPU between these two commands, losing an accurate lost count.
Post by Paul Moore
paul moore
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2016-12-07 00:17:33 UTC
Permalink
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).
I realized as I was coding it up that we would potentially lose an
accurate count if the read and reset were not atomic. This was the
reason for using atomic_xchg().
Well, okay, but that isn't what I was talking about ... ? The
audit_cmd_mutex is held for both AUDIT_GET and AUDIT_SET so we should
never process these requests at the same time.
I guess I still don't follow what you are talking about... I hadn't
thought of including both an AUDIT_GET and an AUDIT_SET in the same skb,
but that would ensure that no other command reads or resets the lost
value. However, the lost value could still be incrementing on another
CPU between these two commands, losing an accurate lost count.
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2016-12-07 03:32:20 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).
I realized as I was coding it up that we would potentially lose an
accurate count if the read and reset were not atomic. This was the
reason for using atomic_xchg().
Well, okay, but that isn't what I was talking about ... ? The
audit_cmd_mutex is held for both AUDIT_GET and AUDIT_SET so we should
never process these requests at the same time.
I guess I still don't follow what you are talking about... I hadn't
thought of including both an AUDIT_GET and an AUDIT_SET in the same skb,
but that would ensure that no other command reads or resets the lost
value. However, the lost value could still be incrementing on another
CPU between these two commands, losing an accurate lost count.
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Post by Paul Moore
paul moore
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2016-12-07 15:05:30 UTC
Permalink
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).
I realized as I was coding it up that we would potentially lose an
accurate count if the read and reset were not atomic. This was the
reason for using atomic_xchg().
Well, okay, but that isn't what I was talking about ... ? The
audit_cmd_mutex is held for both AUDIT_GET and AUDIT_SET so we should
never process these requests at the same time.
I guess I still don't follow what you are talking about... I hadn't
thought of including both an AUDIT_GET and an AUDIT_SET in the same skb,
but that would ensure that no other command reads or resets the lost
value. However, the lost value could still be incrementing on another
CPU between these two commands, losing an accurate lost count.
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.

I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.

I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
--
paul moore
www.paul-moore.com
Steve Grubb
2016-12-07 15:53:04 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you get
the number of records lost. If you do it the next day, you have to do math to
see what the one day delta is. So, to make reporting easy, they want it to be
reset whenever they do audictl -s.

You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl that
allows an admin to say also reset the counters. If that command line option is
passed, I call AUDIT_GET_RESET otherwise I call AUDIT_GET. Thought?

-Steve
Richard Guy Briggs
2016-12-07 15:58:49 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you get
the number of records lost. If you do it the next day, you have to do math to
see what the one day delta is. So, to make reporting easy, they want it to be
reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl that
allows an admin to say also reset the counters. If that command line option is
passed, I call AUDIT_GET_RESET otherwise I call AUDIT_GET. Thought?
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
Post by Steve Grubb
-Steve
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2016-12-07 23:10:49 UTC
Permalink
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you get
the number of records lost. If you do it the next day, you have to do math to
see what the one day delta is. So, to make reporting easy, they want it to be
reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl that
allows an admin to say also reset the counters. If that command line option is
passed, I call AUDIT_GET_RESET otherwise I call AUDIT_GET. Thought?
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
I'd prefer not to introduce another command message type for something
small like this.

Steve, do you have any objection to the AUDIT_SET based approach?
Based on what you've said above, it would seem like the potential race
condition with AUDIT_SET wouldn't be a significant issue.
--
paul moore
www.paul-moore.com
Steve Grubb
2016-12-07 23:30:22 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you
get the number of records lost. If you do it the next day, you have to
do math to see what the one day delta is. So, to make reporting easy,
they want it to be reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl
that allows an admin to say also reset the counters. If that command
line option is passed, I call AUDIT_GET_RESET otherwise I call
AUDIT_GET. Thought?>
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
I'd prefer not to introduce another command message type for something
small like this.
Steve, do you have any objection to the AUDIT_SET based approach?
Either way, we'd need a feature flag so that I can tell if the kernel supports
this or not. Also, it should accept "0" as the only valid value. We can do
this and I can make auditctl do the two back to back before displaying the
results to minimize the window of risk.
Post by Paul Moore
Based on what you've said above, it would seem like the potential race
condition with AUDIT_SET wouldn't be a significant issue.
All a matter of perspective. What I think is a reasonable risk someone else
may disagree. Does anyone else on the list object? If not I'd say go with it.

-Steve
Paul Moore
2016-12-07 23:45:00 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you
get the number of records lost. If you do it the next day, you have to
do math to see what the one day delta is. So, to make reporting easy,
they want it to be reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl
that allows an admin to say also reset the counters. If that command
line option is passed, I call AUDIT_GET_RESET otherwise I call
AUDIT_GET. Thought?>
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
I'd prefer not to introduce another command message type for something
small like this.
Steve, do you have any objection to the AUDIT_SET based approach?
Either way, we'd need a feature flag so that I can tell if the kernel supports
this or not.
I think we are okay without a specific feature flag as sending a
AUDIT_SET/reset on an old kernel will be harmless; it won't do
anything, but it shouldn't return an error either.
Post by Steve Grubb
Also, it should accept "0" as the only valid value.
Of course.
Post by Steve Grubb
We can do this and I can make auditctl do the two back to back before displaying the
results to minimize the window of risk.
Post by Paul Moore
Based on what you've said above, it would seem like the potential race
condition with AUDIT_SET wouldn't be a significant issue.
All a matter of perspective. What I think is a reasonable risk someone else
may disagree. Does anyone else on the list object? If not I'd say go with it.
Judgement calls are always a matter of perspective, since you are the
only one who has asked for this (even if it is by proxy) I was asking
for your perspective. Anyway, it looks like we're probably okay here;
if we don't hear anything in the next day or two lets go ahead with
the AUDIT_SET approach. If it proves to be a problem we can always
introduce one of the other approaches later.
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2016-12-08 03:53:11 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you
get the number of records lost. If you do it the next day, you have to
do math to see what the one day delta is. So, to make reporting easy,
they want it to be reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl
that allows an admin to say also reset the counters. If that command
line option is passed, I call AUDIT_GET_RESET otherwise I call
AUDIT_GET. Thought?>
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
I'd prefer not to introduce another command message type for something
small like this.
Steve, do you have any objection to the AUDIT_SET based approach?
Either way, we'd need a feature flag so that I can tell if the kernel supports
this or not.
I think we are okay without a specific feature flag as sending a
AUDIT_SET/reset on an old kernel will be harmless; it won't do
anything, but it shouldn't return an error either.
Ok, so userspace is still left wondering if it worked until the next
time it reads that value, and even then it can't be certain if that
value is the same or higher than it was when userspace thought it reset
that value. At this point, an old kernel will not return an error and
simply ignore any new AUDIT_STATUS_* flag since each flag is treated
independently and extra flags are ignored and not blocked. This seems
sloppy since we have two ways of fixing this uncertainty pretty easily.
Post by Paul Moore
Post by Steve Grubb
Also, it should accept "0" as the only valid value.
Of course.
No problem.
Post by Paul Moore
Post by Steve Grubb
We can do this and I can make auditctl do the two back to back before displaying the
results to minimize the window of risk.
Having it depend on userspace implementation to minimize the risk of a
race strikes me as unsound design.
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Based on what you've said above, it would seem like the potential race
condition with AUDIT_SET wouldn't be a significant issue.
All a matter of perspective. What I think is a reasonable risk someone else
may disagree. Does anyone else on the list object? If not I'd say go with it.
Judgement calls are always a matter of perspective, since you are the
only one who has asked for this (even if it is by proxy) I was asking
for your perspective. Anyway, it looks like we're probably okay here;
if we don't hear anything in the next day or two lets go ahead with
the AUDIT_SET approach. If it proves to be a problem we can always
introduce one of the other approaches later.
If it isn't dependable and accurate, what's the point? Why not do it
right the first time?
Post by Paul Moore
paul moore
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2016-12-08 14:05:46 UTC
Permalink
Post by Richard Guy Briggs
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to
my
fundamental point: How do we get an accurate reading of the last
value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you
get the number of records lost. If you do it the next day, you have to
do math to see what the one day delta is. So, to make reporting easy,
they want it to be reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl
that allows an admin to say also reset the counters. If that command
line option is passed, I call AUDIT_GET_RESET otherwise I call
AUDIT_GET. Thought?>
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
I'd prefer not to introduce another command message type for something
small like this.
Steve, do you have any objection to the AUDIT_SET based approach?
Either way, we'd need a feature flag so that I can tell if the kernel supports
this or not.
I think we are okay without a specific feature flag as sending a
AUDIT_SET/reset on an old kernel will be harmless; it won't do
anything, but it shouldn't return an error either.
Ok, so userspace is still left wondering if it worked until the next
time it reads that value, and even then it can't be certain if that
value is the same or higher than it was when userspace thought it reset
that value. At this point, an old kernel will not return an error and
simply ignore any new AUDIT_STATUS_* flag since each flag is treated
independently and extra flags are ignored and not blocked. This seems
sloppy since we have two ways of fixing this uncertainty pretty easily.
Comments like the above aren't helpful, they are just annoying. The
drawbacks to the AUDIT_SET/reset approach have already been discussed
on this thread, if you want to do something constructive think about
how you can resolve these limitations within the context of others
comments/feedback.

I've already mentioned that I didn't like the AUDIT_GET/reset approach
because I thought the interface was bad. As I'm sure you know, the
audit kernel/userspace interface is a bit of a hot-button topic with
me; I think it has a lot of problems and I'm very intent on not making
it worse (in my opinion, I will admit that API design is not entirely
objective). Continuing to argue for a interface design that I've
already expressed a dislike for is not likely to win me over to your
side; regardless of the outcome you will end up frustrating both
yourself and the maintainer, neither are good things.

We all agree that there is a potential race window with respect to
reading/resetting the lost counter, where we disagree is the
likelihood of that happening in practice. You feel very strongly that
the window is of grave concern, Steve and myself much less so. If you
still feel strongly about this, think about some different ways in
which you can avoid losing a lost message counter bump. Off the top
of my head, there are really only two ways for the kernel's audit
subsystem to send information back to userspace in this case, via a
netlink return/error message or an audit record. We could possibly do
something with the netlink error message by returning the lost counter
as a positive integer (negative integer is a failure code, zero is
success), but that might get tricky in the future, although we could
mitigate that risk by forcing the AUDIT_SET/reset to happen by itself
(in other words, don't simply check to see if the bit is set in the
bitmask, e.g. (s.mask & AUDIT_STATUS_LOST), check to see it is equal,
e.g. (s.mask == AUDIT_STATUS_LOST)). We could also mitigate the race
via an audit record by emitting a record indicating that the lost
counter was reset and record the lost counter (before the reset) in
that record; honestly, now that I'm writing this, it seems like
something we should be doing regardless, as tampering with the lost
counter seems like a security relevant event.

There you go, two possible solutions for eliminating/mitigating the
potential race while sticking with the simpler AUDIT_SET/reset
interface. I suppose you could even implement both of the solutions
above, they aren't mutually exclusive; that would depend on what
Steve/userspace would prefer. Finally, as for the feature bitmap to
signal to userspace that we support this new feature: if you can't
live without it, go ahead and add it in. As I said before, I'm a
little concerned at the rate we are consuming this bitmap, but I'll
admit we still have plenty of room before we have to start worrying
about alternatives.
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2016-12-09 07:00:14 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to
my
fundamental point: How do we get an accurate reading of the last
value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you
get the number of records lost. If you do it the next day, you have to
do math to see what the one day delta is. So, to make reporting easy,
they want it to be reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl
that allows an admin to say also reset the counters. If that command
line option is passed, I call AUDIT_GET_RESET otherwise I call
AUDIT_GET. Thought?>
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
I'd prefer not to introduce another command message type for something
small like this.
Steve, do you have any objection to the AUDIT_SET based approach?
Either way, we'd need a feature flag so that I can tell if the kernel supports
this or not.
I think we are okay without a specific feature flag as sending a
AUDIT_SET/reset on an old kernel will be harmless; it won't do
anything, but it shouldn't return an error either.
Ok, so userspace is still left wondering if it worked until the next
time it reads that value, and even then it can't be certain if that
value is the same or higher than it was when userspace thought it reset
that value. At this point, an old kernel will not return an error and
simply ignore any new AUDIT_STATUS_* flag since each flag is treated
independently and extra flags are ignored and not blocked. This seems
sloppy since we have two ways of fixing this uncertainty pretty easily.
Comments like the above aren't helpful, they are just annoying. The
drawbacks to the AUDIT_SET/reset approach have already been discussed
on this thread, if you want to do something constructive think about
how you can resolve these limitations within the context of others
comments/feedback.
I'm sorry to offend. That wasn't my intent. I was trying to bring new
information and observations into the discussion. Is there anything
factually wrong in my observations other than the opinion of it being
sloppy?
Post by Paul Moore
I've already mentioned that I didn't like the AUDIT_GET/reset approach
because I thought the interface was bad. As I'm sure you know, the
audit kernel/userspace interface is a bit of a hot-button topic with
me; I think it has a lot of problems and I'm very intent on not making
it worse (in my opinion, I will admit that API design is not entirely
objective). Continuing to argue for a interface design that I've
already expressed a dislike for is not likely to win me over to your
side; regardless of the outcome you will end up frustrating both
yourself and the maintainer, neither are good things.
I've re-read the thread from the beginning. I guess I must have missed
what was the fundamental problem with the AUDIT_GET/reset method other
than taste. What don't you like about the API that precludes
using/abusing it this way? Is it the issue that a GET would
surprisingly change a value rather than just reading it? I didn't like
it either, but it was the next obvious way to tackle the issue without
losing information.
Post by Paul Moore
We all agree that there is a potential race window with respect to
reading/resetting the lost counter, where we disagree is the
likelihood of that happening in practice.
We've had other races that looked pretty unlikely in practice that were
deemed to be unacceptable. Granted the risks were higher if they were
exploited, but they were mitigated to the best of our ability.

None of this should matter now since there are ideas below that should
work.
Post by Paul Moore
You feel very strongly that
the window is of grave concern, Steve and myself much less so. If you
still feel strongly about this, think about some different ways in
which you can avoid losing a lost message counter bump. Off the top
of my head, there are really only two ways for the kernel's audit
subsystem to send information back to userspace in this case, via a
netlink return/error message or an audit record. We could possibly do
something with the netlink error message by returning the lost counter
as a positive integer (negative integer is a failure code, zero is
success), but that might get tricky in the future, although we could
mitigate that risk by forcing the AUDIT_SET/reset to happen by itself
(in other words, don't simply check to see if the bit is set in the
bitmask, e.g. (s.mask & AUDIT_STATUS_LOST), check to see it is equal,
e.g. (s.mask == AUDIT_STATUS_LOST)).
This could work. What risk do you see in doing it with other flags?
That another set failure could usurp the return code? If so, yes, I
agree with requiring it to be a lone flag.
Post by Paul Moore
We could also mitigate the race
via an audit record by emitting a record indicating that the lost
counter was reset and record the lost counter (before the reset) in
that record; honestly, now that I'm writing this, it seems like
something we should be doing regardless, as tampering with the lost
counter seems like a security relevant event.
Agreed. This should be added regardless of reset method.
Post by Paul Moore
There you go, two possible solutions for eliminating/mitigating the
potential race while sticking with the simpler AUDIT_SET/reset
interface. I suppose you could even implement both of the solutions
above, they aren't mutually exclusive; that would depend on what
Steve/userspace would prefer. Finally, as for the feature bitmap to
signal to userspace that we support this new feature: if you can't
live without it, go ahead and add it in. As I said before, I'm a
little concerned at the rate we are consuming this bitmap, but I'll
admit we still have plenty of room before we have to start worrying
about alternatives.
I would suggest that the return value (presuming it was reset when
non-zero) or the audit record generated reporting the lost value
reset would be sufficient confirmation that the feature exists on the
running kernel and the addition to the feature bitmap is not strictly
necessary, but you only find this out upon attempting that lost reset.

Well, we haven't used much of that bitmap space and if it isn't to be
used when needed, why is it there? If there is a relatively simple
alternate non-destructive way to discover the presence of a feature use
of the bitmap isn't necessary.
Post by Paul Moore
paul moore
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2016-12-09 23:46:43 UTC
Permalink
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to
my
fundamental point: How do we get an accurate reading of the last
value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
To start with, this request comes from users of the audit system. I just
passed along the request. The issue is that when you do auditctl -s, you
get the number of records lost. If you do it the next day, you have to
do math to see what the one day delta is. So, to make reporting easy,
they want it to be reset whenever they do audictl -s.
You could also make a AUDIT_GET_RESET that gets the status and resets the
number atomically. Then I can add another commandline option to auditctl
that allows an admin to say also reset the counters. If that command
line option is passed, I call AUDIT_GET_RESET otherwise I call
AUDIT_GET. Thought?>
This would be slightly simpler in kernel implementation than the method
I proposed and would work fine, off the top of my head.
I'd prefer not to introduce another command message type for something
small like this.
Steve, do you have any objection to the AUDIT_SET based approach?
Either way, we'd need a feature flag so that I can tell if the kernel supports
this or not.
I think we are okay without a specific feature flag as sending a
AUDIT_SET/reset on an old kernel will be harmless; it won't do
anything, but it shouldn't return an error either.
Ok, so userspace is still left wondering if it worked until the next
time it reads that value, and even then it can't be certain if that
value is the same or higher than it was when userspace thought it reset
that value. At this point, an old kernel will not return an error and
simply ignore any new AUDIT_STATUS_* flag since each flag is treated
independently and extra flags are ignored and not blocked. This seems
sloppy since we have two ways of fixing this uncertainty pretty easily.
Comments like the above aren't helpful, they are just annoying. The
drawbacks to the AUDIT_SET/reset approach have already been discussed
on this thread, if you want to do something constructive think about
how you can resolve these limitations within the context of others
comments/feedback.
I'm sorry to offend. That wasn't my intent. I was trying to bring new
information and observations into the discussion. Is there anything
factually wrong in my observations other than the opinion of it being
sloppy?
Your email wasn't offensive, maybe a little snarky at times, but this
is an open mailing list and developers are nothing if not occasionally
snarky (at least I know I am); that wasn't my objection.

My objection to that first paragraph was that you weren't bringing
anything new to this thread (in my opinion), you were simply
regurgitating the same arguments without any thought of how to solve
these concerns within the context of our ongoing discussion. I tried
to point you in a more constructive direction with my last email, but
perhaps I did a poor job, let me try again ... Opposing viewpoints,
especially when technical matters are concerned, is a good thing, but
ultimately a decision needs to be made if we are to solve our problems
and move forward; it is nice if that decision is the result of
unanimous vote, but it is foolish to expect that, or delay a fix,
waiting for consensus. When you present a possible solution, and the
solution is rejected, in whole or in part, look at the other proposed
solutions: do they resolve all your concerns? If yes, wonderful, work
to embrace that solution. If not, try to resolve your concerns within
the context of this other proposal; don't continue to push something
which has already been rejected. The ultimate goal should be to solve
the problem, not to merge a specific solution; the "best" solution for
the problem is always the one that gets merged.

Hopefully this helps, because I don't know how else to explain this,
and to be quite frank, I'm growing tired of talking about this.
Post by Richard Guy Briggs
Post by Paul Moore
I've already mentioned that I didn't like the AUDIT_GET/reset approach
because I thought the interface was bad. As I'm sure you know, the
audit kernel/userspace interface is a bit of a hot-button topic with
me; I think it has a lot of problems and I'm very intent on not making
it worse (in my opinion, I will admit that API design is not entirely
objective). Continuing to argue for a interface design that I've
already expressed a dislike for is not likely to win me over to your
side; regardless of the outcome you will end up frustrating both
yourself and the maintainer, neither are good things.
I've re-read the thread from the beginning. I guess I must have missed
what was the fundamental problem with the AUDIT_GET/reset method other
than taste. What don't you like about the API that precludes
using/abusing it this way?
A few things come to mind immediately: abusing GET this way
effectively makes it a "write" operation which can make access control
policy difficult, having to bracket the GET with a FEATURE operation
is ugly and potentially racy in its own way, complexity compared to
other solutions.
Post by Richard Guy Briggs
Post by Paul Moore
You feel very strongly that
the window is of grave concern, Steve and myself much less so. If you
still feel strongly about this, think about some different ways in
which you can avoid losing a lost message counter bump. Off the top
of my head, there are really only two ways for the kernel's audit
subsystem to send information back to userspace in this case, via a
netlink return/error message or an audit record. We could possibly do
something with the netlink error message by returning the lost counter
as a positive integer (negative integer is a failure code, zero is
success), but that might get tricky in the future, although we could
mitigate that risk by forcing the AUDIT_SET/reset to happen by itself
(in other words, don't simply check to see if the bit is set in the
bitmask, e.g. (s.mask & AUDIT_STATUS_LOST), check to see it is equal,
e.g. (s.mask == AUDIT_STATUS_LOST)).
This could work. What risk do you see in doing it with other flags?
That another set failure could usurp the return code? If so, yes, I
agree with requiring it to be a lone flag.
Possible conflicts with other SET operations failing as well as
potential future use. We can always relax the restriction in the
future, we can't make it more restrictive.
Post by Richard Guy Briggs
Post by Paul Moore
There you go, two possible solutions for eliminating/mitigating the
potential race while sticking with the simpler AUDIT_SET/reset
interface. I suppose you could even implement both of the solutions
above, they aren't mutually exclusive; that would depend on what
Steve/userspace would prefer. Finally, as for the feature bitmap to
signal to userspace that we support this new feature: if you can't
live without it, go ahead and add it in. As I said before, I'm a
little concerned at the rate we are consuming this bitmap, but I'll
admit we still have plenty of room before we have to start worrying
about alternatives.
I would suggest that the return value (presuming it was reset when
non-zero) or the audit record generated reporting the lost value
reset would be sufficient confirmation that the feature exists on the
running kernel and the addition to the feature bitmap is not strictly
necessary, but you only find this out upon attempting that lost reset.
Well, we haven't used much of that bitmap space and if it isn't to be
used when needed, why is it there? If there is a relatively simple
alternate non-destructive way to discover the presence of a feature use
of the bitmap isn't necessary.
My concern isn't the absolute consumption of the bitmap, but rather
the rate of the consumption.
--
paul moore
www.paul-moore.com
Steve Grubb
2016-12-10 20:40:25 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
I would suggest that the return value (presuming it was reset when
non-zero) or the audit record generated reporting the lost value
reset would be sufficient confirmation that the feature exists on the
running kernel and the addition to the feature bitmap is not strictly
necessary, but you only find this out upon attempting that lost reset.
Well, we haven't used much of that bitmap space and if it isn't to be
used when needed, why is it there? If there is a relatively simple
alternate non-destructive way to discover the presence of a feature use
of the bitmap isn't necessary.
My concern isn't the absolute consumption of the bitmap, but rather
the rate of the consumption.
I'm not concerned much about it. There are very few more RFE's that are either
in the pipeline or something I can think of that we need.

-Steve
Paul Moore
2016-12-12 20:53:52 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
I would suggest that the return value (presuming it was reset when
non-zero) or the audit record generated reporting the lost value
reset would be sufficient confirmation that the feature exists on the
running kernel and the addition to the feature bitmap is not strictly
necessary, but you only find this out upon attempting that lost reset.
Well, we haven't used much of that bitmap space and if it isn't to be
used when needed, why is it there? If there is a relatively simple
alternate non-destructive way to discover the presence of a feature use
of the bitmap isn't necessary.
My concern isn't the absolute consumption of the bitmap, but rather
the rate of the consumption.
I'm not concerned much about it. There are very few more RFE's that are either
in the pipeline or something I can think of that we need.
We always need to plan on more features, regardless of what you know
about today, something (many somethings actually) is almost certain to
come up in the future.
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2016-12-07 15:55:18 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_GET message will get the current audit_lost value and reset the
counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
feature is set.
If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
feature bitmap, the feature is settable by setting the
AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
AUDIT_SET_FEATURE call. This setting is lockable.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
it is possible to read all the entries from audit_feature_names from
userspace.
---
include/uapi/linux/audit.h | 7 +++++--
kernel/audit.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
Instead of resetting the lost counter on an AUDIT_GET if the reset
feature is set, how about preserving the AUDIT_GET behavior, skipping
the AUDIT_FEATURE_* addition, and simply reset the lost value by
sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
to add this to the uapi header).
I realized as I was coding it up that we would potentially lose an
accurate count if the read and reset were not atomic. This was the
reason for using atomic_xchg().
Well, okay, but that isn't what I was talking about ... ? The
audit_cmd_mutex is held for both AUDIT_GET and AUDIT_SET so we should
never process these requests at the same time.
I guess I still don't follow what you are talking about... I hadn't
thought of including both an AUDIT_GET and an AUDIT_SET in the same skb,
but that would ensure that no other command reads or resets the lost
value. However, the lost value could still be incrementing on another
CPU between these two commands, losing an accurate lost count.
Okay, back up ... this whole mess about atomic_xchg() was always
unrelated to my original suggestion, let's focus on my original
comment ... don't reset the counter on a AUDIT_GET, reset it on a
AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
I understood that. It sounds like a nice simple and straightforward
method to do it but for the question of accuracy. Please rewind to my
fundamental point: How do we get an accurate reading of the last value
of audit_lost before resetting it?
Okay, I thought you were worried about a different race, which is why
this discussion wasn't making much sense to me. I understand your
point, but I really dislike the API; although that's not your fault,
it's really the only way to do it via AUDIT_GET.
I'd much prefer we go with the cleaner AUDIT_SET approach and just not
worry about the small race window. It would only be an issue if you
reset the count under heavy audit load, and why would you reset the
lost value if you were under a heavy audit load? That just doesn't
make sense.
I agree the AUDIT_SET approach is much more elegant, which is the
original way I had envisioned doing it, but it is lossy, and I'd lean
towards accuracy and reliability than uncertainty and doubt.
Post by Paul Moore
I suppose we should hear from Steve on this since he was the one who
has been asking for this feature, although I'm pretty sure I know what
he is going to say.
--
paul moore
www.paul-moore.com
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Richard Guy Briggs
2016-12-16 06:59:25 UTC
Permalink
Add a method to reset the audit_lost value.

An AUDIT_SET message with the AUDIT_STATUS_LOST flag set by itself
will return a positive value repesenting the current audit_lost value
and reset the counter to zero. If AUDIT_STATUS_LOST is not the
only flag set, the reset command will be ignored. The value sent with
the command is ignored.

An AUDIT_LOST_RESET message will be queued to the listening audit
daemon. The message will be similar to a CONFIG_CHANGE message with the
fields "lost=0" and "old=" containing the value of audit_lost at reset
ending with a successful result code.

See: https://github.com/linux-audit/audit-kernel/issues/3

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
v3: Switch, from returing a message to the initiating process, to
queueing the message for the audit log.

v2: Switch from AUDIT_GET to AUDIT_SET, adding a +ve return code and
sending a dedicated AUDIT_LOST_RESET message.
---
include/uapi/linux/audit.h | 2 ++
kernel/audit.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..6d38bff 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -70,6 +70,7 @@
#define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off */
#define AUDIT_GET_FEATURE 1019 /* Get which features are enabled */
+#define AUDIT_LOST_RESET 1020 /* Reset the audit_lost value */

#define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly uninteresting to kernel */
#define AUDIT_USER_AVC 1107 /* We filter this differently */
@@ -325,6 +326,7 @@ enum {
#define AUDIT_STATUS_RATE_LIMIT 0x0008
#define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
+#define AUDIT_STATUS_LOST 0x0040

#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..441e8c0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);

/* The netlink socket. */
static struct sock *audit_sock;
@@ -920,6 +920,20 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
if (err < 0)
return err;
}
+ if (s.mask == AUDIT_STATUS_LOST) {
+ struct audit_buffer *ab;
+ u32 lost = atomic_xchg(&audit_lost, 0);
+
+ ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOST_RESET);
+ if (unlikely(!ab))
+ return lost;
+ audit_log_format(ab, "lost=0 old=%u", lost);
+ audit_log_session_info(ab);
+ audit_log_task_context(ab);
+ audit_log_format(ab, " res=1");
+ audit_log_end(ab);
+ return lost;
+ }
break;
}
case AUDIT_GET_FEATURE:
--
1.7.1
Paul Moore
2016-12-16 22:58:39 UTC
Permalink
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_SET message with the AUDIT_STATUS_LOST flag set by itself
will return a positive value repesenting the current audit_lost value
and reset the counter to zero. If AUDIT_STATUS_LOST is not the
only flag set, the reset command will be ignored. The value sent with
the command is ignored.
An AUDIT_LOST_RESET message will be queued to the listening audit
daemon. The message will be similar to a CONFIG_CHANGE message with the
fields "lost=0" and "old=" containing the value of audit_lost at reset
ending with a successful result code.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
v3: Switch, from returing a message to the initiating process, to
queueing the message for the audit log.
v2: Switch from AUDIT_GET to AUDIT_SET, adding a +ve return code and
sending a dedicated AUDIT_LOST_RESET message.
---
include/uapi/linux/audit.h | 2 ++
kernel/audit.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..6d38bff 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -70,6 +70,7 @@
#define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off */
#define AUDIT_GET_FEATURE 1019 /* Get which features are enabled */
+#define AUDIT_LOST_RESET 1020 /* Reset the audit_lost value */
#define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly uninteresting to kernel */
#define AUDIT_USER_AVC 1107 /* We filter this differently */
@@ -325,6 +326,7 @@ enum {
#define AUDIT_STATUS_RATE_LIMIT 0x0008
#define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
+#define AUDIT_STATUS_LOST 0x0040
#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..441e8c0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -920,6 +920,20 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
if (err < 0)
return err;
}
+ if (s.mask == AUDIT_STATUS_LOST) {
+ struct audit_buffer *ab;
+ u32 lost = atomic_xchg(&audit_lost, 0);
+
+ ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOST_RESET);
+ if (unlikely(!ab))
+ return lost;
I'm generally not a fan of adding the likely/unlikely optimizations in
non-critial/control path code like this one, but don't respin the
patch just for this. However, if you do have to respin the patch
please fix this.
Post by Richard Guy Briggs
+ audit_log_format(ab, "lost=0 old=%u", lost);
+ audit_log_session_info(ab);
+ audit_log_task_context(ab);
+ audit_log_format(ab, " res=1");
We're still need to userspace code, so no rush on this, but we should
get Steve's opinion on the format; he'll surely have something to say.
Post by Richard Guy Briggs
+ audit_log_end(ab);
+ return lost;
+ }
break;
}
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
Steve Grubb
2017-01-11 18:56:46 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_SET message with the AUDIT_STATUS_LOST flag set by itself
will return a positive value repesenting the current audit_lost value
and reset the counter to zero. If AUDIT_STATUS_LOST is not the
only flag set, the reset command will be ignored. The value sent with
the command is ignored.
An AUDIT_LOST_RESET message will be queued to the listening audit
daemon. The message will be similar to a CONFIG_CHANGE message with the
fields "lost=0" and "old=" containing the value of audit_lost at reset
ending with a successful result code.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
v3: Switch, from returing a message to the initiating process, to
queueing the message for the audit log.
v2: Switch from AUDIT_GET to AUDIT_SET, adding a +ve return code and
sending a dedicated AUDIT_LOST_RESET message.
---
include/uapi/linux/audit.h | 2 ++
kernel/audit.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..6d38bff 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -70,6 +70,7 @@
#define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off
*/ #define AUDIT_GET_FEATURE 1019 /* Get which features are
enabled */>
+#define AUDIT_LOST_RESET 1020 /* Reset the audit_lost value */
The 1000 block is for command and control, not logging. If this is used in
logging, it should be in the 1300 block. But see below, this probably is not
needed.
Post by Paul Moore
Post by Richard Guy Briggs
#define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly
uninteresting to kernel */ #define AUDIT_USER_AVC 1107 /* We
filter this differently */>
@@ -325,6 +326,7 @@ enum {
#define AUDIT_STATUS_RATE_LIMIT 0x0008
#define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
+#define AUDIT_STATUS_LOST 0x0040
#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..441e8c0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -920,6 +920,20 @@ static int audit_receive_msg(struct sk_buff *skb,
struct nlmsghdr *nlh)>
if (err < 0)
return err;
}
+ if (s.mask == AUDIT_STATUS_LOST) {
+ struct audit_buffer *ab;
+ u32 lost = atomic_xchg(&audit_lost, 0);
+
+ ab = audit_log_start(NULL, GFP_KERNEL,
AUDIT_LOST_RESET); + if (unlikely(!ab))
+ return lost;
I'm generally not a fan of adding the likely/unlikely optimizations in
non-critial/control path code like this one, but don't respin the
patch just for this. However, if you do have to respin the patch
please fix this.
Post by Richard Guy Briggs
+ audit_log_format(ab, "lost=0 old=%u", lost);
+ audit_log_session_info(ab);
+ audit_log_task_context(ab);
+ audit_log_format(ab, " res=1");
We're still need to userspace code, so no rush on this, but we should
get Steve's opinion on the format; he'll surely have something to say.
So, I'm looking at this and wondering a few things. The config option right
above this is audit_set_backlog_wait_time. Wouldn't you want to pattern this
after it? IOW, make an audit_reset_lost function which calls
audit_do_config_change() which calls audit_log_config_change()? This would make
the event type CONFIG_CHANGE instead of LOST_RESET. Since no one is counting
on this event at the moment, no one has software that would try to interpret
LOST_RESET events so we can change it.

I'm thinking its probably more important to be consistent than creating a new
event type. I admit, I didn't follow this whole thread in detail and maybe
there was a good reason to separate out LOST_RESET. By using
audit_do_config_change() you also become consistent with other rules like if
config changes are disallowed because we are immutable.

These changes are on the logging side. This won't affect integration with
auditctl. If you do want to keep LOST_RESET, then it affects all searching and
reporting utilities.

-Steve
Post by Paul Moore
Post by Richard Guy Briggs
+ audit_log_end(ab);
+ return lost;
+ }
break;
}
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
Steve Grubb
2017-01-11 23:35:43 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_SET message with the AUDIT_STATUS_LOST flag set by itself
will return a positive value repesenting the current audit_lost value
and reset the counter to zero. If AUDIT_STATUS_LOST is not the
only flag set, the reset command will be ignored. The value sent with
the command is ignored.
An AUDIT_LOST_RESET message will be queued to the listening audit
daemon. The message will be similar to a CONFIG_CHANGE message with the
fields "lost=0" and "old=" containing the value of audit_lost at reset
ending with a successful result code.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
v3: Switch, from returing a message to the initiating process, to
queueing the message for the audit log.
v2: Switch from AUDIT_GET to AUDIT_SET, adding a +ve return code and
sending a dedicated AUDIT_LOST_RESET message.
---
include/uapi/linux/audit.h | 2 ++
kernel/audit.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..6d38bff 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -70,6 +70,7 @@
#define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off
*/ #define AUDIT_GET_FEATURE 1019 /* Get which features are
enabled */>
+#define AUDIT_LOST_RESET 1020 /* Reset the audit_lost value */
The 1000 block is for command and control, not logging. If this is used in
logging, it should be in the 1300 block. But see below, this probably is not
needed.
Post by Paul Moore
Post by Richard Guy Briggs
#define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly
uninteresting to kernel */ #define AUDIT_USER_AVC 1107 /* We
filter this differently */>
@@ -325,6 +326,7 @@ enum {
#define AUDIT_STATUS_RATE_LIMIT 0x0008
#define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
+#define AUDIT_STATUS_LOST 0x0040
#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..441e8c0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -920,6 +920,20 @@ static int audit_receive_msg(struct sk_buff *skb,
struct nlmsghdr *nlh)>
if (err < 0)
return err;
}
+ if (s.mask == AUDIT_STATUS_LOST) {
+ struct audit_buffer *ab;
+ u32 lost = atomic_xchg(&audit_lost, 0);
+
+ ab = audit_log_start(NULL, GFP_KERNEL,
AUDIT_LOST_RESET); + if (unlikely(!ab))
+ return lost;
I'm generally not a fan of adding the likely/unlikely optimizations in
non-critial/control path code like this one, but don't respin the
patch just for this. However, if you do have to respin the patch
please fix this.
Post by Richard Guy Briggs
+ audit_log_format(ab, "lost=0 old=%u", lost);
+ audit_log_session_info(ab);
+ audit_log_task_context(ab);
+ audit_log_format(ab, " res=1");
We're still need to userspace code, so no rush on this, but we should
get Steve's opinion on the format; he'll surely have something to say.
So, I'm looking at this and wondering a few things. The config option right
above this is audit_set_backlog_wait_time. Wouldn't you want to pattern this
after it? IOW, make an audit_reset_lost function which calls
audit_do_config_change() which calls audit_log_config_change()? This would
make the event type CONFIG_CHANGE instead of LOST_RESET. Since no one is
counting on this event at the moment, no one has software that would try to
interpret LOST_RESET events so we can change it.
I'm thinking its probably more important to be consistent than creating a
new event type. I admit, I didn't follow this whole thread in detail and
maybe there was a good reason to separate out LOST_RESET. By using
audit_do_config_change() you also become consistent with other rules like
if config changes are disallowed because we are immutable.
These changes are on the logging side. This won't affect integration with
auditctl. If you do want to keep LOST_RESET, then it affects all searching
and reporting utilities.
OK. the code to support this is in svn. However, since we didn't use a feature
bit like we normally do, there is absolutely no way to report that the
underlying kernel does not support this. It quietly fails and pretends
everything is fine. I'd prefer that we had a feature bit to output a proper
error message.

-Steve
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
+ audit_log_end(ab);
+ return lost;
+ }
break;
}
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
Richard Guy Briggs
2017-01-12 04:19:42 UTC
Permalink
Post by Steve Grubb
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_SET message with the AUDIT_STATUS_LOST flag set by itself
will return a positive value repesenting the current audit_lost value
and reset the counter to zero. If AUDIT_STATUS_LOST is not the
only flag set, the reset command will be ignored. The value sent with
the command is ignored.
An AUDIT_LOST_RESET message will be queued to the listening audit
daemon. The message will be similar to a CONFIG_CHANGE message with the
fields "lost=0" and "old=" containing the value of audit_lost at reset
ending with a successful result code.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
v3: Switch, from returing a message to the initiating process, to
queueing the message for the audit log.
v2: Switch from AUDIT_GET to AUDIT_SET, adding a +ve return code and
sending a dedicated AUDIT_LOST_RESET message.
---
include/uapi/linux/audit.h | 2 ++
kernel/audit.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..6d38bff 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -70,6 +70,7 @@
#define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off
*/ #define AUDIT_GET_FEATURE 1019 /* Get which features are
enabled */>
+#define AUDIT_LOST_RESET 1020 /* Reset the audit_lost value */
The 1000 block is for command and control, not logging. If this is used in
logging, it should be in the 1300 block. But see below, this probably is not
needed.
Post by Paul Moore
Post by Richard Guy Briggs
#define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly
uninteresting to kernel */ #define AUDIT_USER_AVC 1107 /* We
filter this differently */>
@@ -325,6 +326,7 @@ enum {
#define AUDIT_STATUS_RATE_LIMIT 0x0008
#define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
+#define AUDIT_STATUS_LOST 0x0040
#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..441e8c0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -920,6 +920,20 @@ static int audit_receive_msg(struct sk_buff *skb,
struct nlmsghdr *nlh)>
if (err < 0)
return err;
}
+ if (s.mask == AUDIT_STATUS_LOST) {
+ struct audit_buffer *ab;
+ u32 lost = atomic_xchg(&audit_lost, 0);
+
+ ab = audit_log_start(NULL, GFP_KERNEL,
AUDIT_LOST_RESET); + if (unlikely(!ab))
+ return lost;
I'm generally not a fan of adding the likely/unlikely optimizations in
non-critial/control path code like this one, but don't respin the
patch just for this. However, if you do have to respin the patch
please fix this.
Post by Richard Guy Briggs
+ audit_log_format(ab, "lost=0 old=%u", lost);
+ audit_log_session_info(ab);
+ audit_log_task_context(ab);
+ audit_log_format(ab, " res=1");
We're still need to userspace code, so no rush on this, but we should
get Steve's opinion on the format; he'll surely have something to say.
So, I'm looking at this and wondering a few things. The config option right
above this is audit_set_backlog_wait_time. Wouldn't you want to pattern this
after it? IOW, make an audit_reset_lost function which calls
audit_do_config_change() which calls audit_log_config_change()? This would
make the event type CONFIG_CHANGE instead of LOST_RESET. Since no one is
counting on this event at the moment, no one has software that would try to
interpret LOST_RESET events so we can change it.
I'm thinking its probably more important to be consistent than creating a
new event type. I admit, I didn't follow this whole thread in detail and
maybe there was a good reason to separate out LOST_RESET. By using
audit_do_config_change() you also become consistent with other rules like
if config changes are disallowed because we are immutable.
These changes are on the logging side. This won't affect integration with
auditctl. If you do want to keep LOST_RESET, then it affects all searching
and reporting utilities.
OK. the code to support this is in svn. However, since we didn't use a feature
bit like we normally do, there is absolutely no way to report that the
underlying kernel does not support this. It quietly fails and pretends
everything is fine. I'd prefer that we had a feature bit to output a proper
error message.
Do you still want to switch to CONFIG_CHANGE? (I think that is a good
idea.)

I agree detecting this feature is a destructive operation requiring an
existing lost count and checking the positive return code, but not
impossible, and would prefer a feature bit.

As for audit being immutable, I could see an argument to have this
feature usable even though the config is locked. What's your take?
Post by Steve Grubb
-Steve
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
+ audit_log_end(ab);
+ return lost;
+ }
break;
}
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Steve Grubb
2017-01-12 14:58:56 UTC
Permalink
Post by Richard Guy Briggs
Post by Steve Grubb
OK. the code to support this is in svn. However, since we didn't use a
feature bit like we normally do, there is absolutely no way to report
that the underlying kernel does not support this. It quietly fails and
pretends everything is fine. I'd prefer that we had a feature bit to
output a proper error message.
Do you still want to switch to CONFIG_CHANGE? (I think that is a good
idea.)
Sure.
Post by Richard Guy Briggs
I agree detecting this feature is a destructive operation requiring an
existing lost count and checking the positive return code, but not
impossible, and would prefer a feature bit.
I'd prefer a feature bit so that I can tell people your kernel doesn't support
this. Audit runs on a large variety of kernels.
Post by Richard Guy Briggs
As for audit being immutable, I could see an argument to have this
feature usable even though the config is locked. What's your take?
I can see value in resetting the count even when immutable. Perhaps just use
its logging function. So we don't have a new record type.

-Steve

Richard Guy Briggs
2017-01-12 04:12:47 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Add a method to reset the audit_lost value.
An AUDIT_SET message with the AUDIT_STATUS_LOST flag set by itself
will return a positive value repesenting the current audit_lost value
and reset the counter to zero. If AUDIT_STATUS_LOST is not the
only flag set, the reset command will be ignored. The value sent with
the command is ignored.
An AUDIT_LOST_RESET message will be queued to the listening audit
daemon. The message will be similar to a CONFIG_CHANGE message with the
fields "lost=0" and "old=" containing the value of audit_lost at reset
ending with a successful result code.
See: https://github.com/linux-audit/audit-kernel/issues/3
---
v3: Switch, from returing a message to the initiating process, to
queueing the message for the audit log.
v2: Switch from AUDIT_GET to AUDIT_SET, adding a +ve return code and
sending a dedicated AUDIT_LOST_RESET message.
---
include/uapi/linux/audit.h | 2 ++
kernel/audit.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 208df7b..6d38bff 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -70,6 +70,7 @@
#define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
#define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off
*/ #define AUDIT_GET_FEATURE 1019 /* Get which features are
enabled */>
+#define AUDIT_LOST_RESET 1020 /* Reset the audit_lost value */
The 1000 block is for command and control, not logging. If this is used in
logging, it should be in the 1300 block. But see below, this probably is not
needed.
Fair enough. This won't even be needed if we use CONFIG_CHANGE.
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
#define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly
uninteresting to kernel */ #define AUDIT_USER_AVC 1107 /* We
filter this differently */>
@@ -325,6 +326,7 @@ enum {
#define AUDIT_STATUS_RATE_LIMIT 0x0008
#define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
#define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
+#define AUDIT_STATUS_LOST 0x0040
#define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001
#define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..441e8c0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -122,7 +122,7 @@
3) suppressed due to audit_rate_limit
4) suppressed due to audit_backlog_limit
*/
-static atomic_t audit_lost = ATOMIC_INIT(0);
+static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
@@ -920,6 +920,20 @@ static int audit_receive_msg(struct sk_buff *skb,
struct nlmsghdr *nlh)>
if (err < 0)
return err;
}
+ if (s.mask == AUDIT_STATUS_LOST) {
+ struct audit_buffer *ab;
+ u32 lost = atomic_xchg(&audit_lost, 0);
+
+ ab = audit_log_start(NULL, GFP_KERNEL,
AUDIT_LOST_RESET); + if (unlikely(!ab))
+ return lost;
I'm generally not a fan of adding the likely/unlikely optimizations in
non-critial/control path code like this one, but don't respin the
patch just for this. However, if you do have to respin the patch
please fix this.
Post by Richard Guy Briggs
+ audit_log_format(ab, "lost=0 old=%u", lost);
+ audit_log_session_info(ab);
+ audit_log_task_context(ab);
+ audit_log_format(ab, " res=1");
We're still need to userspace code, so no rush on this, but we should
get Steve's opinion on the format; he'll surely have something to say.
So, I'm looking at this and wondering a few things. The config option right
above this is audit_set_backlog_wait_time. Wouldn't you want to pattern this
after it? IOW, make an audit_reset_lost function which calls
audit_do_config_change() which calls audit_log_config_change()? This would make
the event type CONFIG_CHANGE instead of LOST_RESET. Since no one is counting
on this event at the moment, no one has software that would try to interpret
LOST_RESET events so we can change it.
It isn't upstream yet so now is the time to get it right.

It does in many ways fit into CONFIG_CHANGE. It was suggested by Paul
Moore that it use a message type similar to CONFIG_CHANGE. It isn't
strictly a config change. Do we want it treated like a config change in
that the operation is prevented if audit_enabled == AUDIT_LOCKED? This
does seem reasonable. We could switch it seperately from config
immutable mode by using the set feature facility (not to be confused
with the feature bitmap).
Post by Steve Grubb
I'm thinking its probably more important to be consistent than creating a new
event type. I admit, I didn't follow this whole thread in detail and maybe
there was a good reason to separate out LOST_RESET. By using
audit_do_config_change() you also become consistent with other rules like if
config changes are disallowed because we are immutable.
The thread wasn't *that* long...

Slotting it in to CONFIG_CHANGE does make sense to me.
Post by Steve Grubb
These changes are on the logging side. This won't affect integration with
auditctl. If you do want to keep LOST_RESET, then it affects all searching and
reporting utilities.
Can you define "on the logging side" and what implications that has?

Do you not want to be able to trigger this from auditctl? I agree
putting this in CONFIG_CHANGE will likely make your job easier. There
are some minor differences including checking that the feature exists
either by verifying that the operation succeeded the first time you try
it or by using the feature bitmap or set feature and actually using the
positive return code lost value. There is also the question of how to
respond when it isn't the only flag set in the AUDIT_SET command.
Silently exit having executed the other flags? Return an error before
processing any of the commands? The latter makes more sense to me.
Post by Steve Grubb
From a search and reporting perspective CONFIG_CHANGE will make it much
easier.
Post by Steve Grubb
-Steve
Post by Paul Moore
Post by Richard Guy Briggs
+ audit_log_end(ab);
+ return lost;
+ }
break;
}
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB

--
Richard Guy Briggs <***@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Steve Grubb
2017-01-12 14:56:55 UTC
Permalink
Post by Richard Guy Briggs
Slotting it in to CONFIG_CHANGE does make sense to me.
Post by Steve Grubb
These changes are on the logging side. This won't affect integration with
auditctl. If you do want to keep LOST_RESET, then it affects all searching
and reporting utilities.
Can you define "on the logging side" and what implications that has?
There's 2 parts to this. Resolving the set command and resetting the count and
logging that this was done. What I'm saying is that the AUDIT_STATUS_LOST gets
me into that block of code so auditctl is all set - except for not being able
to tell if it should even try because the underlying kernel doesn't support
this.
Post by Richard Guy Briggs
Do you not want to be able to trigger this from auditctl?
I can. Svn code already does this. The only issue is reporting failure and
logging what happened.
Post by Richard Guy Briggs
I agree
putting this in CONFIG_CHANGE will likely make your job easier. There
are some minor differences including checking that the feature exists
either by verifying that the operation succeeded the first time you try
it or by using the feature bitmap or set feature and actually using the
positive return code lost value. There is also the question of how to
respond when it isn't the only flag set in the AUDIT_SET command.
Just like it is is just fine. Auditctl does not send multiple commands because
there's no way to express that from the rules or command line.
Post by Richard Guy Briggs
Silently exit having executed the other flags? Return an error before
processing any of the commands? The latter makes more sense to me.
From a search and reporting perspective CONFIG_CHANGE will make it much
easier.
Just call audit_log_config_change() from the AUDIT_STATUS_LOST section.

-Steve
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Richard Guy Briggs
+ audit_log_end(ab);
+ return lost;
+ }
break;
}
--
1.7.1
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
Continue reading on narkive:
Loading...