[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]