SONARJAVA-4883 S4423: Support detection of TLS Protocol Downgrades fo…

…r Spring programmatically (#4749)

its/autoscan/src/test/resources/autoscan/diffs/diff_S4423.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S4423",
   "hasTruePositives": true,
-  "falseNegatives": 7,
+  "falseNegatives": 10,
   "falsePositives": 0
-}
+}

java-checks-test-sources/spring-3.2/pom.xml

@@ -34,6 +34,18 @@
       <version>6.1.5</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.springframework.boot</groupId>
+      <artifactId>spring-boot-autoconfigure</artifactId>
+      <version>3.2.4</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.springframework</groupId>
+      <artifactId>spring-context</artifactId>
+      <version>6.1.5</version>
+      <scope>provided</scope>
+    </dependency>
     <dependency>
       <groupId>org.springframework</groupId>
       <artifactId>spring-webmvc</artifactId>
@@ -43,7 +55,7 @@
     <dependency>
       <groupId>org.springframework.security</groupId>
       <artifactId>spring-security-web</artifactId>
-      <version>6.2.1</version>
+      <version>6.2.3</version>
       <scope>provided</scope>
     </dependency>
   </dependencies>

java-checks-test-sources/spring-3.2/src/main/java/checks/WeakSSLContextCheckSample.java

@@ -0,0 +1,40 @@
+package checks;
+
+import java.util.HashSet;
+import java.util.Set;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.ssl.PropertiesSslBundle;
+import org.springframework.boot.autoconfigure.ssl.SslProperties;
+import org.springframework.boot.ssl.DefaultSslBundleRegistry;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration
+public class WeakSSLContextCheckSample {
+
+  public static final String TLSV_1_0 = "TLSv1.0";
+
+  @Autowired
+  public WeakSSLContextCheckSample(SslProperties props, DefaultSslBundleRegistry registry, Set<String> propsSet) {
+    props.getBundle().getJks().get("").getOptions().setEnabledProtocols(Set.of("TLSv1.1", // Noncompliant [[sc=53;ec=72;secondary=18,20]] {{Change this code to use a stronger protocol.}}
+      "test",
+      "TLSv1.0"));
+    props.getBundle().getJks().get("").getOptions().setEnabledProtocols(Set.of("TLSv1.0")); // Noncompliant
+
+    props.getBundle().getJks().get("").getOptions().setEnabledProtocols(Set.of(TLSV_1_0)); // Noncompliant [[sc=53;ec=72;secondary=23]] {{Change this code to use a stronger protocol.}}
+    props.getBundle().getJks().get("").getOptions().setEnabledProtocols(Set.of(null)); // coverage
+
+    props.getBundle().getJks().get("").getOptions().setEnabledProtocols(Set.of("TLSv1")); // Compliant
+    props.getBundle().getJks().get("").getOptions().setEnabledProtocols(getSet()); // Compliant - FN
+    props.getBundle().getJks().get("").getOptions().setEnabledProtocols(propsSet); // Compliant
+    props.getBundle().getJks().get("").getOptions().getCiphers(); // coverage
+
+    registry.updateBundle("", PropertiesSslBundle.get(props.getBundle().getJks().get("")));
+  }
+
+  Set<String> getSet() {
+    Set<String> set = new HashSet<>();
+    set.add("TLSv1.1");
+    return set;
+  }
+
+}

java-checks/src/main/java/org/sonar/java/checks/WeakSSLContextCheck.java

@@ -26,6 +26,7 @@
 import java.util.Optional;
 import java.util.Set;
 import org.sonar.check.Rule;
+import org.sonar.java.model.ExpressionUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
 import org.sonar.plugins.java.api.semantic.MethodMatchers;
