From 1392987f01b748021cda4e98cbaf8e151c2f9a32 Mon Sep 17 00:00:00 2001 From: Brennan Kinney <5098581+polarathene@users.noreply.github.com> Date: Wed, 22 Mar 2023 21:18:36 +1300 Subject: [PATCH 01/10] refactor: Change base image from `alpine:3.17` to `opensuse/leap:15.4` --- Dockerfile | 91 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index 92a42c8..a49f3fe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,11 +1,87 @@ -FROM alpine:3.17 +# syntax=docker.io/docker/dockerfile:1 +# HereDoc (EOF) feature (avoids needing `&& \`) requires BuildKit: +# https://docs.docker.com/engine/reference/builder/#here-documents +ARG LEAP_VERSION=15.4 +ARG CACHE_ZYPPER=/tmp/cache/zypper +ARG INSTALL_ROOT=/rootfs -RUN apk update && \ - apk upgrade && \ - apk add bash procps drill git coreutils libidn curl socat openssl xxd && \ - rm -rf /var/cache/apk/* && \ - adduser -D -s /bin/bash testssl && \ - ln -s /home/testssl/testssl.sh /usr/local/bin/ +FROM opensuse/leap:${LEAP_VERSION} as builder +ARG CACHE_ZYPPER +ARG INSTALL_ROOT +# --mount is only necessary for persisting the zypper cache on the build host, +# Paired with --cache-dir below, RUN layer invalidation does not clear this cache. +# Not useful for CI, only local builds that retain the storage. +RUN --mount=type=cache,target="${CACHE_ZYPPER}",sharing=locked < Date: Wed, 22 Mar 2023 20:58:47 +1300 Subject: [PATCH 02/10] fix: Manually create `testssl` user and home BusyBox `adduser` was reading config from `/etc` that unnecessarily populates the home folder with various dot files. Alternative approach is to create the user and home folder manually. This avoids some extra files like the `-` suffixed backup copies from `adduser`. --- Dockerfile | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Dockerfile b/Dockerfile index a49f3fe..8820cdc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -70,17 +70,17 @@ EOF FROM scratch ARG INSTALL_ROOT COPY --link --from=builder ${INSTALL_ROOT} / +RUN <> /etc/passwd + echo 'testssl:x:1000:' >> /etc/group + echo 'testssl:!::0:::::' >> /etc/shadow -# zypper package `busybox-adduser` fails to install with `--installroot`, -# while the `shadow` package is too heavy just to add a user. -# -# Temporarily bind mount the `/bin` dir from another image that already -# has the `adduser` command, and it'll update `/etc/{group,passwd,shadow}` for us: -# Absolute path provided as some base images PATH would use those binaries instead, -# `adduser` varies in supported args, so this should avoid any surprises: -RUN --mount=type=bind,from=busybox:latest,source=/bin,target=/bin < Date: Wed, 22 Mar 2023 21:16:50 +1300 Subject: [PATCH 03/10] fix: `WORKDIR` before `adduser` avoids surprises The additions from `adduser` reading `/etc` does not appear to apply if the directory already exists, and permissions (including SGID) are adjusted properly for the home dir. This mean the excess backup copies in `/etc` are introduced again however. --- Dockerfile | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8820cdc..d5749a9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -70,24 +70,14 @@ EOF FROM scratch ARG INSTALL_ROOT COPY --link --from=builder ${INSTALL_ROOT} / -RUN <> /etc/passwd - echo 'testssl:x:1000:' >> /etc/group - echo 'testssl:!::0:::::' >> /etc/shadow - - # Create user home with SGID set: - install --mode 2755 --owner testssl --group testssl --directory /home/testssl - - # Add relative symlink to point to content that will COPY later: - ln -sr /home/testssl/testssl.sh /usr/local/bin/ +WORKDIR /home/testssl +RUN --mount=type=bind,from=busybox:latest,source=/bin,target=/bin < Date: Wed, 22 Mar 2023 21:31:34 +1300 Subject: [PATCH 04/10] chore: Revise `Dockerfile` - Removing bulk of the noise from inline documentation. - Packages bundled into single line like previous the Alpine version had. - `CACHE_ZYPPER` is only used as an `ARG` in the `builder` stage. - `zypper clean` wasn't able to clear anything from the install root, other than the `CACHE_ZYPPER` mount. --- Dockerfile | 47 ++++++++--------------------------------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/Dockerfile b/Dockerfile index d5749a9..c041ccb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,66 +1,35 @@ # syntax=docker.io/docker/dockerfile:1 -# HereDoc (EOF) feature (avoids needing `&& \`) requires BuildKit: -# https://docs.docker.com/engine/reference/builder/#here-documents + ARG LEAP_VERSION=15.4 -ARG CACHE_ZYPPER=/tmp/cache/zypper ARG INSTALL_ROOT=/rootfs FROM opensuse/leap:${LEAP_VERSION} as builder -ARG CACHE_ZYPPER +ARG CACHE_ZYPPER=/tmp/cache/zypper ARG INSTALL_ROOT # --mount is only necessary for persisting the zypper cache on the build host, # Paired with --cache-dir below, RUN layer invalidation does not clear this cache. # Not useful for CI, only local builds that retain the storage. RUN --mount=type=cache,target="${CACHE_ZYPPER}",sharing=locked < Date: Wed, 22 Mar 2023 22:13:18 +1300 Subject: [PATCH 05/10] refactor: Support syntax without BuildKit features These have been available via opt-in prior to v23 of Docker Engine with `DOCKER_BUILDKIT=1` ENV as a prefix to running `docker build`, however it's been requested to avoid the syntax. No HereDoc (multi-line RUN with EOF marker) or `RUN --mount` available. This makes the `busybox` approach a hassle, so I've brought back the explicit creation of user and home dir. Without the cache mounts, bring back `zypper clean`. It's not doing much as the `--cache-dir` is still set, but should reduce disk space for the `builder` layer. Local builds will be slower as a result when this layer is invalidated. AFAIK, this also makes it tricky to use the `ZYPPER_OPTIONS`? So no longer DRY. --- Dockerfile | 54 ++++++++++++++++++++++-------------------------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/Dockerfile b/Dockerfile index c041ccb..c4af7ce 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,44 +6,34 @@ ARG INSTALL_ROOT=/rootfs FROM opensuse/leap:${LEAP_VERSION} as builder ARG CACHE_ZYPPER=/tmp/cache/zypper ARG INSTALL_ROOT -# --mount is only necessary for persisting the zypper cache on the build host, -# Paired with --cache-dir below, RUN layer invalidation does not clear this cache. -# Not useful for CI, only local builds that retain the storage. -RUN --mount=type=cache,target="${CACHE_ZYPPER}",sharing=locked <> /etc/passwd \ + && echo 'testssl:x:1000:' >> /etc/group \ + && echo 'testssl:!::0:::::' >> /etc/shadow \ + && install --mode 2755 --owner testssl --group testssl --directory /home/testssl \ + && ln -s /home/testssl/testssl.sh /usr/local/bin/ # Copy over build context (after filtered by .dockerignore): bin/ etc/ testssl.sh COPY --chown=testssl:testssl . /home/testssl/ From 6af0a801ec4f4c5cf22794fb734422a6c3997300 Mon Sep 17 00:00:00 2001 From: Brennan Kinney <5098581+polarathene@users.noreply.github.com> Date: Wed, 22 Mar 2023 22:56:59 +1300 Subject: [PATCH 06/10] chore: Bring back `ZYPPER_OPTIONS` --- Dockerfile | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Dockerfile b/Dockerfile index c4af7ce..85dfc32 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,21 +6,18 @@ ARG INSTALL_ROOT=/rootfs FROM opensuse/leap:${LEAP_VERSION} as builder ARG CACHE_ZYPPER=/tmp/cache/zypper ARG INSTALL_ROOT -# Usually you could source this from /etc/os-release -# Might not always match the image tag: -ARG VERSION_ID=15.4 -RUN zypper --releasever "${VERSION_ID}" --installroot "${INSTALL_ROOT}" --cache-dir "${CACHE_ZYPPER}" \ - --gpg-auto-import-keys refresh \ - && zypper --releasever "${VERSION_ID}" --installroot "${INSTALL_ROOT}" --cache-dir "${CACHE_ZYPPER}" \ - --non-interactive install --download-in-advance --no-recommends \ +# /etc/os-release provides $VERSION_ID +RUN source /etc/os-release \ + && export ZYPPER_OPTIONS=( --releasever "${VERSION_ID}" --installroot "${INSTALL_ROOT}" --cache-dir "${CACHE_ZYPPER}" ) \ + && zypper "${ZYPPER_OPTIONS[@]}" --gpg-auto-import-keys refresh \ + && zypper "${ZYPPER_OPTIONS[@]}" --non-interactive install --download-in-advance --no-recommends \ bash procps grep gawk sed coreutils busybox-util-linux busybox-vi ldns libidn2-0 socat openssl curl \ - && zypper --releasever "${VERSION_ID}" --installroot "${INSTALL_ROOT}" --cache-dir "${CACHE_ZYPPER}" \ - clean --all + && zypper "${ZYPPER_OPTIONS[@]}" clean --all ## Cleanup (reclaim approx 13 MiB): # None of this content should be relevant to the container: RUN rm -r "${INSTALL_ROOT}/usr/share/"{licenses,man,locale,doc,help,info} # Functionality that the container doesn't need: -RUN rm "${INSTALL_ROOT}/usr/share/misc/termcap" \ +RUN rm "${INSTALL_ROOT}/usr/share/misc/termcap" \ && rm -r "${INSTALL_ROOT}/usr/lib/sysimage/rpm" From 48a597e19d42167aeeb4eb886c8365988d222ed6 Mon Sep 17 00:00:00 2001 From: Dirk Date: Thu, 23 Mar 2023 09:11:14 +0100 Subject: [PATCH 07/10] don't forget the kudos ;-) --- CHANGELOG.md | 1 + CREDITS.md | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3324bb2..e485992 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ * Client simulation runs in wide mode which is even better readable * Added --reqheader to support custom headers in HTTP requests * Test for support for RFC 8879 certificate compression +* Dockerfiles refactored to be multistaged: performance gain+address bugs/inconsistencies ### Features implemented / improvements in 3.0 diff --git a/CREDITS.md b/CREDITS.md index 126e0a4..bb6666c 100644 --- a/CREDITS.md +++ b/CREDITS.md @@ -84,6 +84,9 @@ Full contribution, see git log. * Hubert Kario - helped with avoiding accidental TCP fragmentation +* Brandon Kinney + - refactored multistage Dockerfiles: performance gain+address bugs/inconsistencies + * Magnus Larsen - SSL Labs Rating From bad5dedf4262754dc2b2247607498079c8466679 Mon Sep 17 00:00:00 2001 From: Dirk Date: Thu, 23 Mar 2023 14:22:05 +0100 Subject: [PATCH 08/10] correcting Brennan's name --- CREDITS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CREDITS.md b/CREDITS.md index bb6666c..9c73cf1 100644 --- a/CREDITS.md +++ b/CREDITS.md @@ -84,7 +84,7 @@ Full contribution, see git log. * Hubert Kario - helped with avoiding accidental TCP fragmentation -* Brandon Kinney +* Brennan Kinney - refactored multistage Dockerfiles: performance gain+address bugs/inconsistencies * Magnus Larsen From 90aa86ce6bbcb42148beb588e0841acf9d4a7f71 Mon Sep 17 00:00:00 2001 From: Dirk Date: Thu, 23 Mar 2023 14:45:51 +0100 Subject: [PATCH 09/10] add another contributor and change (not related to this PR but it'll be forgotten otherwise) --- CREDITS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CREDITS.md b/CREDITS.md index 9c73cf1..56c3a4b 100644 --- a/CREDITS.md +++ b/CREDITS.md @@ -176,6 +176,9 @@ Full contribution, see git log. * @nvsofts (NV) - LibreSSL patch for GOST +* @w4ntun + - fixed DNS via proxy + Probably more I forgot to mention which did give me feedback, bug reports and helped one way or another. From 91f3d9716bfcdbb135bf27c1013d1fb83b91eea9 Mon Sep 17 00:00:00 2001 From: Dirk Date: Thu, 23 Mar 2023 15:05:15 +0100 Subject: [PATCH 10/10] amending previous commit --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e485992..d21d7c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ * Client simulation runs in wide mode which is even better readable * Added --reqheader to support custom headers in HTTP requests * Test for support for RFC 8879 certificate compression +* Doesn't hang anymore when there's no local resolver * Dockerfiles refactored to be multistaged: performance gain+address bugs/inconsistencies ### Features implemented / improvements in 3.0