Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some problems with /etc/fstab #233

Closed
wants to merge 2 commits into from
Closed

Conversation

mahkoh
Copy link
Contributor

@mahkoh mahkoh commented May 6, 2018

These patches fix some problems I encountered while using gocryptfs via /etc/fstab. See the commit messages for more details.

mahkoh added 2 commits May 6, 2018 17:41
mount passes `-o noexec` if `-o user` is set and `-o exec` is not set.
If both `-o user` and `-o exec` are set, it passes `-o exec`. We handle
both cases by ignoring these arguments.
mount(1) unsets PATH before calling mount.fuse. Therefore it's not set
in gocrpytfs either and daemonization fails if gocryptfs was not
executed via an absolute path.

mount.fuse handles this by leaving the execution of the helper to
/bin/sh. /bin/sh handles an empty PATH by searching a few default
locations.

This patch sets the PATH to a sane default if it's empty or unset.
@rfjakob
Copy link
Owner

rfjakob commented May 7, 2018

Hi, thanks for the changes!

About -noexec, I wonder if we should actually mount with -noexec instead of ignoring it.

rfjakob added a commit that referenced this pull request Jun 7, 2018
When mounted via /etc/fstab like this,

  /a /b fuse.gocryptfs default 0 0

we always get extra options passed. As reported by @mahkoh
at #233 :

  mount passes `-o noexec` if `-o user` is set and `-o exec` is not set.
  If both `-o user` and `-o exec` are set, it passes `-o exec`.

Make these options work, and in addtion, also make -suid and -rw
work the same way.

Reported-by: @mahkoh
@rfjakob
Copy link
Owner

rfjakob commented Jun 7, 2018

Merged with some modifications as 10212d7 and 53d6a99 - thanks!

@rfjakob rfjakob closed this Jun 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants