From: Sebastien Buisson Date: Thu, 4 Feb 2021 08:22:56 +0000 (+0900) Subject: LU-14401 sec: fix migrate for encrypted dir X-Git-Tag: 2.14.51~33 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=67c4cffac6dbd30ce30e1d3132b65d4e4a374dda LU-14401 sec: fix migrate for encrypted dir When setting an encryption policy on a directory that we want to be encrypted, we need to make sure it is empty. But, in some cases, setting the LL_XATTR_NAME_ENCRYPTION_CONTEXT xattr should be allowed on non-empty directories, for instance when a directory is migrated across MDTs into new shard directories. Also, it is required for the encrpytion key to be available on the client when migrating a directory so that the filenames can be properly rehashed for the new MDT directory shard. And, in any case, we need to prevent explicit setting of LL_XATTR_NAME_ENCRYPTION_CONTEXT xattr outside of encryption policy definition. Update sanity-sec test_49 to test migration of non-empty encrypted directory, and add sanity-sec test_57 to test security.c protection. Fixes: e8f74fb0f5 ("LU-12275 sec: verify dir is empty when setting enc policy") Signed-off-by: Sebastien Buisson Change-Id: I2466ea35a871c6c07bdcf9fba7191485e855e655 Reviewed-on: https://review.whamcloud.com/41413 Tested-by: jenkins Reviewed-by: Andreas Dilger Tested-by: Maloo Reviewed-by: John L. Hammond Reviewed-by: Oleg Drokin --- diff --git a/lustre/llite/crypto.c b/lustre/llite/crypto.c index 6662759..7e743c8 100644 --- a/lustre/llite/crypto.c +++ b/lustre/llite/crypto.c @@ -105,6 +105,7 @@ static int ll_set_context(struct inode *inode, const void *ctx, size_t len, return -EPERM; dentry = (struct dentry *)fs_data; + ll_file_set_flag(ll_i2info(inode), LLIF_SET_ENC_CTX); rc = ll_vfs_setxattr(dentry, inode, LL_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len, XATTR_CREATE); if (rc) diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 5974934..449f895 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -4558,6 +4558,17 @@ int ll_migrate(struct inode *parent, struct file *file, struct lmv_user_md *lum, if (is_root_inode(child_inode)) GOTO(out_iput, rc = -EINVAL); + if (IS_ENCRYPTED(child_inode)) { + rc = llcrypt_get_encryption_info(child_inode); + if (rc) + GOTO(out_iput, rc); + if (!llcrypt_has_encryption_key(child_inode)) { + CDEBUG(D_SEC, "no enc key for "DFID"\n", + PFID(ll_inode2fid(child_inode))); + GOTO(out_iput, rc = -ENOKEY); + } + } + op_data = ll_prep_md_op_data(NULL, parent, NULL, name, namelen, child_inode->i_mode, LUSTRE_OPC_ANY, NULL); if (IS_ERR(op_data)) diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h index 4e52cbd..8b3d527 100644 --- a/lustre/llite/llite_internal.h +++ b/lustre/llite/llite_internal.h @@ -406,6 +406,8 @@ enum ll_file_flags { LLIF_UPDATE_ATIME = 4, /* foreign file/dir can be unlinked unconditionnaly */ LLIF_FOREIGN_REMOVABLE = 5, + /* setting encryption context in progress */ + LLIF_SET_ENC_CTX = 6, }; diff --git a/lustre/llite/xattr.c b/lustre/llite/xattr.c index e23c47f..0a395bc 100644 --- a/lustre/llite/xattr.c +++ b/lustre/llite/xattr.c @@ -148,6 +148,17 @@ static int ll_xattr_set_common(const struct xattr_handler *handler, RETURN(-EPERM); } + /* Setting LL_XATTR_NAME_ENCRYPTION_CONTEXT xattr is only allowed + * when defining an encryption policy on a directory, ie when it + * comes from ll_set_context(). + * When new files/dirs are created in an encrypted dir, the xattr + * is set directly in the create request. + */ + if (handler->flags == XATTR_SECURITY_T && + !strcmp(name, "c") && + !ll_file_test_and_clear_flag(ll_i2info(inode), LLIF_SET_ENC_CTX)) + RETURN(-EPERM); + fullname = kasprintf(GFP_KERNEL, "%s%s", xattr_prefix(handler), name); if (!fullname) RETURN(-ENOMEM); diff --git a/lustre/mdd/mdd_internal.h b/lustre/mdd/mdd_internal.h index 6095259..842abe3 100644 --- a/lustre/mdd/mdd_internal.h +++ b/lustre/mdd/mdd_internal.h @@ -630,8 +630,19 @@ static inline int mdo_xattr_set(const struct lu_env *env,struct mdd_object *obj, if (!mdd_object_exists(obj)) return -ENOENT; + /* If we are about to set the LL_XATTR_NAME_ENCRYPTION_CONTEXT + * xattr, it means the file/dir is encrypted. In that case we want + * to set the LUSTRE_ENCRYPT_FL flag as well: it will be stored + * into the LMA, making it more efficient to recognise we are + * dealing with an encrypted file/dir, as LMA info is cached upon + * object init. + * However, marking a dir as encrypted is only possible if it is + * being created or migrated (LU_XATTR_CREATE flag not set), or + * if it is empty. + */ if ((strcmp(name, LL_XATTR_NAME_ENCRYPTION_CONTEXT) == 0) && (!S_ISDIR(mdd_object_type(obj)) || + !(fl & LU_XATTR_CREATE) || (rc = mdd_dir_is_empty(env, obj)) == 0)) { struct lu_attr la = { 0 }; diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh index 160f186..956a222 100755 --- a/lustre/tests/sanity-sec.sh +++ b/lustre/tests/sanity-sec.sh @@ -3575,12 +3575,13 @@ trace_cmd() { local cmd="$@" local xattr_name="security.c" - cancel_lru_locks osc ; cancel_lru_locks mdc + cancel_lru_locks $LCTL set_param debug=+info $LCTL clear echo $cmd eval $cmd + [ $? -eq 0 ] || error "$cmd failed" $LCTL dk | grep -E "get xattr '${xattr_name}'|get xattrs" [ $? -ne 0 ] || error "get xattr event was triggered" @@ -3608,21 +3609,29 @@ test_49() { trace_cmd $TRUNCATE $dirname/f1 10240 trace_cmd $LFS setstripe -E -1 -S 4M $dirname/f2 trace_cmd $LFS migrate -E -1 -S 256K $dirname/f2 - trace_cmd $LFS setdirstripe -i 1 $dirname/d2 - trace_cmd $LFS migrate -m 0 $dirname/d2 - $LFS setdirstripe -i 1 -c 1 $dirname/d3 - dirname=$dirname/d3/subdir - mkdir $dirname - - trace_cmd stat $dirname - trace_cmd touch $dirname/f1 - trace_cmd stat $dirname/f1 - trace_cmd cat $dirname/f1 - dd if=/dev/zero of=$dirname/f1 bs=1M count=10 conv=fsync - trace_cmd $TRUNCATE $dirname/f1 10240 - trace_cmd $LFS setstripe -E -1 -S 4M $dirname/f2 - trace_cmd $LFS migrate -E -1 -S 256K $dirname/f2 + if [[ $MDSCOUNT -gt 1 ]]; then + trace_cmd $LFS setdirstripe -i 1 $dirname/d2 + trace_cmd $LFS migrate -m 0 $dirname/d2 + touch $dirname/d2/subf + # migrate a non-empty encrypted dir + trace_cmd $LFS migrate -m 1 $dirname/d2 + + $LFS setdirstripe -i 1 -c 1 $dirname/d3 + dirname=$dirname/d3/subdir + mkdir $dirname + + trace_cmd stat $dirname + trace_cmd touch $dirname/f1 + trace_cmd stat $dirname/f1 + trace_cmd cat $dirname/f1 + dd if=/dev/zero of=$dirname/f1 bs=1M count=10 conv=fsync + trace_cmd $TRUNCATE $dirname/f1 10240 + trace_cmd $LFS setstripe -E -1 -S 4M $dirname/f2 + trace_cmd $LFS migrate -E -1 -S 256K $dirname/f2 + else + skip_noexit "2nd part needs >= 2 MDTs" + fi } run_test 49 "Avoid getxattr for encryption context" @@ -4170,6 +4179,41 @@ test_56() { } run_test 56 "FIEMAP on encrypted file" +test_57() { + local testdir=$DIR/$tdir/mytestdir + local testfile=$DIR/$tdir/$tfile + + [[ $(facet_fstype ost1) == zfs ]] && skip "skip ZFS backend" + + $LCTL get_param mdc.*.import | grep -q client_encryption || + skip "client encryption not supported" + + mount.lustre --help |& grep -q "test_dummy_encryption:" || + skip "need dummy encryption support" + + mkdir $DIR/$tdir + mkdir $testdir + setfattr -n security.c -v myval $testdir && + error "setting xattr on $testdir should have failed (1)" + touch $testfile + setfattr -n security.c -v myval $testfile && + error "setting xattr on $testfile should have failed (1)" + + rm -rf $DIR/$tdir + + stack_trap cleanup_for_enc_tests EXIT + setup_for_enc_tests + + mkdir $testdir + setfattr -n security.c -v myval $testdir && + error "setting xattr on $testdir should have failed (2)" + touch $testfile + setfattr -n security.c -v myval $testfile && + error "setting xattr on $testfile should have failed (2)" + return 0 +} +run_test 57 "security.c xattr protection" + log "cleanup: ======================================================" sec_unsetup() {