]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-17399 add owasp asvs 4.0 level support in hotspot search API
authorSteve Marion <steve.marion@sonarsource.com>
Fri, 7 Oct 2022 13:48:50 +0000 (15:48 +0200)
committersonartech <sonartech@sonarsource.com>
Mon, 10 Oct 2022 20:03:09 +0000 (20:03 +0000)
server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java
server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java
server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java
server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java
server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryTest.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java

index e450c323e928e7467d5deba63d7610b54dd90ae4..1661d13cf7adc4d95c4e5c51b6445431d18143ad 100644 (file)
@@ -330,12 +330,16 @@ public final class SecurityStandards {
     return new SecurityStandards(standards, cwe, sqCategory, ignoredSQCategories);
   }
 
-  public static Set<String> getRequirementsForCategoryAndLevel(OwaspAsvs category, int level) {
+  public static Set<String> 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<String> getRequirementsForCategoryAndLevel(OwaspAsvs category, int level) {
+    return getRequirementsForCategoryAndLevel(category.category(), level);
+  }
+
   private static Set<String> getMatchingStandards(Set<String> securityStandards, String prefix) {
     return securityStandards.stream()
       .filter(s -> s.startsWith(prefix))
index 84af82f9e5ac047fbd11c6aeb105cf3951dc4178..25ff1fb637a62d38fc23fca0b53cd6be91a50d80 100644 (file)
@@ -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<String> 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<String> calculateRequirementsForOwaspAsvs40Params(IssueQuery query) {
+    int level = query.getOwaspAsvsLevel().orElse(3);
+    List<String> 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<String> 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<String> facetNames = options.getFacets();
     Set<TopAggregationDefinition<?>> 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);
     }
   }
index 38c3a2299bab3624e5d5632f1456d4f38db4bd29..9721db7df82bb06fba753183b52048625a65b657 100644 (file)
@@ -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<String> pciDss32;
   private final Collection<String> pciDss40;
   private final Collection<String> owaspAsvs40;
+  private final Integer owaspAsvsLevel;
   private final Collection<String> owaspTop10For2021;
   private final Collection<String> sansTop25;
   private final Collection<String> 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<String> owaspAsvs40() {
     return owaspAsvs40;
   }
+
+  public Optional<Integer> getOwaspAsvsLevel() {
+    return Optional.ofNullable(owaspAsvsLevel);
+  }
+
   public Collection<String> owaspTop10() {
     return owaspTop10;
   }
@@ -353,6 +361,7 @@ public class IssueQuery {
     private Collection<String> pciDss32;
     private Collection<String> pciDss40;
     private Collection<String> owaspAsvs40;
+    private Integer owaspAsvsLevel;
     private Collection<String> owaspTop10;
     private Collection<String> owaspTop10For2021;
     private Collection<String> 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<String> o) {
       this.owaspTop10 = o;
       return this;
index 8312ff90cf4d6932261c35022ef3c023b47bf54e..9ec67e24fb9bf4270f3f3b7ea2579a725ff78bc7 100644 (file)
@@ -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();
index 24aa3bec6a9cce5037e0e365cd19cf5f4c0f0791..9122b089ff7d0f89d77b60807f6ac5de54c8ed29 100644 (file)
@@ -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()
index 295de299b02ae0656e1e68a21523234db99b84e4..ac4146c29660131481ea537e2d6ffd6e7d0be4f6 100644 (file)
@@ -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<Integer> 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) {
index b54499c50ba4cf41e7738db8f49087f7c2bfda9f..951f51c00ac1a4b68df5fba354f8868fec17e7c8 100644 (file)
@@ -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);