Zygo / bees

Best-Effort Extent-Same, a btrfs dedupe agent
GNU General Public License v3.0
630 stars 57 forks source link

Further sandbox beesd using systemd.exec options #190

Closed NobodyXu closed 2 years ago

NobodyXu commented 2 years ago

I've verified that using this setup, user will be able to access the log in /run/bees, but cannot access the mounted filesystem.

Signed-off-by: Jiahao XU Jiahao_XU@outlook.com

NobodyXu commented 2 years ago

Since beesd right now is hard coded to ensure it runs as root instead of examing the capabilities, I cannot use “DynamicUser” option to run the daemon as an dynamically created, non root, random user which has no write access to any part of the system.

It would be better if beesd can checks for required capabilities instead.

Edit:

Seems like uid checking is done by beesd.in, then I can use capsh to check for the required capabilities and perhaps also drop CAP_SYS_ADMIN before executing beesd.

kakra commented 2 years ago

What would DynamicUser mean for the persistent state files created by bees, that is the hash and progress files?

NobodyXu commented 2 years ago

@kakra OOPs, didn’t consider that.

It would mainly affect newly created files and directories, causing them to be owned by some random user.

It wouldn’t affect accessing existing file and with the capabilities I set, it should be able to access the files just fine.

Though it seems that with persistent state, it would be better to create a new user beesd and run beesd as that.

NobodyXu commented 2 years ago

@kakra Seems like DynamicUser isn’t a good idea, but none the less using capsh to check capabilities instead of checking for root and dropping CAP_SYS_ADMIN before executing the actual binary is still doable.

kakra commented 2 years ago

systemd has ways of creating sysusers via configuration, the uid should persist across the life time of the OS installation.

NobodyXu commented 2 years ago

systemd has ways of creating sysusers via configuration, the uid should persist across the life time of the OS installation.

Thanks for the advice!

After a bit googling, I found sysuser.d for automatically creating non-interactive user for servoce.

NobodyXu commented 2 years ago

@kakra Turns out CAP_SYS_ADMIN is necessary for beesd.

Removing it makes beesd throws

crawl_transid[3274]:         exception type std::system_error: BTRFS_IOC_TREE_SEARCH_V2: /run/bees/mnt/a7273dfd-4e68-4572-870a-76c51182b871 at fs.cc:762: >

on every crawl.

NobodyXu commented 2 years ago

@kakra I think I will submit it in another PR once this one is merged.

kakra commented 2 years ago

Turns out CAP_SYS_ADMIN is necessary for beesd.

Might be some (expected or unexpected) btrfs thing... Tree search probably needs CAP_SYS_ADMIN and there's no way around it.

Zygo commented 2 years ago

Where are we on this PR and #189? Should I pull them and tag v0.7, or tag v0.7 with what we have now, and pull these immediately after?

Zygo commented 2 years ago

bees needs to have full write access to the deduped filesystem. A lot of it is hidden behind the oddball dedupe access rules, where you can dedupe files that have been opened O_RDONLY if you are root or own the file, but they definitely need to be writable by the bees process.

There are also O_TMPFILE files which require write access to the root directory of the filesystem (though that could be any directory on the filesystem--right now it uses the subvol root because that directory must exist and bees running as root can write to it).

Zygo commented 2 years ago

Welp...apparently git push --no-tags pushes tags. Who knew? :-P

NobodyXu commented 2 years ago

@Zygo I would prefer #189 to be merged in, since it removes the unused and confusing new option MOUNT_OPTIONS I introduced in #188 (but it does preserve PRIVATE_MOUNT I introduced in that commit)

Zygo commented 2 years ago

I've accidentally merged them both. I created the v0.7 tag but didn't intend to push it, then pushed it...and now we have a v0.7 tag.