Skip to content

Commit 58998ba

Browse files
authored
Add timeout for retrieving certificates in test (#84)
* Move TestCertStore to separate file * Add connect and read timeouts * Use HttpsURLConnection again * Use SSLContext instead of SSLCertificateSocketFactory * Replace AllowAllHostnameVerifier with HostnameVerifier * Add timeouts * Remove obsolete parenthesis * Use use to close stream automatically * Move certificate retrieval to @BeforeClass * Accept only valid hostnames and certificates * Replace @BeforeClass with by-lazy * Simplify certificate retrieval
1 parent 42d883e commit 58998ba

File tree

2 files changed

+114
-136
lines changed

2 files changed

+114
-136
lines changed

lib/src/test/java/at/bitfire/cert4android/CustomCertManagerTest.kt

Lines changed: 18 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -10,39 +10,22 @@
1010

1111
package at.bitfire.cert4android
1212

13-
import androidx.annotation.VisibleForTesting
14-
import kotlinx.coroutines.flow.StateFlow
15-
import org.junit.Assume.assumeNotNull
1613
import org.junit.Before
1714
import org.junit.Test
18-
import java.io.IOException
1915
import java.net.URL
20-
import java.security.SecureRandom
2116
import java.security.cert.CertificateException
2217
import java.security.cert.X509Certificate
23-
import java.util.logging.Level
24-
import java.util.logging.Logger
25-
import javax.net.ssl.SSLContext
26-
import javax.net.ssl.SSLSocket
27-
import javax.net.ssl.TrustManager
28-
import javax.net.ssl.X509TrustManager
18+
import javax.net.ssl.HttpsURLConnection
2919

3020
class CustomCertManagerTest {
3121

22+
private val siteCerts: List<X509Certificate> by lazy {
23+
getSiteCertificates(URL("https://www.davx5.com"))
24+
}
3225
private lateinit var certStore: CertStore
3326
private lateinit var certManager: CustomCertManager
3427
private lateinit var paranoidCertManager: CustomCertManager
3528

36-
private var siteCerts: List<X509Certificate>? =
37-
try {
38-
getSiteCertificates(URL("https://www.davx5.com"))
39-
} catch(_: IOException) {
40-
null
41-
}
42-
init {
43-
assumeNotNull("Couldn't load certificate from Web", siteCerts)
44-
}
45-
4629
@Before
4730
fun createCertManager() {
4831
certStore = TestCertStore()
@@ -58,18 +41,18 @@ class CustomCertManagerTest {
5841

5942
@Test
6043
fun testTrustedCertificate() {
61-
certManager.checkServerTrusted(siteCerts!!.toTypedArray(), "RSA")
44+
certManager.checkServerTrusted(siteCerts.toTypedArray(), "RSA")
6245
}
6346

6447
@Test(expected = CertificateException::class)
6548
fun testParanoidCertificate() {
66-
paranoidCertManager.checkServerTrusted(siteCerts!!.toTypedArray(), "RSA")
49+
paranoidCertManager.checkServerTrusted(siteCerts.toTypedArray(), "RSA")
6750
}
6851

6952
@Test
7053
fun testAddCustomCertificate() {
7154
addTrustedCertificate()
72-
paranoidCertManager.checkServerTrusted(siteCerts!!.toTypedArray(), "RSA")
55+
paranoidCertManager.checkServerTrusted(siteCerts.toTypedArray(), "RSA")
7356
}
7457

7558
@Test(expected = CertificateException::class)
@@ -80,132 +63,31 @@ class CustomCertManagerTest {
8063
// should now be rejected for the whole session
8164
addUntrustedCertificate()
8265

83-
paranoidCertManager.checkServerTrusted(siteCerts!!.toTypedArray(), "RSA")
66+
paranoidCertManager.checkServerTrusted(siteCerts.toTypedArray(), "RSA")
8467
}
8568

8669

8770
// helpers
8871

8972
private fun addTrustedCertificate() {
90-
certStore.setTrustedByUser(siteCerts!!.first())
73+
certStore.setTrustedByUser(siteCerts.first())
9174
}
9275

9376
private fun addUntrustedCertificate() {
94-
certStore.setUntrustedByUser(siteCerts!!.first())
77+
certStore.setUntrustedByUser(siteCerts.first())
9578
}
9679

97-
/**
98-
* Get the certificates of a site (bypassing all trusted checks).
99-
*
100-
* @param url the URL to get the certificates from
101-
* @return the certificates of the site
102-
*/
10380
fun getSiteCertificates(url: URL): List<X509Certificate> {
104-
val port = if (url.port != -1) url.port else 443
105-
val host = url.host
106-
107-
// Create a TrustManager which accepts all certificates
108-
val trustAll = object : X509TrustManager {
109-
override fun checkClientTrusted(chain: Array<out X509Certificate>?, authType: String?) {}
110-
override fun checkServerTrusted(chain: Array<out X509Certificate>?, authType: String?) {}
111-
override fun getAcceptedIssuers(): Array<X509Certificate> = emptyArray()
112-
}
113-
114-
// Create an SSLContext using the trust-all manager
115-
val sslContext = SSLContext.getInstance("TLS").apply {
116-
init(null, arrayOf<TrustManager>(trustAll), SecureRandom())
117-
}
118-
119-
// Create an SSL socket and force a TLS handshake
120-
// (HttpsURLConnection performs the handshake lazily and sometimes the handshake is not
121-
// executed before this method gets called)
122-
sslContext.socketFactory.createSocket(host, port).use { socket ->
123-
val sslSocket = socket as SSLSocket
124-
// Explicitly start the handshake (gets certificate)
125-
sslSocket.startHandshake()
126-
// server certificates now available in SSLSession
127-
return sslSocket.session.peerCertificates.map { it as X509Certificate }
128-
}
129-
}
130-
131-
132-
class TestCertStore(): CertStore {
133-
134-
private val logger
135-
get() = Logger.getLogger(javaClass.name)
136-
137-
/** custom TrustStore (simple map) */
138-
@VisibleForTesting
139-
internal val userKeyStore = mutableMapOf<String, X509Certificate>()
140-
141-
/** in-memory store for untrusted certs */
142-
@VisibleForTesting
143-
internal var untrustedCerts = HashSet<X509Certificate>()
144-
145-
@Synchronized
146-
override fun clearUserDecisions() {
147-
logger.info("Clearing user-(dis)trusted certificates")
148-
149-
for (alias in userKeyStore.keys)
150-
userKeyStore.remove(alias)
151-
152-
// clear untrusted certs
153-
untrustedCerts.clear()
154-
}
155-
156-
/**
157-
* Determines whether a certificate chain is trusted.
158-
*/
159-
override fun isTrusted(chain: Array<X509Certificate>, authType: String, trustSystemCerts: Boolean, appInForeground: StateFlow<Boolean>?): Boolean {
160-
if (chain.isEmpty())
161-
throw IllegalArgumentException("Certificate chain must not be empty")
162-
val cert = chain[0]
163-
164-
synchronized(this) {
165-
// explicitly accepted by user?
166-
if (isTrustedByUser(cert))
167-
return true
168-
169-
// explicitly rejected by user?
170-
if (untrustedCerts.contains(cert))
171-
return false
172-
173-
// trusted by system? (if applicable)
174-
if (trustSystemCerts)
175-
return true // system trusts all certificates
81+
val conn = url.openConnection() as HttpsURLConnection
82+
try {
83+
conn.connectTimeout = 5000
84+
conn.readTimeout = 5000
85+
conn.inputStream.use {
86+
return conn.serverCertificates.filterIsInstance<X509Certificate>()
17687
}
177-
logger.log(Level.INFO, "Certificate not known and running in non-interactive mode, rejecting")
178-
return false
179-
}
180-
181-
/**
182-
* Determines whether a certificate has been explicitly accepted by the user. In this case,
183-
* we can ignore an invalid host name for that certificate.
184-
*/
185-
@Synchronized
186-
override fun isTrustedByUser(cert: X509Certificate): Boolean =
187-
userKeyStore.containsValue(cert)
188-
189-
@Synchronized
190-
override fun setTrustedByUser(cert: X509Certificate) {
191-
val alias = CertUtils.getTag(cert)
192-
logger.info("Trusted by user: ${cert.subjectDN.name} ($alias)")
193-
userKeyStore[alias] = cert
194-
untrustedCerts -= cert
88+
} finally {
89+
conn.disconnect()
19590
}
196-
197-
@Synchronized
198-
override fun setUntrustedByUser(cert: X509Certificate) {
199-
logger.info("Distrusted by user: ${cert.subjectDN.name}")
200-
201-
// find certificate
202-
val alias = userKeyStore.entries.find { it.value == cert }?.key
203-
if (alias != null)
204-
// and delete, if applicable
205-
userKeyStore.remove(alias)
206-
untrustedCerts += cert
207-
}
208-
20991
}
21092

21193
}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
3+
*
4+
* This Source Code Form is subject to the terms of the Mozilla Public
5+
* License, v. 2.0. If a copy of the MPL was not distributed with this
6+
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
7+
*
8+
* SPDX-License-Identifier: MPL-2.0
9+
*/
10+
11+
package at.bitfire.cert4android
12+
13+
import androidx.annotation.VisibleForTesting
14+
import kotlinx.coroutines.flow.StateFlow
15+
import java.security.cert.X509Certificate
16+
import java.util.logging.Level
17+
import java.util.logging.Logger
18+
19+
class TestCertStore: CertStore {
20+
21+
private val logger
22+
get() = Logger.getLogger(javaClass.name)
23+
24+
/** custom TrustStore (simple map) */
25+
@VisibleForTesting
26+
internal val userKeyStore = mutableMapOf<String, X509Certificate>()
27+
28+
/** in-memory store for untrusted certs */
29+
@VisibleForTesting
30+
internal var untrustedCerts = HashSet<X509Certificate>()
31+
32+
@Synchronized
33+
override fun clearUserDecisions() {
34+
logger.info("Clearing user-(dis)trusted certificates")
35+
36+
for (alias in userKeyStore.keys)
37+
userKeyStore.remove(alias)
38+
39+
// clear untrusted certs
40+
untrustedCerts.clear()
41+
}
42+
43+
/**
44+
* Determines whether a certificate chain is trusted.
45+
*/
46+
override fun isTrusted(chain: Array<X509Certificate>, authType: String, trustSystemCerts: Boolean, appInForeground: StateFlow<Boolean>?): Boolean {
47+
if (chain.isEmpty())
48+
throw IllegalArgumentException("Certificate chain must not be empty")
49+
val cert = chain[0]
50+
51+
synchronized(this) {
52+
// explicitly accepted by user?
53+
if (isTrustedByUser(cert))
54+
return true
55+
56+
// explicitly rejected by user?
57+
if (untrustedCerts.contains(cert))
58+
return false
59+
60+
// trusted by system? (if applicable)
61+
if (trustSystemCerts)
62+
return true // system trusts all certificates
63+
}
64+
logger.log(Level.INFO, "Certificate not known and running in non-interactive mode, rejecting")
65+
return false
66+
}
67+
68+
/**
69+
* Determines whether a certificate has been explicitly accepted by the user. In this case,
70+
* we can ignore an invalid host name for that certificate.
71+
*/
72+
@Synchronized
73+
override fun isTrustedByUser(cert: X509Certificate): Boolean =
74+
userKeyStore.containsValue(cert)
75+
76+
@Synchronized
77+
override fun setTrustedByUser(cert: X509Certificate) {
78+
val alias = CertUtils.getTag(cert)
79+
logger.info("Trusted by user: ${cert.subjectDN.name} ($alias)")
80+
userKeyStore[alias] = cert
81+
untrustedCerts -= cert
82+
}
83+
84+
@Synchronized
85+
override fun setUntrustedByUser(cert: X509Certificate) {
86+
logger.info("Distrusted by user: ${cert.subjectDN.name}")
87+
88+
// find certificate
89+
val alias = userKeyStore.entries.find { it.value == cert }?.key
90+
if (alias != null)
91+
// and delete, if applicable
92+
userKeyStore.remove(alias)
93+
untrustedCerts += cert
94+
}
95+
96+
}

0 commit comments

Comments
 (0)