From 57a44ace74f9472a5d74478f9156946bc403ccc5 Mon Sep 17 00:00:00 2001 From: Steve Marion Date: Fri, 7 Oct 2022 15:48:50 +0200 Subject: [PATCH] SONAR-17399 add owasp asvs 4.0 level support in hotspot search API --- .../server/security/SecurityStandards.java | 8 ++- .../sonar/server/issue/index/IssueIndex.java | 59 ++++++++++++----- .../sonar/server/issue/index/IssueQuery.java | 14 ++++ .../issue/index/IssueIndexFiltersTest.java | 52 +++++++++++++++ .../server/issue/index/IssueQueryTest.java | 3 + .../sonar/server/hotspot/ws/SearchAction.java | 9 +-- .../server/hotspot/ws/SearchActionTest.java | 66 +++++++++++++++++-- 7 files changed, 182 insertions(+), 29 deletions(-) diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java index e450c323e92..1661d13cf7a 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java @@ -330,12 +330,16 @@ public final class SecurityStandards { return new SecurityStandards(standards, cwe, sqCategory, ignoredSQCategories); } - public static Set getRequirementsForCategoryAndLevel(OwaspAsvs category, int level) { + public static Set getRequirementsForCategoryAndLevel(String category, int level) { return OWASP_ASVS_40_REQUIREMENTS_BY_LEVEL.get(level).stream() - .filter(req -> req.startsWith(category.category() + ".")) + .filter(req -> req.startsWith(category + ".")) .collect(Collectors.toSet()); } + public static Set getRequirementsForCategoryAndLevel(OwaspAsvs category, int level) { + return getRequirementsForCategoryAndLevel(category.category(), level); + } + private static Set getMatchingStandards(Set securityStandards, String prefix) { return securityStandards.stream() .filter(s -> s.startsWith(prefix)) diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java index 84af82f9e5a..25ff1fb637a 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -95,6 +95,7 @@ import org.sonar.server.security.SecurityStandards.PciDss; import org.sonar.server.security.SecurityStandards.SQCategory; import org.sonar.server.user.UserSession; import org.sonar.server.view.index.ViewIndexDefinition; +import org.springframework.util.CollectionUtils; import static com.google.common.base.Preconditions.checkState; import static java.lang.String.format; @@ -463,7 +464,7 @@ public class IssueIndex { // security category addSecurityCategoryPrefixFilter(FIELD_ISSUE_PCI_DSS_32, PCI_DSS_32, query.pciDss32(), filters); addSecurityCategoryPrefixFilter(FIELD_ISSUE_PCI_DSS_40, PCI_DSS_40, query.pciDss40(), filters); - addSecurityCategoryPrefixFilter(FIELD_ISSUE_OWASP_ASVS_40, OWASP_ASVS_40, query.owaspAsvs40(), filters); + addOwaspAsvsFilter(FIELD_ISSUE_OWASP_ASVS_40, OWASP_ASVS_40, query, filters); addSecurityCategoryFilter(FIELD_ISSUE_OWASP_TOP_10, OWASP_TOP_10, query.owaspTop10(), filters); addSecurityCategoryFilter(FIELD_ISSUE_OWASP_TOP_10_2021, OWASP_TOP_10_2021, query.owaspTop10For2021(), filters); addSecurityCategoryFilter(FIELD_ISSUE_SANS_TOP_25, SANS_TOP_25, query.sansTop25(), filters); @@ -480,6 +481,34 @@ public class IssueIndex { return filters; } + private static void addOwaspAsvsFilter(String fieldName, Facet facet, IssueQuery query, AllFilters allFilters) { + if (!CollectionUtils.isEmpty(query.owaspAsvs40())) { + Set requirements = calculateRequirementsForOwaspAsvs40Params(query); + QueryBuilder securityCategoryFilter = termsQuery(fieldName, requirements); + allFilters.addFilter( + fieldName, + facet.getFilterScope(), + boolQuery() + .must(securityCategoryFilter) + .must(termsQuery(FIELD_ISSUE_TYPE, VULNERABILITY.name(), SECURITY_HOTSPOT.name()))); + } + } + + + private static Set calculateRequirementsForOwaspAsvs40Params(IssueQuery query) { + int level = query.getOwaspAsvsLevel().orElse(3); + List levelRequirements = OWASP_ASVS_40_REQUIREMENTS_BY_LEVEL.get(level); + return query.owaspAsvs40().stream() + .flatMap(value -> { + // it's a specific category required + if (value.contains(".")) { + return Stream.of(value).filter(levelRequirements::contains); + } else { + return SecurityStandards.getRequirementsForCategoryAndLevel(value, level).stream(); + } + }).collect(Collectors.toSet()); + } + private static void addSecurityCategoryFilter(String fieldName, Facet facet, Collection values, AllFilters allFilters) { QueryBuilder securityCategoryFilter = createTermsFilter(fieldName, values); if (securityCategoryFilter != null) { @@ -627,11 +656,11 @@ public class IssueIndex { private static RequestFiltersComputer newFilterComputer(SearchOptions options, AllFilters allFilters) { Collection facetNames = options.getFacets(); Set> facets = Stream.concat( - Stream.of(EFFORT_TOP_AGGREGATION), - facetNames.stream() - .map(FACETS_BY_NAME::get) - .filter(Objects::nonNull) - .map(Facet::getTopAggregationDef)) + Stream.of(EFFORT_TOP_AGGREGATION), + facetNames.stream() + .map(FACETS_BY_NAME::get) + .filter(Objects::nonNull) + .map(Facet::getTopAggregationDef)) .collect(MoreCollectors.toSet(facetNames.size())); return new RequestFiltersComputer(allFilters, facets); @@ -836,11 +865,11 @@ public class IssueIndex { RESOLUTIONS.getName(), RESOLUTIONS.getTopAggregationDef(), RESOLUTIONS.getNumberOfTerms(), NO_EXTRA_FILTER, t -> - // add aggregation of type "missing" to return count of unresolved issues in the facet - t.subAggregation( - addEffortAggregationIfNeeded(query, AggregationBuilders - .missing(RESOLUTIONS.getName() + FACET_SUFFIX_MISSING) - .field(RESOLUTIONS.getFieldName())))); + // add aggregation of type "missing" to return count of unresolved issues in the facet + t.subAggregation( + addEffortAggregationIfNeeded(query, AggregationBuilders + .missing(RESOLUTIONS.getName() + FACET_SUFFIX_MISSING) + .field(RESOLUTIONS.getFieldName())))); esRequest.aggregation(aggregation); } @@ -960,10 +989,10 @@ public class IssueIndex { ASSIGNED_TO_ME.getNumberOfTerms(), NO_EXTRA_FILTER, t -> - // add sub-aggregation to return issue count for current user - aggregationHelper.getSubAggregationHelper() - .buildSelectedItemsAggregation(ASSIGNED_TO_ME.getName(), ASSIGNED_TO_ME.getTopAggregationDef(), new String[] {uuid}) - .ifPresent(t::subAggregation)); + // add sub-aggregation to return issue count for current user + aggregationHelper.getSubAggregationHelper() + .buildSelectedItemsAggregation(ASSIGNED_TO_ME.getName(), ASSIGNED_TO_ME.getTopAggregationDef(), new String[]{uuid}) + .ifPresent(t::subAggregation)); esRequest.aggregation(aggregation); } } diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java index 38c3a2299ba..9721db7df82 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.Map; +import java.util.Optional; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -83,6 +84,7 @@ public class IssueQuery { private final Collection pciDss32; private final Collection pciDss40; private final Collection owaspAsvs40; + private final Integer owaspAsvsLevel; private final Collection owaspTop10For2021; private final Collection sansTop25; private final Collection cwe; @@ -125,6 +127,7 @@ public class IssueQuery { this.pciDss32 = defaultCollection(builder.pciDss32); this.pciDss40 = defaultCollection(builder.pciDss40); this.owaspAsvs40 = defaultCollection(builder.owaspAsvs40); + this.owaspAsvsLevel = builder.owaspAsvsLevel; this.owaspTop10 = defaultCollection(builder.owaspTop10); this.owaspTop10For2021 = defaultCollection(builder.owaspTop10For2021); this.sansTop25 = defaultCollection(builder.sansTop25); @@ -230,6 +233,11 @@ public class IssueQuery { public Collection owaspAsvs40() { return owaspAsvs40; } + + public Optional getOwaspAsvsLevel() { + return Optional.ofNullable(owaspAsvsLevel); + } + public Collection owaspTop10() { return owaspTop10; } @@ -353,6 +361,7 @@ public class IssueQuery { private Collection pciDss32; private Collection pciDss40; private Collection owaspAsvs40; + private Integer owaspAsvsLevel; private Collection owaspTop10; private Collection owaspTop10For2021; private Collection sansTop25; @@ -483,6 +492,11 @@ public class IssueQuery { return this; } + public Builder owaspAsvsLevel(@Nullable Integer level) { + this.owaspAsvsLevel = level; + return this; + } + public Builder owaspTop10(@Nullable Collection o) { this.owaspTop10 = o; return this; diff --git a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java index 8312ff90cf4..9ec67e24fb9 100644 --- a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java +++ b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java @@ -775,6 +775,58 @@ public class IssueIndexFiltersTest extends IssueIndexTestCommon { assertThatSearchReturnsOnly(IssueQuery.builder().cwe(singletonList("20")), "I1"); } + @Test + public void filter_by_owaspAsvs40_category() { + ComponentDto project = newPrivateProjectDto(); + ComponentDto file = newFileDto(project, null); + + indexIssues( + newDoc("I1", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(asList("1.1.1", "1.2.2", "2.2.2")), + newDoc("I2", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(asList("1.1.1", "1.2.2")), + newDoc("I3", file)); + + assertThatSearchReturnsOnly(IssueQuery.builder().owaspAsvs40(singletonList("1")), "I1", "I2"); + assertThatSearchReturnsOnly(IssueQuery.builder().owaspAsvs40(singletonList("2")), "I1"); + assertThatSearchReturnsOnly(IssueQuery.builder().owaspAsvs40(singletonList("3"))); + } + + @Test + public void filter_by_owaspAsvs40_specific_requirement() { + ComponentDto project = newPrivateProjectDto(); + ComponentDto file = newFileDto(project, null); + + indexIssues( + newDoc("I1", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(asList("1.1.1", "1.2.2", "2.2.2")), + newDoc("I2", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(asList("1.1.1", "1.2.2")), + newDoc("I3", file)); + + assertThatSearchReturnsOnly(IssueQuery.builder().owaspAsvs40(singletonList("1.1.1")), "I1", "I2"); + assertThatSearchReturnsOnly(IssueQuery.builder().owaspAsvs40(singletonList("2.2.2")), "I1"); + assertThatSearchReturnsOnly(IssueQuery.builder().owaspAsvs40(singletonList("3.3.3"))); + } + + @Test + public void filter_by_owaspAsvs40_level() { + ComponentDto project = newPrivateProjectDto(); + ComponentDto file = newFileDto(project, null); + + indexIssues( + newDoc("I1", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(asList("2.1.1", "1.1.1", "1.11.3")), + newDoc("I2", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(asList("1.1.1", "1.11.3")), + newDoc("I3", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(singletonList("1.11.3")), + newDoc("IError1", file).setType(RuleType.VULNERABILITY).setOwaspAsvs40(asList("5.5.1", "7.2.2", "10.2.6")), + newDoc("IError2", file)); + + assertThatSearchReturnsOnly( + IssueQuery.builder().owaspAsvs40(singletonList("1.1.1")).owaspAsvsLevel(1)); + assertThatSearchReturnsOnly( + IssueQuery.builder().owaspAsvs40(singletonList("1.1.1")).owaspAsvsLevel(2), + "I1", "I2"); + assertThatSearchReturnsOnly( + IssueQuery.builder().owaspAsvs40(singletonList("1.1.1")).owaspAsvsLevel(3), + "I1", "I2"); + } + @Test public void filter_by_owaspTop10() { ComponentDto project = newPrivateProjectDto(); diff --git a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryTest.java b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryTest.java index 24aa3bec6a9..9122b089ff7 100644 --- a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryTest.java +++ b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryTest.java @@ -105,9 +105,11 @@ public class IssueQueryTest { public void build_owasp_asvs_query() { IssueQuery query = IssueQuery.builder() .owaspAsvs40(List.of("1.2.3", "3.2.1")) + .owaspAsvsLevel(2) .build(); assertThat(query.owaspAsvs40()).containsOnly("1.2.3", "3.2.1"); + assertThat(query.getOwaspAsvsLevel()).isPresent().hasValue(2); } @Test @@ -121,6 +123,7 @@ public class IssueQueryTest { assertThat(query.owaspTop10For2021()).containsOnly("a3", "a4"); } + @Test public void build_query_without_dates() { IssueQuery query = IssueQuery.builder() diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java index 295de299b02..ac4146c2966 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java @@ -199,6 +199,7 @@ public class SearchAction implements HotspotsWsAction { } if (!wsRequest.getOwaspAsvs40().isEmpty()) { builder.owaspAsvs40(wsRequest.getOwaspAsvs40()); + wsRequest.getOwaspAsvsLevel().ifPresent(builder::owaspAsvsLevel); } if (!wsRequest.getOwaspTop10For2017().isEmpty()) { builder.owaspTop10(wsRequest.getOwaspTop10For2017()); @@ -269,6 +270,7 @@ public class SearchAction implements HotspotsWsAction { .setDescription("Filters hotspots with lower or equal OWASP ASVS level to the parameter value. Should be used in combination with the 'owaspAsvs-4.0' parameter.") .setSince("9.7") .setPossibleValues(1, 2, 3) + .setRequired(false) .setExampleValue("2"); action.createParam(PARAM_PCI_DSS_32) .setDescription("Comma-separated list of PCI DSS v3.2 categories.") @@ -449,13 +451,6 @@ public class SearchAction implements HotspotsWsAction { checkArgument(wsRequest.getProjectKey().isPresent(), "Parameter '%s' can be used with parameter '%s' only", PARAM_ONLY_MINE, PARAM_PROJECT_KEY); } - Set validLevels = Set.of(1, 2, 3); - String levelsStringValue = "{1, 2, 3}"; - wsRequest.getOwaspAsvsLevel().ifPresent( - oal -> checkArgument(validLevels.contains(oal), - "value \"%d\" is not valid for parameter %s. only one of %s is acceptable.", - oal, PARAM_OWASP_ASVS_LEVEL, levelsStringValue) - ); } private static void addMainBranchFilter(@NotNull ComponentDto project, IssueQuery.Builder builder) { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java index b54499c50ba..951f51c00ac 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java @@ -125,6 +125,7 @@ public class SearchActionTest { private static final String PARAM_PCI_DSS_32 = "pciDss-3.2"; private static final String PARAM_PCI_DSS_40 = "pciDss-4.0"; private static final String PARAM_OWASP_ASVS_40 = "owaspAsvs-4.0"; + private static final String PARAM_OWASP_ASVS_LEVEL = "owaspAsvsLevel"; private static final String PARAM_OWASP_TOP_10_2017 = "owaspTop10"; private static final String PARAM_OWASP_TOP_10_2021 = "owaspTop10-2021"; private static final String PARAM_SANS_TOP_25 = "sansTop25"; @@ -1235,7 +1236,7 @@ public class SearchActionTest { newHotspotLocation(file.uuid(), "security hotspot flow message 1", 3, 3, 0, 10), newHotspotLocation(anotherFile.uuid(), "security hotspot flow message 2", 5, 5, 0, 15), newHotspotLocation(anotherFile.uuid(), "security hotspot flow message 3", 7, 7, 0, 18), - newHotspotLocation(null,"security hotspot flow message 4", 12, 12, 2, 8)) + newHotspotLocation(null, "security hotspot flow message 4", 12, 12, 2, 8)) .collect(toList()); DbIssues.Locations.Builder locations = DbIssues.Locations.newBuilder().addFlow(DbIssues.Flow.newBuilder().addAllLocation(hotspotLocations)); @@ -1264,7 +1265,7 @@ public class SearchActionTest { tuple(file.getKey(), "security hotspot flow message 1", 3, 3, 0, 10), tuple(anotherFile.getKey(), "security hotspot flow message 2", 5, 5, 0, 15), tuple(anotherFile.getKey(), "security hotspot flow message 3", 7, 7, 0, 18), - tuple(file.getKey(),"security hotspot flow message 4", 12, 12, 2, 8)); + tuple(file.getKey(), "security hotspot flow message 4", 12, 12, 2, 8)); } @Test @@ -1530,6 +1531,61 @@ public class SearchActionTest { .containsExactly(hotspot4.getKey()); } + @Test + public void returns_hotspots_with_specified_owaspAsvs_level() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDto rule1 = newRule(SECURITY_HOTSPOT); + RuleDto rule2 = newRule(SECURITY_HOTSPOT, r -> r.setSecurityStandards(of("cwe:117", "cwe:190"))); + RuleDto rule3 = newRule(SECURITY_HOTSPOT, r -> r.setSecurityStandards(of("owaspAsvs-4.0:2.1.1"))); + RuleDto rule4 = newRule(SECURITY_HOTSPOT, r -> r.setSecurityStandards(of("owaspAsvs-4.0:1.1.1"))); + RuleDto rule5 = newRule(SECURITY_HOTSPOT, r -> r.setSecurityStandards(of("owaspAsvs-4.0:3.6.1"))); + insertHotspot(project, file, rule1); + insertHotspot(project, file, rule2); + IssueDto hotspot3 = insertHotspot(project, file, rule3); + IssueDto hotspot4 = insertHotspot(project, file, rule4); + IssueDto hotspot5 = insertHotspot(project, file, rule5); + indexIssues(); + + SearchWsResponse responseFor1 = newRequest(project) + .setParam(PARAM_OWASP_ASVS_40, "1,2,3") + .setParam(PARAM_OWASP_ASVS_LEVEL, "1") + .executeProtobuf(SearchWsResponse.class); + + assertThat(responseFor1.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsExactlyInAnyOrder(hotspot3.getKey()); + + SearchWsResponse responseFor2 = newRequest(project) + .setParam(PARAM_OWASP_ASVS_40, "1,2,3") + .setParam(PARAM_OWASP_ASVS_LEVEL, "2") + .executeProtobuf(SearchWsResponse.class); + + assertThat(responseFor2.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsExactlyInAnyOrder(hotspot3.getKey(), hotspot4.getKey()); + + SearchWsResponse responseFor3 = newRequest(project) + .setParam(PARAM_OWASP_ASVS_40, "1.1.1,2,3") + .setParam(PARAM_OWASP_ASVS_LEVEL, "3") + .executeProtobuf(SearchWsResponse.class); + + assertThat(responseFor3.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsExactlyInAnyOrder(hotspot3.getKey(), hotspot4.getKey(), hotspot5.getKey()); + + SearchWsResponse responseFor1111 = newRequest(project) + .setParam(PARAM_OWASP_ASVS_40, "1.1.1") + .setParam(PARAM_OWASP_ASVS_LEVEL, "1") + .executeProtobuf(SearchWsResponse.class); + + assertThat(responseFor1111.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .isEmpty(); + } + @Test public void returns_hotspots_with_specified_owasp2021Top10_category() { ComponentDto project = dbTester.components().insertPublicProject(); @@ -1877,8 +1933,8 @@ public class SearchActionTest { IssueDto[] hotspots = IntStream.range(0, 3) .mapToObj(i -> { - RuleKey ruleKey = RuleKey.of("repository-"+i,"rule-"+i); - RuleDto rule = newRule(SECURITY_HOTSPOT,ruleKey) + RuleKey ruleKey = RuleKey.of("repository-" + i, "rule-" + i); + RuleDto rule = newRule(SECURITY_HOTSPOT, ruleKey) .setSecurityStandards(Sets.newHashSet(SQCategory.WEAK_CRYPTOGRAPHY.getKey())); return insertHotspot(rule, project, fileWithHotspot, issueDto -> issueDto.setKee("hotspot-" + i) .setAssigneeUuid("assignee-uuid") @@ -2015,7 +2071,7 @@ public class SearchActionTest { }); } - private RuleDto newRule(RuleType ruleType, RuleKey ruleKey){ + private RuleDto newRule(RuleType ruleType, RuleKey ruleKey) { RuleDto ruleDto = RuleTesting.newRule(ruleKey) .setType(ruleType); dbTester.rules().insert(ruleDto); -- 2.39.5