Lets RTP packets generated from mute data sources from time to time in order...

Lets RTP packets generated from mute data sources from time to time in order to fix an issue at the receiver which could cause it to get out of sync with respect to the SRTP index of the sender. Issue reported and fix suggested by Emil Ivov.

src/org/jitsi/impl/neomedia/transform/csrc/SsrcTransformEngine.java

@@ -14,6 +14,8 @@
 
 import org.jitsi.impl.neomedia.*;
 import org.jitsi.impl.neomedia.transform.*;
+import org.jitsi.service.configuration.*;
+import org.jitsi.service.libjitsi.*;
 import org.jitsi.service.neomedia.*;
 
 /**
@@ -41,11 +43,36 @@ public class SsrcTransformEngine
             + ".dropMutedAudioSourceInReverseTransform";
 
     /**
-     * The indicator which determines whether this <tt>SsrcTransformEngine</tt>
-     * is to drop RTP packets indicated as generated from a muted audio source
-     * in {@link #reverseTransform(RawPacket)}.
+     * The indicator which determines whether <tt>SsrcTransformEngine</tt> is to
+     * drop RTP packets indicated as generated from a muted audio source in
+     * {@link #reverseTransform(RawPacket)}.
+     */
+    private static boolean dropMutedAudioSourceInReverseTransform = false;
+
+    /**
+     * The maximum number of consecutive RTP packets indicated as generated from
+     * a muted audio source to be dropped in
+     * {@link #reverseTransform(RawPacket)}. Makes sure that SRTP at the
+     *  receiving endpoint/peer sees enough sequence numbers to not get its SRTP
+     *  index out of sync with the sending endpoint/peer. 
+     */
+    private static final int MAX_DROPPED_MUTED_AUDIO_SOURCE_IN_REVERSE_TRANSFORM
+        = 1023;
+
+    /**
+     * The indicator which determines whether the method
+     * {@link #readConfigurationServicePropertiesOnce()} is to read the values
+     * of certain <tt>ConfigurationService</tt> properties of concern to
+     * <tt>SsrcTransformEngine</tt> once during the initialization of the first
+     * instance.
+     */
+    private static boolean readConfigurationServicePropertiesOnce = true;
+
+    /**
+     * The number of consecutive RTP packets indicated as generated from a muted
+     * audio source and dropped in {@link #reverseTransform(RawPacket)}.
      */
-    private final boolean dropMutedAudioSourceInReverseTransform;
+    private int droppedMutedAudioSourceInReverseTransform;
 
     /**
      * The <tt>MediaDirection</tt> in which this RTP header extension is active.
@@ -92,15 +119,7 @@ public SsrcTransformEngine(MediaStreamImpl mediaStream)
             }
         }
 
-        /*
-         * Should we simply drop RTP packets from muted audio sources? It turns
-         * out that we cannot do that because SRTP at the receiver will
-         * eventually fail to guess the rollover counter (ROC) of the sender.
-         * That is why we will never drop RTP packets from muted audio sources
-         * here and we will rather raise Buffer.FLAG_SILENCE and have SRTP drop
-         * RTP packets from muted audio sources.
-         */
-        dropMutedAudioSourceInReverseTransform = false;
+        readConfigurationServicePropertiesOnce();
     }
 
     /**
@@ -135,6 +154,30 @@ public PacketTransformer getRTPTransformer()
         return this;
     }
 
+    /**
+     * Reads the values of certain <tt>ConfigurationService</tt> properties of
+     * concern to <tt>SsrcTransformEngine</tt> once during the initialization of
+     * the first instance.
+     */
+    private static synchronized void readConfigurationServicePropertiesOnce()
+    {
+        if (readConfigurationServicePropertiesOnce)
+            readConfigurationServicePropertiesOnce = false;
+        else
+            return;
+
+        ConfigurationService cfg = LibJitsi.getConfigurationService();
+
+        if (cfg != null)
+        {
+            dropMutedAudioSourceInReverseTransform
+                = cfg.getBoolean(
+                        SsrcTransformEngine
+                            .DROP_MUTED_AUDIO_SOURCE_IN_REVERSE_TRANSFORM,
+                        dropMutedAudioSourceInReverseTransform);
+        }
+    }
+
     /**
      * Extracts the list of CSRC identifiers and passes it to the
      * <tt>MediaStream</tt> associated with this engine. Other than that the
@@ -150,6 +193,8 @@ public PacketTransformer getRTPTransformer()
      */
     public RawPacket reverseTransform(RawPacket pkt)
     {
+        boolean dropPkt = false;
+
         if ((ssrcAudioLevelExtID > 0)
                 && ssrcAudioLevelDirection.allowsReceiving()
                 && !pkt.isInvalid()
@@ -160,13 +205,27 @@ public RawPacket reverseTransform(RawPacket pkt)
             if (level == 127 /* a muted audio source */)
             {
                 if (dropMutedAudioSourceInReverseTransform)
-                    pkt = null;
+                {
+                    dropPkt
+                        = droppedMutedAudioSourceInReverseTransform
+                            < MAX_DROPPED_MUTED_AUDIO_SOURCE_IN_REVERSE_TRANSFORM;
+                }
                 else
+                {
                     pkt.setFlags(Buffer.FLAG_SILENCE | pkt.getFlags());
+                }
             }
         }
-
-        return pkt;
+        if (dropPkt)
+        {
+            droppedMutedAudioSourceInReverseTransform++;
+            return null;
+        }
+        else
+        {
+            droppedMutedAudioSourceInReverseTransform = 0;
+            return pkt;
+        }
     }
 
     public void setSsrcAudioLevelExtensionID(byte extID, MediaDirection dir)

