Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(misconf): Add fallback support for trivy-checks #8062

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat(misconf): Add fallback support for trivy-checks
Signed-off-by: Simar <[email protected]>
simar7 committed Dec 6, 2024
commit d3254ff6d02cc8e5e04841ffdb1b48956f962aa8
2 changes: 1 addition & 1 deletion pkg/commands/artifact/run.go
Original file line number Diff line number Diff line change
@@ -646,7 +646,7 @@ func initMisconfScannerOption(ctx context.Context, opts flag.Options) (misconf.S
var downloadedPolicyPaths []string
var disableEmbedded bool

downloadedPolicyPaths, err := operation.InitBuiltinChecks(ctx, opts.CacheDir, opts.Quiet, opts.SkipCheckUpdate, opts.MisconfOptions.ChecksBundleRepository, opts.RegistryOpts())
downloadedPolicyPaths, err := operation.InitBuiltinChecks(ctx, opts.CacheDir, opts.Quiet, opts.SkipCheckUpdate, opts.MisconfOptions.ChecksBundleRepositories, opts.RegistryOpts())
if err != nil {
if !opts.SkipCheckUpdate {
log.ErrorContext(ctx, "Falling back to embedded checks", log.Err(err))
2 changes: 1 addition & 1 deletion pkg/commands/clean/run.go
Original file line number Diff line number Diff line change
@@ -96,7 +96,7 @@ func cleanJavaDB(ctx context.Context, opts flag.Options) error {

func cleanCheckBundle(opts flag.Options) error {
log.Info("Removing check bundle...")
c, err := policy.NewClient(opts.CacheDir, true, opts.MisconfOptions.ChecksBundleRepository)
c, err := policy.NewClient(opts.CacheDir, true, opts.MisconfOptions.ChecksBundleRepositories)
if err != nil {
return xerrors.Errorf("failed to instantiate check client: %w", err)
}
4 changes: 2 additions & 2 deletions pkg/commands/operation/operation.go
Original file line number Diff line number Diff line change
@@ -78,11 +78,11 @@ func DownloadVEXRepositories(ctx context.Context, opts flag.Options) error {
}

// InitBuiltinChecks downloads the built-in policies and loads them
func InitBuiltinChecks(ctx context.Context, cacheDir string, quiet, skipUpdate bool, checkBundleRepository string, registryOpts ftypes.RegistryOptions) ([]string, error) {
func InitBuiltinChecks(ctx context.Context, cacheDir string, quiet, skipUpdate bool, checkBundleRepositories []string, registryOpts ftypes.RegistryOptions) ([]string, error) {
mu.Lock()
defer mu.Unlock()

client, err := policy.NewClient(cacheDir, quiet, checkBundleRepository)
client, err := policy.NewClient(cacheDir, quiet, checkBundleRepositories)
if err != nil {
return nil, xerrors.Errorf("check client error: %w", err)
}
64 changes: 33 additions & 31 deletions pkg/flag/misconf_flags.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package flag

import (
"fmt"

"github.com/samber/lo"

"github.com/aquasecurity/trivy/pkg/fanal/analyzer"
@@ -82,17 +80,21 @@ var (
ConfigName: "misconfiguration.terraform.exclude-downloaded-modules",
Usage: "exclude misconfigurations for downloaded terraform modules",
}
ChecksBundleRepositoryFlag = Flag[string]{
Name: "checks-bundle-repository",
ConfigName: "misconfiguration.checks-bundle-repository",
Default: fmt.Sprintf("%s:%d", policy.BundleRepository, policy.BundleVersion),
Usage: "OCI registry URL to retrieve checks bundle from",
ChecksBundleRepositoriesFlag = Flag[[]string]{
Name: "checks-bundle-repositories",
ConfigName: "misconfiguration.checks-bundle-repositories",
Default: policy.BundleRepositories,
Usage: "OCI registry URL(s) to retrieve checks bundle from",
Aliases: []Alias{
{
Name: "policy-bundle-repository",
ConfigName: "misconfiguration.policy-bundle-repository",
Deprecated: true,
},
{
Name: "checks-bundle-repository",
ConfigName: "misconfiguration.checks-bundle-repository",
},
},
}
MisconfigScannersFlag = Flag[[]string]{
@@ -112,9 +114,9 @@ var (

// MisconfFlagGroup composes common printer flag structs used for commands providing misconfiguration scanning.
type MisconfFlagGroup struct {
IncludeNonFailures *Flag[bool]
ResetChecksBundle *Flag[bool]
ChecksBundleRepository *Flag[string]
IncludeNonFailures *Flag[bool]
ResetChecksBundle *Flag[bool]
ChecksBundleRepositories *Flag[[]string]

// Values Files
HelmValues *Flag[[]string]
@@ -131,9 +133,9 @@ type MisconfFlagGroup struct {
}

type MisconfOptions struct {
IncludeNonFailures bool
ResetChecksBundle bool
ChecksBundleRepository string
IncludeNonFailures bool
ResetChecksBundle bool
ChecksBundleRepositories []string

// Values Files
HelmValues []string
@@ -151,9 +153,9 @@ type MisconfOptions struct {

func NewMisconfFlagGroup() *MisconfFlagGroup {
return &MisconfFlagGroup{
IncludeNonFailures: IncludeNonFailuresFlag.Clone(),
ResetChecksBundle: ResetChecksBundleFlag.Clone(),
ChecksBundleRepository: ChecksBundleRepositoryFlag.Clone(),
IncludeNonFailures: IncludeNonFailuresFlag.Clone(),
ResetChecksBundle: ResetChecksBundleFlag.Clone(),
ChecksBundleRepositories: ChecksBundleRepositoriesFlag.Clone(),

HelmValues: HelmSetFlag.Clone(),
HelmFileValues: HelmSetFileFlag.Clone(),
@@ -177,7 +179,7 @@ func (f *MisconfFlagGroup) Flags() []Flagger {
return []Flagger{
f.IncludeNonFailures,
f.ResetChecksBundle,
f.ChecksBundleRepository,
f.ChecksBundleRepositories,
f.HelmValues,
f.HelmValueFiles,
f.HelmFileValues,
@@ -198,19 +200,19 @@ func (f *MisconfFlagGroup) ToOptions() (MisconfOptions, error) {
}

return MisconfOptions{
IncludeNonFailures: f.IncludeNonFailures.Value(),
ResetChecksBundle: f.ResetChecksBundle.Value(),
ChecksBundleRepository: f.ChecksBundleRepository.Value(),
HelmValues: f.HelmValues.Value(),
HelmValueFiles: f.HelmValueFiles.Value(),
HelmFileValues: f.HelmFileValues.Value(),
HelmStringValues: f.HelmStringValues.Value(),
HelmAPIVersions: f.HelmAPIVersions.Value(),
HelmKubeVersion: f.HelmKubeVersion.Value(),
TerraformTFVars: f.TerraformTFVars.Value(),
CloudFormationParamVars: f.CloudformationParamVars.Value(),
TfExcludeDownloaded: f.TerraformExcludeDownloaded.Value(),
MisconfigScanners: xstrings.ToTSlice[analyzer.Type](f.MisconfigScanners.Value()),
ConfigFileSchemas: f.ConfigFileSchemas.Value(),
IncludeNonFailures: f.IncludeNonFailures.Value(),
ResetChecksBundle: f.ResetChecksBundle.Value(),
ChecksBundleRepositories: f.ChecksBundleRepositories.Value(),
HelmValues: f.HelmValues.Value(),
HelmValueFiles: f.HelmValueFiles.Value(),
HelmFileValues: f.HelmFileValues.Value(),
HelmStringValues: f.HelmStringValues.Value(),
HelmAPIVersions: f.HelmAPIVersions.Value(),
HelmKubeVersion: f.HelmKubeVersion.Value(),
TerraformTFVars: f.TerraformTFVars.Value(),
CloudFormationParamVars: f.CloudformationParamVars.Value(),
TfExcludeDownloaded: f.TerraformExcludeDownloaded.Value(),
MisconfigScanners: xstrings.ToTSlice[analyzer.Type](f.MisconfigScanners.Value()),
ConfigFileSchemas: f.ConfigFileSchemas.Value(),
}, nil
}
2 changes: 1 addition & 1 deletion pkg/k8s/commands/cluster.go
Original file line number Diff line number Diff line change
@@ -69,7 +69,7 @@ func nodeCollectorOptions(ctx context.Context, opts flag.Options) []trivyk8s.Nod

ctx = log.WithContextPrefix(ctx, log.PrefixMisconfiguration)
contentPath, err := operation.InitBuiltinChecks(ctx, opts.CacheDir, opts.Quiet, opts.SkipCheckUpdate,
opts.MisconfOptions.ChecksBundleRepository, opts.RegistryOpts())
opts.MisconfOptions.ChecksBundleRepositories, opts.RegistryOpts())
if err != nil {
log.Error("Falling back to embedded checks", log.Err(err))
nodeCollectorOptions = append(nodeCollectorOptions,
109 changes: 70 additions & 39 deletions pkg/policy/policy.go
Original file line number Diff line number Diff line change
@@ -18,10 +18,16 @@ import (
)

const (
BundleVersion = 1 // Latest released MAJOR version for trivy-checks
BundleRepository = "mirror.gcr.io/aquasec/trivy-checks"
policyMediaType = "application/vnd.cncf.openpolicyagent.layer.v1.tar+gzip"
updateInterval = 24 * time.Hour
BundleVersion = 1 // Latest released MAJOR version for trivy-checks
policyMediaType = "application/vnd.cncf.openpolicyagent.layer.v1.tar+gzip"
updateInterval = 24 * time.Hour
)

var (
BundleRepositories = []string{
fmt.Sprintf("%s:%d", "mirror.gcr.io/aquasec/trivy-checks", BundleVersion), // primary
fmt.Sprintf("%s:%d", "ghcr.io/aquasecurity/trivy-checks", BundleVersion), // secondary
}
)

type options struct {
@@ -49,9 +55,9 @@ type Option func(*options)
// Client implements check operations
type Client struct {
*options
policyDir string
checkBundleRepo string
quiet bool
policyDir string
checkBundleRepos []string
quiet bool
}

// Metadata holds default check metadata
@@ -68,7 +74,7 @@ func (m Metadata) String() string {
}

// NewClient is the factory method for check client
func NewClient(cacheDir string, quiet bool, checkBundleRepo string, opts ...Option) (*Client, error) {
func NewClient(cacheDir string, quiet bool, checkBundleRepos []string, opts ...Option) (*Client, error) {
o := &options{
clock: clock.RealClock{},
}
@@ -77,47 +83,72 @@ func NewClient(cacheDir string, quiet bool, checkBundleRepo string, opts ...Opti
opt(o)
}

if checkBundleRepo == "" {
checkBundleRepo = fmt.Sprintf("%s:%d", BundleRepository, BundleVersion)
if len(checkBundleRepos) == 0 {
checkBundleRepos = BundleRepositories
}

return &Client{
options: o,
policyDir: filepath.Join(cacheDir, "policy"),
checkBundleRepo: checkBundleRepo,
quiet: quiet,
options: o,
policyDir: filepath.Join(cacheDir, "policy"),
checkBundleRepos: checkBundleRepos,
quiet: quiet,
}, nil
}

func (c *Client) populateOCIArtifact(ctx context.Context, registryOpts types.RegistryOptions) {
func (c *Client) populateOCIArtifact(ctx context.Context, repo string, registryOpts types.RegistryOptions) {
if c.artifact == nil {
log.DebugContext(ctx, "Loading check bundle", log.String("repository", c.checkBundleRepo))
c.artifact = oci.NewArtifact(c.checkBundleRepo, registryOpts)
if repo == "" {
repo = c.checkBundleRepos[0]
}
log.DebugContext(ctx, "Loading check bundle", log.String("repo", repo))
c.artifact = oci.NewArtifact(repo, registryOpts)
}
}

// DownloadBuiltinChecks download default policies from GitHub Pages
// DownloadBuiltinChecks download default policies from OCI registry
func (c *Client) DownloadBuiltinChecks(ctx context.Context, registryOpts types.RegistryOptions) error {
c.populateOCIArtifact(ctx, registryOpts)

dst := c.contentDir()
if err := c.artifact.Download(ctx, dst, oci.DownloadOption{
MediaType: policyMediaType,
Quiet: c.quiet,
},
); err != nil {
return xerrors.Errorf("download error: %w", err)
}

digest, err := c.artifact.Digest(ctx)
if err != nil {
return xerrors.Errorf("digest error: %w", err)
}
log.DebugContext(ctx, "Digest of the built-in checks", log.String("digest", digest))

// Update metadata.json with the new digest and the current date
if err = c.updateMetadata(digest, c.clock.Now()); err != nil {
return xerrors.Errorf("unable to update the check metadata: %w", err)
oldart := c.artifact
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why fall back to a previous artifact?


for i, repo := range c.checkBundleRepos {
c.populateOCIArtifact(ctx, repo, registryOpts)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work because the artifact is only initialized if it is missing, so the old one will be used. https://github.com/aquasecurity/trivy/blob/main/pkg/policy/policy.go#L92


dst := c.contentDir()
if err := c.artifact.Download(ctx, dst, oci.DownloadOption{
MediaType: policyMediaType,
Quiet: c.quiet,
},
); err != nil {
if i == len(c.checkBundleRepos)-1 {
return xerrors.Errorf("download error: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we accumulate all the errors and in case the download fails from any source return it?

}
log.ErrorContext(ctx, "Failed to download checks bundle", log.String("repo", repo), log.Err(err))
c.artifact = oldart
continue
}

digest, err := c.artifact.Digest(ctx)
if err != nil {
if i == len(c.checkBundleRepos)-1 {
return xerrors.Errorf("digest error: %w", err)
}
log.ErrorContext(ctx, "Failed to get digest for check bundle", log.String("repo", repo), log.Err(err))
c.artifact = oldart
continue
}
log.DebugContext(ctx, "Digest of the built-in checks", log.String("digest", digest))

// Update metadata.json with the new digest and the current date
if err = c.updateMetadata(digest, c.clock.Now()); err != nil {
if i == len(c.checkBundleRepos)-1 {
return xerrors.Errorf("unable to update the check metadata: %w", err)
}
log.ErrorContext(ctx, "Failed to update metadata", log.String("digest", digest), log.Err(err))
c.artifact = oldart
continue
}
Comment on lines +115 to +148
Copy link
Contributor

@nikpivkin nikpivkin Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifact download can be extracted into a separate function to avoid multiple conditions:

func (c *Client) downloadArtifact(ctx context.Context) error {
	dst := c.contentDir()
	if err := c.artifact.Download(ctx, dst, oci.DownloadOption{
		MediaType: policyMediaType,
		Quiet:     c.quiet,
	},
	); err != nil {
		return xerrors.Errorf("download error: %w", err)
	}

	digest, err := c.artifact.Digest(ctx)
	if err != nil {
		return xerrors.Errorf("digest error: %w", err)
	}
	log.DebugContext(ctx, "Digest of the built-in checks", log.String("digest", digest))

	if err = c.updateMetadata(digest, c.clock.Now()); err != nil {
		return xerrors.Errorf("unable to update the check metadata: %w", err)
	}

	log.DebugContext(ctx, "Successfully loaded checks bundle", log.String("digest", digest))
	return nil
}


log.DebugContext(ctx, "Successfully loaded check bundle", log.String("repo", repo), log.String("digest", digest))
break
}

return nil
@@ -162,7 +193,7 @@ func (c *Client) NeedsUpdate(ctx context.Context, registryOpts types.RegistryOpt
return false, nil
}

c.populateOCIArtifact(ctx, registryOpts)
c.populateOCIArtifact(ctx, "", registryOpts)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The artifact for the default repository will be initialized here, even if the user has overridden the repositories.

digest, err := c.artifact.Digest(ctx)
if err != nil {
return false, xerrors.Errorf("digest error: %w", err)
58 changes: 48 additions & 10 deletions pkg/policy/policy_test.go
Original file line number Diff line number Diff line change
@@ -117,7 +117,7 @@ func TestClient_LoadBuiltinPolicies(t *testing.T) {

// Mock OCI artifact
art := oci.NewArtifact("repo", ftypes.RegistryOptions{}, oci.WithImage(img))
c, err := policy.NewClient(tt.cacheDir, true, "", policy.WithOCIArtifact(art))
c, err := policy.NewClient(tt.cacheDir, true, nil, policy.WithOCIArtifact(art))
require.NoError(t, err)

got, err := c.LoadBuiltinChecks()
@@ -256,7 +256,7 @@ func TestClient_NeedsUpdate(t *testing.T) {
}

art := oci.NewArtifact("repo", ftypes.RegistryOptions{}, oci.WithImage(img))
c, err := policy.NewClient(tmpDir, true, "", policy.WithOCIArtifact(art), policy.WithClock(tt.clock))
c, err := policy.NewClient(tmpDir, true, nil, policy.WithOCIArtifact(art), policy.WithClock(tt.clock))
require.NoError(t, err)

// Assert results
@@ -272,24 +272,31 @@ func TestClient_DownloadBuiltinPolicies(t *testing.T) {
h v1.Hash
err error
}
type digestReturnsOnCall map[int]struct {
h v1.Hash
err error
}
type layersReturns struct {
layers []v1.Layer
err error
}
tests := []struct {
name string
clock clock.Clock
layersReturns layersReturns
digestReturns digestReturns
want *policy.Metadata
wantErr string
name string
clock clock.Clock
repos []string
layersReturns layersReturns
digestReturns digestReturns
digestReturnsOnCall digestReturnsOnCall
want *policy.Metadata
wantErr string
}{
{
name: "happy path",
clock: fake.NewFakeClock(time.Date(2021, 1, 1, 1, 0, 0, 0, time.UTC)),
layersReturns: layersReturns{
layers: []v1.Layer{newFakeLayer(t)},
},
repos: []string{"repo0"},
digestReturns: digestReturns{
h: v1.Hash{
Algorithm: "sha256",
@@ -301,6 +308,31 @@ func TestClient_DownloadBuiltinPolicies(t *testing.T) {
DownloadedAt: time.Date(2021, 1, 1, 1, 0, 0, 0, time.UTC),
},
},
{
name: "mixed path, first repo fails, second succeeds",
clock: fake.NewFakeClock(time.Date(2021, 1, 1, 1, 0, 0, 0, time.UTC)),
layersReturns: layersReturns{
layers: []v1.Layer{newFakeLayer(t)},
},
repos: []string{"repo0", "repo1"},
digestReturnsOnCall: digestReturnsOnCall{
0: struct {
h v1.Hash
err error
}{err: fmt.Errorf("error")},
1: struct {
h v1.Hash
err error
}{h: v1.Hash{
Algorithm: "sha256",
Hex: "01e033e78bd8a59fa4f4577215e7da06c05e1152526094d8d79d2aa06e98cb9d",
}},
},
want: &policy.Metadata{
Digest: "sha256:01e033e78bd8a59fa4f4577215e7da06c05e1152526094d8d79d2aa06e98cb9d",
DownloadedAt: time.Date(2021, 1, 1, 1, 0, 0, 0, time.UTC),
},
},
{
name: "sad: broken layer",
clock: fake.NewFakeClock(time.Date(2021, 1, 1, 1, 0, 0, 0, time.UTC)),
@@ -340,6 +372,12 @@ func TestClient_DownloadBuiltinPolicies(t *testing.T) {
img := new(fakei.FakeImage)
img.DigestReturns(tt.digestReturns.h, tt.digestReturns.err)
img.LayersReturns(tt.layersReturns.layers, tt.layersReturns.err)

if len(tt.digestReturnsOnCall) > 0 {
img.DigestReturnsOnCall(0, tt.digestReturnsOnCall[0].h, tt.digestReturnsOnCall[0].err)
img.DigestReturnsOnCall(1, tt.digestReturnsOnCall[1].h, tt.digestReturnsOnCall[1].err)
}

img.ManifestReturns(&v1.Manifest{
Layers: []v1.Descriptor{
{
@@ -358,7 +396,7 @@ func TestClient_DownloadBuiltinPolicies(t *testing.T) {

// Mock OCI artifact
art := oci.NewArtifact("repo", ftypes.RegistryOptions{}, oci.WithImage(img))
c, err := policy.NewClient(tempDir, true, "", policy.WithClock(tt.clock), policy.WithOCIArtifact(art))
c, err := policy.NewClient(tempDir, true, tt.repos, policy.WithClock(tt.clock), policy.WithOCIArtifact(art))
require.NoError(t, err)

err = c.DownloadBuiltinChecks(context.Background(), ftypes.RegistryOptions{})
@@ -388,7 +426,7 @@ func TestClient_Clear(t *testing.T) {
err := os.MkdirAll(filepath.Join(cacheDir, "policy"), 0755)
require.NoError(t, err)

c, err := policy.NewClient(cacheDir, true, "")
c, err := policy.NewClient(cacheDir, true, nil)
require.NoError(t, err)
require.NoError(t, c.Clear())
}
2 changes: 1 addition & 1 deletion pkg/version/version.go
Original file line number Diff line number Diff line change
@@ -76,7 +76,7 @@ func NewVersionInfo(cacheDir string) VersionInfo {
}

var pbMeta *policy.Metadata
pc, err := policy.NewClient(cacheDir, false, "")
pc, err := policy.NewClient(cacheDir, false, nil)
if err != nil {
log.Debug("Failed to instantiate policy client", log.Err(err))
}