linux-kernel - Re: [PATCH v7 4/6] security: Allow all LSMs to provide xattrs for inode_init

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: <22f48112c2fbc2812317c33af13accb022e9abdf.camel@linux.ibm.com>
Date:   Mon, 20 Feb 2023 07:43:24 -0500
From:   Mimi Zohar <zohar@...ux.ibm.com>
To:     Roberto Sassu <roberto.sassu@...weicloud.com>, mark@...heh.com,
        jlbec@...lplan.org, joseph.qi@...ux.alibaba.com,
        dmitry.kasatkin@...il.com, paul@...l-moore.com, jmorris@...ei.org,
        serge@...lyn.com, stephen.smalley.work@...il.com,
        eparis@...isplace.org, casey@...aufler-ca.com
Cc:     ocfs2-devel@....oracle.com, reiserfs-devel@...r.kernel.org,
        linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
        linux-kernel@...r.kernel.org, keescook@...omium.org,
        nicolas.bouchinet@...p-os.org,
        Roberto Sassu <roberto.sassu@...wei.com>
Subject: Re: [PATCH v7 4/6] security: Allow all LSMs to provide xattrs for
 inode_init_security hook

On Thu, 2022-12-01 at 11:41 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@...wei.com>
> 
> Currently, security_inode_init_security() supports only one LSM providing
> an xattr and EVM calculating the HMAC on that xattr, plus other inode
> metadata.
> 
> Allow all LSMs to provide one or multiple xattrs, by extending the security
> blob reservation mechanism. Introduce the new lbs_xattr field of the
> lsm_blob_sizes structure, so that each LSM can specify how many xattrs it
> needs, and the LSM infrastructure knows how many xattr slots it should
> allocate.
> 
> Dynamically allocate the xattrs array to be populated by LSMs with the
> inode_init_security hook, and pass it to the latter instead of the
> name/value/len triple. Update the documentation accordingly, and fix the
> description of the xattr name, as it is not allocated anymore.
> 
> Since the LSM infrastructure, at initialization time, updates the number of
> the requested xattrs provided by each LSM with a corresponding offset in
> the security blob (in this case the xattr array), it makes straightforward
> for an LSM to access the right position in the xattr array.
> 
> There is still the issue that an LSM might not fill the xattr, even if it
> requests it (legitimate case, for example it might have been loaded but not
> initialized with a policy). Since users of the xattr array (e.g. the
> initxattrs() callbacks) detect the end of the xattr array by checking if
> the xattr name is NULL, not filling an xattr would cause those users to
> stop scanning xattrs prematurely.
> 
> Solve that issue by introducing security_check_compact_filled_xattrs(),
> which does a basic check of the xattr array (if the xattr name is filled,
> the xattr value should be too, and viceversa), and compacts the xattr array
> by removing the holes.
> 
> An alternative solution would be to let users of the xattr array know the
> number of elements of that array, so that they don't have to check the
> termination. However, this seems more invasive, compared to a simple move
> of few array elements.
> 
> security_check_compact_filled_xattrs() also determines how many xattrs in
> the xattr array have been filled. If there is none, skip
> evm_inode_init_security() and initxattrs(). Skipping the former also avoids
> EVM to crash the kernel, as it is expecting a filled xattr.
> 
> Finally, adapt both SELinux and Smack to use the new definition of the
> inode_init_security hook, and to correctly fill the designated slots in the
> xattr array. For Smack, reserve space for the other defined xattrs although
> they are not set yet in smack_inode_init_security().
> 
> Reported-by: Nicolas Bouchinet <nicolas.bouchinet@...p-os.org> (EVM crash)
> Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> Reviewed-by: Casey Schaufler <casey@...aufler-ca.com>

Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>