Source-Changes-HG archive

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

[src/trunk]: src Fix creds passed to FUSE when requests are done on behalf of...



details:   https://anonhg.NetBSD.org/src/rev/0fa32449c3ff
branches:  trunk
changeset: 767888:0fa32449c3ff
user:      manu <manu%NetBSD.org@localhost>
date:      Tue Aug 02 14:53:38 2011 +0000

description:
Fix creds passed to FUSE when requests are done on behalf of the kernel.
We previously sent uid/gid set to -1, we now set it to 0.

diffstat:

 lib/libperfuse/ops.c    |  102 ++++++++++++++++++++++++++++++++++++-----------
 usr.sbin/perfused/msg.c |   26 +++++++++--
 2 files changed, 98 insertions(+), 30 deletions(-)

diffs (truncated from 333 to 300 lines):

diff -r 953527dd2a7c -r 0fa32449c3ff lib/libperfuse/ops.c
--- a/lib/libperfuse/ops.c      Tue Aug 02 14:06:15 2011 +0000
+++ b/lib/libperfuse/ops.c      Tue Aug 02 14:53:38 2011 +0000
@@ -1,4 +1,4 @@
-/*  $NetBSD: ops.c,v 1.35 2011/07/19 07:29:39 manu Exp $ */
+/*  $NetBSD: ops.c,v 1.36 2011/08/02 14:53:38 manu Exp $ */
 
 /*-
  *  Copyright (c) 2010-2011 Emmanuel Dreyfus. All rights reserved.
@@ -53,7 +53,7 @@
 static int node_lookup_dir_nodot(struct puffs_usermount *,
     puffs_cookie_t, char *, size_t, struct puffs_node **);
 static int node_lookup_common(struct puffs_usermount *, puffs_cookie_t, 
-    const char*, struct puffs_node **);
+    const char *, const struct puffs_cred *, 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,
@@ -137,6 +137,11 @@
        /*
         * release_flags may be set to FUSE_RELEASE_FLUSH
         * to flush locks. lock_owner must be set in that case
+        *
+        * ps_new_msg() is called with NULL creds, which will
+        * be interpreted as FUSE superuser. We come here from the 
+        * inactive method, which provides no creds, but obviously      
+        * runs with kernel privilege.
         */
        pm = ps->ps_new_msg(pu, opc, op, sizeof(*fri), NULL);
        fri = GET_INPAYLOAD(ps, pm, fuse_release_in);
@@ -240,8 +245,20 @@
 
        tdir = PERFUSE_NODE_DATA(targ)->pnd_parent;
 
-       if ((error = puffs_cred_getuid(pcr, &uid)) != 0)
-               return error;
+       /*
+        * This covers the case where the kernel requests a DELETE
+        * or RENAME on its own, and where puffs_cred_getuid would 
+        * return -1. While such a situation should not happen, 
+        * we allow it here. 
+        *
+        * This also allows root to tamper with other users' files
+        * that have the sticky bit.
+        */
+       if (puffs_cred_isjuggernaut(pcr))
+               return 0;
+
+       if (puffs_cred_getuid(pcr, &uid) != 0)
+               DERRX(EX_SOFTWARE, "puffs_cred_getuid fails in %s", __func__);
 
        sticky = puffs_pn_getvap(tdir)->va_mode & S_ISTXT;
        owner = puffs_pn_getvap(targ)->va_uid == uid;
@@ -330,7 +347,7 @@
        (void)snprintf(path, namelen, "%s/%s", 
                       perfuse_node_path((puffs_cookie_t)dpn), name);
 
-       error = node_lookup_common(pu, opc, path, pnp);
+       error = node_lookup_common(pu, opc, path, NULL, pnp);
        
        free(path);
 
@@ -338,10 +355,11 @@
 }
 
 static int
-node_lookup_common(pu, opc, path, pnp)
+node_lookup_common(pu, opc, path, pcr, pnp)
        struct puffs_usermount *pu;
        puffs_cookie_t opc;
        const char *path;
+       const struct puffs_cred *pcr;
        struct puffs_node **pnp;
 {
        struct perfuse_state *ps;
@@ -385,7 +403,7 @@
 
        len = strlen(path) + 1;
 
-       pm = ps->ps_new_msg(pu, opc, FUSE_LOOKUP, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_LOOKUP, len, pcr);
        (void)strlcpy(_GET_INPAYLOAD(ps, pm, char *), path, len);
 
        if ((error = xchg_msg(pu, opc, pm, sizeof(*feo), wait_reply)) != 0)
@@ -491,13 +509,16 @@
        ps =  puffs_getspecific(pu);
 
        /* 
-        * Set owner and group
+        * Set owner and group. The kernel cannot create a file
+        * on its own (puffs_cred_getuid would return -1), right?
         */
-       (void)puffs_cred_getuid(pcn->pcn_cred, &pn->pn_va.va_uid);
-       (void)puffs_cred_getgid(pcn->pcn_cred, &pn->pn_va.va_gid);
+       if (puffs_cred_getuid(pcn->pcn_cred, &pn->pn_va.va_uid) != 0)
+               DERRX(EX_SOFTWARE, "puffs_cred_getuid fails in %s", __func__);
+       if (puffs_cred_getgid(pcn->pcn_cred, &pn->pn_va.va_gid) != 0)
+               DERRX(EX_SOFTWARE, "puffs_cred_getgid fails in %s", __func__);
 
        pm = ps->ps_new_msg(pu, (puffs_cookie_t)pn, 
-                           FUSE_SETATTR, sizeof(*fsi), NULL);
+                           FUSE_SETATTR, sizeof(*fsi), pcn->pcn_cred);
        fsi = GET_INPAYLOAD(ps, pm, fuse_setattr_in);
        fsi->uid = pn->pn_va.va_uid;
        fsi->gid = pn->pn_va.va_gid;
