Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add vulnerability scanning to cloudbuild_kustomize_image #4264

Closed
wants to merge 1 commit into from

Conversation

yuwenma
Copy link
Contributor

@yuwenma yuwenma commented Nov 1, 2021

Fix Issue #4238 @KnVerey @monopole @natasha41575

To have this vulnerability scanning work, we also need to enable the "On-Demand Scanning API" for GCP project "k8s-staging-kustomize" (I don't have the permission to the project though)

Test: This PR is tested locally via cloud-build-local

Step 1: Add one more step to releasing/cloudbuild_kustomize_image.yaml so as we can check the vulnerability result.

  - id: push
    name: gcr.io/cloud-builders/docker
    entrypoint: /bin/bash
    args:
    - -c
    - |
      docker push gcr.io/$PROJECT_ID/kustomize:${_GIT_TAG}

Step 2: run loud-build-local --config=./releasing/cloudbuild_kustomize_image.yaml --dryrun=false .

Output:

cloud-build-local --config=./releasing/cloudbuild_kustomize_image.yaml  --dryrun=false .
2021/11/01 16:21:36 Warning: The server docker version installed (20.10.5) is different from the one used in GCB (19.03.8)
2021/11/01 16:21:36 Warning: The client docker version installed (20.10.5) is different from the one used in GCB (19.03.8)
Using default tag: latest
latest: Pulling from cloud-builders/metadata
Digest: sha256:a043ab882783fd68d8a7986f2bd8697758055a9ce6c999f91a464ebd036ab0f2
Status: Image is up to date for gcr.io/cloud-builders/metadata:latest
gcr.io/cloud-builders/metadata:latest
2021/11/01 16:21:52 Started spoofed metadata server
2021/11/01 16:21:52 Build id = localbuild_30ab1fbe-2cbc-4923-b4ab-e9194879ba29
2021/11/01 16:21:53 status changed to "BUILD"
BUILD
Starting Step #0
Step #0: Already have image (with digest): bash
Step #0: Cloud build substitution check:  BUILD_ID=localbuild_30ab1fbe-2cbc-4923-b4ab-e9194879ba29 PROJECT_ID=yuwenma-gke-playground _GIT_TAG=v4.0.0-yuwenDirty _PULL_BASE_REF=master _SEVERITY=high
Finished Step #0
2021/11/01 16:21:54 Step Step #0 finished
Starting Step #1
Step #1: Already have image (with digest): gcr.io/cloud-builders/docker
Step #1: #1 [internal] load build definition from kustomize.Dockerfile
Step #1: #1 transferring dockerfile: 642B 0.0s done
Step #1: #1 DONE 0.0s
Step #1: 
Step #1: #2 [internal] load .dockerignore
Step #1: #2 transferring context:
Step #1: #2 transferring context: 94B done
Step #1: #2 DONE 0.0s
Step #1: 
Step #1: #4 [internal] load metadata for docker.io/library/golang:alpine
Step #1: #4 DONE 0.9s
Step #1: 
Step #1: #3 [internal] load metadata for docker.io/library/alpine:latest
Step #1: #3 DONE 0.9s
Step #1: 
Step #1: #5 [stage-1 1/4] FROM docker.io/library/alpine@sha256:e1c082e3d3c45cccac829...
Step #1: #5 DONE 0.0s
Step #1: 
Step #1: #7 [builder 1/5] FROM docker.io/library/golang:alpine@sha256:5519c8752f6b53...
Step #1: #7 CACHED
Step #1: 
Step #1: #9 [internal] load build context
Step #1: #9 ...
Step #1: 
Step #1: #8 [builder 2/5] RUN mkdir /build
Step #1: #8 DONE 0.8s
Step #1: 
Step #1: #9 [internal] load build context
Step #1: #9 transferring context: 103.20MB 4.1s
Step #1: #9 transferring context: 106.77MB 4.3s done
Step #1: #9 DONE 4.3s
Step #1: 
Step #1: #10 [builder 3/5] ADD . /build/
Step #1: #10 DONE 0.7s
Step #1: 
Step #1: #11 [builder 4/5] WORKDIR /build/kustomize
Step #1: #11 DONE 0.0s
Step #1: 
Step #1: #12 [builder 5/5] RUN CGO_ENABLED=0 GO111MODULE=on go build     -ldflags="-s...
Step #1: #12 1.092 go: downloading github.com/spf13/cobra v1.0.0
Step #1: 
Step #1: #12 1.109 go: downloading github.com/spf13/pflag v1.0.5
Step #1: #12 1.114 go: downloading sigs.k8s.io/yaml v1.2.0
Step #1: #12 1.143 go: downloading github.com/pkg/errors v0.9.1
Step #1: #12 1.179 go: downloading github.com/go-errors/errors v1.0.1
Step #1: #12 1.187 go: downloading github.com/olekukonko/tablewriter v0.0.4
Step #1: #12 1.192 go: downloading k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e
Step #1: #12 1.205 go: downloading gopkg.in/yaml.v2 v2.4.0
Step #1: #12 1.209 go: downloading github.com/davecgh/go-spew v1.1.1
Step #1: #12 1.213 go: downloading github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00
Step #1: #12 1.280 go: downloading github.com/stretchr/testify v1.7.0
Step #1: #12 1.284 go: downloading github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca
Step #1: #12 1.295 go: downloading gopkg.in/inf.v0 v0.9.1
Step #1: #12 1.339 go: downloading github.com/evanphx/json-patch v4.11.0+incompatible
Step #1: #12 1.353 go: downloading github.com/imdario/mergo v0.3.5
Step #1: #12 1.374 go: downloading github.com/mattn/go-runewidth v0.0.7
Step #1: #12 1.395 go: downloading go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5
Step #1: #12 1.465 go: downloading github.com/pmezard/go-difflib v1.0.0
Step #1: #12 1.466 go: downloading gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
Step #1: #12 1.475 go: downloading github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
Step #1: #12 1.485 go: downloading github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a
Step #1: #12 1.485 go: downloading github.com/mitchellh/mapstructure v1.1.2
Step #1: #12 1.487 go: downloading github.com/go-openapi/swag v0.19.5
Step #1: #12 1.509 go: downloading github.com/go-openapi/jsonreference v0.19.3
Step #1: #12 1.521 go: downloading github.com/mailru/easyjson v0.7.0
Step #1: #12 1.524 go: downloading github.com/go-openapi/jsonpointer v0.19.3
Step #1: #12 1.524 go: downloading github.com/PuerkitoBio/purell v1.1.1
Step #1: #12 1.543 go: downloading github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578
Step #1: #12 1.544 go: downloading golang.org/x/net v0.0.0-20201110031124-69a78807bb2b
Step #1: #12 1.549 go: downloading golang.org/x/text v0.3.4
Step #1: #12 DONE 19.2s
Step #1: 
Step #1: #6 [stage-1 2/4] RUN apk add --no-cache git openssh
Step #1: #6 CACHED
Step #1: 
Step #1: #13 [stage-1 3/4] COPY --from=builder /build/kustomize/kustomize /app/
Step #1: #13 DONE 0.1s
Step #1: 
Step #1: #14 [stage-1 4/4] WORKDIR /app
Step #1: #14 DONE 0.0s
Step #1: 
Step #1: #15 exporting to image
Step #1: #15 exporting layers 0.1s done
Step #1: #15 writing image sha256:60bb1376cafbd60b49ed76c6969ecd9ebed17ece4dddffbc20dd7ce6a909b168 done
Step #1: #15 naming to gcr.io/yuwenma-gke-playground/kustomize:v4.0.0-yuwenDirty done
Step #1: #15 naming to gcr.io/yuwenma-gke-playground/kustomize:latest done
Step #1: #15 DONE 0.1s
Finished Step #1
2021/11/01 16:22:21 Step Step #1 finished
Starting Step #2 - "scan"
Step #2 - "scan": Already have image (with digest): gcr.io/cloud-builders/gcloud
Step #2 - "scan": Scanning container image
Step #2 - "scan": Locally extracting packages and versions from local container image..........................done
Step #2 - "scan": Remotely initiating analysis of packages and versions.......done
Step #2 - "scan": Waiting for analysis operation to complete......done
Step #2 - "scan": Done.
Finished Step #2 - "scan"
2021/11/01 16:22:27 Step Step #2 - "scan" finished
Starting Step #3 - "severity check"
Step #3 - "severity check": Already have image (with digest): gcr.io/cloud-builders/gcloud
Finished Step #3 - "severity check"
2021/11/01 16:22:29 Step Step #3 - "severity check" finished
Starting Step #4 - "push"
Step #4 - "push": Already have image (with digest): gcr.io/cloud-builders/docker
Step #4 - "push": The push refers to repository [gcr.io/yuwenma-gke-playground/kustomize]
Step #4 - "push": 5f70bf18a086: Preparing
Step #4 - "push": e4161a088bed: Preparing
Step #4 - "push": f2432ab6e3a6: Preparing
Step #4 - "push": e2eb06d8af82: Preparing
Step #4 - "push": 5f70bf18a086: Layer already exists
Step #4 - "push": e2eb06d8af82: Layer already exists
Step #4 - "push": e4161a088bed: Pushed
Step #4 - "push": f2432ab6e3a6: Pushed
Step #4 - "push": v4.0.0-yuwenDirty: digest: sha256:1fad8235211aa5d7883e8e7552e979d2bb701adfe0340de6812332213e3edb51 size: 1156
Finished Step #4 - "push"
2021/11/01 16:22:40 Step Step #4 - "push" finished
2021/11/01 16:22:41 status changed to "DONE"
DONE

Here's the scanning result
Screen Shot 2021-11-01 at 4 23 26 PM

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 1, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @yuwenma. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 1, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yuwenma
To complete the pull request process, please assign justinsb after the PR has been reviewed.
You can assign the PR to them by writing /assign @justinsb in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@natasha41575
Copy link
Contributor

/cc @KnVerey

@k8s-ci-robot k8s-ci-robot requested a review from KnVerey November 1, 2021 23:29
@natasha41575
Copy link
Contributor

natasha41575 commented Nov 1, 2021

To have this vulnerability scanning work, we also need to enable the "On-Demand Scanning API" for GCP project "k8s-staging-kustomize" (I don't have the permission to the project though)

To enable IAM roles on this project, you can make PRs like the following example:

Looks like we can enable roles/ondemandscanning.admin for this. If you need help with this LMK and I can point you to the person who helped me set up our other permissions.

I think a line such as

ensure_project_role_binding "${project}" "${principal}" "roles/ondemandscanning.admin"

in this function should be sufficient. You can either give the permission to the kustomize owners or to the service account that runs the cloud build job. I am not sure which is needed.

@@ -9,7 +9,8 @@ steps:
- "PROJECT_ID=$PROJECT_ID"
- "_GIT_TAG=$_GIT_TAG"
- "_PULL_BASE_REF=$_PULL_BASE_REF"
# We need to use bash to configure the build date and version properly.
- "_SEVERITY=$_SEVERITY"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only want to add this vulnerability check for building kustomize docker image and not in https://github.com/kubernetes-sigs/kustomize/blob/master/releasing/cloudbuild.yaml for the binaries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. This vulnerability check is using gcloud to scan the kustomize image, while releasing/cloudbuild.yaml builds the kustomize binary. I don't think cloudbuild provides officially solutions to scan go source code (and to implement our own seems an overkill). I'd prefer just scanning the image based on what the current cloudbuild has.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we scanning the image, not the binary, is this going to tell us anything about Kustomize itself? Or is it just going to tell us we need to bump the version of alpine we're adding the binary to? I'm surprised to hear that given that the explanation of this feature in #4238 was to enhance Kustomize security. We don't have data on this, but I expect a minority of our users are running the image.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"enhance Kustomize security" -> base image and the kustomize code are treated as a whole, right?

Copy link
Contributor Author

@yuwenma yuwenma Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm originally thinking of both (binary and image).

For kustomize binary, if we don't provide LTS kustomize version, I'm not bothered to setup a vulnerability check. If we are interested in current vulnerability status of kustomize, here's what I found:

https://github.com/securego/gosec But I think it has two potential issues.

  1. It uses Apache 2.0 license. So I doubt if we really want to add it to the kustomize source code (so that we can have a "gosec ./... " check added to the kustomize presubmit make rules like verify-kustomize) , or merely install gosec separately and only run in local development.
  2. I also did a gosec check against kustomize HEAD. Here's what I get so far. No high severity issues. But if we do want to enable gosec, either fixing some of these issues or exclude some checks via flag --exclude=.
[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/internal/plugins/compiler/compiler.go:80] - G204 (CWE-78): Subprocess launched with variable (Confidence: HIGH, Severity: MEDIUM)
    79: 	}
  > 80: 	cmd := exec.Command(goBin, commands...)
    81: 	b.stderr.Reset()



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go:145] - G204 (CWE-78): Subprocess launched with a potential tainted input or cmd arguments (Confidence: HIGH, Severity: MEDIUM)
    144: 	stderr := new(bytes.Buffer)
  > 145: 	cmd := exec.Command(p.h.GeneralConfig().HelmConfig.Command, args...)
    146: 	cmd.Stdout = stdout



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/fn/runtime/exec/exec.go:31] - G204 (CWE-78): Subprocess launched with a potential tainted input or cmd arguments (Confidence: HIGH, Severity: MEDIUM)
    30: func (c *Filter) Run(reader io.Reader, writer io.Writer) error {
  > 31: 	cmd := exec.Command(c.Path, c.Args...)
    32: 	cmd.Stdin = reader



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/gorepomod/internal/edit/editor.go:27-29] - G204 (CWE-78): Subprocess launched with a potential tainted input or cmd arguments (Confidence: HIGH, Severity: MEDIUM)
    26: func (e *Editor) run(args ...string) error {
  > 27: 	c := exec.Command(
  > 28: 		"go",
  > 29: 		append([]string{"mod"}, args...)...)
    30: 	c.Dir = string(e.module.ShortName())



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/config/internal/commands/cmdxargs.go:102] - G204 (CWE-78): Subprocess launched with a potential tainted input or cmd arguments (Confidence: HIGH, Severity: MEDIUM)
    101: 	r.Args = r.Args[cmdIndex:]
  > 102: 	run := exec.Command(r.Args[0])
    103: 



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/internal/plugins/execplugin/execplugin.go:169-170] - G204 (CWE-78): Subprocess launched with a potential tainted input or cmd arguments (Confidence: HIGH, Severity: MEDIUM)
    168: 	//nolint:gosec
  > 169: 	cmd := exec.Command(
  > 170: 		p.path, append([]string{f.Name()}, p.args...)...)
    171: 	cmd.Env = p.getEnv()



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/builtins/HelmChartInflationGenerator.go:140] - G204 (CWE-78): Subprocess launched with a potential tainted input or cmd arguments (Confidence: HIGH, Severity: MEDIUM)
    139: 	stderr := new(bytes.Buffer)
  > 140: 	cmd := exec.Command(p.h.GeneralConfig().HelmConfig.Command, args...)
    141: 	cmd.Stdout = stdout



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/yaml/rnode.go:55] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    54: func ReadFile(path string) (*RNode, error) {
  > 55: 	b, err := ioutil.ReadFile(path)
    56: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/openapi/openapi.go:125] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    124: func parseOpenAPI(openAPIPath string) (*yaml.RNode, error) {
  > 125: 	b, err := ioutil.ReadFile(openAPIPath)
    126: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/kio/pkgio_writer.go:91] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    90: 
  > 91: 		f, err := os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.FileMode(0600))
    92: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/kio/pkgio_reader.go:266] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    265: func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode, error) {
  > 266: 	f, err := os.Open(path)
    267: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/fn/framework/command/command.go:141] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    140: func functionConfigFromFile(file string) (*yaml.RNode, error) {
  > 141: 	b, err := ioutil.ReadFile(file)
    142: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/filesys/fsondisk.go:117] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    116: // ReadFile delegates to ioutil.ReadFile.
  > 117: func (fsOnDisk) ReadFile(name string) ([]byte, error) { return ioutil.ReadFile(name) }
    118: 



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/filesys/fsondisk.go:43] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    42: // Open delegates to os.Open.
  > 43: func (fsOnDisk) Open(name string) (File, error) { return os.Open(name) }
    44: 



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/copyutil/copyutil.go:165] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    164: 
  > 165: 	input, err := ioutil.ReadFile(src)
    166: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/copyutil/copyutil.go:119] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    118: 		}
  > 119: 		b2, err := ioutil.ReadFile(filepath.Join(sourceDir, f))
    120: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/copyutil/copyutil.go:115] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    114: 		// compare upstreamFiles
  > 115: 		b1, err := ioutil.ReadFile(filepath.Join(destDir, f))
    116: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/copyutil/copyutil.go:44] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    43: 		// copy file by reading and writing it
  > 44: 		b, err := ioutil.ReadFile(filepath.Join(src, copyTo))
    45: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/pluginator/internal/krmfunction/converter.go:148] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    147: 	p := c.outputDir
  > 148: 	f, err := os.Open(p)
    149: 	if err == nil || f != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/pluginator/internal/krmfunction/converter.go:139] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    138: func (c *Converter) readDiskFile(path string) (string, error) {
  > 139: 	f, err := ioutil.ReadFile(path)
    140: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/pluginator/internal/builtinplugin/builtinplugin.go:35] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    34: 	}
  > 35: 	file, err := os.Open(root + ".go")
    36: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/mdtogo/main.go:99] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    98: 	} else {
  > 99: 		b, err := ioutil.ReadFile(licenseFile)
    100: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/mdtogo/main.go:82] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    81: 		}
  > 82: 		b, err := ioutil.ReadFile(filepath.Join(source, f.Name()))
    83: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/k8scopy/internal/modulespec.go:28] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    27: func ReadSpec(fileName string) *ModuleSpec {
  > 28: 	bytes, err := ioutil.ReadFile(fileName)
    29: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/k8scopy/internal/copier.go:75-76] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    74: func (c Copier) CopyFile(dir, fName string) error {
  > 75: 	inFile, err := os.Open(
  > 76: 		filepath.Join(c.goModCache, c.spec.Name(), dir, fName))
    77: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/gorepomod/internal/repo/protomodule.go:68] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    67: 	mPath := filepath.Join(path, goModFile)
  > 68: 	content, err := ioutil.ReadFile(mPath)
    69: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/config/internal/commands/cmdcreatesetter.go:214] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
    213: 	}
  > 214: 	sch, err := ioutil.ReadFile(schemaPath)
    215: 	if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/openapi/kustomizationapi/swagger.go:28] - G110 (CWE-409): Potential DoS vulnerability via decompression bomb (Confidence: MEDIUM, Severity: MEDIUM)
    27: 	var buf bytes.Buffer
  > 28: 	_, err = io.Copy(&buf, gz)
    29: 	clErr := gz.Close()



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/openapi/kubernetesapi/v1204/swagger.go:28] - G110 (CWE-409): Potential DoS vulnerability via decompression bomb (Confidence: MEDIUM, Severity: MEDIUM)
    27: 	var buf bytes.Buffer
  > 28: 	_, err = io.Copy(&buf, gz)
    29: 	clErr := gz.Close()



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go:68] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    67: 			// TODO: move this test into the filter.
  > 68: 			if smellsLikeANameChange(&fs) {
    69: 				// "metadata/name" is the only field.



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/types/helmchartargs.go:100] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    99: 	for _, old := range oldArgs {
  > 100: 		charts = append(charts, makeHelmChartFromHca(&old))
    101: 		if old.HelmHome != "" {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/resmap/factory.go:107] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    106: 	for _, args := range argsList {
  > 107: 		res, err := rmF.resF.MakeSecret(kvLdr, &args)
    108: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/resmap/factory.go:82] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    81: 	for _, args := range argList {
  > 82: 		res, err := rmF.resF.MakeConfigMap(kvLdr, &args)
    83: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/filters/replacement/replacement.go:52] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    51: 			for _, id := range ids {
  > 52: 				if id.IsSelectedBy(t.Select.ResId) && !rejectId(t.Reject, &id) {
    53: 					err := applyToNode(n, value, t)



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/filters/replacement/replacement.go:26] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    25: 		}
  > 26: 		value, err := getReplacement(nodes, &r)
    27: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/builtins/PrefixSuffixTransformer.go:64] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    63: 			// TODO: move this test into the filter.
  > 64: 			if smellsLikeANameChange(&fs) {
    65: 				// "metadata/name" is the only field.



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/pluginator/internal/krmfunction/converter.go:153] - G301 (CWE-276): Expect directory permissions to be 0750 or less (Confidence: HIGH, Severity: MEDIUM)
    152: 
  > 153: 	return os.MkdirAll(p, 0755)
    154: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/k8scopy/internal/writer.go:19] - G301 (CWE-276): Expect directory permissions to be 0750 or less (Confidence: HIGH, Severity: MEDIUM)
    18: func newWriter(toDir, name string) (*writer, error) {
  > 19: 	if err := os.MkdirAll(toDir, 0755); err != nil {
    20: 		log.Printf("unable to create directory: %s", toDir)



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go:219] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    218: 	path := filepath.Join(p.tmpDir, p.Name+"-kustomize-values.yaml")
  > 219: 	return path, ioutil.WriteFile(path, b, 0644)
    220: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/filesys/fsondisk.go:121] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    120: func (fsOnDisk) WriteFile(name string, c []byte) error {
  > 121: 	return ioutil.WriteFile(name, c, 0666)
    122: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/pluginator/internal/krmfunction/converter.go:159] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    158: 		p := filepath.Join(c.outputDir, k)
  > 159: 		err := ioutil.WriteFile(p, []byte(v), 0644)
    160: 		if err != nil {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/builtins/HelmChartInflationGenerator.go:214] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
    213: 	path := filepath.Join(p.tmpDir, p.Name+"-kustomize-values.yaml")
  > 214: 	return path, ioutil.WriteFile(path, b, 0644)
    215: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/kio/pkgio_writer.go:96] - G307 (CWE-703): Deferring unsafe method "Close" on type "*os.File" (Confidence: HIGH, Severity: MEDIUM)
    95: 		if err := func() error {
  > 96: 			defer f.Close()
    97: 			w := ByteWriter{



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/kio/pkgio_reader.go:270] - G307 (CWE-703): Deferring unsafe method "Close" on type "*os.File" (Confidence: HIGH, Severity: MEDIUM)
    269: 	}
  > 270: 	defer f.Close()
    271: 



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/pluginator/internal/builtinplugin/builtinplugin.go:39] - G307 (CWE-703): Deferring unsafe method "Close" on type "*os.File" (Confidence: HIGH, Severity: MEDIUM)
    38: 	}
  > 39: 	defer file.Close()
    40: 	scanner := bufio.NewScanner(file)



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/k8scopy/internal/copier.go:80] - G307 (CWE-703): Deferring unsafe method "Close" on type "*os.File" (Confidence: HIGH, Severity: MEDIUM)
    79: 	}
  > 80: 	defer inFile.Close()
    81: 	scanner := bufio.NewScanner(inFile)



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/gorepomod/internal/gen/main.go:21] - G307 (CWE-703): Deferring unsafe method "Close" on type "*os.File" (Confidence: HIGH, Severity: MEDIUM)
    20: 	}
  > 21: 	defer inFile.Close()
    22: 	scanner := bufio.NewScanner(inFile)



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/config/internal/commands/cat.go:89] - G307 (CWE-703): Deferring unsafe method "Close" on type "*os.File" (Confidence: HIGH, Severity: MEDIUM)
    88: 		}
  > 89: 		defer o.Close()
    90: 		writer = o



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go:224] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    223: 	if p.tmpDir != "" {
  > 224: 		os.RemoveAll(p.tmpDir)
    225: 	}



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/yaml/rnode.go:408] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    407: func (rn *RNode) SetApiVersion(av string) {
  > 408: 	rn.SetMapField(NewScalarRNode(av), APIVersionField)
    409: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/yaml/rnode.go:395] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    394: func (rn *RNode) SetKind(k string) {
  > 395: 	rn.SetMapField(NewScalarRNode(k), KindField)
    396: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/kio/testing.go:54] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    53: func (s Setup) Clean() {
  > 54: 	os.RemoveAll(s.Root)
    55: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/filesys/fsnode.go:557-570] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    556: func (n *fsNode) DebugPrint() {
  > 557: 	n.WalkMe(func(path string, info os.FileInfo, err error) error {
  > 558: 		if err != nil {
  > 559: 			fmt.Printf("err '%v' at path %q\n", err, path)
  > 560: 			return nil
  > 561: 		}
  > 562: 		if info.IsDir() {
  > 563: 			if info.Size() == 0 {
  > 564: 				fmt.Println("empty dir: " + path)
  > 565: 			}
  > 566: 		} else {
  > 567: 			fmt.Println("     file: " + path)
  > 568: 		}
  > 569: 		return nil
  > 570: 	})
    571: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kyaml/filesys/fsnode.go:544-552] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    543: 	count := 0
  > 544: 	n.WalkMe(func(path string, info os.FileInfo, err error) error {
  > 545: 		if err != nil {
  > 546: 			return err
  > 547: 		}
  > 548: 		if !info.IsDir() {
  > 549: 			count++
  > 550: 		}
  > 551: 		return nil
  > 552: 	})
    553: 	return count



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kustomize/commands/openapi/fetch/fetch.go:60] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    59: 	output := stdout.Bytes()
  > 60: 	json.Unmarshal(output, &jsonSchema)
    61: 	output, _ = json.MarshalIndent(jsonSchema, "", "  ")



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/kustomize/commands/commands.go:63] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    62: 	// https://github.com/kubernetes/kubernetes/issues/17162
  > 63: 	flag.CommandLine.Parse([]string{})
    64: 	return c



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/k8scopy/internal/writer.go:32] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    31: func (w *writer) close() {
  > 32: 	w.f.Close()
    33: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/cmd/config/kubectl-krm/main.go:16] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    15: func main() {
  > 16: 	os.Setenv(commandutil.EnableAlphaCommmandsEnvName, "true")
    17: 	cmd := configcobra.AddCommands(&cobra.Command{



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/testutils/kusttest/plugintestenv.go:78] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    77: 	if x.wasSet {
  > 78: 		os.Setenv(konfig.KustomizePluginHomeEnv, x.oldXdg)
    79: 	} else {



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/testutils/kusttest/plugintestenv.go:73] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    72: 	x.oldXdg, x.wasSet = os.LookupEnv(konfig.KustomizePluginHomeEnv)
  > 73: 	os.Setenv(konfig.KustomizePluginHomeEnv, x.pluginRoot)
    74: }



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/testutils/kusttest/harnessenhanced.go:112] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    111: 		}
  > 112: 		os.RemoveAll(th.ldr.Root())
    113: 	}



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/internal/target/kusttarget.go:79] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    78: 	b, _ := json.Marshal(*kt.kustomization)
  > 79: 	json.Unmarshal(b, &result)
    80: 	return result



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/internal/generators/secret.go:57] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    56: 	copyLabelsAndAnnotations(rn, args.Options)
  > 57: 	setImmutable(rn, args.Options)
    58: 	return rn, nil



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/internal/generators/secret.go:56] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    55: 	}
  > 56: 	copyLabelsAndAnnotations(rn, args.Options)
    57: 	setImmutable(rn, args.Options)



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/hasher/hasher.go:97] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    96: 			var v map[string]interface{}
  > 97: 			json.Unmarshal(vs, &v)
    98: 			values[p] = v



