10:28 a.m.
On 10/24/05, Steve Grubb <sgrubb(a)redhat.com> wrote:
On Friday 21 October 2005 19:24, Dustin Kirkland wrote:
> This patch defines the bitmask values of each of the 6 comparators (and
> includes a nice documentation chart explaning how they were chosen).
We need to go ahead and take the next 2 upper bits in the same patch and save
those for future use. For now, if those bits are set, the kernel should
reject the rule. To support this, we also need some code added to
audit_add_rule to check that the operators is something the kernel
understands.
I'm curious why we should seize these bits now? Why the magic number
'2'? Isn't it implied that the next N upper bits will be used for the
next agreed-upon useful purpose? What is the point in rejecting the
rule if those bits are set? The user should understand that if they're
running a kernel that doesn't understand the operations being passed to
it, it won't be able to do anything about it. What about the next two
bits beyond that? What if those are set? Do we just ignore them? For
that matter should we just split the u32 in half and reserve the upper
16 for special flags, and leave the lower 16 for constant enumerations?
I'm not against this, Steve. I'm just trying to understand the goal.
> I didn't add audit_comparator() to audit.h... Should I?
Might this be
> used elsewhere in the audit system?
Not unless you use it somewhere. Keep it local until the need arise to prevent
name collisions.
Thanks, I was unclear on this.
> diff -urpbBN linux-2.6.14-rc4/kernel/auditsc.c
> linux-2.6.14-rc4-audit_ops/kernel/auditsc.c ---
> linux-2.6.14-rc4/kernel/auditsc.c2005-10-19 09:40:29.000000000 -0500 +++
> linux-2.6.14-rc4-audit_ops/kernel/auditsc.c2005-10-21 18:08:32.000000000
> -0500 @@ -385,6 +385,36 @@ int audit_receive_filter(int type, int p
> return err;
> }
>
> +static int audit_comparator(const u32 left, const u32 operator, const u32
> right) +{
<snip>
> +if ( operator & AUDIT_NEGATE )
> +return !rc;
> +else
> +return rc;
> +}
Does this make sense? What does !< mean? I think AUDIT_NEGATE only makes sense
in relation to AUDIT_EQUAL. It should be moved to that case if not eliminated
outright.
Actually, I think the use of AUDIT_NEGATE should be deprecated all
together. As of this patch, we have two separate operators, one for "="
and one for "!=". AUDIT_NEGATE should never be used in future versions
of audit userspace.
On the other hand, we have to remain backward compatible, in the case
that the user has this patched kernel, but older audit userspace. In
that case, there was an implicit "=", and the AUDIT_NEGATE bit simply
negated the implied "=" when on.
Here's the reasoning I used for the code I previously submitted...
There is no longer an implicit "=", but instead a total of 6 operators
(including "=" and "!=" as separate operations).
I couldn't commandeer the existing AUDIT_NEGATE bit, so how best should
I deal with it logically and remain backward compatible?
It made the most sense to me to have the AUDIT_NEGATE bit simply negate
whatever the rest of the logic determined. In the one case where the
operation is "=", this was make it "!=", which preserves previous
functionality. In the rest of the cases, the table would read something
like this:
~(=) is !=
~(!=) is =
~(>) is <=
~(<) is >=
~(>=) is <
~(<=) is >
Which isn't terribly useful and in fact more confusing if you wrote code
in audit userspace to represent operators in such a way... But
logically, it is valid.
So we have to support AUDIT_NEGATE in so much as to maintain new kernel
compatibility with older audit userspace, though we can decree that the
AUDIT_NEGATE flag is not to be used anymore.
And as for interpreting AUDIT_NEGATE within the kernel, I think as long
as it's doing so in a logically valid manner and maintaining said
backward compatibility, the code should be acceptable.
:-Dustin
11:37 a.m.
New subject: [PATCH] Audit filter rule operators (2/2)
On Monday 24 October 2005 11:28, Dustin Kirkland wrote:
I'm curious why we should seize these bits now?
So that future kernels can detect that the rule is trying to use an operator
that it doesn't understand and send an error back to the user. For example,
someone my be using kernel 2.6.25 with audit 3.0.
Why the magic number '2'?
I don't think we should hog the bits. If we see we are using those up, we
should grab the next 2 in reserve and issue errors if they are non-zero.
Isn't it implied that the next N upper bits will be used for the
next agreed-upon useful purpose?
I really don't care what they are used for, but we need ways for older kernels
to reject commands from newer user space tools.
What is the point in rejecting the rule if those bits are set? The
user
should understand that if they're running a kernel that doesn't
understand
the operations being passed to it, it won't be able to do
anything about
it.
Right. But how does the user find out that the kernel can't do what was asked
of it? We need a message to come out.
What about the next two bits beyond that? What if those are set? Do
we
just ignore them?
I really think that we should be looking at all the undefined bits and
checking that they are 0.
For that matter should we just split the u32 in half and reserve the
upper
16 for special flags, and leave the lower 16 for constant enumerations?
No need to go that far. I think we should be checking all unused bits for 0 as
a start.
I couldn't commandeer the existing AUDIT_NEGATE bit, so how best
should
I deal with it logically and remain backward compatible?
Put negate under EQUAL, maybe. You will want to test your new kernel with
patched and unpatched auditctl.
It made the most sense to me to have the AUDIT_NEGATE bit simply
negate
whatever the rest of the logic determined.
To me this doesn't make sense and adds one more "if" statement in the common
path. We need this to be lean and mean.
~(=) is !=
~(!=) is =
~(>) is <=
~(<) is >=
~(>=) is <
~(<=) is >
Which isn't terribly useful and in fact more confusing if you wrote code
in audit userspace to represent operators in such a way... But
logically, it is valid.
Let's dump this. I don't think the user space tools will ever send something
like that into the kernel, so don't support it.
And as for interpreting AUDIT_NEGATE within the kernel, I think as
long
as it's doing so in a logically valid manner and maintaining said
backward compatibility, the code should be acceptable.
Right. Run the LTP stuff with new kernel and both versions of auditctl. If
that passes, you are done.
Thanks,
-Steve
6998
days inactive
6998
days old
linux-audit@lists.linux-audit.osci.io
1 comments
2 participants
participants (2)
-
Dustin Kirkland -
Steve Grubb