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.

Sign up for GitHub

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

Jump to bottom

[database] enable userns-remap and make redis process runs as non-root user #15972

Closed
wants to merge 16 commits into from
Closed

[database] enable userns-remap and make redis process runs as non-root user #15972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
23b241b
make redis process runs as non-root user
maipbui Jul 26, 2023
600e8fd
change grp redis /var/run/redis
maipbui Jul 27, 2023
9c3b22b
remove vars
maipbui Jul 27, 2023
e8bca1e
change file permissions to fix kvmtest
maipbui Aug 2, 2023
abaea85
remove file permission after container starts
maipbui Aug 2, 2023
28964c0
remove chmod
maipbui Aug 2, 2023
5d62b3d
chmod 770 to 760
maipbui Aug 3, 2023
1bbac96
change redis permission for preStartAction and postStartAction
maipbui Aug 3, 2023
ff23035
make /var/run/redis/ owned by redis:redis
maipbui Aug 3, 2023
b7edd4f
address pr comments
maipbui Aug 4, 2023
d2f24f2
fix repeated var
maipbui Aug 4, 2023
642c287
non-root redis and userns-remap for db
maipbui Aug 11, 2023
72464cd
Merge remote-tracking branch 'upstream/master' into nonroot_redis
maipbui Aug 11, 2023
c2985af
fix PR comments
maipbui Aug 14, 2023
04bc7a8
host network to bridge network for db
maipbui Aug 15, 2023
1db0021
simplify code
maipbui Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions build_debian.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ HOSTNAME=sonic
DEFAULT_USERINFO="Default admin user,,,"
BUILD_TOOL_PATH=src/sonic-build-hooks/buildinfo
TRUSTED_GPG_DIR=$BUILD_TOOL_PATH/trusted.gpg.d
## Remapped docker usernames and group names
REMAPREDIS=remapredis
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question - should we use a meaningful name here? remapredis doesn't sound like something that is describing this item.
Maybe use unpriv-redis? Do depict what we intend this user to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can mutually agree on better variable name here

REMAPROOT=remaproot

## Read ONIE image related config file
. ./onie-image.conf
Expand Down Expand Up @@ -317,9 +320,20 @@ sudo LANG=C chroot $FILESYSTEM_ROOT useradd -G sudo,docker $USERNAME -c "$DEFAUL
## Create password for the default user
echo "$USERNAME:$PASSWORD" | sudo LANG=C chroot $FILESYSTEM_ROOT chpasswd

## Create redis group
sudo LANG=C chroot $FILESYSTEM_ROOT groupadd -f redis
sudo LANG=C chroot $FILESYSTEM_ROOT usermod -aG redis $USERNAME
## Start UID and GID for the default user defined in /etc/subuid and /etc/subgid
START_UID=$(sudo grep ^$USERNAME $FILESYSTEM_ROOT/etc/subuid | cut -d: -f2)
START_GID=$(sudo grep ^$USERNAME $FILESYSTEM_ROOT/etc/subgid | cut -d: -f2)
## Remapped Redis user id and group id
REDIS_REMAP_UID=$(($START_UID + $REDIS_USER_UID))
REDIS_REMAP_GID=$(($START_GID + $REDIS_USER_GID))

## Create remapped redis group and user, add default user to remapped redis group
sudo LANG=C chroot $FILESYSTEM_ROOT groupadd -f -g $REDIS_REMAP_GID $REMAPREDIS
sudo LANG=C chroot $FILESYSTEM_ROOT useradd -u $REDIS_REMAP_UID -g $REMAPREDIS $REMAPREDIS -c "remapped docker redis user" -m -s /bin/bash
sudo LANG=C chroot $FILESYSTEM_ROOT usermod -aG $REMAPREDIS $USERNAME
## Create remapped docker root user and group
sudo LANG=C chroot $FILESYSTEM_ROOT groupadd -f -g $START_UID $REMAPROOT
sudo LANG=C chroot $FILESYSTEM_ROOT useradd -u $START_GID -g $REMAPROOT $REMAPROOT -c "remapped docker root user" -m -s /bin/bash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the docker root user or the system root user?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's docker root user inside container, but it's remapped to lower privileged user on host.


if [[ $CONFIGURED_ARCH == amd64 ]]; then
## Pre-install hardware drivers
Expand Down Expand Up @@ -547,6 +561,8 @@ sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get -y in

## Create /var/run/redis folder for docker-database to mount
sudo mkdir -p $FILESYSTEM_ROOT/var/run/redis
## Enable userns-remap in docker
sudo cp files/docker/daemon.json $FILESYSTEM_ROOT/etc/docker/

## Config DHCP for eth0
sudo tee -a $FILESYSTEM_ROOT/etc/network/interfaces > /dev/null <<EOF
Expand Down
9 changes: 9 additions & 0 deletions dockers/docker-database/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
FROM docker-config-engine-bullseye-{{DOCKER_USERNAME}}:{{DOCKER_USERTAG}}

ARG docker_container_name
ARG redis_user_uid
ARG redis_user_gid

# Make apt-get non-interactive
ENV DEBIAN_FRONTEND=noninteractive
Expand All @@ -12,6 +14,11 @@ RUN apt-get update
# Install redis-server
RUN apt-get install -y redis-tools redis-server

RUN if [ "$(id -u redis)" -ne $redis_user_uid ] || [ "$(id -g redis)" -ne $redis_user_gid ]; then \
echo "Error: Default Redis user must have UID/GID $redis_user_uid/$redis_user_gid." && \
exit 1; \
fi

{% if docker_database_debs.strip() -%}
# Copy locally-built Debian package dependencies
{{ copy_files("debs/", docker_database_debs.split(' '), "/debs/") }}
Expand All @@ -32,6 +39,8 @@ RUN apt-get clean -y && \
s/^# unixsocket/unixsocket/; \
s/redis-server.sock/redis.sock/g; \
s/^client-output-buffer-limit pubsub [0-9]+mb [0-9]+mb [0-9]+/client-output-buffer-limit pubsub 0 0 0/; \
s/^bind 127.0.0.1 ::1$/# bind 127.0.0.1 ::1/; \
s/^protected-mode yes/protected-mode no/; \
s/^notify-keyspace-events ""$/notify-keyspace-events AKE/ \
' /etc/redis/redis.conf

Expand Down
1 change: 1 addition & 0 deletions dockers/docker-database/docker-database-init.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ then
fi

REDIS_DIR=/var/run/redis$NAMESPACE_ID
chown -R redis:redis $REDIS_DIR
mkdir -p $REDIS_DIR/sonic-db
mkdir -p /etc/supervisor/conf.d/