src/org/jitsi/impl/neomedia/transform/srtp/SRTPCryptoContext.java

@@ -28,8 +28,6 @@
 
 import java.util.*;
 
-import javax.media.*;
-
 import org.bouncycastle.crypto.*;
 import org.bouncycastle.crypto.digests.*;
 import org.bouncycastle.crypto.engines.*;
@@ -38,7 +36,6 @@
 import org.jitsi.bccontrib.macs.SkeinMac;
 import org.jitsi.bccontrib.params.*;
 import org.jitsi.impl.neomedia.*;
-import org.jitsi.impl.neomedia.transform.csrc.*;
 import org.jitsi.service.configuration.*;
 import org.jitsi.service.libjitsi.*;
 import org.jitsi.util.*;
@@ -84,13 +81,6 @@ public class SRTPCryptoContext
      */
     private static boolean checkReplay = true;
 
-    /**
-     * The indicator which determines whether <tt>SRTPCryptoContext</tt>
-     * is to drop RTP packets indicated as generated from a muted audio source
-     * in {@link #reverseTransformPacket(RawPacket)}.
-     */
-    private static boolean dropMutedAudioSourceInReverseTransform = false;
-
     /**
      * The <tt>Logger</tt> used by the <tt>SRTPCryptoContext</tt> class and its
      * instances to print out debug information.
@@ -506,64 +496,42 @@ public boolean reverseTransformPacket(RawPacket pkt)
         }
 
         // Guess the SRTP index (48 bit), see RFC 3711, 3.3.1
-        // Stores the guessed ROC in this.guessedROC
+        // Stores the guessed rollover counter (ROC) in this.guessedROC.
         long guessedIndex = guessIndex(seqNo);
+        boolean b = false;
 
-        /* Replay control */
-        if (!checkReplay(seqNo, guessedIndex))
-            return false;
-
-        /* Authenticate the packet. */
-        if (checkReplay && !authenticatePacket(pkt))
-            return false;
-
-        /*
-         * If the specified pkt represents an RTP packet generated by a muted
-         * audio source, (maybe) do not waste processing power on decrypting it.
-         */
-        boolean dropPkt;
-
-        if (!dropMutedAudioSourceInReverseTransform
-                || ((Buffer.FLAG_SILENCE & pkt.getFlags()) == 0))
+        // Replay control
+        if (checkReplay(seqNo, guessedIndex))
         {
-            /*
-             * XXX If we are to drop (S)RTP packets indicated as generated from
-             * a muted audio source and the protection against relay attacks is
-             * deactivated, we will NOT authenticate such a packet in order to
-             * maximize the sparing of the CPU.
-             */
-            if (!checkReplay && !authenticatePacket(pkt))
-                return false;
-
-            switch (policy.getEncType())
+            // Authenticate the packet.
+            if (authenticatePacket(pkt))
             {
-            // Decrypt the packet using Counter Mode encryption.
-            case SRTPPolicy.AESCM_ENCRYPTION:
-            case SRTPPolicy.TWOFISH_ENCRYPTION:
-                processPacketAESCM(pkt);
-                break;
-    
-            // Decrypt the packet using F8 Mode encryption.
-            case SRTPPolicy.AESF8_ENCRYPTION:
-            case SRTPPolicy.TWOFISHF8_ENCRYPTION:
-                processPacketAESF8(pkt);
-                break;
+                switch (policy.getEncType())
+                {
+                // Decrypt the packet using Counter Mode encryption.
+                case SRTPPolicy.AESCM_ENCRYPTION:
+                case SRTPPolicy.TWOFISH_ENCRYPTION:
+                    processPacketAESCM(pkt);
+                    break;
+        
+                // Decrypt the packet using F8 Mode encryption.
+                case SRTPPolicy.AESF8_ENCRYPTION:
+                case SRTPPolicy.TWOFISHF8_ENCRYPTION:
+                    processPacketAESF8(pkt);
+                    break;
+                }
+
+                /*
+                 * Update the rollover counter and highest sequence number if
+                 * necessary.
+                 */
+                update(seqNo, guessedIndex);
+
+                b = true;
             }
-            dropPkt = false;
         }
-        else
-        {
-            /*
-             * Buffer.FLAG_SILENCE is set for RTP (as opposed to RTCP) audio (as
-             * opposed to video) packets only.
-             */
-            dropPkt = true;
-        }
-
-        // Update the rollover counter and highest sequence number if necessary.
-        update(seqNo, guessedIndex);
 
-        return !dropPkt;
+        return b;
     }
 
     /**
@@ -978,13 +946,6 @@ private static synchronized void readConfigurationServicePropertiesOnce()
         ConfigurationService cfg = LibJitsi.getConfigurationService();
 
         if (cfg != null)
-        {
             checkReplay = cfg.getBoolean(CHECK_REPLAY_PNAME, checkReplay);
-            dropMutedAudioSourceInReverseTransform
-                = cfg.getBoolean(
-                        SsrcTransformEngine
-                            .DROP_MUTED_AUDIO_SOURCE_IN_REVERSE_TRANSFORM,
-                        dropMutedAudioSourceInReverseTransform);
-        }
     }
 }