fix NFS mount.nfs options v4
Subsystems (2)
PATCH 1/1 fix NFS mount.nfs options v4
AG
Signature · 2 lines
May the source be with you. http://www.cis.ksu.edu/~gud
If the 'user' or 'users' mount option is specified on commandline, verify with /etc/fstab if it is really specified. Setuid bit is still ON and should remain ON since mount drops the root privileges.
I've given this patch a good hammering, but not exhaustive by any means.
---
support/include/fstab.h +8 -0
@@ -3,6 +3,10 @@#include "nfs_mntent.h" +#ifndef _PATH_FSTAB +#define _PATH_FSTAB "/etc/fstab" +#endif +int mtab_is_writable (void );int mtab_does_not_exist (void );
@@ -16,6 +20,10 @@ struct mntentchn *getmntoptfile (const cstruct mntentchn *getmntdirbackward (const char *dir,struct mntentchn *mc);struct mntentchn *getmntdevbackward (const char *dev,struct mntentchn *mc); +struct mntentchn *fstab_head (void ); +struct mntentchn *getfsfile (const char *file); +struct mntentchn *getfsspec (const char *spec); +void lock_mtab (void );void unlock_mtab (void );void update_mtab (const char *special,nfs_mntent_t *with);
support/include/nfs_mntent.h +4 -4
@@ -7,10 +7,10 @@#define _NFS_MNTENT_Htypedef struct nfs_mntent_s { -const char *mnt_fsname; -const char *mnt_dir; -const char *mnt_type; -const char *mnt_opts; +char *mnt_fsname; +char *mnt_dir; +char *mnt_type; +char *mnt_opts;int mnt_freq;int mnt_passno; }nfs_mntent_t ;
support/nfs/fstab.c +54 -3
82 lines not on this page · raw message
utils/mount/Makefile.am +4 -3
@@ -14,9 +14,10 @@ MAINTAINERCLEANFILES = Makefile.in install-exec-hook: (cd $(DESTDIR)$(sbindir) && \ - ln -sf $(sbin_PROGRAMS) mount.nfs4 && \ - ln -sf $(sbin_PROGRAMS) umount.nfs && \ - ln -sf $(sbin_PROGRAMS) umount.nfs4) + chmod +s $(sbin_PROGRAMS) && \ + cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/mount.nfs4 && \ + cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/umount.nfs && \ + cp -p $(sbin_PROGRAMS) $(DESTDIR)$(sbindir)/umount.nfs4) uninstall-hook: (cd $(DESTDIR)$(sbindir) && \ rm -f mount.nfs4 umount.nfs umount.nfs4)
utils/mount/mount.c +123 -18
231 lines not on this page · raw message
utils/mount/nfs_mount.h +2 -0
@@ -80,5 +80,7 @@ struct nfs_mount_data {int nfsmount (const char *,const char *,int *,char **,char **,int *,int );void mount_errors (char *,int ,int ); +int contains (const char *,const char *); +char *get_value (const char *,const char *);#endif /* _NFS_MOUNT_H */
utils/mount/nfsumount.c +18 -1
@@ -23,12 +23,14 @@#include <mntent.h> #include <sys/mount.h> #include <ctype.h> +#include <pwd.h> #include "xcommon.h" #include "fstab.h" #include "nls.h" #include "conn.h" +#include "nfs_mount.h" #include "mount_constants.h" #include "mount.h" #include "nfsumount.h"
@@ -307,7 +309,7 @@ int _nfsumount(const char *spec, const c goto out_bad;return nfs_call_umount (&mnt_server, &dirname); out_bad: -printf ("%s: %s: not found or not mounted\n" , progname, spec); +fprintf (stderr,"%s: %s: not found or not mounted\n" , progname, spec);return 0 ; }
@@ -376,6 +378,21 @@ int nfsumount(int argc, char *argv[])printf (_ ("Could not find %s in mtab\n" ), spec);if (mc) { +if (contains (mc->m.mnt_opts,"user" ) &&getuid () !=0 ) { +struct passwd *pw =getpwuid (getuid ()); +if (!pw ||strcmp (pw->pw_name,get_value (mc->m.mnt_opts,"user=" ))) { +fprintf (stderr,"%s: permission denied to unmount %s\n" , + progname, spec); +exit (1 ); + } + }else { +if (!contains (mc->m.mnt_opts,"users" ) &&getuid () !=0 ) { +fprintf (stderr,"%s: only root can unmount %s from %s\n" , + progname, mc->m.mnt_fsname, mc->m.mnt_dir); +exit (1 ); + } + } + ret =_nfsumount (mc->m.mnt_fsname, mc->m.mnt_opts);if (ret) ret = add_mtab2(mc->m.mnt_fsname, mc->m.mnt_dir,
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
Quoted 47 lines · Amit Gud
AG
------------------------------------------------------------------------
If the 'user' or 'users' mount option is specified on commandline, verify with
/etc/fstabif it is really specified. Setuid bit is still ON and should remain ON since mount drops the root privileges.I've given this patch a good hammering, but not exhaustive by any means.
Signed-off-by Amit Gud <[email protected]>Signed-off-by Steve Dickson <[email protected]>---
diff -uprN -X ../dontdiff
nfs-utils/utils/mount/mount.cnfs-utils-ag/utils/mount/mount.cutils/mount/mount.c +18
-1@@ -355,9 +433,33 @@ int main(int argc, char *argv[]) spec = argv[1 ]; mount_point =canonicalize (argv[2 ]); - +parse_opts (mount_opts, &flags, &extra_opts); +if (uid !=0 && !(flags &MS_USERS ) && !(flags &MS_USER )) { +fprintf (stderr,"%s: permission denied\n" , progname); +exit (1 ); + } + +if ((flags &MS_USER || flags &MS_USERS ) && uid !=0 ) { +/* check if fstab has entry, and further see if the user or users option is given */ +if ((mc =getfsspec (spec)) ==NULL && + (mc =getfsfile (spec)) ==NULL ) { +fprintf (stderr,"%s: permission denied - invalid option\n" , progname); +exit (1 ); + } +else { +if ((flags &MS_USER ) && !contains (mc->m.mnt_opts,"user" )) { +fprintf (stderr,"%s: permission denied - invalid option\n" , progname); +exit (1 ); + }
utils/mount/mount.c int main(int argc, char *argv[]) · line 465
Quoted 10 lines
utils/mount/mount.c +7
-0+if ((flags &MS_USERS ) && !contains (mc->m.mnt_opts,"users" )) { +fprintf (stderr,"%s: permission denied - invalid option\n" , progname); +exit (1 ); + } + } + } +if (!strcmp (progname,"mount.nfs4" ) || nfs_mount_vers ==4 ) { nfs_mount_vers =4 ; mnt_err =nfs4mount (spec, mount_point, &flags, &extra_opts, &mount_opts,0 );
It seems to me that these two "permission denied" messages may be somewhat less than completely helpful. Would it be possible to be a little more explicit about why permission was being denied?
Thanx...
ps-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
Quoted 5 lines · Peter Staubach
It seems to me that these two "permission denied" messages may be somewhat less than completely helpful. Would it be possible to be a little more explicit about why permission was being denied?
Yes possible. I was thinking of not letting the users know which user mounted the export, but that is known by mount command anyways. So, yes these messages could be more elaborate :)
AG
Signature · 12 lines
May the source be with you. http://www.cis.ksu.edu/~gud ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ NFS maillist - [email protected] https://lists.sourceforge.net/lists/listinfo/nfs
On Friday July 28, [email protected] wrote:
Sorry for the delay in reviewing this.
Quoted 6 lines
If the 'user' or 'users' mount option is specified on commandline, verify with
/etc/fstabif it is really specified. Setuid bit is still ON and should remain ON since mount drops the root privileges.I've given this patch a good hammering, but not exhaustive by any means.
I still don't see this as being adequately secure.
All you are checking is whether the user has permission to mount onto the given directory, or from the given server:/directory, but not if they are allow to mount that server on that directory, or what options are required.
e.g. /etc/fstab could say you are only allowed to mount with noexec,nosuid, but mount.nfs isn't enforcing that.
It seems to me that if getuid() returns non-zero, then you have to either ignore or check everything given on the command line.
i.e.
find the entry in /etc/fstab which has the same mointpoint and the same 'special' (server:/path).
Make sure the options in /etc/fstab have an appropriate user or users entry.
Use the options from /etc/fstab and ignore the options on the command line.
Does that sound fair?
NeilBrown
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs