[PATCH] Audit filter rule operators (2/2)
Amy Griffis
amy.griffis at hp.com
Tue Oct 25 17:16:15 UTC 2005
Hi Dustin,
Just a few comments below...
On Fri, Oct 21, 2005 at 06:24:31PM -0500, Dustin Kirkland wrote:
> diff -urpbBN linux-2.6.14-rc4/include/linux/audit.h
> linux-2.6.14-rc4-audit_ops/include/linux/audit.h
> --- linux-2.6.14-rc4/include/linux/audit.h 2005-10-19 09:40:27.000000000 -0500
> +++ linux-2.6.14-rc4-audit_ops/include/linux/audit.h 2005-10-21 18:05:43.000000000 -0500
> @@ -128,8 +128,29 @@
> #define AUDIT_ARG2 (AUDIT_ARG0+2)
> #define AUDIT_ARG3 (AUDIT_ARG0+3)
>
> -#define AUDIT_NEGATE 0x80000000
> +/* These are the supported operators.
> + 4 2 1
> + > < =
> + -------
> + 0 0 0 0 undef
> + 0 0 1 1 =
> + 0 1 0 2 <
> + 0 1 1 3 <=
> + 1 0 0 4 >
> + 1 0 1 5 >=
> + 1 1 0 6 !=
> + 1 1 1 7 range
> + */
> +#define AUDIT_OPERATORS 0xF0000000
> +#define AUDIT_EQUAL 0x10000000
> +#define AUDIT_LESS_THAN 0x20000000
> +#define AUDIT_LESS_THAN_OR_EQUAL 0x30000000
Here's another way to specify this that may be more readable and
common in the kernel:
#define AUDIT_LESS_THAN_OR_EQUAL AUDIT_EQUAL|AUDIT_LESS_THAN
> +#define AUDIT_GREATER_THAN 0x40000000
> +#define AUDIT_GREATER_THAN_OR_EQUAL 0x50000000
> +#define AUDIT_NOT_EQUAL 0x60000000
> +#define AUDIT_RANGE 0x70000000
AUDIT_RANGE isn't needed at all, as you already have an implementation
that can express that.
audit_filter_rules() treats the fields as implicit ANDs, so the
following would express a uid range 10..20
field[0] = AUDIT_UID;
value[0] = 10 | AUDIT_GREATER_THAN_OR_EQUAL;
field[1] = AUDIT_UID;
value[1] = 20 | AUDIT_LESS_THAN_OR_EQUAL;
> +#define AUDIT_NEGATE 0x80000000
>
> /* Status symbols */
> /* Mask values */
> 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.c 2005-10-19 09:40:29.000000000 -0500
> +++ linux-2.6.14-rc4-audit_ops/kernel/auditsc.c 2005-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)
> +{
> + int rc;
> + switch (operator) {
> + case AUDIT_LESS_THAN:
> + rc = (left < right);
> + break;
> + case AUDIT_LESS_THAN_OR_EQUAL:
> + rc = (left <= right);
> + break;
> + case AUDIT_GREATER_THAN:
> + rc = (left > right);
> + break;
> + case AUDIT_GREATER_THAN_OR_EQUAL:
> + rc = (left >= right);
> + break;
> + case AUDIT_NOT_EQUAL:
> + rc = (left != right);
> + break;
> + case AUDIT_EQUAL:
> + default:
> + rc = (left == right);
> + break;
> + }
Or how about this?
int rc = 0;
if ((operator & AUDIT_EQUAL) && (left == right))
rc = 1;
else if ((operator & AUDIT_LESS_THAN) && (left < right))
rc = 1;
else if ((operator & AUDIT_GREATER_THAN) && (left > right))
rc = 1;
return rc;
> + if ( operator & AUDIT_NEGATE )
> + return !rc;
> + else
> + return rc;
I think this may have been alluded to in another email, but we need to
do comaptibility checking on rule insert, not during rule evaluation.
So if AUDIT_NEGATE is set, clear it and set AUDIT_NOT_EQUAL. If no
bits are set, set AUDIT_EQUAL. Doing so eliminates these two branches
in fast-path code.
> +}
> +
> /* Compare a task_struct with an audit_rule. Return 1 on match, 0
> * otherwise. */
> static int audit_filter_rules(struct task_struct *tsk,
> @@ -395,62 +425,63 @@ static int audit_filter_rules(struct tas
> int i, j;
>
> for (i = 0; i < rule->field_count; i++) {
> - u32 field = rule->fields[i] & ~AUDIT_NEGATE;
> + u32 field = rule->fields[i] & ~AUDIT_OPERATORS;
> + u32 op = rule->fields[i] & AUDIT_OPERATORS;
> u32 value = rule->values[i];
> int result = 0;
>
> switch (field) {
> case AUDIT_PID:
> - result = (tsk->pid == value);
> + result = audit_comparator(tsk->pid, op, value);
> break;
> case AUDIT_UID:
> - result = (tsk->uid == value);
> + result = audit_comparator(tsk->uid, op, value);
> break;
> case AUDIT_EUID:
> - result = (tsk->euid == value);
> + result = audit_comparator(tsk->euid, op, value);
> break;
> case AUDIT_SUID:
> - result = (tsk->suid == value);
> + result = audit_comparator(tsk->suid, op, value);
> break;
> case AUDIT_FSUID:
> - result = (tsk->fsuid == value);
> + result = audit_comparator(tsk->fsuid, op, value);
> break;
> case AUDIT_GID:
> - result = (tsk->gid == value);
> + result = audit_comparator(tsk->gid, op, value);
> break;
> case AUDIT_EGID:
> - result = (tsk->egid == value);
> + result = audit_comparator(tsk->egid, op, value);
> break;
> case AUDIT_SGID:
> - result = (tsk->sgid == value);
> + result = audit_comparator(tsk->sgid, op, value);
> break;
> case AUDIT_FSGID:
> - result = (tsk->fsgid == value);
> + result = audit_comparator(tsk->fsgid, op, value);
> break;
> case AUDIT_PERS:
> - result = (tsk->personality == value);
> + result = audit_comparator(tsk->personality, op, value);
Many lines in this function are way over 80 chars -- see
Documentation/CodingStyle Chapter 2.
> break;
> case AUDIT_ARCH:
> if (ctx)
> - result = (ctx->arch == value);
> + result = audit_comparator(ctx->arch, op, value);
> break;
>
> case AUDIT_EXIT:
> if (ctx && ctx->return_valid)
> - result = (ctx->return_code == value);
> + result = audit_comparator(ctx->return_code, op, value);
> break;
> case AUDIT_SUCCESS:
> if (ctx && ctx->return_valid) {
> if (value)
> - result = (ctx->return_valid == AUDITSC_SUCCESS);
> + result = audit_comparator(ctx->return_valid, op, AUDITSC_SUCCESS);
> else
> - result = (ctx->return_valid == AUDITSC_FAILURE);
> + result = audit_comparator(ctx->return_valid, op, AUDITSC_FAILURE);
> }
> break;
> case AUDIT_DEVMAJOR:
> if (ctx) {
> for (j = 0; j < ctx->name_count; j++) {
> - if (MAJOR(ctx->names[j].dev)==value) {
> + if ( audit_comparator(MAJOR(ctx->names[j].dev), op, value) ) {
> ++result;
> break;
> }
> @@ -460,7 +491,7 @@ static int audit_filter_rules(struct tas
> case AUDIT_DEVMINOR:
> if (ctx) {
> for (j = 0; j < ctx->name_count; j++) {
> - if (MINOR(ctx->names[j].dev)==value) {
> + if ( audit_comparator(MINOR(ctx->names[j].dev), op, value) ) {
> ++result;
> break;
> }
> @@ -470,7 +501,7 @@ static int audit_filter_rules(struct tas
> case AUDIT_INODE:
> if (ctx) {
> for (j = 0; j < ctx->name_count; j++) {
> - if (ctx->names[j].ino == value) {
> + if ( audit_comparator(ctx->names[j].ino, op, value) ) {
> ++result;
> break;
> }
> @@ -480,19 +511,17 @@ static int audit_filter_rules(struct tas
> case AUDIT_LOGINUID:
> result = 0;
> if (ctx)
> - result = (ctx->loginuid == value);
> + result = audit_comparator(ctx->loginuid, op, value);
> break;
> case AUDIT_ARG0:
> case AUDIT_ARG1:
> case AUDIT_ARG2:
> case AUDIT_ARG3:
> if (ctx)
> - result = (ctx->argv[field-AUDIT_ARG0]==value);
> + result = audit_comparator(ctx->argv[field-AUDIT_ARG0], op, value);
> break;
> }
>
> - if (rule->fields[i] & AUDIT_NEGATE)
> - result = !result;
> if (!result)
> return 0;
> }
>
>
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
More information about the Linux-audit
mailing list