Hello,
I have done a quick and brutal fix to this. Patch file is attached.
The fix:
- terminate, if run setuid. So only root can mount. Reason: davfs2 does
not enforce mount control by fstab. So if run setuid, any user could
mount with the uid of any other user.
- set uid and gid according to the values given as option. Set file mode
600 and directory mode 700. Only root and the user given as option may
use the file system.
- do not allow to change uid, gid and mode of any part of the mounted
file system.
- for every request (coda upcall) the requesting uid is checked against
the uid of the file system.
Rationale:
Checking is more restrictive than necessary. But this way it could be
done with little effort. I also think that a more sophisticated checking
of permissions should be done together with the redisign of other parts
of davfs2.
Greetings
Werner
diff -Naur davfs2-0.2.3.orig/ChangeLog davfs2-0.2.3/ChangeLog
--- davfs2-0.2.3.orig/ChangeLog 2005-06-03 21:03:13.000000000 +0200
+++ davfs2-0.2.3/ChangeLog 2005-06-04 13:53:20.000000000 +0200
@@ -1,5 +1,14 @@
ChangeLog for Davfs2
+2005-06-03 Werner Baumann
+ security fix (quick and brutal) concerning file access:
+ davfsd.c, util.c, util.h, webdav.c:
+ * set filemode to 0600 and dirmode to 0700
+ * don't allow change of uid, gid or mode
+ * check every coda upcoll for permissions:
+ access is only allowed from owner and root
+ * terminate if run setuid
+
2004-11-01 Robert Spier
* Seems like a good time for 0.2.3
* Changes in the past 11 months include...
diff -Naur davfs2-0.2.3.orig/README davfs2-0.2.3/README
--- davfs2-0.2.3.orig/README 2005-06-03 21:03:13.000000000 +0200
+++ davfs2-0.2.3/README 2005-06-04 13:51:52.000000000 +0200
@@ -53,7 +53,14 @@
- Use umount for unmount
- example : umount /dav
-4. Debugging
+4. User mount
+ - For security reasons only root may mount. mount.davfs must not be run
setuid.
+
+5. File permissions:
+ - Permissions are set to 600 (700 for directories).
+ It is not possible to change uid, gid or mode.
+
+6. Debugging
- mount.davfs will not run as a daemon mode.
- configure with --with-debug option
- Coda debug log goes out stdout
@@ -61,9 +68,9 @@
- To save log : ./mount.davfs http://127.0.0.1/repos/ /dav > coda.log
2>webdav.log
- To kill all running mount.davfs, do 'killall mount.davfs'
-5. For more information : http://dav.sf.net
+7. For more information : http://dav.sf.net
-6. Participation
+8. Participation
DAVFS is an Open Source project, and we welcome your participation.
Please join developer mailing list [EMAIL PROTECTED]
For cvs commit info, join [EMAIL PROTECTED]
diff -Naur davfs2-0.2.3.orig/src/davfsd.c davfs2-0.2.3/src/davfsd.c
--- davfs2-0.2.3.orig/src/davfsd.c 2005-06-03 21:03:13.000000000 +0200
+++ davfs2-0.2.3/src/davfsd.c 2005-06-04 13:42:46.000000000 +0200
@@ -68,82 +68,20 @@
static int count = 0;
-/* default stat */
-struct stat generic_stat = { 0 /* dev */ , 0 /* pad */ ,
- 0 /* inode */ , S_IFREG | 0666 /* mode */ ,
- 0, 0, /* uid, gid */ 0 /* device */ , 0 /* pad */ ,
- 0 /* size */ , 1024 /* blksize */
-}; /* rest are 0 */
-
-/* Mkdir and Create need to return attr to kernel */
-static void set_mkdir_attr(struct coda_vattr *attr) {
- struct stat stat;
-
- /* Get default mode */
- dav_get_fstat_default(&stat);
-
- attr->va_type = C_VDIR;
-
- /* FIXME: Mode?? */
- attr->va_mode = stat.st_mode | S_IXUSR;
- IFTOCDT(attr->va_mode);
- attr->va_mode |= CDT_DIR;
-
- attr->va_uid = stat.st_uid;
- attr->va_gid = stat.st_gid;
+struct stat dav_file_stat;
+struct stat dav_dir_stat;
- attr->va_size = 512;
- attr->va_blocksize = 1;
-}
-
-
-/* Mkdir and Create need to return attr to kernel */
-static void set_create_attr(struct coda_vattr *attr) {
-#if 0
- struct stat stat;
-
- /* Get default mode */
- dav_get_fstat_default(&stat);
-
- attr->va_mode = stat.st_mode;
- IFTOCDT(attr->va_mode);
- attr->va_mode |= CDT_REG;
-
- attr->va_uid = stat.st_uid;
- attr->va_gid = stat.st_gid;
-
- /* Zero for new creation */
- attr->va_size = 0;
- attr->va_blocksize = 0;
-
-#endif
-}
-
-/* change uid/gid to match credentials given by kernel */
-static void setfscred(union inputArgs *in_buf)
-{
-#if CODA_KERNEL_VERSION > 2
- DBG1(" (uid=%d)", in_buf->ih.uid);
-
- setfsuid(in_buf->ih.uid);
- setfsgid(0);
+int dav_has_permission(struct coda_in_hdr *ih) {
+#ifdef NEW_CODA_STRUCTURES
+ DBG1("id: %i\n", ih->uid);
+ if ((ih->uid != dav_file_stat.st_uid) && (ih->uid != 0))
+ return 0;
#else
- DBG2(" (uid=%d,euid=%d,",
- in_buf->ih.cred.cr_uid, in_buf->ih.cred.cr_euid);
- DBG2(" suid=%d,fsuid=%d) ",
- in_buf->ih.cred.cr_suid, in_buf->ih.cred.cr_fsuid);
-
- setfsuid(in_buf->ih.cred.cr_fsuid);
- setfsgid(in_buf->ih.cred.cr_fsgid);
-#endif /* CODA_FS_OLD_API */
-}
-
-
-/* reset uid/gid */
-static void resetfscred(void)
-{
- seteuid(0);
- setegid(0);
+ DBG1("id: %i\n", ih->cred.cr_uid);
+ if ((ih->cred.cr_uid != dav_file_stat.st_uid) && (ih->cred.cr_uid != 0))
+ return 0;
+#endif /* NEW_CODA_STRUCTURES */
+ return 1;
}
/* Delete mnt table ans try to umount */
@@ -151,8 +89,6 @@
{
DBG1("signal_handler(%d)\n", signo);
- resetfscred();
-
/* Unount */
/* Scenario:
1. mount.davfs
@@ -704,7 +640,7 @@
}
else
- st = generic_stat;
+ st = dav_file_stat;
/* Directory */
if (S_ISDIR(st.st_mode)) {
@@ -780,6 +716,10 @@
union outputArgs *out_buf;
int server_nolocks=0;
+ if (geteuid() != getuid()) {
+ fprintf(stderr,"You must not run mount.davfs setuid.\n"), exit(1);
+ }
+
if (geteuid() != 0) {
fprintf(stderr,"You must be root to use mount.davfs\n"), exit(1);
}
@@ -888,7 +828,7 @@
/* Save pid */
dav_save_mount_pid(mopt.dev);
-
+
/* Read message from the CODA device */
for ( ; ; ) {
char *name;
@@ -931,9 +871,6 @@
DBG2("got %3.3d byte command: opcode = %2.2ld ", msg,
in_buf->ih.opcode);
- /* switch to uid/gid of requesting process */
- setfscred(in_buf);
-
out_buf->oh.opcode = in_buf->ih.opcode;
out_buf->oh.unique = in_buf->ih.unique;
out_buf->oh.result = ENOSYS;
@@ -943,6 +880,10 @@
switch (in_buf->ih.opcode) {
case CODA_ROOT:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(root);
DBG0(": ");
alloc_vfid((DAVCodaFid *)&out_buf->coda_root.VFid, "/", 1);
@@ -969,6 +910,10 @@
/********************* file props & access *******************/
case CODA_GETATTR:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(getattr);
DUMP(getattr);
GET_VFID_name(getattr);
@@ -976,11 +921,15 @@
STAT(name); /* fills in st */
}
else
- st = generic_stat;
+ st = dav_file_stat;
st2attr(&st, &out_buf->coda_getattr.attr);
break;
case CODA_SETATTR:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD_NOREP(setattr);
DUMP(setattr);
GET_VFID_name(setattr);
@@ -989,12 +938,17 @@
in_buf->coda_setattr.attr.va_mode,
in_buf->coda_setattr.attr.va_mtime.tv_sec,
in_buf->coda_setattr.attr.va_uid);
- if (in_buf->coda_setattr.attr.va_mode != (u_short) - 1)
- dav_chmod( name, in_buf->coda_setattr.attr.va_mode );
- /* cannot change owner, group or dates ....... */
+ if ((in_buf->coda_setattr.attr.va_mode != (u_short) -1)
+ || (in_buf->coda_setattr.attr.va_uid != -1)
+ || (in_buf->coda_setattr.attr.va_gid != -1))
+ out_buf->oh.result = ENOTSUP;
break;
case CODA_ACCESS:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
/* this always returns TRUE; this seems to be ok with current CODA
design */
CMD_NOREP(access);
DUMP(access);
@@ -1003,6 +957,10 @@
break;
case CODA_LOOKUP: /* !! ttd: search for existing VFid's instead
of making many new ones */
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(lookup);
DUMP_NAME(lookup);
GET_VFID_name(lookup);
@@ -1015,6 +973,10 @@
break;
case CODA_READLINK:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(readlink);
DUMP(readlink);
GET_VFID_name(readlink);
@@ -1035,6 +997,10 @@
/************************ open, close, create & mkdir ******************/
case CODA_OPEN_BY_FD:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(open);
DUMP(open);
DBG1("flags:%#x ", in_buf->coda_open.flags);
@@ -1051,6 +1017,10 @@
break;
case CODA_OPEN:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(open);
DUMP(open);
DBG1("flags:%#x ", in_buf->coda_open.flags);
@@ -1069,6 +1039,10 @@
break;
case CODA_OPEN_BY_PATH:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(open_by_path);
DUMP(open_by_path);
DBG1("flags:%#x ", in_buf->coda_open_by_path.flags);
@@ -1090,6 +1064,10 @@
/*# FIXME : RELEASE */
case CODA_RELEASE:
case CODA_CLOSE:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD_NOREP(close);
DUMP(close);
GET_VFID_name(close);
@@ -1105,10 +1083,18 @@
/*# FIXME : What should I do ? */
case CODA_FSYNC:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD_NOREP(fsync);
break;
case CODA_CREATE:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(create);
DUMP_NAME(create);
GET_VFID_name(create);
@@ -1119,11 +1105,15 @@
v_created(out_buf->coda_create.VFid) = 1;
/* get inherient */
- out_buf->coda_create.attr = in_buf->coda_create.attr;
- set_create_attr(&(out_buf->coda_create.attr));
+ out_buf->coda_create.attr = in_buf->coda_create.attr;
+ st2attr(&dav_file_stat, &(out_buf->coda_create.attr));
break;
case CODA_MKDIR:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD(mkdir);
DUMP_NAME(mkdir);
GET_VFID_name(mkdir);
@@ -1133,8 +1123,8 @@
alloc_vfid((DAVCodaFid *)&out_buf->coda_mkdir.VFid, buf, 1);
/* get inherient */
- out_buf->coda_mkdir.attr = in_buf->coda_mkdir.attr;
- set_mkdir_attr(&(out_buf->coda_mkdir.attr));
+ out_buf->coda_mkdir.attr = in_buf->coda_mkdir.attr;
+ st2attr(&dav_dir_stat, &(out_buf->coda_mkdir.attr));
}
else
out_buf->oh.result = dav_get_errno();
@@ -1142,6 +1132,10 @@
/********************* deletions & rename ******************/
case CODA_RMDIR:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD_NOREP(rmdir);
DUMP_NAME(rmdir);
GET_VFID_name(rmdir);
@@ -1155,6 +1149,10 @@
out_buf->oh.result = dav_get_errno();
break;
case CODA_REMOVE:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD_NOREP(remove);
DUMP_NAME(remove);
GET_VFID_name(remove);
@@ -1168,6 +1166,10 @@
out_buf->oh.result = dav_get_errno();
break;
case CODA_RENAME:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD_NOREP(rename);
{
int is_dir_src;
@@ -1197,11 +1199,19 @@
/* Not implemented, but should return OK */
case CODA_STATFS:
case CODA_STORE:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
CMD_NOREP(store);
DBG0(" What should I save?");
break;
default:
+ if (!dav_has_permission(&in_buf->ih)) {
+ out_buf->oh.result = EPERM;
+ break;
+ }
DBG0(" unimplemented coda call");
break;
}
@@ -1214,7 +1224,6 @@
close(toclose);
fflush(stdout);
fflush(stderr);
- resetfscred();
time_last_msg = time(NULL);
} /* while */
}
diff -Naur davfs2-0.2.3.orig/src/mount.c davfs2-0.2.3/src/mount.c
--- davfs2-0.2.3.orig/src/mount.c 2005-06-03 21:03:13.000000000 +0200
+++ davfs2-0.2.3/src/mount.c 2005-06-04 13:48:43.000000000 +0200
@@ -20,6 +20,7 @@
#include <stdio.h>
#include <fcntl.h>
#include <errno.h>
+#include <pwd.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
diff -Naur davfs2-0.2.3.orig/src/util.c davfs2-0.2.3/src/util.c
--- davfs2-0.2.3.orig/src/util.c 2005-06-03 21:03:13.000000000 +0200
+++ davfs2-0.2.3/src/util.c 2005-06-04 13:44:46.000000000 +0200
@@ -261,31 +261,23 @@
NE_FREE(mopt->option);
}
-/* Static global variable for fstat default */
-static uid_t G_uid;
-static gid_t G_gid;
-static mode_t G_mode;
-
-/* Set default uid, gid, and mode */
+/* 2005-06-03, werner, fix permission checking
+ set uid and guid from mounting user, fixed mode
+ dont't allow to change any of this */
+extern struct stat dav_file_stat;
+extern struct stat dav_dir_stat;
void dav_set_fstat_default(uid_t uid, gid_t gid, mode_t mode) {
- G_uid = uid;
- G_gid = gid;
- G_mode = mode;
-}
-
-/* Get uid, gid, and mode. Still need to set mode is DIR or REG */
-void dav_get_fstat_default(struct stat *stat) {
-
- /* Not set ?? */
- if(G_mode==0) {
- DBG0("Fstat default is not set!");
- stat->st_mode = DEFAULT_MODE;
- } else {
- stat->st_mode = G_mode;
- }
-
- stat->st_uid = G_uid;
- stat->st_gid = G_gid;
+ dav_file_stat.st_uid = uid;
+ dav_file_stat.st_gid = gid;
+ dav_file_stat.st_mode = S_IRUSR | S_IWUSR | S_IFREG;
+ dav_file_stat.st_nlink = 1;
+ dav_file_stat.st_blksize = 1024;
+ dav_dir_stat.st_uid = getuid();
+ dav_dir_stat.st_gid = getgid();
+ dav_dir_stat.st_mode = S_IRWXU | S_IFDIR;
+ dav_dir_stat.st_nlink = 1;
+ dav_dir_stat.st_blksize = 1024;
+ dav_dir_stat.st_size = 512;
}
/* dav_kill_prev_mount */
diff -Naur davfs2-0.2.3.orig/src/util.h davfs2-0.2.3/src/util.h
--- davfs2-0.2.3.orig/src/util.h 2005-06-03 21:03:13.000000000 +0200
+++ davfs2-0.2.3/src/util.h 2005-06-04 13:44:51.000000000 +0200
@@ -91,9 +91,6 @@
/* Set default uid, gid, and mode */
void dav_set_fstat_default(uid_t uid, gid_t gid, mode_t mode);
-/* Get uid, gid, and mode. Still need to set mode is DIR or REG */
-void dav_get_fstat_default(struct stat *stat);
-
/* Save pid */
int dav_save_mount_pid(const char *dev);
diff -Naur davfs2-0.2.3.orig/src/webdav.c davfs2-0.2.3/src/webdav.c
--- davfs2-0.2.3.orig/src/webdav.c 2005-06-03 21:03:13.000000000 +0200
+++ davfs2-0.2.3/src/webdav.c 2005-06-03 22:39:20.000000000 +0200
@@ -40,6 +40,8 @@
#include "util.h"
/* Global variables */
+extern struct stat dav_file_stat;
+extern struct stat dav_dir_stat;
static ne_session *session;
static char *proxy_hostname;
static int proxy_port;
@@ -418,20 +420,11 @@
data = ne_propset_value(set, &stat_props[1]);
- /*
- * Get default value
- * gid, gid, and mode
- */
- dav_get_fstat_default(&(result->f_st));
-
if (data && strstr(data, "collection")) {
- /* Directory should have X permission */
- result->f_st.st_mode |= S_IFDIR;
- result->f_st.st_mode |= S_IXUSR;
- result->f_st.st_size = 512;
+ result->f_st = dav_dir_stat;
}
else {
- result->f_st.st_mode |= S_IFREG;
+ result->f_st = dav_file_stat;
}
/* FIXME : Do we need to return . and .. ?? */
@@ -476,8 +469,9 @@
/* executable */
data = ne_propset_value(set, &stat_props[4]);
- if( data )
- result->f_st.st_mode |= S_IXUSR | S_IXGRP | S_IXOTH;
+ /* 2005-06-03, werner, dont't allow execute */
+ /*if( data )
+ result->f_st.st_mode |= S_IXUSR | S_IXGRP | S_IXOTH;*/
result->f_st.st_blksize = DAVFS_BLKSIZE;
result->f_st.st_blocks =
@@ -780,11 +774,6 @@
}
-/* FIXME: How can we change mode? */
-int dav_chmod(const char *name, unsigned short mode) {
- return 0;
-}
-
/* start emacs stuff */
/*
* local variables:
