pkg/assessor/manifest: Add sensitive variable names checks (#189)

* pkg/assessor/manifest: Add sensitive/suspicious vars checking to history cmds Signed-off-by: Alvaro Frias Garay <[email protected]> * Add flag -sensitive-word to add keyword when searching in sensitive words assessment Signed-off-by: Alvaro Frias Garay <[email protected]> * pkg/assessor/manifest: Add unit tests Signed-off-by: Alvaro Frias Garay <[email protected]> * pkg/assessor/manifest: Apply corrections Use suspiciousEnvKey and remove sensitiveWords slice. Change senstiveVars function signature. Returns boolean and sensitive word string if found. Update regex to ignore case sensitivity & handle regex error. Remove useless for. Use Sprintf instead of ReplaceAll. Update assessment message; now prints the suspicious env key found. Signed-off-by: Alvaro Frias Garay <[email protected]> * pkg/assessor/manifest: Add unit test for mixed cases sensitive key Signed-off-by: Alvaro Frias Garay <[email protected]> * pkg/assessor/manifest: Add acceptance keys checks to sensitiveVars Signed-off-by: Alvaro Frias Garay <[email protected]> * remove: check img.Config.Env, precompile: regexp.Compiler * pkg/assessor/manifest: Removed API from suspiciousEnvKey slice Signed-off-by: Alvaro Frias Garay <[email protected]> * fix TestSensitiveVars Co-authored-by: Alvaro Frias Garay <[email protected]> Co-authored-by: Tomoya AMACHI <[email protected]>
goodwithtech · Jul 23, 2022 · b7b64e3 · b7b64e3
1 parent da1a15c
commit b7b64e3
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 22 deletions.
diff --git a/go.mod b/go.mod
@@ -8,6 +8,7 @@ require (
 	github.com/docker/go-connections v0.4.0
 	github.com/goodwithtech/deckoder v0.0.1
 	github.com/google/go-cmp v0.5.6
+	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/owenrumney/go-sarif/v2 v2.0.17
 	github.com/urfave/cli v1.22.4
 	go.uber.org/zap v1.17.0

diff --git a/go.sum b/go.sum
@@ -494,6 +494,8 @@ github.com/google/pprof v0.0.0-20210601050228-01bbb1931b22/go.mod h1:kpwsk12EmLe
 github.com/google/pprof v0.0.0-20210609004039-a478d1d731e9/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
+github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
+github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=

diff --git a/pkg/app.go b/pkg/app.go
@@ -2,9 +2,10 @@ package pkg
 
 import (
 	"fmt"
-	"github.com/urfave/cli"
 	"os"
 	"time"
+
+	"github.com/urfave/cli"
 )
 
 var (
@@ -62,6 +63,11 @@ OPTIONS:
 			EnvVar: "DOCKLE_ACCEPT_KEYS",
 			Usage:  "For CIS-DI-0010. You can add acceptable keywords. e.g) -ak GPG_KEY -ak KEYCLOAK",
 		},
+		cli.StringSliceFlag{
+			Name:   "sensitive-word, sw",
+			EnvVar: "DOCKLE_REJECT_KEYS",
+			Usage:  "For CIS-DI-0010. You can add sensitive keywords to look for. e.g) -ak api_password -sw keys",
+		},
 		cli.StringSliceFlag{
 			Name:   "accept-file, af",
 			EnvVar: "DOCKLE_ACCEPT_FILES",

diff --git a/pkg/assessor/manifest/manifest.go b/pkg/assessor/manifest/manifest.go
@@ -4,12 +4,16 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"os"
+	"regexp"
 	"strings"
 	"time"
 
 	deckodertypes "github.com/goodwithtech/deckoder/types"
 
+	"github.com/google/shlex"
+
 	"github.com/goodwithtech/dockle/pkg/log"
 
 	"github.com/goodwithtech/dockle/pkg/types"
@@ -19,9 +23,10 @@ type ManifestAssessor struct{}
 
 var ConfigFileName = "metadata"
 var (
-	sensitiveDirs    = map[string]struct{}{"/sys": {}, "/dev": {}, "/proc": {}}
-	suspiciousEnvKey = []string{"PASSWD", "PASSWORD", "SECRET", "KEY", "ACCESS"}
-	acceptanceEnvKey = map[string]struct{}{"GPG_KEY": {}, "GPG_KEYS": {}}
+	sensitiveDirs      = map[string]struct{}{"/sys": {}, "/dev": {}, "/proc": {}}
+	suspiciousEnvKey   = []string{"PASS", "PASSWD", "PASSWORD", "SECRET", "KEY", "ACCESS", "TOKEN"}
+	acceptanceEnvKey   = map[string]struct{}{"GPG_KEY": {}, "GPG_KEYS": {}}
+	suspiciousCompiler *regexp.Regexp
 )
 
 func (a ManifestAssessor) Assess(fileMap deckodertypes.FileMap) (assesses []*types.Assessment, err error) {
@@ -41,13 +46,30 @@ func (a ManifestAssessor) Assess(fileMap deckodertypes.FileMap) (assesses []*typ
 	return checkAssessments(d)
 }
 
+func AddSensitiveWords(words []string) {
+	suspiciousEnvKey = append(suspiciousEnvKey, words...)
+}
+
 func AddAcceptanceKeys(keys []string) {
 	for _, key := range keys {
 		acceptanceEnvKey[key] = struct{}{}
 	}
 }
 
+func compileSensitivePatterns() error {
+	pat := fmt.Sprintf(`.*(?i)%s.*`, strings.Join(suspiciousEnvKey, "|"))
+	r, err := regexp.Compile(pat)
+	if err != nil {
+		return fmt.Errorf("compile suspicious key: %w", err)
+	}
+	suspiciousCompiler = r
+	return nil
+}
+
 func checkAssessments(img types.Image) (assesses []*types.Assessment, err error) {
+	if err := compileSensitivePatterns(); err != nil {
+		return nil, err
+	}
 	if img.Config.User == "" || img.Config.User == "root" {
 		assesses = append(assesses, &types.Assessment{
 			Code:     types.AvoidRootDefault,
@@ -56,23 +78,6 @@ func checkAssessments(img types.Image) (assesses []*types.Assessment, err error)
 		})
 	}
 
-	for _, envVar := range img.Config.Env {
-		e := strings.Split(envVar, "=")
-		envKey := e[0]
-		for _, suspiciousKey := range suspiciousEnvKey {
-			if strings.Contains(envKey, suspiciousKey) {
-				if _, ok := acceptanceEnvKey[envKey]; ok {
-					continue
-				}
-				assesses = append(assesses, &types.Assessment{
-					Code:     types.AvoidCredential,
-					Filename: ConfigFileName,
-					Desc:     fmt.Sprintf("Suspicious ENV key found : %s (You can suppress it with --accept-key)", envKey),
-				})
-			}
-		}
-	}
-
 	if img.Config.Healthcheck == nil {
 		assesses = append(assesses, &types.Assessment{
 			Code:     types.AddHealthcheck,
@@ -132,6 +137,15 @@ func splitByCommands(line string) map[int][]string {
 func assessHistory(index int, cmd types.History) []*types.Assessment {
 	var assesses []*types.Assessment
 	cmdSlices := splitByCommands(cmd.CreatedBy)
+
+	found, varName := sensitiveVars(cmd.CreatedBy)
+	if found {
+		assesses = append(assesses, &types.Assessment{
+			Code:     types.AvoidCredential,
+			Filename: ConfigFileName,
+			Desc:     fmt.Sprintf("Suspicious ENV key found : %s on %s (You can suppress it with --accept-key)", varName, cmd.CreatedBy),
+		})
+	}
 	if reducableApkAdd(cmdSlices) {
 		assesses = append(assesses, &types.Assessment{
 			Code:     types.UseApkAddNoCache,
@@ -210,6 +224,32 @@ func useADDstatement(cmdSlices map[int][]string) bool {
 	return false
 }
 
+func sensitiveVars(cmd string) (bool, string) {
+	if !strings.Contains(cmd, "=") {
+		return false, ""
+	}
+	toklexer := shlex.NewLexer(strings.NewReader(strings.ReplaceAll(cmd, "#", "")))
+	for {
+		word, err := toklexer.Next()
+		if err == io.EOF {
+			break
+		}
+		if !strings.Contains(word, "=") {
+			continue
+		}
+		varName := strings.Split(word, "=")[0]
+		if _, ok := acceptanceEnvKey[varName]; ok {
+			continue
+		}
+
+		if suspiciousCompiler.MatchString(varName) {
+			return true, varName
+		}
+	}
+
+	return false, ""
+}
+
 func checkAptCommand(target []string, command string) bool {
 	if containsThreshold(target, []string{"apt-get", "apt", command}, 2) {
 		return true

diff --git a/pkg/assessor/manifest/manifest_test.go b/pkg/assessor/manifest/manifest_test.go
@@ -401,6 +401,30 @@ func TestAddStatement(t *testing.T) {
 	}
 }
 
+func TestSensitiveVars(t *testing.T) {
+	if err := compileSensitivePatterns(); err != nil {
+		t.Fatalf("compile sensitive var patterns: %s", err)
+	}
+	var tests = map[string]struct {
+		cmd      string
+		expected bool
+	}{
+		"basic":              {cmd: "/bin/sh -c #(nop) ENV PASS=ADMIN", expected: true},
+		"mixed cases":        {cmd: "/bin/sh -c #(nop) ENV PasS=ADMIN", expected: true},
+		"two vars":           {cmd: "/bin/sh -c #(nop) ENV abc=hello password=sensibledata", expected: true},
+		"run command":        {cmd: `/bin/sh -c  SECRET_API_KEY=63AF7AA15067C05616FDDD88A3A2E8F226F0BC06 echo "data"`, expected: true},
+		"run false positive": {cmd: `/bin/sh -c HELLO="PASS=\"notThis\"" echo "false positive"`, expected: false},
+		"run command 2":      {cmd: `/bin/sh -c SECRET=myLittleSecret VAR2=VALUE2 VAR3=VALUE3 echo "Do something"`, expected: true},
+	}
+	for testname, v := range tests {
+		actual, _ := sensitiveVars(v.cmd)
+		if actual != v.expected {
+			t.Errorf("%s want: %t, got %t", testname, v.expected, actual)
+		}
+	}
+
+}
+
 func TestUseDistUpgrade(t *testing.T) {
 	var tests = map[string]struct {
 		cmdSlices map[int][]string

diff --git a/pkg/run.go b/pkg/run.go
@@ -4,11 +4,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/goodwithtech/dockle/pkg/assessor/manifest"
 	l "log"
 	"os"
 	"strings"
 
+	"github.com/goodwithtech/dockle/pkg/assessor/manifest"
+
 	"github.com/containers/image/v5/transports/alltransports"
 	deckodertypes "github.com/goodwithtech/deckoder/types"
 
@@ -75,6 +76,7 @@ func Run(c *cli.Context) (err error) {
 			return fmt.Errorf("invalid image: %w", err)
 		}
 	}
+	manifest.AddSensitiveWords(c.StringSlice("sensitive-word"))
 	manifest.AddAcceptanceKeys(c.StringSlice("accept-key"))
 	scanner.AddAcceptanceFiles(c.StringSlice("accept-file"))
 	scanner.AddAcceptanceExtensions(c.StringSlice("accept-file-extension"))