Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/lib/libperfuse - fix access control: pcn->pcn_cred is not us...



details:   https://anonhg.NetBSD.org/src/rev/f6e9fcd56271
branches:  trunk
changeset: 757984:f6e9fcd56271
user:      manu <manu%NetBSD.org@localhost>
date:      Mon Oct 11 01:08:26 2010 +0000

description:
- fix access control: pcn->pcn_cred is not user credentials
- Keep track of file generation
- remove size tracking in pnd_size, we have it in pn_va.va_size

diffstat:

 lib/libperfuse/ops.c          |  251 ++++++++++++++++++++++-------------------
 lib/libperfuse/perfuse_priv.h |    5 +-
 lib/libperfuse/subr.c         |    3 +-
 3 files changed, 137 insertions(+), 122 deletions(-)

diffs (truncated from 610 to 300 lines):

diff -r 8304f8504575 -r f6e9fcd56271 lib/libperfuse/ops.c
--- a/lib/libperfuse/ops.c      Sun Oct 10 21:27:16 2010 +0000
+++ b/lib/libperfuse/ops.c      Mon Oct 11 01:08:26 2010 +0000
@@ -1,4 +1,4 @@
-/*  $NetBSD: ops.c,v 1.20 2010/10/04 03:56:24 manu Exp $ */
+/*  $NetBSD: ops.c,v 1.21 2010/10/11 01:08:26 manu Exp $ */
 
 /*-
  *  Copyright (c) 2010 Emmanuel Dreyfus. All rights reserved.
@@ -217,9 +217,16 @@
        if (pcr == NULL)
                return 0;
 
+       /*
+        * pcr is NULL for self open through fsync or readdir.
+        * In both case, access control is useless, as it was
+        * done before, at open time.
+        */
+       if (pcr == NULL)
+               return 0;
+
        pn = (struct puffs_node *)opc;
        va = puffs_pn_getvap(pn);
-
        return puffs_access(va->va_type, va->va_mode, 
                            va->va_uid, va->va_gid,
                            mode, pcr);
@@ -252,13 +259,13 @@
        vap->va_flags = 0;
        vap->va_rdev = fa->rdev;
        vap->va_bytes = fa->size;
-       vap->va_filerev = 0;
+       vap->va_filerev = (u_quad_t)PUFFS_VNOVAL;
        vap->va_vaflags = 0;
 
        if (vap->va_blocksize == 0)
                vap->va_blocksize = DEV_BSIZE;
 
-       if (vap->va_size == (size_t)-1) /* XXX */
+       if (vap->va_size == (size_t)PUFFS_VNOVAL) /* XXX */
                vap->va_size = 0;
 
        return;
@@ -369,6 +376,7 @@
        PERFUSE_NODE_DATA(pn)->pnd_ino = feo->nodeid;
 
        fuse_attr_to_vap(ps, &pn->pn_va, &feo->attr);
+       pn->pn_va.va_gen = (u_long)(feo->generation);
 
        if (pnp != NULL)
                *pnp = pn;
@@ -419,6 +427,8 @@
        PERFUSE_NODE_DATA(pn)->pnd_ino = feo->nodeid;
 
        fuse_attr_to_vap(ps, &pn->pn_va, &feo->attr);
+       pn->pn_va.va_gen = (u_long)(feo->generation);
+
        puffs_newinfo_setcookie(pni, pn);
 
 #ifdef PERFUSE_DEBUG
@@ -927,7 +937,7 @@
 {
        struct puffs_node *pn;
        int error;
-       
+
        error = 0;
 
        /*
@@ -980,11 +990,14 @@
        if (PERFUSE_NODE_DATA(opc)->pnd_flags & PND_REMOVED)
                return ENOENT;
 
+#if notyet
        /*
-        * Create an object require -WX permission in the parent directory
+        * XXX create needs -WX on the parent directory. No pcr is
+        * given here, we cannot enforce this.
         */
-       if (no_access(opc, pcn->pcn_cred, PUFFS_VWRITE|PUFFS_VEXEC))
+       if (no_access(opc, pcr, PUFFS_VWRITE|PUFFS_VEXEC))
                return EACCES;
+#endif
 
        /*
         * If create is unimplemented: Check that it does not
@@ -1004,9 +1017,22 @@
                if (error != 0) 
                        return error;
 
+               /*
+                * FUSE does the open at create time, while
+                * NetBSD will open in a subsequent operation.
+                * We need to open now, in order to retain FUSE
+                * semantics, but we have no credentials to use
+                * since the PUFFS interface gives no pcr here. 
+                *
+                * open with NULL pcr will skip permission checks.
+                * There is no security hole, since we know we
+                * can open this file: we just created it. The 
+                * calling process will not get a file descriptor
+                * before the kernel sends the open operation, 
+                * which will have a pcr, anyway.
+                */
                opc = (puffs_cookie_t)pn;
-
-               error = perfuse_node_open(pu, opc, FWRITE, pcn->pcn_cred);
+               error = perfuse_node_open(pu, opc, FWRITE, NULL);
                if (error != 0) 
                        return error;
 
@@ -1023,7 +1049,7 @@
         * 
         * mode must contain file type (ie: S_IFREG), use VTTOIF(vap->va_type)
         */
-       pm = ps->ps_new_msg(pu, opc, FUSE_CREATE, len, pcn->pcn_cred);
+       pm = ps->ps_new_msg(pu, opc, FUSE_CREATE, len, NULL);
        fci = GET_INPAYLOAD(ps, pm, fuse_create_in);
        fci->flags = O_CREAT | O_TRUNC | O_RDWR;
        fci->mode = vap->va_mode | VTTOIF(vap->va_type);
@@ -1048,6 +1074,8 @@
        PERFUSE_NODE_DATA(pn)->pnd_ino = feo->nodeid;
 
        fuse_attr_to_vap(ps, &pn->pn_va, &feo->attr);
+       pn->pn_va.va_gen = (u_long)(feo->generation);
+               
        puffs_newinfo_setcookie(pni, pn);
 
        /*
@@ -1101,19 +1129,25 @@
         * directories, files, socks, fifo and links.
         *
         * Create an object require -WX permission in the parent directory
+        *
+        * XXX The PUFFS interface gives us no pcr here, we cannot perfom 
+        * access control.
         */
        switch (vap->va_type) {
        case VDIR:      /* FALLTHROUGH */
        case VREG:      /* FALLTHROUGH */
        case VFIFO:     /* FALLTHROUGH */
        case VSOCK:     /* FALLTHROUGH */
-       case VLNK:
-               if (no_access(opc, pcn->pcn_cred, PUFFS_VWRITE|PUFFS_VEXEC))
+#if notyet
+               if (no_access(opc, pcr, PUFFS_VWRITE|PUFFS_VEXEC))
                        return EACCES;
+#endif
                break;
        default:        /* VNON, VBLK, VCHR, VBAD */
-               if (!puffs_cred_isjuggernaut(pcn->pcn_cred))
+#if notyet
+               if (!puffs_cred_isjuggernaut(pcr))
                        return EACCES;
+#endif
                break;
        }
 
@@ -1125,7 +1159,7 @@
        /*      
         * mode must contain file type (ie: S_IFREG), use VTTOIF(vap->va_type)
         */
-       pm = ps->ps_new_msg(pu, opc, FUSE_MKNOD, len, pcn->pcn_cred);
+       pm = ps->ps_new_msg(pu, opc, FUSE_MKNOD, len, NULL);
        fmi = GET_INPAYLOAD(ps, pm, fuse_mknod_in);
        fmi->mode = vap->va_mode | VTTOIF(vap->va_type);
        fmi->rdev = (uint32_t)vap->va_rdev;
@@ -1165,7 +1199,7 @@
 
        if (puffs_pn_getvap(pn)->va_type == VDIR) {
                op = FUSE_OPENDIR;
-               pmode = PUFFS_VREAD|PUFFS_VEXEC;
+               pmode = PUFFS_VREAD;
        } else {
                op = FUSE_OPEN;
                if (mode & FWRITE)
@@ -1175,11 +1209,11 @@
        }
 
        /*
-        * Opening a directory require R-X on the directory
+        * Opening a directory require R-- on the directory
         * Opening a file requires R-- for reading, -W- for writing
-        * In both cases, --X is required on the parent.
+        * In both cases, R-- is required on the parent.
         */
-       if ((no_access((puffs_cookie_t)pnd->pnd_parent, pcr, PUFFS_VEXEC)) ||
+       if ((no_access((puffs_cookie_t)pnd->pnd_parent, pcr, PUFFS_VREAD)) ||
            (no_access(opc, pcr, pmode))) {
                error = EACCES;
                goto out;
@@ -1273,6 +1307,12 @@
        return 0;
 }
 
+/*
+ * XXX
+ * This fails as unprivilegied, it should not: touch testa/testx/a
+ * d-wx-wx-wx  2 root  wheel  512 Oct  5 04:32 testa/testx
+ * -rwxrwxrwx  1 root  wheel  0   Oct  5 04:39 testa/testx/a
+ */
 int
 perfuse_node_access(pu, opc, mode, pcr)
        struct puffs_usermount *pu;
@@ -1340,14 +1380,11 @@
                error = puffs_access(VREG, fao->attr.mode, fao->attr.uid,
                                     fao->attr.gid, (mode_t)mode, pcr); 
 
-               /*
-                * While we are there, grab size if unknown.
-                */
-               if (!(PERFUSE_NODE_DATA(opc)->pnd_flags & PND_GOTSIZE)) {
-                       PERFUSE_NODE_DATA(opc)->pnd_size = fao->attr.size;
-                       PERFUSE_NODE_DATA(opc)->pnd_flags |= PND_GOTSIZE;
-               }
                
+               fuse_attr_to_vap(ps,
+                                &((struct puffs_node *)opc)->pn_va,
+                                &fao->attr);
+
                ps->ps_destroy_msg(pm);
        }
 
@@ -1372,10 +1409,10 @@
                return ENOENT;
 
        /*
-        * getattr requires --X on the parent directory
+        * getattr requires --X on the parent directory 
+        * no right is required on the object.
         */
-       if (no_access((puffs_cookie_t)PERFUSE_NODE_DATA(opc)->pnd_parent,
-           pcr, PUFFS_VEXEC))
+       if (no_access(PERFUSE_NODE_DATA(opc)->pnd_parent, pcr, PUFFS_VEXEC))
                return EACCES;
 
        ps = puffs_getspecific(pu);
@@ -1410,13 +1447,6 @@
         */
        fuse_attr_to_vap(ps, vap, &fao->attr);
 
-       /*
-        * While we are there, grab size if unknown.
-        */
-       if (!(PERFUSE_NODE_DATA(opc)->pnd_flags & PND_GOTSIZE)) {
-               PERFUSE_NODE_DATA(opc)->pnd_size = fao->attr.size;
-               PERFUSE_NODE_DATA(opc)->pnd_flags |= PND_GOTSIZE;
-       }
 out:
        ps->ps_destroy_msg(pm);
 
@@ -1456,12 +1486,6 @@
                goto out;
        }
 
