From 0f18a0144a463f853b74caedcf347ee573ef8e14 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Thu, 11 May 2023 11:51:42 -0400 Subject: [PATCH] Do not include image annotations when building spec These annotations can have security implications - crun, for example, allows rootless containers to preserve the user's groups through an annotation. We absolutely should not include annotations from an untrusted image off the internet by default. We may consider whitelisting some annotations (e.g. the legacy WASM annotations), but given that there is now a more explicit way of specifying an image uses the WASM runtime in the OCI image spec, I'm just tearing this out entirely for now. Signed-off-by: Matt Heon --- pkg/specgen/generate/container.go | 10 +++---- .../basicalpine/Containerfile.with_label | 2 ++ test/e2e/run_test.go | 26 +++++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 test/e2e/build/basicalpine/Containerfile.with_label diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index 2c9dbb7a0f..7a3f479b19 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -166,7 +166,6 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat s.Env = envLib.Join(defaultEnvs, s.Env) // Labels and Annotations - annotations := make(map[string]string) if newImage != nil { labels, err := newImage.Labels(ctx) if err != nil { @@ -183,12 +182,8 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat } } - // Add annotations from the image - for k, v := range inspectData.Annotations { - if !define.IsReservedAnnotation(k) { - annotations[k] = v - } - } + // Do NOT include image annotations - these can have security + // implications, we don't want untrusted images setting them. } // in the event this container is in a pod, and the pod has an infra container @@ -199,6 +194,7 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat // VM, which is the default behavior // - "container" denotes the container should join the VM of the SandboxID // (the infra container) + annotations := make(map[string]string) if len(s.Pod) > 0 { p, err := r.LookupPod(s.Pod) if err != nil { diff --git a/test/e2e/build/basicalpine/Containerfile.with_label b/test/e2e/build/basicalpine/Containerfile.with_label new file mode 100644 index 0000000000..b2fee6aa29 --- /dev/null +++ b/test/e2e/build/basicalpine/Containerfile.with_label @@ -0,0 +1,2 @@ +FROM quay.io/libpod/alpine:latest +LABEL testlabel=testvalue diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 506dc37042..e18fe4d36d 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -2071,4 +2071,30 @@ WORKDIR /madethis`, BB) Expect(t).To(BeTrue()) Expect(strings[0]).Should(ContainSubstring("size=10240k")) }) + + It("podman run does not preserve image annotations", func() { + annoName := "test.annotation.present" + annoValue := "annovalue" + imgName := "basicalpine" + build := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile.with_label", "--annotation", fmt.Sprintf("%s=%s", annoName, annoValue), "-t", imgName}) + build.WaitWithDefaultTimeout() + Expect(build).Should(Exit(0)) + Expect(build.ErrorToString()).To(BeEmpty(), "build error logged") + + ctrName := "ctr1" + run := podmanTest.Podman([]string{"run", "-d", "--name", ctrName, imgName, "top"}) + run.WaitWithDefaultTimeout() + Expect(run).Should(Exit(0)) + Expect(run.ErrorToString()).To(BeEmpty(), "run error logged") + + inspect := podmanTest.Podman([]string{"inspect", ctrName}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.ErrorToString()).To(BeEmpty(), "inspect error logged") + + inspectData := inspect.InspectContainerToJSON() + Expect(inspectData).To(HaveLen(1)) + Expect(inspectData[0].Config.Annotations).To(Not(HaveKey(annoName))) + Expect(inspectData[0].Config.Annotations).To(Not(HaveKey("testlabel"))) + }) })