linux-kernel - RE: [RFC][PATCH 1/3] evm: Move hooks outside LSM infrastructure

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]

Message-ID: <4fb6c8457ac44af3b464fab712a10a37@huawei.com>
Date:   Wed, 13 May 2020 07:21:33 +0000
From:   Roberto Sassu <roberto.sassu@...wei.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>,
        "david.safford@...il.com" <david.safford@...il.com>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "jmorris@...ei.org" <jmorris@...ei.org>,
        "John Johansen" <john.johansen@...onical.com>,
        "matthewgarrett@...gle.com" <matthewgarrett@...gle.com>
CC:     "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Silviu Vlasceanu <Silviu.Vlasceanu@...wei.com>
Subject: RE: [RFC][PATCH 1/3] evm: Move hooks outside LSM infrastructure

> From: Mimi Zohar [mailto:zohar@...ux.ibm.com]
> Sent: Tuesday, May 12, 2020 9:38 PM
> On Tue, 2020-05-12 at 16:31 +0000, Roberto Sassu wrote:
> > > From: Mimi Zohar [mailto:zohar@...ux.ibm.com]
> 
> > > > > Each time the EVM protected file metadata is updated, the EVM
> HMAC
> > > is
> > > > > updated, assuming the existing EVM HMAC is valid.  Userspace
> should
> > > > > not have access to the HMAC key, so we only allow writing EVM
> > > > > signatures.
> > > > >
> > > > > The only difference between writing the original EVM signature and
> the
> > > > > new portable and immutable signature is the security.ima xattr
> > > > > requirement.  Since the new EVM signature does not include the
> > > > > filesystem specific data, something else needs to bind the file
> > > > > metadata to the file data.  Thus the IMA xattr requirement.
> > > > >
> > > > > Assuming that the new EVM signature is written last, as long as there
> > > > > is an IMA xattr, there shouldn't be a problem writing the new EVM
> > > > > signature.
> > > >
> > > >         /* first need to know the sig type */
> > > >         rc = vfs_getxattr_alloc(dentry, XATTR_NAME_EVM, (char
> > > **)&xattr_data, 0,
> > > >                                 GFP_NOFS);
> > > >         if (rc <= 0) {
> > > >                 evm_status = INTEGRITY_FAIL;
> > > >                 if (rc == -ENODATA) {
> > > >                         rc = evm_find_protected_xattrs(dentry);
> > > >                         if (rc > 0)
> > > >                                 evm_status = INTEGRITY_NOLABEL;
> > > >                         else if (rc == 0)
> > > >                                 evm_status = INTEGRITY_NOXATTRS; /* new file */
> > > >
> > > > If EVM_ALLOW_METADATA_WRITES is cleared, only the first xattr
> > > > can be written (status INTEGRITY_NOXATTRS is ok). After,
> > > > evm_find_protected_xattrs() returns rc > 0, so the status is
> > > > INTEGRITY_NOLABEL, which is not ignored by evm_protect_xattr().
> > >
> > > With EVM HMAC enabled, as a result of writing the first protected
> > > xattr, an EVM HMAC should be calculated and written in
> > > evm_inode_post_setxattr().
> >
> > To solve the ordering issue, wouldn't allowing setxattr() on a file
> > with portable signature that does not yet pass verification be safe?
> > evm_update_evmxattr() checks if the signature is portable and
> > if yes, does not calculate the HMAC.
> 
> Before agreeing to allowing the protected xattrs to be written on a
> file with a portable signature that does not yet pass verification are
> safe, would we be introducing any new types of attacks?

Allowing xattr/attr update means that someone can do:

setxattr(path, "security.evm", ...);	with type=5

all subsequent setxattr()/setattr() succeed until the correct
combination is set.

At that point, any xattr/attr operation fails, even if one tries to set
an xattr with the same value. If we still deny the operation when the
verification succeeds, we have to fix that.

It is common that the signature passes verification before user space
tools finish to set xattrs/attrs. For example, if a file is created with
mode 644 and this was the mode at the time of signing, any attempt
by tar for example to set again the same mode fails.

If allowing a change of xattrs/attrs for portable signatures is safe or
not, I would say yes. Portable signatures cannot be modified even
if __vfs_setxattr_noperm() is called directly.

> For example, would we differentiate between portable signatures that
> don't pass verification and ones that do?  If we don't differentiate,
> could it be used for DoS?  Should it be limited to new files?

I would prefer to lock files when signatures pass the verification to
avoid accidental changes.

Unless we find a better way to identify new file, without depending
on the appraisal policy, I would allow the operation even for existing
files.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli