xbmc
xbmc copied to clipboard
Adds UID / GID and permissions features in NFS Connections
Description
Adds the ability to set the UID/GID used for the connection to the NFS server. These two new parameters are added to the NFS client service: the UID value and the GID value. They are conditioned by a boolean parameter which if disabled leaves the UID/GID defined by the underlying OS. Also fixes inappropriate permissions applied when creating files and directories with NFS version 4. Adds the ability to set desired permissions when creating files and directories.
Motivation and context
The UID/GID assigned by Android are not necessarily identical from one installation to another. So when libraries and databases are shared between several Kodi installations on different OS, it becomes complex or even impossible to set up functional ACLs for all the different installations unless you overload everything with the squash features of the NAS server. With this capability, it is now possible for all installations to use the same UID/GID and thus simplify the ACLs. In addition, the ability to explicitly set permissions finalizes the consistency of ACLs.
How has this been tested?
The test was done using the settings file widget to create a folder and copy files to the NAS (without squash option in the export) and then check that the UID/GID of the folder on the NAS file system are identical with those defined. The tests were done with Android and Windows.
What is the effect on users?
The effect is a simpler procedure to set up a shared library between different kodi's installations.
Screenshots (if appropriate):
Types of change
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Clean up (non-breaking change which removes non-working, unmaintained functionality)
- [X] Improvement (non-breaking change which improves existing functionality)
- [X] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that will cause existing functionality to change)
- [ ] Cosmetic change (non-breaking change that doesn't touch code)
- [ ] None of the above (please explain below)
Checklist:
- [X] My code follows the Code Guidelines of this project
- [ ] My change requires a change to the documentation, either Doxygen or wiki
- [ ] I have updated the documentation accordingly
- [X] I have read the Contributing document
- [ ] I have added tests to cover my change
- [ ] All new and existing tests passed
You didn't want to just reopen the old pull request? #23271
I wanted to change the desciption and I am not a GIT expert. So the simplest way for me was to open a new PR.
Hello @lrusak, I added permissions features due to the following use cases. Kodi should should have its own UID different from other user of the NAS. So with a 644 permissions the other users can only read files (no ability to modify or remove them. This can be done only with a common group and a 664 permissions. This permissions is required even if you set extended acls with setfacl as long as the effective permissions is a bitwise and between extended acls and file permissions. Also, in general, removing read access to "other" is a good practise. For examples, in my home network I have setted up some services which shares files with kodi boxes but each one has a different UID to identify each and a common group to share.
I think the ability to change permissions via settings need more discussion.
First of all, the current code has a problem creating/copying files or creating directories from Kodi to the server with NSF V4. It does not set permissions in this case.
As a reminder, the default permissions (0755 for directories and 0644 for files) under Unix are defined so that the user who creates a file/directory is the only one who can modify it until he/she decides to adapt them to obtain the appropriate permissions. In this way, only the owner and the root user can do so.
In general, in large productions, the umask is changed from 0022 to 0027 to ensure that the confidentiality of files is respected.
In the case of Kodi, to respect this logic, Kodi would have to allow the right permissions to be set after copying or creating, which is not compatible with its purpose. However, the rights currently applied by default do not allow another standard user (different UID) to be able to delete/modify the files and directories created by Kodi on the NFS server. To modify the permissions, the end user must connect as root (or play with sudoers) in console to do it on the created objects or schedule a batch by cron to do it regularly.
So I think it is better to leave the possibility to apply directly the desired permissions by the end user rather than imposing something which will be inefficient for him. The discussion on issue #23162 highlights precisely this need.
Hi, @lrusak. I do not understand why allowing end user set his own permissions seems to be a problem. The default values proposed in the feature respects the unix default ones. As I tried to explain few messages ago, default permissions does not suit to all users including me.
Here the difference between 660 and 644, my account and the kodi one have the "users" group as primary The two files are copied from Kodi with nfs v4 or v3
root@????:/data/kodi/test# ls -l
-rw-r--r--+ 1 kodi users 1 May 30 23:35 test-0644.txt
-rw-rw----+ 1 kodi users 10 May 31 00:38 test-0660.txt
root@????:/data/kodi/test# getfacl -e *
# file: test-0644.txt
# owner: kodi
# group: users
user::rw-
user:philippe:rwx #effective:r--
user:kodi:rwx #effective:r--
group::r-x #effective:r--
group:users:rwx #effective:r--
mask::r--
other::r--
# file: test-0660.txt
# owner: kodi
# group: users
user::rw-
user:philippe:rwx #effective:rw-
user:kodi:rwx #effective:rw-
group::r-x #effective:r--
group:users:rwx #effective:rw-
mask::rw-
other::---
Hello @lrusak, It has been 6 months now that I have initiated the discussion on the management of permissions by Kodi without any response or comment in particular on your part. I am still looking for the motivations to open a discussion on the merits of my proposal as long as it also corrects a bug noted by several other users.
Thus, I remain convinced that letting the identifiers (UID/GID) choose without the associated permissions is like making a bridge without the access roads.
I have 25 years of experience as an architect of solutions on sensitive IT systems (defence, bank ...) with thousands of nodes based on all possible OS (a huge nightmare in reality). Although I am not a "security expert", my experience allows me to have a deep "security" approach in the architectures I design.
Thus, I do not think that the proposed evolution will introduce a vulnerability in Kodi (quite the opposite) by allowing a Kodi user to position the permissions he wishes under his own responsibility.
Hi @neo1973, Yes you are right. The discussion has stalled unilaterally by @lrusak for months now even if my proposal could fix two issues which affects all users who wants to use NFSv4 regardless of the hosting OS and also the uid/gid on android. I demonstrated the convenience of fixing the two issues and listed arguments without any answer. I read "would recommend splitting" thinking that my arguments would convince that this recommendation has no sense as long as the proposal solves two linked issues. The reality is that there is only one thing which can't be discussed : the recommendation itself. My patience is now running out. So I built my own factory to get my private version of Kodi. Sorry for others Such a waste of time. Next time I'll have to solve conflicts I'll simply close to PR.
What is the normal process of trying to re-open discussion on this issue? It looks like this is worth getting done & merged in given it will fix a serious NFSv4 bug and make NFS more viable for Kodi users generally.
If you find it, you'll save the planet
@lrusak Comment? You're busy with life so delay forever is allowed.