Skip to content

Commit

Permalink
Update fingerprint handling code for RFC 8122 conformance. (#2254)
Browse files Browse the repository at this point in the history
  • Loading branch information
@JonathanLennox
JonathanLennox authored Nov 26, 2024
1 parent 847cb3e commit a05e681
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 66 deletions.
28 changes: 28 additions & 0 deletions jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/dtls/DtlsConfig.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.jitsi.nlj.dtls

import org.bouncycastle.operator.DefaultDigestAlgorithmIdentifierFinder
import org.bouncycastle.tls.CipherSuite
import org.jitsi.config.JitsiConfig
import org.jitsi.metaconfig.ConfigException
Expand All @@ -36,11 +37,38 @@ class DtlsConfig {
}
}

val localFingerprintHashFunction: String by config {
"jmt.dtls.local-fingerprint-hash-function".from(JitsiConfig.newConfig).transformedBy {
validateHashFunction(it)
}
}

val acceptedFingerprintHashFunctions: List<String> by config {
"jmt.dtls.accepted-fingerprint-hash-functions".from(JitsiConfig.newConfig).convertFrom<List<String>> { list ->
if (list.isEmpty()) {
throw ConfigException.UnableToRetrieve.ConditionNotMet(
"accepted-fingerprint-hash-functions must not be empty"
)
}
list.map { validateHashFunction(it) }
}
}

companion object {
val config = DtlsConfig()
}
}

private fun validateHashFunction(func: String): String {
val ucFunc = func.uppercase()
DefaultDigestAlgorithmIdentifierFinder().find(ucFunc)
?: throw ConfigException.UnableToRetrieve.WrongType("Unknown hash function $func")
if (ucFunc == "MD5" || ucFunc == "MD2") {
throw ConfigException.UnableToRetrieve.WrongType("Forbidden hash function $func")
}
return func.lowercase()
}

