Skip to content

Commit

Permalink
feat(veinmind-iac): add dockerfile security detection (#228)
Browse files Browse the repository at this point in the history
* feat(plugins): add ftp protocol support in plugin
docs(plugins): fix Readme table bug

* docs(plugins): add ftp protocol support in plugin

* feat(plugins): update dependency version

* feat(plugins): add ftp protocol support in weekpass plugin

* Refactor(plugins-weakpass): change Mod name to Service name mapping from one-to-one to one-to-many

* Test(plugins-weakpass): Add unit tests for ftp service

* Test(plugins-weakpass): Add unit tests for ftp service

* feat(veinmind-iac): add dockerfile security detection

* test(veinmind-iac): add dockerfile security detection tests

* fix(veinmind-iac): fix format bug

* Test(veinmind-iac): Add unit tests for dockerfile detection
  • Loading branch information
ek1ng authored Apr 25, 2023
1 parent abe6140 commit 2f636ea
Show file tree
Hide file tree
Showing 83 changed files with 1,058 additions and 3 deletions.
16 changes: 16 additions & 0 deletions .github/workflows/veinmind-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,19 @@ jobs:
cd ./plugins/go/veinmind-weakpass && go mod tidy
cd hash && go test -tags dynamic
cd ../service && go test -tags dynamic
test-veinmind-iac:
runs-on: ubuntu-20.04
env:
CI_GOOS: linux
CI_GOARCH: amd64
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: '1.18'
- run: |
sudo make install
cd ./plugins/go/veinmind-iac && go mod tidy
cd ./rules/test && go test
4 changes: 4 additions & 0 deletions plugins/go/veinmind-iac/pkg/scanner/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func (bs *Scanner) Scan(ctx context.Context, iacFile api.IAC) ([]Result, error)

// parse
input, err := parseHandle(file, iacFile.Path)
//jsonBytes, _ := json.Marshal(input)
//jsonString := string(jsonBytes)
//
//fmt.Println(jsonString)
if err != nil {
return nil, err
}
Expand Down
156 changes: 155 additions & 1 deletion plugins/go/veinmind-iac/rules/common/docker.rego
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ meta_data["DF-009"] := {
"reference": "",
}


meta_data["DF-010"] := {
"id": "DF-010",
"name": "use COPY instead of ADD",
Expand Down Expand Up @@ -140,4 +139,159 @@ meta_data["DF-014"] := {
"description": "Avoid using 'sudo' in the 'RUN' instruction, which may lead to unexpected results",
"solution": "Avoid using sudo",
"reference": "https://docs.docker.com/engine/reference/builder/#run",
}

meta_data["DF-015"] := {
"id": "DF-015",
"name": "add with parent directory",
"type": "dockerfile",
"severity": "Low",
"description": "Avoid using ADD with parent directory",
"solution": "Change path to absolute path",
"reference": "https://docs.docker.com/engine/reference/builder/#add",
}

meta_data["DF-016"] := {
"id": "DF-016",
"name": "Multiple CMD commands are used",
"type": "dockerfile",
"severity": "CRITICAL",
"description": "There can only be one CMD command in a Docker file. If you list multiple CMDs, only the last one will take effect.",
"solution": "Delete other cmd commands and use only one cmd command",
"reference": "https://docs.docker.com/engine/reference/builder/#cmd",
}

meta_data["DF-017"] := {
"id": "DF-017",
"name": "Multiple entrypoint commands are used",
"type": "dockerfile",
"severity": "CRITICAL",
"description": "There can only be one entrypont command in a Docker file. If you list multiple entrypoints, only the last one will take effect.",
"solution": "Delete other entrypoint commands and use only one entrypoint command",
"reference": "https://docs.docker.com/engine/reference/builder/#entrypoint",
}
meta_data["DF-018"] := {
"id": "DF-018",
"name": "Duplicate aliases defined in different FROMs",
"type": "dockerfile",
"severity": "CRITICAL",
"description": "Different FROMs cannot have the same alias definition.",
"solution": "Change the alias to be different",
"reference": "https://docs.docker.com/develop/develop-images/multistage-build/",
}
meta_data["DF-019"] := {
"id": "DF-019",
"name": "`COPY --from` comes from the current image",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "COPY --from` should not use the from alias of the current mirror, as it cannot copy from itself.",
"solution": "Change `--from` so it doesn't refer to itself",
"reference": "https://docs.docker.com/develop/develop-images/multistage-build/",
}
meta_data["DF-020"] := {
"id": "DF-020",
"name": "`COPY --from` does not use the `--link` argument",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "Use the `--link` parameter to avoid the need to rebuild the intermediate stages when the build fails. `--link` will reuse the previously generated layer and merge it on top of the new layer. When the base image receives an update, you can Easily rebase an image without having to do the entire build again.",
"solution": "add --link arg",
"reference": " https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#leverage-build-cache",
}
meta_data["DF-021"] := {
"id": "DF-021",
"name": "Using `FROM` specific `ARG` variable",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "The `ARG` variable before `FROM` only applies to `FROM`, and cannot be obtained in subsequent instructions. To use it, you need to redefine `ARG`",
"solution": "Please redefine `ARG` after `FROM`",
"reference": "https://docs.docker.com/engine/reference/builder/#understand-how-arg-and-from-interact",
}
meta_data["DF-022"] := {
"id": "DF-022",
"name": "Use of deprecated directive MAINTAINER",
"type": "dockerfile",
"severity": "High",
"description": "MAINTAINER is deprecated since Docker 1.13.0.",
"solution": "Use LABEL instead of MAINTAINER",
"reference": "https://docs.docker.com/engine/deprecated/#maintainer-in-dockerfile",
}

meta_data["DF-023"] := {
"id": "DF-023",
"name": "Complicated `RUN cd` syntax is used",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "You should use `WORKDIR` instead of a plethora of instructions like `RUN cd…`, which are difficult to read, troubleshoot and maintain",
"solution": "use `WORKDIR` instead of complicated `RUN cd` switching working directory",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#workdir",
}

meta_data["DF-024"] := {
"id": "DF-024",
"name": "Used `RUN <package-manager> update` alone",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "The directive `RUN <package-manager> update` should always be followed by `<package-manager> install` within the same RUN statement.",
"solution": "Merge `<packagemanager>update` and `<package manager> install` directives into one directive",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run",
}

meta_data["DF-025"] := {
"id": "DF-025",
"name": "RUN <package-manager> install` does not use the `-y` parameter",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "The `-y` parameter should be used after `RUN <package-manager> install` to avoid manual input.",
"solution": "Add `-y` such as: `yum install -y`, `apt-get install -y`, ``",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run",
}

meta_data["DF-026"] := {
"id": "DF-026",
"name": "Didn't clean up after using `RUN apk add`",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "Cleanup command should be used after `RUN apk add` to clear package cache data and reduce image size.",
"solution": "Add a cleanup command such as: `apk add --no-cache`",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run",
}

meta_data["DF-027"] := {
"id": "DF-027",
"name": "Didn't clean up after using `RUN dnf install`",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "Cleanup command should be used after `RUN dnf install` to clear package cache data and reduce image size.",
"solution": "Add clean command like: `dnf clean all`",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run",
}

meta_data["DF-028"] := {
"id": "DF-028",
"name": "Didn't clean up after using `RUN yum install`",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "Cleanup command should be used after `RUN yum install` to clear package cache data and reduce image size.",
"solution": "Add cleaning commands such as: `yum clean all`",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run",
}

meta_data["DF-029"] := {
"id": "DF-029",
"name": "Didn't clean up after using `RUN zypper in/remove/source-install`",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "Cleanup command should be used after `RUN apk add` to clear package cache data and reduce image size.",
"solution": "Add cleaning commands such as: `zypper clean` or `zypper cc`",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run",
}

meta_data["DF-030"] := {
"id": "DF-030",
"name": "RUN useradd` needs to add `--no-log-init` parameter",
"type": "dockerfile",
"severity": "MEDIUM",
"description": "Due to an unresolved bug in the Go archive/tar package's handling of sparse files, attempting to create a user with a very large UID in a Docker container could lead to disk exhaustion as /var/log/faillog in the container layer fills up Empty characters. The workaround is to pass the --no-log-init flag to useradd. The Debian/Ubuntu adduser wrapper does not support this flag.",
"solution": "Use the `--no-log-init` parameter in `useradd`",
"reference": "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run",
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package brightMirror.dockerfile

import future.keywords.in
import data.common

risks[res]{
inner:= input[_]
inner.Cmd=="add"
some val in inner.Value
contains(val,"../")
res := common.result(inner,"DF-015")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package brightMirror.dockerfile

import data.common

get_apk[output] {
run := input[_]
run.Cmd=="run"
arg := run.Value[0]
regex.match("apk (-[a-zA-Z]+\\s*)*add", arg)
not contains_no_cache(arg)
output=run
}

risks[res] {
output := get_apk[_]
res:=common.result(output, "DF-026")
}

contains_no_cache(cmd) {
split(cmd, " ")[_] == "--no-cache"
}
15 changes: 15 additions & 0 deletions plugins/go/veinmind-iac/rules/dockerfile/cmd_multiple.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package brightMirror.dockerfile

import data.common

risks[res] {
count(get_cmd) > 1
obj := [cmd | cmd := get_cmd[_]; true]
res := common.result(obj[1], "DF-016")
}

get_cmd[inner] {
inner := input[_]
inner.Cmd == "cmd"
}

29 changes: 29 additions & 0 deletions plugins/go/veinmind-iac/rules/dockerfile/copy_from_self.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package brightMirror.dockerfile

import data.common

find_alias_from_copy[out]{
inner:=input[_]
inner.Cmd=="copy"
flags:=inner.Flags[_]
contains(flags,"from")
parts := split(flags, "=")
is_equal(inner.Stage,parts[1])
out:=inner
}

is_equal(stage,alias)=allow{
inner:=input[_]
inner.Stage==stage
inner.Cmd="from"
val:=inner.Value
val[i]="as"
current_alias:=val[i+1]
current_alias==alias
allow=true
}

risks[res]{
out:=find_alias_from_copy[_]
res:= common.result(out, "DF-019")
}
13 changes: 13 additions & 0 deletions plugins/go/veinmind-iac/rules/dockerfile/copy_missing_link.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package brightMirror.dockerfile

import future.keywords.every
import data.common

risks[res] {
inner := input[_]
inner.Cmd == "copy"
every flag in inner.Flags{
flag !="--link"
}
res := common.result(inner, "DF-020")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package brightMirror.dockerfile

import data.common

dnf_install_regex := `(dnf install)|(dnf in)|(dnf reinstall)|(dnf rei)|(dnf install-n)|(dnf install-na)|(dnf install-nevra)`
dnf_regex = sprintf("(%s).*dnf.*clean.*all", [dnf_install_regex])

get_dnf[output] {
run:=input[_]
run.Cmd=="run"
arg := run.Value[0]

regex.match(dnf_install_regex, arg)

not contains_clean_after_dnf(arg)
output := run
}

risks[res] {
output := get_dnf[_]
res:=common.result(output, "DF-027")
}

contains_clean_after_dnf(cmd) {
regex.match(dnf_regex, cmd)
}
15 changes: 15 additions & 0 deletions plugins/go/veinmind-iac/rules/dockerfile/entrypoint_multiple.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package brightMirror.dockerfile

import future.keywords.in
import data.common

risks[res] {
count(get_entrypoint) > 1
obj := [entrypoint | entrypoint := get_entrypoint[_]; true]
res := common.result(obj[1], "DF-017")
}

get_entrypoint[inner] {
inner := input[_]
inner.Cmd == "entrypoint"
}
31 changes: 31 additions & 0 deletions plugins/go/veinmind-iac/rules/dockerfile/from_alias_multiple.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package brightMirror.dockerfile

import future.keywords.in
import data.common

get_aliased_name[output] {
inner:=input[_]
inner.Cmd=="from"
value:=inner.Value
value[i]=="as"
output={
"cmd":inner,
"startLine":inner.StartLine,
"alias":value[i+1]
}
}

checkDuplicate[output]{
alias1:=get_aliased_name[_]
alias2:=get_aliased_name[_]
alias1.startLine!=alias2.startLine
alias1.alias==alias2.alias
output:=alias1.cmd
}

risks[res]{
count(checkDuplicate)>1
finalResults:=[finalResult|finalResult:=checkDuplicate[_]]
index:=count(finalResults)-1
res:=common.result(finalResults[index],"DF-018")
}
Loading

0 comments on commit 2f636ea

Please sign in to comment.