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

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

Closed
wants to merge 16 commits into from

Conversation

maipbui
Copy link
Contributor

@maipbui maipbui commented Jul 26, 2023

Dependency:
#13783
#15176

Why I did it

Enable user namespace remapping technique for database docker.
Running the Redis server as the "root" user is not recommended. It is suggested that the server should be operated by a non-privileged user.

Work item tracking
  • Microsoft ADO (number only): 15895240, 17418646

How I did it

Add a userns-remap in the /etc/docker/daemon.json file to enable user namespace remap.
Ensure the Redis process is operating under the 'redis' user in supervisord and adjust the necessary file permissions accordingly.

How to verify it

Built new image, verify redis process is running as 'redis' user and all containers are up. Processes remapped to lower privileged user on host.

Before

admin@sonic:~$ redis-cli PING
PONG
admin@sonic:~$ sonic-db-cli PING
PONG

admin@sonic:~$ ps -ef | grep redis
root        2640    1902 16 18:50 pts/0    00:14:12 /usr/bin/redis-server 127.0.0.1:6379

admin@sonic:~$ docker exec -it database bash
root@sonic:/# ps -ef | grep redis
root          46       1 16 18:50 pts/0    00:14:20 /usr/bin/redis-server 127.0.0.1:6379

root@sonic:/# ls -l /etc/redis/
total 84
-rw-r----- 1 redis redis 85802 Jul 15 15:42 redis.conf

admin@sonic:~$ ls -l /var/run/ | grep redis
drwxr-xr-x  3 root root    100 Jul 29 14:36 redis
drwxr-xr-x  2 root root     40 Jul 28 05:39 redis-chassis

admin@sonic:~$ ls -l /var/run/redis/
total 4
-rw-r--r-- 1 root root   3 Jul 29 14:36 redis.pid
srwxrw---- 1 root redis  0 Jul 29 14:36 redis.sock
drwxr-xr-x 2 root root  60 Jul 29 14:36 sonic-db

root@sonic:/# ls -l /var/run/ | grep redis
drwxr-xr-x 3 root root  100 Jul 29 14:36 redis

root@sonic:/# ls -l /var/run/redis/
total 4
-rw-r--r-- 1 root root  3 Jul 26 18:50 redis.pid
srwxrw---- 1 root 1001  0 Jul 26 18:50 redis.sock
drwxr-xr-x 2 root root 60 Jul 26 18:50 sonic-db

After

admin@vlab-01:~$ redis-cli PING
PONG
admin@vlab-01:~$ sonic-db-cli PING
PONG

admin@vlab-01:~$ ps -e -o pid,uid,gid,user,%cpu,%mem,vsz,rss,tty,stat,start,time,command | grep remap
    775 100000 100000 remapro+ 0.0  0.6 31204 26700 pts/0    Ss+  15:12:19 00:00:00 /usr/bin/python3 /usr/local/bin/supervisord
    908 100000 100000 remapro+ 0.1  0.6 124028 26224 pts/0   Sl   15:12:22 00:00:04 python3 /usr/bin/supervisor-proc-exit-listener --container-name database
    971 100000 100000 remapro+ 0.0  0.1 222184 6028 pts/0    Sl   15:12:23 00:00:00 /usr/sbin/rsyslogd -n -iNONE
   1033 100101 100101 remapre+ 1.8  1.7 124660 68404 pts/0   Sl   15:12:25 00:00:49 /usr/bin/redis-server *:6379
   1037 100000 100000 remapro+ 0.0  0.5 40712 22864 pts/0    S    15:12:25 00:00:00 python3 /usr/local/bin/containercfgd

admin@vlab-01:~$ docker exec -it database bash
root@vlab-01:/# ps -e -o pid,uid,gid,user,%cpu,%mem,vsz,rss,tty,stat,start,time,command
    PID   UID   GID USER     %CPU %MEM    VSZ   RSS TT       STAT  STARTED     TIME COMMAND
      1     0     0 root      0.0  0.6  31204 26700 pts/0    Ss+  15:12:19 00:00:01 /usr/bin/python3 /usr/local/bin/supervisord
     47     0     0 root      0.1  0.6 124028 26224 pts/0    Sl   15:12:22 00:00:05 python3 /usr/bin/supervisor-proc-exit-listener --container-name database
     56     0     0 root      0.0  0.1 222184  6028 pts/0    Sl   15:12:23 00:00:00 /usr/sbin/rsyslogd -n -iNONE
     66   101   101 redis     1.8  1.7 124660 68068 pts/0    Sl   15:12:25 00:00:50 /usr/bin/redis-server *:6379
     70     0     0 root      0.0  0.5  40712 22864 pts/0    S    15:12:25 00:00:00 python3 /usr/local/bin/containercfgd

