NodePublishVolume target_path needs additional clarifications

[julian-hj]

The spec says:

  // The path to which the volume will be published. It MUST be an
  // absolute path in the root filesystem of the process serving this
  // request. This is a REQUIRED field.
  string target_path = 5;

However, the spec doesn't describe:

1. Whether the plugin should expect the directory at the target_path to already be created by the CO or the plugin is responsible for creating the directory at target_path itself.
2. If the CO pre-creates the directory, what permissions the plugin can expect it to have.
3. If the plugin must create the directory, what permissions the parent path should have.
4. If more than one directory in the path must be created by the plugin, what permissions the parent directories should be given in order to not break other plugins that will perhaps use the same path.

Points 2 and 3 are mainly relevant for plugins that use fuse mounts and will not run with CAP_SYS_ADMIN. One advantage to allowing the plugin to determine its own mountpoint is that the plugin can be responsible for making sure that it has create rights on the path where the mount points will go, even if it is running as some arbitrary non-root user.

As the spec is written, the CO is responsible for dictating the mount path so I suppose that the mount points must be open for world write, or the CO needs some way to know what user the plugin will run as and chown the directory to that user.

[jieyu]

I think CO should be responsible for creating the target_path. Regarding the permission issue for fuse mounts, fuse program will fork/exec fusermount, which will do the actual mount syscall. fusermount is a setuid program. Exec'ing it will give the process an effective uid of 0. So it should be about to access to the CO created mount point.

vagrant@vagrant-ubuntu-trusty-64:~$ stat -c "%a %U:%G %n" /bin/fusermount
4755 root:root /bin/fusermount

[julian-hj]

Hey, sorry for the slow response. I think it's fine if the CO is responsible to create the mount point, but I wanted to actually experiment with fuse mounts before responding. Sadly it doesn't appear that fusermount is capable/willing to work with mount points that the plugin user lacks write permission to. Here's what I get when I try to fuse mount a directory created by root with 0755 permits:

cvcap$ fuse-nfs -n nfs://10.10.200.72/export2/certs -m /var/vcap/data/volumes/nfs/myfoo2
fusermount: user has no write access to mountpoint /var/vcap/data/volumes/nfs/myfoo2

[jieyu]

Ah, fusermount uses access to check if it has write access to the mount point.
https://github.com/fuse4x/fuse/blob/master/util/fusermount.c#L934-L945

		if ((stbuf->st_mode & S_ISVTX) && stbuf->st_uid != getuid()) {
			fprintf(stderr, "%s: mountpoint %s not owned by user\n",
				progname, origmnt);
			return -1;
		}

		res = access(mnt, W_OK);
		if (res == -1) {
			fprintf(stderr, "%s: user has no write access to mountpoint %s\n",
				progname, origmnt);
			return -1;
		}

And, access will use real user id (rather than effective user id) to perform the check:

The access() system call uses the real user ID in place of the effective user ID, the real group ID in place of the effective group ID, and the rest of the group access list.

So look like the CO needs to make sure plugin have access to the mount point. Since it's just a mount point, i think it's ok to give the mount point 777 permission? We definitely need to clarify that in the spec. Thanks for catching this.

[julian-hj]

Sounds good, thanks!

[jdef]

should we target this for the v0.1 milestone?

[julian-hj]

Yeah, we probably should. I will try to submit a PR today.