Complete fix of a privilege escalation exploit in the sudo configuration of Knockd (followup to #3472)

build/alpine/setup-user.sh

@@ -3,4 +3,6 @@
 set -e
 
 addgroup -g 1000 minecraft
+addgroup -g 1001 service-group
 adduser -Ss /bin/false -u 1000 -G minecraft -h /home/minecraft minecraft
+adduser -Ss /bin/sh -u 1001 -G service-group -H service-account

build/ol/setup-user.sh

@@ -1,2 +1,4 @@
 groupadd --gid 1000 minecraft
+groupadd --gid 1001 service-group
 useradd --system --shell /bin/false --uid 1000 -g minecraft --home /data minecraft
+useradd --system --shell /bin/sh --uid 1001 -g service-group -M service-account

build/ubuntu/setup-user.sh

@@ -7,4 +7,6 @@ if id ubuntu > /dev/null 2>&1; then
 fi
 
 addgroup --gid 1000 minecraft
-adduser --system --shell /bin/false --uid 1000 --ingroup minecraft --home /data minecraft
+addgroup --gid 1001 service-group
+adduser --system --shell /bin/false --uid 1000 --ingroup minecraft --home /data minecraft
+adduser --system --shell /bin/sh --uid 1001 --ingroup service-group --no-create-home service-account

docker-minecraft-server.code-workspace

@@ -0,0 +1,9 @@
+{
+	"folders": [
+		{
+			"name": "docker-minecraft-server",
+			"path": "."
+		}
+	],
+	"settings": {}
+}

files/sudoers-service

@@ -0,0 +1,2 @@
+service-account ALL=(ALL) NOPASSWD:/usr/bin/pkill
+service-account ALL=(ALL) NOPASSWD:/usr/local/sbin/knockd

scripts/start

@@ -6,49 +6,44 @@
 # The Dockerfile ENVs take precedence here, but defaulting for testing consistency
 : "${UID:=1000}"
 : "${GID:=1000}"
+: "${UID_SERVICE:=1001}"
+: "${GID_SERVICE:=1001}"
 : "${SKIP_CHOWN_DATA:=false}"
+export UID GID UID_SERVICE GID_SERVICE
 
 umask "${UMASK:=0002}"
 
 # Remove from previous run and do this as elevated user since file used to be created before demoting
 rm -f "$HOME/.rcon-cli.env"
 
 if ! isTrue "${SKIP_SUDO:-false}" && [ "$(id -u)" = 0 ]; then
-  runAsUser=minecraft
-  runAsGroup=minecraft
-
-  if [[ -v UID ]]; then
-    if [[ $UID != 0 ]]; then
-      if [[ $UID != $(id -u minecraft) ]]; then
-        log "Changing uid of minecraft to $UID"
-        usermod -u $UID minecraft
+  runAsUser=service-account
+  runAsGroup=service-group
+
+  if [[ -v UID_SERVICE ]]; then
+    if [[ $UID_SERVICE != 0 ]]; then
+      if [[ $UID_SERVICE != $(id -u service-account) ]]; then
+        log "Changing uid of service-account to $UID_SERVICE"
+        usermod -u $UID_SERVICE service-account
       fi
     else
       runAsUser=root
     fi
   fi
 
-  if [[ -v GID ]]; then
-    if [[ $GID != 0 ]]; then
-      if [[ $GID != $(id -g minecraft) ]]; then
-        log "Changing gid of minecraft to $GID"
-        groupmod -o -g "$GID" minecraft
+  if [[ -v GID_SERVICE ]]; then
+    if [[ $GID_SERVICE != 0 ]]; then
+      if [[ $GID_SERVICE != $(id -g service-account) ]]; then
+        log "Changing gid of service-group to $GID_SERVICE"
+        groupmod -o -g "$GID_SERVICE" service-account
       fi
     else
       runAsGroup=root
     fi
   fi
 
-  if isFalse "${SKIP_CHOWN_DATA}" && [[ $(stat -c "%u" /data) != "$UID" ]]; then
-    log "Changing ownership of /data to $UID ..."
-    chown -R ${runAsUser}:${runAsGroup} /data
-  fi
-
   if [[ ${SKIP_NSSWITCH_CONF^^} != TRUE ]]; then
     echo 'hosts: files dns' > /etc/nsswitch.conf
   fi
-
-  exec $(getSudoFromDistro) ${runAsUser}:${runAsGroup} "${SCRIPTS:-/}start-configuration" "$@"
-else
-  exec "${SCRIPTS:-/}start-configuration" "$@"
 fi
+"${SCRIPTS:-/}start-configuration" "$@"

scripts/start-configuration

@@ -140,11 +140,11 @@ export DECLARED_TYPE=${TYPE^^}
 export DECLARED_VERSION="$VERSION"
 
 if isTrue "${ENABLE_AUTOPAUSE}"; then
-  "${SCRIPTS:-/}start-autopause"
+  $(getSudoFromDistro) ${UID_SERVICE}:${GID_SERVICE} "${SCRIPTS:-/}start-autopause"
 fi
 
 if isTrue "${ENABLE_AUTOSTOP}"; then
-  "${SCRIPTS:-/}start-autostop"
+  $(getSudoFromDistro) ${UID_SERVICE}:${GID_SERVICE} "${SCRIPTS:-/}start-autostop"
 fi
 
 if 

scripts/start-finalExec

@@ -13,6 +13,8 @@ serverIconPath=${baseDataDir}/server-icon.png
 mcHealthEnvPath=${baseDataDir}/.mc-health.env
 bootstrapPath=${baseDataDir}/bootstrap.txt
 
+chmod -R 744 /data
+
 if [ -n "$ICON" ]; then
     if [ ! -e server-icon.png ] || isTrue "${OVERRIDE_ICON}"; then
       log "Using server icon from $ICON..."
@@ -355,9 +357,9 @@ else
   fi
 
   if isTrue "${EXEC_DIRECTLY:-false}"; then
-    exec java "${finalArgs[@]}"
+    java "${finalArgs[@]}"
   else
-    exec mc-server-runner ${bootstrapArgs} "${mcServerRunnerArgs[@]}" java "${finalArgs[@]}"
+    mc-server-runner ${bootstrapArgs} "${mcServerRunnerArgs[@]}" java "${finalArgs[@]}"
   fi
 fi
 

scripts/start-setupRbac

@@ -97,4 +97,7 @@ if [[ -v WHITELIST ]]; then
     $WHITELIST
 fi
 
-exec "${SCRIPTS:-/}start-finalExec" "$@"
+log "Changing ownership of /data to Minecraft (UID: $UID) ..."
+chown -R ${UID}:${GID} /data
+
+exec $(getSudoFromDistro) ${UID}:${GID} "${SCRIPTS:-/}start-finalExec" "$@"