Skip to content

Commit e99a75d

Browse files
Address review feedback for mTLS default trust handling
1 parent b6f5f6b commit e99a75d

6 files changed

Lines changed: 65 additions & 53 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
#### Exporters
88

9-
* OTLP: Fix OkHttp HTTP sender dropping client certificates when client TLS is configured without custom trusted certificates.
9+
* Fix OkHttp client mTLS when using the platform default trust store ([#8565](https://github.com/open-telemetry/opentelemetry-java/pull/8565))
1010

1111
## Version 1.63.0 (2026-06-05)
1212

exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ public X509TrustManager getTrustManager() {
111111
return trustManager;
112112
}
113113

114-
@Nullable
115-
public X509TrustManager getEffectiveTrustManager() throws SSLException {
114+
private X509TrustManager getEffectiveTrustManager() throws SSLException {
116115
if (trustManager != null) {
117116
return trustManager;
118117
}

exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsUtil.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,18 +105,23 @@ public static X509TrustManager defaultTrustManager() throws SSLException {
105105
// Initialize with the platform default trust store.
106106
tmf.init((KeyStore) null);
107107

108-
for (TrustManager trustManager : tmf.getTrustManagers()) {
109-
if (trustManager instanceof X509TrustManager) {
110-
return (X509TrustManager) trustManager;
111-
}
112-
}
113-
114-
throw new SSLException("No X509TrustManager found");
108+
return defaultTrustManager(tmf);
115109
} catch (KeyStoreException | NoSuchAlgorithmException e) {
116110
throw new SSLException("Could not build default TrustManager.", e);
117111
}
118112
}
119113

114+
// Visible for testing
115+
static X509TrustManager defaultTrustManager(TrustManagerFactory tmf) throws SSLException {
116+
for (TrustManager trustManager : tmf.getTrustManagers()) {
117+
if (trustManager instanceof X509TrustManager) {
118+
return (X509TrustManager) trustManager;
119+
}
120+
}
121+
122+
throw new SSLException("No X509TrustManager found");
123+
}
124+
120125
// Visible for testing
121126
static PrivateKey generatePrivateKey(PKCS8EncodedKeySpec keySpec, List<KeyFactory> keyFactories)
122127
throws SSLException {

exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -97,19 +97,6 @@ void setSslContext_AlreadyExists_Throws() throws Exception {
9797
.hasMessageContaining("sslContext or trustManager has been previously configured");
9898
}
9999

100-
@Test
101-
void getEffectiveTrustManager_returnsConfiguredTrustManager()
102-
throws CertificateEncodingException, SSLException {
103-
helper.setTrustManagerFromCerts(serverTls.certificate().getEncoded());
104-
105-
assertThat(helper.getEffectiveTrustManager()).isSameAs(helper.getTrustManager());
106-
}
107-
108-
@Test
109-
void getEffectiveTrustManager_returnsDefaultTrustManager() throws SSLException {
110-
assertThat(helper.getEffectiveTrustManager()).isNotNull();
111-
}
112-
113100
@Test
114101
void getSslContext_wrapsDefaultTrustManagerFailure() {
115102
String originalAlgorithm = Security.getProperty("ssl.TrustManagerFactory.algorithm");

exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsUtilTest.java

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55

66
package io.opentelemetry.exporter.internal;
77

8+
import static org.assertj.core.api.Assertions.assertThat;
89
import static org.assertj.core.api.Assertions.assertThatCode;
9-
import static org.assertj.core.api.Assertions.assertThatThrownBy;
10+
import static org.mockito.Mockito.mock;
1011

1112
import com.linecorp.armeria.internal.common.util.SelfSignedCertificate;
1213
import java.io.File;
@@ -16,15 +17,19 @@
1617
import java.nio.file.Path;
1718
import java.nio.file.StandardOpenOption;
1819
import java.security.KeyFactory;
19-
import java.security.NoSuchAlgorithmException;
20-
import java.security.Security;
20+
import java.security.KeyStore;
2121
import java.security.cert.CertificateException;
2222
import java.security.spec.PKCS8EncodedKeySpec;
2323
import java.time.Instant;
2424
import java.util.Collections;
2525
import java.util.Date;
2626
import java.util.stream.Stream;
27+
import javax.net.ssl.ManagerFactoryParameters;
2728
import javax.net.ssl.SSLException;
29+
import javax.net.ssl.TrustManager;
30+
import javax.net.ssl.TrustManagerFactory;
31+
import javax.net.ssl.TrustManagerFactorySpi;
32+
import javax.net.ssl.X509TrustManager;
2833
import org.junit.jupiter.api.BeforeEach;
2934
import org.junit.jupiter.api.Test;
3035
import org.junit.jupiter.api.io.TempDir;
@@ -88,20 +93,36 @@ void defaultTrustManager() {
8893
}
8994

9095
@Test
91-
void defaultTrustManager_NoSuchAlgorithmException() {
92-
String originalAlgorithm = Security.getProperty("ssl.TrustManagerFactory.algorithm");
96+
void defaultTrustManager_returnsX509TrustManager() throws Exception {
97+
X509TrustManager trustManager = mock(X509TrustManager.class);
9398

94-
try {
95-
Security.setProperty("ssl.TrustManagerFactory.algorithm", "invalid-algorithm");
99+
assertThat(TlsUtil.defaultTrustManager(trustManagerFactory(new TrustManager[] {trustManager})))
100+
.isSameAs(trustManager);
101+
}
96102

97-
assertThatThrownBy(TlsUtil::defaultTrustManager)
98-
.isInstanceOf(SSLException.class)
99-
.hasMessage("Could not build default TrustManager.")
100-
.hasCauseInstanceOf(NoSuchAlgorithmException.class);
103+
private static TrustManagerFactory trustManagerFactory(TrustManager[] trustManagers) {
104+
return new TrustManagerFactory(
105+
new TrustManagerFactorySpi() {
106+
@Override
107+
protected void engineInit(KeyStore keyStore) {}
108+
109+
@Override
110+
protected void engineInit(ManagerFactoryParameters spec) {}
111+
112+
@Override
113+
protected TrustManager[] engineGetTrustManagers() {
114+
return trustManagers;
115+
}
116+
},
117+
null,
118+
"test") {};
119+
}
101120

102-
} finally {
103-
Security.setProperty("ssl.TrustManagerFactory.algorithm", originalAlgorithm);
104-
}
121+
@Test
122+
void defaultTrustManager_NoX509TrustManagerFound() {
123+
assertThatCode(() -> TlsUtil.defaultTrustManager(trustManagerFactory(new TrustManager[0])))
124+
.isInstanceOf(SSLException.class)
125+
.hasMessage("No X509TrustManager found");
105126
}
106127

107128
/**

exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import static org.assertj.core.api.Assertions.assertThat;
99
import static org.assertj.core.api.Assertions.assertThatThrownBy;
10+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
1011

1112
import io.opentelemetry.sdk.common.export.HttpResponse;
1213
import io.opentelemetry.sdk.common.export.MessageWriter;
@@ -70,22 +71,21 @@ void constructor_usesDefaultTrustManagerWhenTrustManagerIsNull() throws Exceptio
7071
SSLContext sslContext = SSLContext.getInstance("TLS");
7172
sslContext.init(null, null, null);
7273

73-
OkHttpHttpSender sender =
74-
new OkHttpHttpSender(
75-
URI.create("https://localhost"),
76-
"text/plain",
77-
null,
78-
Duration.ofSeconds(10),
79-
Duration.ofSeconds(10),
80-
Collections::emptyMap,
81-
null,
82-
null,
83-
sslContext,
84-
null,
85-
null,
86-
Long.MAX_VALUE);
87-
88-
assertThat(sender).isNotNull();
74+
assertDoesNotThrow(
75+
() ->
76+
new OkHttpHttpSender(
77+
URI.create("https://localhost"),
78+
"text/plain",
79+
null,
80+
Duration.ofSeconds(10),
81+
Duration.ofSeconds(10),
82+
Collections::emptyMap,
83+
null,
84+
null,
85+
sslContext,
86+
null,
87+
null,
88+
Long.MAX_VALUE));
8989
}
9090

9191
@Test

0 commit comments

Comments
 (0)