search

ericpaulbishop / gargoyle

Gargoyle Router Management Utility
http://www.gargoyle-router.com
468 stars 221 forks source link

Samba shares permissions #870

Closed obsy closed 4 years ago

obsy commented 4 years ago

Storages are mounted in folder with 777 permission. If samba share root folder (/) everything is ok, user can create directories etc. But if share is not root but folder (ex. /test) for specific user, it is only created, here https://github.com/ericpaulbishop/gargoyle/blob/master/package/plugin-gargoyle-usb-storage-noshare/files/www/js/usb_storage.js#L192 , with default permission 755, so this specific user cannot write anything in this share. Question: should we set 777 on this folder? What if there is other share on parent folder with other permission (user could see/write something in this folder)

lantis1008 commented 4 years ago

I can reproduce, thanks. I'm struggling with the root share permissions though.

root@Gargoyle:~# ls -al /tmp/usb_mount
drwxr-xr-x    3 root     root            80 Apr 14 15:19 .
drwxrwxrwt   26 root     root           720 Apr 14 15:21 ..
lrwxrwxrwx    1 root     root            26 Apr 14 15:19 dev_sda1 -> /tmp/usb_mount/f4b6b312-01
drwxr-xr-x    5 root     root          4096 Apr 14 15:21 f4b6b312-01

The shares are using f4b6b312-01, not dev_sda1, so permissions are 755 there as well. Perhaps this should be fixed as well? Can you confirm similar behaviour? Will also need to check the interaction with FTP and NFS when fixing all of this.

Question: should we set 777 on this folder? What if there is other share on parent folder with other permission (user could see/write something in this folder)

That is the quick solution. Your point about other user is valid. An alternative is to chgrp the folder, but this might be intrusive to the user (e.g. folder already exists, don't mess with my permissions!). If we just do 777, it is easy to explain that root share can see all below shares. If you don't want this, you need to rethink how your shares are setup.

Thoughts? Or should this be more sophisticated?

By the way, i didn't spot this before because i always test with FAT32 external disks, and they don't care about permissions at all so it works just fine.

obsy commented 4 years ago

IMO we should set created folder on 777.

lantis1008 commented 4 years ago

Ok, i will look at fixing this. By the way, mount -t ext4 -o noatime "$d" "/tmp/usb_mount/$id" in usb_storage init.d is reducing permissions from 777 to 755. Fascinating.

lantis1008 commented 4 years ago

My mistake, it is the drives permissions that were incorrect. OK just your issue to fix. Thank you! I will look at other issue soon also.

lantis1008 commented 4 years ago

Backported to 1.12 as well.