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

Pr orly ast support #356

Merged
merged 30 commits into from
Jul 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a670d2b
AST support
olgakil Jul 14, 2020
50b4551
Fix error integration with new common
olgakil Jul 15, 2020
a7b5cde
Support AST
olgakil Jul 19, 2020
9eeb9ab
Support AST - merge last commits
olgakil Jul 19, 2020
8366a25
Fix issue with scanner.isEnabled
olgakil Jul 20, 2020
05f2570
change version of SDK to 0.4.31
olgakil Jul 21, 2020
7cbfa12
change version of SDK to 0.4.31
olgakil Jul 21, 2020
8bddd95
SDK version 32 - fixes related to incorrect includes + fix CxGo
olgakil Jul 22, 2020
9b5da7d
AST support
olgakil Jul 14, 2020
c440184
Fix error integration with new common
olgakil Jul 15, 2020
d3603db
Support AST
olgakil Jul 19, 2020
2baaab6
Support AST - merge last commits
olgakil Jul 19, 2020
71b2541
Fix issue with scanner.isEnabled
olgakil Jul 20, 2020
a5b60ff
SDK version 32 - fixes related to incorrect includes + fix CxGo
olgakil Jul 22, 2020
f38fa21
AST support
olgakil Jul 14, 2020
0f9c4b5
Fix error integration with new common
olgakil Jul 15, 2020
82d23f6
Support AST
olgakil Jul 19, 2020
5c29398
Support AST - merge last commits
olgakil Jul 19, 2020
0bb5683
Fix issue with scanner.isEnabled
olgakil Jul 20, 2020
a8aa339
change version of SDK to 0.4.31
olgakil Jul 21, 2020
a514f55
SDK version 32 - fixes related to incorrect includes + fix CxGo
olgakil Jul 22, 2020
76ead43
AST support
olgakil Jul 14, 2020
ebc2186
Fix error integration with new common
olgakil Jul 15, 2020
5d6ecae
Support AST
olgakil Jul 19, 2020
59495d8
Support AST - merge last commits
olgakil Jul 19, 2020
0c2be79
Fix issue with scanner.isEnabled
olgakil Jul 20, 2020
e039ca4
change version of SDK to 0.4.31
olgakil Jul 21, 2020
745e1b1
change version of SDK to 0.4.31
olgakil Jul 21, 2020
07fd7aa
SDK version 32 - fixes related to incorrect includes + fix CxGo
olgakil Jul 22, 2020
e176e48
Add AST scan to batch flow
olgakil Jul 22, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion build-11.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import org.gradle.api.tasks.testing.Test

