buildah icon indicating copy to clipboard operation
buildah copied to clipboard

feat: add --with-ssl-cert-file option to build command

Open aeijdenberg opened this issue 8 months ago • 9 comments

This makes the root CAs from the host available to containers during a build.

What type of PR is this?

/kind feature

What this PR does / why we need it:

This makes it easier to use buildah to build an image in environments that require custom certificate authorities to make outbound internet connections (for example when using a TLS intercepting HTTPS proxy).

Currently buildah will automatically pass https_proxy (and related) environment variables as ephemeral environment variables to the container for RUN commands.

This change adds an option --with-ssl-cert-file (defaults to false) that when set will:

  1. Resolve the root CA file currently in use by the host (uses SSL_CERT_FILE if set, else falls back to searching common paths).
  2. Mount this file within the container during the RUN command at a different location (/host-ssl-cert-file).
  3. Sets SSL_CERT_FILE=/host-ssl-cert-file ephemeral environment variable for RUN commands.

A similar outcome can be achieved today by editing buildah command to bind mount the current CA store and then edit each RUN instruction in the Containerfile to use, however it felt like it would be nice to make a short-cut for this which seems to augment well the existing https_proxy variable propagation.

Current workaround:

./bin/buildah build --secret=id=ca,src=/etc/ssl/certs/ca-certificates.crt -f - <<'EOF'
FROM alpine:latest
ENV SSL_CERT_FILE=/host-ca-file
RUN --mount=type=secret,id=ca,target=/host-ca-file echo "SSL_CERT_FILE: ${SSL_CERT_FILE} - Contents:" && head -n10 "${SSL_CERT_FILE}"
EOF

How to verify it

Behaviour without flag:

./bin/buildah build -f - <<'EOF'
FROM alpine:latest
RUN echo "SSL_CERT_FILE: ${SSL_CERT_FILE} - Contents:" && head -n10 "${SSL_CERT_FILE}"
EOF

Output:

STEP 1/2: FROM alpine:latest
STEP 2/2: RUN echo "SSL_CERT_FILE: ${SSL_CERT_FILE} - Contents:" && head -n10 "${SSL_CERT_FILE}"
SSL_CERT_FILE:  - Contents:
head: : No such file or directory
Error: building at STEP "RUN echo "SSL_CERT_FILE: ${SSL_CERT_FILE} - Contents:" && head -n10 "${SSL_CERT_FILE}"": while running runtime: exit status 1

Behaviour with flag:

./bin/buildah build --with-ssl-cert-file -f - <<'EOF'
FROM alpine:latest
RUN echo "SSL_CERT_FILE: ${SSL_CERT_FILE} - Contents:" && head -n10 "${SSL_CERT_FILE}"
EOF

Output:

STEP 1/2: FROM alpine:latest
STEP 2/2: RUN echo "SSL_CERT_FILE: ${SSL_CERT_FILE} - Contents:" && head -n10 "${SSL_CERT_FILE}"
SSL_CERT_FILE: /host-ssl-cert-file - Contents:
-----BEGIN CERTIFICATE-----
MIIH0zCCBbugAwIBAgIIXsO3pkN/pOAwDQYJKoZIhvcNAQEFBQAwQjESMBAGA1UE
(lots more output)

Tested with a local HTTPS proxy server with a self-signed CA and verified that basic apk add --update --no-cache python3 and similar commands worked without error.

(Tested similar on ubuntu image, although apt I think runs over HTTP so test wasn't as meaningful)

Includes unit and integration tests to verify behaviour.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Some issues to consider:

Search path for host CAs

To get the search path of commonly used host CA files I used the list found here: https://cs.opensource.google/go/go/+/refs/tags/go1.24.1:src/crypto/x509/root_linux.go;l=9-17

Unfortunately that variable is private, and I'm not able to find anything within the standard x509 module that exports it or otherwise makes it available, which is why I put that list in a separate file and copy/pasted the Go LICENSE within there.

It feels less than ideal and open to other ideas.

Path to bind to within the container

This PR currently binds it to /host-ssl-cert-file so that it doesn't clash with commonly used paths within a container. This seemed to work fine in my limited testing when using SSL_CERT_FILE to point to it, however it's not clear whether SSL_CERT_FILE will work for all applications and whether it may be best to instead bind this on top of well-known locations for where this file normally is.

Happy to take feedback on alternate suggestions.

Does this PR introduce a user-facing change?

Adds --with-ssl-cert-file option to build command. If set this propagates the host CAs into the container during the build process.

aeijdenberg avatar Mar 09 '25 05:03 aeijdenberg

/approve

TomSweeneyRedHat avatar Mar 20 '25 20:03 TomSweeneyRedHat

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aeijdenberg, TomSweeneyRedHat

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • ~~OWNERS~~ [TomSweeneyRedHat]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci[bot] avatar Mar 20 '25 20:03 openshift-ci[bot]

Generally looks good to me, a couple of nits and it looks like your branch needs updating.

TomSweeneyRedHat avatar Mar 20 '25 20:03 TomSweeneyRedHat

Ephemeral COPR build failed. @containers/packit-build please check.

Accepted changes, rebased, squashed and fixed the issue in the Fedora tests, by using the same pattern as for similar files (such as /etc/hostname). Thanks for the review.

aeijdenberg avatar Mar 21 '25 11:03 aeijdenberg

@nalind or @Luap99 PTAL

TomSweeneyRedHat avatar Mar 21 '25 19:03 TomSweeneyRedHat

I can certainly see why this argument might be handy but allowing this might open the door for a lot of other similar things. Personally I think this is better done by callers to set up the env and mount in any way they like. Hard coding /host-ssl-cert-file just feels super awkward, and no it is not about the specific path name but just in general.

If it wasn't set to a hard-coded path, and rather something like /tmp-3727282 instead (different random on each run) would that remove part of the objection?

ie this patch also sets env variable SSL_CERT_FILE to point to that path so it ougn't matter where it is mounted and that would avoid a hard-coded path name.

aeijdenberg avatar Mar 25 '25 09:03 aeijdenberg

If it wasn't set to a hard-coded path, and rather something like /tmp-3727282 instead (different random on each run) would that remove part of the objection?

(I was making some other changes to address other feedback, and made a change like the above at the same time and pushed that onto this branch for discussion)

aeijdenberg avatar Mar 25 '25 09:03 aeijdenberg

A friendly reminder that this PR had no activity for 30 days.

github-actions[bot] avatar May 16 '25 00:05 github-actions[bot]

For background on what I'm trying to achieve, I'd like to be able to make it easier to build images (without modifying existing Dockerfiles) in situations where they need to go through TLS intercepting HTTP proxies.

While I thought this might work well and be convenient, I think I can achieve the same using other more generic buildah features if they land.

  1. https://github.com/containers/buildah/pull/6285 which allows effectively ephemeral env variables to be "mounted" during a RUN instruction
  2. (PR to be written) an option to build which modifies all RUN instructions to add a set of mounts (similar to what the documentation for TransientMounts says it does)

I think it might be best to abandon/close this PR for now, and persue those others.

aeijdenberg avatar Jul 14 '25 22:07 aeijdenberg

#6289 opened for the second part as decribed above.

aeijdenberg avatar Jul 15 '25 00:07 aeijdenberg