private fun String.toBcCipherSuite(): Int = try {
CipherSuite::class.java.getDeclaredField(this).getInt(null)
} catch (e: Exception) {
Expand Down
2 changes: 1 addition & 1 deletion jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/dtls/DtlsStack.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DtlsStack(
/**
* The remote fingerprints sent to us over the signaling path.
*/
var remoteFingerprints: Map<String, String> = HashMap()
var remoteFingerprints: Map<String, List<String>> = mapOf()

/**
* A handler which will be invoked when DTLS application data is received
Expand Down
83 changes: 25 additions & 58 deletions jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/dtls/DtlsUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class DtlsUtils {
val cn = generateCN("TODO-APP-NAME", "TODO-APP-VERSION")
val keyPair = generateEcKeyPair()
val x509certificate = generateCertificate(cn, keyPair)
val localFingerprintHashFunction = x509certificate.getHashFunction()
val localFingerprintHashFunction = config.localFingerprintHashFunction
val localFingerprint = x509certificate.getFingerprint(localFingerprintHashFunction)

val certificate = org.bouncycastle.tls.Certificate(
Expand Down Expand Up @@ -158,7 +158,7 @@ class DtlsUtils {
*/
fun verifyAndValidateCertificate(
certificateInfo: org.bouncycastle.tls.Certificate,
remoteFingerprints: Map<String, String>
remoteFingerprints: Map<String, List<String>>
) {
if (certificateInfo.certificateList.isEmpty()) {
throw DtlsException("No remote fingerprints.")
Expand All @@ -179,66 +179,33 @@ class DtlsUtils {
* and validate against the fingerprints presented by the remote endpoint
* via the signaling path.
*/
private fun verifyAndValidateCertificate(certificate: Certificate, remoteFingerprints: Map<String, String>) {
// RFC 4572 "Connection-Oriented Media Transport over the Transport
// Layer Security (TLS) Protocol in the Session Description Protocol
// (SDP)" defines that "[a] certificate fingerprint MUST be computed
// using the same one-way hash function as is used in the certificate's
// signature algorithm."

val hashFunction = certificate.getHashFunction()

// As RFC 5763 "Framework for Establishing a Secure Real-time Transport
// Protocol (SRTP) Security Context Using Datagram Transport Layer
// Security (DTLS)" states, "the certificate presented during the DTLS
// handshake MUST match the fingerprint exchanged via the signaling path
// in the SDP."
val remoteFingerprint = remoteFingerprints[hashFunction] ?: throw DtlsException(
"No fingerprint declared over the signaling path with hash function: $hashFunction"
)

// TODO(boris) check if the below is still true, and re-introduce the hack if it is.
// Unfortunately, Firefox does not comply with RFC 5763 at the time
// of this writing. Its certificate uses SHA-1 and it sends a
// fingerprint computed with SHA-256. We could, of course, wait for
// Mozilla to make Firefox compliant. However, we would like to
// support Firefox in the meantime. That is why we will allow the
// fingerprint to "upgrade" the hash function of the certificate
// much like SHA-256 is an "upgrade" of SHA-1.
/*
if (remoteFingerprint == null)
{
val hashFunctionUpgrade = findHashFunctionUpgrade(hashFunction, remoteFingerprints)
if (hashFunctionUpgrade != null
&& !hashFunctionUpgrade.equalsIgnoreCase(hashFunction)) {
fingerprint = fingerprints[hashFunctionUpgrade]
if (fingerprint != null)
hashFunction = hashFunctionUpgrade
}
}
private fun verifyAndValidateCertificate(
certificate: Certificate,
remoteFingerprints: Map<String, List<String>>
) {
/** RFC 8122:
* An endpoint MUST select the set of fingerprints that use its most
* preferred hash function (out of those offered by the peer) and verify
* that each certificate used matches one fingerprint out of that set.
*/
config.acceptedFingerprintHashFunctions.forEach { hashFunction ->
val fingerprints = remoteFingerprints[hashFunction] ?: return@forEach

val certificateFingerprint = certificate.getFingerprint(hashFunction)
val certificateFingerprint = certificate.getFingerprint(hashFunction)

if (remoteFingerprint != certificateFingerprint) {
throw DtlsException(
"Fingerprint $remoteFingerprint does not match the $hashFunction-hashed " +
"certificate $certificateFingerprint"
)
if (fingerprints.none { it == certificateFingerprint }) {
throw DtlsException(
"None of the fingerprints ${fingerprints.joinToString()} match the $hashFunction-hashed " +
"certificate $certificateFingerprint"
)
}
return@verifyAndValidateCertificate
}
}

/**
* Determine and return the hash function (as a [String]) used by this certificate
*/
private fun Certificate.getHashFunction(): String {
val digAlgId = DefaultDigestAlgorithmIdentifierFinder().find(signatureAlgorithm)

return BcDefaultDigestProvider.INSTANCE
.get(digAlgId)
.algorithmName
.lowercase()
/* If we got here none of our accepted fingerprint functions were listed. */
throw DtlsException(
"No fingerprint declared over the signaling path with any of the accepted hash functions: " +
config.acceptedFingerprintHashFunctions.joinToString()
)
}

/**
Expand Down
5 changes: 5 additions & 0 deletions jitsi-media-transform/src/main/resources/reference.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ jmt {
// TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
// TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
]
// The hash function to use to generate certificate fingerprints
local-fingerprint-hash-function = sha-256
// The hash functions that are accepted for remote certificate fingerprints, in decreasing strength order
accepted-fingerprint-hash-functions = [ sha-512, sha-384, sha-256, sha-1 ]

}
srtp {
// The maximum number of packets that can be discarded early (without going through the SRTP stack for
Expand Down
54 changes: 54 additions & 0 deletions jitsi-media-transform/src/test/kotlin/org/jitsi/nlj/dtls/DtlsConfigTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package org.jitsi.nlj.dtls

import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.ShouldSpec
import io.kotest.matchers.collections.shouldContainExactly
import io.kotest.matchers.shouldBe
import org.bouncycastle.tls.CipherSuite
import org.jitsi.config.withNewConfig
Expand Down Expand Up @@ -64,5 +65,58 @@ class DtlsConfigTest : ShouldSpec() {
}
}
}
context("Valid fingerprint hash functions") {
withNewConfig(
"""
jmt.dtls.local-fingerprint-hash-function = sha-512
jmt.dtls.accepted-fingerprint-hash-functions = [ sha-512, sha-384, sha-256 ]
""".trimIndent()
) {
DtlsConfig.config.localFingerprintHashFunction shouldBe "sha-512"
DtlsConfig.config.acceptedFingerprintHashFunctions shouldContainExactly
setOf("sha-512", "sha-384", "sha-256")
}
context("With inconsistent capitalization") {
withNewConfig(
"""
jmt.dtls.local-fingerprint-hash-function = SHA-512
jmt.dtls.accepted-fingerprint-hash-functions = [ Sha-512, sHa-384, shA-256 ]
""".trimIndent()
) {
DtlsConfig.config.localFingerprintHashFunction shouldBe "sha-512"
DtlsConfig.config.acceptedFingerprintHashFunctions shouldContainExactly
setOf("sha-512", "sha-384", "sha-256")
}
}
}
context("Invalid local fingerprint hash function") {
context("Invalid name") {
withNewConfig("jmt.dtls.local-fingerprint-hash-function = sha-257") {
shouldThrow<ConfigException> { DtlsConfig.config.localFingerprintHashFunction }
}
}
context("Forbidden function") {
withNewConfig("jmt.dtls.local-fingerprint-hash-function = md5") {
shouldThrow<ConfigException> { DtlsConfig.config.localFingerprintHashFunction }
}
}
}
context("Invalid accepted accepted fingerprint hash functions") {
context("Invalid entry") {
withNewConfig("jmt.dtls.accepted-fingerprint-hash-functions = [ sha-256, sha-257 ]") {
shouldThrow<ConfigException> { DtlsConfig.config.acceptedFingerprintHashFunctions }
}
}
context("Empty") {
withNewConfig("jmt.dtls.accepted-fingerprint-hash-functions = []") {
shouldThrow<ConfigException> { DtlsConfig.config.acceptedFingerprintHashFunctions }
}
}
context("Forbidden function") {
withNewConfig("jmt.dtls.accepted-fingerprint-hash-functions = [ sha-1, md5 ]") {
shouldThrow<ConfigException> { DtlsConfig.config.acceptedFingerprintHashFunctions }
}
}
}
}
}
4 changes: 2 additions & 2 deletions jitsi-media-transform/src/test/kotlin/org/jitsi/nlj/dtls/DtlsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ class DtlsTest : ShouldSpec() {
val pcapWriter = if (pcapEnabled) PcapWriter(logger, "/tmp/dtls-test.pcap") else null

dtlsClient.remoteFingerprints = mapOf(
dtlsServer.localFingerprintHashFunction to dtlsServer.localFingerprint
dtlsServer.localFingerprintHashFunction to listOf(dtlsServer.localFingerprint)
)
dtlsServer.remoteFingerprints = mapOf(
dtlsClient.localFingerprintHashFunction to dtlsClient.localFingerprint
dtlsClient.localFingerprintHashFunction to listOf(dtlsClient.localFingerprint)
)

// The DTLS server's send is wired directly to the DTLS client's receive
Expand Down
5 changes: 3 additions & 2 deletions jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -788,11 +788,12 @@ class Endpoint @JvmOverloads constructor(
* transport information.
*/
fun setTransportInfo(transportInfo: IceUdpTransportPacketExtension) {
val remoteFingerprints = mutableMapOf<String, String>()
val remoteFingerprints = mutableMapOf<String, MutableList<String>>()
val fingerprintExtensions = transportInfo.getChildExtensionsOfType(DtlsFingerprintPacketExtension::class.java)
fingerprintExtensions.forEach { fingerprintExtension ->
if (fingerprintExtension.hash != null && fingerprintExtension.fingerprint != null) {
remoteFingerprints[fingerprintExtension.hash] = fingerprintExtension.fingerprint
remoteFingerprints.getOrPut(fingerprintExtension.hash.lowercase()) { mutableListOf() }
.add(fingerprintExtension.fingerprint)
} else {
logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toStringOpt()}")
}
Expand Down
5 changes: 3 additions & 2 deletions jvb/src/main/kotlin/org/jitsi/videobridge/relay/Relay.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,11 +612,12 @@ class Relay @JvmOverloads constructor(
* transport information.
*/
fun setTransportInfo(transportInfo: IceUdpTransportPacketExtension) {
val remoteFingerprints = mutableMapOf<String, String>()
val remoteFingerprints = mutableMapOf<String, MutableList<String>>()
val fingerprintExtensions = transportInfo.getChildExtensionsOfType(DtlsFingerprintPacketExtension::class.java)
fingerprintExtensions.forEach { fingerprintExtension ->
if (fingerprintExtension.hash != null && fingerprintExtension.fingerprint != null) {
remoteFingerprints[fingerprintExtension.hash] = fingerprintExtension.fingerprint
remoteFingerprints.getOrPut(fingerprintExtension.hash.lowercase()) { mutableListOf() }
.add(fingerprintExtension.fingerprint)
} else {
logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toStringOpt()}")
}
Expand Down
2 changes: 1 addition & 1 deletion jvb/src/main/kotlin/org/jitsi/videobridge/transport/dtls/DtlsTransport.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class DtlsTransport(parentLogger: Logger, id: String) {
}
}

fun setRemoteFingerprints(remoteFingerprints: Map<String, String>) {
fun setRemoteFingerprints(remoteFingerprints: Map<String, List<String>>) {
// Don't pass an empty list to the stack in order to avoid wiping
// certificates that were contained in a previous request.
if (remoteFingerprints.isEmpty()) {
Expand Down

0 comments on commit a05e681

Please sign in to comment.