buildscript {
ext {
CxSBSDK = "0.4.30"
CxSBSDK = "0.4.32"
//cxVersion = "8.90.5"
springBootVersion = '2.2.6.RELEASE'
sonarqubeVersion = '2.8'
Expand Down Expand Up @@ -78,6 +78,7 @@ dependencies {
compile ("org.apache.httpcomponents:httpclient:4.5.10")
compile ("org.codehaus.groovy:groovy-all:2.5.8")
compile ("org.apache.ivy:ivy:2.5.0")
compile ("org.apache.commons:commons-lang3:3.11")
compile ("org.eclipse.jgit:org.eclipse.jgit:5.5.1.201910021850-r")
compile group: 'com.sun.xml.bind', name: 'jaxb-impl', version: '2.4.0-b180830.0438'
compile group: 'javax.xml.ws', name: 'jaxws-api', version: '2.3.1'
Expand Down
2 changes: 1 addition & 1 deletion build-cxgo.gradle
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
buildscript {
ext {
CxSBSDK = "0.1.24"
CxSBSDK = "0.1.25"
//cxVersion = "8.90.5"
springBootVersion = '2.2.6.RELEASE'
sonarqubeVersion = '2.8'
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
buildscript {
ext {
CxSBSDK = "0.4.30"
CxSBSDK = "0.4.32"
//cxVersion = "8.90.5"
springBootVersion = '2.2.6.RELEASE'
sonarqubeVersion = '2.8'
Expand Down
64 changes: 49 additions & 15 deletions src/main/java/com/checkmarx/flow/CxFlowRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ public class CxFlowRunner implements ApplicationRunner {
public static final String BATCH_OPTION = "batch";
private static final String SAST_SCANNER = "sast";
private static final String SCA_SCANNER = "sca";

private static final String AST_SCANNER = "ast";

private final FlowProperties flowProperties;
private final CxProperties cxProperties;
private final JiraProperties jiraProperties;
Expand All @@ -52,6 +53,7 @@ public class CxFlowRunner implements ApplicationRunner {
private final HelperService helperService;
private final SastScanner sastScanner;
private final SCAScanner scaScanner;
private final ASTScanner astScanner;
private final List<ThreadPoolTaskExecutor> executors;
private final ResultsService resultsService;
private final OsaScannerService osaScannerService;
Expand Down Expand Up @@ -451,8 +453,7 @@ private String getOptionValues(ApplicationArguments arg, String option){
}

private void cxScan(ScanRequest request, String gitUrl, String gitAuthUrl, String branch, ScanRequest.Repository repoType) throws ExitThrowable {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a more meaningful name instead of cxScan()? It's not clear by looking at the code what is the difference between cxScan() and scan().

ScanResults sastScanResults = null;
ScanResults scaScanResults = null;

log.info("Initiating scan using Checkmarx git clone");
request.setRepoType(repoType);
log.info("Git url: {}", gitUrl);
Expand All @@ -461,33 +462,66 @@ private void cxScan(ScanRequest request, String gitUrl, String gitAuthUrl, Strin
request.setRepoUrlWithAuth(gitAuthUrl);
request.setRefs(Constants.CX_BRANCH_PREFIX.concat(branch));

if(flowProperties.getEnabledVulnerabilityScanners() == null ||
flowProperties.getEnabledVulnerabilityScanners().contains(SAST_SCANNER)) {
ScanResults scanResults = scan(request);
processResults(request, scanResults);
}

private ScanResults scan(ScanRequest request) throws ExitThrowable {
ScanResults sastScanResults = null;
ScanResults scaScanResults = null;
ScanResults astScanResults = null;

if(isSastEnabled()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a 'for' loop here or reuse the existing one?

sastScanResults = sastScanner.cxFullScan(request);
}
if(flowProperties.getEnabledVulnerabilityScanners().contains(SCA_SCANNER)) {
if(isScaEnabled()) {
scaScanResults = scaScanner.scan(request);
}
ScanResults scanResults = resultsService.joinResults(sastScanResults, scaScanResults);
processResults(request, scanResults);
if(isAstEnabled()) {
astScanResults = astScanner.scan(request);
}
return resultsService.joinResults(sastScanResults, scaScanResults, astScanResults);
}

private boolean isScaEnabled() {
return flowProperties.getEnabledVulnerabilityScanners().contains(SCA_SCANNER);
}

private boolean isAstEnabled() {
return flowProperties.getEnabledVulnerabilityScanners().contains(AST_SCANNER);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there cases when getEnabledVulnerabilityScanners() returns null?

}

private void cxScan(ScanRequest request, String path) throws ExitThrowable {
ScanResults sastScanResults = null;
ScanResults scaScanResults = null;

if(ScanUtils.empty(request.getProject())){
log.error("Please provide --cx-project to define the project in Checkmarx");
exit(2);
}
if(flowProperties.getEnabledVulnerabilityScanners() == null ||
flowProperties.getEnabledVulnerabilityScanners().contains(SAST_SCANNER)) {
ScanResults scanResults = scan(request, path);
processResults(request, scanResults);
}

private ScanResults scan(ScanRequest request, String path) throws ExitThrowable {

ScanResults sastScanResults = null;
ScanResults scaScanResults = null;
ScanResults astScanResults = null;

if(isSastEnabled()) {
sastScanResults = sastScanner.cxFullScan(request, path);
}
if(flowProperties.getEnabledVulnerabilityScanners().contains(SCA_SCANNER)) {
if(isScaEnabled()) {
scaScanResults = scaScanner.scan(request, path);
}
ScanResults scanResults = resultsService.joinResults(sastScanResults, scaScanResults);
processResults(request, scanResults);
if(isAstEnabled()) {
astScanResults = astScanner.scan(request, path);
}
return resultsService.joinResults(sastScanResults, scaScanResults, astScanResults);
}

private boolean isSastEnabled() {
return flowProperties.getEnabledVulnerabilityScanners() == null ||
flowProperties.getEnabledVulnerabilityScanners().contains(SAST_SCANNER);
}

private void cxOsaParse(ScanRequest request, File file, File libs) throws ExitThrowable {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/checkmarx/flow/aop/LoggingAop.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class LoggingAop {
"execution(* com.checkmarx.flow.service.ResultsService.processScanResultsAsync(.., com.checkmarx.flow.dto.ScanRequest, ..)) ||" +
"execution(* com.checkmarx.flow.service.SastScanner.executeCxScanFlow(com.checkmarx.flow.dto.ScanRequest, ..)) ||" +
"execution(* com.checkmarx.flow.service.SCAScanner.scan(com.checkmarx.flow.dto.ScanRequest, ..)) ||" +
"execution(* com.checkmarx.flow.service.ASTScanner.scan(com.checkmarx.flow.dto.ScanRequest, ..)) ||" +
"execution(* com.checkmarx.flow.service.ResultsService.publishCombinedResults(com.checkmarx.flow.dto.ScanRequest, ..)) ||" +
"execution(* com.checkmarx.flow.service.*.process(.., com.checkmarx.flow.dto.ScanRequest, ..))) && (args(.., request) || args(request, ..))")
public void beforeAdvice(JoinPoint joinPoint, ScanRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import lombok.NoArgsConstructor;
import lombok.Setter;

import java.util.EnumMap;
import java.util.HashMap;
import java.util.Map;

Expand Down
42 changes: 42 additions & 0 deletions src/main/java/com/checkmarx/flow/service/ASTScanner.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.checkmarx.flow.service;

import com.checkmarx.flow.config.FlowProperties;

import com.checkmarx.sdk.config.AstProperties;

import com.checkmarx.sdk.dto.ScanResults;

import com.checkmarx.sdk.dto.ast.ASTResultsWrapper;


import com.cx.restclient.AstClientImpl;

import lombok.extern.slf4j.Slf4j;

import org.springframework.stereotype.Service;



@Service

@Slf4j
public class ASTScanner extends AbstractASTScanner {

public ASTScanner(AstClientImpl astClient, FlowProperties flowProperties) {
super(astClient, flowProperties, AstProperties.CONFIG_PREFIX);
}

@Override
protected ScanResults toScanResults(ASTResultsWrapper internalResults) {
return ScanResults.builder()
.astResults(internalResults.getAstResults())
.build();
}

@Override
protected String getScanId(ASTResultsWrapper internalResults) {
return internalResults.getAstResults().getScanId();
}


}
149 changes: 149 additions & 0 deletions src/main/java/com/checkmarx/flow/service/AbstractASTScanner.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package com.checkmarx.flow.service;

import com.checkmarx.flow.config.FlowProperties;
import com.checkmarx.flow.dto.OperationResult;
import com.checkmarx.flow.dto.OperationStatus;
import com.checkmarx.flow.dto.ScanRequest;
import com.checkmarx.flow.dto.report.ScanReport;
import com.checkmarx.flow.exception.ExitThrowable;
import com.checkmarx.flow.exception.MachinaRuntimeException;
import com.checkmarx.flow.utils.ZipUtils;
import com.checkmarx.sdk.dto.ScanResults;
import com.checkmarx.sdk.dto.ast.ASTResults;
import com.checkmarx.sdk.dto.ast.ASTResultsWrapper;
import com.checkmarx.sdk.dto.ast.SCAResults;
import com.checkmarx.sdk.dto.ast.ScanParams;
import com.checkmarx.sdk.service.AstClient;
import lombok.extern.slf4j.Slf4j;


import java.io.File;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.List;
import java.util.UUID;


@Slf4j
public abstract class AbstractASTScanner implements VulnerabilityScanner{

private String scanType ;
private com.checkmarx.sdk.service.AstClient client;
private FlowProperties flowProperties;

public AbstractASTScanner() {
}

public AbstractASTScanner(AstClient astClient, FlowProperties flowProperties, String scanType) {
this.client = astClient;
this.flowProperties = flowProperties;
this.scanType = scanType;
}

public ScanResults scan(ScanRequest scanRequest) {
ScanResults result = null;
log.info("--------------------- Initiating new {} scan ---------------------", scanType);
ScanParams internalScaParams = toParams(scanRequest);
ASTResultsWrapper internalResults = new ASTResultsWrapper(new SCAResults(), new ASTResults());
try {
internalResults = client.scanRemoteRepo(internalScaParams);
logRequest(scanRequest, getScanId(internalResults), OperationResult.successful());
result = toScanResults(internalResults);
} catch (Exception e) {
final String message = "SCA scan failed.";
log.error(message, e);
OperationResult scanCreationFailure = new OperationResult(OperationStatus.FAILURE, e.getMessage());
logRequest(scanRequest, getScanId(internalResults), scanCreationFailure);
throw new MachinaRuntimeException(message);
Comment on lines +56 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Please resolve this code duplication.

}
return result;
}

public ScanResults scan(ScanRequest scanRequest, String path) throws ExitThrowable {
ScanResults result;
log.info("--------------------- Initiating new {} scan ---------------------", scanType);
ASTResultsWrapper internalResults = new ASTResultsWrapper(new SCAResults(), new ASTResults());

try {
String cxZipFile = FileSystems.getDefault().getPath("cx.".concat(UUID.randomUUID().toString()).concat(".zip")).toAbsolutePath().toString();
ZipUtils.zipFile(path, cxZipFile, flowProperties.getZipExclude());
File f = new File(cxZipFile);
log.debug("Creating temp file {}", f.getPath());
log.debug("free space {}", f.getFreeSpace());
log.debug("total space {}", f.getTotalSpace());
log.debug(f.getAbsolutePath());
ScanParams internalScaParams = toParams(scanRequest, cxZipFile);

internalResults = client.scanLocalSource(internalScaParams);
logRequest(scanRequest, getScanId(internalResults), OperationResult.successful());
result = toScanResults(internalResults);

log.debug("Deleting temp file {}", f.getPath());
Files.deleteIfExists(Paths.get(cxZipFile));

} catch (Exception e) {
final String message = "SCA scan failed.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like 'scanType' should be used here instead of SCA.

log.error(message, e);
OperationResult scanCreationFailure = new OperationResult(OperationStatus.FAILURE, e.getMessage());
logRequest(scanRequest, getScanId(internalResults), scanCreationFailure);
throw new MachinaRuntimeException(message);
}
return result;
}

protected abstract String getScanId(ASTResultsWrapper internalResults) ;

private ScanParams toParams(ScanRequest scanRequest, String zipPath) {
return ScanParams.builder()
.projectName(scanRequest.getProject())
.zipPath(zipPath)
.build();
}

protected abstract ScanResults toScanResults(ASTResultsWrapper internalResults);


protected ScanParams toParams(ScanRequest scanRequest) {
URL parsedUrl = getRepoUrl(scanRequest);

return ScanParams.builder()
.projectName(scanRequest.getProject())
.remoteRepoUrl(parsedUrl)
.build();
}




private URL getRepoUrl(ScanRequest scanRequest) {
URL parsedUrl;
try {
parsedUrl = new URL(scanRequest.getRepoUrlWithAuth());
} catch (MalformedURLException e) {
log.error("Failed to parse repository URL: '{}'", scanRequest.getRepoUrl());
throw new MachinaRuntimeException("Invalid repository URL.");
}
return parsedUrl;
}

@Override
public boolean isEnabled() {
List<String> enabledScanners = flowProperties.getEnabledVulnerabilityScanners();

return enabledScanners != null
&& enabledScanners.stream().anyMatch(scanner -> scanner.equalsIgnoreCase(scanType));

}

private void logRequest(ScanRequest request, String scanId, OperationResult scanCreationResult) {
ScanReport report = new ScanReport(scanId, request,request.getRepoUrl(), scanCreationResult, ScanReport.SCA);
report.log();
}




}
1 change: 1 addition & 0 deletions src/main/java/com/checkmarx/flow/service/FlowService.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.springframework.stereotype.Service;

import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;

/**
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/checkmarx/flow/service/JiraService.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.slf4j.Logger;
import org.springframework.stereotype.Service;
import org.springframework.web.client.HttpClientErrorException;

import javax.annotation.PostConstruct;
import java.beans.ConstructorProperties;
import java.net.URI;
Expand Down
Loading