Add unprivileged/non-root docker image in addition to current image that starts as root

[dasMulli]

The provided docker-built container contains user clamav (100) and group clamav (101), however when trying to run as this user/group, the image fails to start (same for current :stable and :latest tags):

> docker run --rm -it --user 100:101 clamav/clamav:latest
Unable to find image 'clamav/clamav:latest' locally
latest: Pulling from clamav/clamav
Digest: sha256:8083453c1d23818ed281629943fe83318a77542ae07fc5113b6e70ff9dfda102
Status: Downloaded newer image for clamav/clamav:latest
install: can't create directory '/run/clamav': Permission denied

This is needed to run the container in Kubernetes as non-root container in order to pass stricter policies without additional need to work around them - e.g. creating PodSecurityPolicy objects to use. In the upcoming Pod Security Admissions feature, this container will be unable to be used in the Pod Security Standard "Restricted" ("Heavily restricted policy, following current Pod hardening best practices.") (see https://kubernetes.io/docs/concepts/security/pod-security-standards/).

Since I don't think ClamAV needs to do things that need root access or additional capabilities by default, being able to run as non-root user by default or having an unprivileged image would be good.

This also shows up as "HIGH" vulnerability/misconfiguration in some security scans (it's just a tool and good practices, not something to blindly make decisions on):

$ trivy repo --security-checks config https://github.com/Cisco-Talos/clamav
Enumerating objects: 52591, done.
Counting objects: 100% (52591/52591), done.
Compressing objects: 100% (25552/25552), done.
Total 52591 (delta 33169), reused 44462 (delta 26102), pack-reused 0
2022-02-21T15:48:29.073+0100    INFO    Detected config files: 1

Dockerfile (dockerfile)
=======================
Tests: 23 (SUCCESSES: 20, FAILURES: 3, EXCEPTIONS: 0)
Failures: 3 (UNKNOWN: 0, LOW: 0, MEDIUM: 2, HIGH: 1, CRITICAL: 0)

+---------------------------+------------+----------------------------------+----------+------------------------------------------------+
|           TYPE            | MISCONF ID |              CHECK               | SEVERITY |                    MESSAGE                     |
+---------------------------+------------+----------------------------------+----------+------------------------------------------------+
| Dockerfile Security Check |   DS001    | ':latest' tag used               |  MEDIUM  | Specify a tag in the                           |
|                           |            |                                  |          | 'FROM' statement for image                     |
|                           |            |                                  |          | 'index.docker.io/library/alpine'               |
|                           |            |                                  |          | -->avd.aquasec.com/appshield/ds001             |
+                           +------------+----------------------------------+----------+------------------------------------------------+
|                           |   DS002    | root user                        |   HIGH   | Specify at least 1 USER                        |
|                           |            |                                  |          | command in Dockerfile with                     |
|                           |            |                                  |          | non-root user as argument                      |
|                           |            |                                  |          | -->avd.aquasec.com/appshield/ds002             |
+                           +------------+----------------------------------+----------+------------------------------------------------+
|                           |   DS013    | 'RUN cd ...' to change directory |  MEDIUM  | RUN should not be used to change directory:    |
|                           |            |                                  |          | 'apk add --no-cache         bsd-compat-headers |
|                           |            |                                  |          |         bzip2-dev         check-dev            |
|                           |            |                                  |          |    cmake         curl-dev         file         |

[PMillsArch]

We are facing this problem too! Unprivileged would be really helpful to have as an option.

[micahsnyder]

The current Dockerfile implementation is set up so that clamd and freshclam services start as root but switch to run as the clamav user after service startup.

I don't really have any objection to changing things so the container runs as the clamav user to begin with. It would just take a little work.

I have my hands full. If any volunteers would like to give it a try, I (or one of my teammates) wouldn't mind reviewing a pull-request.

[dasMulli]

Poking around: it seems to run when necessary permissions are prepared in the dockerfile and all pid and lock files are moved to a dedicated run (/var/run/clamav) dir, can tinker more with it over the weekend and create a PR

[codebitts]

We are also facing this issue. As an interim solution can you share how the directories are supposed to be prepared in the docker file? Would be great if we could use the base image and add more layers for preparing the directories.

[dasMulli]

I had some issues testing all features, but the main idea was to move all mkdir, install and ln invocations from docker-entrypoint.sh to the Dockerfile, setting a fixed UID in the adduser command that is already there, changing the sed commands in the Dockerfile to configure everything into /var/run instead of /run and fixing permissions through chown where they occur. It appeared that clamd and freshclam were working, but did not test milterd yet (at least it starts)

[codebitts]

For reference, this seems to have worked for me. Albeit with some modifications to the default configuration files as well.

FROM clamav/clamav:0.104

COPY etc /etc/clamav

RUN mkdir /var/run/clamav /run/lock && \
    chown clamav:clamav /var/run/clamav /run/clamav /var/log/clamav /var/lock /run/lock && \
    chmod 770 /var/run/clamav /run/clamav /var/log/clamav /var/lock /run/lock

USER clamav

[dasMulli]

My diff so far is https://github.com/Cisco-Talos/clamav/compare/main...dasMulli:feature/nonroot-container

[msamendinger]

Also looking for a solution to run the container as non-root. Can anyone give more insight why these lines are needed? At a first try the container seems to run without these lines but I guess they are there for a reason. https://github.com/Cisco-Talos/clamav/blob/140c88aa4e8c6d17ba771772b93cb94e088e9e4c/dockerfiles/docker-entrypoint.sh#L34-L36

@dasMulli I was looking through your diff and wondering why the location for the pid file and socket get moved to /var/run/lock and /var/run/clamav and later the directory /run/clamav is created that doesn't seem to be used. What do you think about using /run/lock/clamav and /run/clamav respectively? When do you plan to create the PR?

[dasMulli]

later the directory /run/clamav is created that doesn't seem to be used.

It is an unfinished snapshot, the permissions for /run would just be more invasive to set up.

No timeline yet but before the end of the month

[cmcdougall]

Is there an update to this issue? I'm currently looking at trying to run the image non-root as well.

[micahsnyder]

Is there an update to this issue? I'm currently looking at trying to run the image non-root as well.

Sorry, no update on this issue from the clamav development team.

[cmcdougall]

Thanks! I've created a pull request (#666) that fixes the issue. Let me know if you need any changes!

[micahsnyder]

@dasMulli and any others who are interested, able: If you're able to help test @cmcdougall's PR to verify that it meets your needs, that will be helpful.

[candrews]

I think a new non-root user image should be published alongside the existing root user images for a few reasons:

1. backwards compatibility. People who currently use the images expect them to be using root, so changing to non-root can (and will) surprise them. A major reason for surprise is...
2. root images cannot run commands that root can run, such as installing packages. For example, today I can run docker run -it --entrypoint /bin/sh clamav:latest then apk add curl. If the image runs as a non-root user, I can't do that.

Publishing both root and non-root images is a common pattern. For example, nginx publishes nginxinc/nginx-unprivileged (which uses the non-root user nginx) and nginx which uses root.

Therefore, I suggest clamav publish images using a tag pattern as demonstrated by these examples:

• latest
• latest-unprivileged
• 0.105.1
• 0.105.1-unprivileged

For backwards compatibility, un-suffixed images run as root, while those with the -unprivileged suffix run as the clamav user.

[cmcdougall]

I'd be happy to add a new Dockerfile for a separate unprivileged image. Would best practice be to have the unprivileged Dockerfile extend the root image and make the config changes? Or to keep them completely separate and dependent from each other?

Thanks!

[candrews]

Would best practice be to have the unprivileged Dockerfile extend the root image and make the config changes? Or to keep them completely separate and dependent from each other?

I don't have a strong opinion... I think that whatever approach is most maintainable is the way to go.

[cmcdougall]

Would best practice be to have the unprivileged Dockerfile extend the root image and make the config changes? Or to keep them completely separate and dependent from each other?

I don't have a strong opinion... I think that whatever approach is most maintainable is the way to go.

I'll update my pull request. Thanks for the suggestion!

[rmueller83]

I see no need of such an additional image. In my experience, the above mentioned case is solved by a Dockerfile like this:

FROM clamav/clamav:latest
USER root
apk add curl
USER clamav

In the end it's the decision of @micahsnyder if they want to maintain two images.

[candrews]

the above mentioned case is solved by a Dockerfile like this:

That can be a solution where it's possible.

One scenario where that isn't possible is a use case I and my colleagues use frequently - GitLab jobs.

For example:

clamav:
  image:
    name: clamav/clamav:0.105.1  # clamav official, verified image
    entrypoint: [""]
  script:
    - |
      apk add --no-cache curl
      freshclam --quiet
      # change file permissions so clamav can access them
      chown -R clamav .
      chmod -R 777 .
      su - -s /bin/sh -c "clamscan --alert-encrypted --allmatch --archive-verbose --recursive  --suppress-ok-results $(pwd)" clamav 2>&1 | grep -E -v ': Empty file$|: Symbolic link$' | tee scan_log.txt
      scan_result=$?
      curl --fail -X POST --data-urlencode "success=$scan_result" https://<redacted>

A couple immediate reasons why this needs to run as root in this case are:

• The files scanned aren't owned/accessible by the clamav user. The files are created by another job and have file permissions that restrict access. Therefore, unless the image runs as root so the permission can be modified, those files are unscannable.
• curl is installed which can only be done by root.

There are other ways to do this (such as splitting up into multiple jobs), but those have trade off (such as longer latency / more compute resources used). Plus, it would be a surprising break in backwards compatibility for this approach to work for clamav/clamav:0.105.1 but suddenly stop working for clamav/clamav:0.XXX.

[cmcdougall]

I will wait for further advice from @micahsnyder before updating my pull request

[albundy83]

Maybe if adding USER directive in Dockerfile is an issue in this MR https://github.com/Cisco-Talos/clamav/pull/666/commits/bde960c675d27ffeba2ad89371aa0186ab280582 at least you can keep all the others updates that allow running Container image under non-root user ?

[candrews]

@cmcdougall @micahsnyder can we please progress this issue?

As I said in https://github.com/Cisco-Talos/clamav/issues/478#issuecomment-1210603913 if we go with the approach of publishing only one docker image that is rootless, my organization won't be able to use the ClamAV docker image any longer and it's going to be a big problem for us, and I suspect we're not the only ones. Please publish two images (a rootless and root one) :)

[micahsnyder]

@candrews thanks for the bump on this. I spent some time today reviewing https://github.com/Cisco-Talos/clamav/pull/666 and am happy with the results. I merged the PR.

However, we cannot publish a new image with this change without a new clamav software version. I could include this change in a backport fix for the next 0.105 patch version (0.105.2). But we do not have a timeline for the next patch version. Otherwise this will have to wait until the next feature release (1.0.0), which is anticipated in late October or early November.

A better plan is to migrate our Dockerfiles out of the clamav and clamav-bytecode-compiler project repositories and into their own separate repos so we can update images on demand, and will not have to coordinate new images with new software versions. I will work towards this goal, but can not provide an ETA for the transition.

[candrews]

As I said in https://github.com/Cisco-Talos/clamav/issues/478#issuecomment-1210603913, https://github.com/Cisco-Talos/clamav/commit/28f8337d96855002e2e7388d8d147ad26fd499c3 is a huge problem for my use case.

This is a breaking change - users will be surprised by this and will experience breakage like my organization will be experiencing.

Can you please revert that commit and reconsider?

[micahsnyder]

Apologies @candrews. Too much going on at once. I skimmed the conversation far too fast, and went straight to testing the changes.

Yes, I can revert the commits.

I do not relish maintaining multiple images, but if @cmcdougall's idea to extend the image with root and make the configuration change to drop root is viable, then I say we do it that way.

With that said, we're looking at switching from Alpine to Debian (slim) so we can build multi-arch images -- something I haven't been able to do with Alpine. Ref: https://github.com/Cisco-Talos/clamav/pull/673

I am certain people will experience breakage with a switch from Alpine -> Debian as well. And though I do not relish the idea of having to maintain two different images (alpine & debian), which given the above would mean having all of:

• clamav/clamav:stable
• clamav/clamav:stable-debian
• clamav/clamav:stable-unprivileged
• clamav/clamav:stable-debian-unprivileged
• clamav/clamav:stable_base
• clamav/clamav:stable_base-debian
• clamav/clamav:stable_base-unprivileged
• clamav/clamav:stable_base-debian-unprivileged

I suppose it will all be done by automation anyways...

[albundy83]

As I said in previous comment (https://github.com/Cisco-Talos/clamav/issues/478#issuecomment-1231723807), accepting the commit WITHOUT USER clamav directive will be a first step to be able to run root-less. It will be also a better idea to specify group id when group is added: From here: https://github.com/Cisco-Talos/clamav/blob/f720b00d495248ed54a663ea117e3011a4ec0c97/Dockerfile#L106 to:

    addgroup -S -g 100 "clamav" && \

It will remove all commands that prevent image from being run under non-root user (chown and so on) that are in the docker-entrypoint.sh.

Under Kubernetes for example, specifying securityContext like this should do the job:

<...>
    spec:
      securityContext:
        runAsUser: 100
        runAsGroup: 100
        fsGroup: 100
<...>

It will also give time to @candrews to update his root stuff :-)

[msamendinger]

I agree with @albundy83. That way the image is prepared for users that want to drop privileges. They can just start the container under user clamav. Others just run default as root and won't experience any difference. If the default should change one day can be another conversation.

[micahsnyder]

I have implemented most of the changes from @cmcdougall's https://github.com/Cisco-Talos/clamav/pull/666/files PR in the Dockerfiles in our new clamav-docker repo.

For example:

• https://github.com/Cisco-Talos/clamav-docker/blob/main/clamav/0.105/alpine/Dockerfile#L84
• https://github.com/Cisco-Talos/clamav-docker/blob/main/clamav/unstable/alpine/Dockerfile#L84

Today we published updated images for the 0.105 / 0.105.1 / latest / etc. as well as for the unstable tags based on these. That is to say, the latest images now have the changes from @cmcdougall to place files in /tmp instead of under root-owned directories.

So I think the next step is to build an image based off of the current image and switch users. Maybe we can implement it in the update_db_image.sh script, in a similar way as with this clamav_db_update() function: https://github.com/Cisco-Talos/clamav-docker/blob/main/clamav/0.105/alpine/scripts/update_db_image.sh#L80-L99

Note: I'm trying to replicate the same in debian-based Dockerfiles for building multi-arch images -- something I haven't been able to do successfully with Alpine. They are more of a work in progress. If you're interested, see:

• https://github.com/Cisco-Talos/clamav-docker/tree/main/clamav/0.105/debian
• https://github.com/Cisco-Talos/clamav-docker/tree/main/clamav/latest/debian

[micahsnyder]

We did not consider the case where a custom config is already in use and specified the /run/clamav and /run/lock paths. For compatibility, we may need to revert this change back to using /run/clamav and /run/lock instead of /tmp and find a way to change the config to /tmp only for a downstream image, such as changing those paths in the downstream Dockerfile used to switch users.

[micahsnyder]

Here is the change to (mostly) revert the base Dockerfile: https://github.com/Cisco-Talos/clamav-docker/pull/1

I think the next step is going to be making a Dockerfile for the unprivileged image and incorporating that build into the Jenkinsfiles, so we can complete this ticket.