[/Users/yuwenma/go/src/github.com/kubernetes-sigs/kustomize/api/builtins/HelmChartInflationGenerator.go:219] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    218: 	if p.tmpDir != "" {
  > 219: 		os.RemoveAll(p.tmpDir)
    220: 	}



Summary:
  Gosec  : dev
  Files  : 439
  Lines  : 51161
  Nosec  : 0
  Issues : 66

For kustomize image, it is officially managed by sig-k8s-infra. either to "bump the version of alpine" or fix other CVE issue the process would be much easier since it does not require rebuilding the yaml/api dependencies or even the kustomize CLI. So IIUC it would be a simplified release. Do you have any additional concerns?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, BTW have we considered to rebase the alpine to distroless-image?

@yuwenma
Copy link
Contributor Author

yuwenma commented Nov 2, 2021

To have this vulnerability scanning work, we also need to enable the "On-Demand Scanning API" for GCP project "k8s-staging-kustomize" (I don't have the permission to the project though)

To enable IAM roles on this project, you can make PRs like the following example:

Looks like we can enable roles/ondemandscanning.admin for this. If you need help with this LMK and I can point you to the person who helped me set up our other permissions.

I think a line such as

ensure_project_role_binding "${project}" "${principal}" "roles/ondemandscanning.admin"

in this function should be sufficient. You can either give the permission to the kustomize owners or to the service account that runs the cloud build job. I am not sure which is needed.

Thank you! Here's the PR kubernetes/k8s.io#3017

@yuwenma yuwenma requested a review from natasha41575 November 2, 2021 19:06
@yuwenma
Copy link
Contributor Author

yuwenma commented Nov 3, 2021

/ok-to-test

@k8s-ci-robot
Copy link
Contributor

@yuwenma: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@natasha41575
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 3, 2021
@yuwenma yuwenma closed this Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants