smack: fix bug: invalid label of unix socket file

According to [1], the label of a UNIX domain socket (UDS)
file (i.e., the filesystem object representing the socket)
is not supposed to participate in Smack security.

To achieve this, [1] labels UDS files with "*"
in smack_d_instantiate().

Before [2], smack_d_instantiate() was responsible
for initializing Smack security for all inodes,
except ones under /proc

[2] imposed the sole responsibility for initializing
inode security for newly created filesystem objects
on smack_inode_init_security().

However, smack_inode_init_security() lacks some logic
present in smack_d_instantiate().
In particular, it does not label UDS files with "*".

This patch adds the missing labeling of UDS files
with "*" to smack_inode_init_security().

Labeling UDS files with "*" in smack_d_instantiate()
still works for stale UDS files that already exist on
disk. Stale UDS files are useless, but I keep labeling
them for consistency and maybe to make easier for user
to delete them.

Compared to [1], this version introduces the following
improvements:

  * UDS file label is held inside inode only
    and not saved to xattrs.

  * relabeling UDS files (setxattr, removexattr, etc.)
    is blocked.

[1] 2010-11-24 Casey Schaufler
commit b4e0d5f0791b ("Smack: UDS revision")

[2] 2023-11-16 roberto.sassu
Fixes: e63d86b8b764 ("smack: Initialize the in-memory inode in smack_inode_init_security()")
Link: https://lore.kernel.org/linux-security-module/20231116090125.187209-5-roberto.sassu@huaweicloud.com/
Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

diff --git a/Documentation/admin-guide/LSM/Smack.rst b/Documentation/admin-guide/LSM/Smack.rst
index 6d44f4fdbf59fff272f868986f4d0b4e3db1b3c2..1b554b5bf98e6ae88718f1c7957bca34d9eeac23 100644 (file)
--- a/Documentation/admin-guide/LSM/Smack.rst
+++ b/Documentation/admin-guide/LSM/Smack.rst
@@ -696,6 +696,11 @@ sockets.
        A privileged program may set this to match the label of another
        task with which it hopes to communicate.
 
+UNIX domain socket (UDS) with a BSD address functions both as a file in a
+filesystem and as a socket. As a file, it carries the SMACK64 attribute. This
+attribute is not involved in Smack security enforcement and is immutably
+assigned the label "*".
+
 Smack Netlabel Exceptions
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5cd19f3498cbd594164e2538be4bd264d16f7260..30f20fef3331e060e21c85f7731389fec069ba96 100644 (file)
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1020,6 +1020,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
        bool trans_cred;
        bool trans_rule;
 
+       /*
+        * UNIX domain sockets use lower level socket data. Let
+        * UDS inode have fixed * label to keep smack_inode_permission() calm
+        * when called from unix_find_bsd()
+        */
+       if (S_ISSOCK(inode->i_mode)) {
+               /* forced label, no need to save to xattrs */
+               issp->smk_inode = &smack_known_star;
+               goto instant_inode;
+       }
        /*
         * If equal, transmuting already occurred in
         * smack_dentry_create_files_as(). No need to check again.
@@ -1056,14 +1066,16 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
                }
        }
 
-       issp->smk_flags |= (SMK_INODE_INSTANT | transflag);
-       if (rc)
-               return rc;
-
-       return xattr_dupval(xattrs, xattr_count,
+       if (rc == 0)
+               if (xattr_dupval(xattrs, xattr_count,
                            XATTR_SMACK_SUFFIX,
                            issp->smk_inode->smk_known,
-                    strlen(issp->smk_inode->smk_known));
+                    strlen(issp->smk_inode->smk_known)
+               ))
+                       rc = -ENOMEM;
+instant_inode:
+       issp->smk_flags |= (SMK_INODE_INSTANT | transflag);
+       return rc;
 }
 
 /**
@@ -1337,13 +1349,23 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
        int check_import = 0;
        int check_star = 0;
        int rc = 0;
+       umode_t const i_mode = d_backing_inode(dentry)->i_mode;
 
        /*
         * Check label validity here so import won't fail in post_setxattr
         */
-       if (strcmp(name, XATTR_NAME_SMACK) == 0 ||
-           strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
-           strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) {
+       if (strcmp(name, XATTR_NAME_SMACK) == 0) {
+               /*
+                * UDS inode has fixed label
+                */
+               if (S_ISSOCK(i_mode)) {
+                       rc = -EINVAL;
+               } else {
+                       check_priv = 1;
+                       check_import = 1;
+               }
+       } else if (strcmp(name, XATTR_NAME_SMACKIPIN) == 0 ||
+                  strcmp(name, XATTR_NAME_SMACKIPOUT) == 0) {
                check_priv = 1;
                check_import = 1;
        } else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0 ||
@@ -1353,7 +1375,7 @@ static int smack_inode_setxattr(struct mnt_idmap *idmap,
                check_star = 1;
        } else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) {
                check_priv = 1;
-               if (!S_ISDIR(d_backing_inode(dentry)->i_mode) ||
+               if (!S_ISDIR(i_mode) ||
                    size != TRANS_TRUE_SIZE ||
                    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
                        rc = -EINVAL;
@@ -1484,12 +1506,15 @@ static int smack_inode_removexattr(struct mnt_idmap *idmap,
         * Don't do anything special for these.
         *      XATTR_NAME_SMACKIPIN
         *      XATTR_NAME_SMACKIPOUT
+        *      XATTR_NAME_SMACK if S_ISSOCK (UDS inode has fixed label)
         */
        if (strcmp(name, XATTR_NAME_SMACK) == 0) {
-               struct super_block *sbp = dentry->d_sb;
-               struct superblock_smack *sbsp = smack_superblock(sbp);
+               if (!S_ISSOCK(d_backing_inode(dentry)->i_mode)) {
+                       struct super_block *sbp = dentry->d_sb;
+                       struct superblock_smack *sbsp = smack_superblock(sbp);
 
-               isp->smk_inode = sbsp->smk_default;
+                       isp->smk_inode = sbsp->smk_default;
+               }
        } else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0)
                isp->smk_task = NULL;
        else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0)
@@ -3607,7 +3632,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
                 */
 
                /*
-                * UNIX domain sockets use lower level socket data.
+                * UDS inode has fixed label (*)
                 */
                if (S_ISSOCK(inode->i_mode)) {
                        final = &smack_known_star;
@@ -4872,6 +4897,11 @@ static int smack_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
 
 static int smack_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
 {
+       /*
+        * UDS inode has fixed label. Ignore nfs label.
+        */
+       if (S_ISSOCK(inode->i_mode))
+               return 0;
        return smack_inode_setsecurity(inode, XATTR_SMACK_SUFFIX, ctx,
                                       ctxlen, 0);
 }