lkml.org 
[lkml]   [2022]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v35 22/29] Audit: Keep multiple LSM data in audit_names
    On Mon, Apr 25, 2022 at 7:32 PM John Johansen
    <john.johansen@canonical.com> wrote:
    > On 4/18/22 07:59, Casey Schaufler wrote:
    > > Replace the osid field in the audit_names structure
    > > with a lsmblob structure. This accomodates the use
    > > of an lsmblob in security_audit_rule_match() and
    > > security_inode_getsecid().
    > >
    > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
    > > Acked-by: Paul Moore <paul@paul-moore.com>
    > > ---
    > > kernel/audit.h | 2 +-
    > > kernel/auditsc.c | 22 ++++++++--------------
    > > 2 files changed, 9 insertions(+), 15 deletions(-)

    ...

    > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
    > > index 231631f61550..6fe9f2525fc1 100644
    > > --- a/kernel/auditsc.c
    > > +++ b/kernel/auditsc.c
    > > @@ -700,17 +700,16 @@ static int audit_filter_rules(struct task_struct *tsk,
    > > * lsmblob, which happens later in
    > > * this patch set.
    > > */
    > > - lsmblob_init(&blob, name->osid);
    > > result = security_audit_rule_match(
    > > - &blob,
    > > + &name->lsmblob,
    > > f->type,
    > > f->op,
    > > &f->lsm_rules);
    > > } else if (ctx) {
    > > list_for_each_entry(n, &ctx->names_list, list) {
    > > - lsmblob_init(&blob, n->osid);
    > > if (security_audit_rule_match(
    > > - &blob, f->type, f->op,
    > > + &n->lsmblob,
    > > + f->type, f->op,
    > > &f->lsm_rules)) {
    > > ++result;
    > > break;
    > > @@ -1589,13 +1588,12 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
    > > from_kgid(&init_user_ns, n->gid),
    > > MAJOR(n->rdev),
    > > MINOR(n->rdev));
    > > - if (n->osid != 0) {
    > > - struct lsmblob blob;
    > > + if (lsmblob_is_set(&n->lsmblob)) {
    > > struct lsmcontext lsmctx;
    > >
    > > - lsmblob_init(&blob, n->osid);
    > > - if (security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_FIRST)) {
    > > - audit_log_format(ab, " osid=%u", n->osid);
    > > + if (security_secid_to_secctx(&n->lsmblob, &lsmctx,
    > > + LSMBLOB_FIRST)) {
    > > + audit_log_format(ab, " osid=?");
    >
    > is there something better we can do here? This feels like a regression

    Unfortunately no, or at least nothing has been suggested that is an
    improvement on this approach. We could overload the existing field,
    but that runs the risk of confusing userspace tooling and potentially
    bumping into the buffer limit in some more complex configurations.
    The "?" value was chosen as it is a commonly accepted way for the
    audit subsystem to indicate that a value is "missing" and in the case
    of new/updated userspace tooling this would be an indication to look
    for the new record type which provides all of the necessary LSM
    labels. In the case of old/unaware userspace tooling it would serve
    as a graceful indicator that something is awry, i.e. you are using new
    kernel functionality without updating your userspace.

    --
    paul-moore.com

    \
     
     \ /
      Last update: 2022-04-26 19:59    [W:6.854 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site