From: Steve Grubb <sgrubb@redhat.com>
To: Peter Moody <pmoody@google.com>
Cc: linux-audit@redhat.com
Subject: Re: test patch for auditctl inter-field comparisons on euid/uid, egid/gid
Date: Tue, 28 Feb 2012 18:07:33 -0500 [thread overview]
Message-ID: <201202281807.34269.sgrubb@redhat.com> (raw)
In-Reply-To: <CALnj_=4mQ+CAW3ZznxE55Tb_X6G5kzUsQQLkaxA7jkgR8F3Ubw@mail.gmail.com>
On Wednesday, December 14, 2011 08:18:30 PM Peter Moody wrote:
> > Testing has revealed a couple of bugs in this. Fixing these and
> > testing some more now.
>
> Ok, this should be the last one. I was apparently a little more tired
> riding the shuttle home than I thought I was. The patch last night was
> missing a break and a number of the AUDIT_COMPARE_* fields were plain
> wrong.
>
> This one tests good to me though.
This patch + some changes and additions has been applied.
https://fedorahosted.org/audit/changeset/642
If I could go back in time, I would have suggested using a bitmap for left and
right side so we could avoid the big switch/case constructs. But it works fine
the way it is. :)
-Steve
> Signed-off-by: Peter Moody <pmoody@google.com>
> ---
> trunk/auparse/typetab.h | 2 +
> trunk/lib/errormsg.h | 7 +-
> trunk/lib/fieldtab.h | 3 +
> trunk/lib/libaudit.c | 303
> +++++++++++++++++++++++++++++++++++++++++++ trunk/lib/libaudit.h |
> 2 +
> trunk/src/auditctl.c | 19 +++-
> trunk/src/ausearch-report.c | 1 +
> 7 files changed, 334 insertions(+), 3 deletions(-)
>
> diff --git a/trunk/auparse/typetab.h b/trunk/auparse/typetab.h
> index 746573c..cf13f4d 100644
> --- a/trunk/auparse/typetab.h
> +++ b/trunk/auparse/typetab.h
> @@ -32,6 +32,8 @@ _S(AUPARSE_TYPE_UID, "iuid" )
> _S(AUPARSE_TYPE_UID, "id" )
> _S(AUPARSE_TYPE_UID, "inode_uid" )
> _S(AUPARSE_TYPE_UID, "sauid" )
> +_S(AUPARSE_TYPE_UID, "obj_uid" )
> +_S(AUPARSE_TYPE_UID, "obj_gid" )
> _S(AUPARSE_TYPE_GID, "gid" )
> _S(AUPARSE_TYPE_GID, "egid" )
> _S(AUPARSE_TYPE_GID, "sgid" )
> diff --git a/trunk/lib/errormsg.h b/trunk/lib/errormsg.h
> index fd6b1c0..a7bc61d 100644
> --- a/trunk/lib/errormsg.h
> +++ b/trunk/lib/errormsg.h
> @@ -54,5 +54,10 @@ static const struct msg_tab err_msgtab[] = {
> { -19, 0, "Key field needs a watch or syscall given prior to it"
> }, { -20, 2, "-F missing value after operation for" },
> { -21, 2, "-F value should be number for" },
> - { -22, 2, "-F missing field name before operator for" }
> + { -22, 2, "-F missing field name before operator for" },
> + { -23, 2, "-C missng operation for "},
> + { -24, 2, "-C missing field name before operator for" },
> + { -25, 2, "-C missing value after operation for "},
> + { -26, 2, "-C unknown field:" },
> + { -27, 2, "-C unknown right hand value for comparison with:" },
> };
> diff --git a/trunk/lib/fieldtab.h b/trunk/lib/fieldtab.h
> index ad95814..c0432cc 100644
> --- a/trunk/lib/fieldtab.h
> +++ b/trunk/lib/fieldtab.h
> @@ -55,6 +55,8 @@ _S(AUDIT_WATCH, "path" )
> _S(AUDIT_PERM, "perm" )
> _S(AUDIT_DIR, "dir" )
> _S(AUDIT_FILETYPE, "filetype" )
> +_S(AUDIT_OBJ_UID, "obj_uid" )
> +_S(AUDIT_OBJ_GID, "obj_gid" )
>
> _S(AUDIT_ARG0, "a0" )
> _S(AUDIT_ARG1, "a1" )
> @@ -63,3 +65,4 @@ _S(AUDIT_ARG3, "a3" )
>
> _S(AUDIT_FILTERKEY, "key" )
>
> +_S(AUDIT_FIELD_COMPARE, "field_compare" )
> diff --git a/trunk/lib/libaudit.c b/trunk/lib/libaudit.c
> index 9a5070c..b179b5c 100644
> --- a/trunk/lib/libaudit.c
> +++ b/trunk/lib/libaudit.c
> @@ -783,6 +783,307 @@ int audit_rule_syscallbyname_data(struct
> audit_rule_data *rule,
> }
> hidden_def(audit_rule_syscallbyname_data)
>
> +int audit_rule_interfield_fieldpair_data(struct audit_rule_data **rulep,
> + const char *pair,
> + int flags) {
> + const char *f = pair;
> + char *v;
> + int op;
> + int field1, field2;
> + int vlen;
> + int offset;
> + struct audit_rule_data *rule = *rulep;
> +
> + if (f == NULL)
> + return -1;
> +
> + /* look for 2-char operators first
> + then look for 1-char operators afterwards
> + when found, null out the bytes under the operators to split
> + and set value pointer just past operator bytes
> + */
> + if ( (v = strstr(pair, "!=")) ) {
> + *v++ = '\0';
> + *v++ = '\0';
> + op = AUDIT_NOT_EQUAL;
> + } else if ( (v = strstr(pair, "=")) ) {
> + *v++ = '\0';
> + op = AUDIT_EQUAL;
> + } else {
> + return -13;
> + }
> +
> + if (v == NULL)
> + return -23;
> +
> + if (*f == 0)
> + return -24;
> +
> + if (*v == 0)
> + return -25;
> +
> + if ((field1 = audit_name_to_field(f)) < 0)
> + return -26;
> +
> + if ((field2 = audit_name_to_field(v)) < 0)
> + return -27;
> +
> + /* Exclude filter can be used only with MSGTYPE field */
> + if (flags == AUDIT_FILTER_EXCLUDE && field1 != AUDIT_MSGTYPE)
> + return -12;
> +
> + // It should always be AUDIT_FIELD_COMPARE
> + rule->fields[rule->field_count] = AUDIT_FIELD_COMPARE;
> + rule->fieldflags[rule->field_count] = op;
> + /* oh god, so many permutations */
> + switch (field1)
> + {
> + /* UID comparison */
> + case AUDIT_EUID:
> + switch(field2) {
> + case AUDIT_LOGINUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_AUID_TO_EUID;
> + break;
> + case AUDIT_FSUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EUID_TO_FSUID;
> + break;
> + case AUDIT_OBJ_UID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_EUID_TO_OBJ_UID;
> + break;
> + case AUDIT_SUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EUID_TO_SUID;
> + break;
> + case AUDIT_UID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_EUID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_FSUID:
> + switch(field2) {
> + case AUDIT_LOGINUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_AUID_TO_FSUID;
> + break;
> + case AUDIT_EUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EUID_TO_FSUID;
> + break;
> + case AUDIT_OBJ_UID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_FSUID_TO_OBJ_UID;
> + break;
> + case AUDIT_SUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_SUID_TO_FSUID;
> + break;
> + case AUDIT_UID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_FSUID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_LOGINUID:
> + switch(field2) {
> + case AUDIT_EUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_AUID_TO_EUID;
> + break;
> + case AUDIT_FSUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_AUID_TO_FSUID;
> + break;
> + case AUDIT_OBJ_UID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_AUID_TO_OBJ_UID;
> + break;
> + case AUDIT_SUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_AUID_TO_SUID;
> + break;
> + case AUDIT_UID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_AUID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_SUID:
> + switch(field2) {
> + case AUDIT_LOGINUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_AUID_TO_SUID;
> + break;
> + case AUDIT_EUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EUID_TO_SUID;
> + break;
> + case AUDIT_FSUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_SUID_TO_FSUID;
> + break;
> + case AUDIT_OBJ_UID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_SUID_TO_OBJ_UID;
> + break;
> + case AUDIT_UID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_SUID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_OBJ_UID:
> + switch(field2) {
> + case AUDIT_LOGINUID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_AUID_TO_OBJ_UID;
> + break;
> + case AUDIT_EUID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_EUID_TO_OBJ_UID;
> + break;
> + case AUDIT_FSUID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_FSUID_TO_OBJ_UID;
> + break;
> + case AUDIT_UID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_UID_TO_OBJ_UID;
> + break;
> + case AUDIT_SUID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_SUID_TO_OBJ_UID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_UID:
> + switch(field2) {
> + case AUDIT_LOGINUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_AUID;
> + break;
> + case AUDIT_EUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_EUID;
> + break;
> + case AUDIT_FSUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_FSUID;
> + break;
> + case AUDIT_OBJ_UID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_UID_TO_OBJ_UID;
> + break;
> + case AUDIT_SUID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_UID_TO_SUID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> +
> + /* GID comparisons */
> + case AUDIT_EGID:
> + switch(field2) {
> + case AUDIT_FSGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EGID_TO_FSGID;
> + break;
> + case AUDIT_GID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_GID_TO_EGID;
> + break;
> + case AUDIT_OBJ_GID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_EGID_TO_OBJ_GID;
> + break;
> + case AUDIT_SGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EGID_TO_SGID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_FSGID:
> + switch(field2) {
> + case AUDIT_SGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_SGID_TO_FSGID;
> + break;
> + case AUDIT_GID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_GID_TO_FSGID;
> + break;
> + case AUDIT_OBJ_GID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_FSGID_TO_OBJ_GID;
> + break;
> + case AUDIT_EGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EGID_TO_FSGID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_GID:
> + switch(field2) {
> + case AUDIT_EGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_GID_TO_EGID;
> + break;
> + case AUDIT_FSGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_GID_TO_FSGID;
> + break;
> + case AUDIT_OBJ_GID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_GID_TO_OBJ_GID;
> + break;
> + case AUDIT_SGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_GID_TO_SGID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_OBJ_GID:
> + switch(field2) {
> + case AUDIT_EGID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_EGID_TO_OBJ_GID;
> + break;
> + case AUDIT_FSGID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_FSGID_TO_OBJ_GID;
> + break;
> + case AUDIT_GID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_GID_TO_OBJ_GID;
> + break;
> + case AUDIT_SGID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_SGID_TO_OBJ_GID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + case AUDIT_SGID:
> + switch(field2) {
> + case AUDIT_FSGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_SGID_TO_FSGID;
> + break;
> + case AUDIT_GID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_GID_TO_SGID;
> + break;
> + case AUDIT_OBJ_GID:
> + rule->values[rule->field_count] =
AUDIT_COMPARE_SGID_TO_OBJ_GID;
> + break;
> + case AUDIT_EGID:
> + rule->values[rule->field_count] = AUDIT_COMPARE_EGID_TO_SGID;
> + break;
> + default:
> + return -1;
> + }
> + break;
> + /* fallthrough */
> + default:
> + if (field1 == AUDIT_INODE) {
> + if (!(op == AUDIT_NOT_EQUAL ||
> + op == AUDIT_EQUAL))
> + return -13;
> + }
> +
> + if (field1 == AUDIT_PPID && !(flags == AUDIT_FILTER_EXIT
> + || flags == AUDIT_FILTER_ENTRY))
> + return -17;
> +
> + if (!isdigit((char)*(v)))
> + return -21;
> +
> + if (field1 == AUDIT_INODE)
> + rule->values[rule->field_count] =
> + strtoul(v, NULL, 0);
> + else
> + rule->values[rule->field_count] =
> + strtol(v, NULL, 0);
> + break;
> + }
> + rule->field_count++;
> + return 0;
> +}
> +
> int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char
> *pair, int flags)
> {
> @@ -857,6 +1158,8 @@ int audit_rule_fieldpair_data(struct
> audit_rule_data **rulep, const char *pair,
> case AUDIT_SUID:
> case AUDIT_FSUID:
> case AUDIT_LOGINUID:
> + case AUDIT_OBJ_UID:
> + case AUDIT_OBJ_GID:
> // Do positive & negative separate for 32 bit systems
> vlen = strlen(v);
> if (isdigit((char)*(v)))
> diff --git a/trunk/lib/libaudit.h b/trunk/lib/libaudit.h
> index 8feaa39..911bce4 100644
> --- a/trunk/lib/libaudit.h
> +++ b/trunk/lib/libaudit.h
> @@ -428,6 +428,8 @@ extern int audit_rule_syscallbyname_data(struct
> audit_rule_data *rule,
> * adding new fields */
> extern int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
> const char *pair, int flags);
> +extern int audit_rule_interfield_fieldpair_data(struct audit_rule_data
> **rulep, + const char *pair, int flags);
> extern void audit_rule_free_data(struct audit_rule_data *rule);
>
> #ifdef __cplusplus
> diff --git a/trunk/src/auditctl.c b/trunk/src/auditctl.c
> index 34b7935..d7ec998 100644
> --- a/trunk/src/auditctl.c
> +++ b/trunk/src/auditctl.c
> @@ -482,7 +482,7 @@ static int setopt(int count, int lineno, char *vars[])
> keylen = AUDIT_MAX_KEY_LEN;
>
> while ((retval >= 0) && (c = getopt(count, vars,
> - "hislDvte:f:r:b:a:A:d:S:F:m:R:w:W:k:p:q:")) != EOF) {
> + "hislDvtC:e:f:r:b:a:A:d:S:F:m:R:w:W:k:p:q:")) != EOF) {
> int flags = AUDIT_FILTER_UNSET;
> rc = 10; // Init to something impossible to see if unused.
> switch (c) {
> @@ -731,7 +731,6 @@ static int setopt(int count, int lineno, char *vars[])
> retval = -1;
> break;
> }
> -
> rc = audit_rule_fieldpair_data(&rule_new,optarg,flags);
> if (rc != 0) {
> audit_number_to_errmsg(rc, optarg);
> @@ -743,6 +742,22 @@ static int setopt(int count, int lineno, char *vars[])
> }
>
> break;
> + case 'C':
> + if (add != AUDIT_FILTER_UNSET)
> + flags = add & AUDIT_FILTER_MASK;
> + else if (del != AUDIT_FILTER_UNSET)
> + flags = del & AUDIT_FILTER_MASK;
> +
> + rc = audit_rule_interfield_fieldpair_data(&rule_new, optarg, flags);
> + if (rc != 0) {
> + audit_number_to_errmsg(rc, optarg);
> + retval = -1;
> + } else {
> + if (rule_new->fields[rule_new->field_count - 1] ==
> + AUDIT_PERM)
> + audit_permadded = 1;
> + }
> + break;
> case 'm':
> if (count > 3) {
> fprintf(stderr,
> diff --git a/trunk/src/ausearch-report.c b/trunk/src/ausearch-report.c
> index d50c732..62e1ae0 100644
> --- a/trunk/src/ausearch-report.c
> +++ b/trunk/src/ausearch-report.c
> @@ -333,6 +333,7 @@ static struct nv_pair typetab[] = {
> {T_UID, "id"},
> {T_UID, "inode_uid"},
> {T_UID, "sauid"},
> + {T_UID, "obj_uid"},
> {T_GID, "gid"},
> {T_GID, "egid"},
> {T_GID, "sgid"},
next prev parent reply other threads:[~2012-02-28 23:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-11 19:09 test patch for auditctl inter-field comparisons on euid/uid, egid/gid Peter Moody
2011-12-12 14:40 ` Steve Grubb
2011-12-12 16:40 ` Peter Moody
2011-12-14 1:38 ` Peter Moody
2011-12-15 0:16 ` Peter Moody
2011-12-15 1:18 ` Peter Moody
2011-12-15 6:55 ` Stephen Quinney
2011-12-15 13:36 ` Steve Grubb
2011-12-16 23:34 ` Peter Moody
2012-02-28 23:07 ` Steve Grubb [this message]
2012-02-28 23:12 ` Peter Moody
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201202281807.34269.sgrubb@redhat.com \
--to=sgrubb@redhat.com \
--cc=linux-audit@redhat.com \
--cc=pmoody@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.