check permissions in any dir (#57)

* check permissions in any dir

* check all setuid, setgid & fix test

Author: tomoyamachi

CHECKPOINT.md

@@ -73,7 +73,7 @@ HEALTHCHECK --interval=5m --timeout=3s \
 RUN apt-get update && apt-get install -y package-a
 ```
 
-### CIS-DI-0008: Remove `setuid` and `setgid` permissions in the images
+### CIS-DI-0008: Confirm safety of `setuid` and `setgid` files
 
 > Removing `setuid` and `setgid` permissions in the images would prevent privilege escalation attacks in the containers.<br/>
 > `setuid` and `setgid` permissions could be used for elevating privileges.

README.md

@@ -239,7 +239,7 @@ $ docker run --rm goodwithtech/dockle:v${DOCKLE_LATEST} [YOUR_IMAGE_NAME]
 | [CIS-DI-0005](CHECKPOINT.md#cis-di-0005-enable-content-trust-for-docker) | Enable Content trust for Docker | FATAL
 | [CIS-DI-0006](CHECKPOINT.md#cis-di-0006-add-healthcheck-instruction-to-the-container-image) | Add `HEALTHCHECK` instruction to the container image | WARN
 | [CIS-DI-0007](CHECKPOINT.md#cis-di-0007-do-not-use-update-instructions-alone-in-the-dockerfile) | Do not use `update` instructions alone in the Dockerfile | FATAL
-| [CIS-DI-0008](CHECKPOINT.md#cis-di-0008-remove-setuid-and-setgid-permissions-in-the-images) | Remove `setuid` and `setgid` permissions in the images | INFO
+| [CIS-DI-0008](CHECKPOINT.md#cis-di-0008-comfirm-safety-of-setuid-setgid-files) | Confirm safety of `setuid` and `setgid` files | INFO
 | [CIS-DI-0009](CHECKPOINT.md#cis-di-0009-use-copy-instead-of-add-in-dockerfile) | Use `COPY` instead of `ADD` in Dockerfile | FATAL
 | [CIS-DI-0010](CHECKPOINT.md#cis-di-0010-do-not-store-secrets-in-dockerfiles) | Do not store secrets in Dockerfiles | FATAL
 | [CIS-DI-0011](CHECKPOINT.md#cis-di-0011-install-verified-packages-only) | Install verified packages only | INFO

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/elazarl/goproxy v0.0.0-20190421051319-9d40249d3c2f // indirect
 	github.com/elazarl/goproxy/ext v0.0.0-20190421051319-9d40249d3c2f // indirect
 	github.com/genuinetools/reg v0.16.0
-	github.com/goodwithtech/deckoder v0.0.0-20190914200440-2d3347be7dd3
+	github.com/goodwithtech/deckoder v0.0.0-20191103063217-e4d89094c19d
 	github.com/google/go-cmp v0.3.0
 	github.com/moul/http2curl v1.0.0 // indirect
 	github.com/parnurzeal/gorequest v0.2.15

go.sum

@@ -80,8 +80,8 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
 github.com/golang/protobuf v1.3.1 h1:YF8+flBXS5eO826T4nzqPrxfhQThhXl0YzfuUPu4SBg=
 github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
-github.com/goodwithtech/deckoder v0.0.0-20190914200440-2d3347be7dd3 h1:fHsxZkhj9PBDtNwo/6x+uOGe0TZajp/nVLFn7RgbVag=
-github.com/goodwithtech/deckoder v0.0.0-20190914200440-2d3347be7dd3/go.mod h1:+OL/nA/BaFd697EYdEP111Opjo26rf2a6CUtO671V8s=
+github.com/goodwithtech/deckoder v0.0.0-20191103063217-e4d89094c19d h1:T1racnAujPtKvGaE6HR2zHilu9P0/DkI34JAtZXgPcE=
+github.com/goodwithtech/deckoder v0.0.0-20191103063217-e4d89094c19d/go.mod h1:+OL/nA/BaFd697EYdEP111Opjo26rf2a6CUtO671V8s=
 github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
 github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ=
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=

pkg/assessor/contentTrust/contentTrust.go

@@ -8,6 +8,8 @@ import (
 	"github.com/goodwithtech/dockle/pkg/types"
 )
 
+var HostEnvironmentFileName = "ENVIRONMENT variable on HOST OS"
+
 type ContentTrustAssessor struct{}
 
 func (a ContentTrustAssessor) Assess(fileMap extractor.FileMap) ([]*types.Assessment, error) {
@@ -17,7 +19,7 @@ func (a ContentTrustAssessor) Assess(fileMap extractor.FileMap) ([]*types.Assess
 		return []*types.Assessment{
 			{
 				Type:     types.UseContentTrust,
-				Filename: "ENVIRONMENT variable",
+				Filename: HostEnvironmentFileName,
 				Desc:     "export DOCKER_CONTENT_TRUST=1 before docker pull/build",
 			},
 		}, nil

pkg/assessor/manifest/manifest.go

@@ -16,9 +16,12 @@ import (
 
 type ManifestAssessor struct{}
 
-var sensitiveDirs = map[string]struct{}{"/sys": {}, "/dev": {}, "/proc": {}}
-var suspiciousEnvKey = []string{"PASSWD", "PASSWORD", "SECRET", "KEY", "ACCESS"}
-var acceptanceEnvKey = map[string]struct{}{"GPG_KEY": {}, "GPG_KEYS": {}}
+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": {}}
+)
 
 func (a ManifestAssessor) Assess(fileMap extractor.FileMap) (assesses []*types.Assessment, err error) {
 	log.Logger.Debug("Scan start : config file")
@@ -41,7 +44,7 @@ func checkAssessments(img types.Image) (assesses []*types.Assessment, err error)
 	if img.Config.User == "" || img.Config.User == "root" {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.AvoidRootDefault,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     "Last user should not be root",
 		})
 	}
@@ -56,7 +59,7 @@ func checkAssessments(img types.Image) (assesses []*types.Assessment, err error)
 				}
 				assesses = append(assesses, &types.Assessment{
 					Type:     types.AvoidEnvKeySecret,
-					Filename: "docker config",
+					Filename: ConfigFileName,
 					Desc:     fmt.Sprintf("Suspicious ENV key found : %s", envKey),
 				})
 			}
@@ -66,7 +69,7 @@ func checkAssessments(img types.Image) (assesses []*types.Assessment, err error)
 	if img.Config.Healthcheck == nil {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.AddHealthcheck,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     "not found HEALTHCHECK statement",
 		})
 	}
@@ -92,7 +95,7 @@ func checkAssessments(img types.Image) (assesses []*types.Assessment, err error)
 		if _, ok := sensitiveDirs[volume]; ok {
 			assesses = append(assesses, &types.Assessment{
 				Type:     types.AvoidSensitiveDirectoryMounting,
-				Filename: "docker config",
+				Filename: ConfigFileName,
 				Desc:     fmt.Sprintf("Avoid mounting sensitive dirs : %s", volume),
 			})
 		}
@@ -125,46 +128,46 @@ func assessHistory(index int, cmd types.History) []*types.Assessment {
 	if reducableApkAdd(cmdSlices) {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.UseApkAddNoCache,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     fmt.Sprintf("Use --no-cache option if use 'apk add': %s", cmd.CreatedBy),
 		})
 	}
 
 	if reducableAptGetInstall(cmdSlices) {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.MinimizeAptGet,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     fmt.Sprintf("Use 'rm -rf /var/lib/apt/lists' after 'apt-get install' : %s", cmd.CreatedBy),
 		})
 	}
 
 	if reducableAptGetUpdate(cmdSlices) {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.UseAptGetUpdateNoCache,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     fmt.Sprintf("Always combine 'apt-get update' with 'apt-get install' : %s", cmd.CreatedBy),
 		})
 	}
 
 	if index != 0 && useADDstatement(cmdSlices) {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.UseCOPY,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     fmt.Sprintf("Use COPY : %s", cmd.CreatedBy),
 		})
 	}
 
 	if useDistUpgrade(cmdSlices) {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.AvoidDistUpgrade,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     fmt.Sprintf("Avoid upgrade in container : %s", cmd.CreatedBy),
 		})
 	}
 	if useSudo(cmdSlices) {
 		assesses = append(assesses, &types.Assessment{
 			Type:     types.AvoidSudo,
-			Filename: "docker config",
+			Filename: ConfigFileName,
 			Desc:     fmt.Sprintf("Avoid sudo in container : %s", cmd.CreatedBy),
 		})
 	}

pkg/assessor/manifest/manifest_test.go

@@ -21,11 +21,11 @@ func TestAssess(t *testing.T) {
 			assesses: []*types.Assessment{
 				{
 					Type:     types.AvoidRootDefault,
-					Filename: "docker config",
+					Filename: ConfigFileName,
 				},
 				{
 					Type:     types.AddHealthcheck,
-					Filename: "docker config",
+					Filename: ConfigFileName,
 				},
 			},
 		},
@@ -35,19 +35,19 @@ func TestAssess(t *testing.T) {
 			assesses: []*types.Assessment{
 				{
 					Type:     types.AvoidRootDefault,
-					Filename: "docker config",
+					Filename: ConfigFileName,
 				},
 				{
 					Type:     types.UseApkAddNoCache,
-					Filename: "docker config",
+					Filename: ConfigFileName,
 				},
 				{
 					Type:     types.AddHealthcheck,
-					Filename: "docker config",
+					Filename: ConfigFileName,
 				},
 				{
 					Type:     types.UseCOPY,
-					Filename: "docker config",
+					Filename: ConfigFileName,
 				},
 			},
 		},

pkg/assessor/privilege/suid.go

@@ -3,60 +3,45 @@ package privilege
 import (
 	"fmt"
 	"os"
-	"strings"
 
 	"github.com/goodwithtech/deckoder/extractor"
 	"github.com/goodwithtech/dockle/pkg/types"
 )
 
 type PrivilegeAssessor struct{}
 
-var ignorePaths = []string{"bin/", "usr/lib/"}
-
 func (a PrivilegeAssessor) Assess(fileMap extractor.FileMap) ([]*types.Assessment, error) {
 	var assesses []*types.Assessment
 
 	for filename, filedata := range fileMap {
-		if containIgnorePath(filename) {
-			continue
-		}
 		if filedata.FileMode&os.ModeSetuid != 0 {
 			assesses = append(
 				assesses,
 				&types.Assessment{
-					Type:     types.RemoveSetuidSetgid,
+					Type:     types.CheckSuidGuid,
 					Filename: filename,
-					Desc:     fmt.Sprintf("Found setuid file: %s %s", filename, filedata.FileMode),
+					Desc:     fmt.Sprintf("setuid file: %s %s", filename, filedata.FileMode),
 				})
 		}
 		if filedata.FileMode&os.ModeSetgid != 0 {
 			assesses = append(
 				assesses,
 				&types.Assessment{
-					Type:     types.RemoveSetuidSetgid,
+					Type:     types.CheckSuidGuid,
 					Filename: filename,
-					Desc:     fmt.Sprintf("Found setuid file: %s %s", filename, filedata.FileMode),
+					Desc:     fmt.Sprintf("setgid file: %s %s", filename, filedata.FileMode),
 				})
 		}
 
 	}
 	return assesses, nil
 }
 
-func containIgnorePath(filename string) bool {
-	for _, ignoreDir := range ignorePaths {
-		if strings.Contains(filename, ignoreDir) {
-			return true
-		}
-	}
-	return false
-}
-
 func (a PrivilegeAssessor) RequiredFiles() []string {
 	return []string{}
 }
 
 //const GidMode os.FileMode = 4000
 func (a PrivilegeAssessor) RequiredPermissions() []os.FileMode {
-	return []os.FileMode{os.ModeSocket, os.ModeSetuid}
+	return []os.FileMode{os.ModeSetgid, os.ModeSetuid}
 }

pkg/scanner/scan_test.go

@@ -5,6 +5,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/goodwithtech/dockle/pkg/assessor/contentTrust"
+
+	"github.com/goodwithtech/dockle/pkg/assessor/manifest"
+
 	"github.com/google/go-cmp/cmp/cmpopts"
 
 	deckodertypes "github.com/goodwithtech/deckoder/types"
@@ -24,19 +28,26 @@ func TestScanImage(t *testing.T) {
 		wantErr   error
 		expected  []*types.Assessment
 	}{
-		"test-image": {
-			fileName:  "",
-			imageName: "goodwithtech/test-image:v1",
+		"Dockerfile.base": {
+			fileName: "",
+			// testdata/Dockerfile.base
+			imageName: "goodwithtech/dockle-test:base-test",
 			option:    deckodertypes.DockerOption{Timeout: time.Minute},
 			expected: []*types.Assessment{
-				{Type: types.AvoidEmptyPassword},
-				{Type: types.AvoidRootDefault},
-				{Type: types.AvoidCredentialFile},
-				{Type: types.UseCOPY},
-				{Type: types.AddHealthcheck},
-				{Type: types.MinimizeAptGet},
-				{Type: types.AvoidEnvKeySecret},
-				{Type: types.UseContentTrust},
+				{Type: types.AvoidEmptyPassword, Filename: "etc/shadow"},
+				{Type: types.AvoidRootDefault, Filename: manifest.ConfigFileName},
+				{Type: types.AvoidCredentialFile, Filename: "app/credentials.json"},
+				{Type: types.CheckSuidGuid, Filename: "app/gid.txt"},
+				{Type: types.CheckSuidGuid, Filename: "app/suid.txt"},
+				{Type: types.CheckSuidGuid, Filename: "bin/mount"},
+				{Type: types.CheckSuidGuid, Filename: "bin/su"},
+				{Type: types.CheckSuidGuid, Filename: "bin/umount"},
+				{Type: types.CheckSuidGuid, Filename: "usr/lib/openssh/ssh-keysign"},
+				{Type: types.UseCOPY, Filename: manifest.ConfigFileName},
+				{Type: types.AddHealthcheck, Filename: manifest.ConfigFileName},
+				{Type: types.MinimizeAptGet, Filename: manifest.ConfigFileName},
+				{Type: types.AvoidEnvKeySecret, Filename: manifest.ConfigFileName},
+				{Type: types.UseContentTrust, Filename: contentTrust.HostEnvironmentFileName},
 			},
 		},
 		"emptyArg": {
@@ -50,8 +61,13 @@ func TestScanImage(t *testing.T) {
 		}
 
 		cmpopts := []cmp.Option{
-			cmpopts.SortSlices(func(x, y *types.Assessment) bool { return x.Type < y.Type }),
-			cmpopts.IgnoreTypes(""),
+			cmpopts.SortSlices(func(x, y *types.Assessment) bool {
+				if x.Type == y.Type {
+					return x.Filename < y.Filename
+				}
+				return x.Type < y.Type
+			}),
+			cmpopts.IgnoreFields(types.Assessment{}, "Desc"),
 		}
 		if diff := cmp.Diff(assesses, v.expected, cmpopts...); diff != "" {
 			t.Errorf("%s : tasks diff %v", name, diff)

pkg/scanner/testdata/Dockerfile.base

@@ -0,0 +1,11 @@
+FROM debian:jessie-slim
+
+RUN apt-get update && apt-get install -y git
+RUN useradd nopasswd -p ""
+ADD credentials.json /app/credentials.json
+COPY suid.txt once-suid.txt gid.txt once-gid.txt /app/
+RUN chmod u+s /app/suid.txt /app/once-suid.txt && chmod g+s /app/gid.txt /app/once-gid.txt
+RUN chmod u-s /app/once-suid.txt && chmod g-s /app/once-gid.txt && echo "once" >> /app/once-suid.txt
+ENV MYSQL_PASSWD password
+RUN rm /sbin/unix_chkpwd /usr/bin/*
+VOLUME /usr