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

implement scan and skip rules #441

Merged
merged 7 commits into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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
15 changes: 15 additions & 0 deletions config/terrascan.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,18 @@
[notifications]
[notifications.webhook]
url = "https://httpbin.org/post"

# scan and skip rules configuration
[rules]
# scan rules (list of rules to scan)
# adding rules here will override rules in the policy path
scan-rules = [
"AWS.S3Bucket.DS.High.1043",
"AWS.S3Bucket.IAM.High.0370"
]

# skip rules (list of rules to skip)
skip-rules = [
"AWS.S3Bucket.DS.High.1043",
"AWS.S3Bucket.IAM.High.0370",
]
11 changes: 8 additions & 3 deletions pkg/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ type ScanOptions struct {
UseColors bool
useColors string // used for flag processing

// ScanRules is the array of rules to scan
scanRules []string

// SkipRules is the array of rules to skip while scanning
skipRules []string

// Verbose indicates whether to display all fields in default human readlbe output
Verbose bool
}
Expand All @@ -101,8 +107,7 @@ func (s *ScanOptions) Scan() error {
//Init initalises and validates ScanOptions
func (s *ScanOptions) Init() error {
s.initColor()
err := s.validate()
if err != nil {
if err := s.validate(); err != nil {
zap.S().Error("failed to start scan", zap.Error(err))
return err
}
Expand Down Expand Up @@ -162,7 +167,7 @@ func (s *ScanOptions) Run() error {

// create a new runtime executor for processing IaC
executor, err := runtime.NewExecutor(s.iacType, s.iacVersion, s.policyType,
s.iacFilePath, s.iacDirPath, s.configFile, s.policyPath)
s.iacFilePath, s.iacDirPath, s.configFile, s.policyPath, s.scanRules, s.skipRules)
if err != nil {
return err
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/cli/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func TestRun(t *testing.T) {
testDirPath := "testdata/run-test"
kustomizeTestDirPath := testDirPath + "/kustomize-test"
testTerraformFilePath := testDirPath + "/config-only.tf"
ruleSlice := []string{"AWS.ECR.DataSecurity.High.0579", "AWS.SecurityGroup.NetworkPortsSecurity.Low.0561"}

table := []struct {
name string
Expand Down Expand Up @@ -133,6 +134,43 @@ func TestRun(t *testing.T) {
},
wantErr: true,
},
{
name: "incorrect config file",
scanOptions: &ScanOptions{
policyType: []string{"all"},
iacDirPath: testTerraformFilePath,
outputType: "json",
configFile: "invalidFile",
},
wantErr: true,
},
{
name: "run with skip rules",
scanOptions: &ScanOptions{
policyType: []string{"all"},
iacDirPath: testDirPath,
outputType: "json",
skipRules: ruleSlice,
},
},
{
name: "run with scan rules",
scanOptions: &ScanOptions{
policyType: []string{"all"},
iacDirPath: testDirPath,
outputType: "yaml",
scanRules: ruleSlice,
},
},
{
name: "config file with rules",
scanOptions: &ScanOptions{
policyType: []string{"all"},
iacDirPath: testDirPath,
outputType: "yaml",
configFile: "testdata/configFile.toml",
},
},
}

for _, tt := range table {
Expand Down
2 changes: 2 additions & 0 deletions pkg/cli/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,7 @@ func init() {
// flag passes a string, but we normalize to bool in PreRun
scanCmd.Flags().StringVar(&scanOptions.useColors, "use-colors", "auto", "color output (auto, t, f)")
scanCmd.Flags().BoolVarP(&scanOptions.Verbose, "verbose", "v", false, "will show violations with details (applicable for default output)")
scanCmd.Flags().StringSliceVarP(&scanOptions.scanRules, "scan-rules", "", []string{}, "one or more rules to scan (example: --scan-rules=\"ruleID1,ruleID2\")")
scanCmd.Flags().StringSliceVarP(&scanOptions.skipRules, "skip-rules", "", []string{}, "one or more rules to skip while scanning (example: --skip-rules=\"ruleID1,ruleID2\")")
RegisterCommand(rootCmd, scanCmd)
}
7 changes: 7 additions & 0 deletions pkg/cli/testdata/configFile.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[rules]
scan-rules = [
"AWS.ECR.DataSecurity.High.0579"
]
skip-rules = [
"AWS.SecurityGroup.NetworkPortsSecurity.Low.0561"
]
4 changes: 2 additions & 2 deletions pkg/http-server/file-scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ func (g *APIHandler) scanFile(w http.ResponseWriter, r *http.Request) {
var executor *runtime.Executor
if g.test {
executor, err = runtime.NewExecutor(iacType, iacVersion, cloudType,
tempFile.Name(), "", "", []string{"./testdata/testpolicies"})
tempFile.Name(), "", "", []string{"./testdata/testpolicies"}, []string{}, []string{})
williepaul marked this conversation as resolved.
Show resolved Hide resolved
} else {
executor, err = runtime.NewExecutor(iacType, iacVersion, cloudType,
tempFile.Name(), "", "", []string{})
tempFile.Name(), "", "", []string{}, []string{}, []string{})
}
if err != nil {
zap.S().Error(err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/http-server/remote-repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (s *scanRemoteRepoReq) ScanRemoteRepo(iacType, iacVersion string, cloudType

// create a new runtime executor for scanning the remote repo
executor, err := runtime.NewExecutor(iacType, iacVersion, cloudType,
"", iacDirPath, "", policyPath)
"", iacDirPath, "", policyPath, []string{}, []string{})
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be taking rules to scan and skip for the API server as well. This functionality applies to the scanning for API server as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented

if err != nil {
zap.S().Error(err)
return output, err
Expand Down
8 changes: 5 additions & 3 deletions pkg/notifications/notifiers.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ const (

var (
errNotifierNotSupported = fmt.Errorf("notifier not supported")
errTomlKeyNotPresent = fmt.Errorf("key not present in toml config")

// ErrTomlKeyNotPresent will be returned when config file does not have notificationsConfigKey
ErrTomlKeyNotPresent = fmt.Errorf("key not present in toml config")
)

// NewNotifier returns a new notifier
Expand Down Expand Up @@ -63,7 +65,7 @@ func NewNotifiers(configFile string) ([]Notifier, error) {
keyConfig := config.Get(notificationsConfigKey)
if keyConfig == nil {
zap.S().Infof("key '%s' not present in toml config", notificationsConfigKey)
return notifiers, errTomlKeyNotPresent
return notifiers, ErrTomlKeyNotPresent
}

// get all the notifier types configured in TOML config
Expand All @@ -84,7 +86,7 @@ func NewNotifiers(configFile string) ([]Notifier, error) {
nTypeConfig := keyTomlConfig.Get(nType)
if nTypeConfig.(*toml.Tree).String() == "" {
zap.S().Errorf("notifier '%v' config not present", nType)
allErrs = utils.WrapError(errTomlKeyNotPresent, allErrs)
allErrs = utils.WrapError(ErrTomlKeyNotPresent, allErrs)
continue
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/notifications/notifiers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestNewNotifiers(t *testing.T) {
{
name: "key not present",
configFile: "testdata/nokey.toml",
wantErr: errTomlKeyNotPresent,
wantErr: ErrTomlKeyNotPresent,
},
{
name: "invalid notifier",
Expand All @@ -72,7 +72,7 @@ func TestNewNotifiers(t *testing.T) {
{
name: "empty notifier config",
configFile: "testdata/empty-notifier-config.toml",
wantErr: errTomlKeyNotPresent,
wantErr: ErrTomlKeyNotPresent,
},
{
name: "invalid notifier config",
Expand Down
3 changes: 2 additions & 1 deletion pkg/policy/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ package policy

// Engine Policy Engine interface
type Engine interface {
Init(string) error
// to initialize engine with policy path, scan and skip rules
williepaul marked this conversation as resolved.
Show resolved Hide resolved
Init(string, []string, []string) error
Configure() error
Evaluate(EngineInput) (EngineOutput, error)
GetResults() EngineOutput
Expand Down
58 changes: 55 additions & 3 deletions pkg/policy/opa/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ var (
)

// NewEngine returns a new OPA policy engine
func NewEngine(policyPath string) (*Engine, error) {
func NewEngine(policyPath string, scanRules, skipRules []string) (*Engine, error) {
williepaul marked this conversation as resolved.
Show resolved Hide resolved

// opa engine struct
engine := &Engine{}

// initialize the engine
if err := engine.Init(policyPath); err != nil {
if err := engine.Init(policyPath, scanRules, skipRules); err != nil {
zap.S().Error("failed to initialize OPA policy engine", zap.Error(err))
return engine, errInitFailed
}
Expand Down Expand Up @@ -249,14 +249,20 @@ func (e *Engine) CompileRegoFiles() error {

// Init initializes the Opa engine
// Handles loading all rules, filtering, compiling, and preparing for evaluation
func (e *Engine) Init(policyPath string) error {
func (e *Engine) Init(policyPath string, scanRules, skipRules []string) error {
e.context = context.Background()

if err := e.LoadRegoFiles(policyPath); err != nil {
zap.S().Error("error loading rego files", zap.String("policy path", policyPath), zap.Error(err))
return err
}

// before compiling the rego files, filter the rules based on scan and skip rules supplied
filterRules(e, policyPath, scanRules, skipRules)
williepaul marked this conversation as resolved.
Show resolved Hide resolved

// update the rule count
e.stats.ruleCount = len(e.regoDataMap)
Copy link
Contributor

Choose a reason for hiding this comment

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

May be it's a good idea to error out if the ruleCount is less than 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

multiple engines are created based on policy path slice, cannot error out.


err := e.CompileRegoFiles()
if err != nil {
zap.S().Error("error compiling rego files", zap.String("policy path", policyPath), zap.Error(err))
Expand Down Expand Up @@ -395,3 +401,49 @@ func (e *Engine) Evaluate(engineInput policy.EngineInput) (policy.EngineOutput,
e.results.ViolationStore.Summary.TotalPolicies += e.stats.ruleCount
return e.results, nil
}

func filterRules(e *Engine, policyPath string, scanRules, skipRules []string) {
// apply scan rules
if len(scanRules) > 0 {
filterScanRules(e, policyPath, scanRules)
}

// apply skip rules
if len(skipRules) > 0 {
filterSkipRules(e, policyPath, skipRules)
}
}

func filterScanRules(e *Engine, policyPath string, scanRules []string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we tie filterScanRules with the Engine{} struct?


// temporary map to store data from original rego data map
tempMap := make(map[string]*RegoData)
for _, ruleID := range scanRules {
regoData, ok := e.regoDataMap[ruleID]
if ok {
zap.S().Infof("scan rule added. rule id: %+v found in policy path: %s", ruleID, policyPath)
tempMap[ruleID] = regoData
} else {
zap.S().Warnf("scan rule id: %+v not found in policy path: %s", ruleID, policyPath)
}
}
if len(tempMap) == 0 {
zap.S().Warnf("scan rule id's: %+v not found in policy path: %s", scanRules, policyPath)
}

// the regoDataMap should only contain regoData for supplied scan rules
e.regoDataMap = tempMap
}

func filterSkipRules(e *Engine, policyPath string, skipRules []string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we tie filterSkipRules with the Engine{} struct?

// remove rules to be skipped from the rego data map
for _, ruleID := range skipRules {
_, ok := e.regoDataMap[ruleID]
if ok {
zap.S().Infof("skip rule added. rule id: %+v found in policy path: %s", ruleID, policyPath)
delete(e.regoDataMap, ruleID)
} else {
zap.S().Warnf("skip rule id: %+v not found in policy path: %s", ruleID, policyPath)
}
}
}
97 changes: 97 additions & 0 deletions pkg/policy/opa/engine_test.go
Original file line number Diff line number Diff line change
@@ -1 +1,98 @@
package opa

import (
"fmt"
"testing"
)

func TestFilterRules(t *testing.T) {
testPolicyPath := "test"

type args struct {
e *Engine
policyPath string
scanRules []string
skipRules []string
}
tests := []struct {
name string
args args
assert bool
regoMapSize int
}{
{
name: "no scan and skip rules",
args: args{
e: &Engine{},
},
},
{
name: "scan rules test",
args: args{
e: &Engine{
regoDataMap: getTestRegoDataMap(10),
},
policyPath: testPolicyPath,
scanRules: []string{"Rule.0", "Rule.1", "Rule.2", "Rule.3", "Rule.11"},
},
assert: true,
regoMapSize: 4,
},
{
name: "scan rules not found in path",
args: args{
e: &Engine{
regoDataMap: getTestRegoDataMap(10),
},
policyPath: testPolicyPath,
scanRules: []string{"Rule.11", "Rule.12", "Rule.13"},
},
assert: true,
regoMapSize: 0,
},
{
name: "skip rules test",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a test for scan and skip rules present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a test for it.

args: args{
e: &Engine{
regoDataMap: getTestRegoDataMap(6),
},
policyPath: testPolicyPath,
skipRules: []string{"Rule.1"},
},
assert: true,
regoMapSize: 5,
},
{
name: "skip rules not found in policy path",
args: args{
e: &Engine{
regoDataMap: getTestRegoDataMap(20),
},
policyPath: testPolicyPath,
skipRules: []string{"Rule.21", "Rule.22"},
},
assert: true,
regoMapSize: 20,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
filterRules(tt.args.e, tt.args.policyPath, tt.args.scanRules, tt.args.skipRules)
if tt.assert {
if len(tt.args.e.regoDataMap) != tt.regoMapSize {
t.Errorf("filterRules(): expected regoDataMap size = %d, got = %d", tt.regoMapSize, len(tt.args.e.regoDataMap))
}
}
})
}
}

// helper func to generate test rego data map of given size
func getTestRegoDataMap(size int) map[string]*RegoData {
testRegoDataMap := make(map[string]*RegoData)
for i := 0; i < size; i++ {
ruleID := fmt.Sprintf("Rule.%d", i)
testRegoDataMap[ruleID] = &RegoData{}
}
return testRegoDataMap
}
Loading