all-in-one icon indicating copy to clipboard operation
all-in-one copied to clipboard

feat(clamav): clamav arm64 support using debian based image

Open jogerj opened this issue 10 months ago • 15 comments

Solves #4223

Implementation based on https://github.com/docker/buildx/issues/805#issuecomment-946478949

edit: sorry for force pushes, not used to using DCO and needed a couple retries

jogerj avatar Apr 02 '24 13:04 jogerj

I would maybe add --platform (https://docs.docker.com/reference/dockerfile/#from) to both arch FROM steps

Zoey2936 avatar Apr 02 '24 14:04 Zoey2936

Also ARG TARGETARCH should be added to the beginning of the Dockerfile

Zoey2936 avatar Apr 02 '24 14:04 Zoey2936

hadolint reports DL3006 but it's a false positive due to arg-generated image name. See https://github.com/hadolint/hadolint/issues/979

we can fix this with

# hadolint ignore=DL3006
FROM build-${TARGETARCH}

Also ARG TARGETARCH should be added to the beginning of the Dockerfile

BuildKit should already resolve this value so we don't need to redefine this. https://github.com/docker/buildx/issues/574#issuecomment-808601577

TODO

So for amd64, it will retain its current behavior with alpine images but for arm64 this will be based off debian image

  • [ ] How do we test clamav on arm64? ~~Will this be released as beta or something?~~
  • [x] Update docs that says "clamav on arm64 not supported"

jogerj avatar Apr 02 '24 14:04 jogerj

https://github.com/nextcloud/all-in-one/blob/9736a77f10d36a1726fe3891902399251f45c0cb/php/templates/containers.twig#L600 you also need to remove this if part and move the script line to the other script lines in the isAnyRunning part below

Zoey2936 avatar Apr 02 '24 15:04 Zoey2936

BuildKit should already resolve this value so we don't need to redefine this. https://github.com/docker/buildx/issues/574#issuecomment-808601577

didn't know that, but for RUN steps it seems to be needed

Zoey2936 avatar Apr 02 '24 15:04 Zoey2936

Honestly, I am still not sure about this. I am wondering if we should rather go the Mailcow way: https://github.com/mailcow/mailcow-dockerized/blob/master/data/Dockerfiles/clamd/Dockerfile because of security and size concerns. However this would put the maintenance on our burden. So both possibilities are not great imho...

szaimen avatar Apr 02 '24 15:04 szaimen

Honestly, I am still not sure about this. I am wondering if we should rather go the Mailcow way: https://github.com/mailcow/mailcow-dockerized/blob/master/data/Dockerfiles/clamd/Dockerfile because of security and size concerns. However this would put the maintenance on our burden. So both possibilities are not great imho...

yes I would also prefer downloading clamav from alpine, but that would remove version pinning

Zoey2936 avatar Apr 02 '24 15:04 Zoey2936

Honestly, I am still not sure about this. I am wondering if we should rather go the Mailcow way: https://github.com/mailcow/mailcow-dockerized/blob/master/data/Dockerfiles/clamd/Dockerfile because of security and size concerns. However this would put the maintenance on our burden. So both possibilities are not great imho...

~~well we can just use the compiled image from mailcow https://hub.docker.com/r/mailcow/clamd then the question just becomes do we want to have mailcow as a dependency 🤔~~ after careful delibration, this is likely not what we want as mailcow's clamd includes custom rules and whitelists specific for emails.

mailcow is GPLv3 licensed -> should be compatible to Nextcloud if we choose to reuse and modify it.

jogerj avatar Apr 02 '24 16:04 jogerj

I would maybe add --platform (https://docs.docker.com/reference/dockerfile/#from) to both arch FROM steps

it seems adding this breaks a different hadolint rule DL3029, although I'm not sure if this should be added as exception.

jogerj avatar Apr 02 '24 16:04 jogerj

it seems adding this breaks a different hadolint rule DL3029, although I'm not sure if this should be added as exception.

can be ignored

Zoey2936 avatar Apr 02 '24 16:04 Zoey2936

It seems that clamav-docker maintainers refuse to add alpine arm64 images. If we fully commit to just use clamav-debian images, the changes would've been very minimal. Size delta is ~60MB while security wise it's just as good imo.

# syntax=docker/dockerfile:latest
# Probably from this file: https://github.com/Cisco-Talos/clamav-docker/blob/main/clamav/1.3/debian/Dockerfile
FROM clamav/clamav-debian:1.3.0-24

COPY clamav.conf /tmp/clamav.conf

# DL3008 warning: Pin versions in apt get install. Instead of `apt-get install <package>` use `apt-get install <package>=<version>`
# hadolint ignore=DL3008
RUN set -ex; \
    apt-get update; \
    apt-get install --no-install-recommends -y \
        tzdata \
    ; \
    rm -vrf /var/lib/apt/lists/*; \
    cat /tmp/clamav.conf >> /etc/clamav/clamd.conf; \
    rm /tmp/clamav.conf; \
    mkdir -p /var/run/clamav /run/lock; \
    chown -R clamav:clamav /var/run/clamav /run/clamav /var/log/clamav /var/lock /run/lock; \
    chmod 777 -R /var/run/clamav /run/clamav /var/log/clamav /var/lock /run/lock /tmp

VOLUME /var/lib/clamav

USER clamav

LABEL com.centurylinklabs.watchtower.enable="false"

P.S. I experienced issues when building the image on my Windows machine. I would recommend adding .gitattributes file to ensure line endings of clamav.conf remain LF, otherwise clamav won't parse the resulting conf file correctly

jogerj avatar Apr 03 '24 20:04 jogerj

I am wondering should we maybe simply use the mailcow/clamd:1.65 image here and customize it if needed?

szaimen avatar May 14 '24 13:05 szaimen

I am wondering should we maybe simply use the mailcow/clamd:1.65 image here and customize it if needed?

Well do we want the mailcow image become a dependency to the project? This assumes of course they don't add breaking changes of the docker image in the future. As for customization, we could override the hardcoded whitelist with RUN touch /etc/clamav/whitelist.ign2 then the default mailcow configuration should be mostly same.

jogerj avatar May 14 '24 13:05 jogerj

theier image is bassically apk add clamav: https://github.com/mailcow/mailcow-dockerized/blob/master/data/Dockerfiles/clamd/Dockerfile I think we could also create our own dockerfile with alpine as base

Zoey2936 avatar May 14 '24 14:05 Zoey2936

theier image is bassically apk add clamav: https://github.com/mailcow/mailcow-dockerized/blob/master/data/Dockerfiles/clamd/Dockerfile I think we could also create our own dockerfile with alpine as base

yeah but we would need to maintain their scripts in that case too, now? https://github.com/mailcow/mailcow-dockerized/tree/master/data/Dockerfiles/clamd I mean we could probably sync them over...

szaimen avatar May 17 '24 12:05 szaimen