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

Feature/136 trivy integration #139

Open
wants to merge 35 commits into
base: develop
Choose a base branch
from

Conversation

alexander-dammeier
Copy link
Contributor

No description provided.

robertauer and others added 30 commits November 18, 2024 13:47
As trivy exits with exit code 1 when it encountered an error, we will use exit code 10 if trivy scanned successful and found vulnerabilities.
Also don't use RuntimeException, which is blocked by Jenkins.
To exclude CVEs from trivy scan, add a .trivyignore file to the
workspace.
The workspace is mounted to the docker container and therefore the
.trivyignore file is integrated into the scan process, if it exists
also update compiler dependencies
This reverts commit e6736d9.
The problem with this commit was, that now the jenkins pipeline
only creates empty jars. It only works on local machines when
we call our groovy tests directly
String additionalFlags = "--db-repository public.ecr.aws/aquasecurity/trivy-db --java-db-repository public.ecr.aws/aquasecurity/trivy-java-db",
String trivyReportFile = "trivy/trivyReport.json"
) {
Integer exitCode = docker.image("aquasec/trivy:${trivyVersion}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not hardcode the trivy image as Docker Hub has a rate limiting, which makes trouble already. If we make this configurable we could use a mirror, like a artifact repo in our own cloud.

fileExtension = "txt"
break
default:
script.error("This format did not match the supported formats: " + format)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why no support for custom formats?

Copy link
Member

Choose a reason for hiding this comment

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

Because this PR was made to combine the functionality in dogu-build-lib and ces-build-lib and we didn't have custom format options yet. Custom formats could be its own issue/PR.

* @param formattedTrivyReportFilename The file name your report files should get, without file extension. E.g. "ubuntu24report"
* @param trivyReportFile The "trivyReportFile" parameter you used in the "scanImage" function, if it was set
*/
void saveFormattedTrivyReport(String format = TrivyScanFormat.HTML, String formattedTrivyReportFilename = "trivyReport", String trivyReportFile = "trivy/trivyReport.json") {
Copy link
Contributor Author

@alexander-dammeier alexander-dammeier Nov 29, 2024

Choose a reason for hiding this comment

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

My intuition was that the order of parameters should be: trivyReportFile, format, formattedTrivyReportFilename.

Also it feels strange to expect the user to leave out the file extension. This could also be a problem when we want to support custom formats as then we have no clue, which extension we should add via the code but it is not so easy to provide a default parameter with a matching filename. Maybe we could use the format parameter for the default parameter.

A third thing is, that there is no way at the moment to set the output path for the formattedReport.

Suggested change
void saveFormattedTrivyReport(String format = TrivyScanFormat.HTML, String formattedTrivyReportFilename = "trivyReport", String trivyReportFile = "trivy/trivyReport.json") {
void formatReport(String inputReportFile = "trivy/trivyReport.json", String format = TrivyScanFormat.HTML, String outputReportFile = "$trivyDirectory/trivyReport.${format}") {

Copy link
Member

@robertauer robertauer Dec 2, 2024

Choose a reason for hiding this comment

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

If the user wants to use the default report file, he/she can just call saveFormattedTrivyReport(TrivyScanFormat.HTML). If we use your order, you always have to declare the (default) report file name, which you may not even know as a new user of this function: saveFormattedTrivyReport("trivy/trivyReport.json", TrivyScanFormat.HTML). This would be much less convenient for the user.

Comment on lines +15 to +20
static String HIGH = "CRITICAL,HIGH"

/**
* Medium or higher vulnerabilities.
*/
static String MEDIUM = "CRITICAL,HIGH,MEDIUM"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
static String HIGH = "CRITICAL,HIGH"
/**
* Medium or higher vulnerabilities.
*/
static String MEDIUM = "CRITICAL,HIGH,MEDIUM"
static String HIGH_AND_ABOVE = "CRITICAL,HIGH"
/**
* Medium or higher vulnerabilities.
*/
static String MEDIUM_AND_ABOVE = "CRITICAL,HIGH,MEDIUM"

Comment on lines +1305 to +1306
Note that the flags "--db-repository public.ecr.aws/aquasecurity/trivy-db --java-db-repository public.ecr.aws/aquasecurity/trivy-java-db"
are set by default to avoid rate limiting of Trivy database downloads.

Choose a reason for hiding this comment

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

should we specify here, that these defaults are implemented in a way, that they are not set if the additionalFlags is used?

* Notes:
* - Use a .trivyignore file for allowed CVEs
* - This function will generate a JSON formatted report file which can be converted to other formats via saveFormattedTrivyReport()
* - Evaluate via exit codes: 0 = no vulnerability; 1 = vulnerabilities found; other = function call failed

Choose a reason for hiding this comment

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

It has to be exit code 10 for vulnerabilities or am I not understanding the code correctly?

Suggested change
* - Evaluate via exit codes: 0 = no vulnerability; 1 = vulnerabilities found; other = function call failed
* - Evaluate via exit codes: 0 = no vulnerability; 10 = vulnerabilities found; other = function call failed

Copy link
Member

Choose a reason for hiding this comment

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

This seems to be an old comment which doesn't apply any more as the function now has a boolean return value

* Notes:
* - Use a .trivyignore file for allowed CVEs
* - This function will generate a JSON formatted report file which can be converted to other formats via saveFormattedTrivyReport()
* - Evaluate via exit codes: 0 = no vulnerability; 1 = vulnerabilities found; other = function call failed

Choose a reason for hiding this comment

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

see below. Should be exit code 10 instead of 1, shouldn't it?

Suggested change
* - Evaluate via exit codes: 0 = no vulnerability; 1 = vulnerabilities found; other = function call failed
* - Evaluate via exit codes: 0 = no vulnerability; 10 = vulnerabilities found; other = function call failed

Copy link
Member

Choose a reason for hiding this comment

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

This seems to be an old comment which doesn't apply any more as the function now has a boolean return value

}
docker.image("aquasec/trivy:${trivyVersion}")
.inside("-v ${script.env.WORKSPACE}/.trivy/.cache:/root/.cache/") {
script.sh(script: "trivy convert --format ${formatString} --output ${trivyDirectory}${formattedTrivyReportFilename}.${fileExtension} ${trivyReportFile}")

Choose a reason for hiding this comment

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

Feels like there is a slash missing

Suggested change
script.sh(script: "trivy convert --format ${formatString} --output ${trivyDirectory}${formattedTrivyReportFilename}.${fileExtension} ${trivyReportFile}")
script.sh(script: "trivy convert --format ${formatString} --output ${trivyDirectory}/${formattedTrivyReportFilename}.${fileExtension} ${trivyReportFile}")

Copy link
Member

Choose a reason for hiding this comment

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

In this method, trivyDirectory already holds the slash (line 109): String trivyDirectory = "trivy/"

.inside("-v ${script.env.WORKSPACE}/.trivy/.cache:/root/.cache/") {
script.sh(script: "trivy convert --format ${formatString} --output ${trivyDirectory}${formattedTrivyReportFilename}.${fileExtension} ${trivyReportFile}")
}
script.archiveArtifacts artifacts: "${trivyDirectory}${formattedTrivyReportFilename}.*", allowEmptyArchive: true

Choose a reason for hiding this comment

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

Feels like there is a slash missing

Suggested change
script.archiveArtifacts artifacts: "${trivyDirectory}${formattedTrivyReportFilename}.*", allowEmptyArchive: true
script.archiveArtifacts artifacts: "${trivyDirectory}/${formattedTrivyReportFilename}.*", allowEmptyArchive: true

Copy link
Member

Choose a reason for hiding this comment

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

In this method, trivyDirectory already holds the slash (line 109): String trivyDirectory = "trivy/"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not hard code this directory. Please use the field of the trivy class for the working directory. then @meiserloh s comment is valid again.

Copy link
Member

@robertauer robertauer Dec 3, 2024

Choose a reason for hiding this comment

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

You are right, we should use Trivy.trivyDirectory here and remove the trivyDirectory declared in this method.

* @param formattedTrivyReportFilename The file name your report files should get, without file extension. E.g. "ubuntu24report"
* @param trivyReportFile The "trivyReportFile" parameter you used in the "scanImage" function, if it was set
*/
void saveFormattedTrivyReport(String format = TrivyScanFormat.HTML, String formattedTrivyReportFilename = "trivyReport", String trivyReportFile = "trivy/trivyReport.json") {

Choose a reason for hiding this comment

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

We could add the option to filter by severity, too. This way we could scan for all severities and create reports by severity or something.

https://trivy.dev/v0.48/docs/references/configuration/cli/trivy_convert/

Copy link
Member

Choose a reason for hiding this comment

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

I think this is an additional feature and should have its own issue/PR.
There may be a debate if we even want to implement this, because setting the severity level in the scan-methods is reducing the scan runtime, which would always be at maximum, if you filter the report after scanning.


def gotException = false
try {
doTestScan(imageName, severityLevel, TrivyScanStrategy.FAIL, 10)doTestScan(imageName, severityLevel, TrivyScanStrategy.FAIL, 10)

Choose a reason for hiding this comment

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

this looks like double copy pasta?

}

void testScanImage_unsuccessfulTrivyExecution() {
// with hopes that this image will always have CVEs

Choose a reason for hiding this comment

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

delete this comment

* @param formattedTrivyReportFilename The file name your report files should get, without file extension. E.g. "ubuntu24report"
* @param trivyReportFile The "trivyReportFile" parameter you used in the "scanImage" function, if it was set
*/
void saveFormattedTrivyReport(String format = TrivyScanFormat.HTML, String formattedTrivyReportFilename = "trivyReport", String trivyReportFile = "trivy/trivyReport.json") {
Copy link
Contributor Author

@alexander-dammeier alexander-dammeier Nov 29, 2024

Choose a reason for hiding this comment

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

"trivyReport" as default for the file name could lead to an override of the initial trivy report if we convert to json again as the default report filename also is "trivyReport.json"

Copy link
Member

@robertauer robertauer Dec 2, 2024

Choose a reason for hiding this comment

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

The report file is already in json format (scanImage(): trivy image --exit-code 10 --exit-on-eol 10 --format ${TrivyScanFormat.JSON}...), so there should be no harm in converting it again I guess

Copy link

@meiserloh meiserloh Dec 2, 2024

Choose a reason for hiding this comment

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

As of now, you are right, but it would be problematic if we add for example the severity option to the saveFormattedTrivyReport function.

Copy link
Member

Choose a reason for hiding this comment

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

That's right, let's give it a better name. Any suggestions? How about formattedTrivyReport?

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.

3 participants