Re: [PATCH v10 2/4] security: Allow all LSMs to provide xattrs for inode_init_security hook

From: Roberto Sassu
Date: Thu Apr 06 2023 - 05:09:04 EST


On 4/5/2023 10:43 PM, Casey Schaufler wrote:
On 4/5/2023 12:59 PM, Paul Moore wrote:
On Wed, Apr 5, 2023 at 5:44 AM Roberto Sassu
<roberto.sassu@xxxxxxxxxxxxxxx> wrote:
On 4/5/2023 4:08 AM, Casey Schaufler wrote:
On 4/4/2023 11:54 AM, Paul Moore wrote:
On Fri, Mar 31, 2023 at 8:33 AM Roberto Sassu
<roberto.sassu@xxxxxxxxxxxxxxx> wrote:
..

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index cfcbb748da2..8392983334b 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -52,6 +52,15 @@
#define SMK_RECEIVING 1
#define SMK_SENDING 2

+/*
+ * Smack uses multiple xattrs.
+ * SMACK64 - for access control, SMACK64EXEC - label for the program,
I think it would be good to move SMACK64EXEC to its own line; it took
me a minute to figure out why SMACK_INODE_INIT_XATTRS was set to '4'
when I only say three comment lines ... ;)

+ * SMACK64MMAP - controls library loading,
+ * SMACK64TRANSMUTE - label initialization,
+ * Not saved on files - SMACK64IPIN and SMACK64IPOUT
+ */
+#define SMACK_INODE_INIT_XATTRS 4
If smack_inode_init_security() only ever populates a single xattr, and
that is the only current user of SMACK_INODE_INIT_XATTRS, can we make
this '1' and shrink the xattr allocation a bit?
If the parent directory is marked with SMACK64_TRANSMUTE, the access
rule allowing the access has the "t" mode, and the object being initialized
is a directory, the new inode should get the SMACK64_TRANSMUTE attribute.
The callers of security_inode_init_security() don't seem to care.
I can't say if the evm code is getting SMACK64_TRANSMUTE or, for that
matter, SMACK64_EXEC and SMACK64_MMAP, some other way. The older system
allowed for multiple Smack xattrs, but I'm not clear on exactly how.
If you like to set an additional xattr, that would be possible now.
Since we reserve multiple xattrs, we can call lsm_get_xattr_slot()
another time and set SMACK64_TRANSMUTE.

I think, if the kernel config has CONFIG_EVM_EXTRA_SMACK_XATTRS set,
EVM would protect SMACK64_TRANSMUTE too.
Ooookay, but can someone explain to me how either the current, or
patched, smack_inode_init_security() function can return multiple
xattrs via the security_inode_init_security() LSM hook?

It can't.

I'm hoping
I'm missing something really obvious, but I can only see a single
Smack xattr being returned ...

Smack is setting the transmute attribute in smack_d_instantiate().
The exec and mmap attributes are always set explicitly.

I don't know how the "extra" Smack attributes were obtained by evm
before, and I haven't been looking at how they're doing it now.
I have assumed that CONFIG_EVM_EXTRA_SMACK_XATTRS does something
meaningful.

EVM has a list of xattrs to protect. Without CONFIG_EVM_EXTRA_SMACK_XATTRS, EVM protects only security.SMACK64. With it, also the other Smack xattrs.

EVM calculates the HMAC of xattrs from that list at inode creation time (during the execution of security_inode_init_security(), after other LSMs) and during set/remove xattrs operations on the VFS.

Currently, Smack provides only security.SMACK64, so I agree with Paul that we should reserve as many xattr as we use. If in the future, we add security.SMACK_TRANSMUTE64, we can increase the number of reserved xattrs to 2.

Roberto