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

Rework RUNAS_UID0 #362

Open
jacobalberty opened this issue Oct 14, 2020 · 5 comments
Open

Rework RUNAS_UID0 #362

jacobalberty opened this issue Oct 14, 2020 · 5 comments

Comments

@jacobalberty
Copy link
Owner

A few things have changed in the docker world since I originally added RUNAS_UID0 and BIND_PRIV.

I am now using net.ipv4.ip_unprivileged_port_start: 0 sysctl in my docker-compose which completely removes the need for BIND_PRIV, with BIND_PRIV unneeded the normal docker user parameters for the most part work.

I think I'm going to update the documentation to reflect using the sysctl instead and officially deprecate RUNAS_UID0 and BIND_PRIV, the next step will be to strip those out of the beta tag entirely to start merging it back into main.

The only possible issue I foresee is file system permissions. I need to do some testing on that during the deprecation time and make sure nothing breaks. Removing the features themselves shouldnt cause anyones setup to break, rather it will escalate permissions back to root. Which is the default anyway.

I'm doing this for two reasons

  1. The environment variables were always a hack and not something I liked, but it worked best.
  2. The gosu dependency is now causing an issue with Multi-arch image support #360
@jacobalberty
Copy link
Owner Author

I forgot to put it in the commit but beta now has the relevant code removed and master now has the documentation changes done. The only breaking issue I see with commiting this to master is the old RUNAS_UID0 would automatically set permissions on the relevant folders whereas the new setup will not.

@buckaroogeek
Copy link

buckaroogeek commented Oct 14, 2020 via email

@jacobalberty
Copy link
Owner Author

Ok, Found another downside to dropping RUNAS_UID0, I'm not convinced it needs fixing as there are workarounds.

Right now beta image installs take a PKGURL and just pull that on first launch. Docker does not provide any way to run pre-environment commands as root when using --user so everything runs without root permissions, so I can't install the package after build if we use --user that breaks non root users with the current beta tag.

There's two options as I see it

  1. Non root beta installs just require building the image using the github repo instead of pulling prebuild from docker hub
  2. Extend docker some how to allow some special root commands to execute before the environment is fully launched. Some sort of custom setuid program that only runs a single script and checks permissions to try and ensure security.

I'm not a fan of 2 though and this only affects the beta image. I think 1 is the best option. But maybe someone else has tackled option 2 already and published the code, so I'm going to dig around and see.

@ruud-coder
Copy link

Ok, Found another downside to dropping RUNAS_UID0, I'm not convinced it needs fixing as there are workarounds.

I believe there are more issues... personally I’m using your image for years now on my Synology NAS and with RUNAS_UID0 set to false I was (and quite some relatives are..) able to run it safely as a non-root user; using the Synology docker GUI. However, the GUI does not support passing run flags to Docker. So wouldn’t it make sense to make “running as non-root user” the default behavior now this RUNAS_UID0 is about to be dropped?

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Dec 22, 2021
@jacobalberty jacobalberty reopened this Jan 20, 2022
efuce pushed a commit to efuce/unifi-docker that referenced this issue Jul 19, 2023
* Remove BIND_PRIV handling since there is no need for it according to jacobalberty#362
* Add functionality to ensure that system.properties file is always created, even if no settings are actally written
* Backport permset handling from master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants