Skip to content

Commit

Permalink
Cleanup DTLS 1.0 leftovers (#1983)
Browse files Browse the repository at this point in the history
* Removed DTLS 1.0 from client and server

* added more ciphers

* updated cipher list based on Mozilla SSL configurator

* Make dtls cipher suites configurable.

* Use cipher suites from config.

* squash: Remove unused imports.

* Require non-empty cipher suites.

* squash: Fix ktlint.

* Fix typo.

* Better exception

* commented out AES 256 ciphers, because they are not supported yet

* exit early on mis-configured dtls ciphers

* don't restart after configuration error

Co-authored-by: Boris Grozev <boris@jitsi.org>
  • Loading branch information
nils-ohlmeier and bgrozev committed Jan 26, 2023
1 parent ad606ca commit 0aa0b1f
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 23 deletions.
2 changes: 2 additions & 0 deletions debian/jitsi-videobridge2.service
Expand Up @@ -5,6 +5,8 @@ Wants=network-online.target

[Service]
SuccessExitStatus=143
# configuration error prevents restart loops
RestartPreventExitStatus=78
# allow bind to 80 and 443
AmbientCapabilities=CAP_NET_BIND_SERVICE
EnvironmentFile=/etc/jitsi/videobridge/config
Expand Down
Expand Up @@ -15,12 +15,34 @@
*/
package org.jitsi.nlj.dtls

import org.bouncycastle.tls.CipherSuite
import org.jitsi.config.JitsiConfig
import org.jitsi.metaconfig.ConfigException
import org.jitsi.metaconfig.config
import java.time.Duration

class DtlsConfig {
val handshakeTimeout: Duration by config {
"jmt.dtls.handshake-timeout".from(JitsiConfig.newConfig)
}

val cipherSuites: List<Int> by config {
"jmt.dtls.cipher-suites".from(JitsiConfig.newConfig).convertFrom<List<String>> { list ->
val ciphers = list.map { it.toBcCipherSuite() }
if (ciphers.isEmpty()) {
throw ConfigException.UnableToRetrieve.ConditionNotMet("cipher-suites must not be empty")
}
ciphers
}
}

companion object {
val config = DtlsConfig()
}
}

private fun String.toBcCipherSuite(): Int = try {
CipherSuite::class.java.getDeclaredField(this).getInt(null)
} catch (e: Exception) {
throw ConfigException.UnableToRetrieve.ConditionNotMet("Value is not a valid BouncyCastle cipher suite name: $this")
}
Expand Up @@ -19,7 +19,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
import org.bouncycastle.crypto.util.PrivateKeyFactory
import org.bouncycastle.tls.Certificate
import org.bouncycastle.tls.CertificateRequest
import org.bouncycastle.tls.CipherSuite
import org.bouncycastle.tls.DefaultTlsClient
import org.bouncycastle.tls.ExporterLabel
import org.bouncycastle.tls.ExtensionType
Expand Down Expand Up @@ -130,12 +129,7 @@ class TlsClientImpl(
DtlsUtils.chooseSrtpProtectionProfile(SrtpConfig.protectionProfiles, protectionProfiles.asIterable())
}

override fun getCipherSuites(): IntArray {
return intArrayOf(
CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
)
}
override fun getCipherSuites() = DtlsConfig.config.cipherSuites.toIntArray()

override fun getHandshakeTimeoutMillis(): Int = DtlsUtils.config.handshakeTimeout.toMillis().toInt()

Expand Down Expand Up @@ -174,7 +168,7 @@ class TlsClientImpl(
}

override fun getSupportedVersions(): Array<ProtocolVersion> =
ProtocolVersion.DTLSv12.downTo(ProtocolVersion.DTLSv10)
arrayOf<ProtocolVersion>(ProtocolVersion.DTLSv12)

override fun notifyAlertRaised(alertLevel: Short, alertDescription: Short, message: String?, cause: Throwable?) =
logger.notifyAlertRaised(alertLevel, alertDescription, message, cause)
Expand Down
Expand Up @@ -20,7 +20,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
import org.bouncycastle.crypto.util.PrivateKeyFactory
import org.bouncycastle.tls.Certificate
import org.bouncycastle.tls.CertificateRequest
import org.bouncycastle.tls.CipherSuite
import org.bouncycastle.tls.ClientCertificateType
import org.bouncycastle.tls.DefaultTlsServer
import org.bouncycastle.tls.ExporterLabel
Expand Down Expand Up @@ -99,12 +98,7 @@ class TlsServerImpl(
DtlsUtils.chooseSrtpProtectionProfile(SrtpConfig.protectionProfiles, protectionProfiles.asIterable())
}

override fun getCipherSuites(): IntArray {
return intArrayOf(
CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
)
}
override fun getCipherSuites() = DtlsConfig.config.cipherSuites.toIntArray()

override fun getRSAEncryptionCredentials(): TlsCredentialedDecryptor {
return BcDefaultTlsCredentialedDecryptor(
Expand Down Expand Up @@ -132,15 +126,7 @@ class TlsServerImpl(
override fun getCertificateRequest(): CertificateRequest {
val signatureAlgorithms = Vector<SignatureAndHashAlgorithm>(1)
signatureAlgorithms.add(SignatureAndHashAlgorithm(HashAlgorithm.sha256, SignatureAlgorithm.ecdsa))
signatureAlgorithms.add(SignatureAndHashAlgorithm(HashAlgorithm.sha1, SignatureAlgorithm.rsa))
return when (context.clientVersion) {
ProtocolVersion.DTLSv10 -> {
CertificateRequest(
shortArrayOf(ClientCertificateType.rsa_sign),
null,
null
)
}
ProtocolVersion.DTLSv12 -> {
CertificateRequest(
shortArrayOf(ClientCertificateType.ecdsa_sign),
Expand Down
13 changes: 13 additions & 0 deletions jitsi-media-transform/src/main/resources/reference.conf
Expand Up @@ -30,6 +30,19 @@ jmt {
dtls {
// The maximum length of time to wait for a DTLS handshake to complete.
handshake-timeout = 30 seconds

// The list of cipher suites to use for DTLS. The names must correspond to the constants in BouncyCastle's
// CipherSuite class.
cipher-suites = [
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
TLS_DHE_RSA_WITH_AES_128_GCM_SHA256
// Looks like none of the browsers supports AES 256 for DTLS yet:
// TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
// TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
]
}
srtp {
// The maximum number of packets that can be discarded early (without going through the SRTP stack for
Expand Down
@@ -0,0 +1,28 @@
/*
* Copyright @ 2022 - present 8x8, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.jitsi.nlj

import io.kotest.core.config.AbstractProjectConfig
import org.jitsi.metaconfig.MetaconfigSettings

class KotestProjectConfig : AbstractProjectConfig() {
override fun beforeAll() = super.beforeAll().also {
// The only purpose of config caching is performance. We always want caching disabled in tests (so we can
// freely modify the config without affecting other tests executing afterwards).
MetaconfigSettings.cacheEnabled = false
}
}
@@ -0,0 +1,68 @@
/*
* Copyright @ 2023 - present 8x8, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jitsi.nlj.dtls

import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.ShouldSpec
import io.kotest.matchers.shouldBe
import org.bouncycastle.tls.CipherSuite
import org.jitsi.config.withNewConfig
import org.jitsi.metaconfig.ConfigException

class DtlsConfigTest : ShouldSpec() {
init {
context("Valid cipher suites") {
withNewConfig(
"""
jmt.dtls.cipher-suites = [
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
]
""".trimIndent()
) {
DtlsConfig.config.cipherSuites shouldBe listOf(
CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
)
}
}
context("Invalid cipher suites") {
context("Invalid name") {
withNewConfig(
"""
jmt.dtls.cipher-suites = [
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
invalid
]
""".trimIndent()
) {
shouldThrow<ConfigException> { DtlsConfig.config.cipherSuites }
}
}

context("Empty") {
withNewConfig("jmt.dtls.cipher-suites = []") {
shouldThrow<ConfigException> { DtlsConfig.config.cipherSuites }
}
}
context("Wrong type") {
withNewConfig("jmt.dtls.cipher-suites = 42") {
shouldThrow<ConfigException> { DtlsConfig.config.cipherSuites }
}
}
}
}
}
12 changes: 12 additions & 0 deletions jvb/src/main/kotlin/org/jitsi/videobridge/Main.kt
Expand Up @@ -20,8 +20,10 @@ import org.eclipse.jetty.servlet.ServletHolder
import org.glassfish.jersey.servlet.ServletContainer
import org.ice4j.ice.harvest.MappingCandidateHarvesters
import org.jitsi.config.JitsiConfig
import org.jitsi.metaconfig.ConfigException
import org.jitsi.metaconfig.MetaconfigLogger
import org.jitsi.metaconfig.MetaconfigSettings
import org.jitsi.nlj.dtls.DtlsConfig
import org.jitsi.rest.JettyBundleActivatorConfig
import org.jitsi.rest.createServer
import org.jitsi.rest.enableCors
Expand Down Expand Up @@ -74,6 +76,16 @@ fun main() {
XmppStringPrepUtil.setMaxCacheSizes(XmppClientConnectionConfig.config.jidCacheSize)
PacketQueue.setEnableStatisticsDefault(true)

// Trigger an exception early in case the DTLS cipher suites are misconfigured
try {
DtlsConfig.config.cipherSuites
} catch (ce: ConfigException) {
logger.error("Dtls configuration error: $ce")
// According to https://freedesktop.org/software/systemd/man/systemd.exec…html#Process%20Exit%20Code
// 78 means "configuration error"
exitProcess(78)
}

val xmppConnection = XmppConnection().apply { start() }
val shutdownService = ShutdownServiceImpl()
val videobridge = Videobridge(
Expand Down

0 comments on commit 0aa0b1f

Please sign in to comment.