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

permissions error for chmod on new socket files w/ force_owner #629

Closed
brainchild0 opened this issue Dec 29, 2021 · 11 comments
Closed

permissions error for chmod on new socket files w/ force_owner #629

brainchild0 opened this issue Dec 29, 2021 · 11 comments

Comments

@brainchild0
Copy link

I have tried working with version 2.2.1-18-g5749e70 (go-fuse v2.1.1-0.20211219085202-934a183ed914; 2021-12-26 go1.13.8 linux/amd64) (built locally from development repository, commit 5749e70), and discovered what I would describe as a regression.

Permission is denied for opening a socket file on the mounted file system.

The problem is not appearing with gocryptfs 1.7.1 (go-fuse 0.0~git20190214.58dcd77) (available through distribution).

Note that I have only tested with force_owner and nonempty options in effect. Options are the same on both tests (expressed with -- syntax on later version). The user attempting to access the socket is the user receiving the forced ownership. This user has sufficient permissions for the file on both the mounted and ciphered sides.

@rfjakob rfjakob added the bug label Jan 3, 2022
@rfjakob
Copy link
Owner

rfjakob commented Jan 3, 2022

Hi, thanks for the report. I tried to reproduce the problem like this:

As user 1027:

gocryptfs -force_owner=1026:1026 a b

As user 1026:

cd b
nc -U -l nc.sock

2nd terminal:

cd b
nc -U nc.sock

...but this seems to work ok.

Can you reproduce the issue with nc?

@brainchild0
Copy link
Author

brainchild0 commented Jan 9, 2022

After further investigation, it seems I may have hastily reached some inaccurate conclusions.

The issue may not be a regression, and is not related to sockets as much as permissions.

It seems now simply that when a mount is made with either the --acl or --force_owner option, then changing permissions of a file in the mount.

Is this behavior intentional?

@rfjakob
Copy link
Owner

rfjakob commented Jan 9, 2022

Hmm, cannot reproduce

$ chmod 000 nc.sock
$ chmod 777 nc.sock
$ ls -l
total 0
srwxrwxrwx. 1 jakob jakob 0  9. Jan 11:45 nc.sock

@brainchild0
Copy link
Author

I am having trouble reproducing too, which may simply speak to the sloppiness of my testing.

However, where I notice a consistent problem is running an application built on Twisted, which depends on a socket for IPC.

The process is to create a socket and to give it mode 666, which consistently fails, except when I run it with a break point. Then, as long as I continue manually without any further interaction, the operation succeeds.

These observations suggest a race condition in Twisted, but as it seems none have yet been reported, it may at least be useful also to consider whether the current design or implementation in Gocryptfs opens the possibility for such issues more so than other file systems.

@rfjakob
Copy link
Owner

rfjakob commented Jan 9, 2022

Thanks, the link helped. I can reproduce this now with nc:

$ rm -f nc.sock ; nc -U -l nc.sock & sleep 0.01 ; chmod 777 nc.sock
[1] 59621
chmod: changing permissions of 'nc.sock': Operation not permitted

@rfjakob
Copy link
Owner

rfjakob commented Jan 9, 2022

Bug in gocryptfs! And this is the fix, will merge soon:

diff --git a/internal/fusefrontend/node.go b/internal/fusefrontend/node.go
index 182cda5..21348f2 100644
--- a/internal/fusefrontend/node.go
+++ b/internal/fusefrontend/node.go
@@ -278,6 +278,11 @@ func (n *Node) Mknod(ctx context.Context, name string, mode, rdev uint32, out *f
                return
        }
        inode = n.newChild(ctx, st, out)
+
+       if rn.args.ForceOwner != nil {
+               out.Owner = *rn.args.ForceOwner
+       }
+
        return inode, 0
 }
 

@rfjakob
Copy link
Owner

rfjakob commented Jan 9, 2022

PS: The reason that the breakpoint fixed things in your testing is that the problem disappears when the FUSE attribute cache expires after 2 seconds.

@brainchild0
Copy link
Author

brainchild0 commented Jan 10, 2022

I'm glad you were able to find a way to reproduce.

I was beginning to feel quite confused about the inconsistency, and embarrassed about the quality of the test cases.

For the case you reproduced, the failure depends on an entry previously existing with the same name, leaving artifacts not cleared synchronously?

Would the patch be safe to overlay on the head of master for regular use, at least as safe as the current head, or is more testing required?

@rfjakob
Copy link
Owner

rfjakob commented Jan 10, 2022

When a socket file is created (MKNOD fuse command), gocryptfs sends the file attributes of the new file to the kernel. The bug is that gocryptfs does not apply force_owner to the attributes at this point. The kernel remembers these attributes for two seconds. After the two seconds, the kernel queries the file attributes again (GETATTR fuse command), and gocryptfs does apply force_owner.

Patch is completely safe.

@brainchild0 brainchild0 changed the title socket access regression for v2.2.1 (from v 1.7.1) permissions error for chmod on new socket files w/ force_owner Jan 10, 2022
@balupton
Copy link

Ran into this myself:

gocryptfs --rw --noprealloc --allow_other --force_owner 1001:1002 /mnt/tank/TankCipher /media/TankSecure

Then doing any chmod or chown inside /media/TankSecure fails with Operation not permitted

@rfjakob
Copy link
Owner

rfjakob commented Jan 10, 2022

@balupton I'm not sure if you are seeing the same issue, but if it you are, the patch I just pushed fixes it.

@balupton balupton mentioned this issue Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants