From 1d8faaf6caf4acaf0e2d4943b51c024a96c80624 Mon Sep 17 00:00:00 2001 From: Etienne AUJAMES Date: Thu, 6 Oct 2022 15:30:54 +0200 Subject: [PATCH] LU-16210 llite: replace selinux_is_enabled() selinux_is_enabled() was removed from kernel 5.1. The commit 39e5bfa add the kernel support by assuming SELinux to be enabled if the function selinux_is_enabled() does not exist. This has performances impacts: on older kernel (e.g: Centos7) getxattr RPCs was not send for "security.selinux" if selinux was disabled. Utilities like "ls -l" always try to get "security.selinux". See the LU-549 for more information. This patch uses security_inode_listsecurity() when mounting the client to know if a LSM module (selinux) required a xattr to store file contexts. If a xattr is returned we store it and use it for in request security context. For getxattr/setxattr we use the stored LSM's xattr to filter xattr security contexts like security.selinux. If xattr does not match the stored xattr name we returned -EOPNOTSUPP to userspace. It adds also the s_security check for security_inode_notifysecctx() to avoid calling this function if selinux is disabled (as in nfs_setsecurity()). For "Enforcing SELinux Policy Check" functionnality, the selinux check have been moved in l_getsepol: -ENODEV is returned if selinux is disabled. Add a regresion test "sanity test_434" for this use case. *Note:* This patch detects that selinux is disabled without explicitly disabled it in kernel cmdline. This is recommended for RHEL >= 8.5. *Performances:* Tests with "strace -c ls -l" with 100000 files on root in a multi VMs env (on Rocky 9). FS is remount for each tests (cache is cleaned) and selinux is disabled. __________________ ___________ _________ | Total time % | lgetxattr | statx | |__________________|___________|_________| |Without the patch:| 29% | 51% | |__________________|___________|_________| |With the patch: | 0% | 87% | |__________________|___________|_________| "ls -l" uses lgetxattr to get "security.selinux". Linux-commit: 3d252529480c68bfd6a6774652df7c8968b28e41 Fixes: 39e5bfa ("LU-12355 llite: include file linux/selinux.h removed") Fixes: 9bcac0b ("LU-549 llite: Improve statfs performance if selinux is disabled") Test-Parameters: clientselinux=false clientdistro=el7.9 testlist=sanity env=ONLY=434,ONLY_REPEAT=20 Test-Parameters: clientselinux=false clientdistro=el8.5 testlist=sanity env=ONLY=434,ONLY_REPEAT=20 Test-Parameters: clientselinux=false clientdistro=el8.6 testlist=sanity env=ONLY=434,ONLY_REPEAT=20 Test-Parameters: clientselinux clientdistro=el8.6 testlist=sanity-selinux Test-Parameters: clientselinux clientdistro=el8.6 testlist=sanity-selinux Test-Parameters: clientselinux clientdistro=el7.9 testlist=sanity-selinux Test-Parameters: clientselinux clientdistro=el7.9 testlist=sanity-selinux Signed-off-by: Etienne AUJAMES Change-Id: I4dac87ac0341b45a1c2fef836cdce0361017b3f5 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/48875 Reviewed-by: Andreas Dilger Reviewed-by: Sebastien Buisson Reviewed-by: Oleg Drokin Tested-by: jenkins Tested-by: Maloo --- lustre/autoconf/lustre-core.m4 | 23 ----- lustre/include/lustre_compat.h | 9 +- lustre/llite/dir.c | 20 ++--- lustre/llite/llite_internal.h | 37 ++++++-- lustre/llite/llite_lib.c | 12 +++ lustre/llite/namei.c | 94 +++++++------------- lustre/llite/xattr.c | 16 ++-- lustre/llite/xattr_cache.c | 6 +- lustre/llite/xattr_security.c | 196 ++++++++++++++++++++++++++++++++--------- lustre/ptlrpc/sec.c | 8 +- lustre/tests/sanity.sh | 35 ++++++++ lustre/utils/l_getsepol.c | 20 ++++- 12 files changed, 314 insertions(+), 162 deletions(-) diff --git a/lustre/autoconf/lustre-core.m4 b/lustre/autoconf/lustre-core.m4 index 99a30be..8ccc779 100644 --- a/lustre/autoconf/lustre-core.m4 +++ b/lustre/autoconf/lustre-core.m4 @@ -2459,28 +2459,6 @@ EXTRA_KCFLAGS="$tmp_flags" ]) # LC_HAVE_SUNRPC_CACHE_HASH_LOCK_IS_A_SPINLOCK # -# LC_HAS_LINUX_SELINUX_ENABLED -# -# kernel 5.1 commit 3d252529480c68bfd6a6774652df7c8968b28e41 -# SELinux: Remove unused selinux_is_enabled -# -AC_DEFUN([LC_HAS_LINUX_SELINUX_ENABLED], [ -tmp_flags="$EXTRA_KCFLAGS" -EXTRA_KCFLAGS="-Werror" -LB_CHECK_COMPILE([if linux/selinux.h exists], -selinux_is_enabled, [ - #include -],[ - bool has_selinux = selinux_is_enabled(); - (void)has_selinux; -],[ - AC_DEFINE(HAVE_LINUX_SELINUX_IS_ENABLED, 1, - [if linux/selinux.h exists]) -]) -EXTRA_KCFLAGS="$tmp_flags" -]) # LC_HAS_LINUX_SELINUX_ENABLED - -# # LB_HAVE_BVEC_ITER_ALL # # kernel 5.1 commit 6dc4f100c175dd0511ae8674786e7c9006cdfbfa @@ -2999,7 +2977,6 @@ AC_DEFUN([LC_PROG_LINUX], [ LC_HAVE_SUNRPC_CACHE_HASH_LOCK_IS_A_SPINLOCK # 5.1 - LC_HAS_LINUX_SELINUX_ENABLED LB_HAVE_BVEC_ITER_ALL # 5.2 diff --git a/lustre/include/lustre_compat.h b/lustre/include/lustre_compat.h index e8c6128..aca622a 100644 --- a/lustre/include/lustre_compat.h +++ b/lustre/include/lustre_compat.h @@ -446,9 +446,14 @@ static inline struct timespec current_time(struct inode *inode) kmem_cache_create(name, size, align, flags, ctor) #endif -#ifndef HAVE_LINUX_SELINUX_IS_ENABLED -#define selinux_is_enabled() 1 +static inline bool ll_security_xattr_wanted(struct inode *in) +{ +#ifdef CONFIG_SECURITY + return in->i_security && in->i_sb->s_security; +#else + return false; #endif +} static inline int ll_vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name, diff --git a/lustre/llite/dir.c b/lustre/llite/dir.c index b692661..869fc9f 100644 --- a/lustre/llite/dir.c +++ b/lustre/llite/dir.c @@ -458,6 +458,7 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump, .hash = ll_full_name_hash(dparent, dirname, strlen(dirname)), }, + .d_sb = dparent->d_sb, }; bool encrypt = false; int hash_flags; @@ -539,9 +540,9 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump, /* selinux_dentry_init_security() uses dentry->d_parent and name * to determine the security context for the file. So our fake * dentry should be real enough for this purpose. */ - err = ll_dentry_init_security(parent, - &dentry, mode, &dentry.d_name, + err = ll_dentry_init_security(&dentry, mode, &dentry.d_name, &op_data->op_file_secctx_name, + &op_data->op_file_secctx_name_size, &op_data->op_file_secctx, &op_data->op_file_secctx_size); if (err < 0) @@ -573,17 +574,12 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump, dentry.d_inode = inode; - if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) { - /* no need to protect selinux_inode_setsecurity() by - * inode_lock. Taking it would lead to a client deadlock - * LU-13617 - */ - err = security_inode_notifysecctx(inode, - op_data->op_file_secctx, - op_data->op_file_secctx_size); - } else { + if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) + err = ll_inode_notifysecctx(inode, op_data->op_file_secctx, + op_data->op_file_secctx_size); + else err = ll_inode_init_security(&dentry, inode, parent); - } + if (err) GOTO(out_inode, err); diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h index 6a1161f..45ab7b1 100644 --- a/lustre/llite/llite_internal.h +++ b/lustre/llite/llite_internal.h @@ -471,15 +471,36 @@ static inline void obd_connect_set_secctx(struct obd_connect_data *data) #endif } -int ll_dentry_init_security(struct inode *parent, struct dentry *dentry, - int mode, struct qstr *name, - const char **secctx_name, void **secctx, - __u32 *secctx_size); +/* Only smack and selinux is known to use security contexts */ +static inline bool ll_xattr_is_seclabel(const char *name) +{ + return !strcmp(name, XATTR_NAME_SELINUX) || + !strcmp(name, XATTR_NAME_SMACK); +} + +static inline bool ll_xattr_suffix_is_seclabel(const char *suffix) +{ + return !strcmp(suffix, XATTR_SELINUX_SUFFIX) || + !strcmp(suffix, XATTR_SMACK_SUFFIX); +} + +int ll_dentry_init_security(struct dentry *dentry, int mode, struct qstr *name, + const char **secctx_name, __u32 *secctx_name_size, + void **secctx, __u32 *secctx_size); int ll_inode_init_security(struct dentry *dentry, struct inode *inode, struct inode *dir); -int ll_listsecurity(struct inode *inode, char *secctx_name, - size_t secctx_name_size); +int ll_inode_notifysecctx(struct inode *inode, + void *secctx, __u32 secctxlen); + +void ll_secctx_name_free(struct ll_sb_info *sbi); + +int ll_secctx_name_store(struct inode *in); + +__u32 ll_secctx_name_get(struct ll_sb_info *sbi, const char **secctx_name); + +int ll_security_secctx_name_filter(struct ll_sb_info *sbi, int xattr_type, + const char *suffix); static inline bool obd_connect_has_enc(struct obd_connect_data *data) { @@ -814,6 +835,10 @@ struct ll_sb_info { struct ll_foreign_symlink_upcall_item *ll_foreign_symlink_upcall_items; /* foreign symlink path upcall nb infos */ unsigned int ll_foreign_symlink_upcall_nb_items; + + /* cached file security context xattr name. e.g: security.selinux */ + char *ll_secctx_name; + __u32 ll_secctx_name_size; }; #define SBI_DEFAULT_HEAT_DECAY_WEIGHT ((80 * 256 + 50) / 100) diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index e097c33..e67ab23 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -147,6 +147,9 @@ static struct ll_sb_info *ll_init_sbi(void) * not enabled by default */ + sbi->ll_secctx_name = NULL; + sbi->ll_secctx_name_size = 0; + sbi->ll_ra_info.ra_max_pages = min(pages / 32, SBI_DEFAULT_READ_AHEAD_MAX); sbi->ll_ra_info.ra_max_pages_per_file = @@ -257,6 +260,9 @@ static void ll_free_sbi(struct super_block *sb) sizeof(struct ll_foreign_symlink_upcall_item)); sbi->ll_foreign_symlink_upcall_items = NULL; } + if (sbi->ll_secctx_name) + ll_secctx_name_free(sbi); + ll_free_rw_stats_info(sbi); pcc_super_fini(&sbi->ll_pcc_super); OBD_FREE(sbi, sizeof(*sbi)); @@ -726,6 +732,12 @@ retry_connect: GOTO(out_root, err); } + err = ll_secctx_name_store(root); + if (err < 0 && ll_security_xattr_wanted(root)) + CWARN("%s: file security contextes not supported: rc = %d\n", + sbi->ll_fsname, err); + + err = 0; if (encctxlen) { CDEBUG(D_SEC, "server returned encryption ctx for root inode "DFID"\n", diff --git a/lustre/llite/namei.c b/lustre/llite/namei.c index 9d2e37e..549b887 100644 --- a/lustre/llite/namei.c +++ b/lustre/llite/namei.c @@ -734,19 +734,8 @@ static int ll_lookup_it_finish(struct ptlrpc_request *request, PFID(ll_inode2fid(inode))); } - if (secctx != NULL && secctxlen != 0) { - /* no need to protect selinux_inode_setsecurity() by - * inode_lock. Taking it would lead to a client deadlock - * LU-13617 - */ - rc = security_inode_notifysecctx(inode, secctx, - secctxlen); - if (rc) - CWARN("%s: cannot set security context for "DFID": rc = %d\n", - ll_i2sbi(inode)->ll_fsname, - PFID(ll_inode2fid(inode)), - rc); - } + /* resume normally on error */ + ll_inode_notifysecctx(inode, secctx, secctxlen); } /* Only hash *de if it is unhashed (new dentry). @@ -832,14 +821,14 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry, struct ptlrpc_request *req = NULL; struct md_op_data *op_data = NULL; struct lov_user_md *lum = NULL; + struct ll_sb_info *sbi = ll_i2sbi(parent); __u32 opc; int rc; - char secctx_name[XATTR_NAME_MAX + 1]; struct llcrypt_name fname; struct lu_fid fid; ENTRY; - if (dentry->d_name.len > ll_i2sbi(parent)->ll_namelen) + if (dentry->d_name.len > sbi->ll_namelen) RETURN(ERR_PTR(-ENAMETOOLONG)); CDEBUG(D_VFSTRACE, "VFS Op:name=%pd, dir="DFID"(%p), intent=%s\n", @@ -902,11 +891,11 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry, it->it_create_mode &= ~current_umask(); if (it->it_op & IT_CREAT && - test_bit(LL_SBI_FILE_SECCTX, ll_i2sbi(parent)->ll_flags)) { - rc = ll_dentry_init_security(parent, - dentry, it->it_create_mode, + test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) { + rc = ll_dentry_init_security(dentry, it->it_create_mode, &dentry->d_name, &op_data->op_file_secctx_name, + &op_data->op_file_secctx_name_size, &op_data->op_file_secctx, &op_data->op_file_secctx_size); if (rc < 0) @@ -1002,22 +991,12 @@ inherit: *encctxlen = 0; } - /* ask for security context upon intent */ - if (it->it_op & (IT_LOOKUP | IT_GETATTR | IT_OPEN)) { - /* get name of security xattr to request to server */ - rc = ll_listsecurity(parent, secctx_name, - sizeof(secctx_name)); - if (rc < 0) { - CDEBUG(D_SEC, "cannot get security xattr name for " - DFID": rc = %d\n", - PFID(ll_inode2fid(parent)), rc); - } else if (rc > 0) { - op_data->op_file_secctx_name = secctx_name; - op_data->op_file_secctx_name_size = rc; - CDEBUG(D_SEC, "'%.*s' is security xattr for "DFID"\n", - rc, secctx_name, PFID(ll_inode2fid(parent))); - } - } + /* ask for security context upon intent: + * get name of security xattr to request to server + */ + if (it->it_op & (IT_LOOKUP | IT_GETATTR | IT_OPEN)) + op_data->op_file_secctx_name_size = + ll_secctx_name_get(sbi, &op_data->op_file_secctx_name); if (pca && pca->pca_dataset) { OBD_ALLOC_PTR(lum); @@ -1435,19 +1414,13 @@ static int ll_create_it(struct inode *dir, struct dentry *dentry, if (IS_ERR(inode)) RETURN(PTR_ERR(inode)); - if (test_bit(LL_SBI_FILE_SECCTX, ll_i2sbi(inode)->ll_flags) && - secctx) { - /* must be done before d_instantiate, because it calls - * security_d_instantiate, which means a getxattr if security - * context is not set yet */ - /* no need to protect selinux_inode_setsecurity() by - * inode_lock. Taking it would lead to a client deadlock - * LU-13617 - */ - rc = security_inode_notifysecctx(inode, secctx, secctxlen); - if (rc) - RETURN(rc); - } + /* must be done before d_instantiate, because it calls + * security_d_instantiate, which means a getxattr if security + * context is not set yet + */ + rc = ll_inode_notifysecctx(inode, secctx, secctxlen); + if (rc) + RETURN(rc); d_instantiate(dentry, inode); @@ -1585,9 +1558,9 @@ again: ll_qos_mkdir_prep(op_data, dir); if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) { - err = ll_dentry_init_security(dir, - dchild, mode, &dchild->d_name, + err = ll_dentry_init_security(dchild, mode, &dchild->d_name, &op_data->op_file_secctx_name, + &op_data->op_file_secctx_name_size, &op_data->op_file_secctx, &op_data->op_file_secctx_size); if (err < 0) @@ -1727,20 +1700,15 @@ again: if (err) GOTO(err_exit, err); - if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) { - /* must be done before d_instantiate, because it calls - * security_d_instantiate, which means a getxattr if security - * context is not set yet */ - /* no need to protect selinux_inode_setsecurity() by - * inode_lock. Taking it would lead to a client deadlock - * LU-13617 - */ - err = security_inode_notifysecctx(inode, - op_data->op_file_secctx, - op_data->op_file_secctx_size); - if (err) - GOTO(err_exit, err); - } + /* must be done before d_instantiate, because it calls + * security_d_instantiate, which means a getxattr if security + * context is not set yet + */ + err = ll_inode_notifysecctx(inode, + op_data->op_file_secctx, + op_data->op_file_secctx_size); + if (err) + GOTO(err_exit, err); d_instantiate(dchild, inode); diff --git a/lustre/llite/xattr.c b/lustre/llite/xattr.c index 27157ba3..9231650 100644 --- a/lustre/llite/xattr.c +++ b/lustre/llite/xattr.c @@ -138,10 +138,9 @@ static int ll_xattr_set_common(const struct xattr_handler *handler, (handler->flags == XATTR_LUSTRE_T && !strcmp(name, "lov")))) RETURN(0); - /* LU-549: Disable security.selinux when selinux is disabled */ - if (handler->flags == XATTR_SECURITY_T && !selinux_is_enabled() && - strcmp(name, "selinux") == 0) - RETURN(-EOPNOTSUPP); + rc = ll_security_secctx_name_filter(sbi, handler->flags, name); + if (rc) + RETURN(rc); /* * In user.* namespace, only regular files and directories can have @@ -398,7 +397,7 @@ int ll_xattr_list(struct inode *inode, const char *name, int type, void *buffer, GOTO(out_xattr, rc = -EPERM); if (sbi->ll_xattr_cache_enabled && type != XATTR_ACL_ACCESS_T && - (type != XATTR_SECURITY_T || strcmp(name, "security.selinux")) && + (type != XATTR_SECURITY_T || !ll_xattr_is_seclabel(name)) && (type != XATTR_TRUSTED_T || strcmp(name, XATTR_NAME_SOM))) { rc = ll_xattr_cache_get(inode, name, buffer, size, valid); if (rc == -EAGAIN) @@ -471,10 +470,9 @@ static int ll_xattr_get_common(const struct xattr_handler *handler, if (rc) RETURN(rc); - /* LU-549: Disable security.selinux when selinux is disabled */ - if (handler->flags == XATTR_SECURITY_T && !selinux_is_enabled() && - !strcmp(name, "selinux")) - RETURN(-EOPNOTSUPP); + rc = ll_security_secctx_name_filter(sbi, handler->flags, name); + if (rc) + RETURN(rc); #ifdef CONFIG_LUSTRE_FS_POSIX_ACL /* posix acl is under protection of LOOKUP lock. when calling to this, diff --git a/lustre/llite/xattr_cache.c b/lustre/llite/xattr_cache.c index 2064ba7..0a75174 100644 --- a/lustre/llite/xattr_cache.c +++ b/lustre/llite/xattr_cache.c @@ -505,9 +505,9 @@ static int ll_xattr_cache_refill(struct inode *inode) CDEBUG(D_CACHE, "not caching %s\n", XATTR_NAME_ACL_ACCESS); rc = 0; - } else if (!strcmp(xdata, "security.selinux")) { - /* Filter out security.selinux, it is cached in slab */ - CDEBUG(D_CACHE, "not caching security.selinux\n"); + } else if (ll_xattr_is_seclabel(xdata)) { + /* Filter out security label, it is cached in slab */ + CDEBUG(D_CACHE, "not caching %s\n", xdata); rc = 0; } else if (!strcmp(xdata, XATTR_NAME_SOM)) { /* Filter out trusted.som, it is not cached on client */ diff --git a/lustre/llite/xattr_security.c b/lustre/llite/xattr_security.c index 9bcb800..57e1d7a 100644 --- a/lustre/llite/xattr_security.c +++ b/lustre/llite/xattr_security.c @@ -50,19 +50,20 @@ /* * Check for LL_SBI_FILE_SECCTX before calling. */ -int ll_dentry_init_security(struct inode *parent, struct dentry *dentry, - int mode, struct qstr *name, - const char **secctx_name, void **secctx, - __u32 *secctx_size) +int ll_dentry_init_security(struct dentry *dentry, int mode, struct qstr *name, + const char **secctx_name, __u32 *secctx_name_size, + void **secctx, __u32 *secctx_size) { + struct ll_sb_info *sbi = ll_s2sbi(dentry->d_sb); +#ifdef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG + const char *secctx_name_lsm = NULL; +#endif int rc; /* - * security_dentry_init_security() is strange. Like - * security_inode_init_security() it may return a context (provided a - * Linux security module is enabled) but unlike - * security_inode_init_security() it does not return to us the name of - * the extended attribute to store the context under (for example + * Before kernel 5.15-rc1-20-g15bf32398ad4, + * security_inode_init_security() does not return to us the name of the + * extended attribute to store the context under (for example * "security.selinux"). So we only call it when we think we know what * the name of the extended attribute will be. This is OK-ish since * SELinux is the only module that implements @@ -71,36 +72,28 @@ int ll_dentry_init_security(struct inode *parent, struct dentry *dentry, * from SELinux. */ - if (!selinux_is_enabled()) + *secctx_name_size = ll_secctx_name_get(sbi, secctx_name); + /* xattr name length == 0 means no LSM module manage file contexts */ + if (*secctx_name_size == 0) return 0; - /* fetch length of security xattr name */ - rc = security_inode_listsecurity(parent, NULL, 0); - /* xattr name length == 0 means SELinux is disabled */ - if (rc == 0) - return 0; - /* we support SELinux only */ - if (rc != strlen(XATTR_NAME_SELINUX) + 1) - return -EOPNOTSUPP; - rc = security_dentry_init_security(dentry, mode, name, #ifdef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG - secctx_name, + &secctx_name_lsm, #endif secctx, secctx_size); - /* Usually, security_dentry_init_security() returns -EOPNOTSUPP when - * SELinux is disabled. - * But on some kernels (e.g. rhel 8.5) it returns 0 when SELinux is - * disabled, and in this case the security context is empty. - */ - if (rc == -EOPNOTSUPP || (rc == 0 && *secctx_size == 0)) - /* do nothing */ + /* ignore error if the hook is not supported by the LSM module */ + if (rc == -EOPNOTSUPP) return 0; if (rc < 0) return rc; -#ifndef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG - *secctx_name = XATTR_NAME_SELINUX; +#ifdef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG + if (strncmp(*secctx_name, secctx_name_lsm, *secctx_name_size) != 0) { + CERROR("%s: LSM secctx_name '%s' does not match the one stored by Lustre '%s'\n", + sbi->ll_fsname, secctx_name_lsm, *secctx_name); + return -EOPNOTSUPP; + } #endif return 0; @@ -161,7 +154,7 @@ ll_inode_init_security(struct dentry *dentry, struct inode *inode, { int rc; - if (!selinux_is_enabled()) + if (!ll_security_xattr_wanted(dir)) return 0; rc = security_inode_init_security(inode, dir, NULL, @@ -173,23 +166,146 @@ ll_inode_init_security(struct dentry *dentry, struct inode *inode, } /** - * Get security context xattr name used by policy. + * Notify security context to the security layer + * + * Notify security context @secctx of inode @inode to the security layer. * - * \retval >= 0 length of xattr name - * \retval < 0 failure to get security context xattr name + * \retval 0 success, or SELinux is disabled or not supported by the fs + * \retval < 0 failure to set the security context */ -int -ll_listsecurity(struct inode *inode, char *secctx_name, size_t secctx_name_size) +int ll_inode_notifysecctx(struct inode *inode, + void *secctx, __u32 secctxlen) { + struct ll_sb_info *sbi = ll_i2sbi(inode); int rc; - if (!selinux_is_enabled()) + if (!test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags) || + !ll_security_xattr_wanted(inode) || + !secctx || !secctxlen) return 0; - rc = security_inode_listsecurity(inode, secctx_name, secctx_name_size); - if (rc >= secctx_name_size) + /* no need to protect selinux_inode_setsecurity() by + * inode_lock. Taking it would lead to a client deadlock + * LU-13617 + */ + rc = security_inode_notifysecctx(inode, secctx, secctxlen); + if (rc) + CWARN("%s: cannot set security context for "DFID": rc = %d\n", + sbi->ll_fsname, PFID(ll_inode2fid(inode)), rc); + + return rc; +} + +/** + * Free the security context xattr name used by policy + */ +void ll_secctx_name_free(struct ll_sb_info *sbi) +{ + OBD_FREE(sbi->ll_secctx_name, sbi->ll_secctx_name_size + 1); + sbi->ll_secctx_name = NULL; + sbi->ll_secctx_name_size = 0; +} + +/** + * Get security context xattr name used by policy and save it. + * + * \retval > 0 length of xattr name + * \retval == 0 no LSM module registered supporting security contexts + * \retval <= 0 failure to get xattr name or xattr is not supported + */ +int ll_secctx_name_store(struct inode *in) +{ + struct ll_sb_info *sbi = ll_i2sbi(in); + int rc = 0; + + if (!ll_security_xattr_wanted(in)) + return 0; + + /* get size of xattr name */ + rc = security_inode_listsecurity(in, NULL, 0); + if (rc <= 0) + return rc; + + if (sbi->ll_secctx_name) + ll_secctx_name_free(sbi); + + OBD_ALLOC(sbi->ll_secctx_name, rc + 1); + if (!sbi->ll_secctx_name) + return -ENOMEM; + + /* save the xattr name */ + sbi->ll_secctx_name_size = rc; + rc = security_inode_listsecurity(in, sbi->ll_secctx_name, + sbi->ll_secctx_name_size); + if (rc <= 0) + goto err_free; + + if (rc > sbi->ll_secctx_name_size) { rc = -ERANGE; - else if (rc >= 0) - secctx_name[rc] = '\0'; + goto err_free; + } + + /* sanity check */ + sbi->ll_secctx_name[rc] = '\0'; + if (rc < sizeof(XATTR_SECURITY_PREFIX)) { + rc = -EINVAL; + goto err_free; + } + if (strncmp(sbi->ll_secctx_name, XATTR_SECURITY_PREFIX, + sizeof(XATTR_SECURITY_PREFIX) - 1) != 0) { + rc = -EOPNOTSUPP; + goto err_free; + } + + return rc; + +err_free: + ll_secctx_name_free(sbi); return rc; } + +/** + * Retrieved file security context xattr name stored. + * + * \retval security context xattr name size stored. + * \retval 0 no xattr name stored. + */ +__u32 ll_secctx_name_get(struct ll_sb_info *sbi, const char **secctx_name) +{ + if (!sbi->ll_secctx_name || !sbi->ll_secctx_name_size) + return 0; + + *secctx_name = sbi->ll_secctx_name; + + return sbi->ll_secctx_name_size; +} + +/** + * Filter out xattr file security context if not managed by LSM + * + * This is done to improve performance for application that blindly try to get + * file context (like "ls -l" for security.linux). + * See LU-549 for more information. + * + * \retval 0 xattr not filtered + * \retval -EOPNOTSUPP no enabled LSM security module supports the xattr + */ +int ll_security_secctx_name_filter(struct ll_sb_info *sbi, int xattr_type, + const char *suffix) +{ + const char *cached_suffix = NULL; + + if (xattr_type != XATTR_SECURITY_T || + !ll_xattr_suffix_is_seclabel(suffix)) + return 0; + + /* is the xattr label used by lsm ? */ + if (!ll_secctx_name_get(sbi, &cached_suffix)) + return -EOPNOTSUPP; + + cached_suffix += sizeof(XATTR_SECURITY_PREFIX) - 1; + if (strcmp(suffix, cached_suffix) != 0) + return -EOPNOTSUPP; + + return 0; +} diff --git a/lustre/ptlrpc/sec.c b/lustre/ptlrpc/sec.c index c308b69..f6b9ace 100644 --- a/lustre/ptlrpc/sec.c +++ b/lustre/ptlrpc/sec.c @@ -1829,7 +1829,7 @@ static inline int sptlrpc_sepol_needs_check(struct ptlrpc_sec *imp_sec) { ktime_t checknext; - if (send_sepol == 0 || !selinux_is_enabled()) + if (send_sepol == 0) return 0; if (send_sepol == -1) @@ -1875,7 +1875,7 @@ int sptlrpc_get_sepol(struct ptlrpc_request *req) RETURN(0); #endif - if (send_sepol == 0 || !selinux_is_enabled()) + if (send_sepol == 0) RETURN(0); if (imp_sec == NULL) @@ -1889,6 +1889,10 @@ int sptlrpc_get_sepol(struct ptlrpc_request *req) memcpy(req->rq_sepol, imp_sec->ps_sepol, sizeof(req->rq_sepol)); spin_unlock(&imp_sec->ps_lock); + } else if (rc == -ENODEV) { + CDEBUG(D_SEC, + "Client cannot report SELinux status, SELinux is disabled.\n"); + rc = 0; } RETURN(rc); diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 900de19..cbe3909 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -27622,6 +27622,41 @@ test_433() { } run_test 433 "ldlm lock cancel releases dentries and inodes" +test_434() { + local file + local getxattr_count + local mdc_stat_param="mdc.$FSNAME-MDT0000*.md_stats" + local p="$TMP/$TESTSUITE-$TESTNAME.parameters" + + [[ $(getenforce) == "Disabled" ]] || + skip "lsm selinux module have to be disabled for this test" + + test_mkdir -i 0 -c1 $DIR/$tdir/ || + error "fail to create $DIR/$tdir/ on MDT0000" + + touch $DIR/$tdir/$tfile-{001..100} + + # disable the xattr cache + save_lustre_params client "llite.*.xattr_cache" > $p + lctl set_param llite.*.xattr_cache=0 + stack_trap "restore_lustre_params < $p; rm -f $p" EXIT + + # clear clients mdc stats + clear_stats $mdc_stat_param || + error "fail to clear stats on mdc MDT0000" + + for file in $DIR/$tdir/$tfile-{001..100}; do + getfattr -n security.selinux $file |& + grep -q "Operation not supported" || + error "getxattr on security.selinux should return EOPNOTSUPP" + done + + getxattr_count=$(calc_stats $mdc_stat_param "getxattr") + (( getxattr_count < 100 )) || + error "client sent $getxattr_count getxattr RPCs to the MDS" +} +run_test 434 "Client should not send RPCs for security.selinux with SElinux disabled" + prep_801() { [[ $MDS1_VERSION -lt $(version_code 2.9.55) ]] || [[ $OST1_VERSION -lt $(version_code 2.9.55) ]] && diff --git a/lustre/utils/l_getsepol.c b/lustre/utils/l_getsepol.c index a3e07c4..990d016 100644 --- a/lustre/utils/l_getsepol.c +++ b/lustre/utils/l_getsepol.c @@ -69,6 +69,8 @@ static void errlog(const char *fmt, ...) va_start(args, fmt); vsyslog(LOG_NOTICE, fmt, args); + if (isatty(STDIN_FILENO)) + vfprintf(stderr, fmt, args); va_end(args); closelog(); @@ -205,7 +207,7 @@ int get_opts(int argc, char *const argv[]) ref_pol_mtime = (time_t)strtoul(sel_mtime, &res, 0); if (*res != '\0') { /* not a valid number */ - errlog("invalid sel_mtime"); + errlog("invalid sel_mtime\n"); return -EINVAL; } } @@ -214,7 +216,7 @@ int get_opts(int argc, char *const argv[]) ref_selinux_mode = sel_mode[0] - '0'; if (ref_selinux_mode != 0 && ref_selinux_mode != 1) { /* not a valid enforcing mode */ - errlog("invalid sel_mode"); + errlog("invalid sel_mode\n"); return -EINVAL; } } @@ -331,6 +333,7 @@ int main(int argc, char **argv) struct stat st; time_t policymtime = 0; int enforce; + int is_selinux; char *policy_type = NULL; unsigned char *mdval = NULL; unsigned int mdsize = 0; @@ -342,6 +345,19 @@ int main(int argc, char **argv) if (rc < 0) goto out; + is_selinux = is_selinux_enabled(); + if (is_selinux < 0) { + errlog("is_selinux_enabled() failed\n"); + rc = -errno; + goto out; + } + + if (!is_selinux) { + errlog("SELinux is disabled, ptlrpc 'send_sepol' value should be set to 0\n"); + rc = -ENODEV; + goto out; + } + /* Max version of loaded policy */ policyver = security_policyvers(); if (policyver < 0) { -- 1.8.3.1