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: <d3f4a53e386d4bb1b8c608ac8b6bec1f@huawei.com>
Date:   Tue, 12 May 2020 15:31:01 +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: owner-linux-security-module@...r.kernel.org [mailto:owner-linux-
> security-module@...r.kernel.org] On Behalf Of Mimi Zohar
> Sent: Tuesday, May 12, 2020 4:17 PM
> On Tue, 2020-05-12 at 07:54 +0000, Roberto Sassu wrote:
> > > > > Roberto, EVM is only triggered by IMA, unless you've modified the
> > > > > kernel to do otherwise.
> > > >
> > > > EVM would deny xattr/attr operations even if IMA is disabled in the
> > > > kernel configuration. For example, evm_setxattr() returns the value
> > > > from evm_protect_xattr(). IMA is not involved there.
> > >
> > > Commit ae1ba1676b88 ("EVM: Allow userland to permit modification of
> > > EVM-protected metadata")
> introduced EVM_ALLOW_METADATA_WRITES
> > > to allow writing the EVM portable and immutable file signatures.
> >
> > According to Documentation/ABI/testing/evm:
> >
> > Note that once a key has been loaded, it will no longer be
> > possible to enable metadata modification.
> 
> Not any key, but the HMAC key.
> 
> 2         Permit modification of EVM-protected metadata at
>           runtime. Not supported if HMAC validation and
>           creation is enabled.

#ifdef CONFIG_EVM_LOAD_X509
void __init evm_load_x509(void)
{
[...]
        rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
        if (!rc)
                evm_initialized |= EVM_INIT_X509;


static ssize_t evm_write_key(struct file *file, const char __user *buf,
                             size_t count, loff_t *ppos)
{
[...]
        /* Don't allow a request to freshly enable metadata writes if
         * keys are loaded.
         */
        if ((i & EVM_ALLOW_METADATA_WRITES) &&
            ((evm_initialized & EVM_KEY_MASK) != 0) &&
            !(evm_initialized & EVM_ALLOW_METADATA_WRITES))
                return -EPERM;

Should have been:

        if ((i & EVM_ALLOW_METADATA_WRITES) &&
            ((evm_initialized & EVM_INIT_HMAC) != 0) &&
            !(evm_initialized & EVM_ALLOW_METADATA_WRITES))
                return -EPERM;

> 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().

Roberto

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