Expand Down
3 changes: 2 additions & 1 deletion dockers/docker-database/supervisord.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ dependent_startup=true
{%- else -%}
{%- set LOOPBACK_IP = '' -%}
{%- endif -%}
command=/bin/bash -c "{ [[ -s /var/lib/{{ redis_inst }}/dump.rdb ]] || rm -f /var/lib/{{ redis_inst }}/dump.rdb; } && mkdir -p /var/lib/{{ redis_inst }} && exec /usr/bin/redis-server /etc/redis/redis.conf --bind {{ LOOPBACK_IP }} {{ redis_items['hostname'] }} --port {{ redis_items['port'] }} --unixsocket {{ redis_items['unix_socket_path'] }} --pidfile /var/run/redis/{{ redis_inst }}.pid --dir /var/lib/{{ redis_inst }}"
command=/bin/bash -c "{ [[ -s /var/lib/{{ redis_inst }}/dump.rdb ]] || rm -f /var/lib/{{ redis_inst }}/dump.rdb; } && mkdir -p /var/lib/{{ redis_inst }} && exec /usr/bin/redis-server /etc/redis/redis.conf --port {{ redis_items['port'] }} --unixsocket {{ redis_items['unix_socket_path'] }} --pidfile /var/run/redis/{{ redis_inst }}.pid --dir /var/lib/{{ redis_inst }}"
priority=2
user=redis
autostart=false
autorestart=false
stdout_logfile=syslog
Expand Down
14 changes: 11 additions & 3 deletions files/build_templates/docker_image_ctl.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ function postStartAction()
fi
REDIS_SOCK="/var/run/redis-chassis/redis_chassis.sock"
fi
chgrp -f redis $REDIS_SOCK && chmod -f 0760 $REDIS_SOCK
chmod -f 0760 $REDIS_SOCK
maipbui marked this conversation as resolved.
Show resolved Hide resolved
{%- elif docker_container_name == "swss" %}
docker exec swss$DEV rm -f /ready # remove cruft
if [[ "$BOOT_TYPE" == "fast" ]] && [[ -d /host/fast-reboot ]]; then
Expand Down Expand Up @@ -308,7 +308,6 @@ start() {
# Obtain our platform as we will mount directories with these names in each docker
PLATFORM=${PLATFORM:-`$SONIC_CFGGEN -H -v DEVICE_METADATA.localhost.platform`}


# Parse the device specific asic conf file, if it exists
ASIC_CONF=/usr/share/sonic/device/$PLATFORM/asic.conf
if [ -f "$ASIC_CONF" ]; then
Expand Down Expand Up @@ -474,7 +473,12 @@ start() {
{%- endif %}

if [ -z "$DEV" ]; then
NET="host"
{%- if docker_container_name == "database" %}
NET="bridge"
PORT_MAP="-p 127.0.0.1:6379:6379"
{%- else %}
NET="host"
{%- endif %}

# For Multi-ASIC platform we have to mount the redis paths for database instances running in different
# namespaces, into the single instance dockers like snmp, pmon on linux host. These global dockers
Expand Down Expand Up @@ -531,6 +535,9 @@ start() {
# TODO: Mellanox will remove the --tmpfs exception after SDK socket path changed in new SDK version
{%- endif %}
docker create {{docker_image_run_opt}} \
{%- if docker_container_name != "database" %}
--userns=host \
{%- endif %}
--net=$NET \
-e RUNTIME_OWNER=local \
--uts=host \{# W/A: this should be set per-docker, for those dockers which really need host's UTS namespace #}
Expand Down Expand Up @@ -580,6 +587,7 @@ start() {
-v /etc/sonic/frr/$DEV:/etc/frr:rw \
{%- endif %}
{%- if docker_container_name == "database" %}
$PORT_MAP \
$DB_OPT \
{%- else %}
-v /var/run/redis$DEV:/var/run/redis:rw \
Expand Down
3 changes: 3 additions & 0 deletions files/docker/daemon.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"userns-remap": "admin"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the name of our NS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, admin is not the name of a namespace, but rather the username for the user namespace remapping in Docker

}
4 changes: 4 additions & 0 deletions rules/config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ DEFAULT_KERNEL_PROCURE_METHOD = build
FRR_USER_UID = 300
FRR_USER_GID = 300

# Redis user and group default id values inside database container.
REDIS_USER_UID = 101
REDIS_USER_GID = 101

# DPKG cache allows the .deb files to be stored in the cache path. This allows the submodules
# package to be cached and restored back if its commit hash is not modified and its dependencies are not modified.
# SONIC_DPKG_CACHE_METHOD - Default method of deb package caching
Expand Down
10 changes: 10 additions & 0 deletions slave.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,12 @@ export FRR_USER_GID
export INCLUDE_FIPS
export ENABLE_FIPS

###############################################################################
## Redis related exports
###############################################################################
export REDIS_USER_UID
export REDIS_USER_GID

###############################################################################
## Build Options
###############################################################################
Expand Down Expand Up @@ -394,6 +400,8 @@ ifeq ($(SONIC_ROUTING_STACK),frr)
$(info "FRR_USER_UID" : "$(FRR_USER_UID)")
$(info "FRR_USER_GID" : "$(FRR_USER_GID)")
endif
$(info "REDIS_USER_UID" : "$(REDIS_USER_UID)")
$(info "REDIS_USER_GID" : "$(REDIS_USER_GID)")
$(info "ENABLE_SYNCD_RPC" : "$(ENABLE_SYNCD_RPC)")
$(info "SAITHRIFT_V2" : "$(SAITHRIFT_V2)")
$(info "ENABLE_ORGANIZATION_EXTENSIONS" : "$(ENABLE_ORGANIZATION_EXTENSIONS)")
Expand Down Expand Up @@ -1095,6 +1103,8 @@ $(addprefix $(TARGET_PATH)/, $(DOCKER_IMAGES)) : $(TARGET_PATH)/%.gz : .platform
--build-arg docker_container_name=$($*.gz_CONTAINER_NAME) \
--build-arg frr_user_uid=$(FRR_USER_UID) \
--build-arg frr_user_gid=$(FRR_USER_GID) \
--build-arg redis_user_uid=$(REDIS_USER_UID) \
--build-arg redis_user_gid=$(REDIS_USER_GID) \
--build-arg SONIC_VERSION_CACHE=$(SONIC_VERSION_CACHE) \
--build-arg SONIC_VERSION_CACHE_SOURCE=$(SONIC_VERSION_CACHE_SOURCE) \
--build-arg image_version=$(SONIC_IMAGE_VERSION) \
Expand Down
Loading