SONAR-23680 Support changeit as default keystore password

6 files changed, 110 insertions, 27 deletions

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ScannerWsClientProvider.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ScannerWsClientProvider.java
index 67e224f8845..f11e481d749 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ScannerWsClientProvider.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ScannerWsClientProvider.java
@@ -32,8 +32,10 @@ import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
 import java.time.Duration;
 import java.time.format.DateTimeParseException;
+import javax.annotation.Nullable;
 import nl.altindag.ssl.SSLFactory;
 import nl.altindag.ssl.exception.GenericKeyStoreException;
+import nl.altindag.ssl.util.KeyStoreUtils;
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -137,10 +139,10 @@ public class ScannerWsClientProvider {
 
   private static SslConfig parseSslConfig(ScannerProperties scannerProperties, SonarUserHome sonarUserHome) {
     var keyStorePath = defaultIfBlank(scannerProperties.property("sonar.scanner.keystorePath"), sonarUserHome.getPath().resolve("ssl/keystore.p12").toString());
-    var keyStorePassword = defaultIfBlank(scannerProperties.property("sonar.scanner.keystorePassword"), CertificateStore.DEFAULT_PASSWORD);
-    var trustStorePath = defaultIfBlank(scannerProperties.property("sonar.scanner.truststorePath"), sonarUserHome.getPath().resolve("ssl/truststore.p12").toString());
-    var trustStorePassword = defaultIfBlank(scannerProperties.property("sonar.scanner.truststorePassword"), CertificateStore.DEFAULT_PASSWORD);
+    var keyStorePassword = scannerProperties.property("sonar.scanner.keystorePassword");
     var keyStore = new CertificateStore(Path.of(keyStorePath), keyStorePassword);
+    var trustStorePath = defaultIfBlank(scannerProperties.property("sonar.scanner.truststorePath"), sonarUserHome.getPath().resolve("ssl/truststore.p12").toString());
+    var trustStorePassword = scannerProperties.property("sonar.scanner.truststorePassword");
     var trustStore = new CertificateStore(Path.of(trustStorePath), trustStorePassword);
     return new SslConfig(keyStore, trustStore);
   }
@@ -154,15 +156,18 @@ public class ScannerWsClientProvider {
     }
     var keyStoreConfig = sslConfig.getKeyStore();
     if (keyStoreConfig != null && Files.exists(keyStoreConfig.getPath())) {
-      sslFactoryBuilder.withIdentityMaterial(keyStoreConfig.getPath(), keyStoreConfig.getKeyStorePassword().toCharArray(), keyStoreConfig.getKeyStoreType());
+      keyStoreConfig.getKeyStorePassword()
+        .ifPresentOrElse(
+          password -> sslFactoryBuilder.withIdentityMaterial(keyStoreConfig.getPath(), password.toCharArray(), keyStoreConfig.getKeyStoreType()),
+          () -> loadIdentityMaterialWithDefaultPassword(sslFactoryBuilder, keyStoreConfig.getPath()));
     }
     var trustStoreConfig = sslConfig.getTrustStore();
     if (trustStoreConfig != null && Files.exists(trustStoreConfig.getPath())) {
       KeyStore trustStore;
       try {
-        trustStore = loadKeyStoreWithBouncyCastle(
+        trustStore = loadTrustStoreWithBouncyCastle(
           trustStoreConfig.getPath(),
-          trustStoreConfig.getKeyStorePassword().toCharArray(),
+          trustStoreConfig.getKeyStorePassword().orElse(null),
           trustStoreConfig.getKeyStoreType());
         LOG.debug("Loaded truststore from '{}' containing {} certificates", trustStoreConfig.getPath(), trustStore.size());
       } catch (KeyStoreException | IOException | CertificateException | NoSuchAlgorithmException e) {
@@ -173,12 +178,36 @@ public class ScannerWsClientProvider {
     return sslFactoryBuilder.build();
   }
 
-  static KeyStore loadKeyStoreWithBouncyCastle(Path keystorePath, char[] keystorePassword, String keystoreType) throws IOException,
+  private static void loadIdentityMaterialWithDefaultPassword(SSLFactory.Builder sslFactoryBuilder, Path path) {
+    try {
+      var keystore = KeyStoreUtils.loadKeyStore(path, CertificateStore.DEFAULT_PASSWORD.toCharArray(), CertificateStore.DEFAULT_STORE_TYPE);
+      sslFactoryBuilder.withIdentityMaterial(keystore, CertificateStore.DEFAULT_PASSWORD.toCharArray());
+    } catch (GenericKeyStoreException e) {
+      var keystore = KeyStoreUtils.loadKeyStore(path, CertificateStore.OLD_DEFAULT_PASSWORD.toCharArray(), CertificateStore.DEFAULT_STORE_TYPE);
+      LOG.warn("Using deprecated default password for keystore '{}'.", path);
+      sslFactoryBuilder.withIdentityMaterial(keystore, CertificateStore.OLD_DEFAULT_PASSWORD.toCharArray());
+    }
+  }
+
+  static KeyStore loadTrustStoreWithBouncyCastle(Path keystorePath, @Nullable String keystorePassword, String keystoreType) throws IOException,
     KeyStoreException, CertificateException, NoSuchAlgorithmException {
+    KeyStore keystore = KeyStore.getInstance(keystoreType, new BouncyCastleProvider());
+    if (keystorePassword != null) {
+      loadKeyStoreWithPassword(keystorePath, keystore, keystorePassword);
+    } else {
+      try {
+        loadKeyStoreWithPassword(keystorePath, keystore, CertificateStore.DEFAULT_PASSWORD);
+      } catch (Exception e) {
+        loadKeyStoreWithPassword(keystorePath, keystore, CertificateStore.OLD_DEFAULT_PASSWORD);
+        LOG.warn("Using deprecated default password for truststore '{}'.", keystorePath);
+      }
+    }
+    return keystore;
+  }
+
+  private static void loadKeyStoreWithPassword(Path keystorePath, KeyStore keystore, String oldDefaultPassword) throws IOException, NoSuchAlgorithmException, CertificateException {
     try (InputStream keystoreInputStream = Files.newInputStream(keystorePath, StandardOpenOption.READ)) {
-      KeyStore keystore = KeyStore.getInstance(keystoreType, new BouncyCastleProvider());
-      keystore.load(keystoreInputStream, keystorePassword);
-      return keystore;
+      keystore.load(keystoreInputStream, oldDefaultPassword.toCharArray());
     }
   }
 
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ssl/CertificateStore.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ssl/CertificateStore.java
index b285d3bb478..79d490cb12e 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ssl/CertificateStore.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/http/ssl/CertificateStore.java
@@ -20,15 +20,22 @@
 package org.sonar.scanner.http.ssl;
 
 import java.nio.file.Path;
+import java.util.Optional;
+import javax.annotation.Nullable;
 
 public class CertificateStore {
-  public static final String DEFAULT_PASSWORD = "sonar";
+  public static final String DEFAULT_PASSWORD = "changeit";
+  /**
+   * @deprecated it was a bad decision to use this value as default password, as the keytool utility requires a password to be at least 6 characters long
+   */
+  @Deprecated(since = "11.4")
+  public static final String OLD_DEFAULT_PASSWORD = "sonar";
   public static final String DEFAULT_STORE_TYPE = "PKCS12";
   private final Path path;
   private final String keyStorePassword;
   private final String keyStoreType;
 
-  public CertificateStore(Path path, String keyStorePassword) {
+  public CertificateStore(Path path, @Nullable String keyStorePassword) {
     this.path = path;
     this.keyStorePassword = keyStorePassword;
     this.keyStoreType = DEFAULT_STORE_TYPE;
@@ -38,8 +45,8 @@ public class CertificateStore {
     return path;
   }
 
-  public String getKeyStorePassword() {
-    return keyStorePassword;
+  public Optional<String> getKeyStorePassword() {
+    return Optional.ofNullable(keyStorePassword);
   }
 
   public String getKeyStoreType() {
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/http/ScannerWsClientProviderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/http/ScannerWsClientProviderTest.java
index 3aea5460db9..b041be11fbb 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/http/ScannerWsClientProviderTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/http/ScannerWsClientProviderTest.java
@@ -30,6 +30,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
+import javax.annotation.Nullable;
 import nl.altindag.ssl.exception.GenericKeyStoreException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
@@ -38,6 +39,8 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junitpioneer.jupiter.RestoreSystemProperties;
 import org.sonar.api.notifications.AnalysisWarnings;
 import org.sonar.api.utils.System2;
@@ -58,6 +61,8 @@ import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMoc
 import static com.github.tomakehurst.wiremock.stubbing.Scenario.STARTED;
 import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -109,6 +114,61 @@ class ScannerWsClientProviderTest {
     assertThat(httpConnector.okHttpClient().proxy()).isNull();
   }
 
+  @ParameterizedTest
+  @CsvSource({
+    "keystore_changeit.p12, wrong,        false",
+    "keystore_changeit.p12, changeit,     true",
+    "keystore_changeit.p12,,              true",
+    "keystore_sonar.p12,    wrong,        false",
+    "keystore_sonar.p12,    sonar,        true",
+    "keystore_sonar.p12,,                 true",
+    "keystore_anotherpwd.p12, wrong,      false",
+    "keystore_anotherpwd.p12, anotherpwd, true",
+    "keystore_anotherpwd.p12,,            false"})
+  void it_should_fail_if_invalid_truststore_password(String keystore, @Nullable String password, boolean shouldSucceed) {
+    scannerProps.put("sonar.scanner.truststorePath", toPath(requireNonNull(ScannerWsClientProviderTest.class.getResource("/ssl/" + keystore))).toString());
+    if (password != null) {
+      scannerProps.put("sonar.scanner.truststorePassword", password);
+    }
+
+    var scannerPropsObj = new ScannerProperties(scannerProps);
+    if (shouldSucceed) {
+      assertThatNoException().isThrownBy(() -> underTest.provide(scannerPropsObj, env, GLOBAL_ANALYSIS_MODE, system2, ANALYSIS_WARNINGS, sonarUserHome));
+    } else {
+      assertThatThrownBy(() -> underTest.provide(scannerPropsObj, env, GLOBAL_ANALYSIS_MODE, system2, ANALYSIS_WARNINGS, sonarUserHome))
+        .isInstanceOf(GenericKeyStoreException.class)
+        .hasMessageContaining("Unable to read truststore from")
+        .hasStackTraceContaining("wrong password or corrupted file");
+    }
+  }
+
+  @ParameterizedTest
+  @CsvSource({
+    "keystore_changeit.p12, wrong,        false",
+    "keystore_changeit.p12, changeit,     true",
+    "keystore_changeit.p12,,              true",
+    "keystore_sonar.p12,    wrong,        false",
+    "keystore_sonar.p12,    sonar,        true",
+    "keystore_sonar.p12,,                 true",
+    "keystore_anotherpwd.p12, wrong,      false",
+    "keystore_anotherpwd.p12, anotherpwd, true",
+    "keystore_anotherpwd.p12,,            false"})
+  void it_should_fail_if_invalid_keystore_password(String keystore, @Nullable String password, boolean shouldSucceed) {
+    scannerProps.put("sonar.scanner.keystorePath", toPath(requireNonNull(ScannerWsClientProviderTest.class.getResource("/ssl/" + keystore))).toString());
+    if (password != null) {
+      scannerProps.put("sonar.scanner.keystorePassword", password);
+    }
+
+    var scannerPropsObj = new ScannerProperties(scannerProps);
+    if (shouldSucceed) {
+      assertThatNoException().isThrownBy(() -> underTest.provide(scannerPropsObj, env, GLOBAL_ANALYSIS_MODE, system2, ANALYSIS_WARNINGS, sonarUserHome));
+    } else {
+      assertThatThrownBy(() -> underTest.provide(scannerPropsObj, env, GLOBAL_ANALYSIS_MODE, system2, ANALYSIS_WARNINGS, sonarUserHome))
+        .isInstanceOf(GenericKeyStoreException.class)
+        .hasMessageContaining("keystore password was incorrect");
+    }
+  }
+
   @Nested
   class WithMockHttpSonarQube {
 
@@ -320,19 +380,6 @@ class ScannerWsClientProviderTest {
         assertThat(r.content()).isEqualTo("Success");
       }
     }
-
-    @Test
-    void it_should_fail_if_invalid_truststore_password() {
-      scannerProps.put("sonar.host.url", sonarqubeMock.baseUrl());
-      scannerProps.put("sonar.scanner.truststorePath", toPath(requireNonNull(ScannerWsClientProviderTest.class.getResource("/ssl/client-truststore.p12"))).toString());
-      scannerProps.put("sonar.scanner.truststorePassword", "wrong_password");
-
-      var scannerPropsObj = new ScannerProperties(scannerProps);
-      var thrown = assertThrows(GenericKeyStoreException.class,
-        () -> underTest.provide(scannerPropsObj, env, GLOBAL_ANALYSIS_MODE, system2, ANALYSIS_WARNINGS, sonarUserHome));
-
-      assertThat(thrown).hasStackTraceContaining("Unable to read truststore");
-    }
   }
 
   @Nested
diff --git a/sonar-scanner-engine/src/test/resources/ssl/keystore_anotherpwd.p12 b/sonar-scanner-engine/src/test/resources/ssl/keystore_anotherpwd.p12
new file mode 100644
index 00000000000..1f01b2a828d
--- /dev/null
+++ b/sonar-scanner-engine/src/test/resources/ssl/keystore_anotherpwd.p12
diff --git a/sonar-scanner-engine/src/test/resources/ssl/keystore_changeit.p12 b/sonar-scanner-engine/src/test/resources/ssl/keystore_changeit.p12
new file mode 100644
index 00000000000..0238cdef48d
--- /dev/null
+++ b/sonar-scanner-engine/src/test/resources/ssl/keystore_changeit.p12
diff --git a/sonar-scanner-engine/src/test/resources/ssl/keystore_sonar.p12 b/sonar-scanner-engine/src/test/resources/ssl/keystore_sonar.p12
new file mode 100644
index 00000000000..262e591f69c
--- /dev/null
+++ b/sonar-scanner-engine/src/test/resources/ssl/keystore_sonar.p12