@@ -39,9 +40,13 @@
 @Rule(key = "S4423")
 public class WeakSSLContextCheck extends IssuableSubscriptionVisitor {
 
+  private static final String ISSUE_MESSAGE = "Change this code to use a stronger protocol.";
+  private static final String SECONDARY_LOCATION_MESSAGE = "Other weak protocol.";
+
   private static final Set<String> STRONG_PROTOCOLS = new HashSet<>(Arrays.asList("TLSv1.2", "DTLSv1.2", "TLSv1.3", "DTLSv1.3"));
   private static final Set<String> STRONG_AFTER_JAVA_8 = new HashSet<>(Arrays.asList("TLS", "DTLS"));
   private static final Set<String> WEAK_FOR_OK_HTTP = new HashSet<>(Arrays.asList("TLSv1", "TLSv1.1", "TLS_1_0", "TLS_1_1"));
+  private static final Set<String> WEAK_FOR_SET_ENABLED_PROTOCOLS = new HashSet<>(Set.of("TLSv1.0", "TLSv1.1"));
 
   private static final MethodMatchers SSLCONTEXT_GETINSTANCE_MATCHER = MethodMatchers.create()
     .ofTypes("javax.net.ssl.SSLContext")
@@ -55,6 +60,12 @@ public class WeakSSLContextCheck extends IssuableSubscriptionVisitor {
     .withAnyParameters()
     .build();
 
+  private static final MethodMatchers OPTIONS_ENABLED_PROTOCOLS = MethodMatchers.create()
+    .ofTypes("org.springframework.boot.autoconfigure.ssl.SslBundleProperties$Options")
+    .names("setEnabledProtocols")
+    .addParametersMatcher("java.util.Set")
+    .build();
+
   private boolean javaVersionNotSetOr8OrHigher;
 
   @Override
@@ -76,17 +87,31 @@ public void visitNode(Tree tree) {
       ExpressionTree firstArgument = arguments.get(0);
       firstArgument.asConstant(String.class).ifPresent(protocol -> {
         if (!isStrongProtocol(protocol)) {
-          reportIssue(firstArgument, "Change this code to use a stronger protocol.");
+          reportIssue(firstArgument, ISSUE_MESSAGE);
         }
       });
     } else if (OK_HTTP_TLS_VERSION.matches(mit)) {
       List<ExpressionTree> unsecureVersions = getUnsecureVersionsInArguments(arguments);
       if (!unsecureVersions.isEmpty()) {
         List<JavaFileScannerContext.Location> secondaries = unsecureVersions.stream()
           .skip(1)
-          .map(secondary -> new JavaFileScannerContext.Location("Other weak protocol.", secondary))
+          .map(secondary -> new JavaFileScannerContext.Location(SECONDARY_LOCATION_MESSAGE, secondary))
           .toList();
-        reportIssue(unsecureVersions.get(0), "Change this code to use a stronger protocol.", secondaries, null);
+        reportIssue(unsecureVersions.get(0), ISSUE_MESSAGE, secondaries, null);
+      }
+    } else if (OPTIONS_ENABLED_PROTOCOLS.matches(mit)) {
+      ExpressionTree argument = arguments.get(0);
+      if (argument instanceof MethodInvocationTree methodInvocation) {
+        List<JavaFileScannerContext.Location> secondaryLocations = methodInvocation.arguments().stream()
+          .filter(arg -> {
+            var argValue = ExpressionUtils.resolveAsConstant(arg);
+            return argValue != null && WEAK_FOR_SET_ENABLED_PROTOCOLS.contains(argValue);
+          })
+          .map(arg -> new JavaFileScannerContext.Location(SECONDARY_LOCATION_MESSAGE, arg))
+          .toList();
+        if (!secondaryLocations.isEmpty()) {
+          reportIssue(((MemberSelectExpressionTree) mit.methodSelect()).identifier(), ISSUE_MESSAGE, secondaryLocations, null);
+        }
       }
     }
   }

java-checks/src/test/java/org/sonar/java/checks/WeakSSLContextCheckTest.java

@@ -19,13 +19,19 @@
  */
 package org.sonar.java.checks;
 
+import java.io.File;
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.sonar.java.checks.verifier.CheckVerifier;
+import org.sonar.java.test.classpath.TestClasspathUtils;
 
 import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPathInModule;
 
 class WeakSSLContextCheckTest {
 
+  private static final List<File> SPRING_3_2_CLASSPATH = TestClasspathUtils.loadFromFile(Constants.SPRING_3_2_CLASSPATH);
+
   @Test
   void test() {
     CheckVerifier.newVerifier()
@@ -52,4 +58,13 @@ void test_java_8() {
       .verifyIssues();
   }
 
+  @Test
+  void test_with_spring_3_2() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPathInModule(Constants.SPRING_3_2, "checks/WeakSSLContextCheckSample.java"))
+      .withCheck(new WeakSSLContextCheck())
+      .withClassPath(SPRING_3_2_CLASSPATH)
+      .verifyIssues();
+  }
+
 }