LU-4101 mdt: protect internal xattrs

In mdt_reint_setxattr() require CAP_SYS_ADMIN to modify a trusted
xattr and silently disallow modification those trusted xattrs used by
Lustre internally.

Signed-off-by: John L. Hammond <john.hammond@intel.com>
Change-Id: Ic616dca74a90da0aedb0ec2624618f91ac6fcaf4
Reviewed-on: http://review.whamcloud.com/7943
Reviewed-by: Andrew Perepechko <andrew_perepechko@xyratex.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>

diff --git a/lustre/mdt/mdt_xattr.c b/lustre/mdt/mdt_xattr.c
index a89013d..dfb7676 100644 (file)
--- a/lustre/mdt/mdt_xattr.c
+++ b/lustre/mdt/mdt_xattr.c
@@ -398,29 +398,39 @@ int mdt_reint_setxattr(struct mdt_thread_info *info,
 
                perm = mdt_identity_get_perm(uc->uc_identity, remote,
                                             req->rq_peer.nid);
-                if (!(perm & CFS_RMTACL_PERM))
-                        GOTO(out, rc = err_serious(-EPERM));
-        }
+               if (!(perm & CFS_RMTACL_PERM))
+                       GOTO(out, rc = err_serious(-EPERM));
+       }
 
-        if (strncmp(xattr_name, XATTR_USER_PREFIX,
-                    sizeof(XATTR_USER_PREFIX) - 1) == 0) {
+       if (strncmp(xattr_name, XATTR_USER_PREFIX,
+                   sizeof(XATTR_USER_PREFIX) - 1) == 0) {
                if (!(exp_connect_flags(req->rq_export) & OBD_CONNECT_XATTR))
                        GOTO(out, rc = -EOPNOTSUPP);
-                if (strcmp(xattr_name, XATTR_NAME_LOV) == 0)
-                        GOTO(out, rc = -EACCES);
-                if (strcmp(xattr_name, XATTR_NAME_LMA) == 0)
-                        GOTO(out, rc = 0);
-                if (strcmp(xattr_name, XATTR_NAME_LINK) == 0)
-                        GOTO(out, rc = 0);
-        } else if ((valid & OBD_MD_FLXATTR) &&
-                   (strncmp(xattr_name, XATTR_NAME_ACL_ACCESS,
-                            sizeof(XATTR_NAME_ACL_ACCESS) - 1) == 0 ||
-                    strncmp(xattr_name, XATTR_NAME_ACL_DEFAULT,
-                            sizeof(XATTR_NAME_ACL_DEFAULT) - 1) == 0)) {
-                /* currently lustre limit acl access size */
-                if (xattr_len > LUSTRE_POSIX_ACL_MAX_SIZE)
-                        GOTO(out, -ERANGE);
-        }
+       } else if (strncmp(xattr_name, XATTR_TRUSTED_PREFIX,
+                   sizeof(XATTR_TRUSTED_PREFIX) - 1) == 0) {
+
+               if (!md_capable(mdt_ucred(info), CFS_CAP_SYS_ADMIN))
+                       GOTO(out, rc = -EPERM);
+
+               if (strcmp(xattr_name, XATTR_NAME_LOV) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_LMA) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_LMV) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_LINK) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_FID) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_VERSION) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_SOM) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_HSM) == 0 ||
+                   strcmp(xattr_name, XATTR_NAME_LFSCK_NAMESPACE) == 0)
+                       GOTO(out, rc = 0);
+       } else if ((valid & OBD_MD_FLXATTR) &&
+                  (strncmp(xattr_name, XATTR_NAME_ACL_ACCESS,
+                           sizeof(XATTR_NAME_ACL_ACCESS) - 1) == 0 ||
+                   strncmp(xattr_name, XATTR_NAME_ACL_DEFAULT,
+                           sizeof(XATTR_NAME_ACL_DEFAULT) - 1) == 0)) {
+               /* currently lustre limit acl access size */
+               if (xattr_len > LUSTRE_POSIX_ACL_MAX_SIZE)
+                       GOTO(out, rc = -ERANGE);
+       }
 
         lockpart = MDS_INODELOCK_UPDATE;
         /* Revoke all clients' lookup lock, since the access

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index b4eea62..8d8ab01 100644 (file)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -6411,6 +6411,75 @@ run_test 102m "Ensure listxattr fails on small bufffer ========"
 
 cleanup_test102
 
+getxattr() { # getxattr path name
+       # Return the base64 encoding of the value of xattr name on path.
+       local path=$1
+       local name=$2
+
+       # # getfattr --absolute-names --encoding=base64 --name=trusted.lov $path
+       # file: $path
+       # trusted.lov=0s0AvRCwEAAAAGAAAAAAAAAAAEAAACAAAAAAAQAAEAA...AAAAAAAAA=
+       #
+       # We print just 0s0AvRCwEAAAAGAAAAAAAAAAAEAAACAAAAAAAQAAEAA...AAAAAAAAA=
+
+       getfattr --absolute-names --encoding=base64 --name=$name $path |
+               awk -F= -v name=$name '$1 == name {
+                       print substr($0, index($0, "=") + 1);
+       }'
+}
+
+test_102n() { # LU-4101 mdt: protect internal xattrs
+       local file0=$DIR/$tfile.0
+       local file1=$DIR/$tfile.1
+       local xattr0=$TMP/$tfile.0
+       local xattr1=$TMP/$tfile.1
+       local name
+       local value
+
+       if [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.5.50) ]
+       then
+               skip "MDT < 2.5.50 allows setxattr on internal trusted xattrs"
+               return
+       fi
+
+       rm -rf $file0 $file1 $xattr0 $xattr1
+       touch $file0 $file1
+
+       # Get 'before' xattrs of $file1.
+       getfattr --absolute-names --dump --match=- $file1 > $xattr0
+
+       for name in lov lma lmv link fid version som hsm lfsck_namespace; do
+               # Try to copy xattr from $file0 to $file1.
+               value=$(getxattr $file0 trusted.$name 2> /dev/null)
+
+               setfattr --name=trusted.$name --value="$value" $file1 ||
+                       error "setxattr 'trusted.$name' failed"
+
+               # Try to set a garbage xattr.
+               value=0sVGhlIHF1aWNrIGJyb3duIGZveCBqdW1wcyBvdmVyIGl0c2VsZi4=
+
+               setfattr --name=trusted.$name --value="$value" $file1 ||
+                       error "setxattr 'trusted.$name' failed"
+
+               # Try to remove the xattr from $file1. We don't care if this
+               # appears to succeed or fail, we just don't want there to be
+               # any changes or crashes.
+               setfattr --remove=$trusted.$name $file1 2> /dev/null
+       done
+
+       # Get 'after' xattrs of file1.
+       getfattr --absolute-names --dump --match=- $file1 > $xattr1
+
+       if ! diff $xattr0 $xattr1; then
+               error "before and after xattrs of '$file1' differ"
+       fi
+
+       rm -rf $file0 $file1 $xattr0 $xattr1
+
+       return 0
+}
+run_test 102n "silently ignore setxattr on internal trusted xattrs"
+
 run_acl_subtest()
 {
     $LUSTRE/tests/acl/run $LUSTRE/tests/acl/$1.test