From e5d05a2fc212901e32ceaac6c7959b1e3fa6b13a Mon Sep 17 00:00:00 2001 From: Tomoya Amachi Date: Sun, 3 Nov 2019 16:32:22 +0900 Subject: [PATCH] check permissions in any dir (#57) * check permissions in any dir * check all setuid, setgid & fix test --- CHECKPOINT.md | 2 +- README.md | 2 +- go.mod | 2 +- go.sum | 4 +-- pkg/assessor/contentTrust/contentTrust.go | 4 ++- pkg/assessor/manifest/manifest.go | 29 +++++++++------- pkg/assessor/manifest/manifest_test.go | 12 +++---- pkg/assessor/privilege/suid.go | 25 +++----------- pkg/scanner/scan_test.go | 42 ++++++++++++++++------- pkg/scanner/testdata/Dockerfile.base | 11 ++++++ pkg/types/checkpoint.go | 7 ++-- 11 files changed, 79 insertions(+), 61 deletions(-) create mode 100644 pkg/scanner/testdata/Dockerfile.base diff --git a/CHECKPOINT.md b/CHECKPOINT.md index 10e1929..f616e79 100644 --- a/CHECKPOINT.md +++ b/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.
> `setuid` and `setgid` permissions could be used for elevating privileges. diff --git a/README.md b/README.md index b90c5bf..4313f38 100644 --- a/README.md +++ b/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 diff --git a/go.mod b/go.mod index 09643df..88e49b2 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index dbc9a01..baeee93 100644 --- a/go.sum +++ b/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= diff --git a/pkg/assessor/contentTrust/contentTrust.go b/pkg/assessor/contentTrust/contentTrust.go index ab9f235..f3d10ae 100644 --- a/pkg/assessor/contentTrust/contentTrust.go +++ b/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 diff --git a/pkg/assessor/manifest/manifest.go b/pkg/assessor/manifest/manifest.go index 76c5bad..659c34d 100644 --- a/pkg/assessor/manifest/manifest.go +++ b/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,7 +128,7 @@ 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), }) } @@ -133,7 +136,7 @@ func assessHistory(index int, cmd types.History) []*types.Assessment { 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), }) } @@ -141,7 +144,7 @@ func assessHistory(index int, cmd types.History) []*types.Assessment { 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), }) } @@ -149,7 +152,7 @@ func assessHistory(index int, cmd types.History) []*types.Assessment { 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), }) } @@ -157,14 +160,14 @@ func assessHistory(index int, cmd types.History) []*types.Assessment { 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), }) } diff --git a/pkg/assessor/manifest/manifest_test.go b/pkg/assessor/manifest/manifest_test.go index f84bb09..ffdcb65 100644 --- a/pkg/assessor/manifest/manifest_test.go +++ b/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, }, }, }, diff --git a/pkg/assessor/privilege/suid.go b/pkg/assessor/privilege/suid.go index e3419ed..4595cc8 100644 --- a/pkg/assessor/privilege/suid.go +++ b/pkg/assessor/privilege/suid.go @@ -3,7 +3,6 @@ package privilege import ( "fmt" "os" - "strings" "github.com/goodwithtech/deckoder/extractor" "github.com/goodwithtech/dockle/pkg/types" @@ -11,31 +10,26 @@ import ( 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), }) } @@ -43,20 +37,11 @@ func (a PrivilegeAssessor) Assess(fileMap extractor.FileMap) ([]*types.Assessmen 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} } diff --git a/pkg/scanner/scan_test.go b/pkg/scanner/scan_test.go index 0b2e0d1..5e1b0c3 100644 --- a/pkg/scanner/scan_test.go +++ b/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) diff --git a/pkg/scanner/testdata/Dockerfile.base b/pkg/scanner/testdata/Dockerfile.base new file mode 100644 index 0000000..2410af5 --- /dev/null +++ b/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 diff --git a/pkg/types/checkpoint.go b/pkg/types/checkpoint.go index 15bc183..cc4677d 100644 --- a/pkg/types/checkpoint.go +++ b/pkg/types/checkpoint.go @@ -9,7 +9,7 @@ const ( AddHealthcheck UseAptGetUpdateNoCache - RemoveSetuidSetgid + CheckSuidGuid UseCOPY AvoidEnvKeySecret AvoidCredentialFile @@ -71,11 +71,12 @@ var AlertDetails = map[int]AlertDetail{ Code: "CIS-DI-0007", }, - RemoveSetuidSetgid: { + CheckSuidGuid: { DefaultLevel: InfoLevel, - Title: "Remove setuid and setgid permissions in the images", + Title: "Confirm safety of setuid/setgid files", Code: "CIS-DI-0008", }, + UseCOPY: { DefaultLevel: FatalLevel, Title: "Use COPY instead of ADD in Dockerfile",