Skip to content

Commit 69c1f4c

Browse files
henryjuhenryju
committed
SCANJLIB-237 Support changeit as default keystore password
1 parent 55ef6ec commit 69c1f4c

8 files changed

Lines changed: 130 additions & 27 deletions

File tree

‎lib/src/main/java/org/sonarsource/scanner/lib/ScannerEngineBootstrapper.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.sonarsource.scanner.lib.internal.cache.FileCache;
4343
import org.sonarsource.scanner.lib.internal.http.HttpConfig;
4444
import org.sonarsource.scanner.lib.internal.http.ScannerHttpClient;
45+
import org.sonarsource.scanner.lib.internal.http.ssl.CertificateStore;
4546
import org.sonarsource.scanner.lib.internal.util.VersionUtils;
4647

4748
import static org.sonarsource.scanner.lib.ScannerProperties.SCANNER_ARCH;
@@ -170,12 +171,12 @@ Map<String, String> adaptDeprecatedPropertiesForInProcessBootstrapping(Map<Strin
170171
var keyStore = httpConfig.getSslConfig().getKeyStore();
171172
if (keyStore != null) {
172173
setSystemPropertyIfNotAlreadySet("javax.net.ssl.keyStore", keyStore.getPath().toString());
173-
setSystemPropertyIfNotAlreadySet("javax.net.ssl.keyStorePassword", keyStore.getKeyStorePassword());
174+
setSystemPropertyIfNotAlreadySet("javax.net.ssl.keyStorePassword", keyStore.getKeyStorePassword().orElse(CertificateStore.DEFAULT_PASSWORD));
174175
}
175176
var trustStore = httpConfig.getSslConfig().getTrustStore();
176177
if (trustStore != null) {
177178
setSystemPropertyIfNotAlreadySet("javax.net.ssl.trustStore", trustStore.getPath().toString());
178-
setSystemPropertyIfNotAlreadySet("javax.net.ssl.trustStorePassword", trustStore.getKeyStorePassword());
179+
setSystemPropertyIfNotAlreadySet("javax.net.ssl.trustStorePassword", trustStore.getKeyStorePassword().orElse(CertificateStore.DEFAULT_PASSWORD));
179180
}
180181

181182
return Map.copyOf(adaptedProperties);

‎lib/src/main/java/org/sonarsource/scanner/lib/internal/http/HttpConfig.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,7 @@ private static CertificateStore loadTrustStoreConfig(Map<String, String> bootstr
165165
var trustStorePath = parseFileProperty(bootstrapProperties, SONAR_SCANNER_TRUSTSTORE_PATH, "truststore", sonarUserHome.resolve("ssl/truststore.p12"));
166166
if (trustStorePath != null) {
167167
LOG.debug("Using truststore: {}", trustStorePath);
168-
var trustStorePassword = defaultIfBlank(bootstrapProperties.get(SONAR_SCANNER_TRUSTSTORE_PASSWORD), CertificateStore.DEFAULT_PASSWORD);
169-
return new CertificateStore(trustStorePath, trustStorePassword);
168+
return new CertificateStore(trustStorePath, bootstrapProperties.get(SONAR_SCANNER_TRUSTSTORE_PASSWORD));
170169
}
171170
return null;
172171
}
@@ -176,8 +175,7 @@ private static CertificateStore loadKeyStoreConfig(Map<String, String> bootstrap
176175
var keyStorePath = parseFileProperty(bootstrapProperties, SONAR_SCANNER_KEYSTORE_PATH, "keystore", sonarUserHome.resolve("ssl/keystore.p12"));
177176
if (keyStorePath != null) {
178177
LOG.debug("Using keystore: {}", keyStorePath);
179-
var keyStorePassword = defaultIfBlank(bootstrapProperties.get(SONAR_SCANNER_KEYSTORE_PASSWORD), CertificateStore.DEFAULT_PASSWORD);
180-
return new CertificateStore(keyStorePath, keyStorePassword);
178+
return new CertificateStore(keyStorePath, bootstrapProperties.get(SONAR_SCANNER_KEYSTORE_PASSWORD));
181179
}
182180
return null;
183181
}

‎lib/src/main/java/org/sonarsource/scanner/lib/internal/http/OkHttpClientFactory.java‎

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
*/
2020
package org.sonarsource.scanner.lib.internal.http;
2121

22+
import java.io.IOException;
2223
import java.io.InputStream;
2324
import java.net.CookieManager;
2425
import java.net.CookiePolicy;
@@ -27,9 +28,14 @@
2728
import java.nio.file.Path;
2829
import java.nio.file.StandardOpenOption;
2930
import java.security.KeyStore;
31+
import java.security.KeyStoreException;
32+
import java.security.NoSuchAlgorithmException;
33+
import java.security.cert.CertificateException;
3034
import java.util.concurrent.TimeUnit;
35+
import javax.annotation.Nullable;
3136
import nl.altindag.ssl.SSLFactory;
3237
import nl.altindag.ssl.exception.GenericKeyStoreException;
38+
import nl.altindag.ssl.util.KeyStoreUtils;
3339
import okhttp3.ConnectionSpec;
3440
import okhttp3.Credentials;
3541
import okhttp3.JavaNetCookieJar;
@@ -38,6 +44,7 @@
3844
import org.bouncycastle.jce.provider.BouncyCastleProvider;
3945
import org.slf4j.Logger;
4046
import org.slf4j.LoggerFactory;
47+
import org.sonarsource.scanner.lib.internal.http.ssl.CertificateStore;
4148
import org.sonarsource.scanner.lib.internal.http.ssl.SslConfig;
4249

4350
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -113,27 +120,59 @@ private static SSLFactory configureSsl(SslConfig sslConfig) {
113120
sslFactoryBuilder.withSystemPropertyDerivedIdentityMaterial();
114121
}
115122
var keyStoreConfig = sslConfig.getKeyStore();
116-
if (keyStoreConfig != null) {
117-
sslFactoryBuilder.withIdentityMaterial(keyStoreConfig.getPath(), keyStoreConfig.getKeyStorePassword().toCharArray(), keyStoreConfig.getKeyStoreType());
123+
if (keyStoreConfig != null && Files.exists(keyStoreConfig.getPath())) {
124+
keyStoreConfig.getKeyStorePassword()
125+
.ifPresentOrElse(
126+
password -> sslFactoryBuilder.withIdentityMaterial(keyStoreConfig.getPath(), password.toCharArray(), keyStoreConfig.getKeyStoreType()),
127+
() -> loadIdentityMaterialWithDefaultPassword(sslFactoryBuilder, keyStoreConfig.getPath()));
118128
}
119129
var trustStoreConfig = sslConfig.getTrustStore();
120-
if (trustStoreConfig != null) {
121-
KeyStore trustStore = loadKeyStoreWithBouncyCastle(
122-
trustStoreConfig.getPath(),
123-
trustStoreConfig.getKeyStorePassword().toCharArray(),
124-
trustStoreConfig.getKeyStoreType());
130+
if (trustStoreConfig != null && Files.exists(trustStoreConfig.getPath())) {
131+
KeyStore trustStore;
132+
try {
133+
trustStore = loadTrustStoreWithBouncyCastle(
134+
trustStoreConfig.getPath(),
135+
trustStoreConfig.getKeyStorePassword().orElse(null),
136+
trustStoreConfig.getKeyStoreType());
137+
LOG.debug("Loaded truststore from '{}' containing {} certificates", trustStoreConfig.getPath(), trustStore.size());
138+
} catch (KeyStoreException | IOException | CertificateException | NoSuchAlgorithmException e) {
139+
throw new GenericKeyStoreException("Unable to read truststore from '" + trustStoreConfig.getPath() + "'", e);
140+
}
125141
sslFactoryBuilder.withTrustMaterial(trustStore);
126142
}
127143
return sslFactoryBuilder.build();
128144
}
129145

130-
public static KeyStore loadKeyStoreWithBouncyCastle(Path keystorePath, char[] keystorePassword, String keystoreType) {
146+
private static void loadIdentityMaterialWithDefaultPassword(SSLFactory.Builder sslFactoryBuilder, Path path) {
147+
try {
148+
var keystore = KeyStoreUtils.loadKeyStore(path, CertificateStore.DEFAULT_PASSWORD.toCharArray(), CertificateStore.DEFAULT_STORE_TYPE);
149+
sslFactoryBuilder.withIdentityMaterial(keystore, CertificateStore.DEFAULT_PASSWORD.toCharArray());
150+
} catch (GenericKeyStoreException e) {
151+
var keystore = KeyStoreUtils.loadKeyStore(path, CertificateStore.OLD_DEFAULT_PASSWORD.toCharArray(), CertificateStore.DEFAULT_STORE_TYPE);
152+
LOG.warn("Using deprecated default password for keystore '{}'.", path);
153+
sslFactoryBuilder.withIdentityMaterial(keystore, CertificateStore.OLD_DEFAULT_PASSWORD.toCharArray());
154+
}
155+
}
156+
157+
static KeyStore loadTrustStoreWithBouncyCastle(Path keystorePath, @Nullable String keystorePassword, String keystoreType) throws IOException,
158+
KeyStoreException, CertificateException, NoSuchAlgorithmException {
159+
KeyStore keystore = KeyStore.getInstance(keystoreType, new BouncyCastleProvider());
160+
if (keystorePassword != null) {
161+
loadKeyStoreWithPassword(keystorePath, keystore, keystorePassword);
162+
} else {
163+
try {
164+
loadKeyStoreWithPassword(keystorePath, keystore, CertificateStore.DEFAULT_PASSWORD);
165+
} catch (Exception e) {
166+
loadKeyStoreWithPassword(keystorePath, keystore, CertificateStore.OLD_DEFAULT_PASSWORD);
167+
LOG.warn("Using deprecated default password for truststore '{}'.", keystorePath);
168+
}
169+
}
170+
return keystore;
171+
}
172+
173+
private static void loadKeyStoreWithPassword(Path keystorePath, KeyStore keystore, String oldDefaultPassword) throws IOException, NoSuchAlgorithmException, CertificateException {
131174
try (InputStream keystoreInputStream = Files.newInputStream(keystorePath, StandardOpenOption.READ)) {
132-
KeyStore keystore = KeyStore.getInstance(keystoreType, new BouncyCastleProvider());
133-
keystore.load(keystoreInputStream, keystorePassword);
134-
return keystore;
135-
} catch (Exception e) {
136-
throw new GenericKeyStoreException(e);
175+
keystore.load(keystoreInputStream, oldDefaultPassword.toCharArray());
137176
}
138177
}
139178

‎lib/src/main/java/org/sonarsource/scanner/lib/internal/http/ssl/CertificateStore.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,22 @@
2020
package org.sonarsource.scanner.lib.internal.http.ssl;
2121

2222
import java.nio.file.Path;
23+
import java.util.Optional;
24+
import javax.annotation.Nullable;
2325

2426
public class CertificateStore {
25-
public static final String DEFAULT_PASSWORD = "sonar";
27+
public static final String DEFAULT_PASSWORD = "changeit";
28+
/**
29+
* @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
30+
*/
31+
@Deprecated(since = "11.4")
32+
public static final String OLD_DEFAULT_PASSWORD = "sonar";
2633
public static final String DEFAULT_STORE_TYPE = "PKCS12";
2734
private final Path path;
2835
private final String keyStorePassword;
2936
private final String keyStoreType;
3037

31-
public CertificateStore(Path path, String keyStorePassword) {
38+
public CertificateStore(Path path, @Nullable String keyStorePassword) {
3239
this.path = path;
3340
this.keyStorePassword = keyStorePassword;
3441
this.keyStoreType = DEFAULT_STORE_TYPE;
@@ -38,8 +45,8 @@ public Path getPath() {
3845
return path;
3946
}
4047

41-
public String getKeyStorePassword() {
42-
return keyStorePassword;
48+
public Optional<String> getKeyStorePassword() {
49+
return Optional.ofNullable(keyStorePassword);
4350
}
4451

4552
public String getKeyStoreType() {

‎lib/src/test/java/org/sonarsource/scanner/lib/internal/http/OkHttpClientFactoryTest.java‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
import java.util.Base64;
3030
import java.util.HashMap;
3131
import java.util.Map;
32+
import javax.annotation.Nullable;
3233
import javax.net.ssl.SSLHandshakeException;
34+
import nl.altindag.ssl.exception.GenericKeyStoreException;
3335
import okhttp3.Request;
3436
import okhttp3.Response;
3537
import org.junit.jupiter.api.BeforeEach;
@@ -40,6 +42,8 @@
4042
import org.junit.jupiter.api.TestInstance;
4143
import org.junit.jupiter.api.extension.RegisterExtension;
4244
import org.junit.jupiter.api.io.TempDir;
45+
import org.junit.jupiter.params.ParameterizedTest;
46+
import org.junit.jupiter.params.provider.CsvSource;
4347
import org.junitpioneer.jupiter.RestoreSystemProperties;
4448

4549
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
@@ -55,6 +59,7 @@
5559
import static com.github.tomakehurst.wiremock.stubbing.Scenario.STARTED;
5660
import static java.util.Objects.requireNonNull;
5761
import static org.assertj.core.api.Assertions.assertThat;
62+
import static org.assertj.core.api.Assertions.assertThatNoException;
5863
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5964

6065
class OkHttpClientFactoryTest {
@@ -73,6 +78,59 @@ void prepareMocks() {
7378
bootstrapProperties.clear();
7479
}
7580

81+
@ParameterizedTest
82+
@CsvSource({
83+
"keystore_changeit.p12, wrong, false",
84+
"keystore_changeit.p12, changeit, true",
85+
"keystore_changeit.p12,, true",
86+
"keystore_sonar.p12, wrong, false",
87+
"keystore_sonar.p12, sonar, true",
88+
"keystore_sonar.p12,, true",
89+
"keystore_anotherpwd.p12, wrong, false",
90+
"keystore_anotherpwd.p12, anotherpwd, true",
91+
"keystore_anotherpwd.p12,, false"})
92+
void it_should_fail_if_invalid_truststore_password(String keystore, @Nullable String password, boolean shouldSucceed) {
93+
bootstrapProperties.put("sonar.scanner.truststorePath", toPath(requireNonNull(OkHttpClientFactoryTest.class.getResource("/ssl/" + keystore))).toString());
94+
if (password != null) {
95+
bootstrapProperties.put("sonar.scanner.truststorePassword", password);
96+
}
97+
98+
if (shouldSucceed) {
99+
assertThatNoException().isThrownBy(() -> OkHttpClientFactory.create(new HttpConfig(bootstrapProperties, sonarUserHome)));
100+
} else {
101+
assertThatThrownBy(() -> OkHttpClientFactory.create(new HttpConfig(bootstrapProperties, sonarUserHome)))
102+
.isInstanceOf(GenericKeyStoreException.class)
103+
.hasMessageContaining("Unable to read truststore from")
104+
.hasStackTraceContaining("wrong password or corrupted file");
105+
}
106+
}
107+
108+
@ParameterizedTest
109+
@CsvSource({
110+
"keystore_changeit.p12, wrong, false",
111+
"keystore_changeit.p12, changeit, true",
112+
"keystore_changeit.p12,, true",
113+
"keystore_sonar.p12, wrong, false",
114+
"keystore_sonar.p12, sonar, true",
115+
"keystore_sonar.p12,, true",
116+
"keystore_anotherpwd.p12, wrong, false",
117+
"keystore_anotherpwd.p12, anotherpwd, true",
118+
"keystore_anotherpwd.p12,, false"})
119+
void it_should_fail_if_invalid_keystore_password(String keystore, @Nullable String password, boolean shouldSucceed) {
120+
bootstrapProperties.put("sonar.scanner.keystorePath", toPath(requireNonNull(OkHttpClientFactoryTest.class.getResource("/ssl/" + keystore))).toString());
121+
if (password != null) {
122+
bootstrapProperties.put("sonar.scanner.keystorePassword", password);
123+
}
124+
125+
if (shouldSucceed) {
126+
assertThatNoException().isThrownBy(() -> OkHttpClientFactory.create(new HttpConfig(bootstrapProperties, sonarUserHome)));
127+
} else {
128+
assertThatThrownBy(() -> OkHttpClientFactory.create(new HttpConfig(bootstrapProperties, sonarUserHome)))
129+
.isInstanceOf(GenericKeyStoreException.class)
130+
.hasMessageContaining("keystore password was incorrect");
131+
}
132+
}
133+
76134
@Nested
77135
// Workaround until we move to Java 17+ and can make Wiremock extension static
78136
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@@ -366,10 +424,10 @@ void it_should_support_jvm_system_properties() throws IOException {
366424

367425
private Response call(String url) throws IOException {
368426
return OkHttpClientFactory.create(new HttpConfig(bootstrapProperties, sonarUserHome)).newCall(
369-
new Request.Builder()
370-
.url(url)
371-
.get()
372-
.build())
427+
new Request.Builder()
428+
.url(url)
429+
.get()
430+
.build())
373431
.execute();
374432
}
375433

‎lib/src/test/resources/ssl/keystore_anotherpwd.p12‎

3.49 KB
Binary file not shown.

‎lib/src/test/resources/ssl/keystore_changeit.p12‎

3.49 KB
Binary file not shown.

‎lib/src/test/resources/ssl/keystore_sonar.p12‎

4.24 KB
Binary file not shown.

0 commit comments

Comments
 (0)