Skip to content

Conversation

oliver-goetz
Copy link
Member

How to categorize this PR?

/area compliance security
/kind enhancement

What this PR does / why we need it:
This PR introduces gosec for Static Application Security Testing at Gardener and should replace other code scanners.

It uses the default ruleset of gosec with two exceptions.

  1. Scanning generated code is disabled. The Kubernetes code-gen creates hundreds of low priority findings for unsafe pointers (G103). The generated code has proven that it is working and it is not possible to address those findings.
  2. The ./hack folder is excluded, because gosec does not support nested go modules (like logcheck) and there is no productive code in this folder anyway.

All findings based on these settings have been mitigated in this PR.

There are two new make targets.

  • make sast is supposed to be called in unit tests. It logs all finding to console and fails in case of errors.
  • make sast-report can be used in build pipelines. It fails on errors too. Additionally, it creates a report in sarif format to ./gosec-report.sarif file. This report also tracks suppressed findings.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
cc @ccwienk @ThormaehlenFred @dkistner

Release note:

`gosec` was introduced for Static Application Security Testing (SAST).

G101: Look for hard coded credentials
G404: Insecure random number source (rand)
G109: Potential Integer overflow made by strconv.Atoi result conversion to int16/32
G402: Look for bad TLS connection settings
G301: Poor file permissions used when creating a directory
G306: Poor file permissions used when writing to a new file
G304: File path provided as taint input
G204: Audit use of command execution
G401: Detect the usage of DES, RC4, MD5 or SHA1
G505: Import blocklist: crypto/sha1
@gardener-prow gardener-prow bot added area/compliance Compliance related area/security Security related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 11, 2024
@gardener-prow gardener-prow bot requested review from acumino and ialidzhikov June 11, 2024 21:43
Copy link
Member

@LucaBernstein LucaBernstein left a comment

Choose a reason for hiding this comment

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

Really nice, thanks!
Just one small comment 😉

@timuthy
Copy link
Member

timuthy commented Jun 12, 2024

/assign

Copy link
Member

@LucaBernstein LucaBernstein left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2024
Copy link
Contributor

gardener-prow bot commented Jun 13, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LucaBernstein

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

The pull request process is described 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

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2024
@LucaBernstein
Copy link
Member

/hold
until further reviews

@gardener-prow gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 13, 2024
@oliver-goetz
Copy link
Member Author

/lgtm cancel
/unhold

If there are multiple reviewers, one can add /approve and the other one /lgtm 🙂

@gardener-prow gardener-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 14, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-os-gardenlinux that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-os-gardenlinux that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-os-suse-chost that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-os-ubuntu that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-os-gardenlinux that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-os-suse-chost that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-os-coreos that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-runtime-gvisor that referenced this pull request Nov 18, 2024
MrBatschner added a commit to gardener/gardener-extension-os-suse-chost that referenced this pull request Nov 18, 2024
MrBatschner added a commit to gardener/gardener-extension-os-gardenlinux that referenced this pull request Nov 18, 2024
MrBatschner added a commit to gardener/gardener-extension-os-coreos that referenced this pull request Nov 18, 2024
MrBatschner added a commit to gardener/gardener-extension-os-ubuntu that referenced this pull request Nov 18, 2024
MrBatschner added a commit to MrBatschner/gardener-extension-runtime-gvisor that referenced this pull request Nov 19, 2024
MrBatschner added a commit to gardener/gardener-extension-runtime-gvisor that referenced this pull request Nov 21, 2024
* enable gosec for static code analysis as enabled by gardener/gardener#9959
* address G304 finding
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/compliance Compliance related area/security Security related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants