ZNC: switch from root user earlier, support --user

[DarthGandalf]

Fixes znc/znc-docker#20 and
znc/znc-docker#12

[yosifkit]

Diff:

diff --git a/znc_slim/50-chown.sh b/znc_slim/20-chown.sh
similarity index 30%
rename from znc_slim/50-chown.sh
rename to znc_slim/20-chown.sh
index 15b5206..0b53472 100644
--- a/znc_slim/50-chown.sh
+++ b/znc_slim/20-chown.sh
@@ -1,5 +1,12 @@
 # Make sure $DATADIR is owned by znc user. This affects ownership of the
 # mounted directory on the host machine too.
+# Also allow the container to be started with `--user`.
 
+if [ "$(id -u)" = '0' ]; then
     chown -R znc:znc "$DATADIR" || exit 1
     chmod 700 "$DATADIR" || exit 2
+    exec su-exec znc:znc /entrypoint.sh "$@"
+fi
+
+chown -R "$(id -u):$(id -g)" "$DATADIR"
+chmod 700 "$DATADIR"
diff --git a/znc_slim/99-launch.sh b/znc_slim/99-launch.sh
index e76c449..49e4d3b 100644
--- a/znc_slim/99-launch.sh
+++ b/znc_slim/99-launch.sh
@@ -2,4 +2,4 @@
 # started via *shell module is not guaranteed to reap their children.
 # That's why using tini.
 
-exec /sbin/tini -- su-exec znc:znc /opt/znc/bin/znc --foreground --datadir "$DATADIR" "$@"
+exec /sbin/tini -- /opt/znc/bin/znc --foreground --datadir "$DATADIR" "$@"
diff --git a/znc_slim/Dockerfile b/znc_slim/Dockerfile
index 4a66d16..4c68662 100644
--- a/znc_slim/Dockerfile
+++ b/znc_slim/Dockerfile
@@ -53,7 +53,7 @@ RUN set -x \
 COPY entrypoint.sh /
 COPY 00-try-sh.sh /startup-sequence/
 COPY 01-options.sh /startup-sequence/
-COPY 50-chown.sh /startup-sequence/
+COPY 20-chown.sh /startup-sequence/
 COPY 99-launch.sh /startup-sequence/
 
 VOLUME /znc-data

[yosifkit]

I believe the new chown -R "$(id -u):$(id -g)" "$DATADIR" will have basically the same problems as before, but now it will be running as non-root and can't chown or chmod something that it doesn't already own. Ah, but no set -e, so they'll basically just print warnings to users without exiting? 🤔

When supporting --user in our images, we usually tell users that they are required to supply any external storage with permissions for the user they provide and also place wide-open permissions on any directory/file we provide (like config files and volumes).

See also docker-library/redmine#136, docker-library/rabbitmq#60, docker-library/cassandra#48, docker-library/mongo#81, redis/docker-library-redis#48, docker-library/mysql#161, MariaDB/mariadb-docker#59, docker-library/percona#21, docker-library/ghost#54, docker-library/postgres#253.

Did you want these changes on full too? 😉

[DarthGandalf]

but now it will be running as non-root and can't chown or chmod something that it doesn't already own

I was mostly looking at https://github.com/docker-library/postgres/blob/45b855af13f6a753fa77bb830c482af6a69d50da/9.5/docker-entrypoint.sh which has those commands. That one uses || : though to shut up set -e.

Removed it.

I also couldn't use BASH_SOURCE because it's not bash...

When supporting --user in our images, we usually tell users that they are required to supply any external storage with permissions for the user they provide and also place wide-open permissions on any directory/file we provide (like config files and volumes).

Shall I also update the docs, or that's too obscure setup, and it's common knowledge for docker users anyway?

Did you want these changes on full too?

It's already here, in this same change :) Full image is build from slim one by installing additional packages, and adding another .sh file to the startup sequence. Which is why I had to move this one from 50 to 20 - so that znc-buildmod is run without root (that's not technically necessary though... but I didn't want to run znc-buildmod twice)

[yosifkit]

Yeah, I know that line in the postgres Dockerfile. I still have no idea why my past self created it or why it is still there. 😕❓

I forgot that full was FROM slim; that's what I get for not looking 😊

Up to you on the documentation part.

LGTM

Diff:

diff --git a/znc_slim/50-chown.sh b/znc_slim/20-chown.sh
similarity index 36%
rename from znc_slim/50-chown.sh
rename to znc_slim/20-chown.sh
index 15b5206..20816fa 100644
--- a/znc_slim/50-chown.sh
+++ b/znc_slim/20-chown.sh
@@ -1,5 +1,9 @@
 # Make sure $DATADIR is owned by znc user. This affects ownership of the
 # mounted directory on the host machine too.
+# Also allow the container to be started with `--user`.
 
+if [ "$(id -u)" = '0' ]; then
     chown -R znc:znc "$DATADIR" || exit 1
     chmod 700 "$DATADIR" || exit 2
+    exec su-exec znc:znc /entrypoint.sh "$@"
+fi
diff --git a/znc_slim/99-launch.sh b/znc_slim/99-launch.sh
index e76c449..49e4d3b 100644
--- a/znc_slim/99-launch.sh
+++ b/znc_slim/99-launch.sh
@@ -2,4 +2,4 @@
 # started via *shell module is not guaranteed to reap their children.
 # That's why using tini.
 
-exec /sbin/tini -- su-exec znc:znc /opt/znc/bin/znc --foreground --datadir "$DATADIR" "$@"
+exec /sbin/tini -- /opt/znc/bin/znc --foreground --datadir "$DATADIR" "$@"
diff --git a/znc_slim/Dockerfile b/znc_slim/Dockerfile
index 4a66d16..4c68662 100644
--- a/znc_slim/Dockerfile
+++ b/znc_slim/Dockerfile
@@ -53,7 +53,7 @@ RUN set -x \
 COPY entrypoint.sh /
 COPY 00-try-sh.sh /startup-sequence/
 COPY 01-options.sh /startup-sequence/
-COPY 50-chown.sh /startup-sequence/
+COPY 20-chown.sh /startup-sequence/
 COPY 99-launch.sh /startup-sequence/
 
 VOLUME /znc-data

Build test of #5436; b661a61; amd64 (znc):

$ bashbrew build znc:1.7.2-slim
Building bashbrew/cache:607d71b9f07d61b3037865dcb0c294037d4b47eadb58ce3a46ac342053cc4994 (znc:1.7.2-slim)
Tagging znc:1.7.2-slim
Tagging znc:1.7-slim
Tagging znc:slim

$ test/run.sh znc:1.7.2-slim
testing znc:1.7.2-slim
	'utc' [1/5]...passed
	'cve-2014--shellshock' [2/5]...passed
	'no-hard-coded-passwords' [3/5]...passed
	'override-cmd' [4/5]...passed
	'znc-basics' [5/5]...passed


$ bashbrew build znc:1.7.2
Building bashbrew/cache:983f747231584c778528f849a4665f649ea6e1c8048225528bd53b7a46c800e3 (znc:1.7.2)
Tagging znc:1.7.2
Tagging znc:1.7
Tagging znc:latest

$ test/run.sh znc:1.7.2
testing znc:1.7.2
	'utc' [1/5]...passed
	'cve-2014--shellshock' [2/5]...passed
	'no-hard-coded-passwords' [3/5]...passed
	'override-cmd' [4/5]...passed
	'znc-basics' [5/5]...passed