Safer container names (#6441)

* Allow building without pushing

This enables easier local testing

* Refactor fetching Docker container names to be safer

Fixes #5680

* Wrap shell variable with quotes

And change spaces to tabs

* Make cgroup-name quieter

* Make DOCKER_USR overridable

* Update documentation to explain safe usage

* Remove recommended image for docker socket proxy

* Add capability to pass in a privileged GID

* Fix some documentation typos

* Update documentation to remove socket reference and clean up wording

collectors/cgroups.plugin/cgroup-name.sh.in

@@ -53,18 +53,25 @@ function docker_get_name_classic() {
 }
 
 function docker_get_name_api() {
-	local id="${1}"
-	if [ ! -S "${DOCKER_HOST}" ]; then
-		warning "Can't find ${DOCKER_HOST}"
+	local path="/containers/${1}/json"
+	if [ -z "${DOCKER_HOST}" ]; then
+		warning "No DOCKER_HOST is set"
 		return 1
 	fi
 	if ! command -v jq >/dev/null 2>&1; then
 		warning "Can't find jq command line tool. jq is required for netdata to retrieve docker container name using ${DOCKER_HOST} API, falling back to docker ps"
 		return 1
 	fi
-
-	info "Running API command: /containers/${id}/json"
-	JSON=$(echo -e "GET /containers/${id}/json HTTP/1.0\\r\\n" | nc -U "${DOCKER_HOST}" | grep '^{.*')
+	if [ -S "${DOCKER_HOST}" ]; then
+		info "Running API command: curl --unix-socket ${DOCKER_HOST} http://localhost${path}"
+		JSON=$(curl -sS --unix-socket "${DOCKER_HOST}" "http://localhost${path}")
+	elif [ "${DOCKER_HOST}" == "/var/run/docker.sock" ]; then
+		warning "Docker socket was not found at ${DOCKER_HOST}"
+		return 1
+	else
+		info "Running API command: curl ${DOCKER_HOST}${path}"
+		JSON=$(curl -sS "${DOCKER_HOST}${path}")
+	fi
 	NAME=$(echo "$JSON" | jq -r .Name,.Config.Hostname | grep -v null | head -n1 | sed 's|^/||')
 	return 0
 }

packaging/docker/Dockerfile

@@ -58,9 +58,11 @@ COPY --from=builder /app /
 # Configure system
 ARG NETDATA_UID=201
 ARG NETDATA_GID=201
+ENV DOCKER_GRP netdata
+ENV DOCKER_USR netdata
 RUN \
     # provide judy installation to base image
-    apk add make alpine-sdk && \
+    apk add make alpine-sdk shadow && \
     cd /judy-${JUDY_VER} && make install && cd / && \
     # Clean the source stuff once judy is installed
     rm -rf /judy-${JUDY_VER} && apk del make alpine-sdk && \
@@ -69,8 +71,8 @@ RUN \
     chmod 4755 /usr/local/bin/fping && \
     mkdir -p /var/log/netdata && \
     # Add netdata user
-    addgroup -g ${NETDATA_GID} -S netdata && \
-    adduser -S -H -s /usr/sbin/nologin -u ${NETDATA_GID} -h /etc/netdata -G netdata netdata && \
+    addgroup -g ${NETDATA_GID} -S "${DOCKER_GRP}" && \
+    adduser -S -H -s /usr/sbin/nologin -u ${NETDATA_GID} -h /etc/netdata -G "${DOCKER_GRP}" "${DOCKER_USR}" && \
     # Apply the permissions as described in
     # https://github.com/netdata/netdata/wiki/netdata-security#netdata-directories
     chown -R root:netdata /etc/netdata && \

packaging/docker/README.md

@@ -28,7 +28,6 @@ docker run -d --name=netdata \
   -v /etc/group:/host/etc/group:ro \
   -v /proc:/host/proc:ro \
   -v /sys:/host/sys:ro \
-  -v /var/run/docker.sock:/var/run/docker.sock:ro \
   --cap-add SYS_PTRACE \
   --security-opt apparmor=unconfined \
   netdata/netdata
@@ -53,35 +52,53 @@ services:
       - /etc/group:/host/etc/group:ro
       - /proc:/host/proc:ro
       - /sys:/host/sys:ro
-      - /var/run/docker.sock:/var/run/docker.sock:ro
 ```
 
 If you don't want to use the apps.plugin functionality, you can remove the mounts of `/etc/passwd` and `/etc/group` (they are used to get proper user and group names for the monitored host) to get slightly better security.
 
 ### Docker container names resolution
 
-If you want to have your container names resolved by netdata, you need to do two things:
-1) Make netdata user be part of the group that owns the socket.
-   To achieve that just add environment variable `PGID=[GROUP NUMBER]` to the netdata container,
-   where `[GROUP NUMBER]` is practically the group id of the group assigned to the docker socket, on your host.
-   This group number can be found by running the following (if socket group ownership is docker):
-   ```bash
-   grep docker /etc/group | cut -d ':' -f 3
-   ```
-
-2) Change docker socket access level to read/write like so:
-   from
-   ```
-   /var/run/docker.sock:/var/run/docker.sock:ro
-   ```
-
-   change to
-   ```
-   /var/run/docker.sock:/var/run/docker.sock:rw
-   ```
+There are a few options for resolving container names within netdata. Some methods of doing so will allow root access to your machine from within the container. Please read the following carefully.
+
+#### Docker Socket Proxy (Safest Option)
+
+Deploy a Docker socket proxy that accepts and filter out requests using something like [HAProxy](https://docs.netdata.cloud/docs/running-behind-haproxy/) so that it restricts connections to read-only access to the CONTAINERS endpoint.
+
+The reason it's safer to expose the socket to the proxy is because netdata has a TCP port exposed outside the Docker network. Access to the proxy container is limited to only within the network.
+
+#### Giving group access to Docker Socket (Less safe)
+
+**Important Note**: You should seriously consider the necessity of activating this option,
+as it grants to the netdata user access to the privileged socket connection of docker service and therefore your whole machine.
+
+If you want to have your container names resolved by Netdata, make the `netdata` user be part of the group that owns the socket.
+
+To achieve that just add environment variable `PGID=[GROUP NUMBER]` to the Netdata container, 
+where `[GROUP NUMBER]` is practically the group id of the group assigned to the docker socket, on your host.
+
+This group number can be found by running the following (if socket group ownership is docker):
+
+```bash
+grep docker /etc/group | cut -d ':' -f 3
+```
+
+#### Running as root (Unsafe)
 
 **Important Note**: You should seriously consider the necessity of activating this option,
-as it grants to the netdata user access to the privileged socket connection of docker service
+as it grants to the netdata user access to the privileged socket connection of docker service and therefore your whole machine.
+
+```yaml
+version: '3'
+services:
+  netdata:
+    image: netdata/netdata
+    # ... rest of your config ...
+    volumes:
+      # ... other volumes ...
+      - /var/run/docker.sock:/var/run/docker.sock:ro
+    environment:
+      - DOCKER_USR=root
+```
 
 ### Pass command line options to Netdata 
 

packaging/docker/build-test.sh

@@ -46,27 +46,29 @@ do
 	esac
 done
 
-if [ -n "${REPOSITORY}" ] && [ -n "${VERSION}" ] && [ -n "${DOCKER_USERNAME}" ] && [ -n "${DOCKER_PWD}" ] ; then
+if [ -n "${REPOSITORY}" ]; then
 	if [ $DOBUILD -eq 1 ] ; then
-		echo "Building ${VERSION} of ${REPOSITORY} container"
+		echo "Building ${VERSION:-latest} of ${REPOSITORY} container"
 		docker run --rm --privileged multiarch/qemu-user-static:register --reset
 
 		# Build images using multi-arch Dockerfile.
-		eval docker build --build-arg ARCH="amd64" --tag "${REPOSITORY}:${VERSION}" --file packaging/docker/Dockerfile ./
+		eval docker build --build-arg ARCH="amd64" --tag "${REPOSITORY}:${VERSION:-latest}" --file packaging/docker/Dockerfile ./
 
 		# Create temporary docker CLI config with experimental features enabled (manifests v2 need it)
 		mkdir -p /tmp/docker
 		#echo '{"experimental":"enabled"}' > /tmp/docker/config.json
 	fi
 
-	# Login to docker hub to allow futher operations
-	echo "Logging into docker"
-	echo "$DOCKER_PWD" | docker --config /tmp/docker login -u "$DOCKER_USERNAME" --password-stdin
+    if [ -n "${DOCKER_USERNAME}" ] && [ -n "${DOCKER_PWD}" ] ; then
+        # Login to docker hub to allow futher operations
+        echo "Logging into docker"
+        echo "$DOCKER_PWD" | docker --config /tmp/docker login -u "$DOCKER_USERNAME" --password-stdin
 
-	echo "Pushing ${REPOSITORY}:${VERSION}"
-	docker --config /tmp/docker push "${REPOSITORY}:${VERSION}"
+        echo "Pushing ${REPOSITORY}:${VERSION}"
+        docker --config /tmp/docker push "${REPOSITORY}:${VERSION}"
+    fi
 else
-	echo "Missing parameter. REPOSITORY=${REPOSITORY} VERSION=${VERSION} DOCKER_USERNAME=${DOCKER_USERNAME} DOCKER_PWD=${DOCKER_PWD}"
+	echo "Missing parameter. REPOSITORY=${REPOSITORY}"
 	printhelp
 	exit 1
 fi

packaging/docker/run.sh

@@ -9,41 +9,15 @@ set -e
 
 echo "Netdata entrypoint script starting"
 if [ ${RESCRAMBLE+x} ]; then
-    echo "Reinstalling all packages to get the latest Polymorphic Linux scramble"
-    apk upgrade --update-cache --available
+	echo "Reinstalling all packages to get the latest Polymorphic Linux scramble"
+	apk upgrade --update-cache --available
 fi
 
-create_group_and_assign_to_user() {
-	local local_DOCKER_GROUP="$1"
-	local local_DOCKER_GID="$2"
-	local local_DOCKER_USR="$3"
-
-	echo >&2 "Adding group with ID ${local_DOCKER_GID} and name '${local_DOCKER_GROUP}'"
-	addgroup -g "${local_DOCKER_GID}" "${local_DOCKER_GROUP}" || echo >&2 "Could not add group ${local_DOCKER_GROUP} with ID ${local_DOCKER_GID}, its already there probably"
-
-	echo >&2 "Adding user '${local_DOCKER_USR}' to group '${local_DOCKER_GROUP}/${local_DOCKER_GID}'"
-	sed -i "s/:${local_DOCKER_GID}:$/:${local_DOCKER_GID}:${local_DOCKER_USR}/g" /etc/group
-
-	# Make sure we use the right docker group
-	GRP_TO_ASSIGN="$(grep ":x:${local_DOCKER_GID}:" /etc/group | cut -d':' -f1)"
-	if [ -z "${GRP_TO_ASSIGN}" ]; then
-		echo >&2 "Could not find group ID ${local_DOCKER_GID} in /etc/group. Check your logs and report it if this is an unrecovereable error"
-	else
-		echo >&2 "Group creation and assignment completed, netdata was assigned to group ${GRP_TO_ASSIGN}/${local_DOCKER_GID}"
-		echo "${GRP_TO_ASSIGN}"
-	fi
-}
-
-DOCKER_USR="netdata"
-DOCKER_SOCKET="/var/run/docker.sock"
-DOCKER_GROUP="docker"
-
-if [ -S "${DOCKER_SOCKET}" ] && [ -n "${PGID}" ]; then
-	GRP=$(create_group_and_assign_to_user "${DOCKER_GROUP}" "${PGID}" "${DOCKER_USR}")
-	if [ -n "${GRP}" ]; then
-		echo "Adjusting ownership of mapped docker socket '${DOCKER_SOCKET}' to root:${GRP}"
-		chown "root:${GRP}" "${DOCKER_SOCKET}" || echo "Failed to change ownership on docker socket, container name resolution might not work"
-	fi
+if [ -n "${PGID}" ]; then
+    echo "Creating docker group ${PGID}"
+    addgroup -g "${PGID}" "docker" || echo >&2 "Could not add group docker with ID ${PGID}, its already there probably"
+    echo "Assign netdata user to docker group ${PGID}"
+    usermod -a -G ${PGID} ${DOCKER_USR} || echo >&2 "Could not add netdata user to group docker with ID ${PGID}"
 fi
 
 exec /usr/sbin/netdata -u "${DOCKER_USR}" -D -s /host -p "${NETDATA_PORT}" "$@"