root@vlab-01:/# ls -l /etc/redis/
total 84
-rw-r----- 1 redis redis 85802 Jul 26 19:13 redis.conf

admin@vlab-01:~$ ls -l /var/run/ | grep remap
drwx------  3 remaproot  remaproot    60 Aug 11 15:12 docker.100000.100000
drwxr-xr-x  2 remaproot  remaproot    40 Aug 11 15:12 docker-syncd
drwxr-xr-x  2 remaproot  remaproot    40 Aug 11 15:14 platform_cache
drwxr-xr-x  3 remapredis remapredis  100 Aug 11 15:12 redis
drwxr-xr-x  2 remaproot  remaproot    40 Aug 11 15:12 redis-chassis

admin@vlab-01:~$ ls -l /var/run/redis/
total 4
-rw-r--r-- 1 remapredis remapredis  3 Aug 11 15:12 redis.pid
srwxrw---- 1 remapredis remapredis  0 Aug 11 15:12 redis.sock
drwxr-xr-x 2 remaproot  remaproot  60 Aug 11 15:12 sonic-db

root@vlab-01:/# ls -l /var/run/ | grep redis
drwxr-xr-x 3 redis redis  100 Aug  4 19:23 redis

root@vlab-01:/# ls -l /var/run/redis/
total 4
-rw-r--r-- 1 redis redis  3 Aug  4 19:23 redis.pid
srwxrw---- 1 redis redis  0 Aug  4 19:23 redis.sock
drwxr-xr-x 2 root  root  60 Aug  4 19:23 sonic-db

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

  • SONiC.master.0-b060551b7

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@maipbui maipbui requested a review from qiluo-msft July 26, 2023 20:41
@lguohan
Copy link
Collaborator

lguohan commented Jul 27, 2023

is there a design doc?

build_debian.sh Outdated Show resolved Hide resolved
Signed-off-by: Mai Bui <[email protected]>
fi
chgrp -f -R redis "$(dirname "$REDIS_SOCK")" && \
chmod -f -R 0775 "$(dirname "$REDIS_SOCK")"
Copy link
Collaborator

@qiluo-msft qiluo-msft Aug 3, 2023

Choose a reason for hiding this comment

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

0775

Should it be 760? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/var/run/redis/ needs 775 for sufficient permission to get Redis start if /var/run/redis/ is owned by group redis. /var/run/redis/sonic-db/ need 755 if owned by root group or 775 if /var/run/redis/sonic-db/ is owned by redis group to pass kvm testcase iface_namingmode/test_iface_namingmode.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because iface_namingmode/test_iface_namingmode.py creates a guest user and guest user is not added to redis group. 770 or 760 will not work because guest user cannot access sonic database configuration json file.
https://github.com/sonic-net/sonic-mgmt/blob/master/tests/iface_namingmode/test_iface_namingmode.py#L124-L126

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or maybe the dir user/group ownership need to be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@maipbui
Copy link
Contributor Author

maipbui commented Aug 3, 2023

is there a design doc?

@lguohan I will draft a design doc.

qiluo-msft
qiluo-msft previously approved these changes Aug 5, 2023
build_debian.sh Outdated Show resolved Hide resolved
build_debian.sh Outdated Show resolved Hide resolved
@maipbui maipbui changed the title make redis process runs as non-root user [database] enable userns-remap and make redis process runs as non-root user Aug 11, 2023
build_debian.sh Outdated Show resolved Hide resolved
build_debian.sh Outdated Show resolved Hide resolved
build_debian.sh Outdated Show resolved Hide resolved
build_debian.sh Outdated Show resolved Hide resolved
@maipbui
Copy link
Contributor Author

maipbui commented Aug 16, 2023

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 15972 in repo sonic-net/sonic-buildimage

@maipbui
Copy link
Contributor Author

maipbui commented Aug 16, 2023

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -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

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

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

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.

@@ -0,0 +1,3 @@
{
"userns-remap": "admin"

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

@maipbui maipbui closed this Sep 5, 2023
@maipbui maipbui deleted the nonroot_redis branch September 5, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants