[src/trunk]: src Redo the access check for setting va_flags in zfs_netbsd_set...

[hannken <hannken%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/61865109fc60
branches:  trunk
changeset: 446235:61865109fc60
user:      hannken <hannken%NetBSD.org@localhost>
date:      Wed Nov 28 10:01:28 2018 +0000

description:
Redo the access check for setting va_flags in zfs_netbsd_setattr().

Use user flag UF_NODUMP instead of UF_IMMUTABLE for the test as it
is the only user flag supported by all tested file systems.

PR kern/47656 test zfs_flags.

diffstat:

 external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c |  51 +++++++----------
 tests/fs/vfs/t_unpriv.c                                |  10 +--
 2 files changed, 26 insertions(+), 35 deletions(-)

diffs (124 lines):

diff -r 5674bdfe0814 -r 61865109fc60 external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c
--- a/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c    Wed Nov 28 09:58:58 2018 +0000
+++ b/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c    Wed Nov 28 10:01:28 2018 +0000
@@ -5345,9 +5345,11 @@
        cred_t *cred = ap->a_cred;
        znode_t *zp = VTOZ(vp);
        xvattr_t xvap;
-       u_long fflags;
+       kauth_action_t action;
+       u_long fflags, sfflags = 0;
        uint64_t zflags;
        int error, flags = 0;
+       bool changing_sysflags;
 
        vattr_init_mask(vap);
        vap->va_mask &= ~AT_NOSET;
@@ -5365,40 +5367,14 @@
                fflags = vap->va_flags;
                if ((fflags & ~(SF_IMMUTABLE|SF_APPEND|SF_NOUNLINK|UF_NODUMP)) != 0)
                        return (EOPNOTSUPP);
-               /*
-                * Callers may only modify the file flags on objects they
-                * have VADMIN rights for.
-                */
-               if ((error = VOP_ACCESS(vp, VWRITE, cred)) != 0)
-                       return (error);
-               /*
-                * Unprivileged processes are not permitted to unset system
-                * flags, or modify flags if any system flags are set.
-                * Privileged non-jail processes may not modify system flags
-                * if securelevel > 0 and any existing system flags are set.
-                * Privileged jail processes behave like privileged non-jail
-                * processes if the security.jail.chflags_allowed sysctl is
-                * is non-zero; otherwise, they behave like unprivileged
-                * processes.
-                */
-               if (kauth_authorize_system(cred, KAUTH_SYSTEM_CHSYSFLAGS, 0,
-                       NULL, NULL, NULL) != 0) {
-
-                       if (zflags &
-                           (ZFS_IMMUTABLE | ZFS_APPENDONLY | ZFS_NOUNLINK)) {
-                               return (EPERM);
-                       }
-                       if (fflags &
-                           (SF_IMMUTABLE | SF_APPEND | SF_NOUNLINK)) {
-                               return (EPERM);
-                       }
-               }
 
 #define        FLAG_CHANGE(fflag, zflag, xflag, xfield)        do {            \
        if (((fflags & (fflag)) && !(zflags & (zflag))) ||              \
            ((zflags & (zflag)) && !(fflags & (fflag)))) {              \
                XVA_SET_REQ(&xvap, (xflag));                            \
                (xfield) = ((fflags & (fflag)) != 0);                   \
+               if (((fflag) & SF_SETTABLE) != 0)                       \
+                       sfflags |= (fflag);                             \
        }                                                               \
 } while (0)
                /* Convert chflags into ZFS-type flags. */
@@ -5412,6 +5388,23 @@
                FLAG_CHANGE(UF_NODUMP, ZFS_NODUMP, XAT_NODUMP,
                    xvap.xva_xoptattrs.xoa_nodump);
 #undef FLAG_CHANGE
+
+               action = KAUTH_VNODE_WRITE_FLAGS;
+               changing_sysflags = false;
+
+               if (zflags & (ZFS_IMMUTABLE|ZFS_APPENDONLY|ZFS_NOUNLINK)) {
+                       action |= KAUTH_VNODE_HAS_SYSFLAGS;
+               }
+               if (sfflags != 0) {
+                       action |= KAUTH_VNODE_WRITE_SYSFLAGS;
+                       changing_sysflags = true;
+               }
+
+               error = kauth_authorize_vnode(cred, action, vp, NULL,
+                   genfs_can_chflags(cred, vp->v_type, zp->z_uid,
+                   changing_sysflags));
+               if (error)
+                       return error;
        }
 
        if (vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL ||
diff -r 5674bdfe0814 -r 61865109fc60 tests/fs/vfs/t_unpriv.c
--- a/tests/fs/vfs/t_unpriv.c   Wed Nov 28 09:58:58 2018 +0000
+++ b/tests/fs/vfs/t_unpriv.c   Wed Nov 28 10:01:28 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: t_unpriv.c,v 1.15 2018/11/28 09:58:58 hannken Exp $    */
+/*     $NetBSD: t_unpriv.c,v 1.16 2018/11/28 10:01:28 hannken Exp $    */
 
 /*-
  * Copyright (c) 2011 The NetBSD Foundation, Inc.
@@ -187,20 +187,18 @@
 
        if (rump_sys_stat(name, &st) == -1)
                atf_tc_fail_errno("stat");
-       if (FSTYPE_ZFS(tc))
-               atf_tc_expect_fail("PR kern/47656: Test known to be broken");
        if (rump_sys_chflags(name, st.st_flags) == -1) {
                if (errno == EOPNOTSUPP)
                        atf_tc_skip("file flags not supported by file system");
                atf_tc_fail_errno("chflags");
        }
 
-       fflags = st.st_flags | UF_IMMUTABLE;
+       fflags = st.st_flags | UF_NODUMP;
 
        rump_pub_lwproc_rfork(RUMP_RFCFDG);
        if (rump_sys_setuid(1) == -1)
                atf_tc_fail_errno("setuid");
-       fflags |= UF_IMMUTABLE;
+       fflags |= UF_NODUMP;
        if (rump_sys_chflags(name, fflags) != -1 || errno != EPERM)
                atf_tc_fail_errno("chflags");
        rump_pub_lwproc_releaselwp();
@@ -208,7 +206,7 @@
        if (rump_sys_chflags(name, fflags) == -1)
                atf_tc_fail_errno("chflags");
 
-       fflags &= ~UF_IMMUTABLE;
+       fflags &= ~UF_NODUMP;
        if (rump_sys_chflags(name, fflags) == -1)
                atf_tc_fail_errno("chflags");