Skip to content

Commit fdafbb8

Browse files
runningcodeclaude
andcommitted
ref(transport): Pass DistrustedCertificates to HttpConnection
Inject the check through HttpConnection's constructor like AuthenticatorWrapper, so its tests can stub it instead of loading the real eMudhra certificate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 494d0d9 commit fdafbb8

6 files changed

Lines changed: 62 additions & 29 deletions

File tree

‎sentry-android-distribution/src/main/java/io/sentry/android/distribution/DistributionHttpClient.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ internal class DistributionHttpClient(private val options: SentryOptions) {
9595
}
9696

9797
connection.connect()
98-
DistrustedCertificates.verify(connection)
98+
DistrustedCertificates.getInstance().verify(connection)
9999

100100
val responseCode = connection.responseCode
101101
val responseBody = readResponse(connection)

‎sentry/api/sentry.api‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7765,7 +7765,8 @@ public final class io/sentry/transport/CurrentDateProvider : io/sentry/transport
77657765
}
77667766

77677767
public final class io/sentry/transport/DistrustedCertificates {
7768-
public static fun verify (Ljava/net/HttpURLConnection;)V
7768+
public static fun getInstance ()Lio/sentry/transport/DistrustedCertificates;
7769+
public fun verify (Ljava/net/HttpURLConnection;)V
77697770
}
77707771

77717772
public abstract interface class io/sentry/transport/ICurrentDateProvider {

‎sentry/src/main/java/io/sentry/transport/DistrustedCertificates.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ public final class DistrustedCertificates {
4747
"ebed6475c2c2d80fcb199f3fe8dd6e6b3ce60b77eb15d4e42c3dec0c6f5b5105",
4848
"ec4ffa1ad0ac88081314b7aaffef6a91f97cd57291bb5ba686c5415f5eb4f6b6"));
4949

50+
private static final DistrustedCertificates instance = new DistrustedCertificates();
51+
52+
public static @NotNull DistrustedCertificates getInstance() {
53+
return instance;
54+
}
55+
5056
private DistrustedCertificates() {}
5157

5258
/**
@@ -57,7 +63,7 @@ private DistrustedCertificates() {}
5763
* do this check, because the JVM's {@link HttpsURLConnection} only calls it when its own hostname
5864
* check fails.
5965
*/
60-
public static void verify(final @NotNull HttpURLConnection connection)
66+
public void verify(final @NotNull HttpURLConnection connection)
6167
throws SSLPeerUnverifiedException {
6268
if (!(connection instanceof HttpsURLConnection)) {
6369
return;

‎sentry/src/main/java/io/sentry/transport/HttpConnection.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ final class HttpConnection {
3333
private final @NotNull RequestDetails requestDetails;
3434
private final @NotNull SentryOptions options;
3535
private final @NotNull RateLimiter rateLimiter;
36+
private final @NotNull DistrustedCertificates distrustedCertificates;
3637

3738
/**
3839
* Constructs a new HTTP transport instance. Notably, the provided {@code requestUpdater} must set
@@ -47,17 +48,24 @@ public HttpConnection(
4748
final @NotNull SentryOptions options,
4849
final @NotNull RequestDetails requestDetails,
4950
final @NotNull RateLimiter rateLimiter) {
50-
this(options, requestDetails, AuthenticatorWrapper.getInstance(), rateLimiter);
51+
this(
52+
options,
53+
requestDetails,
54+
AuthenticatorWrapper.getInstance(),
55+
rateLimiter,
56+
DistrustedCertificates.getInstance());
5157
}
5258

5359
HttpConnection(
5460
final @NotNull SentryOptions options,
5561
final @NotNull RequestDetails requestDetails,
5662
final @NotNull AuthenticatorWrapper authenticatorWrapper,
57-
final @NotNull RateLimiter rateLimiter) {
63+
final @NotNull RateLimiter rateLimiter,
64+
final @NotNull DistrustedCertificates distrustedCertificates) {
5865
this.requestDetails = requestDetails;
5966
this.options = options;
6067
this.rateLimiter = rateLimiter;
68+
this.distrustedCertificates = distrustedCertificates;
6169

6270
this.proxy = resolveProxy(options.getProxy());
6371

@@ -143,7 +151,7 @@ HttpURLConnection open() throws IOException {
143151
}
144152

145153
connection.connect();
146-
DistrustedCertificates.verify(connection);
154+
distrustedCertificates.verify(connection);
147155
return connection;
148156
}
149157

‎sentry/src/test/java/io/sentry/transport/DistrustedCertificatesTest.kt‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@ import org.mockito.kotlin.whenever
1616
class DistrustedCertificatesTest {
1717

1818
private val connection = mock<HttpsURLConnection>()
19+
private val sut = DistrustedCertificates.getInstance()
1920

2021
@Test
2122
fun `throws and disconnects when the chain contains a distrusted certificate`() {
2223
val certificates = arrayOf(trustedCertificate(), distrustedCertificate())
2324
whenever(connection.serverCertificates).thenReturn(certificates)
2425

25-
assertFailsWith<SSLPeerUnverifiedException> { DistrustedCertificates.verify(connection) }
26+
assertFailsWith<SSLPeerUnverifiedException> { sut.verify(connection) }
2627

2728
verify(connection).disconnect()
2829
}
@@ -32,7 +33,7 @@ class DistrustedCertificatesTest {
3233
val certificates = arrayOf(trustedCertificate())
3334
whenever(connection.serverCertificates).thenReturn(certificates)
3435

35-
DistrustedCertificates.verify(connection)
36+
sut.verify(connection)
3637

3738
verify(connection, never()).disconnect()
3839
}
@@ -41,7 +42,7 @@ class DistrustedCertificatesTest {
4142
fun `accepts a connection without server certificates`() {
4243
whenever(connection.serverCertificates).thenReturn(null)
4344

44-
DistrustedCertificates.verify(connection)
45+
sut.verify(connection)
4546

4647
verify(connection, never()).disconnect()
4748
}
@@ -50,26 +51,22 @@ class DistrustedCertificatesTest {
5051
fun `ignores plain HTTP connections`() {
5152
val connection = mock<HttpURLConnection>()
5253

53-
DistrustedCertificates.verify(connection)
54+
sut.verify(connection)
5455

5556
verify(connection, never()).disconnect()
5657
}
5758

58-
companion object {
59-
/** EM DV TLS CA - G3B as cross-signed by Sectigo's AAA Certificate Services root. */
60-
fun distrustedCertificate(): Certificate =
61-
DistrustedCertificatesTest::class
62-
.java
63-
.classLoader!!
64-
.getResourceAsStream("emudhra-em-dv-tls-ca-g3b.pem")!!
65-
.use { CertificateFactory.getInstance("X.509").generateCertificate(it) }
66-
67-
private fun trustedCertificate(): Certificate {
68-
val publicKey = mock<PublicKey>()
69-
whenever(publicKey.encoded).thenReturn(byteArrayOf(1, 2, 3))
70-
val certificate = mock<Certificate>()
71-
whenever(certificate.publicKey).thenReturn(publicKey)
72-
return certificate
59+
/** EM DV TLS CA - G3B as cross-signed by Sectigo's AAA Certificate Services root. */
60+
private fun distrustedCertificate(): Certificate =
61+
javaClass.classLoader!!.getResourceAsStream("emudhra-em-dv-tls-ca-g3b.pem")!!.use {
62+
CertificateFactory.getInstance("X.509").generateCertificate(it)
7363
}
64+
65+
private fun trustedCertificate(): Certificate {
66+
val publicKey = mock<PublicKey>()
67+
whenever(publicKey.encoded).thenReturn(byteArrayOf(1, 2, 3))
68+
val certificate = mock<Certificate>()
69+
whenever(certificate.publicKey).thenReturn(publicKey)
70+
return certificate
7471
}
7572
}

‎sentry/src/test/java/io/sentry/transport/HttpConnectionTest.kt‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ import kotlin.test.assertNull
2727
import kotlin.test.assertTrue
2828
import org.mockito.kotlin.any
2929
import org.mockito.kotlin.check
30+
import org.mockito.kotlin.doThrow
3031
import org.mockito.kotlin.eq
32+
import org.mockito.kotlin.inOrder
3133
import org.mockito.kotlin.mock
3234
import org.mockito.kotlin.never
3335
import org.mockito.kotlin.verify
@@ -41,6 +43,7 @@ class HttpConnectionTest {
4143
val currentDateProvider = mock<ICurrentDateProvider>()
4244
val authenticatorWrapper = mock<AuthenticatorWrapper>()
4345
val rateLimiter = mock<RateLimiter>()
46+
val distrustedCertificates = mock<DistrustedCertificates>()
4447
var sslSocketFactory: SSLSocketFactory? = null
4548
val requestDetails = mock<RequestDetails>()
4649
val socketTagger = mock<ISocketTagger>()
@@ -63,7 +66,13 @@ class HttpConnectionTest {
6366
options.sslSocketFactory = sslSocketFactory
6467
options.setSocketTagger(socketTagger)
6568

66-
return HttpConnection(options, requestDetails, authenticatorWrapper, rateLimiter)
69+
return HttpConnection(
70+
options,
71+
requestDetails,
72+
authenticatorWrapper,
73+
rateLimiter,
74+
distrustedCertificates,
75+
)
6776
}
6877
}
6978

@@ -170,16 +179,28 @@ class HttpConnectionTest {
170179
verify(fixture.connection, never()).sslSocketFactory = any()
171180
}
172181

182+
@Test
183+
fun `verifies the server certificates after connecting and before writing the envelope`() {
184+
val transport = fixture.getSUT()
185+
186+
transport.send(createEnvelope())
187+
188+
val order = inOrder(fixture.connection, fixture.distrustedCertificates)
189+
order.verify(fixture.connection).connect()
190+
order.verify(fixture.distrustedCertificates).verify(fixture.connection)
191+
order.verify(fixture.connection).outputStream
192+
}
193+
173194
@Test
174195
fun `does not send the envelope when the server presents a distrusted certificate`() {
175-
whenever(fixture.connection.serverCertificates)
176-
.thenReturn(arrayOf(DistrustedCertificatesTest.distrustedCertificate()))
196+
doThrow(SSLPeerUnverifiedException("distrusted"))
197+
.whenever(fixture.distrustedCertificates)
198+
.verify(fixture.connection)
177199
val transport = fixture.getSUT()
178200

179201
assertFailsWith<SSLPeerUnverifiedException> { transport.send(createEnvelope()) }
180202

181203
verify(fixture.connection, never()).outputStream
182-
verify(fixture.connection).disconnect()
183204
}
184205

185206
@Test

0 commit comments

Comments
 (0)