[PATCH v3 2/7] add support for the uring filter list
Richard Guy Briggs
rgb at redhat.com
Tue Nov 2 16:32:58 UTC 2021
On 2021-11-01 11:58, Steve Grubb wrote:
> Hello Richard,
>
> On Monday, November 1, 2021 11:05:49 AM EDT Richard Guy Briggs wrote:
> > On 2021-10-29 14:39, Steve Grubb wrote:
> > > On Thursday, October 28, 2021 3:59:34 PM EDT Richard Guy Briggs wrote:
> > > > Kernel support to audit io_uring operations filtering was added with
> > > > commit 67daf270cebc ("audit: add filtering for io_uring records"). Add
> > > > support for the "uring" filter list to auditctl.
> > >
> > > Might have been good to show what the resulting auditctl command looks
> > > like. I think it would be:
> > >
> > > auditctl -a always,io_uring -U open -F uid!=0 -F key=io_uring
> > >
> > > But I wonder, why the choice of -U rather than -S? That would make
> > > remembering the syntax easier.
> > >
> > > auditctl -a always,io_uring -S open -F uid!=0 -F key=io_uring
> >
> > Well, I keep seeing the same what I assume is a typo in your
> > communications about io_uring where the "u" is missing, which might help
> > trigger your memory about the syntax.
>
> Yeah, but I'm thinking that we can abstract that technicality away and keep
> the syntax the same.
They do use the same mechanism to get the data into the kernel, but this
is not user-visible.
> > The io_uring operations name list is different than the syscall list, so
> > it needs to use a different lookup table.
>
> Right. So, if you choose an operation that is not supported, you get an
> error. But to help people know what is supported, we can add the lookup to
> ausyscall where --io_uring could direct it to the right lookup table.
>
> > Have I misunderstood something?
>
> No, but I'm thinking of aesthetics and usability. You already have to specify
> a filter. We don't really need to have completely different syntax in
> addition. Especially since the operations map to the equivalent of a syscall.
In terms of usibility, it is a different lookup table and the operations
don't map exactly to each other. I'd *expect* to use a different
command line option to specify io_uring ops than I did for syscalls
since they are not the same since if I changed a rule text from one list
to another, I'd also need to change the list of ops and which list they
came from. I'd want it to throw an error if I used the wrong argument
type.
> > > > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
> > > > ---
> > > > docs/audit.rules.7 | 19 ++++--
> > > > docs/audit_add_rule_data.3 | 4 ++
> > > > docs/auditctl.8 | 10 ++-
> > > > lib/flagtab.h | 11 ++--
> > > > lib/libaudit.c | 50 ++++++++++++---
> > > > lib/libaudit.h | 7 +++
> > > > lib/lookup_table.c | 20 ++++++
> > > > lib/private.h | 1 +
> > > > src/auditctl-listing.c | 52 ++++++++++------
> > > > src/auditctl.c | 121 ++++++++++++++++++++++++++++++++-----
> > > > 10 files changed, 240 insertions(+), 55 deletions(-)
> > >
> > > <snip a whole lot of documentation>
> > >
> > > > diff --git a/lib/libaudit.c b/lib/libaudit.c
> > > > index 54e276156ef0..3790444f4497 100644
> > > > --- a/lib/libaudit.c
> > > > +++ b/lib/libaudit.c
> > > > @@ -86,6 +86,7 @@ static const struct nv_list failure_actions[] =
> > > > int _audit_permadded = 0;
> > > > int _audit_archadded = 0;
> > > > int _audit_syscalladded = 0;
> > > > +int _audit_uringopadded = 0;
> > > > int _audit_exeadded = 0;
> > > > int _audit_filterfsadded = 0;
> > > > unsigned int _audit_elf = 0U;
> > > > @@ -999,6 +1000,26 @@ int audit_rule_syscallbyname_data(struct
> > > > audit_rule_data *rule, return -1;
> > > > }
> > > >
> > > > +int audit_rule_uringopbyname_data(struct audit_rule_data *rule,
> > > > + const char *uringop)
> > > > +{
> > > > + int nr, i;
> > > > +
> > > > + if (!strcmp(uringop, "all")) {
> > > > + for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
> > > > + rule->mask[i] = ~0;
> > > > + return 0;
> > > > + }
> > > > + nr = audit_name_to_uringop(uringop);
> > > > + if (nr < 0) {
> > > > + if (isdigit(uringop[0]))
> > > > + nr = strtol(uringop, NULL, 0);
> > > > + }
> > > > + if (nr >= 0)
> > > > + return audit_rule_syscall_data(rule, nr);
> > > > + return -1;
> > > > +}
> > > > +
> > > > int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
> > > > const char *pair,
> > > > int flags)
> > > > @@ -1044,7 +1065,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, return -EAU_COMPVALUNKNOWN;
> > > >
> > > > /* Interfield comparison can only be in exit filter */
> > > > - if (flags != AUDIT_FILTER_EXIT)
> > > > + if (flags != AUDIT_FILTER_EXIT && flags !=
> > > > AUDIT_FILTER_URING_EXIT) return -EAU_EXITONLY;
> > > >
> > > > // It should always be AUDIT_FIELD_COMPARE
> > > > @@ -1557,7 +1578,8 @@ int audit_rule_fieldpair_data(struct
> > > > audit_rule_data
> > > > **rulep, const char *pair, }
> > > > break;
> > > > case AUDIT_EXIT:
> > > > - if (flags != AUDIT_FILTER_EXIT)
> > > > + if (flags != AUDIT_FILTER_EXIT &&
> > > > + flags != AUDIT_FILTER_URING_EXIT)
> > > > return -EAU_EXITONLY;
> > > > vlen = strlen(v);
> > > > if (isdigit((char)*(v)))
> > > > @@ -1599,7 +1621,8 @@ int audit_rule_fieldpair_data(struct
> > > > audit_rule_data
> > > > **rulep, const char *pair, case AUDIT_DIR:
> > > > /* Watch & object filtering is invalid on anything
> > > > * but exit */
> > > > - if (flags != AUDIT_FILTER_EXIT)
> > > > + if (flags != AUDIT_FILTER_EXIT &&
> > > > + flags != AUDIT_FILTER_URING_EXIT)
> > > > return -EAU_EXITONLY;
> > > > if (field == AUDIT_WATCH || field == AUDIT_DIR)
> > > > _audit_permadded = 1;
> > > > @@ -1621,9 +1644,11 @@ int audit_rule_fieldpair_data(struct
> > > > audit_rule_data **rulep, const char *pair, _audit_exeadded = 1;
> > > > }
> > > > if (field == AUDIT_FILTERKEY &&
> > > > - !(_audit_syscalladded ||
> > > > _audit_permadded || - _audit_exeadded ||
> > > > - _audit_filterfsadded))
> > > > + !(_audit_syscalladded ||
> > > > + _audit_uringopadded ||
> > > > + _audit_permadded ||
> > > > + _audit_exeadded ||
> > > > + _audit_filterfsadded))
> > > > return -EAU_KEYDEP;
> > > > vlen = strlen(v);
> > > > if (field == AUDIT_FILTERKEY &&
> > > > @@ -1712,7 +1737,8 @@ int audit_rule_fieldpair_data(struct
> > > > audit_rule_data
> > > > **rulep, const char *pair, }
> > > > break;
> > > > case AUDIT_FILETYPE:
> > > > - if (!(flags == AUDIT_FILTER_EXIT))
> > > > + if (!(flags == AUDIT_FILTER_EXIT ||
> > > > + flags == AUDIT_FILTER_URING_EXIT))
> > > > return -EAU_EXITONLY;
> > > > rule->values[rule->field_count] =
> > > > audit_name_to_ftype(v);
> > > > @@ -1754,7 +1780,8 @@ int audit_rule_fieldpair_data(struct
> > > > audit_rule_data
> > > > **rulep, const char *pair, return -EAU_FIELDNOSUPPORT;
> > > > if (flags != AUDIT_FILTER_EXCLUDE &&
> > > > flags != AUDIT_FILTER_USER &&
> > > > - flags != AUDIT_FILTER_EXIT)
> > > > + flags != AUDIT_FILTER_EXIT &&
> > > > + flags != AUDIT_FILTER_URING_EXIT)
> > >
> > > This is in the session_id code. Looking at the example audit event:
> > >
> > > https://listman.redhat.com/archives/linux-audit/2021-September/msg00058.h
> > > tml
> > >
> > > session_id is not in the record.
> >
> > Fair enough. It can be re-added if we are able to reliably report it.
>
> Thanks.
>
> > > > return -EAU_FIELDNOFILTER;
> > > > // Do positive & negative separate for 32 bit systems
> > > > vlen = strlen(v);
> > > > @@ -1775,7 +1802,8 @@ int audit_rule_fieldpair_data(struct
> > > > audit_rule_data
> > > > **rulep, const char *pair, break;
> > >
> > > > case AUDIT_DEVMAJOR...AUDIT_INODE:
> > > ^^^ Can you audit by devmajor, devminor, or inode in io_ring?
> >
> > Should be able to monitor files. The old "-w" syntax is not supported
> > but path= and dir= should be.
>
> But that's not what this is. These is for:
> -F dev_major=
> -F dev_minor=
> -F inode=
>
> It dates back to before watches were possible. In any event, this is being
> allowed when I suspect it shouldn't.
Why shouldn't it be allowed?
> -Steve
- RGB
--
Richard Guy Briggs <rgb at redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
More information about the Linux-audit
mailing list