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

FIX: Upgraded docker dependencies #607

Closed
wants to merge 1 commit into from

Conversation

derekmurawsky
Copy link

There were security issues reported in several components in the
Dockerfile. This PR fixes them. It should not introduce any
breaking changes, but this container does now run as a 'gitleaks'
user.

Issues reported:

  • 3 Vulnerabilities in Openssh v8.1_p1-r0
  • 1 Vulnerability in busybox v 1.31.1-r9
  • 1 Vulnerability in apk-tools v2.10.5-r0
  • 1 CIS best practices issue - Container did not run as non-root

Additional fixes/changes

  • Refactored Dockerfile to allow easier upgrades in the future
    through ARGuments.
  • Refactored makefile to support additional arguments
  • Refactored makefile with an IMAGE_PATH to make future container
    tests easier
  • Downgraded to openssh-client instead of openssh*. Having the
    openssh server in the container is a bad practice.
  • Did not version pin the APKs as that is against Alpine guidance

Fixes #562

NOTE: I was not able to scan using the proprietary twistlock tool.

Description:

Explain the purpose of the PR.

Checklist:

  • Does your PR pass tests?
  • Have you written new tests for your changes?
  • Have you lint your code locally prior to submission?

There were security issues reported in several components in the
Dockerfile. This PR fixes them. It should not introduce any
breaking changes, but this container does now run as a 'gitleaks'
user.

Issues reported:
* 3 Vulnerabilities in Openssh v8.1_p1-r0
* 1 Vulnerability in busybox v 1.31.1-r9
* 1 Vulnerability in apk-tools v2.10.5-r0
* 1 CIS best practices issue - Container did not run as non-root

Additional fixes/changes
* Refactored Dockerfile to allow easier upgrades in the future
through ARGuments.
* Refactored makefile to support additional arguments
* Refactored makefile with an IMAGE_PATH to make future container
tests easier
* Downgraded to openssh-client instead of openssh*. Having the
openssh server in the container is a bad practice.
* Did *not* version pin the APKs as that is against Alpine guidance

Fixes gitleaks#562
@@ -33,18 +37,17 @@ release-builds:
rm -rf build
mkdir build
env GOOS="windows" GOARCH="amd64" go build -o "build/gitleaks-windows-amd64.exe" $(LDFLAGS)
env GOOS="windows" GOARCH="386" go build -o "build/gitleaks-windows-386.exe" $(LDFLAGS)
env GOOS="linux" GOARCH="amd64" go build -o "build/gitleaks-linux-amd64" $(LDFLAGS)
env GOOS="windows" GOARCH="386" go$(DOCKER_BUILD_ARGS) uild -o "build/gitleaks-linux-amd64" $(LDFLAGS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@derekmurawsky Looks like there is a typo here. Also why is amd64 removed?

Copy link
Author

Choose a reason for hiding this comment

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

Apologies for the delayed response, I missed the notification. Closing the loop on this: I'm honestly not sure. I think I may have fat fingered and removed the line by accident. The goal was to move the docker build args to a variable as changing them in multiple places was a bit of a pain when I was trying to build the container using my own version.

@zricethezav
Copy link
Collaborator

Closing this in favor of #615, I will still give you a shout out in the next release

@zricethezav zricethezav closed this Sep 2, 2021
@derekmurawsky
Copy link
Author

Sounds good! Thanks for patching it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vulnerabilities found in latest Docker image
2 participants