-
Notifications
You must be signed in to change notification settings - Fork 728
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
Add unprivileged/non-root docker image in addition to current image that starts as root #478
Comments
We are facing this problem too! Unprivileged would be really helpful to have as an option. |
The current Dockerfile implementation is set up so that I don't really have any objection to changing things so the container runs as the 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. |
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 |
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. |
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 |
For reference, this seems to have worked for me. Albeit with some modifications to the default configuration files as well.
|
My diff so far is main...dasMulli:feature/nonroot-container |
Also looking for a solution to run the container as non-root. clamav/dockerfiles/docker-entrypoint.sh Lines 34 to 36 in 140c88a
@dasMulli I was looking through your diff and wondering why the location for the pid file and socket get moved to |
It is an unfinished snapshot, the permissions for No timeline yet but before the end of the month |
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. |
Thanks! I've created a pull request (#666) that fixes the issue. Let me know if you need any changes! |
@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. |
I think a new non-root user image should be published alongside the existing root user images for a few reasons:
Publishing both root and non-root images is a common pattern. For example, nginx publishes nginxinc/nginx-unprivileged (which uses the non-root user Therefore, I suggest clamav publish images using a tag pattern as demonstrated by these examples:
For backwards compatibility, un-suffixed images run as root, while those with the |
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! |
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! |
I see no need of such an additional image. In my experience, the above mentioned case is solved by a Dockerfile like this:
In the end it's the decision of @micahsnyder if they want to maintain two images. |
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:
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 |
I will wait for further advice from @micahsnyder before updating my pull request |
Maybe if adding USER directive in Dockerfile is an issue in this MR bde960c at least you can keep all the others updates that allow running Container image under non-root user ? |
@cmcdougall @micahsnyder can we please progress this issue? As I said in #478 (comment) 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) :) |
@candrews thanks for the bump on this. I spent some time today reviewing #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. |
As I said in #478 (comment), 28f8337 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? |
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: #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:
I suppose it will all be done by automation anyways... |
As I said in previous comment (#478 (comment)), accepting the commit WITHOUT Line 106 in f720b00
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 <...>
spec:
securityContext:
runAsUser: 100
runAsGroup: 100
fsGroup: 100
<...> It will also give time to @candrews to update his root stuff :-) |
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 |
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 For example:
Today we published updated images for the 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
|
We did not consider the case where a custom config is already in use and specified the /run/clamav and /run/lock paths. |
Here is the change to (mostly) revert the base Dockerfile: Cisco-Talos/clamav-docker#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. |
I think I have a compromise to continue with the change to use /tmp for the new config, while creating the directories necessary for folks using the /run/clamav and /run/lock settings in the original config. |
The change to use /tmp/clamd.sock instead of /var/run/clamd.sock was an intentional step towards being able to build an unprivileged/rootless image as requested by Cisco-Talos/clamav#478 It looks like we forgot to update the documentation to note the change in the clamd.sock location.
The change to use /tmp/clamd.sock instead of /var/run/clamd.sock was an intentional step towards being able to build an unprivileged/rootless image as requested by Cisco-Talos/clamav#478 It looks like we forgot to update the documentation to note the change in the clamd.sock location.
Has the issue been fixed yet? I'm still facing the permission denied on folder '/run/clamav' when set the user as non root. |
would love to see an update on this as well |
I ended up integrating my own Dockerfile and init-file for a variety of problems (proxy, non-root, additional configuration options). The progress here is very slow and I dont expect this to be solved soon at all. You might want to do the same as this is relevant for maintaining security ... |
could you care to post your solution here ? - as this would greatly speed up solution to our problem as well. |
Is it possible to get this maintained by Cisco-Talos? @boindil I see you have some hints and improvements in regards of the rootless image you have made. Is it possible for you to collaborate or introduce a PR that could be maintained by the community surrounding Cisco-Talos? @boindil thank you for the contribution and sharing what you got :) |
another solution (built on top of the clamav official docker image): https://github.com/dradoaica/clamav-docker-openshift P.S.: already built docker image: https://hub.docker.com/r/dradoaica/clamav-openshift/tags |
@dradoaica I have found your implementation perfect to overcome this problem. I try using an even more restrictive container security context, setting readOnlyRootFilesystem: true, but run into some issues like this when trying to scan a file:
I also tried changing folder permissions via Dockerfile, but the problem persists. Have you maybe done any analysis on this aspect or do you have any suggestions? |
If you are using the helm chart from my repo, you need to add another volume mount for the tmp folder:
|
A clean way to start rootless under Kubernetes for example is to simply use the already built feature, here a simple Deployment: apiVersion: apps/v1
kind: Deployment
metadata:
name: clamav
labels:
app: clamav
app.kubernetes.io/component: clamav
app.kubernetes.io/instance: clamav
app.kubernetes.io/managed-by: Kustomize
app.kubernetes.io/name: clamav
spec:
replicas: 1
selector:
matchLabels:
app: clamav
template:
metadata:
name: clamav
labels:
app: clamav
app.kubernetes.io/component: clamav
app.kubernetes.io/instance: clamav
app.kubernetes.io/managed-by: Kustomize
app.kubernetes.io/name: clamav
spec:
automountServiceAccountToken: false
securityContext:
runAsUser: 1000
runAsGroup: 1000
fsGroup: 1000
containers:
- name: clamav
image: docker.io/clamav/clamav-debian:1.4.0-1_base
imagePullPolicy: IfNotPresent
ports:
- name: clamd
containerPort: 3310
protocol: TCP
- name: clamav-milter
containerPort: 7357
protocol: TCP
env:
- name: CLAMAV_NO_MILTERD
value: 'false'
command:
- /bin/bash
args:
- -c
- /init-unprivileged # The very important part :)
livenessProbe:
exec:
command:
- /usr/local/bin/clamdcheck.sh
failureThreshold: 1
periodSeconds: 10
readinessProbe:
exec:
command:
- /usr/local/bin/clamdcheck.sh
initialDelaySeconds: 5
periodSeconds: 5
startupProbe:
exec:
command:
- /usr/local/bin/clamdcheck.sh
failureThreshold: 30
periodSeconds: 10
resources:
limits:
cpu: '1'
ephemeral-storage: 1Gi
memory: 4Gi
requests:
cpu: '0.1'
ephemeral-storage: 300Mi
memory: 64Mi
volumeMounts:
- name: tmp
mountPath: /tmp
- name: clamav-lib
mountPath: /var/lib/clamav
- name: clamav-log
mountPath: /var/log/clamav
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
capabilities:
drop:
- ALL
volumes:
- name: tmp
emptyDir: {}
- name: clamav-lib
emptyDir: {}
- name: clamav-log
emptyDir: {} If you don't want to manage logs in Maybe this issue could be closed no ? |
The provided docker-built container contains user
clamav
(100) and groupclamav
(101), however when trying to run as this user/group, the image fails to start (same for current :stable and :latest tags):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):
The text was updated successfully, but these errors were encountered: