Support cryptex. (#1969)

* Support cryptex. * Add originalLength field to PacketInfo. * Move tccGenerator and remoteBandwidthEstimator after srtpDecryptNode. * Read audio levels from cryptex packets after decyption. --------- Co-authored-by: Boris Grozev <[email protected]>
jitsi · Feb 17, 2023 · 1a00812 · 1a00812
1 parent f6426ea
commit 1a00812
Show file tree

Hide file tree

Showing 17 changed files with 146 additions and 50 deletions.
diff --git a/jitsi-media-transform/pom.xml b/jitsi-media-transform/pom.xml
@@ -22,7 +22,7 @@
         <dependency>
             <groupId>${project.groupId}</groupId>
             <artifactId>jitsi-srtp</artifactId>
-            <version>1.1-7-gd8d1435</version>
+            <version>1.1-12-ga64adcc</version>
         </dependency>
         <dependency>
             <groupId>${project.groupId}</groupId>

diff --git a/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/PacketInfo.kt b/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/PacketInfo.kt
@@ -82,6 +82,8 @@ class EventTimeline(
 @SuppressFBWarnings("CN_IMPLEMENTS_CLONE_BUT_NOT_CLONEABLE")
 open class PacketInfo @JvmOverloads constructor(
     var packet: Packet,
+    /** The original length of the packet, i.e. before decryption.  Stays unchanged even if the packet is updated. */
+    val originalLength: Int = packet.length,
     val timeline: EventTimeline = EventTimeline()
 ) {
     /**
@@ -96,6 +98,9 @@ open class PacketInfo @JvmOverloads constructor(
             }
         }
 
+    /** Whether the packet originally had cryptex RTP header extensions. */
+    var originalHadCryptex: Boolean = false
+
     /**
      * Whether this packet has been recognized to contain only shouldDiscard.
      */
@@ -141,13 +146,14 @@ open class PacketInfo @JvmOverloads constructor(
      */
     fun clone(): PacketInfo {
         val clone = if (ENABLE_TIMELINE) {
-            PacketInfo(packet.clone(), timeline.clone())
+            PacketInfo(packet.clone(), originalLength, timeline.clone())
         } else {
             // If the timeline isn't enabled, we can just share the same one.
             // (This would change if we allowed enabling the timeline at runtime)
-            PacketInfo(packet.clone(), timeline)
+            PacketInfo(packet.clone(), originalLength, timeline)
         }
         clone.receivedTime = receivedTime
+        clone.originalHadCryptex = originalHadCryptex
         clone.shouldDiscard = shouldDiscard
         clone.endpointId = endpointId
         clone.layeringChanged = layeringChanged

diff --git a/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/RtpReceiverImpl.kt b/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/RtpReceiverImpl.kt
@@ -210,18 +210,20 @@ class RtpReceiverImpl @JvmOverloads constructor(
                     path = pipeline {
                         node(PacketLossNode(packetLossConfig), condition = { packetLossConfig.enabled })
                         node(RtpParser(streamInformationStore, logger))
-                        node(tccGenerator)
-                        node(remoteBandwidthEstimator)
                         // TODO: temporarily putting the audioLevelReader node here such that we can determine whether
                         // or not a packet should be discarded before doing SRTP. audioLevelReader has been moved here
                         // (instead of introducing a different class to read audio levels) to avoid parsing the RTP
                         // header extensions twice (which is expensive). In the future we will parse and cache the
                         // header extensions to make this lookup more efficient, at which time we could move
                         // audioLevelReader back to where it was (in the audio path) and add a new node here which would
                         // check for different discard conditions (i.e. checking the audio level for silence)
-                        node(audioLevelReader)
+                        node(audioLevelReader.preDecryptNode)
                         node(videoMuteNode)
                         node(srtpDecryptWrapper)
+                        node(tccGenerator)
+                        node(remoteBandwidthEstimator)
+                        // This reads audio levels from packets that use cryptex. TODO: should it go in the Audio path?
+                        node(audioLevelReader.postDecryptNode)
                         node(toggleablePcapWriter.newObserverNode())
                         node(statsTracker)
                         node(PaddingTermination(logger))

diff --git a/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/Transceiver.kt b/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/Transceiver.kt
@@ -269,18 +269,25 @@ class Transceiver(
         streamInformationStore.addSsrcAssociation(ssrcAssociation)
     }
 
-    fun setSrtpInformation(chosenSrtpProtectionProfile: Int, tlsRole: TlsRole, keyingMaterial: ByteArray) {
+    fun setSrtpInformation(
+        chosenSrtpProtectionProfile: Int,
+        tlsRole: TlsRole,
+        keyingMaterial: ByteArray,
+        cryptex: Boolean
+    ) {
         val srtpProfileInfo =
             SrtpUtil.getSrtpProfileInformationFromSrtpProtectionProfile(chosenSrtpProtectionProfile)
         logger.cdebug {
             "Transceiver $id creating transformers with:\n" +
                 "profile info:\n$srtpProfileInfo\n" +
-                "tls role: $tlsRole"
+                "tls role: $tlsRole\n" +
+                "cryptex: $cryptex"
         }
         srtpTransformers = SrtpUtil.initializeTransformer(
             srtpProfileInfo,
             keyingMaterial,
             tlsRole,
+            cryptex,
             logger
         ).also { setSrtpInformationInternal(it, true) }
     }

diff --git a/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/srtp/SrtpUtil.kt b/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/srtp/SrtpUtil.kt
@@ -120,6 +120,7 @@ class SrtpUtil {
             srtpProfileInformation: SrtpProfileInformation,
             keyingMaterial: ByteArray,
             tlsRole: TlsRole,
+            cryptex: Boolean,
             parentLogger: Logger
         ): SrtpTransformers {
             val clientWriteSrtpMasterKey = ByteArray(srtpProfileInformation.cipherKeyLength)
@@ -167,6 +168,8 @@ class SrtpUtil {
             /* TODO: disable this only in cases where we actually need to use retransmitPlain? */
             srtpPolicy.isSendReplayEnabled = false
 
+            srtpPolicy.isCryptexEnabled = cryptex
+
             val clientSrtpContextFactory = SrtpContextFactory(
                 tlsRole == TlsRole.CLIENT,
                 clientWriteSrtpMasterKey,

diff --git a/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/transform/node/RtpParser.kt b/jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/transform/node/RtpParser.kt
@@ -42,14 +42,18 @@ class RtpParser(
             return null
         }
 
-        packetInfo.packet = when (payloadType.mediaType) {
+        val rtpPacket = when (payloadType.mediaType) {
             MediaType.AUDIO -> when (payloadType.encoding) {
                 RED -> packet.toOtherType(::RedAudioRtpPacket)
                 else -> packet.toOtherType(::AudioRtpPacket)
             }
             MediaType.VIDEO -> packet.toOtherType(::VideoRtpPacket)
             else -> throw Exception("Unrecognized media type: '${payloadType.mediaType}'")
         }
+        packetInfo.packet = rtpPacket
+        if (rtpPacket.extensionsProfileType == 0xC0DE || rtpPacket.extensionsProfileType == 0xC2DE) {
+            packetInfo.originalHadCryptex = true
+        }
 
         packetInfo.resetPayloadVerification()
         return packetInfo

diff --git a/...media-transform/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/AudioLevelReader.kt b/...media-transform/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/AudioLevelReader.kt
@@ -32,7 +32,13 @@ import org.jitsi.rtp.rtp.header_extensions.AudioLevelHeaderExtension
  */
 class AudioLevelReader(
     streamInformationStore: ReadOnlyStreamInformationStore
-) : ObserverNode("Audio level reader") {
+) {
+    /**
+     *  Process packets without cryptex pre-SRTP to allow the "skip decryption" optimization if they are to be dropped.
+     */
+    val preDecryptNode = AudioLevelReaderNode("Audio level reader (pre-srtp)") { !it.originalHadCryptex }
+    val postDecryptNode = AudioLevelReaderNode("Audio level reader (post-srtp)") { it.originalHadCryptex }
+
     private var audioLevelExtId: Int? = null
     var audioLevelListener: AudioLevelListener? = null
     var forwardedSilencePackets: Int = 0
@@ -49,48 +55,56 @@ class AudioLevelReader(
         }
     }
 
-    override fun observe(packetInfo: PacketInfo) {
-        val audioRtpPacket = packetInfo.packet as? AudioRtpPacket ?: return
-
-        audioLevelExtId?.let { audioLevelId ->
-            audioRtpPacket.getHeaderExtension(audioLevelId)?.let { ext ->
-                stats.audioLevel()
-
-                val level = AudioLevelHeaderExtension.getAudioLevel(ext)
-                val silence = level == MUTED_LEVEL
-
-                if (!silence) stats.nonSilence(AudioLevelHeaderExtension.getVad(ext))
-                if (silence && forwardedSilencePackets > forwardedSilencePacketsLimit) {
-                    packetInfo.shouldDiscard = true
-                    stats.discardedSilence()
-                } else if (this.forceMute) {
-                    packetInfo.shouldDiscard = true
-                    stats.discardedForceMute()
-                } else {
-                    forwardedSilencePackets = if (silence) forwardedSilencePackets + 1 else 0
-                    audioLevelListener?.let { listener ->
-                        if (listener.onLevelReceived(audioRtpPacket.ssrc, (127 - level).toPositiveLong())) {
-                            packetInfo.shouldDiscard = true
-                            stats.discardedRanking()
+    inner class AudioLevelReaderNode(
+        name: String,
+        val shouldProcess: (PacketInfo) -> Boolean
+    ) : ObserverNode(name) {
+
+        override fun observe(packetInfo: PacketInfo) {
+            if (!shouldProcess(packetInfo)) return
+
+            val audioRtpPacket = packetInfo.packet as? AudioRtpPacket ?: return
+
+            audioLevelExtId?.let { audioLevelId ->
+                audioRtpPacket.getHeaderExtension(audioLevelId)?.let { ext ->
+                    stats.audioLevel()
+
+                    val level = AudioLevelHeaderExtension.getAudioLevel(ext)
+                    val silence = level == MUTED_LEVEL
+
+                    if (!silence) stats.nonSilence(AudioLevelHeaderExtension.getVad(ext))
+                    if (silence && forwardedSilencePackets > forwardedSilencePacketsLimit) {
+                        packetInfo.shouldDiscard = true
+                        stats.discardedSilence()
+                    } else if (this@AudioLevelReader.forceMute) {
+                        packetInfo.shouldDiscard = true
+                        stats.discardedForceMute()
+                    } else {
+                        forwardedSilencePackets = if (silence) forwardedSilencePackets + 1 else 0
+                        audioLevelListener?.let { listener ->
+                            if (listener.onLevelReceived(audioRtpPacket.ssrc, (127 - level).toPositiveLong())) {
+                                packetInfo.shouldDiscard = true
+                                stats.discardedRanking()
+                            }
                         }
                     }
                 }
             }
         }
-    }
 
-    override fun getNodeStats(): NodeStatsBlock = super.getNodeStats().apply {
-        addString("audio_level_ext_id", audioLevelExtId.toString())
-        addNumber("num_audio_levels", stats.numAudioLevels)
-        addNumber("num_silence_packets_discarded", stats.numDiscardedSilence)
-        addNumber("num_force_mute_discarded", stats.numDiscardedForceMute)
-        addNumber("num_ranking_discarded", stats.numDiscardedRanking)
-        addNumber("num_non_silence", stats.numNonSilence)
-        addNumber("num_non_silence_with_vad", stats.numNonSilenceWithVad)
-        addBoolean("force_mute", forceMute)
-    }
+        override fun getNodeStats(): NodeStatsBlock = super.getNodeStats().apply {
+            addString("audio_level_ext_id", audioLevelExtId.toString())
+            addNumber("num_audio_levels", stats.numAudioLevels)
+            addNumber("num_silence_packets_discarded", stats.numDiscardedSilence)
+            addNumber("num_force_mute_discarded", stats.numDiscardedForceMute)
+            addNumber("num_ranking_discarded", stats.numDiscardedRanking)
+            addNumber("num_non_silence", stats.numNonSilence)
+            addNumber("num_non_silence_with_vad", stats.numNonSilenceWithVad)
+            addBoolean("force_mute", forceMute)
+        }
 
-    override fun trace(f: () -> Unit) = f.invoke()
+        override fun trace(f: () -> Unit) = f.invoke()
+    }
 
     companion object {
         const val MUTED_LEVEL = 127

diff --git a/...ansform/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/RemoteBandwidthEstimator.kt b/...ansform/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/RemoteBandwidthEstimator.kt
@@ -106,7 +106,7 @@ class RemoteBandwidthEstimator(
                     AbsSendTimeHeaderExtension.getTime(ext),
                     packetInfo.receivedTime,
                     rtpPacket.sequenceNumber,
-                    rtpPacket.length.bytes
+                    packetInfo.originalLength.bytes
                 )
                 /* With receiver-side bwe we need to treat each received packet as separate feedback */
                 bwe.feedbackComplete(now)

diff --git a/jitsi-media-transform/src/test/kotlin/org/jitsi/nlj/module_tests/SrtpTransformerFactory.kt b/jitsi-media-transform/src/test/kotlin/org/jitsi/nlj/module_tests/SrtpTransformerFactory.kt
@@ -28,6 +28,7 @@ class SrtpTransformerFactory {
                 srtpData.srtpProfileInformation,
                 srtpData.keyingMaterial,
                 srtpData.tlsRole,
+                cryptex = false, /* TODO: add tests for the cryptex=true case */
                 StdoutLogger()
             )
         }

diff --git a/...-media-transform/src/test/kotlin/org/jitsi/nlj/transform/node/incoming/SrtpDecryptTest.kt b/...-media-transform/src/test/kotlin/org/jitsi/nlj/transform/node/incoming/SrtpDecryptTest.kt
@@ -36,6 +36,7 @@ internal class SrtpDecryptTest : ShouldSpec() {
             SrtpSample.srtpProfileInformation,
             SrtpSample.keyingMaterial.array(),
             SrtpSample.tlsRole,
+            cryptex = false, /* TODO: add tests for cryptex case */
             StdoutLogger()
         )
 

diff --git a/...-media-transform/src/test/kotlin/org/jitsi/nlj/transform/node/outgoing/SrtpEncryptTest.kt b/...-media-transform/src/test/kotlin/org/jitsi/nlj/transform/node/outgoing/SrtpEncryptTest.kt
@@ -41,6 +41,7 @@ internal class SrtpEncryptTest : ShouldSpec() {
             SrtpSample.srtpProfileInformation,
             SrtpSample.keyingMaterial.array(),
             SrtpSample.tlsRole,
+            cryptex = false, /* TODO: add tests for cryptex case */
             StdoutLogger()
         )
 

diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/CryptexConfig.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/CryptexConfig.kt
@@ -0,0 +1,31 @@
+/*
+ * Copyright @ 2018 - 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.videobridge
+
+import org.jitsi.config.JitsiConfig
+import org.jitsi.metaconfig.config
+import org.jitsi.metaconfig.from
+
+class CryptexConfig private constructor() {
+    companion object {
+        val endpoint: Boolean by config(
+            "videobridge.cryptex.endpoint".from(JitsiConfig.newConfig)
+        )
+        val relay: Boolean by config(
+            "videobridge.cryptex.relay".from(JitsiConfig.newConfig)
+        )
+    }
+}
diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt
@@ -129,7 +129,9 @@ class Endpoint @JvmOverloads constructor(
 
     /* TODO: do we ever want to support useUniquePort for an Endpoint? */
     private val iceTransport = IceTransport(id, iceControlling, false, logger)
-    private val dtlsTransport = DtlsTransport(logger)
+    private val dtlsTransport = DtlsTransport(logger).also { it.cryptex = CryptexConfig.endpoint }
+
+    private var cryptex: Boolean = CryptexConfig.endpoint
 
     private val diagnosticContext = conference.newDiagnosticContext().apply {
         put("endpoint_id", id)
@@ -417,7 +419,7 @@ class Endpoint @JvmOverloads constructor(
                 keyingMaterial: ByteArray
             ) {
                 logger.info("DTLS handshake complete")
-                transceiver.setSrtpInformation(chosenSrtpProtectionProfile, tlsRole, keyingMaterial)
+                transceiver.setSrtpInformation(chosenSrtpProtectionProfile, tlsRole, keyingMaterial, cryptex)
                 // TODO(brian): the old code would work even if the sctp connection was created after
                 //  the handshake had completed, but this won't (since this is a one-time event).  do
                 //  we need to worry about that case?
@@ -771,6 +773,9 @@ class Endpoint @JvmOverloads constructor(
             } else {
                 logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toXML()}")
             }
+            if (CryptexConfig.endpoint) {
+                cryptex = cryptex && fingerprintExtension.cryptex
+            }
         }
         dtlsTransport.setRemoteFingerprints(remoteFingerprints)
         if (fingerprintExtensions.isNotEmpty()) {

diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/relay/Relay.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/relay/Relay.kt
@@ -66,6 +66,7 @@ import org.jitsi.utils.logging2.createChildLogger
 import org.jitsi.utils.queue.CountingErrorHandler
 import org.jitsi.videobridge.AbstractEndpoint
 import org.jitsi.videobridge.Conference
+import org.jitsi.videobridge.CryptexConfig
 import org.jitsi.videobridge.EncodingsManager
 import org.jitsi.videobridge.Endpoint
 import org.jitsi.videobridge.PotentialPacketHandler
@@ -149,7 +150,9 @@ class Relay @JvmOverloads constructor(
     private var expired = false
 
     private val iceTransport = IceTransport(id, iceControlling, useUniquePort, logger, clock)
-    private val dtlsTransport = DtlsTransport(logger)
+    private val dtlsTransport = DtlsTransport(logger).also { it.cryptex = CryptexConfig.relay }
+
+    private var cryptex = CryptexConfig.relay
 
     private val diagnosticContext = conference.newDiagnosticContext().apply {
         put("relay_id", id)
@@ -362,6 +365,7 @@ class Relay @JvmOverloads constructor(
             srtpProfileInfo,
             keyingMaterial,
             tlsRole,
+            cryptex,
             logger
         )
         this.srtpTransformers = srtpTransformers
@@ -389,6 +393,10 @@ class Relay @JvmOverloads constructor(
             } else {
                 logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toXML()}")
             }
+
+            if (CryptexConfig.relay) {
+                cryptex = cryptex && fingerprintExtension.cryptex
+            }
         }
         dtlsTransport.setRemoteFingerprints(remoteFingerprints)
         if (fingerprintExtensions.isNotEmpty()) {

diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/transport/dtls/DtlsTransport.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/transport/dtls/DtlsTransport.kt
@@ -56,6 +56,9 @@ class DtlsTransport(parentLogger: Logger) {
 
     private val stats = Stats()
 
+    /** Whether to advertise cryptex to peers. */
+    var cryptex = false
+
     /**
      * The DTLS stack instance
      */
@@ -159,6 +162,9 @@ class DtlsTransport(parentLogger: Logger) {
         }
         fingerprintPE.fingerprint = dtlsStack.localFingerprint
         fingerprintPE.hash = dtlsStack.localFingerprintHashFunction
+        if (cryptex) {
+            fingerprintPE.cryptex = true
+        }
     }
 
     /**