@@ -818,6 +839,9 @@
         * FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES | FUSE_DONT_MASK
         *
         * Linux also sets max_readahead at 32 pages (128 kB)
+        *
+        * ps_new_msg() is called with NULL creds, which will
+        * be interpreted as FUSE superuser. 
         */
        pm = ps->ps_new_msg(pu, 0, FUSE_INIT, sizeof(*fii), NULL);
        fii = GET_INPAYLOAD(ps, pm, fuse_init_in);
@@ -849,8 +873,12 @@
        int error;
 
        ps = puffs_getspecific(pu);
-
        opc = (puffs_cookie_t)puffs_getroot(pu);
+
+       /*
+        * ps_new_msg() is called with NULL creds, which will
+        * be interpreted as FUSE superuser. 
+        */
        pm = ps->ps_new_msg(pu, opc, FUSE_DESTROY, 0, NULL);
 
        if ((error = xchg_msg(pu, opc, pm, UNSPEC_REPLY_LEN, wait_reply)) != 0){
@@ -884,6 +912,11 @@
 
        ps = puffs_getspecific(pu);
        opc = (puffs_cookie_t)puffs_getroot(pu);
+
+       /*
+        * ps_new_msg() is called with NULL creds, which will
+        * be interpreted as FUSE superuser. 
+        */
        pm = ps->ps_new_msg(pu, opc, FUSE_STATFS, 0, NULL);
 
        if ((error = xchg_msg(pu, opc, pm, sizeof(*fso), wait_reply)) != 0)
@@ -1035,7 +1068,7 @@
                pn = PERFUSE_NODE_DATA(opc)->pnd_parent;
        else
                error = node_lookup_common(pu, (puffs_cookie_t)opc,
-                                          pcn->pcn_name, &pn);
+                                          pcn->pcn_name, pcn->pcn_cred, &pn);
        if (error != 0)
                return error;
 
@@ -1110,7 +1143,8 @@
         */
        ps = puffs_getspecific(pu);
        if (ps->ps_flags & PS_NO_CREAT) {
-               error = node_lookup_common(pu, opc, pcn->pcn_name, &pn);
+               error = node_lookup_common(pu, opc, pcn->pcn_name, 
+                                          pcn->pcn_cred, &pn);
                if (error == 0) 
                        return EEXIST;
 
@@ -1118,7 +1152,8 @@
                if (error != 0)
                        return error;
 
-               error = node_lookup_common(pu, opc, pcn->pcn_name, &pn);
+               error = node_lookup_common(pu, opc, pcn->pcn_name,
+                                          pcn->pcn_cred, &pn);
                if (error != 0) 
                        return error;
 
@@ -1148,7 +1183,7 @@
         * 
         * mode must contain file type (ie: S_IFREG), use VTTOIF(vap->va_type)
         */
-       pm = ps->ps_new_msg(pu, opc, FUSE_CREATE, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_CREATE, len, pcn->pcn_cred);
        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);
@@ -1248,7 +1283,7 @@
        /*      
         * mode must contain file type (ie: S_IFREG), use VTTOIF(vap->va_type)
         */
-       pm = ps->ps_new_msg(pu, opc, FUSE_MKNOD, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_MKNOD, len, pcn->pcn_cred);
        fmi = GET_INPAYLOAD(ps, pm, fuse_mknod_in);
        fmi->mode = vap->va_mode | VTTOIF(vap->va_type);
        fmi->rdev = (uint32_t)vap->va_rdev;
@@ -1712,6 +1747,14 @@
        ps = puffs_getspecific(pu);
        /*
         * kh is set if FUSE_POLL_SCHEDULE_NOTIFY is set.
+        *
+        * XXX ps_new_msg() is called with NULL creds, which will
+        * be interpreted as FUSE superuser. We have no way to 
+        * know the requesting process' credential, but since poll
+        * is supposed to operate on a file that has been open,
+        * permission should have already been checked at open time.
+        * That still may breaks on filesystems that provides odd 
+        * semantics.
         */
        pm = ps->ps_new_msg(pu, opc, FUSE_POLL, sizeof(*fpi), NULL);
        fpi = GET_INPAYLOAD(ps, pm, fuse_poll_in);
@@ -1825,7 +1868,7 @@
        /*
         * If fsync_flags  is set, meta data should not be flushed.
         */
