LycheeOrg / Lychee-Docker

Docker image for Lychee
https://lycheeorg.github.io/
165 stars 56 forks source link

Lychee container should not modify files outside of mounts #205

Closed girlpunk closed 3 weeks ago

girlpunk commented 3 weeks ago

At present, the Lychee container writes to several locations (such as /var/www/html/Lychee/) inside the container. Docker containers should not write to any locations outside of those mounted (through a volume or host mount) for this. If Lychee requires files to be mounted in these locations, users should be instructed to mount to this directory (such as /var/www/html/Lychee/public/uploads), or the install of Lychee in the container should be configured to use the expected directory directly.

Furthermore, this prevents running the container as a specific user to correctly manage file permissions where uploaded photos are to be accessed by multiple applications or users, as this causes the entrypoint to crash on startup (see logs below).

lychee-684d458f54-2bbbm -f 

-------------------------------------
  _               _                
 | |   _   _  ___| |__   ___  ___  
 | |  | | | |/ __|  _ \ / _ \/ _ \ 
 | |__| |_| | (__| | | |  __/  __/ 
 |_____\__, |\___|_| |_|\___|\___| 
       |___/                       

-------------------------------------
Lychee Version: 5.5.1 (release)
Lychee Branch:  master
Lychee Commit:  656ffae
https://github.com/LycheeOrg/Lychee/commit/656ffaed2027da370b0b84cdd6d846d5fa5c028f
-------------------------------------
**** Make sure the /conf /uploads /sym /logs folders exist ****
**** Create the symbolic link for the /uploads folder ****
rm: cannot remove '/var/www/html/Lychee/public/uploads/medium2x/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/import/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/small2x/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/thumb2x/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/thumb/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/tracks/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/medium/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/small/index.html': Permission denied
rm: cannot remove '/var/www/html/Lychee/public/uploads/original/index.html': Permission denied
**** Create the symbolic link for the /sym folder ****
touch: cannot touch '/var/www/html/Lychee/public/sym/empty_file': Permission denied
**** Create the symbolic link for the /logs folder ****
touch: cannot touch '/var/www/html/Lychee/storage/logs/empty_file': Permission denied
**** Copy the .env to /conf ****
ln: failed to create symbolic link '/var/www/html/Lychee/.env': Permission denied

For further information on why a read-only container is preferable and industry good practice, please see https://docs.datadoghq.com/security/default_rules/cis-docker-1.2.0-5.12/

d7415 commented 3 weeks ago

/var/www/html/Lychee/public/uploads/ should be a symlink, as details in those logs. If it is not you have misconfigured it.

I accept a fully readonly container may be better, and we may move that way in the future, but for now it works and data is preserved in the volumes.

girlpunk commented 3 weeks ago

If that directory should be a symlink, why is that link not created in the Dockerfile, and only when the container starts?

The container is configured correctly for expected security boundaries (i.e. running as a specific, non-root user), however the startup script tries to modify files outside of the expected mount points. Furthermore, when this is bypassed, the script also incorrectly changes file permissiosns to an unknown user (id 33), causing it to fail to start. Manually correcting the permissions on the larvel log file back to the expected user corrects the issue, however this must be done every time the container starts.

girlpunk commented 3 weeks ago

Additionally, this reliance on arbitrary files outside of mount points causes the key generation to run every time the container is recreated. I'm unsure if this will cause any issues, as it appeared to fail. Based on this, you may also want to read https://www.copado.com/resources/blog/pets-vs-cattle-more-than-an-analogy-for-modern-infrastructures

d7415 commented 3 weeks ago

the script also incorrectly changes file permissiosns to an unknown user (id 33), causing it to fail to start

I guess you didn't try the readme.

d7415 commented 3 weeks ago

causes the key generation to run every time the container is recreated

It also runs the migrations there. That block is designed to be run when the image changes, so is behaving as intended. If the file could not be created for some reason, it could be run on every container start, but it doesn't need to.

girlpunk commented 3 weeks ago

I did read the readme, and have both PUID and PGID set to the user and group the container is running as.

If that running every time (and the resultant error) is expected, that would explain why I haven't seen any issues as a result.