From 85f1fcbfd3a3d1e37688f107729f1595f9970f49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rog=C3=A9rio=20Peixoto?= Date: Thu, 29 Apr 2021 16:06:03 +0100 Subject: [PATCH] feat(cli): parametrizing query execution timeout - closes #3047 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rogério Peixoto --- docs/CONTRIBUTING.md | 35 +++++++++++++------------- docs/about.md | 4 +-- docs/configuration-file.md | 4 +++ docs/getting-started.md | 22 ++++++++-------- docs/queries.md | 2 +- docs/usage/commands.md | 12 +++++---- internal/console/scan.go | 49 ++++++++++++++++-------------------- pkg/engine/inspector.go | 26 +++++++++++-------- pkg/engine/inspector_test.go | 25 ++++++++++++------ 9 files changed, 98 insertions(+), 81 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index fd79a8ec333..9e2ce7b2883 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -1,8 +1,8 @@ ## Contribution -We would like to THANK YOU for considering contributing to KICS! +We would like to THANK YOU for considering contributing to KICS! -KICS is a true community project. It's built as an open source from day one, and anyone can find his own way to contribute to the project. +KICS is a true community project. It's built as an open source from day one, and anyone can find his own way to contribute to the project. Within just minutes, you can start making a difference, by sharing your expertise with a community of thousands of security experts and software developers. @@ -12,7 +12,7 @@ Good news! You don't have to contribute code. There are plenty of ways you can c - Reporting new [bugs or issues](https://github.com/Checkmarx/kics/issues) - Help triaging issues -- Improving and translating the documentation +- Improving and translating the documentation - Volunteering to maintain the project #### Code of Conduct @@ -26,34 +26,35 @@ By participating and contributing to the project, you agree to uphold our [Code Follow the instructions below to setup local KICS development environment and push your changes. 1. Fork the `kics` repo on GitHub. -1. Clone your fork locally: - ``` +1. Clone your fork locally: + ```shell git clone https://github.com/Checkmarx/kics.git ``` -1. Create a branch for local development. -Use succinct but descriptive name (prefix with *feature/issue#-descriptive-name>* or *hotfix/issue#-descriptive-name*): - ``` +1. Create a branch for local development. +Use succinct but descriptive name (prefix with *feature/issue#-descriptive-name>* or *hotfix/issue#-descriptive-name*): + ```shell git checkout -b ``` 1. Make your changes locally. -1. Validate your changes to reassure they meet project quality and contribution standards: - ``` +1. Validate your changes to reassure they meet project quality and contribution standards: + ```shell golint . - ``` ``` + ```shell go mod vendor - ``` ``` + ```shell go test -mod=vendor -v ./... ``` -1. Commit your changes and push your branch to GitHub: - ``` +1. Commit your changes and push your branch to GitHub: + ```shell git add . - ``` ``` - git commit - ``` + We recommend following [conventional commits messages](https://www.conventionalcommits.org/en/v1.0.0-beta.2/) + ```shell + git commit -m "feat(CLI): add new flag --blabla" ``` + ```shell git push --set-upstream origin ``` 1. Submit a pull request on GitHub website. diff --git a/docs/about.md b/docs/about.md index de86ef1b57c..cfb829ae8df 100644 --- a/docs/about.md +++ b/docs/about.md @@ -1,4 +1,4 @@ -KICS - Keeping Infrastructure as Code Secure +KICS - Keeping Infrastructure as Code Secure --- @@ -45,4 +45,4 @@ Main Benefits of Infrastructure as Code: Infrastructure as Code testing examines configuration definitions and scripts used to instantiate infrastructure to ensure the resulting resources are secure. -IaC security testing tools must be able to consume configuration files and scripts in relevant formats, apply tests to ensure conformance with common configuration hardening standards (i.e., Center for Internet Security Benchmarks and many others), identify security issues associated with specific operational environments, identify embedded secrets, and perform other tests supporting organization-specific standards and compliance requirements. Optionally, tools can automatically remediate errors (e.g., changing read/write permissions on storage resources). This capability specifically examines IaC testing in the context of the development process, however tools may also support examination of deployed production instances and responding to issues identified in those systems. \ No newline at end of file +IaC security testing tools must be able to consume configuration files and scripts in relevant formats, apply tests to ensure conformance with common configuration hardening standards (i.e., Center for Internet Security Benchmarks and many others), identify security issues associated with specific operational environments, identify embedded secrets, and perform other tests supporting organization-specific standards and compliance requirements. Optionally, tools can automatically remediate errors (e.g., changing read/write permissions on storage resources). This capability specifically examines IaC testing in the context of the development process, however tools may also support examination of deployed production instances and responding to issues identified in those systems. diff --git a/docs/configuration-file.md b/docs/configuration-file.md index 049585b74a7..c74ae1eff50 100644 --- a/docs/configuration-file.md +++ b/docs/configuration-file.md @@ -70,6 +70,7 @@ KICS is able to infer the format without the need of file extension. "queries-path": "path to directory with queries (default ./assets/queries) (default './assets/queries')", "report-formats": "formats in which the results will be exported (json, sarif, html)", "type": "type of queries to use in the scan", + "timeout": "number of seconds the query has to execute before being canceled", "verbose": true, "profiling": "enables performance profiler that prints resource consumption metrics in the logs during the execution (CPU, MEM)" } @@ -97,6 +98,7 @@ queries-path: "path to directory with queries (default ./assets/queries) (defaul report-formats: "formats in which the results will be exported (json, sarif, html)" silent: false type: "type of queries to use in the scan" +timeout: "number of seconds the query has to execute before being canceled" verbose: true ``` @@ -122,6 +124,7 @@ queries-path = "path to directory with queries (default ./assets/queries) (defau report-formats = "formats in which the results will be exported (json, sarif, html)" silent = false type = "type of queries to use in the scan" +timeout = "number of seconds the query has to execute before being canceled" verbose = true ``` @@ -147,6 +150,7 @@ verbose = true "report-formats" = "formats in which the results will be exported (json, sarif, html)" "silent" = false "type" = "type of queries to use in the scan" +"timeout" = "number of seconds the query has to execute before being canceled" "verbose" = true ``` diff --git a/docs/getting-started.md b/docs/getting-started.md index 2dc1267f5b5..f6f1077d5ec 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -8,7 +8,7 @@ KICS is available as a -o + ```shell + ./kics scan -p '' -o '' ``` #### Build from Sources 1. Download and install Go from https://golang.org/dl/ 1. Clone the repository: - ``` + ```shell git clone https://github.com/Checkmarx/kics.git ``` - ``` + ```shell cd kics ``` 1. Kick a scan! - ``` - go run ./cmd/console/main.go scan -p -o + ```shell + go run ./cmd/console/main.go scan -p '' -o '' ``` --- diff --git a/docs/queries.md b/docs/queries.md index 6d65f75414e..3fbbbe944e8 100644 --- a/docs/queries.md +++ b/docs/queries.md @@ -2,7 +2,7 @@ KICS queries are written in OPA (Rego). -```Opa +```rego CxPolicy [ result ] { resource := input.document[i].resource.aws_s3_bucket[name] role = "public-read" diff --git a/docs/usage/commands.md b/docs/usage/commands.md index 566414474e2..a342bd48845 100644 --- a/docs/usage/commands.md +++ b/docs/usage/commands.md @@ -51,13 +51,13 @@ Flags: -x, --exclude-results strings exclude results by providing the similarity ID of a result can be provided multiple times or as a comma separated string example: 'fec62a97d569662093dbb9739360942f...,31263s5696620s93dbb973d9360942fc2a...' - --fail-on which kind of results should return an exit code different from 0 + --fail-on strings which kind of results should return an exit code different from 0 accetps: high, medium, low and info - example: "high,low" - --ignoreOnExitFlag defines which kind of non-zero exits code should be ignored - accepts: all, results, errors, none - example: if 'results' is set, only engine errors will make KICS exit code different from 0 + example: "high,low" (default [high,medium,low,info]) -h, --help help for scan + --ignore-on-exit string defines which kind of non-zero exits code should be ignored + accepts: all, results, errors, none + example: if 'results' is set, only engine errors will make KICS exit code different from 0 (default "none") --minimal-ui simplified version of CLI output --no-progress hides the progress bar -o, --output-path string directory path to store reports @@ -67,6 +67,7 @@ Flags: --preview-lines int number of lines to be display in CLI results (min: 1, max: 30) (default 3) -q, --queries-path string path to directory with queries (default "./assets/queries") --report-formats strings formats in which the results will be exported (json, sarif, html) + --timeout int number of seconds the query has to execute before being canceled (default 60) -t, --type strings case insensitive list of platform types to scan (Ansible, CloudFormation, Dockerfile, Kubernetes, OpenAPI, Terraform) @@ -79,6 +80,7 @@ Global Flags: --profiling string enables performance profiler that prints resource consumption metrics in the logs during the execution (CPU, MEM) -s, --silent silence stdout messages (mutually exclusive with verbose and ci) -v, --verbose write logs to stdout too (mutually exclusive with silent) + ``` The other commands have no further options. diff --git a/internal/console/scan.go b/internal/console/scan.go index e126075122b..01ce706529c 100644 --- a/internal/console/scan.go +++ b/internal/console/scan.go @@ -57,6 +57,7 @@ var ( queryPath string reportFormats []string types []string + queryExecTimeout int ) const ( @@ -84,6 +85,7 @@ const ( scanCommandStr = "scan" typeFlag = "type" typeShorthand = "t" + queryExecTimeoutFlag = "timeout" ) // NewScanCmd creates a new instance of the scan Command @@ -223,19 +225,15 @@ func setBoundFlags(flagName string, val interface{}, cmd *cobra.Command) { func initScanFlags(scanCmd *cobra.Command) { scanCmd.Flags().StringSliceVarP(&path, - pathFlag, - pathFlagShorthand, + pathFlag, pathFlagShorthand, []string{}, "paths or directories to scan\nexample: \"./somepath,somefile.txt\"") scanCmd.Flags().StringVarP(&cfgFile, configFlag, - "", - "", + "", "", "path to configuration file") - scanCmd.Flags().StringVarP( - &queryPath, - queriesPathCmdName, - queriesPathShorthand, + scanCmd.Flags().StringVarP(&queryPath, + queriesPathCmdName, queriesPathShorthand, "./assets/queries", "path to directory with queries", ) @@ -256,36 +254,30 @@ func initScanFlags(scanCmd *cobra.Command) { 3, "number of lines to be display in CLI results (min: 1, max: 30)") scanCmd.Flags().StringVarP(&payloadPath, - payloadPathFlag, - payloadPathShorthand, + payloadPathFlag, payloadPathShorthand, "", "path to store internal representation JSON file") scanCmd.Flags().StringSliceVarP(&excludePath, - excludePathsFlag, - excludePathsShorthand, + excludePathsFlag, excludePathsShorthand, []string{}, "exclude paths from scan\nsupports glob and can be provided multiple times or as a quoted comma separated string"+ "\nexample: './shouldNotScan/*,somefile.txt'", ) scanCmd.Flags().BoolVarP(&min, - minimalUIFlag, - "", + minimalUIFlag, "", false, "simplified version of CLI output") scanCmd.Flags().StringSliceVarP(&types, - typeFlag, - typeShorthand, + typeFlag, typeShorthand, []string{""}, "case insensitive list of platform types to scan\n"+ fmt.Sprintf("(%s)", strings.Join(source.ListSupportedPlatforms(), ", "))) scanCmd.Flags().BoolVarP(&noProgress, - noProgressFlag, - "", + noProgressFlag, "", false, "hides the progress bar") scanCmd.Flags().StringSliceVarP(&excludeIDs, - excludeQueriesFlag, - "", + excludeQueriesFlag, "", []string{}, "exclude queries by providing the query ID\n"+ "can be provided multiple times or as a comma separated string\n"+ @@ -300,28 +292,29 @@ func initScanFlags(scanCmd *cobra.Command) { "example: 'fec62a97d569662093dbb9739360942f...,31263s5696620s93dbb973d9360942fc2a...'", ) scanCmd.Flags().StringSliceVarP(&excludeCategories, - excludeCategoriesFlag, - "", + excludeCategoriesFlag, "", []string{}, "exclude categories by providing its name\n"+ "can be provided multiple times or as a comma separated string\n"+ "example: 'Access control,Best practices'", ) scanCmd.Flags().StringSliceVarP(&failOn, - failOnFlag, - "", + failOnFlag, "", []string{"high", "medium", "low", "info"}, "which kind of results should return an exit code different from 0\n"+ "accetps: high, medium, low and info\n"+ "example: \"high,low\"", ) scanCmd.Flags().StringVarP(&ignoreOnExit, - ignoreOnExitFlag, - "", + ignoreOnExitFlag, "", "none", "defines which kind of non-zero exits code should be ignored\n"+"accepts: all, results, errors, none\n"+ "example: if 'results' is set, only engine errors will make KICS exit code different from 0", ) + scanCmd.Flags().IntVarP(&queryExecTimeout, + queryExecTimeoutFlag, "", + 60, + "number of seconds the query has to execute before being canceled") } func initScanCmd(scanCmd *cobra.Command) { @@ -374,7 +367,9 @@ func createInspector(t engine.Tracker, querySource source.QueriesSource) (*engin ByCategories: excludeCategories, } - inspector, err := engine.NewInspector(ctx, querySource, engine.DefaultVulnerabilityBuilder, t, excludeQueries, excludeResultsMap) + inspector, err := engine.NewInspector(ctx, + querySource, engine.DefaultVulnerabilityBuilder, + t, excludeQueries, excludeResultsMap, queryExecTimeout) if err != nil { return nil, err } diff --git a/pkg/engine/inspector.go b/pkg/engine/inspector.go index 021fad0abc9..c7d010f9ca0 100644 --- a/pkg/engine/inspector.go +++ b/pkg/engine/inspector.go @@ -75,8 +75,9 @@ type Inspector struct { excludeResults map[string]bool detector *detector.DetectLine - enableCoverageReport bool - coverageReport cover.Report + enableCoverageReport bool + coverageReport cover.Report + queryExecTimeoutSeconds time.Duration } // QueryContext contains the context where the query is executed, which scan it belongs, basic information of query, @@ -104,7 +105,8 @@ func NewInspector( vb VulnerabilityBuilder, tracker Tracker, excludeQueries source.ExcludeQueries, - excludeResults map[string]bool) (*Inspector, error) { + excludeResults map[string]bool, + queryTimeout int) (*Inspector, error) { log.Debug().Msg("engine.NewInspector()") metrics.Metric.Start("get_queries") @@ -170,13 +172,17 @@ func NewInspector( Add(helm.DetectKindLine{}, model.KindHELM). Add(docker.DetectKindLine{}, model.KindDOCKER) + queryExecTimeout := time.Duration(queryTimeout) * time.Second + log.Info().Msgf("Query execution timeout=%v", queryExecTimeout) + return &Inspector{ - queries: opaQueries, - vb: vb, - tracker: tracker, - failedQueries: failedQueries, - excludeResults: excludeResults, - detector: lineDetctor, + queries: opaQueries, + vb: vb, + tracker: tracker, + failedQueries: failedQueries, + excludeResults: excludeResults, + detector: lineDetctor, + queryExecTimeoutSeconds: queryExecTimeout, }, nil } @@ -267,7 +273,7 @@ func (c *Inspector) GetFailedQueries() map[string]error { } func (c *Inspector) doRun(ctx *QueryContext) ([]model.Vulnerability, error) { - timeoutCtx, cancel := context.WithTimeout(ctx.ctx, executeTimeout) + timeoutCtx, cancel := context.WithTimeout(ctx.ctx, c.queryExecTimeoutSeconds) defer cancel() options := []rego.EvalOption{rego.EvalInput(ctx.payload)} diff --git a/pkg/engine/inspector_test.go b/pkg/engine/inspector_test.go index 63aa04a5866..09e91415c02 100644 --- a/pkg/engine/inspector_test.go +++ b/pkg/engine/inspector_test.go @@ -348,12 +348,13 @@ func TestNewInspector(t *testing.T) { // nolint }, }) type args struct { - ctx context.Context - source source.QueriesSource - vb VulnerabilityBuilder - tracker Tracker - excludeQueries source.ExcludeQueries - excludeResults map[string]bool + ctx context.Context + source source.QueriesSource + vb VulnerabilityBuilder + tracker Tracker + excludeQueries source.ExcludeQueries + excludeResults map[string]bool + queryExecTimeout int } tests := []struct { name string @@ -372,7 +373,8 @@ func TestNewInspector(t *testing.T) { // nolint ByIDs: []string{}, ByCategories: []string{}, }, - excludeResults: map[string]bool{}, + excludeResults: map[string]bool{}, + queryExecTimeout: 60, }, want: &Inspector{ vb: vbs, @@ -384,7 +386,14 @@ func TestNewInspector(t *testing.T) { // nolint } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewInspector(tt.args.ctx, tt.args.source, tt.args.vb, tt.args.tracker, tt.args.excludeQueries, tt.args.excludeResults) + got, err := NewInspector(tt.args.ctx, + tt.args.source, + tt.args.vb, + tt.args.tracker, + tt.args.excludeQueries, + tt.args.excludeResults, + tt.args.queryExecTimeout) + if (err != nil) != tt.wantErr { t.Errorf("NewInspector() error: got = %v,\n wantErr = %v", err, tt.wantErr) return