Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SCANJLIB-241 Add property to skip loading of OS-level SSL certificates (+ hardening) #211

Merged
merged 3 commits into from
Dec 2, 2024
+48 −34
Merged

SCANJLIB-241 Add property to skip loading of OS-level SSL certificates (+ hardening) #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf74f30
SCANJLIB-238 Update parent pom
henryju Nov 29, 2024
3859eaf
NO-JIRA Update test dependencies
henryju Nov 29, 2024
ab1aa2d
SCANJLIB-241 Add property to skip loading of OS-level SSL certificates
henryju Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,19 @@
<dependency>
<groupId>org.wiremock</groupId>
<artifactId>wiremock-standalone</artifactId>
<version>3.9.1</version>
<version>3.9.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit-pioneer</groupId>
<artifactId>junit-pioneer</artifactId>
<version>2.2.0</version>
<version>2.3.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.5.8</version>
<version>1.5.12</version>
<scope>test</scope>
</dependency>
</dependencies>
Expand Down
2 changes: 1 addition & 1 deletion lib/src/main/java/org/sonarsource/scanner/lib/ScannerProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ private ScannerProperties() {
public static final String SONAR_SCANNER_KEYSTORE_PASSWORD = "sonar.scanner.keystorePassword";
public static final String SONAR_SCANNER_TRUSTSTORE_PATH = "sonar.scanner.truststorePath";
public static final String SONAR_SCANNER_TRUSTSTORE_PASSWORD = "sonar.scanner.truststorePassword";

public static final String SONAR_SCANNER_SKIP_SYSTEM_TRUSTSTORE = "sonar.scanner.skipSystemTruststore";
/**
* Skip analysis.
*/
Expand Down
7 changes: 7 additions & 0 deletions lib/src/main/java/org/sonarsource/scanner/lib/internal/http/HttpConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import static java.lang.Integer.parseInt;
import static java.lang.String.format;
import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
import static org.sonarsource.scanner.lib.ScannerProperties.SONAR_SCANNER_SKIP_SYSTEM_TRUSTSTORE;
import static org.sonarsource.scanner.lib.ScannerProperties.SONAR_SCANNER_CONNECT_TIMEOUT;
import static org.sonarsource.scanner.lib.ScannerProperties.SONAR_SCANNER_KEYSTORE_PASSWORD;
import static org.sonarsource.scanner.lib.ScannerProperties.SONAR_SCANNER_KEYSTORE_PATH;
Expand Down Expand Up @@ -79,6 +80,7 @@ public class HttpConfig {
private final String proxyUser;
private final String proxyPassword;
private final String userAgent;
private final boolean skipSystemTrustMaterial;

public HttpConfig(Map<String, String> bootstrapProperties, Path sonarUserHome) {
this.webApiBaseUrl = StringUtils.removeEnd(bootstrapProperties.get(ScannerProperties.HOST_URL), "/");
Expand All @@ -94,6 +96,7 @@ public HttpConfig(Map<String, String> bootstrapProperties, Path sonarUserHome) {
this.proxy = loadProxy(bootstrapProperties);
this.proxyUser = loadProxyUser(bootstrapProperties);
this.proxyPassword = loadProxyPassword(bootstrapProperties);
this.skipSystemTrustMaterial = Boolean.parseBoolean(defaultIfBlank(bootstrapProperties.get(SONAR_SCANNER_SKIP_SYSTEM_TRUSTSTORE), "false"));
}

private static String loadProxyPassword(Map<String, String> bootstrapProperties) {
Expand Down Expand Up @@ -249,4 +252,8 @@ public String getProxyUser() {
public String getProxyPassword() {
return proxyPassword;
}

public boolean skipSystemTruststore() {
return skipSystemTrustMaterial;
}
}
13 changes: 9 additions & 4 deletions lib/src/main/java/org/sonarsource/scanner/lib/internal/http/OkHttpClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Arrays.asList;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.sonarsource.scanner.lib.ScannerProperties.SONAR_SCANNER_SKIP_SYSTEM_TRUSTSTORE;

public class OkHttpClientFactory {

Expand All @@ -72,7 +73,7 @@ private OkHttpClientFactory() {

static OkHttpClient create(HttpConfig httpConfig) {

var sslContext = configureSsl(httpConfig.getSslConfig());
var sslContext = configureSsl(httpConfig.getSslConfig(), httpConfig.skipSystemTruststore());

OkHttpClient.Builder okHttpClientBuilder = new OkHttpClient.Builder()
.connectTimeout(httpConfig.getConnectTimeout().toMillis(), TimeUnit.MILLISECONDS)
Expand Down Expand Up @@ -112,10 +113,14 @@ static OkHttpClient create(HttpConfig httpConfig) {
return okHttpClientBuilder.build();
}

private static SSLFactory configureSsl(SslConfig sslConfig) {
private static SSLFactory configureSsl(SslConfig sslConfig, boolean skipSystemTrustMaterial) {
var sslFactoryBuilder = SSLFactory.builder()
.withDefaultTrustMaterial()
.withSystemTrustMaterial();
.withDefaultTrustMaterial();
if (!skipSystemTrustMaterial) {
LOG.debug("Loading OS trusted SSL certificates...");
LOG.debug("This operation might be slow or even get stuck. You can skip it by passing the scanner property '{}=true'", SONAR_SCANNER_SKIP_SYSTEM_TRUSTSTORE);
sslFactoryBuilder.withSystemTrustMaterial();
}
if (System.getProperties().containsKey("javax.net.ssl.keyStore")) {
sslFactoryBuilder.withSystemPropertyDerivedIdentityMaterial();
}
Expand Down
24 changes: 24 additions & 0 deletions lib/src/test/java/org/sonarsource/scanner/lib/internal/http/OkHttpClientFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junitpioneer.jupiter.RestoreSystemProperties;
import org.slf4j.event.Level;
import testutils.LogTester;

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
Expand All @@ -68,6 +70,9 @@ class OkHttpClientFactoryTest {

private final Map<String, String> bootstrapProperties = new HashMap<>();

@RegisterExtension
private LogTester logTester = new LogTester();

@TempDir
private Path sonarUserHomeDir;
private Path sonarUserHome;
Expand Down Expand Up @@ -131,6 +136,25 @@ void it_should_fail_if_invalid_keystore_password(String keystore, @Nullable Stri
}
}

@Test
void should_load_os_certificates_by_default() {
logTester.setLevel(Level.DEBUG);

OkHttpClientFactory.create(new HttpConfig(bootstrapProperties, sonarUserHome));

assertThat(logTester.logs(Level.DEBUG)).contains("Loading OS trusted SSL certificates...");
}

@Test
void should_skip_load_of_os_certificates_if_props_set() {
logTester.setLevel(Level.DEBUG);
bootstrapProperties.put("sonar.scanner.skipSystemTruststore", "true");

OkHttpClientFactory.create(new HttpConfig(bootstrapProperties, sonarUserHome));

assertThat(logTester.logs(Level.DEBUG)).doesNotContain("Loading OS trusted SSL certificates...");
}

@Nested
// Workaround until we move to Java 17+ and can make Wiremock extension static
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
Expand Down
30 changes: 4 additions & 26 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>org.sonarsource.parent</groupId>
<artifactId>parent</artifactId>
<version>77.0.0.2082</version>
<version>82.0.0.2314</version>
</parent>

<groupId>org.sonarsource.scanner.lib</groupId>
Expand Down Expand Up @@ -49,11 +49,12 @@

<properties>
<maven.compiler.release>11</maven.compiler.release>
<license.name>GNU LGPL v3</license.name>

<!-- used for deployment to SonarSource Artifactory -->
<gitRepositoryName>sonar-scanner-java-library</gitRepositoryName>
<okhttp.version>4.12.0</okhttp.version>
<mockito.version>5.13.0</mockito.version>
<mockito.version>5.14.2</mockito.version>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -101,7 +102,7 @@
<dependency>
<groupId>org.junit</groupId>
<artifactId>junit-bom</artifactId>
<version>5.11.0</version>
<version>5.11.3</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down Expand Up @@ -129,29 +130,6 @@
</dependencyManagement>

<profiles>
<profile>
<id>release</id>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>3.1.0</version>
<configuration>
<source>8</source>
</configuration>
<executions>
<execution>
<id>attach-javadocs</id>
<goals>
<goal>jar</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>its</id>
<modules>
Expand Down