From 55fb723a6e00cc36d89a548ab0c7b31544aa6044 Mon Sep 17 00:00:00 2001 From: Teppei Fukuda Date: Mon, 8 May 2023 21:04:22 +0300 Subject: [PATCH] feat(image): enforce image platform (#4083) --- pkg/commands/artifact/run.go | 10 ++- pkg/fanal/artifact/artifact.go | 2 - pkg/fanal/image/image_test.go | 10 ++- pkg/fanal/types/image.go | 10 ++- pkg/flag/image_flags.go | 19 ++++- pkg/remote/remote.go | 123 ++++++++++++++++++++------------- pkg/remote/remote_test.go | 31 ++++++++- pkg/types/scanoptions.go | 1 - 8 files changed, 142 insertions(+), 64 deletions(-) diff --git a/pkg/commands/artifact/run.go b/pkg/commands/artifact/run.go index f45a25ae59..cebd9cbacc 100644 --- a/pkg/commands/artifact/run.go +++ b/pkg/commands/artifact/run.go @@ -550,7 +550,6 @@ func initScannerConfig(opts flag.Options, cacheClient cache.Cache) (ScannerConfi Scanners: opts.Scanners, ImageConfigScanners: opts.ImageConfigScanners, // this is valid only for 'image' subcommand ScanRemovedPackages: opts.ScanRemovedPkgs, // this is valid only for 'image' subcommand - Platform: opts.Platform, // this is valid only for 'image' subcommand ListAllPackages: opts.ListAllPkgs, LicenseCategories: opts.LicenseCategories, FilePatterns: opts.FilePatterns, @@ -642,11 +641,10 @@ func initScannerConfig(opts flag.Options, cacheClient cache.Cache) (ScannerConfi RepoTag: opts.RepoTag, SBOMSources: opts.SBOMSources, RekorURL: opts.RekorURL, - Platform: opts.Platform, - DockerHost: opts.DockerHost, - Slow: opts.Slow, - AWSRegion: opts.Region, - FileChecksum: fileChecksum, + //Platform: opts.Platform, + Slow: opts.Slow, + AWSRegion: opts.Region, + FileChecksum: fileChecksum, // For image scanning ImageOption: ftypes.ImageOptions{ diff --git a/pkg/fanal/artifact/artifact.go b/pkg/fanal/artifact/artifact.go index c4c7b95d13..e4ed5a833b 100644 --- a/pkg/fanal/artifact/artifact.go +++ b/pkg/fanal/artifact/artifact.go @@ -23,8 +23,6 @@ type Option struct { AppDirs []string SBOMSources []string RekorURL string - Platform string - DockerHost string Slow bool // Lower CPU and memory AWSRegion string FileChecksum bool // For SPDX diff --git a/pkg/fanal/image/image_test.go b/pkg/fanal/image/image_test.go index 6005354c4e..24342ce95c 100644 --- a/pkg/fanal/image/image_test.go +++ b/pkg/fanal/image/image_test.go @@ -529,7 +529,12 @@ func TestDockerPlatformArguments(t *testing.T) { }, }, Insecure: true, - Platform: "arm/linux", + Platform: types.Platform{ + Platform: &v1.Platform{ + Architecture: "arm", + OS: "linux", + }, + }, }, }, }, @@ -543,8 +548,7 @@ func TestDockerPlatformArguments(t *testing.T) { defer cleanup() if tt.wantErr != "" { - assert.NotNil(t, err) - assert.Contains(t, err.Error(), tt.wantErr, err) + assert.ErrorContains(t, err, tt.wantErr, err) } else { assert.NoError(t, err) } diff --git a/pkg/fanal/types/image.go b/pkg/fanal/types/image.go index 54ba7ad35d..e3c970ae18 100644 --- a/pkg/fanal/types/image.go +++ b/pkg/fanal/types/image.go @@ -2,6 +2,14 @@ package types import v1 "github.com/google/go-containerregistry/pkg/v1" +type Platform struct { + *v1.Platform + + // Force returns an error if the specified platform is not found. + // This option is for Aqua, and cannot be configured via Trivy CLI. + Force bool +} + type Image interface { v1.Image ImageExtension @@ -44,7 +52,7 @@ type RegistryOptions struct { Insecure bool // Architecture - Platform string + Platform Platform // ECR AWSAccessKey string diff --git a/pkg/flag/image_flags.go b/pkg/flag/image_flags.go index 805449c7db..4e6a6f9d22 100644 --- a/pkg/flag/image_flags.go +++ b/pkg/flag/image_flags.go @@ -1,8 +1,10 @@ package flag import ( + v1 "github.com/google/go-containerregistry/pkg/v1" "golang.org/x/xerrors" + ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" "github.com/aquasecurity/trivy/pkg/types" ) @@ -56,7 +58,7 @@ type ImageOptions struct { Input string ImageConfigScanners types.Scanners ScanRemovedPkgs bool - Platform string + Platform ftypes.Platform DockerHost string } @@ -89,11 +91,24 @@ func (f *ImageFlagGroup) ToOptions() (ImageOptions, error) { if err != nil { return ImageOptions{}, xerrors.Errorf("unable to parse image config scanners: %w", err) } + + var platform ftypes.Platform + if p := getString(f.Platform); p != "" { + pl, err := v1.ParsePlatform(p) + if err != nil { + return ImageOptions{}, xerrors.Errorf("unable to parse platform: %w", err) + } + if pl.OS == "*" { + pl.OS = "" // Empty OS means any OS + } + platform = ftypes.Platform{Platform: pl} + } + return ImageOptions{ Input: getString(f.Input), ImageConfigScanners: scanners, ScanRemovedPkgs: getBool(f.ScanRemovedPkgs), - Platform: getString(f.Platform), + Platform: platform, DockerHost: getString(f.DockerHost), }, nil } diff --git a/pkg/remote/remote.go b/pkg/remote/remote.go index dd67e29903..f5c2ad7099 100644 --- a/pkg/remote/remote.go +++ b/pkg/remote/remote.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "net" "net/http" - "strings" "time" "github.com/google/go-containerregistry/pkg/authn" @@ -37,14 +36,14 @@ func Get(ctx context.Context, ref name.Reference, option types.RegistryOptions) authOpt, } - if option.Platform != "" { - s, err := parsePlatform(ref, option.Platform, remoteOpts) + if option.Platform.Platform != nil { + p, err := resolvePlatform(ref, option.Platform, remoteOpts) if err != nil { return nil, xerrors.Errorf("platform error: %w", err) } // Don't pass platform when the specified image is single-arch. - if s != nil { - remoteOpts = append(remoteOpts, remote.WithPlatform(*s)) + if p.Platform != nil { + remoteOpts = append(remoteOpts, remote.WithPlatform(*p.Platform)) } } @@ -54,6 +53,11 @@ func Get(ctx context.Context, ref name.Reference, option types.RegistryOptions) continue } + if option.Platform.Force { + if err = satisfyPlatform(desc, lo.FromPtr(option.Platform.Platform)); err != nil { + return nil, err + } + } return desc, nil } @@ -113,12 +117,11 @@ func httpTransport(insecure bool) *http.Transport { d := &net.Dialer{ Timeout: 10 * time.Minute, } - return &http.Transport{ - Proxy: http.ProxyFromEnvironment, - DisableKeepAlives: true, - DialContext: d.DialContext, - TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}, - } + tr := http.DefaultTransport.(*http.Transport).Clone() + tr.DialContext = d.DialContext + tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: insecure} + + return tr } func authOptions(ctx context.Context, ref name.Reference, option types.RegistryOptions) []remote.Option { @@ -147,45 +150,69 @@ func authOptions(ctx context.Context, ref name.Reference, option types.RegistryO } } -func parsePlatform(ref name.Reference, p string, options []remote.Option) (*v1.Platform, error) { +// resolvePlatform resolves the OS platform for a given image reference. +// If the platform has an empty OS, the function will attempt to find the first OS +// in the image's manifest list and return the platform with the detected OS. +// It ignores the specified platform if the image is not multi-arch. +func resolvePlatform(ref name.Reference, p types.Platform, options []remote.Option) (types.Platform, error) { + if p.OS != "" { + return p, nil + } + // OS wildcard, implicitly pick up the first os found in the image list. // e.g. */amd64 - if strings.HasPrefix(p, "*/") { - d, err := remote.Get(ref, options...) - if err != nil { - return nil, xerrors.Errorf("image get error: %w", err) - } - switch d.MediaType { - case v1types.OCIManifestSchema1, v1types.DockerManifestSchema2: - // We want an index but the registry has an image, not multi-arch. We just ignore "--platform". - log.Logger.Debug("Ignore --platform as the image is not multi-arch") - return nil, nil - case v1types.OCIImageIndex, v1types.DockerManifestList: - // These are expected. - } - - index, err := d.ImageIndex() - if err != nil { - return nil, xerrors.Errorf("image index error: %w", err) - } - - m, err := index.IndexManifest() - if err != nil { - return nil, xerrors.Errorf("remote index manifest error: %w", err) - } - if len(m.Manifests) == 0 { - log.Logger.Debug("Ignore --platform as the image is not multi-arch") - return nil, nil - } - if m.Manifests[0].Platform != nil { - // Replace with the detected OS - // e.g. */amd64 => linux/amd64 - p = m.Manifests[0].Platform.OS + strings.TrimPrefix(p, "*") - } - } - platform, err := v1.ParsePlatform(p) + d, err := remote.Get(ref, options...) if err != nil { - return nil, xerrors.Errorf("platform parse error: %w", err) + return types.Platform{}, xerrors.Errorf("image get error: %w", err) } - return platform, nil + switch d.MediaType { + case v1types.OCIManifestSchema1, v1types.DockerManifestSchema2: + // We want an index but the registry has an image, not multi-arch. We just ignore "--platform". + log.Logger.Debug("Ignore --platform as the image is not multi-arch") + return types.Platform{}, nil + case v1types.OCIImageIndex, v1types.DockerManifestList: + // These are expected. + } + + index, err := d.ImageIndex() + if err != nil { + return types.Platform{}, xerrors.Errorf("image index error: %w", err) + } + + m, err := index.IndexManifest() + if err != nil { + return types.Platform{}, xerrors.Errorf("remote index manifest error: %w", err) + } + if len(m.Manifests) == 0 { + log.Logger.Debug("Ignore '--platform' as the image is not multi-arch") + return types.Platform{}, nil + } + if m.Manifests[0].Platform != nil { + newPlatform := p.DeepCopy() + // Replace with the detected OS + // e.g. */amd64 => linux/amd64 + newPlatform.OS = m.Manifests[0].Platform.OS + + // Return the platform with the found OS + return types.Platform{ + Platform: newPlatform, + Force: p.Force, + }, nil + } + return types.Platform{}, nil +} + +func satisfyPlatform(desc *remote.Descriptor, platform v1.Platform) error { + img, err := desc.Image() + if err != nil { + return err + } + c, err := img.ConfigFile() + if err != nil { + return err + } + if !lo.FromPtr(c.Platform()).Satisfies(platform) { + return xerrors.Errorf("the specified platform not found") + } + return nil } diff --git a/pkg/remote/remote_test.go b/pkg/remote/remote_test.go index 0421c74b47..024d1cacd8 100644 --- a/pkg/remote/remote_test.go +++ b/pkg/remote/remote_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "fmt" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "net/http/httptest" @@ -126,10 +127,38 @@ func TestGet(t *testing.T) { }, }, Insecure: true, - Platform: "*/amd64", + Platform: types.Platform{ + Platform: &v1.Platform{ + OS: "", + Architecture: "amd64", + }, + }, }, }, }, + { + name: "force platform", + args: args{ + imageName: fmt.Sprintf("%s/library/alpine:3.10", serverAddr), + option: types.RegistryOptions{ + Credentials: []types.Credential{ + { + Username: "test", + Password: "testpass", + }, + }, + Insecure: true, + Platform: types.Platform{ + Force: true, + Platform: &v1.Platform{ + OS: "windows", + Architecture: "amd64", + }, + }, + }, + }, + wantErr: "the specified platform not found", + }, { name: "bad credential", args: args{ diff --git a/pkg/types/scanoptions.go b/pkg/types/scanoptions.go index de42889a8c..0d730baaef 100644 --- a/pkg/types/scanoptions.go +++ b/pkg/types/scanoptions.go @@ -10,7 +10,6 @@ type ScanOptions struct { Scanners Scanners ImageConfigScanners Scanners // Scanners for container image configuration ScanRemovedPackages bool - Platform string ListAllPackages bool LicenseCategories map[types.LicenseCategory][]string FilePatterns []string