2007-08-09 15:31:34

by Miklos Szeredi

[permalink] [raw]
Subject: [RFC PATCH 4/4] VFS: allow filesystem to override mknod capability checks

From: Miklos Szeredi <[email protected]>

Add a new filesystem flag, that results in the VFS not checking if the
current process has enough privileges to do an mknod().

This is needed on filesystems, where an unprivileged user may be able
to create a device node, without causing security problems.

One such example is "mountlo" a loopback mount utility implemented
with fuse and UML, which runs as an unprivileged userspace process.
In this case the user does in fact have the right to create device
nodes within the filesystem image, as long as the user has write
access to the image. Since the filesystem is mounted with "nodev",
adding device nodes is not a security concern.

This feature is basically "fuse-only", so it does not make sense to
change the semantics of ->mknod().

Signed-off-by: Miklos Szeredi <[email protected]>
---

Index: linux/fs/namei.c
===================================================================
--- linux.orig/fs/namei.c 2007-08-09 16:49:07.000000000 +0200
+++ linux/fs/namei.c 2007-08-09 16:49:12.000000000 +0200
@@ -1921,7 +1921,8 @@ int vfs_mknod(struct inode *dir, struct
if (error)
return error;

- if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
+ if (!(dir->i_sb->s_type->fs_flags & FS_MKNOD_CHECKS_PERM) &&
+ (S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
return -EPERM;

if (!dir->i_op || !dir->i_op->mknod)
Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h 2007-08-09 16:49:07.000000000 +0200
+++ linux/include/linux/fs.h 2007-08-09 16:49:12.000000000 +0200
@@ -97,6 +97,7 @@ extern int dir_notify_enable;
#define FS_BINARY_MOUNTDATA 2
#define FS_HAS_SUBTYPE 4
#define FS_SAFE 8 /* Safe to mount by unprivileged users */
+#define FS_MKNOD_CHECKS_PERM 16 /* FS checks if device creation is allowed */
#define FS_REVAL_DOT 16384 /* Check the paths ".", ".." for staleness */
#define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move()
* during rename() internally.

--


2007-08-09 19:21:05

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] VFS: allow filesystem to override mknod capability checks

Quoting [email protected] ([email protected]):
> From: Miklos Szeredi <[email protected]>
>
> Add a new filesystem flag, that results in the VFS not checking if the
> current process has enough privileges to do an mknod().
>
> This is needed on filesystems, where an unprivileged user may be able
> to create a device node, without causing security problems.
>
> One such example is "mountlo" a loopback mount utility implemented
> with fuse and UML, which runs as an unprivileged userspace process.
> In this case the user does in fact have the right to create device
> nodes within the filesystem image, as long as the user has write
> access to the image. Since the filesystem is mounted with "nodev",
> adding device nodes is not a security concern.

Could we enforce at do_new_mount() that if
type->fs_flags&FS_MKNOD_CHECKS_PERM then mnt_flags |= MS_NODEV?

> This feature is basically "fuse-only", so it does not make sense to
> change the semantics of ->mknod().
>
> Signed-off-by: Miklos Szeredi <[email protected]>
> ---
>
> Index: linux/fs/namei.c
> ===================================================================
> --- linux.orig/fs/namei.c 2007-08-09 16:49:07.000000000 +0200
> +++ linux/fs/namei.c 2007-08-09 16:49:12.000000000 +0200
> @@ -1921,7 +1921,8 @@ int vfs_mknod(struct inode *dir, struct
> if (error)
> return error;
>
> - if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
> + if (!(dir->i_sb->s_type->fs_flags & FS_MKNOD_CHECKS_PERM) &&
> + (S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
> return -EPERM;
>
> if (!dir->i_op || !dir->i_op->mknod)
> Index: linux/include/linux/fs.h
> ===================================================================
> --- linux.orig/include/linux/fs.h 2007-08-09 16:49:07.000000000 +0200
> +++ linux/include/linux/fs.h 2007-08-09 16:49:12.000000000 +0200
> @@ -97,6 +97,7 @@ extern int dir_notify_enable;
> #define FS_BINARY_MOUNTDATA 2
> #define FS_HAS_SUBTYPE 4
> #define FS_SAFE 8 /* Safe to mount by unprivileged users */
> +#define FS_MKNOD_CHECKS_PERM 16 /* FS checks if device creation is allowed */
> #define FS_REVAL_DOT 16384 /* Check the paths ".", ".." for staleness */
> #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move()
> * during rename() internally.
>
> --
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2007-08-09 20:12:08

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] VFS: allow filesystem to override mknod capability checks

> > From: Miklos Szeredi <[email protected]>
> >
> > Add a new filesystem flag, that results in the VFS not checking if the
> > current process has enough privileges to do an mknod().
> >
> > This is needed on filesystems, where an unprivileged user may be able
> > to create a device node, without causing security problems.
> >
> > One such example is "mountlo" a loopback mount utility implemented
> > with fuse and UML, which runs as an unprivileged userspace process.
> > In this case the user does in fact have the right to create device
> > nodes within the filesystem image, as long as the user has write
> > access to the image. Since the filesystem is mounted with "nodev",
> > adding device nodes is not a security concern.
>
> Could we enforce at do_new_mount() that if
> type->fs_flags&FS_MKNOD_CHECKS_PERM then mnt_flags |= MS_NODEV?

Well, the problem with that is, there will be fuse filesystems which
will want devices to work and for those the capability checks will be
reenabled inside ->mknod(). In fact, for backward compatibility all
filesystems will have the mknod checks, except ones which explicitly
request to turn it off.

Since unprivileged fuse mounts always have "nodev", the only way
security could be screwed up, is if a filesystem running with
privileges disabled the mknod checks.

I will probably add some safety guards against that into the fuse
library, but of course there's no way to stop a privileged user from
screwing up security anyway.

If for example there's a loop mount, where the disk image file is
writable by a user, and root mounts it without "nodev", the user can
still create device nodes (by modifying the image) even if the mknod
checks are enabled.

Miklos

2007-08-10 14:46:29

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] VFS: allow filesystem to override mknod capability checks

Quoting Miklos Szeredi ([email protected]):
> > > From: Miklos Szeredi <[email protected]>
> > >
> > > Add a new filesystem flag, that results in the VFS not checking if the
> > > current process has enough privileges to do an mknod().
> > >
> > > This is needed on filesystems, where an unprivileged user may be able
> > > to create a device node, without causing security problems.
> > >
> > > One such example is "mountlo" a loopback mount utility implemented
> > > with fuse and UML, which runs as an unprivileged userspace process.
> > > In this case the user does in fact have the right to create device
> > > nodes within the filesystem image, as long as the user has write
> > > access to the image. Since the filesystem is mounted with "nodev",
> > > adding device nodes is not a security concern.
> >
> > Could we enforce at do_new_mount() that if
> > type->fs_flags&FS_MKNOD_CHECKS_PERM then mnt_flags |= MS_NODEV?
>
> Well, the problem with that is, there will be fuse filesystems which
> will want devices to work

Crud, sorry, I forgot all fuse filesystems will have the same fs_flags.

> and for those the capability checks will be
> reenabled inside ->mknod(). In fact, for backward compatibility all
> filesystems will have the mknod checks, except ones which explicitly
> request to turn it off.
>
> Since unprivileged fuse mounts always have "nodev", the only way

Ah yes, I'd forgotten that we do if (!capable(mknod)) mnt_flags |= MNT_NODEV

No objections then anyway. Thanks for indulging me :)

> security could be screwed up, is if a filesystem running with
> privileges disabled the mknod checks.
>
> I will probably add some safety guards against that into the fuse
> library, but of course there's no way to stop a privileged user from
> screwing up security anyway.

Agreed.

> If for example there's a loop mount, where the disk image file is
> writable by a user, and root mounts it without "nodev", the user can
> still create device nodes (by modifying the image) even if the mknod
> checks are enabled.

thanks,
-serge