buildah 1.38.1 build "FROM oci-archive:" fails to find content generated inside a build

[henrywang]

quay.io/buildah/stable:v1.38.1 failed to build image with FROM oci-archive:./out.ociarchive and got error:

[2/2] STEP 1/3: FROM oci-archive:./out.ociarchive
Error: creating build container: creating temp directory: archive file not found: "/builds/redhat/rhel/bifrost/compose-bootc/out.ociarchive"

quay.io/buildah/stable:v1.38.0 does not have this issue.

[vrothberg]

Detected in the Image Mode pipelines which cannot use v1.38.1 for now.

https://gitlab.com/redhat/rhel/bifrost/compose-bootc/-/merge_requests/208/diffs

[vrothberg]

Another MR only bumping buildah fails as well.

[vrothberg]

@henrywang do we have an easy reproducer?

[cgwalters]

I think this is a regression from 25a3b38

This feature is really key as a way to generate chunked/reproducible containers and we've been using it for quite some time for rpm-ostree, and we have plans in the immediate future to promote this more for custom base images (ref https://gitlab.com/fedora/bootc/tracker/-/issues/32 and coreos/rpm-ostree#5221 specifically)

Way back in coreos/rpm-ostree#4688 (comment) there was a soft commitment to support this.

My understanding is that there are security implications (not mentioned in the commit message) that motivated 25a3b38

Can we elaborate on those now that the commit is public (I don't see a corresponding PR with discussion)?

It seems to me that what we always wanted here is a way to write not to the build context, but just any internal filesystem (a tmpfs or a tempdir bind mount) whose lifetime is scoped to the entire build, right?

[mheon]

The relevant CVE is GHSA-5vpc-35f4-r8w6

A core part of this was the ability to write to the host filesystem from mounts, which is why it was disabled - so what you desire is a large part of a high-severity CVE. I don't see us turning this back on. Adding an option with the understanding that enabling it is explicitly insecure and exposes the host to potential escape by malicious Dockerfile is a potential option, but that also feels very bad.

[cgwalters]

@henrywang do we have an easy reproducer?

Here's a trivial example of a "build" that actually just copies from a registry into a local oci: directory via skopeo - but the point is that an "oci" directory is something that any tool running in the container build can synthesize however it wants - without e.g. being subject to injected floating-timestamp content from buildah itself etc. but also more generally having total power over structuring the layout of the target container image.

FROM registry.access.redhat.com/ubi9/ubi:latest  as builder
RUN --mount=type=bind,rw=true,src=.,dst=/buildcontext,bind-propagation=shared \
  dnf -y install skopeo && skopeo copy docker://busybox oci:/buildcontext/out.oci

FROM oci:./out.oci
# Need to reference builder here to force ordering. But since we have to run
# something anyway, we might as well cleanup after ourselves.
RUN --mount=type=bind,from=builder,src=.,target=/var/tmp \
    --mount=type=bind,rw=true,src=.,dst=/buildcontext,bind-propagation=shared \
      rm /buildcontext/out.oci -rf

[cgwalters]

Adding an option with the understanding that enabling it is explicitly insecure and exposes the host to potential escape by malicious Dockerfile is a potential option, but that also feels very bad.

Yes, agree adding an option isn't what we want here. Let's instead dig more into the final paragraph of my previous comment:

It seems to me that what we always wanted here is a way to write not to the build context, but just any internal filesystem (a tmpfs or a tempdir bind mount) whose lifetime is scoped to the entire build, right?

To flesh this out slightly...maybe we could use RUN --mount=type=cache for this since it's already defined to be a writable, persistent-across-run-invocations area. IOW we change to:

FROM registry.access.redhat.com/ubi9/ubi:latest  as builder
RUN --mount=type=cache,sharing=private,dst=/output \
  dnf -y install skopeo && skopeo copy docker://busybox oci:/output/out.oci

# Here I made up the syntax //<cachename>/ to reference a cache
FROM oci://output/out.oci

Note if we take this step and formalize things more it actually gets a lot cleaner as the builder can know the ordering here and we can declare that the cached OCI directory is automatically consumed (since why would you leave it around)?

[nalind]

Note if we take this step and formalize things more it actually gets a lot cleaner as the builder can know the ordering here and we can declare that the cached OCI directory is automatically consumed (since why would you leave it around)?

Caches are not scoped to the lifetime of a given build.

[cgwalters]

Caches are not scoped to the lifetime of a given build.

Yes I know - its lifetime is longer - which isn't needed here, but should still work. In my proposal you'd just end up with an empty directory/image as a cache; which actually we could detect that's the case and just discard.

Do you have any alternative proposals?

[cgwalters]

Maybe another possible fix would be to expand the lifetime of the overlayfs created from a RUN --mount=type=bind,rw=true,src=.,dst=/buildcontext,bind-propagation=shared to the entire build, instead of each RUN invocation?

I guess though a downside of this might be that it's different from what Docker is doing today, which could confuse people.

But at the cost of getting more hacky, we could do that scope expansion only if we detect there's a FROM oci: in the build?

[cgwalters]

I should emphasize here though again: our production builds use this feature today. Having a working version of this (and really ideally compatible with the existing Containerfile, although that wouldn't be a requirement) as relatively soon as possible (e.g. < 1 month) would be quite helpful.

I think in the short term we can just hold off on applying the buildah update, but that's obviously not sustainable. Among other things, a big part of the entire point of this is that we can tell people the base image build is just "podman build" today, and after updating that no longer works.

[cgwalters]

Maybe another possible fix would be to expand the lifetime of the overlayfs created from a RUN --mount=type=bind,rw=true,src=.,dst=/buildcontext,bind-propagation=shared to the entire build, instead of each RUN invocation?

It looks like bind-propagatation is a buildah-specific extension not supported by Docker; so perhaps we could also key off that as a lifetime extension hint? Though when I test things out, a build works without it - so either it was never required or something else changed in buildah.

I guess the more I think about the more I'd say:

• Short term, detect FROM oci in the image and lifetime expand any RUN --mount=type=bind,rw=true,src=.,dst=/buildcontext,bind-propagation=shared to the entire build (mounts identified by source+destination pair)
• Medium term, try to come up with a design that's cleaner and can ideally be added to docker (and other builders) as well, for which I think I like the FROM oci://output/out.oci or FROM --mount=type=cache,target=/output oci:/output/out.oci or so.

[nalind]

I would have (and have previously) suggested solving this at the pipeline level.

Anyway, the reason the FROM fails is that the OCI archive is no longer being written to the actual build context directory by the previous stage, so it's literally not where the FROM instruction says it is. Unless it's a remote build being done via podman, a -v flag at the command line can be used instead of a --mount=type=bind in the Containerfile to expose the build context directory to RUN instructions.

When we teach the --build-context command line flag about OCI layouts (and possibly archives), the value on the FROM line will be able to be replaced by the nickname given as part of the flag's argument. That'll back us off just a bit further away from having custom behavior in Containerfiles, which is a direction I've come to generally try to discourage because the target we're trying to be compatible with moves.

[nalind]

It looks like bind-propagatation is a buildah-specific extension not supported by Docker

It looks like that flag is there because the set of accepted flags for buildah run --mount was modeled after podman run's --mount flag, and the logic for parsing its key-value arguments list was reused when it was later implemented for RUN in Containerfiles.