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

chore: configure and allow cipher suites (WPB-5448) #2233

Merged
merged 4 commits into from
Nov 17, 2023
Merged

chore: configure and allow cipher suites (WPB-5448) #2233

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2ee0c52
chore: configure and allow cipher suites
yamilmedina Nov 16, 2023
63db95c
Merge branch 'develop' into chore/allowed-cipher-suites
yamilmedina Nov 16, 2023
53db50f
Merge branch 'develop' into chore/allowed-cipher-suites
MohamadJaara Nov 16, 2023
151761f
chore: use restricted tls config from ktor
yamilmedina Nov 17, 2023
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
12 changes: 9 additions & 3 deletions network/src/commonJvmAndroid/kotlin/com/wire/kalium/network/HttpEngine.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ import com.wire.kalium.network.tools.isProxyRequired
import io.ktor.client.engine.HttpClientEngine
import io.ktor.client.engine.okhttp.OkHttp
import okhttp3.CertificatePinner
import okhttp3.CipherSuite.Companion.TLS_AES_128_GCM_SHA256
import okhttp3.CipherSuite.Companion.TLS_AES_256_GCM_SHA384
import okhttp3.CipherSuite.Companion.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
import okhttp3.ConnectionSpec
import okhttp3.OkHttpClient
import okhttp3.TlsVersion
Expand All @@ -51,7 +54,7 @@ internal object OkHttpSingleton {
.connectTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
.readTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
.writeTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
}.build()
}.connectionSpecs(supportedConnectionSpecs()).build()

fun createNew(block: OkHttpClient.Builder.() -> Unit): OkHttpClient {
return sharedClient.newBuilder().apply(block).build()
Expand Down Expand Up @@ -98,8 +101,6 @@ actual fun defaultHttpEngine(
proxy(proxy)
}

connectionSpecs(supportedConnectionSpecs())

}.also {
preconfigured = it
webSocketFactory = KaliumWebSocketFactory(it)
Expand All @@ -125,6 +126,11 @@ private fun OkHttpClient.Builder.ignoreAllSSLErrors() {
private fun supportedConnectionSpecs(): List<ConnectionSpec> {
val wireSpec = ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
.tlsVersions(TlsVersion.TLS_1_2)
.cipherSuites(
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
TLS_AES_128_GCM_SHA256,
TLS_AES_256_GCM_SHA384
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: the same okHttp is reused not only to connect to wire server but also to other servers (fetch custom e.g. server config), do we want to limit this change to only okhttp instances that are used to connect to wire servers?

Copy link
Contributor Author

@yamilmedina yamilmedina Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the case, for custom and wire cloud, accordingly to the latest deployments in prod envs.
But anyway I will check if we are able to use a broader spectrum, just in case to be on the safer side.

.build()

return listOf(wireSpec, ConnectionSpec.CLEARTEXT)
Expand Down
61 changes: 59 additions & 2 deletions network/src/jvmTest/kotlin/com/wire/kalium/HttpClientConnectionSpecsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,81 @@
package com.wire.kalium

import com.wire.kalium.network.OkHttpSingleton
import okhttp3.CipherSuite
import okhttp3.CipherSuite.Companion.TLS_CHACHA20_POLY1305_SHA256
import okhttp3.CipherSuite.Companion.TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256
import okhttp3.CipherSuite.Companion.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
import okhttp3.CipherSuite.Companion.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384
import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
import okhttp3.CipherSuite.Companion.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
import okhttp3.CipherSuite.Companion.TLS_RSA_WITH_3DES_EDE_CBC_SHA
import okhttp3.CipherSuite.Companion.TLS_RSA_WITH_AES_128_CBC_SHA
import okhttp3.CipherSuite.Companion.TLS_RSA_WITH_AES_128_CBC_SHA256
import okhttp3.CipherSuite.Companion.TLS_RSA_WITH_AES_128_GCM_SHA256
import okhttp3.CipherSuite.Companion.TLS_RSA_WITH_AES_256_CBC_SHA
import okhttp3.CipherSuite.Companion.TLS_RSA_WITH_AES_256_GCM_SHA384
import okhttp3.ConnectionSpec
import okhttp3.TlsVersion
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class HttpClientConnectionSpecsTest {

@Test
// This test conforms to the following testing standards:
// @SF.Channel @TSFI.RESTfulAPI @S0.2 @S0.3 @S3
fun givenTheHttpClientIsCreated_ThenEnsureOnlySupportedSpecsArePresent() {
val connectionSpecs = OkHttpSingleton.createNew {}.connectionSpecs
with(connectionSpecs[0]) {
tlsVersions?.let {
assertTrue(it.contains(TlsVersion.TLS_1_2) && it.contains(TlsVersion.TLS_1_3))
assertTrue(!it.contains(TlsVersion.TLS_1_1) && !it.contains(TlsVersion.TLS_1_0) && !it.contains(TlsVersion.SSL_3_0))
assertTrue { validTlsVersions.containsAll(it) }
assertFalse { notValidTlsVersions.containsAll(it) }
}

cipherSuites?.let {
assertTrue { validCipherSuites.containsAll(it) }
assertFalse { notValidCipherSuites.containsAll(it) }
}
}

assertEquals(connectionSpecs[1], ConnectionSpec.CLEARTEXT)
}

private companion object {
val validTlsVersions = listOf(TlsVersion.TLS_1_3, TlsVersion.TLS_1_2)
val notValidTlsVersions = listOf(TlsVersion.TLS_1_1, TlsVersion.TLS_1_0, TlsVersion.SSL_3_0)

val notValidCipherSuites = listOf(
TLS_CHACHA20_POLY1305_SHA256,
TLS_RSA_WITH_3DES_EDE_CBC_SHA,
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384,
TLS_RSA_WITH_AES_128_CBC_SHA256,
TLS_RSA_WITH_AES_128_CBC_SHA,
TLS_RSA_WITH_AES_128_GCM_SHA256,
TLS_RSA_WITH_AES_256_CBC_SHA,
TLS_RSA_WITH_AES_256_GCM_SHA384,
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
)

val validCipherSuites = listOf(
CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
CipherSuite.TLS_AES_128_GCM_SHA256,
CipherSuite.TLS_AES_256_GCM_SHA384
)
}
}
Loading