9:07 a.m.
A regression was caused by commit 780a7654cee8:
audit: Make testing for a valid loginuid explicit.
(which in turn attempted to fix a regression caused by e1760bd)
When audit_krule_to_data() fills in the rules to get a listing, there was a
missing clause to convert back from AUDIT_LOGINUID_SET to AUDIT_LOGINUID.
This broke userspace by not returning the same information that was sent and
expected.
The rule:
auditctl -a exit,never -F auid=-1
gives:
auditctl -l
LIST_RULES: exit,never f24=0 syscall=all
when it should give:
LIST_RULES: exit,never auid=-1 (0xffffffff) syscall=all
Tag it so that it is reported the same way it was set.
Note: move the field validation call ahead of the mutation code to have it work
on the original field set.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
include/uapi/linux/audit.h | 3 +++
kernel/auditfilter.c | 19 +++++++++++++------
kernel/auditsc.c | 2 +-
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..860df86 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -274,6 +274,9 @@
#define AUDIT_FILTERKEY 210
+/* Flag to indicate legacy AUDIT_LOGINUID unset usage */
+#define AUDIT_LOGINUID_LEGACY 0x80000000
+
#define AUDIT_NEGATE 0x80000000
/* These are the supported operators.
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 40ed981..39ce3e6 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -438,9 +438,13 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data
*data,
f->type = data->fields[i];
f->val = data->values[i];
+ err = audit_field_valid(entry, f);
+ if (err)
+ goto exit_free;
+
/* Support legacy tests for a valid loginuid */
if ((f->type == AUDIT_LOGINUID) && (f->val == AUDIT_UID_UNSET)) {
- f->type = AUDIT_LOGINUID_SET;
+ f->type = AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY;
f->val = 0;
}
@@ -457,10 +461,6 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data
*data,
rcu_read_unlock();
}
- err = audit_field_valid(entry, f);
- if (err)
- goto exit_free;
-
err = -EINVAL;
switch (f->type) {
case AUDIT_LOGINUID:
@@ -630,6 +630,13 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule
*krule)
data->buflen += data->values[i] =
audit_pack_string(&bufp, krule->filterkey);
break;
+ case AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY:
+ if (!f->val) {
+ data->fields[i] = AUDIT_LOGINUID;
+ data->values[i] = AUDIT_UID_UNSET;
+ break;
+ }
+ /* fallthrough if set */
default:
data->values[i] = f->val;
}
@@ -1270,7 +1277,7 @@ static int audit_filter_user_rules(struct audit_krule *rule, int
type,
int result = 0;
u32 sid;
- switch (f->type) {
+ switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
case AUDIT_PID:
pid = task_pid_nr(current);
result = audit_comparator(pid, f->op, f->val);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8933572..ef25cbc 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -452,7 +452,7 @@ static int audit_filter_rules(struct task_struct *tsk,
int result = 0;
pid_t pid;
- switch (f->type) {
+ switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
case AUDIT_PID:
pid = task_pid_nr(tsk);
result = audit_comparator(pid, f->op, f->val);
--
1.7.1
9:20 a.m.
On 14/09/24, Richard Guy Briggs wrote:
A regression was caused by commit 780a7654cee8:
audit: Make testing for a valid loginuid explicit.
(which in turn attempted to fix a regression caused by e1760bd)
Eric (Paris), does tagging this field in the type member with a high
bit work for you? It only has to be filtered out for
audit_filter_user_rules() and audit_filter_rules() to avoid changing the
normal semantics of that field. I'd rather do this than add a new
member to the field struct.
When audit_krule_to_data() fills in the rules to get a listing, there
was a
missing clause to convert back from AUDIT_LOGINUID_SET to AUDIT_LOGINUID.
This broke userspace by not returning the same information that was sent and
expected.
The rule:
auditctl -a exit,never -F auid=-1
gives:
auditctl -l
LIST_RULES: exit,never f24=0 syscall=all
when it should give:
LIST_RULES: exit,never auid=-1 (0xffffffff) syscall=all
Tag it so that it is reported the same way it was set.
Note: move the field validation call ahead of the mutation code to have it work
on the original field set.
I intend to add a Cc: stable 3.10-rc1 here.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
include/uapi/linux/audit.h | 3 +++
kernel/auditfilter.c | 19 +++++++++++++------
kernel/auditsc.c | 2 +-
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..860df86 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -274,6 +274,9 @@
#define AUDIT_FILTERKEY 210
+/* Flag to indicate legacy AUDIT_LOGINUID unset usage */
+#define AUDIT_LOGINUID_LEGACY 0x80000000
+
#define AUDIT_NEGATE 0x80000000
/* These are the supported operators.
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 40ed981..39ce3e6 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -438,9 +438,13 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data,
f->type = data->fields[i];
f->val = data->values[i];
+ err = audit_field_valid(entry, f);
+ if (err)
+ goto exit_free;
+
/* Support legacy tests for a valid loginuid */
if ((f->type == AUDIT_LOGINUID) && (f->val == AUDIT_UID_UNSET)) {
- f->type = AUDIT_LOGINUID_SET;
+ f->type = AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY;
f->val = 0;
}
@@ -457,10 +461,6 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data,
rcu_read_unlock();
}
- err = audit_field_valid(entry, f);
- if (err)
- goto exit_free;
-
err = -EINVAL;
switch (f->type) {
case AUDIT_LOGINUID:
@@ -630,6 +630,13 @@ static struct audit_rule_data *audit_krule_to_data(struct
audit_krule *krule)
data->buflen += data->values[i] =
audit_pack_string(&bufp, krule->filterkey);
break;
+ case AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY:
+ if (!f->val) {
+ data->fields[i] = AUDIT_LOGINUID;
+ data->values[i] = AUDIT_UID_UNSET;
+ break;
+ }
+ /* fallthrough if set */
default:
data->values[i] = f->val;
}
@@ -1270,7 +1277,7 @@ static int audit_filter_user_rules(struct audit_krule *rule, int
type,
int result = 0;
u32 sid;
- switch (f->type) {
+ switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
case AUDIT_PID:
pid = task_pid_nr(current);
result = audit_comparator(pid, f->op, f->val);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8933572..ef25cbc 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -452,7 +452,7 @@ static int audit_filter_rules(struct task_struct *tsk,
int result = 0;
pid_t pid;
- switch (f->type) {
+ switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
case AUDIT_PID:
pid = task_pid_nr(tsk);
result = audit_comparator(pid, f->op, f->val);
--
1.7.1
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
4:06 p.m.
On Wednesday, September 24, 2014 10:07:20 AM Richard Guy Briggs wrote:
A regression was caused by commit 780a7654cee8:
audit: Make testing for a valid loginuid explicit.
(which in turn attempted to fix a regression caused by e1760bd)
When audit_krule_to_data() fills in the rules to get a listing, there was a
missing clause to convert back from AUDIT_LOGINUID_SET to AUDIT_LOGINUID.
This broke userspace by not returning the same information that was sent and
expected.
The rule:
auditctl -a exit,never -F auid=-1
gives:
auditctl -l
LIST_RULES: exit,never f24=0 syscall=all
when it should give:
LIST_RULES: exit,never auid=-1 (0xffffffff) syscall=all
Tag it so that it is reported the same way it was set.
Note: move the field validation call ahead of the mutation code to have it
work on the original field set.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
include/uapi/linux/audit.h | 3 +++
kernel/auditfilter.c | 19 +++++++++++++------
kernel/auditsc.c | 2 +-
3 files changed, 17 insertions(+), 7 deletions(-)
Sorry for the delays, I'm working on cleaning out the audit bug/patch backlog
and it took me a little while to get to this topic ... so, this looks a messy
little problem; Richard, could you help me understand things a bit better?
As I understand it, when old userspace would set a filter with AUDIT_LOGINUID
but when it listed the audit rules in the kernel it would see
AUDIT_LOGINUID_SET, yes? This patch attempts to fix this by marking a legacy
userspace with the AUDIT_LOGINUID_LEGACY bitmask on the internal kernel
representation so that when the rules are dumped to userspace the
AUDIT_LOGINUID_SET rule can be rewritten as AUDIT_LOGINUID, yes?
However, there are some things that are not immediately obvious to me:
* Why are we using a bit in audit_field->type to indicate the legacy nature of
userspace?
* Why are we reusing the AUDIT_NEGATE bit in the type field to indicate a
legacy userspace?
* Why are we not using something in audit_krule? Without looking to in depth
it would appear that there are multiple fields which might be useful, e.g.
"vers_ops", "flags"?
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..860df86 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -274,6 +274,9 @@
#define AUDIT_FILTERKEY 210
+/* Flag to indicate legacy AUDIT_LOGINUID unset usage */
+#define AUDIT_LOGINUID_LEGACY 0x80000000
+
#define AUDIT_NEGATE 0x80000000
/* These are the supported operators.
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 40ed981..39ce3e6 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -438,9 +438,13 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data, f->type = data->fields[i];
f->val = data->values[i];
+ err = audit_field_valid(entry, f);
+ if (err)
+ goto exit_free;
+
/* Support legacy tests for a valid loginuid */
if ((f->type == AUDIT_LOGINUID) && (f->val == AUDIT_UID_UNSET)) {
- f->type = AUDIT_LOGINUID_SET;
+ f->type = AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY;
f->val = 0;
}
@@ -457,10 +461,6 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data, rcu_read_unlock();
}
- err = audit_field_valid(entry, f);
- if (err)
- goto exit_free;
-
err = -EINVAL;
switch (f->type) {
case AUDIT_LOGINUID:
@@ -630,6 +630,13 @@ static struct audit_rule_data
*audit_krule_to_data(struct audit_krule *krule) data->buflen +=
data->values[i] =
audit_pack_string(&bufp, krule->filterkey);
break;
+ case AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY:
+ if (!f->val) {
+ data->fields[i] = AUDIT_LOGINUID;
+ data->values[i] = AUDIT_UID_UNSET;
+ break;
+ }
+ /* fallthrough if set */
default:
data->values[i] = f->val;
}
@@ -1270,7 +1277,7 @@ static int audit_filter_user_rules(struct audit_krule
*rule, int type, int result = 0;
u32 sid;
- switch (f->type) {
+ switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
case AUDIT_PID:
pid = task_pid_nr(current);
result = audit_comparator(pid, f->op, f->val);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8933572..ef25cbc 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -452,7 +452,7 @@ static int audit_filter_rules(struct task_struct *tsk,
int result = 0;
pid_t pid;
- switch (f->type) {
+ switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
case AUDIT_PID:
pid = task_pid_nr(tsk);
result = audit_comparator(pid, f->op, f->val);
--
paul moore
security and virtualization @ redhat
10:30 a.m.
On 14/12/08, Paul Moore wrote:
On Wednesday, September 24, 2014 10:07:20 AM Richard Guy Briggs
wrote:
> A regression was caused by commit 780a7654cee8:
> audit: Make testing for a valid loginuid explicit.
> (which in turn attempted to fix a regression caused by e1760bd)
>
> When audit_krule_to_data() fills in the rules to get a listing, there was a
> missing clause to convert back from AUDIT_LOGINUID_SET to AUDIT_LOGINUID.
>
> This broke userspace by not returning the same information that was sent and
> expected.
>
> The rule:
> auditctl -a exit,never -F auid=-1
> gives:
> auditctl -l
> LIST_RULES: exit,never f24=0 syscall=all
> when it should give:
> LIST_RULES: exit,never auid=-1 (0xffffffff) syscall=all
>
> Tag it so that it is reported the same way it was set.
>
> Note: move the field validation call ahead of the mutation code to have it
> work on the original field set.
>
> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> ---
> include/uapi/linux/audit.h | 3 +++
> kernel/auditfilter.c | 19 +++++++++++++------
> kernel/auditsc.c | 2 +-
> 3 files changed, 17 insertions(+), 7 deletions(-)
Sorry for the delays, I'm working on cleaning out the audit bug/patch backlog
and it took me a little while to get to this topic ... so, this looks a messy
little problem; Richard, could you help me understand things a bit better?
As I understand it, when old userspace would set a filter with AUDIT_LOGINUID
but when it listed the audit rules in the kernel it would see
AUDIT_LOGINUID_SET, yes? This patch attempts to fix this by marking a legacy
userspace with the AUDIT_LOGINUID_LEGACY bitmask on the internal kernel
representation so that when the rules are dumped to userspace the
AUDIT_LOGINUID_SET rule can be rewritten as AUDIT_LOGINUID, yes?
Correct.
However, there are some things that are not immediately obvious to
me:
* Why are we using a bit in audit_field->type to indicate the legacy nature of
userspace?
Convenience. Adding a new member to audit_field or audit_krule seemed
unnecessary memory overhead (however, it then complicates other code...).
* Why are we reusing the AUDIT_NEGATE bit in the type field to
indicate a
legacy userspace?
It wasn't reaped when commit 18900909 went through... (first introduced
with original audit in b7b0074c, 2004-04-11). It would have been more
clear if I had sent a first patch to remove AUDIT_NEGATE altogether and
re-introduce it with a new name in this patch.
* Why are we not using something in audit_krule? Without looking to
in depth
it would appear that there are multiple fields which might be useful, e.g.
"vers_ops", "flags"?
audit_krule applies to the set of all fields for this rule. I wanted
something that localized it very unambiguously to this one field.
> diff --git a/include/uapi/linux/audit.h
b/include/uapi/linux/audit.h
> index 4d100c8..860df86 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -274,6 +274,9 @@
>
> #define AUDIT_FILTERKEY 210
>
> +/* Flag to indicate legacy AUDIT_LOGINUID unset usage */
> +#define AUDIT_LOGINUID_LEGACY 0x80000000
> +
> #define AUDIT_NEGATE 0x80000000
>
> /* These are the supported operators.
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 40ed981..39ce3e6 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -438,9 +438,13 @@ static struct audit_entry *audit_data_to_entry(struct
> audit_rule_data *data, f->type = data->fields[i];
> f->val = data->values[i];
>
> + err = audit_field_valid(entry, f);
> + if (err)
> + goto exit_free;
> +
> /* Support legacy tests for a valid loginuid */
> if ((f->type == AUDIT_LOGINUID) && (f->val == AUDIT_UID_UNSET)) {
> - f->type = AUDIT_LOGINUID_SET;
> + f->type = AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY;
> f->val = 0;
> }
>
> @@ -457,10 +461,6 @@ static struct audit_entry *audit_data_to_entry(struct
> audit_rule_data *data, rcu_read_unlock();
> }
>
> - err = audit_field_valid(entry, f);
> - if (err)
> - goto exit_free;
> -
> err = -EINVAL;
> switch (f->type) {
> case AUDIT_LOGINUID:
> @@ -630,6 +630,13 @@ static struct audit_rule_data
> *audit_krule_to_data(struct audit_krule *krule) data->buflen +=
> data->values[i] =
> audit_pack_string(&bufp, krule->filterkey);
> break;
> + case AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY:
> + if (!f->val) {
> + data->fields[i] = AUDIT_LOGINUID;
> + data->values[i] = AUDIT_UID_UNSET;
> + break;
> + }
> + /* fallthrough if set */
> default:
> data->values[i] = f->val;
> }
> @@ -1270,7 +1277,7 @@ static int audit_filter_user_rules(struct audit_krule
> *rule, int type, int result = 0;
> u32 sid;
>
> - switch (f->type) {
> + switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
> case AUDIT_PID:
> pid = task_pid_nr(current);
> result = audit_comparator(pid, f->op, f->val);
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 8933572..ef25cbc 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -452,7 +452,7 @@ static int audit_filter_rules(struct task_struct *tsk,
> int result = 0;
> pid_t pid;
>
> - switch (f->type) {
> + switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
> case AUDIT_PID:
> pid = task_pid_nr(tsk);
> result = audit_comparator(pid, f->op, f->val);
--
paul moore
security and virtualization @ redhat
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
9:01 p.m.
On Tuesday, December 09, 2014 11:30:14 AM Richard Guy Briggs wrote:
On 14/12/08, Paul Moore wrote:
> As I understand it, when old userspace would set a filter with
> AUDIT_LOGINUID but when it listed the audit rules in the kernel it would
> see AUDIT_LOGINUID_SET, yes? This patch attempts to fix this by marking a
> legacy userspace with the AUDIT_LOGINUID_LEGACY bitmask on the internal
> kernel representation so that when the rules are dumped to userspace the
> AUDIT_LOGINUID_SET rule can be rewritten as AUDIT_LOGINUID, yes?
Correct.
> However, there are some things that are not immediately obvious to me:
>
> * Why are we using a bit in audit_field->type to indicate the legacy
> nature of userspace?
Convenience. Adding a new member to audit_field or audit_krule seemed
unnecessary memory overhead (however, it then complicates other code...).
> * Why are we reusing the AUDIT_NEGATE bit in the type field to indicate a
> legacy userspace?
It wasn't reaped when commit 18900909 went through... (first introduced
with original audit in b7b0074c, 2004-04-11). It would have been more
clear if I had sent a first patch to remove AUDIT_NEGATE altogether and
re-introduce it with a new name in this patch.
The problem is that AUDIT_NEGATE lives in the userspace visible header file
which means it needs to live there for pretty much forever. While I would
like to see us remote it for clarity's sake, I think we're stuck with it.
> * Why are we not using something in audit_krule? Without
looking to in
> depth it would appear that there are multiple fields which might be
> useful, e.g. "vers_ops", "flags"?
audit_krule applies to the set of all fields for this rule. I wanted
something that localized it very unambiguously to this one field.
You can only add or delete rules, right? Not modify? If you can only add or
delete a rule, then if one of the fields in that rule is sent from legacy
userspace I think it is safe to set an indicator in one of the audit_krule
fields. I understand your point, but I'm not sure it is something to worry
too much about; I'd rather see the legacy indicator here than in the
audit_field->type field where we might have to contend with userspace usage at
some point.
I'd like to explore the idea of not using audit_field->type; I picked
"vers_ops" and "flags" since they seemed like reasonable places to
start. The
"vers_ops" field in particular appears to be almost unused in the current code
and it seems like a good way to track userspace versions perhaps, e.g. 1 =
legacy, 2 = now current, etc.? I'm curious if this sounds reasonable to you.
--
paul moore
security and virtualization @ redhat
11:19 p.m.
On 14/12/09, Paul Moore wrote:
On Tuesday, December 09, 2014 11:30:14 AM Richard Guy Briggs wrote:
> On 14/12/08, Paul Moore wrote:
> > As I understand it, when old userspace would set a filter with
> > AUDIT_LOGINUID but when it listed the audit rules in the kernel it would
> > see AUDIT_LOGINUID_SET, yes? This patch attempts to fix this by marking a
> > legacy userspace with the AUDIT_LOGINUID_LEGACY bitmask on the internal
> > kernel representation so that when the rules are dumped to userspace the
> > AUDIT_LOGINUID_SET rule can be rewritten as AUDIT_LOGINUID, yes?
>
> Correct.
>
> > However, there are some things that are not immediately obvious to me:
> >
> > * Why are we using a bit in audit_field->type to indicate the legacy
> > nature of userspace?
>
> Convenience. Adding a new member to audit_field or audit_krule seemed
> unnecessary memory overhead (however, it then complicates other code...).
>
> > * Why are we reusing the AUDIT_NEGATE bit in the type field to indicate a
> > legacy userspace?
>
> It wasn't reaped when commit 18900909 went through... (first introduced
> with original audit in b7b0074c, 2004-04-11). It would have been more
> clear if I had sent a first patch to remove AUDIT_NEGATE altogether and
> re-introduce it with a new name in this patch.
The problem is that AUDIT_NEGATE lives in the userspace visible header file
which means it needs to live there for pretty much forever. While I would
like to see us remote it for clarity's sake, I think we're stuck with it.
Ok, fair enough, the same goes for AUDIT_{LIST,ADD,DEL} since they are
no longer used, but still remain in the API. I can use another value.
> > * Why are we not using something in audit_krule? Without
looking to in
> > depth it would appear that there are multiple fields which might be
> > useful, e.g. "vers_ops", "flags"?
>
> audit_krule applies to the set of all fields for this rule. I wanted
> something that localized it very unambiguously to this one field.
You can only add or delete rules, right? Not modify? If you can only add or
delete a rule, then if one of the fields in that rule is sent from legacy
userspace I think it is safe to set an indicator in one of the audit_krule
fields. I understand your point, but I'm not sure it is something to worry
too much about; I'd rather see the legacy indicator here than in the
audit_field->type field where we might have to contend with userspace usage at
some point.
I'd like to explore the idea of not using audit_field->type; I picked
"vers_ops" and "flags" since they seemed like reasonable places to
start. The
"vers_ops" field in particular appears to be almost unused in the current code
and it seems like a good way to track userspace versions perhaps, e.g. 1 =
legacy, 2 = now current, etc.? I'm curious if this sounds reasonable to you.
vers_ops appears to be assigned, copied and never otherwise read. And
in fact *that* should have been removed with commit 18900909. I'll just
prepare a patch to rip it out.
flags looks like a better choice... and I'll have to do some similar
filtering that I did for type and as has been already done for
AUDIT_FILTER_PREPEND... In fact, the inverse of AUDIT_FILTER_PREPEND.
And this patch is simpler than the one you are critiquing. Bonus.
paul moore
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
3663
days inactive
3742
days old
linux-audit@lists.linux-audit.osci.io
5 comments
2 participants
participants (2)
-
Paul Moore -
Richard Guy Briggs