Make vulnerabilities scanner less strict (#669)

* Make vulnerabilities scanner less strict

* Only accept images with less critical and less high vulnerabilities

* Parse grype output as json

Author: dnestoro

tests/tck-build-logic/src/main/java/org/graalvm/internal/tck/DockerUtils.java

@@ -1,7 +1,6 @@
 package org.graalvm.internal.tck;
 
 import java.io.BufferedReader;
-import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.net.MalformedURLException;
@@ -10,10 +9,9 @@
 import java.net.URL;
 import java.nio.file.*;
 import java.util.*;
-import java.util.stream.Collectors;
 
 public class DockerUtils {
-    private static final String ALLOWED_DOCKER_IMAGES = "/allowed-docker-images";
+    public static final String ALLOWED_DOCKER_IMAGES = "/allowed-docker-images";
 
     private static URL getDockerfileDirectory() {
         URL url = DockerUtils.class.getResource(ALLOWED_DOCKER_IMAGES);
@@ -47,7 +45,7 @@ private static String imageNameFromFile(URL dockerFile) throws IOException, URIS
         return images.get(0);
     }
 
-    private static String fileNameFromJar(URL jarFile) {
+    public static String fileNameFromJar(URL jarFile) {
         return jarFile.toString().split("!")[1];
     }
 
@@ -78,4 +76,8 @@ public static Set<String> getAllAllowedImages() {
         }
     }
 
+    public static String getImageName(String imageWithVersion) {
+        return imageWithVersion.split(":")[0];
+    }
+
 }

tests/tck-build-logic/src/main/java/org/graalvm/internal/tck/GrypeTask.java

@@ -1,5 +1,13 @@
 package org.graalvm.internal.tck;
 
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import org.graalvm.internal.tck.model.MetadataIndexEntry;
+import org.graalvm.internal.tck.model.grype.GrypeEntry;
 import org.gradle.api.DefaultTask;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.api.tasks.options.Option;
@@ -10,10 +18,13 @@
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.*;
+import java.util.stream.Collectors;
 
-import static org.graalvm.internal.tck.DockerUtils.extractImagesNames;
-import static org.graalvm.internal.tck.DockerUtils.getAllAllowedImages;
 
 public abstract class GrypeTask extends DefaultTask {
 
@@ -33,88 +44,198 @@ void setNewCommit(String newCommit) {
     private String newCommit;
     private String baseCommit;
 
-    private final String jqMatcher = " | jq -c '.matches | .[] | .vulnerability | select(.severity | (contains(\"High\") or contains(\"Critical\")))'";
+    private static final String JQ_MATCHER = " | jq -c '.matches | .[] | .vulnerability | select(.severity | (contains(\"High\") or contains(\"Critical\")))'";
+    private static final String DOCKERFILE_DIRECTORY = "allowed-docker-images";
 
-    private List<URL> getChangedImages(String base, String head){
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        getExecOperations().exec(spec -> {
-            spec.setStandardOutput(baos);
-            spec.commandLine("git", "diff", "--name-only", "--diff-filter=ACMRT", base, head);
-        });
+    private static final String HIGH_VULNERABILITY = "HIGH";
+    private static final String CRITICAL_VULNERABILITY = "CRITICAL";
 
-        String output = baos.toString(StandardCharsets.UTF_8);
-        String dockerfileDirectory = "allowed-docker-images";
-        List<URL> diffFiles = Arrays.stream(output.split("\\r?\\n"))
-                .filter(path -> path.contains(dockerfileDirectory))
-                .map(path -> path.substring(path.lastIndexOf("/") + 1))
-                .map(DockerUtils::getDockerFile)
-                .toList();
+    private record Vulnerabilities(int critical, int high){}
 
-        if (diffFiles.isEmpty()) {
-            throw new RuntimeException("There are no changed or new docker image founded. " +
-                    "This task should be executed only if there are changes in allowed-docker-images directory.");
+    private record DockerImage(String image, Vulnerabilities vulnerabilities) {
+        public String getImageName() {
+            return DockerUtils.getImageName(image);
+        }
+
+        public boolean isVulnerableImage() {
+            return vulnerabilities.critical() > 0 || vulnerabilities.high() > 0;
+        }
+
+        public boolean isLessVulnerable(DockerImage other) {
+            return this.vulnerabilities.critical() < other.vulnerabilities().critical() && this.vulnerabilities.high() < other.vulnerabilities().high();
         }
 
-        return diffFiles;
+        public void printVulnerabilityStatus() {
+            System.out.println("Image: " + image + " contains " + vulnerabilities.critical() + " critical and " + vulnerabilities.high() + " high vulnerabilities");
+        }
     }
 
     @TaskAction
     void run() throws IllegalStateException, IOException, URISyntaxException {
-        List<String> vulnerableImages = new ArrayList<>();
-        Set<String> allowedImages;
-        if (baseCommit == null && newCommit == null) {
-            allowedImages = getAllAllowedImages();
+        boolean scanAllAllowedImages = baseCommit == null && newCommit == null;
+        if (scanAllAllowedImages) {
+            scanAllImages();
         } else {
-            allowedImages = extractImagesNames(getChangedImages(baseCommit, newCommit));
+            scanChangedImages();
         }
+    }
+
+    /**
+     * Re-scans all images from allowed images list
+     */
+    private void scanAllImages() {
+        Set<DockerImage> imagesToCheck = DockerUtils.getAllAllowedImages().stream().map(this::makeDockerImage).collect(Collectors.toSet());
+        List<DockerImage> vulnerableImages = imagesToCheck.stream().filter(DockerImage::isVulnerableImage).toList();
 
-        boolean shouldFail = false;
-        for (String image : allowedImages) {
-            System.out.println("Checking image: " + image);
-            String[] command = { "-c",  "grype -o json " + image + jqMatcher };
-
-            ByteArrayOutputStream execOutput = new ByteArrayOutputStream();
-            getExecOperations().exec(execSpec -> {
-                execSpec.setExecutable("/bin/sh");
-                execSpec.setArgs(List.of(command));
-                execSpec.setStandardOutput(execOutput);
-            });
-
-            ByteArrayInputStream inputStream = new ByteArrayInputStream(execOutput.toByteArray());
-            try (BufferedReader stdOut = new BufferedReader(new InputStreamReader(inputStream))) {
-                int numberOfHigh = 0;
-                int numberOfCritical = 0;
-                String line;
-                while ((line = stdOut.readLine()) != null) {
-                    if (line.contains("\"severity\":\"High\"")) {
-                        numberOfHigh++;
-                    }else if (line.contains("\"severity\":\"Critical\"")) {
-                        numberOfCritical++;
+        if (!vulnerableImages.isEmpty()) {
+            vulnerableImages.forEach(DockerImage::printVulnerabilityStatus);
+            throw new IllegalStateException("Highly vulnerable images found. Please check the list of vulnerable images provided above.");
+        }
+    }
+
+    /**
+     * Scans images that have been changed between org.graalvm.internal.tck.GrypeTask#baseCommit and org.graalvm.internal.tck.GrypeTask#newCommit.
+     * If changed images are less vulnerable than previously allowed images, they won't be reported as vulnerable
+     */
+    private void scanChangedImages() throws IOException, URISyntaxException {
+        Set<DockerImage> imagesToCheck = getChangedImages().stream().map(this::makeDockerImage).collect(Collectors.toSet());
+        List<DockerImage> vulnerableImages = imagesToCheck.stream().filter(DockerImage::isVulnerableImage).toList();
+
+        if (!vulnerableImages.isEmpty()) {
+            int acceptedImages = 0;
+            Set<String> currentlyAllowedImages = getAllowedImagesFromMaster();
+
+            for (DockerImage image : vulnerableImages) {
+                image.printVulnerabilityStatus();
+
+                // get allowed image with the same name, if it exists
+                Optional<String> existingAllowedImage = currentlyAllowedImages.stream()
+                        .filter(allowedImage -> DockerUtils.getImageName(allowedImage).equalsIgnoreCase(image.getImageName()))
+                        .findFirst();
+
+                // check if a new image is less vulnerable than the existing one
+                if (existingAllowedImage.isPresent()) {
+                    DockerImage imageToCompare = makeDockerImage(existingAllowedImage.get());
+                    imageToCompare.printVulnerabilityStatus();
+
+                    if (image.isLessVulnerable(imageToCompare)) {
+                        System.out.println("Accepting: " + image.image() + " because it has less vulnerabilities than existing: " + imageToCompare.image());
+                        acceptedImages++;
                     }
                 }
+            }
+
+            if (acceptedImages < vulnerableImages.size()) {
+                throw new IllegalStateException("Highly vulnerable images found. Please check the list of vulnerable images provided above.");
+            }
+        }
+    }
+
+    private DockerImage makeDockerImage(String image) {
+        System.out.println("Generating info for docker image: " + image);
+        try {
+            return new DockerImage(image, getVulnerabilities(image));
+        } catch (IOException e) {
+            throw new RuntimeException("Cannot parse grype output for image: " + image + " .Reason: " + e.getMessage());
+        }
+    }
+
+    private Vulnerabilities getVulnerabilities(String image) throws IOException {
+        int numberOfHigh = 0;
+        int numberOfCritical = 0;
+        String[] command = {"-c", "grype -o json " + image + JQ_MATCHER};
+
+        // call Grype to get vulnerabilities
+        ByteArrayOutputStream execOutput = new ByteArrayOutputStream();
+        getExecOperations().exec(execSpec -> {
+            execSpec.setExecutable("/bin/sh");
+            execSpec.setArgs(List.of(command));
+            execSpec.setStandardOutput(execOutput);
+        });
+
+        // parse Grype output
+        ByteArrayInputStream inputStream = new ByteArrayInputStream(execOutput.toByteArray());
+        ObjectMapper mapper = new ObjectMapper();
+        JsonFactory factory = mapper.getFactory();
+        try (JsonParser parser = factory.createParser(inputStream)) {
+            while (!parser.isClosed()) {
+                if (parser.nextToken() == null) {
+                    break;
+                }
 
-                if (numberOfHigh > 0 || numberOfCritical > 0) {
-                    vulnerableImages.add("Image: " + image + " contains " + numberOfCritical + " critical, and " + numberOfHigh + " high vulnerabilities");
+                if (parser.currentToken() != com.fasterxml.jackson.core.JsonToken.START_OBJECT) {
+                    continue;
                 }
 
-                if (numberOfHigh > 4 || numberOfCritical > 0) {
-                    shouldFail = true;
+                GrypeEntry vuln = mapper.readValue(parser, GrypeEntry.class);
+                if (vuln.severity.equalsIgnoreCase(CRITICAL_VULNERABILITY)) {
+                    numberOfCritical++;
+                }
+
+                if (vuln.severity.equalsIgnoreCase(HIGH_VULNERABILITY)) {
+                    numberOfHigh++;
                 }
             }
+        }
+
+        return new Vulnerabilities(numberOfCritical, numberOfHigh);
+    }
+
+    /**
+     * Get all docker images introduced between two commits
+     */
+    private Set<String> getChangedImages() throws IOException, URISyntaxException {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        getExecOperations().exec(spec -> {
+            spec.setStandardOutput(baos);
+            spec.commandLine("git", "diff", "--name-only", "--diff-filter=ACMRT", baseCommit, newCommit);
+        });
 
-            inputStream.close();
-            execOutput.close();
+        String output = baos.toString(StandardCharsets.UTF_8);
+        List<URL> diffFiles = Arrays.stream(output.split("\\r?\\n"))
+                .filter(path -> path.contains(DOCKERFILE_DIRECTORY))
+                .map(path -> path.substring(path.lastIndexOf("/") + 1))
+                .map(DockerUtils::getDockerFile)
+                .toList();
+
+        if (diffFiles.isEmpty()) {
+            throw new RuntimeException("There are no changed or new docker image founded. " +
+                    "This task should be executed only if there are changes in allowed-docker-images directory.");
         }
 
-        if (!vulnerableImages.isEmpty()) {
-            System.err.println("Vulnerable images found:");
-            System.err.println("===========================================================");
-            vulnerableImages.forEach(System.err::println);
+        return DockerUtils.extractImagesNames(diffFiles);
+    }
+
+    /**
+     * Return all allowed docker images from master branch
+     */
+    private Set<String> getAllowedImagesFromMaster() throws URISyntaxException, IOException {
+        URL url = GrypeTask.class.getResource(DockerUtils.ALLOWED_DOCKER_IMAGES);
+        if (url == null) {
+            throw new RuntimeException("Cannot find allowed-docker-images directory");
         }
 
-        if (shouldFail) {
-            throw new IllegalStateException("Highly vulnerable images found. Please check the list of vulnerable images provided above.");
+        Set<String> allowedImages = new HashSet<>();
+        try (FileSystem fs = FileSystems.newFileSystem(url.toURI(), Collections.emptyMap())) {
+            List<String> files = Files.walk(fs.getPath(DockerUtils.ALLOWED_DOCKER_IMAGES))
+                    .filter(Files::isRegularFile)
+                    .map(Path::toString)
+                    .map(path -> path.substring(path.lastIndexOf("/") + 1))
+                    .map(DockerUtils::getDockerFile)
+                    .map(DockerUtils::fileNameFromJar)
+                    .toList();
+
+            for (String file : files) {
+                ByteArrayOutputStream baos = new ByteArrayOutputStream();
+                getExecOperations().exec(spec -> {
+                    spec.setStandardOutput(baos);
+                    spec.commandLine("git", "show", "master:tests/tck-build-logic/src/main/resources" + file);
+                });
+
+                allowedImages.add(baos.toString());
+            }
         }
-    }
 
+        return allowedImages.stream().map(line -> line.substring("FROM".length()).trim()).collect(Collectors.toSet());
+    }
 }

tests/tck-build-logic/src/main/java/org/graalvm/internal/tck/model/grype/GrypeEntry.java

@@ -0,0 +1,67 @@
+package org.graalvm.internal.tck.model.grype;
+
+/*
+ * JSON model for metadata/index.json.
+ */
+import java.util.List;
+import java.util.Map;
+
+public class GrypeEntry {
+    public String id;
+    public String dataSource;
+    public String namespace;
+    public String severity;
+    public List<String> urls;
+    public String description;
+    public List<Cvss> cvss;
+    public List<KnownExploited> knownExploited;
+    public List<Epss> epss;
+    public Fix fix;
+    public List<Object> advisories; // unknown structure
+    public Double risk;
+}
+
+class Cvss {
+    public String source;
+    public String type;
+    public String version;
+    public String vector;
+    public Metrics metrics;
+    public Map<String, Object> vendorMetadata;
+}
+class Metrics {
+    public Double baseScore;
+    public Double exploitabilityScore;
+    public Double impactScore;
+}
+
+class KnownExploited {
+    public String cve;
+    public String vendorProject;
+    public String product;
+    public String dateAdded;
+    public String requiredAction;
+    public String dueDate;
+    public String knownRansomwareCampaignUse;
+    public String notes;
+    public List<String> urls;
+    public List<String> cwes;
+}
+
+class Epss {
+    public String cve;
+    public Double epss;
+    public Double percentile;
+    public String date;
+}
+
+class Fix {
+    public List<String> versions;
+    public String state;
+    public List<AvailableFix> available;
+}
+class AvailableFix {
+    public String version;
+    public String date;
+    public String kind;
+}