-       /*
-        * setattr requires --X on the parent directory
-        */
-       if (no_access((puffs_cookie_t)pnd->pnd_parent, pcr, PUFFS_VEXEC))
-               return EACCES;
-
        old_va = puffs_pn_getvap((struct puffs_node *)opc);
 
        /*
@@ -1524,19 +1548,33 @@
                fsi->valid |= FUSE_FATTR_SIZE;
        }
 
-       if (vap->va_atime.tv_sec != (time_t)PUFFS_VNOVAL) {
-               fsi->atime = vap->va_atime.tv_sec;
-               fsi->atimensec = (uint32_t)vap->va_atime.tv_nsec;;
-               fsi->valid |= FUSE_FATTR_ATIME;
+       /*
+        * Setting mtime without atime or vice verse leads to
+        * dates being reset to Epoch on glusterfs. If one
+        * is missing, use the old value.
+        */
+       if ((vap->va_mtime.tv_sec != (time_t)PUFFS_VNOVAL) || 
+           (vap->va_atime.tv_sec != (time_t)PUFFS_VNOVAL)) {
+               
+               if (vap->va_atime.tv_sec != (time_t)PUFFS_VNOVAL) {
+                       fsi->atime = vap->va_atime.tv_sec;
+                       fsi->atimensec = (uint32_t)vap->va_atime.tv_nsec;
+               } else {
+                       fsi->atime = old_va->va_atime.tv_sec;
+                       fsi->atimensec = (uint32_t)old_va->va_atime.tv_nsec;
+               }
+
+               if (vap->va_mtime.tv_sec != (time_t)PUFFS_VNOVAL) {
+                       fsi->mtime = vap->va_mtime.tv_sec;
+                       fsi->mtimensec = (uint32_t)vap->va_mtime.tv_nsec;
+               } else {
+                       fsi->mtime = old_va->va_mtime.tv_sec;
+                       fsi->mtimensec = (uint32_t)old_va->va_mtime.tv_nsec;
+               }
+
+               fsi->valid |= (FUSE_FATTR_MTIME|FUSE_FATTR_ATIME);
        }
 
-       if (vap->va_mtime.tv_sec != (time_t)PUFFS_VNOVAL) {



Home | Main Index | Thread Index | Old Index