rage icon indicating copy to clipboard operation
rage copied to clipboard

rage-mount: update dependencies fuse_mt, time and zip

Open folliehiyuki opened this issue 3 years ago • 0 comments

I picked the first commit of fuse_mt that switched from using fuse to fuser (fuse_mt hasn't had any new releases since 0.5.1). That way rage-mount can be built against libfuse3. time and zip dependencies update is a side effect due to TimeSpec being obsolete.

WARNING: I've never written any Rust code before, so please bear with my ignorance in the PR.

folliehiyuki avatar Jun 02 '22 13:06 folliehiyuki

rage-mount is passing the auto_unmount option:

https://github.com/str4d/rage/blob/26f99e2149110591024367275fbaa3d2cbe75878/rage/src/bin/rage-mount/main.rs#L167-L171

So my guess is that this is related to the migration from the fuse crate to the fuser crate.

str4d avatar Aug 21 '22 14:08 str4d

Force-pushed to fix formatting errors.

str4d avatar Aug 21 '22 15:08 str4d

Codecov Report

Merging #324 (5e212a7) into main (26f99e2) will increase coverage by 0.11%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
+ Coverage   37.65%   37.77%   +0.11%     
==========================================
  Files          34       34              
  Lines        2998     3002       +4     
==========================================
+ Hits         1129     1134       +5     
+ Misses       1869     1868       -1     
Impacted Files Coverage Δ
rage/src/bin/rage-mount/main.rs 0.00% <0.00%> (ø)
rage/src/bin/rage-mount/tar.rs 0.00% <0.00%> (ø)
age/src/format.rs 50.54% <0.00%> (-1.68%) :arrow_down:
age/src/protocol.rs 61.53% <0.00%> (-0.89%) :arrow_down:
age/src/ssh.rs 23.65% <0.00%> (-0.82%) :arrow_down:
age-core/src/format.rs 61.19% <0.00%> (-0.75%) :arrow_down:
age-core/src/plugin.rs 30.72% <0.00%> (-0.61%) :arrow_down:
age/src/primitives/stream.rs 62.38% <0.00%> (+0.18%) :arrow_up:
age/src/primitives/armor.rs 41.66% <0.00%> (+0.81%) :arrow_up:
age/src/ssh/identity.rs 52.77% <0.00%> (+0.89%) :arrow_up:
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 21 '22 15:08 codecov[bot]

Looks like the UX regression is due to https://github.com/cberner/fuser/pull/122, which I don't entirely understand because I can't find a canonical reference that specifies the requirement of either allow_other or allow_root.

str4d avatar Aug 21 '22 15:08 str4d

Aha, this is upstream issue https://github.com/libfuse/libfuse/issues/586. Perhaps this worked for libfuse2 but doesn't for libfuse3. I think the fix is to add allow_root, so that we widen access just enough to support auto-unmounting (which I want to ensure that the rage-mount process getting aborted doesn't leave a dangling filesystem) while not making the mount globally accessible.

str4d avatar Aug 21 '22 15:08 str4d

Oh, except that fuser implements allow_root itself, by enabling allow_other and then restricting to owner+root. So we can't use auto_unmount without requiring rage-mount users to add user_allow_other to their /etc/fuse.conf, which is a system-wide configuration 😢

str4d avatar Aug 21 '22 15:08 str4d

Thanks for all the comments.

I'm pretty sure rage-mount worked for me the last time I updated the PR. Maybe it was just me not using auto_unmount.

folliehiyuki avatar Aug 21 '22 16:08 folliehiyuki

I'd say we keep note in the README about the behavior and reference it to the upstream issue. It's unlikely to get fixed soon.

I don't know how fuser handles stuff, but fusermount command (from fuse3 package) allows users to override mount options at runtime. Maybe we can do that with rage-mount?

folliehiyuki avatar Aug 21 '22 16:08 folliehiyuki

Thanks for all the comments.

I'm pretty sure rage-mount worked for me the last time I updated the PR. Maybe it was just me not using auto_unmount.

auto_unmount is hard-coded into rage-mount, so you would have used it unless you modified the code. Do you have an /etc/fuse.conf file with user_allow_other (or any other settings) in it?

str4d avatar Aug 21 '22 16:08 str4d

I'd say we keep note in the README about the behavior and reference it to the upstream issue. It's unlikely to get fixed soon.

I think what I want to do is add allow_root to keep the required access minimal, try and detect if the mount fails for this reason, and in that case warn the user and retry the mount without auto_unmount,allow_root. That way the user's action has a much greater chance of succeeding, with the downside that if the user kills the rage-mount process uncleanly, they may need to do some manual cleanup (and they also have a way to avoid this issue if they are willing to enable user_allow_other).

I don't know how fuser handles stuff, but fusermount command (from fuse3 package) allows users to override mount options at runtime. Maybe we can do that with rage-mount?

I'm not inclined to provide mount option configurations, in particular because there are options (specifically ro) that we need to enforce, but also because I do not see a use case for this kind of configuration. If someone can provide one, we can consider it separately.

str4d avatar Aug 21 '22 17:08 str4d

I think what I want to do is add allow_root to keep the required access minimal, try and detect if the mount fails for this reason, and in that case warn the user and retry the mount without auto_unmount,allow_root.

Ugh, this is made more annoying by the fact that the Filesystem impl is consumed by mount, and the relevant config check happens inside the external fusermount binary, so we'd need to either read and decrypt the age file twice (which means caching any identities or password), or figure out where the fuse.conf file is (its location differs between fuse2 and fuse3) and parse it ourselves.

Instead, I'm going to attempt to detect the error and provide a more useful error message.

str4d avatar Aug 21 '22 20:08 str4d

I just rebuilt rage again on this branch. rage-mount errors out now.

Sorry for the misinformation.

folliehiyuki avatar Aug 22 '22 13:08 folliehiyuki

Once https://github.com/cberner/fuser/pull/218 is merged and released, the "Error: Success" message will be replaced by "Error: Unknown Error" which is an improvement, but still not great. The blocker on handling this error well is libfuse itself exposing sufficient error information: https://github.com/libfuse/libfuse/issues/693

str4d avatar Aug 22 '22 18:08 str4d

I figured out how to retain the current behaviour: by using spawn_mount instead of mount, and then triggering a channel from both a Ctrl-C handler and FilesystemMT::destroy, we can always cleanly unmount the filesystem on Ctrl+C without auto_unmount, while also reacting correctly to external unmounts.

I've tested this locally and it appears to work fine for me. @FollieHiyuki can you confirm this works for you?

str4d avatar Sep 03 '22 13:09 str4d

Actually, I'm going to go ahead and merge this, to unblock other PRs. If you encounter any problems, please open new issues. And thank you for the PR!

str4d avatar Sep 03 '22 15:09 str4d

I tried it out just now. Works as expected.

Thanks for doing all the heavy lifting in this PR.

2022-09-04-001556_grim

folliehiyuki avatar Sep 03 '22 17:09 folliehiyuki