-       pm = ps->ps_new_msg(pu, opc, op, sizeof(*ffi), NULL);
+       pm = ps->ps_new_msg(pu, opc, op, sizeof(*ffi), pcr);
        ffi = GET_INPAYLOAD(ps, pm, fuse_fsync_in);
        ffi->fh = fh;
        ffi->fsync_flags = (flags & FFILESYNC) ? 0 : 1;
@@ -1920,7 +1963,7 @@
        name = pcn->pcn_name;
        len = pcn->pcn_namelen + 1;
 
-       pm = ps->ps_new_msg(pu, opc, FUSE_UNLINK, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_UNLINK, len, pcn->pcn_cred);
        path = _GET_INPAYLOAD(ps, pm, char *);
        (void)strlcpy(path, name, len);
 
@@ -1971,7 +2014,7 @@
        name = pcn->pcn_name;
        len =  sizeof(*fli) + pcn->pcn_namelen + 1;
 
-       pm = ps->ps_new_msg(pu, opc, FUSE_LINK, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_LINK, len, pcn->pcn_cred);
        fli = GET_INPAYLOAD(ps, pm, fuse_link_in);
        fli->oldnodeid = PERFUSE_NODE_DATA(pn)->pnd_ino;
        (void)strlcpy((char *)(void *)(fli + 1), name, len - sizeof(*fli));
@@ -2029,7 +2072,7 @@
        oldname_len = pcn_src->pcn_namelen + 1;
 
        len = sizeof(*fri) + oldname_len + newname_len;
-       pm = ps->ps_new_msg(pu, opc, FUSE_RENAME, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_RENAME, len, pcn_targ->pcn_cred);
        fri = GET_INPAYLOAD(ps, pm, fuse_rename_in);
        fri->newdir = PERFUSE_NODE_DATA(targ_dir)->pnd_ino;
        np = (char *)(void *)(fri + 1);
@@ -2106,7 +2149,7 @@
        path = pcn->pcn_name;
        len = sizeof(*fmi) + pcn->pcn_namelen + 1; 
 
-       pm = ps->ps_new_msg(pu, opc, FUSE_MKDIR, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_MKDIR, len, pcn->pcn_cred);
        fmi = GET_INPAYLOAD(ps, pm, fuse_mkdir_in);
        fmi->mode = vap->va_mode;
        fmi->umask = 0;         /* Seems unused by libfuse? */
@@ -2147,7 +2190,7 @@
        name = pcn->pcn_name;
        len = pcn->pcn_namelen + 1;
 
-       pm = ps->ps_new_msg(pu, opc, FUSE_RMDIR, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_RMDIR, len, pcn->pcn_cred);
        path = _GET_INPAYLOAD(ps, pm, char *);
        (void)strlcpy(path, name, len);
 
@@ -2203,7 +2246,7 @@
        linkname_len = strlen(link_target) + 1;
        len = path_len + linkname_len;
 
-       pm = ps->ps_new_msg(pu, opc, FUSE_SYMLINK, len, NULL);
+       pm = ps->ps_new_msg(pu, opc, FUSE_SYMLINK, len, pcn_src->pcn_cred);
        np = _GET_INPAYLOAD(ps, pm, char *);
        (void)strlcpy(np, path, path_len);
        np += path_len;
@@ -2509,6 +2552,10 @@
 
                /*
                 * Send the FORGET message
+                *
+                * ps_new_msg() is called with NULL creds, which will
+                * be interpreted as FUSE superuser. This is obviously
+                * fine since we operate with kernel creds here.
                 */
                pm = ps->ps_new_msg(pu, (puffs_cookie_t)pn, FUSE_FORGET, 
                              sizeof(*ffi), NULL);
@@ -2645,6 +2692,13 @@
        else
                fop = (flags & F_WAIT) ? FUSE_SETLKW : FUSE_SETLK;
                        
+       /*
+        * XXX ps_new_msg() is called with NULL creds, which will
+        * be interpreted as FUSE superuser. We have no way to 
+        * know the requesting process' credential, but since advlock()
+        * is supposed to operate on a file that has been open(),
+        * permission should have already been checked at open() time.
+        */
        pm = ps->ps_new_msg(pu, opc, fop, sizeof(*fli), NULL);
        fli = GET_INPAYLOAD(ps, pm, fuse_lk_in);
        fli->fh = perfuse_get_fh(opc, FWRITE);
diff -r 953527dd2a7c -r 0fa32449c3ff usr.sbin/perfused/msg.c
--- a/usr.sbin/perfused/msg.c   Tue Aug 02 14:06:15 2011 +0000
+++ b/usr.sbin/perfused/msg.c   Tue Aug 02 14:53:38 2011 +0000
@@ -1,4 +1,4 @@
-/*  $NetBSD: msg.c,v 1.13 2011/05/30 14:50:08 manu Exp $ */
+/*  $NetBSD: msg.c,v 1.14 2011/08/02 14:53:38 manu Exp $ */
 
 /*-
  *  Copyright (c) 2010 Emmanuel Dreyfus. All rights reserved.



Home | Main Index | Thread Index | Old Index