linux-kernel - Re: [PATCH ghak109 V1] audit: link integrity evm_write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Mar 2019 21:03:45 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Richard Guy Briggs <rgb@...hat.com>
Cc:     linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Linux-Audit Mailing List <linux-audit@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>, sgrubb@...hat.com,
        omosnace@...hat.com, Eric Paris <eparis@...isplace.org>,
        Serge Hallyn <serge@...lyn.com>, zohar@...ux.ibm.com,
        mjg59@...gle.com
Subject: Re: [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record
 to syscall event

On Wed, Mar 20, 2019 at 8:50 PM Richard Guy Briggs <rgb@...hat.com> wrote:
> On 2019-03-20 19:48, Paul Moore wrote:
> > On Sat, Mar 16, 2019 at 8:10 AM Richard Guy Briggs <rgb@...hat.com> wrote:
> > > In commit fa516b66a1bf ("EVM: Allow runtime modification of the set of
> > > verified xattrs"), the call to audit_log_start() is missing a context to
> > > link it to an audit event. Since this event is in user context, add
> > > the process' syscall context to the record.
> > >
> > > In addition, the orphaned keyword "locked" appears in the record.
> > > Normalize this by changing it to "xattr=(locked)".
> > >
> > > Please see the github issue
> > > https://github.com/linux-audit/audit-kernel/issues/109
> > >
> > > Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
> > > ---
> > >  security/integrity/evm/evm_secfs.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)

...

> > > @@ -222,7 +223,7 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
> > >                 inode_lock(inode);
> > >                 err = simple_setattr(evm_xattrs, &newattrs);
> > >                 inode_unlock(inode);
> > > -               audit_log_format(ab, "locked");
> > > +               audit_log_format(ab, "xattr=(locked)");
> >
> > Two things come to mind:
> >
> > * While we can clearly trust the string above, should we be logging
> > the xattr field value as an untrusted string so it is consistent with
> > how we record other xattr names?
>
> That would be a question for Steve.

Yep, that's who I wanted to hear from, it's not really something I
expected you to answer Richard.  I vaguely remember something about
Steve's audit libs being able to handle both trusted and untrusted
value strings for a given field, but I could have confused "able to
handle" with "don't care".

-- 
paul moore
www.paul-moore.com