Fix non-root permissions and allow custom UID & GID - #484
Open
misch7 wants to merge 3 commits into
Open
Conversation
…ustom non-root UIDs
enterpoint.sh: - create missing directories, if needed - allow the user to set custom `UID` and `GID` for the container-user `seafile` via optional parameters (default remains 8000:8000) - (re)set the same secure permissions the volume folders initially had with the standard root-user setup - only change the owner recursively if the main folders have another owner, to avoid unnecessary overhead with large storages - leave the app folders and binaries untouched but grant write access to `/opt/seafile` (e.g. for creating the `pids` folder) and the `runtime` folder, in the container bootstrap.py: - remove the chmod code block in init_seafile_server()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Hi,
I'm new to Seafile, have been using it for about a week now and I'm quite satisfied with it :-)
There were some issues setting up the non-root environment with the
seafile-dockercontainer, for which I aim to provide a fix with this PR.Issues
Starting the container with
NON_ROOT=truealways failed on my machine due to an indefinitely hanging call tochown:seafile-docker/scripts/scripts_13.0/enterpoint.sh
Line 43 in ef8e107
While following the official docs for Running Seafile as non root user inside docker somehow I expected the startup script to securely adjust permissions after preparing them manually before with
chmod -R a+rwx /opt/seafile-data/.However, since they never got adjusted, I began inspecting the scripts, leading me to identify the following issues (if I did not miss anything anywhere else):
The
bootstrap.pyscript (called bystart.py) contains the following snippet at the end of its functioninit_seafile_server():seafile-docker/scripts/scripts_13.0/bootstrap.py
Lines 163 to 166 in ef8e107
But
enterpoint.shexpects those exact permissions already to be set and terminates prior to invoking the aforementioned function, causing thebootstrapcode for setting permissions never to be called.The permissions never get properly adjusted later and just stay overly permissive with
777, which I confirmed running the container for a while. Also with restarts nothing changes.Additionally the current scripts also set the owner of the whole Seafile app binary directories to the
seafileuser. In my understanding one advantage of the wholeNON_ROOTprocedure would have been to leave the app binaries owned byrootbut called byseafile, who owns just the data and runtime folders it needs write access for.My wild guess is that these permissions were chosen for ease of use?
Issues #22 and #86 have been closed but never were fully resolved, as it seems.
To address all those issues I propose the following changes in this PR and leave them up to discussion:
Changes
The
chmodcode block inbootstrap.pygets removed.enterpoint.shnow:NON_ROOT_UIDandNON_ROOT_GIDfor the container-userseafilevia optional parameters (default remains 8000:8000)/opt/seafile(e.g. for creating thepidsfolder) and theruntimefolder, in the containerDockerfileremoves the standard userubuntuwith UID 1000 to avoid conflicts with custom non-root UIDs.In
.envthe following can now be specified:This would also require a change within
seafile-server.yml:Side note: I did not name the new variables
UID/GIDin.envbecause users might expect a different behavior then. Since there are other services likenginxstill running with higher privileges, naming themNON_ROOT_UIDandNON_ROOT_GIDallows to easily distinguish this special non-root use case until these other services eventually get sourced-out into their own containers.Please also note that I only modified the code for Seafile 13 CE for now by intention, to ease the review.
Suggestions and tips for improvements are welcome.
Questions
As stated in this Forum Post, the Docker sources for the Seadoc container are not available publicly yet.
I assume the sources for
seafile-server.ymletc. are also not yet available for patching in a repository? At least I did not manage to find them.Of course the Seadoc container's
enterpoint.shscript would need patching as well.Has the plan changed to publish the missing scripts? I would gladly help to apply the patches to other containers like Seadoc, if they get accepted.
Testing
For anyone interested in testing the changes, here's how I applied them locally:
Dockerfilebased on the official image to apply your patches:docker build -t seafile-mc-13-patched ..envwith your desiredNON_ROOTparameters, comment out theSEAFILE_IMAGEand specify the custom image, like:Log Output
The output of
docker logs seafileshould then look like:Offtopic
notification-server.ymlhasrestart: alwaysbut should haverestart: unless-stoppedlike the other containers.