mount.ecryptfs_private setuid binary

[Dustin Kirkland ]

ecryptfs-utils currently provides a setuid binary:
 /sbin/mount.ecryptfs_private

Also, /sbin/umount.ecryptfs_private is a symbolic link to this same file, providing the umount functionality.

This utility is what allows a normal user to mount and unmount their encrypted home or encrypted private directory.

The code drops root privileges immediately upon execution, sanitizes and validates all input and configuration parameters, and then resumes root privileges again for exactly one of two specific operations, one of:
 * mount()
 * execl(umount)

In both cases, the parameters to mount and umount have been checked such that the only directory the user is able to mount and unmount is their own private/home directory.

Still, setuid programs should be avoided, or at least constrained as much as possible.

For one thing, this program could become setgid, and restricted to an ecryptfs group.

Additionally, this program could perhaps make use of filesystem capabilitys, such as CAP_SYS_MOUNT.

This is a wishlist bug suggesting that this binary should be updated accordingly.

:-Dustin

[Serge Hallyn (serge-hallyn)]

Note that there is no CAP_SYS_MOUNT. You need CAP_SYS_ADMIN to mount a filesystem.

I'm not sure what else the helper programs might do which would require privilege, but it
shouldn't be too bad.

I've got this down as something to play with next week.

[Dustin Kirkland  (kirkland)]

The ONLY operations that are actually executed as setuid(0) are the
actual mount and unmount calls:

    mount case:
                setreuid(-1, 0);
                /* Perform mount */
                if (mount(dev, mnt, FSTYPE, 0, opt) == 0) {
                        if (update_mtab(dev, mnt, opt) != 0) {
                                goto fail;
                        }
                } else {
                        perror("mount");
                        /* Drop privileges since the mount did not succeed */
                        if (setreuid(uid, uid) < 0) {
                                perror("setreuid");
                        }
                        goto fail;
                }

    umount case:
                setresuid(0,0,0);
                execl("/bin/umount", "umount", "-i", "-l", mnt, NULL);
                perror("execl unmount failed");
                goto fail;

:-Dustin

[Serge Hallyn (serge-hallyn)]

The following two patches are in no way intended to be applied, they're just for
conversation.

The one against mount.ecryptfs_private makes it use capabilities instead of
setuid. If we do a mount, it puts cap_sys_admin and cap_dac_override in
pE so it can do mount(2) and write to /etc/mtab.

Currently for umount, mount.ecryptfs_private does execl("/bin/umount"). So
the patch puts those capabilities in pI in the hopes that umount will have
them in fI.

The /bin/umount patch is needed because ordinarily umount refuses to unmount
an entry not in fstab unless the caller is euid=0. So the patch instead checks
whether the needed capabilities are in pE.

Finally, with these patches applied you need to do

  capset cap_sys_admin,cap_dac_override=pe /sbin/mount.ecryptfs_private
  capset cap_sys_admin,cap_dac_override=ie /bin/umount

[Serge Hallyn (serge-hallyn)]

/bin/umount checks whether the caller is euid=0 before trying to umount an fs not
listed in fstab. This patch changes the code to check whether pE has cap_sys_admin
and cap_dac_override.

Note that I don't think this patch will be palatable to most of the community. I think the
way to go is to have umount.ecryptfs_private update /etc/mtab itself and use umount(2),
instead of execing /bin/umount. Then /bin/umount can remain untouched for now.

[Serge Hallyn (serge-hallyn)]

Finally, note that the config scripts will need to be updated to check for and compile
in capabilities, and presumably a backup to setuid root will need to be coded in.
(I just coded the setuid code out altogether)

[Dustin Kirkland  (kirkland)]

I'm assigning this to myself.

We're past feature freeze, so this won't make Karmic. But I should have some cycles to focus on this after Karmic releases. I'll do some testing then.

Thanks Serge.

:-Dustin