Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding --no-canonicalize prevents user mounts #7294

Closed
behlendorf opened this issue Mar 12, 2018 · 8 comments
Closed

Adding --no-canonicalize prevents user mounts #7294

behlendorf opened this issue Mar 12, 2018 · 8 comments
Labels
good first issue Indicates a good issue for first-time contributors
Milestone

Comments

@behlendorf
Copy link
Contributor

System information

Type Version/Name
Distribution Name Ubuntu
Distribution Version 16.04
Linux Kernel Any
Architecture x86_64
ZFS Version zfs-0.7.0-360-g8e5d148
SPL Version spl-0.7.0-30-g3673d03

Describe the problem you're observing

Reposted from #6437 (comment).

PR #6437 appears to prevent user-mode mounting of volumes.

Is it possible that the --no-canonicalize option could be used only when necessary? Or is there another solution to this that doesn't prevent user mounting?

Describe how to reproduce the problem

root@athena:~# zfs allow jules mount testpool/jtest

jules@athena:~$ zfs mount testpool/jtest
mount: only root can use "--no-canonicalize" option
cannot mount 'testpool/jtest': Invalid argument

Include any warning/errors/backtraces from the system logs

None

@behlendorf behlendorf added the good first issue Indicates a good issue for first-time contributors label Mar 12, 2018
@behlendorf behlendorf added this to the 0.8.0 milestone Mar 12, 2018
@loli10K
Copy link
Contributor

loli10K commented Mar 12, 2018

I did not know zfs allow mount could work with any other user than root, from zfs(8):

zfs allow filesystem|volume

Displays permissions that have been delegated on the specified filesystem or volume. See the other forms of zfs allow for more information.

Delegations are supported under Linux with the exception of mount, unmount, mountpoint, canmount, rename, and share. These permissions cannot be delegated because the Linux mount(8) command restricts modifications of the global namespace to the root user.

EDIT: i'm sorry if #6437 broke some functionality, but from the man page i thought this was a non-issue.

@bunder2015
Copy link
Contributor

bunder2015 commented Mar 17, 2018

I would like to second what @loli10K has mentioned, zfs allow has also been a repeated topic of conversation on IRC. It would be nice if it was fully functional on linux. We seem to be in a chicken-and-egg problem where ZFS is blaming linux and linux is blaming ZFS.

edit: FWIW, it actually is possible for users to mount (non-zfs) filesystems if the fstab entry has the "user" mount option set.

As well, the zfs command is installed in /sbin which isn't a part of the default path on some distros.

There was also previously another problem due to the permissions on /dev/zfs.

@behlendorf
Copy link
Contributor Author

behlendorf commented Mar 19, 2018

@bunder2015 @loli10K it's definietly worth taking a second look at delegations for mount, unmount, etc. The landscape here may have changed somewhat since the initial implementation and we may be able to workaround, or live with, some of the restrictions Linux imposes. If someone has the time to work on this that would be great.

For example, if I recall correctly while Linux does allow "user" mounts it only done so when:

  1. The /etc/fstab contains the keyword "user", that means only legacy ZFS mount support, or
  2. The mount is allowed in a private user namespace, never in the global system namespace.

Adding support for the second case might address most peoples concerns since it should allow delegations to work correctly in containers.

As well, the zfs command is installed in /sbin which isn't a part of the default path on some distros.

Perhaps we can symlink it from /bin. It's installed by default in /sbin for FHS conformance reasons.

There was also previously another problem due to the permissions on /dev/zfs.

This should have been addressed by the updated udev rules some time ago.

@bunder2015
Copy link
Contributor

Unfortunately legacy mount points won't work here since users can't edit /etc/fstab (without sudo/etc, at least).

However I just had a thought, the mount binary is setuid root, could we potentially leverage that for our use outside of containers?

@pstch
Copy link

pstch commented Nov 15, 2018

EDIT: I think this is a non-issue, you way want not to bother with these comments and jump to the last one.

I've been trying to find a solution to this problem, although I'm not an experienced filesystem developer.

I don't think that the mount behaviour will change (requiring root to disable canonicalization) any time soon, so it seems that the only solution to this problem is not using --no-canonicalize, or allowing not to use it.

This means that another solution has to be found for #1791 (presumably) and #6429. I understand that both of these issues are caused by the canonicalization of the device path.

@pstch
Copy link

pstch commented Nov 15, 2018

After reading util-linux/lib/canonicalize.c, I conclude that the main cause of #1791 and #6429 is realpath, which assumes that the path is relative to the CWD. This triggers #1791 (because realpath will resolve symlinks) and #6429 (WorkingDirectory being required).

This actually looks hard to workaround in zfs mount. mount seems to always interpret its arguments as files, and requires sanitized paths for non-root users. I'm thinking that maybe something can be done by making zfs mount pass mount a special form of the device path, and handling this special form in mount.zfs, so that the canonicalization done in mount has no effect.

EDIT: maybe this prefix could be a simple /, added by zfs mount, making realpath a no-op, and then removed by mount.zfs. I will run some experiments on this tomorrow. This could cause a problem if a ZFS pool is imported from a file in the root directory that has the same path as the dataset being mounted (as mount.zfs would mount the ZFS pool imported from the file in the root directory rather than the requested dataset). So /nonexistent/ maybe ? Or /zfs/ ? Or /dev/zfs/ (as /dev/zfs will never be a directory) ?

It seems like the core of the problem is that the interface exposed by the mount command requires paths for sources, while mount only requires a character string. Why does zfs mount need to defer to mount to call mount.zfs ?

@pstch
Copy link

pstch commented Nov 18, 2018

This actually seems to be a non-issue. --no-canonicalize doesn't prevent much as --options cannot be used by non-root users either. I don't see how adding it prevented anything, and trying to reproduce the problem without --no-canonicalize returns the same error but for --options.

Also, I don't think this is related to #6865, as mounts in an user namespace can be done by the root user in that namespace (mount checks the inner uid/gid). Allowing zfs mount in unprivileged containers/user namespaces does not require removing --no-canonicalize.

I think that --no-canonicalize should be accepted as the proper solution for the issues it fixed, (especially as the source is not a path when using ZFS, so canonicalizing it is a bad idea anyway.) and that this issue should be closed.

@behlendorf
Copy link
Contributor Author

@pstch thanks for digging in to this one. Since I originally opened this one I'm happy to close it out as a non-issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Indicates a good issue for first-time contributors
Projects
None yet
Development

No branches or pull requests

4 participants