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): loading embedded checks as a fallback #6502

Merged
merged 5 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
3 changes: 2 additions & 1 deletion pkg/cloud/report/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go-v2/aws/arn"

ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
"github.com/aquasecurity/trivy/pkg/iac/rego"
"github.com/aquasecurity/trivy/pkg/iac/scan"
"github.com/aquasecurity/trivy/pkg/types"
)
Expand Down Expand Up @@ -57,7 +58,7 @@ func ConvertResults(results scan.Results, provider string, scoped []string) map[

// empty namespace implies a go rule from defsec, "builtin" refers to a built-in rego rule
// this ensures we don't generate bad links for custom policies
if result.RegoNamespace() == "" || strings.HasPrefix(result.RegoNamespace(), "builtin.") {
if result.RegoNamespace() == "" || rego.IsBuiltinNamespace(result.RegoNamespace()) {
primaryURL = fmt.Sprintf("https://avd.aquasec.com/misconfig/%s", strings.ToLower(result.Rule().AVDID))
}

Expand Down
8 changes: 2 additions & 6 deletions pkg/commands/artifact/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/aquasecurity/trivy/pkg/fanal/cache"
ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
"github.com/aquasecurity/trivy/pkg/flag"
"github.com/aquasecurity/trivy/pkg/iac/rego"
"github.com/aquasecurity/trivy/pkg/javadb"
"github.com/aquasecurity/trivy/pkg/log"
"github.com/aquasecurity/trivy/pkg/misconf"
Expand Down Expand Up @@ -49,11 +50,6 @@ const (
)

var (
defaultPolicyNamespaces = []string{
"appshield",
"defsec",
"builtin",
}
SkipScan = errors.New("skip subsequent processes")
)

Expand Down Expand Up @@ -597,7 +593,7 @@ func initScannerConfig(opts flag.Options, cacheClient cache.Cache) (ScannerConfi
configScannerOptions = misconf.ScannerOption{
Debug: opts.Debug,
Trace: opts.Trace,
Namespaces: append(opts.PolicyNamespaces, defaultPolicyNamespaces...),
Namespaces: append(opts.PolicyNamespaces, rego.BuiltinNamespaces()...),
PolicyPaths: append(opts.PolicyPaths, downloadedPolicyPaths...),
DataPaths: opts.DataPaths,
HelmValues: opts.HelmValues,
Expand Down
6 changes: 3 additions & 3 deletions pkg/iac/rego/embed.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

"github.com/open-policy-agent/opa/ast"

rules2 "github.com/aquasecurity/trivy-policies"
checks "github.com/aquasecurity/trivy-policies"
"github.com/aquasecurity/trivy/pkg/iac/rules"
)

Expand Down Expand Up @@ -62,11 +62,11 @@ func RegisterRegoRules(modules map[string]*ast.Module) {
}

func LoadEmbeddedPolicies() (map[string]*ast.Module, error) {
return LoadPoliciesFromDirs(rules2.EmbeddedPolicyFileSystem, ".")
return LoadPoliciesFromDirs(checks.EmbeddedPolicyFileSystem, ".")
}

func LoadEmbeddedLibraries() (map[string]*ast.Module, error) {
return LoadPoliciesFromDirs(rules2.EmbeddedLibraryFileSystem, ".")
return LoadPoliciesFromDirs(checks.EmbeddedLibraryFileSystem, ".")
}

func LoadPoliciesFromDirs(target fs.FS, paths ...string) (map[string]*ast.Module, error) {
Expand Down
113 changes: 93 additions & 20 deletions pkg/iac/rego/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,30 @@ import (
"fmt"
"io"
"io/fs"
"path/filepath"
"strings"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
"github.com/samber/lo"
)

var builtinNamespaces = map[string]struct{}{
"builtin": {},
"defsec": {},
"appshield": {},
}

func BuiltinNamespaces() []string {
return lo.Keys(builtinNamespaces)
}

func IsBuiltinNamespace(namespace string) bool {
return lo.ContainsBy(BuiltinNamespaces(), func(ns string) bool {
return strings.HasPrefix(namespace, ns+".")
})
}

func IsRegoFile(name string) bool {
return strings.HasSuffix(name, bundle.RegoExt) && !strings.HasSuffix(name, "_test"+bundle.RegoExt)
}
Expand Down Expand Up @@ -38,28 +56,20 @@ func (s *Scanner) loadPoliciesFromReaders(readers []io.Reader) (map[string]*ast.
return modules, nil
}

func (s *Scanner) loadEmbedded(enableEmbeddedLibraries, enableEmbeddedPolicies bool) error {
if enableEmbeddedLibraries {
loadedLibs, errLoad := LoadEmbeddedLibraries()
if errLoad != nil {
return fmt.Errorf("failed to load embedded rego libraries: %w", errLoad)
}
for name, policy := range loadedLibs {
s.policies[name] = policy
}
s.debug.Log("Loaded %d embedded libraries.", len(loadedLibs))
func (s *Scanner) loadEmbedded() error {
loaded, err := LoadEmbeddedLibraries()
if err != nil {
return fmt.Errorf("failed to load embedded rego libraries: %w", err)
}
s.embeddedLibs = loaded
s.debug.Log("Loaded %d embedded libraries.", len(loaded))

if enableEmbeddedPolicies {
loaded, err := LoadEmbeddedPolicies()
if err != nil {
return fmt.Errorf("failed to load embedded rego policies: %w", err)
}
for name, policy := range loaded {
s.policies[name] = policy
}
s.debug.Log("Loaded %d embedded policies.", len(loaded))
loaded, err = LoadEmbeddedPolicies()
if err != nil {
return fmt.Errorf("failed to load embedded rego policies: %w", err)
}
s.embeddedChecks = loaded
s.debug.Log("Loaded %d embedded policies.", len(loaded))

return nil
}
Expand All @@ -75,10 +85,18 @@ func (s *Scanner) LoadPolicies(enableEmbeddedLibraries, enableEmbeddedPolicies b
srcFS = s.policyFS
}

if err := s.loadEmbedded(enableEmbeddedLibraries, enableEmbeddedPolicies); err != nil {
if err := s.loadEmbedded(); err != nil {
return err
}

if enableEmbeddedPolicies {
s.policies = lo.Assign(s.policies, s.embeddedChecks)
}

if enableEmbeddedLibraries {
s.policies = lo.Assign(s.policies, s.embeddedLibs)
}

var err error
if len(paths) > 0 {
loaded, err := LoadPoliciesFromDirs(srcFS, paths...)
Expand Down Expand Up @@ -127,6 +145,60 @@ func (s *Scanner) LoadPolicies(enableEmbeddedLibraries, enableEmbeddedPolicies b
return s.compilePolicies(srcFS, paths)
}

func (s *Scanner) fallbackChecks(compiler *ast.Compiler) {

var excludedFiles []string

for _, e := range compiler.Errors {
if _, ok := e.Details.(*ast.RefErrInvalidDetail); !ok {
continue
}

loc := e.Location.File
Copy link
Contributor

Choose a reason for hiding this comment

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

@nikpivkin @simar7 I have bump trivy-operator with latest trivy 0.51.1 and I'm getting nil pointer exception on L152 while running my policies tests
loc := e.Location.File anyway to protect it?

here is link for test failure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Location can be nil. I'll add a check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chen-keinan Opened the PR with a fix. #6638


if lo.Contains(excludedFiles, loc) {
continue
}

badPolicy, exists := s.policies[loc]
if !exists || badPolicy == nil {
continue
}

if !IsBuiltinNamespace(getModuleNamespace(badPolicy)) {
continue
}

s.debug.Log("Error occurred while parsing: %s, %s. \nTry loading embedded policy.", loc, e.Error())
nikpivkin marked this conversation as resolved.
Show resolved Hide resolved

embedded := s.findMatchedEmbeddedCheck(badPolicy)
if embedded == nil {
s.debug.Log("Failed to find embedded policy: %s", loc)
continue
}

s.debug.Log("Found embedded policy: %s", embedded.Package.Location.File)
nikpivkin marked this conversation as resolved.
Show resolved Hide resolved
delete(s.policies, loc) // remove bad policy
s.policies[embedded.Package.Location.File] = embedded
delete(s.embeddedChecks, embedded.Package.Location.File) // avoid infinite loop if embedded check contains ref error
excludedFiles = append(excludedFiles, e.Location.File)
}

compiler.Errors = lo.Filter(compiler.Errors, func(e *ast.Error, _ int) bool {
return !lo.Contains(excludedFiles, e.Location.File)
})
}

func (s *Scanner) findMatchedEmbeddedCheck(module *ast.Module) *ast.Module {
for _, policy := range s.embeddedChecks {
if policy.Package.Path.String() == module.Package.Path.String() ||
filepath.Base(policy.Package.Location.File) == filepath.Base(module.Package.Location.File) {
return policy
}
}
return nil
}

func (s *Scanner) prunePoliciesWithError(compiler *ast.Compiler) error {
if len(compiler.Errors) > s.regoErrorLimit {
s.debug.Log("Error(s) occurred while loading policies")
Expand Down Expand Up @@ -157,6 +229,7 @@ func (s *Scanner) compilePolicies(srcFS fs.FS, paths []string) error {

compiler.Compile(s.policies)
if compiler.Failed() {
s.fallbackChecks(compiler)
if err := s.prunePoliciesWithError(compiler); err != nil {
return err
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/iac/rego/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"testing/fstest"

trivy_policies "github.com/aquasecurity/trivy-policies"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand All @@ -19,6 +20,9 @@ import (
//go:embed all:testdata/policies
var testEmbedFS embed.FS

//go:embed testdata/embedded
var embeddedPoliciesFS embed.FS

func Test_RegoScanning_WithSomeInvalidPolicies(t *testing.T) {
t.Run("allow no errors", func(t *testing.T) {
var debugBuf bytes.Buffer
Expand Down Expand Up @@ -95,3 +99,37 @@ deny {
})

}

func Test_FallbackToEmbedded(t *testing.T) {
scanner := rego.NewScanner(
types.SourceDockerfile,
options.ScannerWithRegoErrorLimits(0),
)
fsys := fstest.MapFS{
"policies/my-policy2.rego": &fstest.MapFile{
Data: []byte(`# METADATA
# schemas:
# - input: schema["fooschema"]

package builtin.test

deny {
input.evil == "foo bar"
}`),
},
"schemas/fooschema.json": &fstest.MapFile{
Data: []byte(`{
"$schema": "http://json-schema.org/draft-07/schema#",
"type": "object",
"properties": {
"foo": {
"type": "string"
}
}
}`),
},
}
trivy_policies.EmbeddedPolicyFileSystem = embeddedPoliciesFS
err := scanner.LoadPolicies(false, false, fsys, []string{"."}, nil)
assert.NoError(t, err)
}
15 changes: 9 additions & 6 deletions pkg/iac/rego/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"io/fs"
"maps"
"strings"

"github.com/open-policy-agent/opa/ast"
Expand Down Expand Up @@ -41,6 +42,9 @@ type Scanner struct {
spec string
inputSchema interface{} // unmarshalled into this from a json schema document
sourceType types.Source

embeddedLibs map[string]*ast.Module
embeddedChecks map[string]*ast.Module
}

func (s *Scanner) SetUseEmbeddedLibraries(b bool) {
Expand Down Expand Up @@ -135,13 +139,12 @@ func NewScanner(source types.Source, opts ...options.ScannerOption) *Scanner {
s := &Scanner{
regoErrorLimit: ast.CompileErrorLimitDefault,
sourceType: source,
ruleNamespaces: map[string]struct{}{
"builtin": {},
"appshield": {},
"defsec": {},
},
runtimeValues: addRuntimeValues(),
ruleNamespaces: make(map[string]struct{}),
runtimeValues: addRuntimeValues(),
}

maps.Copy(s.ruleNamespaces, builtinNamespaces)

for _, opt := range opts {
opt(s)
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/iac/rego/testdata/embedded/my-policy1.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# METADATA
# schemas:
# - input: schema["fooschema"]

package builtin.test

deny {
input.foo == "foo bar"
}
3 changes: 2 additions & 1 deletion pkg/scanner/local/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/aquasecurity/trivy/pkg/fanal/analyzer"
"github.com/aquasecurity/trivy/pkg/fanal/applier"
ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
"github.com/aquasecurity/trivy/pkg/iac/rego"
"github.com/aquasecurity/trivy/pkg/licensing"
"github.com/aquasecurity/trivy/pkg/log"
"github.com/aquasecurity/trivy/pkg/scanner/langpkg"
Expand Down Expand Up @@ -383,7 +384,7 @@ func toDetectedMisconfiguration(res ftypes.MisconfResult, defaultSeverity dbType

// empty namespace implies a go rule from defsec, "builtin" refers to a built-in rego rule
// this ensures we don't generate bad links for custom policies
if res.Namespace == "" || strings.HasPrefix(res.Namespace, "builtin.") {
if res.Namespace == "" || rego.IsBuiltinNamespace(res.Namespace) {
primaryURL = fmt.Sprintf("https://avd.aquasec.com/misconfig/%s", strings.ToLower(res.ID))
res.References = append(res.References, primaryURL)
}
Expand Down