diff --git a/internal/config/ociartifact/ociartifact.go b/internal/config/ociartifact/ociartifact.go index 03f15a0e05ed..cb286f7b37d0 100644 --- a/internal/config/ociartifact/ociartifact.go +++ b/internal/config/ociartifact/ociartifact.go @@ -3,18 +3,20 @@ package ociartifact import ( "context" "fmt" + "io" "github.com/containers/common/libimage" - "github.com/containers/common/pkg/config" + "github.com/containers/image/v5/docker" + "github.com/containers/image/v5/manifest" + "github.com/containers/image/v5/pkg/blobinfocache" "github.com/containers/image/v5/types" - "github.com/containers/storage" "github.com/cri-o/cri-o/internal/log" ) // Impl is the main implementation interface of this package. type Impl interface { - Pull(context.Context, *types.SystemContext, string) (*Artifact, error) + Pull(context.Context, *types.SystemContext, string, string) (*Artifact, error) } // New returns a new OCI artifact implementation. @@ -24,54 +26,81 @@ func New() Impl { // Artifact can be used to manage OCI artifacts. type Artifact struct { - // MountPath is the local path containing the artifact data. - MountPath string - - // Cleanup has to be called if the artifact is not used any more. - Cleanup func() + // Data is the actual artifact content. + Data []byte } // defaultImpl is the default implementation for the OCI artifact handling. type defaultImpl struct{} -// Pull downloads and mounts the artifact content by using the provided ref. -func (*defaultImpl) Pull(ctx context.Context, sys *types.SystemContext, ref string) (*Artifact, error) { - log.Infof(ctx, "Pulling OCI artifact from ref: %s", ref) +const ( + maxArtifactSize = 1024 * 1024 // 1 MiB +) + +// Pull downloads and mounts the artifact content by using the provided image name. +func (*defaultImpl) Pull( + ctx context.Context, + sys *types.SystemContext, + img string, + enforceConfigMediaType string, +) (*Artifact, error) { + log.Infof(ctx, "Pulling OCI artifact from ref: %s", img) - storeOpts, err := storage.DefaultStoreOptions(false, 0) + name, err := libimage.NormalizeName(img) if err != nil { - return nil, fmt.Errorf("get default storage options: %w", err) + return nil, fmt.Errorf("parse image name: %w", err) } - store, err := storage.GetStore(storeOpts) + ref, err := docker.NewReference(name) if err != nil { - return nil, fmt.Errorf("get container storage: %w", err) + return nil, fmt.Errorf("create docker reference: %w", err) } - runtime, err := libimage.RuntimeFromStore(store, &libimage.RuntimeOptions{SystemContext: sys}) + src, err := ref.NewImageSource(ctx, sys) if err != nil { - return nil, fmt.Errorf("create libimage runtime: %w", err) + return nil, fmt.Errorf("build image source: %w", err) } - images, err := runtime.Pull(ctx, ref, config.PullPolicyAlways, &libimage.PullOptions{}) + manifestBytes, mimeType, err := src.GetManifest(ctx, nil) if err != nil { - return nil, fmt.Errorf("pull OCI artifact: %w", err) + return nil, fmt.Errorf("get manifest: %w", err) } - image := images[0] - mountPath, err := image.Mount(ctx, nil, "") + parsedManifest, err := manifest.FromBlob(manifestBytes, mimeType) if err != nil { - return nil, fmt.Errorf("mount OCI artifact: %w", err) + return nil, fmt.Errorf("parse manifest: %w", err) + } + if enforceConfigMediaType != "" && parsedManifest.ConfigInfo().MediaType != enforceConfigMediaType { + return nil, fmt.Errorf( + "wrong config media type %q, requires %q", + parsedManifest.ConfigInfo().MediaType, enforceConfigMediaType, + ) + } + + layers := parsedManifest.LayerInfos() + if len(layers) < 1 { + return nil, fmt.Errorf("artifact needs at least one layer") } + layer := layers[0] - cleanup := func() { - if err := image.Unmount(true); err != nil { - log.Warnf(ctx, "Unable to unmount OCI artifact path %s: %v", mountPath, err) - } + bic := blobinfocache.DefaultCache(sys) + rc, size, err := src.GetBlob(ctx, layer.BlobInfo, bic) + if err != nil { + return nil, fmt.Errorf("get layer blob: %w", err) + } + defer rc.Close() + if size < 0 { + return nil, fmt.Errorf("unknown layer blob size") + } else if size > maxArtifactSize { + return nil, fmt.Errorf("layer exceeds max size of %d bytes", maxArtifactSize) + } + + layerBytes, err := io.ReadAll(io.LimitReader(rc, maxArtifactSize)) + if err != nil { + return nil, fmt.Errorf("read from blob: %w", err) } return &Artifact{ - MountPath: mountPath, - Cleanup: cleanup, + Data: layerBytes, }, nil } diff --git a/internal/config/seccomp/seccompociartifact/seccompociartifact.go b/internal/config/seccomp/seccompociartifact/seccompociartifact.go index 2b5a8920391a..f37338e55404 100644 --- a/internal/config/seccomp/seccompociartifact/seccompociartifact.go +++ b/internal/config/seccomp/seccompociartifact/seccompociartifact.go @@ -3,9 +3,6 @@ package seccompociartifact import ( "context" "fmt" - "io/fs" - "os" - "path/filepath" "github.com/containers/image/v5/types" @@ -27,9 +24,14 @@ func New() *SeccompOCIArtifact { } } -// SeccompProfilePodAnnotation is the annotation used for matching a whole pod -// rather than a specific container. -const SeccompProfilePodAnnotation = annotations.SeccompProfileAnnotation + "/POD" +const ( + // SeccompProfilePodAnnotation is the annotation used for matching a whole pod + // rather than a specific container. + SeccompProfilePodAnnotation = annotations.SeccompProfileAnnotation + "/POD" + + // requiredConfigMediaType is the config media type for OCI artifact seccomp profiles. + requiredConfigMediaType = "application/vnd.cncf.seccomp-profile.config.v1+json" +) // TryPull tries to pull the OCI artifact seccomp profile while evaluating // the provided annotations. @@ -64,40 +66,11 @@ func (s *SeccompOCIArtifact) TryPull( return nil, nil } - artifact, err := s.ociArtifactImpl.Pull(ctx, sys, profileRef) + artifact, err := s.ociArtifactImpl.Pull(ctx, sys, profileRef, requiredConfigMediaType) if err != nil { return nil, fmt.Errorf("pull OCI artifact: %w", err) } - defer artifact.Cleanup() - - const jsonExt = ".json" - seccompProfilePath := "" - if err := filepath.Walk(artifact.MountPath, - func(p string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - if info.IsDir() || - info.Mode()&os.ModeSymlink == os.ModeSymlink || - filepath.Ext(info.Name()) != jsonExt { - return nil - } - - seccompProfilePath = p - - // TODO(sgrunert): allow merging profiles, not just choosing the first one - return fs.SkipAll - }); err != nil { - return nil, fmt.Errorf("walk %s: %w", artifact.MountPath, err) - } - - log.Infof(ctx, "Trying to read profile from: %s", seccompProfilePath) - profileContent, err := os.ReadFile(seccompProfilePath) - if err != nil { - return nil, fmt.Errorf("read %s from file store: %w", seccompProfilePath, err) - } - log.Infof(ctx, "Retrieved OCI artifact seccomp profile of len: %d", len(profileContent)) - return profileContent, nil + log.Infof(ctx, "Retrieved OCI artifact seccomp profile of len: %d", len(artifact.Data)) + return artifact.Data, nil } diff --git a/internal/config/seccomp/seccompociartifact/seccompociartifact_test.go b/internal/config/seccomp/seccompociartifact/seccompociartifact_test.go index 4b593ef0c0d8..8f63fe6a0645 100644 --- a/internal/config/seccomp/seccompociartifact/seccompociartifact_test.go +++ b/internal/config/seccomp/seccompociartifact/seccompociartifact_test.go @@ -4,8 +4,6 @@ import ( "context" "errors" "io" - "os" - "path/filepath" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" @@ -21,10 +19,7 @@ import ( // The actual test suite var _ = t.Describe("SeccompOCIArtifact", func() { t.Describe("TryPull", func() { - const ( - testProfileContent = "{}" - testSeccompJSONFile = "seccomp.json" - ) + const testProfileContent = "{}" var ( sut *seccompociartifact.SeccompOCIArtifact @@ -44,13 +39,8 @@ var _ = t.Describe("SeccompOCIArtifact", func() { ociArtifactImplMock = ociartifactmock.NewMockImpl(mockCtrl) sut.SetOCIArtifactImpl(ociArtifactImplMock) - tempDir, err := os.MkdirTemp("", "seccompociartifact-test-*") - Expect(err).NotTo(HaveOccurred()) - Expect(os.WriteFile(filepath.Join(tempDir, testSeccompJSONFile), []byte(testProfileContent), 0o644)).NotTo(HaveOccurred()) - testArtifact = &ociartifact.Artifact{ - MountPath: tempDir, - Cleanup: func() { os.RemoveAll(tempDir) }, + Data: []byte(testProfileContent), } }) @@ -71,7 +61,7 @@ var _ = t.Describe("SeccompOCIArtifact", func() { It("should match image specific annotation for whole pod", func() { // Given gomock.InOrder( - ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), + ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), ) // When @@ -88,7 +78,7 @@ var _ = t.Describe("SeccompOCIArtifact", func() { It("should match image specific annotation for container", func() { // Given gomock.InOrder( - ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), + ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), ) // When @@ -105,7 +95,7 @@ var _ = t.Describe("SeccompOCIArtifact", func() { It("should match pod specific annotation", func() { // Given gomock.InOrder( - ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), + ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), ) // When @@ -122,7 +112,7 @@ var _ = t.Describe("SeccompOCIArtifact", func() { It("should match container specific annotation", func() { // Given gomock.InOrder( - ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), + ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), ) // When @@ -152,26 +142,8 @@ var _ = t.Describe("SeccompOCIArtifact", func() { It("should fail if artifact pull fails", func() { // Given gomock.InOrder( - ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errTest), - ) - - // When - res, err := sut.TryPull(context.Background(), nil, "", nil, - map[string]string{ - seccompociartifact.SeccompProfilePodAnnotation: "test", - }) - - // Then - Expect(err).To(HaveOccurred()) - Expect(res).To(BeNil()) - }) - - It("should fail if seccomp.json is not in artifact", func() { - // Given - gomock.InOrder( - ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any()).Return(testArtifact, nil), + ociArtifactImplMock.EXPECT().Pull(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errTest), ) - Expect(os.RemoveAll(filepath.Join(testArtifact.MountPath, testSeccompJSONFile))).NotTo(HaveOccurred()) // When res, err := sut.TryPull(context.Background(), nil, "", nil, diff --git a/test/mocks/ociartifact/ociartifact.go b/test/mocks/ociartifact/ociartifact.go index 6c2c59fe5d03..95c5b6d2b11c 100644 --- a/test/mocks/ociartifact/ociartifact.go +++ b/test/mocks/ociartifact/ociartifact.go @@ -37,16 +37,16 @@ func (m *MockImpl) EXPECT() *MockImplMockRecorder { } // Pull mocks base method. -func (m *MockImpl) Pull(arg0 context.Context, arg1 *types.SystemContext, arg2 string) (*ociartifact.Artifact, error) { +func (m *MockImpl) Pull(arg0 context.Context, arg1 *types.SystemContext, arg2, arg3 string) (*ociartifact.Artifact, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Pull", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "Pull", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(*ociartifact.Artifact) ret1, _ := ret[1].(error) return ret0, ret1 } // Pull indicates an expected call of Pull. -func (mr *MockImplMockRecorder) Pull(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockImplMockRecorder) Pull(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Pull", reflect.TypeOf((*MockImpl)(nil).Pull), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Pull", reflect.TypeOf((*MockImpl)(nil).Pull), arg0, arg1, arg2, arg3) } diff --git a/test/seccomp_oci_artifacts.bats b/test/seccomp_oci_artifacts.bats index 79785f980b2a..855944276bf9 100755 --- a/test/seccomp_oci_artifacts.bats +++ b/test/seccomp_oci_artifacts.bats @@ -16,10 +16,10 @@ function teardown() { cleanup_test } -ARTIFACT_IMAGE_WITH_ANNOTATION=quay.io/crio/nginx-seccomp:generic -ARTIFACT_IMAGE_WITH_POD_ANNOTATION=quay.io/crio/nginx-seccomp:pod -ARTIFACT_IMAGE_WITH_CONTAINER_ANNOTATION=quay.io/crio/nginx-seccomp:container -ARTIFACT_IMAGE=quay.io/crio/seccomp:v1 +ARTIFACT_IMAGE_WITH_ANNOTATION=quay.io/crio/nginx-seccomp:v2 +ARTIFACT_IMAGE_WITH_POD_ANNOTATION=$ARTIFACT_IMAGE_WITH_ANNOTATION-pod +ARTIFACT_IMAGE_WITH_CONTAINER_ANNOTATION=$ARTIFACT_IMAGE_WITH_ANNOTATION-container +ARTIFACT_IMAGE=quay.io/crio/seccomp:v2 CONTAINER_NAME=container1 ANNOTATION=seccomp-profile.kubernetes.cri-o.io POD_ANNOTATION=seccomp-profile.kubernetes.cri-o.io/POD @@ -196,3 +196,16 @@ TEST_SYSCALL=OCI_ARTIFACT_TEST grep -vq "Retrieved OCI artifact seccomp profile" "$CRIO_LOG" crictl inspect "$CTR" | jq -e '.info.runtimeSpec.linux.seccomp == null' } + +@test "seccomp OCI artifact with missing artifact" { + # Run with enabled feature set + create_runtime_with_allowed_annotation seccomp $ANNOTATION + start_crio + + jq '.annotations += { "'$POD_ANNOTATION'": "wrong" }' \ + "$TESTDATA"/sandbox_config.json > "$TESTDIR"/sandbox.json + + ! crictl run "$TESTDATA/container_config.json" "$TESTDIR/sandbox.json" + + grep -q "try to pull OCI artifact seccomp profile" "$CRIO_LOG" +}