[CONTINT-3702] fix(sbom): Fix mount path retrieval from overlayfs (#2…

…5886)

* Get mounts from overlayfs snapshotter, similarly as what is done in MountImage

* prefer opts.UseMount, remove unused NoCache opts, use cache only when scanning images

* move scan from snapshotter out of the default function

pkg/sbom/collectors/containerd/containerd.go

@@ -13,6 +13,8 @@ import (
 	"fmt"
 	"reflect"
 
+	"github.com/containerd/containerd"
+
 	"github.com/DataDog/datadog-agent/comp/core/config"
 	"github.com/DataDog/datadog-agent/comp/core/workloadmeta"
 	"github.com/DataDog/datadog-agent/pkg/sbom"
@@ -33,6 +35,8 @@ type scanRequest struct {
 	imageID string
 }
 
+type scannerFunc func(ctx context.Context, imgMeta *workloadmeta.ContainerImageMetadata, img containerd.Image, client cutil.ContainerdItf, scanOptions sbom.ScanOptions) (sbom.Report, error)
+
 // NewScanRequest creates a new scan request
 func NewScanRequest(imageID string) sbom.ScanRequest {
 	return scanRequest{imageID: imageID}
@@ -64,8 +68,7 @@ type Collector struct {
 	containerdClient cutil.ContainerdItf
 	wmeta            optional.Option[workloadmeta.Component]
 
-	fromFileSystem bool
-	closed         bool
+	closed bool
 }
 
 // CleanCache cleans the cache
@@ -81,7 +84,6 @@ func (c *Collector) Init(cfg config.Component, wmeta optional.Option[workloadmet
 	}
 	c.wmeta = wmeta
 	c.trivyCollector = trivyCollector
-	c.fromFileSystem = cfg.GetBool("sbom.container_image.use_mount")
 	c.opts = sbom.ScanOptionsFromConfig(cfg, true)
 	return nil
 }
@@ -117,23 +119,15 @@ func (c *Collector) Scan(ctx context.Context, request sbom.ScanRequest) sbom.Sca
 	}
 
 	var report sbom.Report
-	if c.fromFileSystem {
-		report, err = c.trivyCollector.ScanContainerdImageFromFilesystem(
-			ctx,
-			imageMeta,
-			image,
-			c.containerdClient,
-			c.opts,
-		)
+	var scanner scannerFunc
+	if c.opts.UseMount {
+		scanner = c.trivyCollector.ScanContainerdImageFromFilesystem
+	} else if c.opts.OverlayFsScan {
+		scanner = c.trivyCollector.ScanContainerdImageFromSnapshotter
 	} else {
-		report, err = c.trivyCollector.ScanContainerdImage(
-			ctx,
-			imageMeta,
-			image,
-			c.containerdClient,
-			c.opts,
-		)
+		scanner = c.trivyCollector.ScanContainerdImage
 	}
+	report, err = scanner(ctx, imageMeta, image, c.containerdClient, c.opts)
 	scanResult := sbom.ScanResult{
 		Error:   err,
 		Report:  report,

pkg/sbom/collectors/docker/docker.go

@@ -27,6 +27,8 @@ import (
 // 1000 is already a very large default value
 const resultChanSize = 1000
 
+type scannerFunc func(ctx context.Context, imgMeta *workloadmeta.ContainerImageMetadata, client client.ImageAPIClient, scanOptions sbom.ScanOptions) (sbom.Report, error)
+
 // scanRequest defines a scan request. This struct should be
 // hashable to be pushed in the work queue for processing.
 type scanRequest struct {
@@ -106,7 +108,13 @@ func (c *Collector) Scan(ctx context.Context, request sbom.ScanRequest) sbom.Sca
 		return sbom.ScanResult{Error: fmt.Errorf("image metadata not found for image id %s: %s", dockerScanRequest.ID(), err)}
 	}
 
-	report, err := c.trivyCollector.ScanDockerImage(
+	var scanner scannerFunc
+	if c.opts.OverlayFsScan {
+		scanner = c.trivyCollector.ScanDockerImageFromGraphDriver
+	} else {
+		scanner = c.trivyCollector.ScanDockerImage
+	}
+	report, err := scanner(
 		ctx,
 		imageMeta,
 		c.cl,

pkg/sbom/sbom.go

@@ -34,9 +34,9 @@ type ScanOptions struct {
 	Timeout          time.Duration
 	WaitAfter        time.Duration
 	Fast             bool
-	NoCache          bool // Caching doesn't really provide any value when scanning filesystem as the filesystem has to be walked to compute the keys
 	CollectFiles     bool
 	UseMount         bool
+	OverlayFsScan    bool
 }
 
 // ScanOptionsFromConfig loads the scanning options from the configuration
@@ -48,8 +48,7 @@ func ScanOptionsFromConfig(cfg config.Component, containers bool) (scanOpts Scan
 		scanOpts.WaitAfter = time.Duration(cfg.GetInt("sbom.container_image.scan_interval")) * time.Second
 		scanOpts.Analyzers = cfg.GetStringSlice("sbom.container_image.analyzers")
 		scanOpts.UseMount = cfg.GetBool("sbom.container_image.use_mount")
-	} else {
-		scanOpts.NoCache = true
+		scanOpts.OverlayFsScan = cfg.GetBool("sbom.container_image.overlayfs_direct_scan")
 	}
 
 	if len(scanOpts.Analyzers) == 0 {

pkg/util/containerd/containerd_util.go

@@ -16,11 +16,12 @@ import (
 	"strings"
 	"time"
 
+	"github.com/opencontainers/image-spec/identity"
+
 	"github.com/DataDog/datadog-agent/pkg/config"
 	dderrors "github.com/DataDog/datadog-agent/pkg/errors"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 	"github.com/DataDog/datadog-agent/pkg/util/retry"
-	"github.com/opencontainers/image-spec/identity"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/api/types"
@@ -70,6 +71,7 @@ type ContainerdItf interface {
 	CallWithClientContext(namespace string, f func(context.Context) error) error
 	IsSandbox(namespace string, ctn containerd.Container) (bool, error)
 	MountImage(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image, targetDir string) (func(context.Context) error, error)
+	Mounts(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image) ([]mount.Mount, error)
 }
 
 // ContainerdUtil is the util used to interact with the Containerd api.
@@ -384,28 +386,28 @@ func (c *ContainerdUtil) IsSandbox(namespace string, ctn containerd.Container) (
 	return labels["io.cri-containerd.kind"] == "sandbox", nil
 }
 
-// MountImage mounts the given image in the targetDir specified
-func (c *ContainerdUtil) MountImage(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image, targetDir string) (func(context.Context) error, error) {
+// getMounts retrieves mounts and returns a function to clean the snapshot and release the lease. The lease is already released in error cases.
+func (c *ContainerdUtil) getMounts(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image) ([]mount.Mount, func(context.Context) error, error) {
 	snapshotter := containerd.DefaultSnapshotter
 	ctx = namespaces.WithNamespace(ctx, namespace)
 
 	// Checking if image is already unpacked
 	imgUnpacked, err := img.IsUnpacked(ctx, snapshotter)
 	if err != nil {
-		return nil, fmt.Errorf("unable to check if image named: %s is unpacked, err: %w", img.Name(), err)
+		return nil, nil, fmt.Errorf("unable to check if image named: %s is unpacked, err: %w", img.Name(), err)
 	}
 	if !imgUnpacked {
-		return nil, fmt.Errorf("unable to scan image named: %s, image is not unpacked", img.Name())
+		return nil, nil, fmt.Errorf("unable to scan image named: %s, image is not unpacked", img.Name())
 	}
 
 	// Getting image id
 	imgConfig, err := img.Config(ctx)
 	if err != nil {
-		return nil, fmt.Errorf("unable to get image config for image named: %s, err: %w", img.Name(), err)
+		return nil, nil, fmt.Errorf("unable to get image config for image named: %s, err: %w", img.Name(), err)
 	}
 	imageID := imgConfig.Digest.String()
 
-	// Adding a lease to cleanup dandling snaphots at expiration
+	// Adding a lease to cleanup dangling snapshots at expiration
 	ctx, done, err := c.cl.WithLease(ctx,
 		leases.WithID(imageID),
 		leases.WithExpiration(expiration),
@@ -414,7 +416,7 @@ func (c *ContainerdUtil) MountImage(ctx context.Context, expiration time.Duratio
 		}),
 	)
 	if err != nil && !errdefs.IsAlreadyExists(err) {
-		return nil, fmt.Errorf("unable to get a lease, err: %w", err)
+		return nil, nil, fmt.Errorf("unable to get a lease, err: %w", err)
 	}
 
 	// Getting top layer image id
@@ -423,18 +425,17 @@ func (c *ContainerdUtil) MountImage(ctx context.Context, expiration time.Duratio
 		if err := done(ctx); err != nil {
 			log.Warnf("Unable to cancel containerd lease with id: %s, err: %v", imageID, err)
 		}
-		return nil, fmt.Errorf("unable to get layers digests for image: %s, err: %w", imageID, err)
+		return nil, nil, fmt.Errorf("unable to get layers digests for image: %s, err: %w", imageID, err)
 	}
 	chainID := identity.ChainID(diffIDs).String()
-
-	// Creating snaphot for the top layer
+	// Creating snapshot for the top layer
 	s := c.cl.SnapshotService(snapshotter)
 	mounts, err := s.View(ctx, imageID, chainID)
 	if err != nil && !errdefs.IsAlreadyExists(err) {
 		if err := done(ctx); err != nil {
 			log.Warnf("Unable to cancel containerd lease with id: %s, err: %v", imageID, err)
 		}
-		return nil, fmt.Errorf("unable to build snapshot for image: %s, err: %w", imageID, err)
+		return nil, nil, fmt.Errorf("unable to build snapshot for image: %s, err: %w", imageID, err)
 	}
 	cleanSnapshot := func(ctx context.Context) error {
 		return s.Remove(ctx, imageID)
@@ -448,7 +449,7 @@ func (c *ContainerdUtil) MountImage(ctx context.Context, expiration time.Duratio
 		if err := done(ctx); err != nil {
 			log.Warnf("Unable to cancel containerd lease with id: %s, err: %v", imageID, err)
 		}
-		return nil, fmt.Errorf("No snapshots returned for image: %s, err: %w", imageID, err)
+		return nil, nil, fmt.Errorf("No snapshots returned for image: %s", imageID)
 	}
 
 	// Transforming mounts in case we're running in a container
@@ -460,32 +461,47 @@ func (c *ContainerdUtil) MountImage(ctx context.Context, expiration time.Duratio
 			}
 		}
 	}
-
-	// Mouting returned mounts
-	log.Infof("Mounting %+v to %s", mounts, targetDir)
-	if err := mount.All(mounts, targetDir); err != nil {
+	return mounts, func(ctx context.Context) error {
+		ctx = namespaces.WithNamespace(ctx, namespace)
 		if err := cleanSnapshot(ctx); err != nil {
 			log.Warnf("Unable to clean snapshot with id: %s, err: %v", imageID, err)
 		}
 		if err := done(ctx); err != nil {
 			log.Warnf("Unable to cancel containerd lease with id: %s, err: %v", imageID, err)
 		}
-		return nil, fmt.Errorf("unable to mount image %s to dir %s, err: %w", imageID, targetDir, err)
+		return nil
+	}, nil
+}
+
+// Mounts returns the mounts for an image
+func (c *ContainerdUtil) Mounts(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image) ([]mount.Mount, error) {
+	mounts, clean, err := c.getMounts(ctx, expiration, namespace, img)
+	if err != nil {
+		return nil, err
+	}
+	if err := clean(ctx); err != nil {
+		return nil, fmt.Errorf("unable to clean snapshot, err: %w", err)
 	}
+	return mounts, nil
+}
 
+// MountImage mounts an image to a directory
+func (c *ContainerdUtil) MountImage(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image, targetDir string) (func(context.Context) error, error) {
+	mounts, clean, err := c.getMounts(ctx, expiration, namespace, img)
+	if err != nil {
+		return nil, err
+	}
+	if err := mount.All(mounts, targetDir); err != nil {
+		if err := clean(ctx); err != nil {
+			log.Warnf("Unable to clean snapshot, err: %v", err)
+		}
+		return nil, fmt.Errorf("unable to mount image %s to dir %s, err: %w", img.Name(), targetDir, err)
+	}
 	return func(ctx context.Context) error {
 		ctx = namespaces.WithNamespace(ctx, namespace)
-
 		if err := mount.UnmountAll(targetDir, 0); err != nil {
-			return fmt.Errorf("unable to unmount directory: %s for image: %s, err: %w", targetDir, imageID, err)
+			return fmt.Errorf("unable to unmount directory: %s for image: %s, err: %w", targetDir, img.Name(), err)
 		}
-		if err := cleanSnapshot(ctx); err != nil && !errdefs.IsNotFound(err) {
-			return fmt.Errorf("unable to cleanup snapshot for image: %s, err: %w", imageID, err)
-		}
-		if err := done(ctx); err != nil {
-			return fmt.Errorf("unable to cancel lease for image: %s, err: %w", imageID, err)
-		}
-
-		return nil
+		return clean(ctx)
 	}, nil
 }

pkg/util/containerd/fake/containerd_util.go

@@ -15,6 +15,7 @@ import (
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/api/types"
 	"github.com/containerd/containerd/containers"
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/oci"
 
 	"github.com/DataDog/datadog-agent/pkg/util/retry"
@@ -47,6 +48,7 @@ type MockedContainerdClient struct {
 	MockCallWithClientContext func(namespace string, f func(context.Context) error) error
 	MockIsSandbox             func(namespace string, ctn containerd.Container) (bool, error)
 	MockMountImage            func(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image, targetDir string) (func(context.Context) error, error)
+	MockMounts                func(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image) ([]mount.Mount, error)
 }
 
 // Close is a mock method
@@ -145,9 +147,7 @@ func (client *MockedContainerdClient) GetEvents() containerd.EventService {
 }
 
 // Spec is a mock method
-//
-//nolint:revive // TODO(CINT) Fix revive linter
-func (client *MockedContainerdClient) Spec(namespace string, ctn containers.Container, maxSize int) (*oci.Spec, error) {
+func (client *MockedContainerdClient) Spec(namespace string, ctn containers.Container, _ int) (*oci.Spec, error) {
 	return client.MockSpec(namespace, ctn)
 }
 
@@ -170,3 +170,8 @@ func (client *MockedContainerdClient) IsSandbox(namespace string, ctn containerd
 func (client *MockedContainerdClient) MountImage(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image, targetDir string) (func(context.Context) error, error) {
 	return client.MockMountImage(ctx, expiration, namespace, img, targetDir)
 }
+
+// Mounts is a mock method
+func (client *MockedContainerdClient) Mounts(ctx context.Context, expiration time.Duration, namespace string, img containerd.Image) ([]mount.Mount, error) {
+	return client.MockMounts(ctx, expiration, namespace, img)
+}

pkg/util/trivy/image.go

@@ -17,7 +17,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/DataDog/datadog-agent/pkg/sbom/telemetry"
 	fimage "github.com/aquasecurity/trivy/pkg/fanal/image"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
@@ -26,6 +25,8 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/tarball"
 	"github.com/samber/lo"
 	"golang.org/x/xerrors"
+
+	"github.com/DataDog/datadog-agent/pkg/sbom/telemetry"
 )
 
 var mu sync.Mutex
@@ -62,7 +63,7 @@ func imageOpener(ctx context.Context, collector, ref string, f *os.File, imageSa
 
 // image is a wrapper for github.com/google/go-containerregistry/pkg/v1/daemon.Image
 // daemon.Image loads the entire image into the memory at first,
-// but it doesn't need to load it if the information is already in the cache,
+// but it doesn't need to load it if the information is already in the persistentCache,
 // To avoid entire loading, this wrapper uses ImageInspectWithRaw and checks image ID and layer IDs.
 type image struct {
 	v1.Image