Whamcloud - gitweb
LU-14401 sec: fix migrate for encrypted dir 13/41413/8
authorSebastien Buisson <sbuisson@ddn.com>
Thu, 4 Feb 2021 08:22:56 +0000 (17:22 +0900)
committerOleg Drokin <green@whamcloud.com>
Sat, 13 Mar 2021 18:34:02 +0000 (18:34 +0000)
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 <sbuisson@ddn.com>
Change-Id: I2466ea35a871c6c07bdcf9fba7191485e855e655
Reviewed-on: https://review.whamcloud.com/41413
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/llite/crypto.c
lustre/llite/file.c
lustre/llite/llite_internal.h
lustre/llite/xattr.c
lustre/mdd/mdd_internal.h
lustre/tests/sanity-sec.sh

index 6662759..7e743c8 100644 (file)
@@ -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;
                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)
        rc = ll_vfs_setxattr(dentry, inode, LL_XATTR_NAME_ENCRYPTION_CONTEXT,
                             ctx, len, XATTR_CREATE);
        if (rc)
index 5974934..449f895 100644 (file)
@@ -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_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))
        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))
index 4e52cbd..8b3d527 100644 (file)
@@ -406,6 +406,8 @@ enum ll_file_flags {
        LLIF_UPDATE_ATIME       = 4,
        /* foreign file/dir can be unlinked unconditionnaly */
        LLIF_FOREIGN_REMOVABLE  = 5,
        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,
 
 };
 
 
 };
 
index e23c47f..0a395bc 100644 (file)
@@ -148,6 +148,17 @@ static int ll_xattr_set_common(const struct xattr_handler *handler,
                        RETURN(-EPERM);
        }
 
                        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);
        fullname = kasprintf(GFP_KERNEL, "%s%s", xattr_prefix(handler), name);
        if (!fullname)
                RETURN(-ENOMEM);
index 6095259..842abe3 100644 (file)
@@ -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 (!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)) ||
        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 };
 
             (rc = mdd_dir_is_empty(env, obj)) == 0)) {
                struct lu_attr la = { 0 };
 
index 160f186..956a222 100755 (executable)
@@ -3575,12 +3575,13 @@ trace_cmd() {
        local cmd="$@"
        local xattr_name="security.c"
 
        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
        $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"
 
        $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 $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"
 
 }
 run_test 49 "Avoid getxattr for encryption context"
 
@@ -4170,6 +4179,41 @@ test_56() {
 }
 run_test 56 "FIEMAP on encrypted file"
 
 }
 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() {
 log "cleanup: ======================================================"
 
 sec_unsetup() {