docker-library / haproxy

Docker Official Image packaging for HAProxy
http://www.haproxy.org/
GNU General Public License v2.0
347 stars 158 forks source link

Resolving missing ca-certificates dependency that causes client ssl verify to fail #216

Closed Darlelet closed 7 months ago

Darlelet commented 1 year ago

Without this package, /etc/ssl/certs is either empty or incomplete, and so does the @system-ca variable within haproxy.

This results in some haproxy client ssl features not working out of the box. For instance, using httpclient with https endpoints will not work with default config since ssl verify is on by default.

For the debian image:

How to test it:

Darlelet commented 7 months ago

Sorry for the bump, making sure this wasn't overlooked @tianon, I can understand if this is not a priority :smile:

Thanks!

tianon commented 7 months ago

Somehow I missed this completely and appreciate the bump! :bow: :heart:

I think we should move the installation up to a separate layer (before ENV HAPROXY_VERSION) that just installs ca-certificates.

Also, I'm not thrilled about the removal of the openssl binary -- I agree it's not ideal that it gets installed too, but I don't think I understand what's solved by just removing the binary (except that now dpkg is going to be annoyed, as are any users trying to do update-ca-certificates after they install their own certs :sweat_smile:).

Darlelet commented 7 months ago

No problem! Ah yes indeed I missed that, thanks for the pointers! I moved the ca-certificates installation in the first RUN section (before ENV declarations), but maybe you meant a completely separate layer?

tianon commented 7 months ago

maybe you meant a completely separate layer?

Yeah, sorry, I meant exactly that :sweat_smile:

Maybe even a comment to remind us why so we don't remove it in the future :eyes:

diff --git a/Dockerfile.template b/Dockerfile.template
index f4154c8..0f5afbb 100644
--- a/Dockerfile.template
+++ b/Dockerfile.template
@@ -1,6 +1,13 @@
 {{ if env.variant == "alpine" then ( -}}
 FROM alpine:{{ .alpine }}

+# runtime dependencies
+RUN set -eux; \
+   apk add --no-cache \
+# @system-ca: https://github.com/docker-library/haproxy/pull/216
+       ca-certificates \
+   ;
+
 # roughly, https://git.alpinelinux.org/aports/tree/main/haproxy/haproxy.pre-install?h=3.12-stable
 RUN set -eux; \
    addgroup --gid 99 --system haproxy; \
@@ -18,6 +25,15 @@ RUN set -eux; \
 {{ ) else ( -}}
 FROM debian:{{ .debian }}

+# runtime dependencies
+RUN set -eux; \
+   apt-get update; \
+   apt-get install -y --no-install-recommends \
+# @system-ca: https://github.com/docker-library/haproxy/pull/216
+       ca-certificates \
+   ; \
+   rm -rf /var/lib/apt/lists/*
+
 # roughly, https://salsa.debian.org/haproxy-team/haproxy/-/blob/732b97ae286906dea19ab5744cf9cf97c364ac1d/debian/haproxy.postinst#L5-6
 RUN set -eux; \
    groupadd --gid 99 --system haproxy; \
tianon commented 7 months ago

Hmm, thinking about it more there's a pretty compelling argument for not installing it on Alpine, but I don't feel super strongly about it one way or another -- @yosifkit?

yosifkit commented 7 months ago

Hmm, thinking about it more there's a pretty compelling argument for not installing it on Alpine, but I don't feel super strongly about it one way or another -- @yosifkit?

It seems fine to install it. ca-certificates is installed in other webserver/proxy servers: nginx:alpine, php:alpine, and httpd:alpine. It is also relatively pretty small:


$ docker images haproxy
REPOSITORY   TAG       IMAGE ID       CREATED      SIZE
haproxy      alpine    aa6c154774d2   2 days ago   25.3MB

$ docker run -it --user root haproxy:alpine sh
# apk add --no-cache ca-certificates
...
(1/1) Installing ca-certificates (20230506-r0)
...
# apk info ca-certificates
...
ca-certificates-20230506-r0 installed size:
688 KiB
Darlelet commented 7 months ago

Thank you very much for your help guys! I updated the PR to include @tianon modifications :smile:

tianon commented 7 months ago

Your patience and long-suffering is appreciated! :heart: