GoogleContainerTools / rules_distroless

Apache License 2.0
55 stars 34 forks source link

fix `cacert()` to work with openssl's defaults #71

Closed lazcamus closed 3 months ago

lazcamus commented 3 months ago

The existing cacert() implementation builds the single bundle of certs in /etc/ssl/certs/ca-certifates.crt. This isn't read by default, and results in a verification error:

r00t@9c80da3575a7:~# echo | openssl s_client -connect www.google.com:443 2>&1 | grep Verify
Verify return code: 20 (unable to get local issuer certificate)

The default path for the single cert bundle is /usr/lib/ssl/cert.pem

r00t@9c80da3575a7:~# ln -sf /etc/ssl/certs/ca-certificates.crt /usr/lib/ssl/cert.pem
r00t@9c80da3575a7:~# echo | openssl s_client -connect www.google.com:443 2>&1 | grep Verify
Verify return code: 0 (ok)

So ship that symlink as part of cacert()

thesayyn commented 3 months ago

@loosebazooka is the expert, wdyt?

thesayyn commented 3 months ago

ping @loosebazooka

loosebazooka commented 3 months ago

Oh I assume it's a Debian thing. Lemme do a little reading but this looks fine

lazcamus commented 3 months ago

Yah, it's unfortunate that the default is compiled into openssl and it seems like everybody picks a different value.

In my use cases, Ubuntu's openssl uses /usr/lib/ssl, and the indygreg standalone python binaries that I'm bundling from rules_python use /etc/ssl. I'm shipping another symlink for the latter when I bundle it up for the container.

They all seem to work out of the box with the individual cert+fingerprint symlink tree in /etc/ssl/certs

On Wed, Aug 21, 2024 at 2:56 AM Appu @.***> wrote:

Oh I assume it's a Debian thing. Lemme do a little reading but this looks fine

— Reply to this email directly, view it on GitHub https://github.com/GoogleContainerTools/rules_distroless/pull/71#issuecomment-2299428347, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWXYTGNOT4M5BDZCPANYFETZSN7OBAVCNFSM6AAAAABMHWDXASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJZGQZDQMZUG4 . You are receiving this because you authored the thread.Message ID: @.***>

loosebazooka commented 2 months ago

@thesayyn I think we have to revert this. It looks to be creating some issues downstream on various distroless containers: https://github.com/GoogleContainerTools/distroless/issues/1679, https://github.com/GoogleContainerTools/distroless/issues/1676

lazcamus commented 2 months ago

GoogleContainerTools/distroless#1679

I think just making the parent directory might fix this one?

    mtree.add_parents("/usr/lib/ssl", mode = "0755", time = ctx.attr.time, skip = [1])

GoogleContainerTools/distroless#1676

this one doesn't seem related to the new symlink?

% docker run --rm -it gcr.io/distroless/base-debian12:debug
/ # mv /usr/lib/ssl/cert.pem /usr/lib/ssl/oldcert.pem
/ # wget -S https://sts.nih.gov/.well-known/openid-configuration -O -
Connecting to sts.nih.gov (128.231.243.251:443)
wget: note: TLS certificate validation not implemented
wget: got bad TLS record (len:0) while expecting handshake record
wget: error getting response: Connection reset by peer

There's also SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt set in the environment on that container ... so why would the symlink matter?

It'd be useful to know what old container it worked on to be able to bisect the breakage. My docker repository-fu isn't good enough to find old releases of the distroless base-debian12 debug image, and the image metadata setting timestamps to 1970 makes a simple timestamp sort not a viable strategy.

loosebazooka commented 2 months ago

I think something like this should be an additional function (exposed via explicit configuration).

I don't mind including this -- but I have to ensure distroless works first and always (as my primary concern) -- where we only need to be debian compliant.

lazcamus commented 2 months ago

Either way, I think something like this should be an additional function (exposed via explicit configuration).

At the risk of re-stating the obvious: without the symlink created by cacerts(), SSL cert verification is broken by default.

The distroless container images that are distributed configure around this breakage by setting an env variable: https://github.com/GoogleContainerTools/distroless/blob/f2714a06a32d11aa3c7ad16ab2a1fe81548d0bc0/base/base.bzl#L79

An alternative to the symlink would be to document the incomplete nature of cacerts(), and call out the need for the environment variable to be set on every container to get SSL to work properly?

I don't mind including this -- but I have to ensure distroless works first and always (as my primary concern) -- where we only need to be debian compliant.

I would hope that "debian compliant" means having openssl or curl .debs that do SSL securely.

On Mon, Sep 23, 2024 at 11:28 PM Appu @.***> wrote:

Either way, I think something like this should be an additional function (exposed via explicit configuration).

I don't mind including this -- but I have to ensure distroless works first and always (as my primary concern) -- where we only need to be debian compliant.

— Reply to this email directly, view it on GitHub https://github.com/GoogleContainerTools/rules_distroless/pull/71#issuecomment-2368456390, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWXYTGLZ4XNDBQIHTW4NKCTZYAQSNAVCNFSM6AAAAABMHWDXASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRYGQ2TMMZZGA . You are receiving this because you authored the thread.Message ID: @.***>

loosebazooka commented 2 months ago

Yeah those are fair clarifications. The intent is not that this feature was wrong, but that we don't know what's wrong yet and we need to revert while we figure it out.