Source-Changes-HG archive

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

[src/trunk]: src/lib/libperfuse - Implement proper unprivilegied user permiss...



details:   https://anonhg.NetBSD.org/src/rev/b3fcd125fe2f
branches:  trunk
changeset: 764478:b3fcd125fe2f
user:      manu <manu%NetBSD.org@localhost>
date:      Mon Apr 25 04:54:53 2011 +0000

description:
- Implement proper unprivilegied user permission verifications
Verification is now done in the lookup method, as it is the way to
go. Of course there are corner cases, such as the sticky bit which
need special handling in the remove method.

- Set full fsidx in vftstat method

- Do not pass O_APPEND to the filesystem. FUSE always sends the
write offset, so setting O_APPEND is useless. If the filesystem
uses it in an open(2) system call, it will even cause file
corruptions, since offsets given to pwrite(2) will be ignored.
This fix allows glusterfs to host a NetBSD ./build.sh -o build

- Do not use the FUSE access method, use getattr and check for
permission on our own. The problem is that a FUSE filesystem will
typically use the Linux-specific setfsuid() to perform access
control. If that is missing, any chack is likely to occur on
behalf of the user running the filesystem (typically root), causing
access method to return wrong information.

- When possible, avoid performing a getattr method call and use
cached value in puffs_node instead. We still retreive the latest
value by calling getattr when performing append write operation,
to minimize the chances that another writer appended since the
last time we did.

- Update puffs_node cached file size in write method

- Remove unused argument to perfuse_destroy_pn()

diffstat:

 lib/libperfuse/ops.c          |  438 +++++++++++++++++++----------------------
 lib/libperfuse/perfuse.c      |   72 +++++-
 lib/libperfuse/perfuse_priv.h |    8 +-
 lib/libperfuse/subr.c         |    7 +-
 4 files changed, 272 insertions(+), 253 deletions(-)

diffs (truncated from 1008 to 300 lines):

diff -r d82fd494ad5c -r b3fcd125fe2f lib/libperfuse/ops.c
--- a/lib/libperfuse/ops.c      Mon Apr 25 04:30:59 2011 +0000
+++ b/lib/libperfuse/ops.c      Mon Apr 25 04:54:53 2011 +0000
@@ -1,7 +1,7 @@
-/*  $NetBSD: ops.c,v 1.23 2010/10/11 05:37:58 manu Exp $ */
+/*  $NetBSD: ops.c,v 1.24 2011/04/25 04:54:53 manu Exp $ */
 
 /*-
- *  Copyright (c) 2010 Emmanuel Dreyfus. All rights reserved.
+ *  Copyright (c) 2010-2011 Emmanuel Dreyfus. All rights reserved.
  * 
  *  Redistribution and use in source and binary forms, with or without
  *  modification, are permitted provided that the following conditions
@@ -34,7 +34,7 @@
 #include <sysexits.h>
 #include <syslog.h>
 #include <puffs.h>
-#include <sys/vnode.h>
+#include <sys/socket.h>
 #include <sys/socket.h>
 #include <machine/vmparam.h>
 
@@ -45,7 +45,8 @@
 
 static int xchg_msg(struct puffs_usermount *, puffs_cookie_t, 
     perfuse_msg_t *, size_t, enum perfuse_xchg_pb_reply); 
-static int no_access(puffs_cookie_t, const struct puffs_cred *, mode_t);
+static int mode_access(puffs_cookie_t, const struct puffs_cred *, mode_t);
+static int sticky_access(struct puffs_node *, const struct puffs_cred *);
 static void fuse_attr_to_vap(struct perfuse_state *,
     struct vattr *, struct fuse_attr *);
 static int node_lookup_dir_nodot(struct puffs_usermount *,
@@ -54,6 +55,8 @@
     const char*, struct puffs_node **);
 static int node_mk_common(struct puffs_usermount *, puffs_cookie_t,
     struct puffs_newinfo *, const struct puffs_cn *pcn, perfuse_msg_t *);
+static int node_mk_common_final(struct puffs_usermount *, puffs_cookie_t,
+    struct puffs_node *, const struct puffs_cn *pcn);
 static ssize_t fuse_to_dirent(struct puffs_usermount *, puffs_cookie_t,
     struct fuse_dirent *, size_t);
 static int readdir_buffered(puffs_cookie_t, struct dirent *, off_t *, 
@@ -201,7 +204,7 @@
 }
 
 static int
-no_access(opc, pcr, mode)
+mode_access(opc, pcr, mode)
        puffs_cookie_t opc;
        const struct puffs_cred *pcr;
        mode_t mode;
@@ -217,14 +220,6 @@
        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, 
@@ -232,6 +227,30 @@
                            mode, pcr);
 }
 
+static int 
+sticky_access(targ, pcr)
+       struct puffs_node *targ;
+       const struct puffs_cred *pcr;
+{
+       uid_t uid;
+       struct puffs_node *tdir;
+       int sticky, owner, error;
+
+       tdir = PERFUSE_NODE_DATA(targ)->pnd_parent;
+
+       if ((error = puffs_cred_getuid(pcr, &uid)) != 0)
+               return error;
+
+       sticky = puffs_pn_getvap(tdir)->va_mode & S_ISTXT;
+       owner = puffs_pn_getvap(targ)->va_uid == uid;
+
+       if (sticky && !owner)
+               error = EACCES;
+
+       return error;   
+}
+
+
 static void
 fuse_attr_to_vap(ps, vap, fa)
        struct perfuse_state *ps;
@@ -239,7 +258,7 @@
        struct fuse_attr *fa;
 {
        vap->va_type = IFTOVT(fa->mode);
-       vap->va_mode = fa->mode;
+       vap->va_mode = fa->mode & ALLPERMS;
        vap->va_nlink = fa->nlink;
        vap->va_uid = fa->uid;
        vap->va_gid = fa->gid;
@@ -395,7 +414,7 @@
 
 
 /*
- * Common final code for methods that create objects:
+ * Common code for methods that create objects:
  * perfuse_node_mkdir
  * perfuse_node_mknod
  * perfuse_node_symlink
@@ -411,7 +430,6 @@
        struct perfuse_state *ps;
        struct puffs_node *pn;
        struct fuse_entry_out *feo;
-       struct fuse_setattr_in *fsi;
        int error;
 
        ps =  puffs_getspecific(pu);
@@ -440,6 +458,36 @@
 #endif
        ps->ps_destroy_msg(pm);
 
+       return node_mk_common_final(pu, opc, pn, pcn);
+
+out:
+       ps->ps_destroy_msg(pm);
+
+       return error;
+}
+
+/*
+ * Common final code for methods that create objects:
+ * perfuse_node_mkdir via node_mk_common
+ * perfuse_node_mknod via node_mk_common
+ * perfuse_node_symlink via node_mk_common
+ * perfuse_node_create
+ */
+static int
+node_mk_common_final(pu, opc, pn, pcn)
+       struct puffs_usermount *pu;
+       puffs_cookie_t opc;
+       struct puffs_node *pn;
+       const struct puffs_cn *pcn;
+{
+       struct perfuse_state *ps;
+       perfuse_msg_t *pm;
+       struct fuse_setattr_in *fsi;
+       struct fuse_attr_out *fao;
+       int error;
+
+       ps =  puffs_getspecific(pu);
+
        /* 
         * Set owner and group
         */
@@ -453,11 +501,12 @@
        fsi->gid = pn->pn_va.va_gid;
        fsi->valid = FUSE_FATTR_UID|FUSE_FATTR_GID;
 
-       /*
-        * A fuse_attr_out is returned, but we ignore it.
-        */
-       error = xchg_msg(pu, (puffs_cookie_t)pn, 
-                        pm, sizeof(struct fuse_attr_out), wait_reply);
+       if ((error = xchg_msg(pu, (puffs_cookie_t)pn, pm, 
+                             sizeof(*fao), wait_reply)) != 0)
+               goto out;
+
+       fao = GET_OUTPAYLOAD(ps, pm, fuse_attr_out);
+       fuse_attr_to_vap(ps, &pn->pn_va, &fao->attr);
 
        /*
         * The parent directory needs a sync
@@ -465,11 +514,13 @@
        PERFUSE_NODE_DATA(opc)->pnd_flags |= PND_DIRTY;
 
 out:
-       ps->ps_destroy_msg(pm);
+       if (pm != NULL)
+               ps->ps_destroy_msg(pm);
 
        return error;
 }
 
+
 static ssize_t
 fuse_to_dirent(pu, opc, fd, fd_len)
        struct puffs_usermount *pu;
@@ -840,9 +891,8 @@
        svfsb->f_asyncreads = ps->ps_asyncreads;
        svfsb->f_asyncwrites = ps->ps_asyncwrites;
 
-       svfsb->f_fsidx.__fsid_val[0] = (int32_t)ps->ps_fsid;
-       svfsb->f_fsidx.__fsid_val[1] = 0;
-       svfsb->f_fsid = ps->ps_fsid;
+       (void)memcpy(&svfsb->f_fsidx, &ps->ps_fsid, sizeof(ps->ps_fsid));
+       svfsb->f_fsid = (unsigned long)ps->ps_fsid;
        svfsb->f_namemax = MAXPATHLEN;  /* XXX */
        svfsb->f_owner = ps->ps_owner_uid;
 
@@ -936,9 +986,26 @@
        const struct puffs_cn *pcn;
 {
        struct puffs_node *pn;
+       mode_t mode;
        int error;
 
-       error = 0;
+       /*
+        * Check permissions
+        */
+       switch(pcn->pcn_nameiop) {
+       case NAMEI_DELETE: /* FALLTHROUGH */
+       case NAMEI_RENAME: /* FALLTHROUGH */
+       case NAMEI_CREATE:
+               mode = PUFFS_VEXEC|PUFFS_VWRITE;
+               break;
+       case NAMEI_LOOKUP: /* FALLTHROUGH */
+       default:
+               mode = PUFFS_VEXEC;
+               break;
+       }
+
+       if ((error = mode_access(opc, pcn->pcn_cred, mode)) != 0) 
+               return error;
 
        /*
         * Special case for ..
@@ -948,14 +1015,40 @@
        else
                error = node_lookup_common(pu, (puffs_cookie_t)opc,
                                           pcn->pcn_name, &pn);
-
        if (error != 0)
                return error;
 
+       /*
+        * Removed node
+        */
        if (PERFUSE_NODE_DATA(pn)->pnd_flags & PND_REMOVED)
                return ENOENT;
 
        /*
+        * Check for sticky bit. Unfortunately there is no way to 
+        * do this before creating the puffs_node, since we require
+        * this operation to get the node owner.
+        */
+       switch (pcn->pcn_nameiop) {
+       case NAMEI_DELETE: /* FALLTHROUGH */
+       case NAMEI_RENAME:
+               error = sticky_access(pn, pcn->pcn_cred);
+               if (error != 0) {
+                       /* 
+                        * kernel will never know about it and will
+                        * not reclaim it. The filesystem needs to
+                        * clean it up anyway, therefore mimick a forget.
+                        */
+                       PERFUSE_NODE_DATA(pn)->pnd_flags |= PND_RECLAIMED;
+                       (void)perfuse_node_reclaim(pu, (puffs_cookie_t)pn);
+                       return error;
+               }
+               break;
+       default:
+               break;
+       }
+
+       /*
         * If that node had a pending reclaim, wipe it out.
         */
        PERFUSE_NODE_DATA(pn)->pnd_flags &= ~PND_RECLAIMED;
@@ -990,15 +1083,6 @@
        if (PERFUSE_NODE_DATA(opc)->pnd_flags & PND_REMOVED)
                return ENOENT;
 
-#if notyet
-       /*
-        * XXX create needs -WX on the parent directory. No pcr is
-        * given here, we cannot enforce this.
-        */
-       if (no_access(opc, pcr, PUFFS_VWRITE|PUFFS_VEXEC))
-               return EACCES;
-#endif
-
        /*
         * If create is unimplemented: Check that it does not
         * already exists, and if not, do mknod and open
@@ -1021,18 +1105,12 @@
                 * 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 



Home | Main Index | Thread Index | Old Index