Fix SRTP AES-GCM ROC poisoning on unauthenticated packets (#1662)

For AEAD/GCM SRTP profiles, packet authentication happens during the
payload decrypt (GcmBlockCipher.DoFinal), not in a separate HMAC step.
UnprotectRtp, however, advanced the replay window / highest-index state
(SsrcSrtpContext.S_l, which carries the ROC) via CheckAndUpdateReplayWindow
*before* that decrypt. As a result a single packet that failed AEAD
authentication - or a stray/reordered packet with a higher sequence number -
permanently advanced S_l and then threw. Every subsequent packet derived its
AES-GCM IV from the poisoned ROC (or was rejected by the replay window),
producing an endless "mac check in GCM failed" with no recovery.

This violated RFC 3711 section 3.3, which requires the replay list, s_l and
ROC to be updated only AFTER the packet has been authenticated. HMAC profiles
were unaffected because their authentication already ran before the replay
update; only the GCM/AEAD path (used by WebRTC / the OpenAI realtime endpoint)
was exposed.

Fix (RTP path):
- Split SsrcSrtpContext.CheckAndUpdateReplayWindow into a read-only
  CheckReplayWindow (run before decrypt) and a mutating UpdateReplayWindow
  (run only after the packet authenticates).
- Wrap the decrypt switch so a failed AEAD authentication returns
  ERROR_HMAC_CHECK_FAILED instead of throwing, and does not advance any state.
- Advance the replay window / ROC only after a successful decrypt.

This also closes a minor DoS: previously a forged high-sequence packet could
advance the replay window and drop legitimate traffic.

The RTCP path (UnprotectRtcp) has the same pre-auth update and additionally
derives its IV from S_l; it is left unchanged here (its explicit per-packet
SRTCP index makes it lower-risk) and tracked as a follow-up.

Adds SrtpGcmReplayAuthUnitTest which injects a packet that fails AEAD auth
mid-stream and asserts (1) it is rejected with ERROR_HMAC_CHECK_FAILED without
throwing and (2) the next legitimate packet still decrypts. The test fails on
the previous implementation and passes after the fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: sipsorcery

src/SIPSorcery/net/DtlsSrtp/Lib/SRTP/SrtpContext.cs

@@ -109,6 +109,81 @@ public bool CheckAndUpdateReplayWindow(uint sequenceNumber)
             return true; /* out of order but good */
         }
 
+        /// <summary>
+        /// Read-only replay check. Returns whether <paramref name="sequenceNumber"/> (a 32-bit packet
+        /// index) is acceptable, WITHOUT mutating any state. Per RFC 3711 section 3.3 the replay list,
+        /// s_l and ROC MUST NOT be advanced until the packet has been authenticated, so callers must do
+        /// this read-only check before decryption and only call <see cref="UpdateReplayWindow"/> once
+        /// the packet has authenticated.
+        /// </summary>
+        /// <param name="sequenceNumber">RTP/RTCP 32-bit packet index.</param>
+        /// <returns>true if the packet is not a replay and is within the window; otherwise false.</returns>
+        /// <remarks>https://datatracker.ietf.org/doc/html/rfc2401 Appendix C</remarks>
+        public bool CheckReplayWindow(uint sequenceNumber)
+        {
+            if (sequenceNumber == 0)
+            {
+                return !S_l_set; /* index 0 only acceptable as the very first packet */
+            }
+
+            if (sequenceNumber > S_l)
+            {
+                return true; /* new larger index */
+            }
+
+            var diff = (int)(S_l - sequenceNumber);
+            if (diff >= REPLAY_WINDOW_SIZE)
+            {
+                return false; /* too old or wrapped */
+            }
+
+            if ((Bitmap & ((ulong)1 << diff)) == ((ulong)1 << diff))
+            {
+                return false; /* already seen */
+            }
+
+            return true; /* out of order but within the window and not yet seen */
+        }
+
+        /// <summary>
+        /// Advances the replay window / highest-index state for an index that has already passed
+        /// <see cref="CheckReplayWindow"/> AND been authenticated. Must only be called after the packet
+        /// authenticates, per RFC 3711 section 3.3, otherwise an unauthenticated packet could desync the
+        /// ROC for the whole stream.
+        /// </summary>
+        /// <param name="sequenceNumber">The 32-bit packet index that was authenticated.</param>
+        public void UpdateReplayWindow(uint sequenceNumber)
+        {
+            if (sequenceNumber == 0)
+            {
+                S_l_set = true;
+                return;
+            }
+
+            if (sequenceNumber > S_l)
+            {
+                var diff = (int)(sequenceNumber - S_l);
+                if (diff < REPLAY_WINDOW_SIZE)
+                {
+                    Bitmap = Bitmap << diff;
+                    Bitmap |= 1; /* set bit for this packet */
+                }
+                else
+                {
+                    Bitmap = 1; /* This packet has a "way larger" index */
+                }
+                S_l = sequenceNumber;
+                S_l_set = true;
+                return;
+            }
+
+            var d = (int)(S_l - sequenceNumber);
+            if (d < REPLAY_WINDOW_SIZE)
+            {
+                Bitmap |= ((ulong)1 << d); /* mark as seen */
+            }
+        }
+
         public void SetInitialSequence(uint sequenceNumber)
         {
             if (!S_l_set)
@@ -824,12 +899,16 @@ public virtual int UnprotectRtp(ReadOnlyBytes input, Bytes output, out int outpu
 
             var offset = RtpReader.ReadHeaderLen(input);
 
-            if (!ssrcContext.CheckAndUpdateReplayWindow(index))
+            // Read-only replay check. The window/ROC state is NOT advanced here; that only happens once
+            // the packet has authenticated (UpdateReplayWindow below). RFC 3711 section 3.3.
+            if (!ssrcContext.CheckReplayWindow(index))
             {
                 outputBufferLength = 0;
                 return ERROR_REPLAY_CHECK_FAILED;
             }
 
+            try
+            {
             switch (context.Cipher)
             {
                 case SrtpCiphers.NULL:
@@ -970,6 +1049,20 @@ public virtual int UnprotectRtp(ReadOnlyBytes input, Bytes output, out int outpu
                         return ERROR_UNSUPPORTED_CIPHER;
                     }
             }
+            }
+            catch (Org.BouncyCastle.Crypto.InvalidCipherTextException)
+            {
+                // AEAD (GCM/CCM) authentication failed. Drop the packet WITHOUT advancing the replay
+                // window / ROC, so a single unauthenticated, corrupted or reordered packet cannot desync
+                // the ROC and cause every subsequent packet to fail to decrypt. RFC 3711 section 3.3.
+                outputBufferLength = 0;
+                return ERROR_HMAC_CHECK_FAILED;
+            }
+
+            // The packet has now been authenticated (HMAC above for HMAC profiles, or the AEAD decrypt
+            // for GCM/CCM profiles). Only now is it safe to advance the replay window / ROC.
+            // RFC 3711 section 3.3.
+            ssrcContext.UpdateReplayWindow(index);
 
             // because of CCM/GCM, RTP headers must be unprotected only after the payload is unprotected and HMAC is verified
             // RFC6904

test/unit/net/SRTP/SrtpGcmReplayAuthUnitTest.cs

@@ -0,0 +1,159 @@
+//-----------------------------------------------------------------------------
+// Filename: SrtpGcmReplayAuthUnitTest.cs
+//
+// Description: Regression test for the AES-GCM SRTP "ROC poisoning" bug
+// (issue #1662). For AEAD/GCM profiles, authentication happens during the
+// payload decrypt. The original UnprotectRtp advanced the replay window /
+// highest-index state (S_l, which carries the ROC) BEFORE that decrypt, so a
+// single packet that failed AEAD authentication - or a stray/reordered packet -
+// permanently advanced S_l. Every subsequent packet then derived the wrong
+// AES-GCM IV (or was rejected by the replay window), producing an endless
+// "mac check in GCM failed" and never recovering.
+//
+// Per RFC 3711 section 3.3 the replay list / s_l / ROC MUST only be updated
+// AFTER the packet authenticates. This test injects a packet that fails AEAD
+// authentication mid-stream and asserts:
+//   1. UnprotectRtp returns ERROR_HMAC_CHECK_FAILED (it does not throw); and
+//   2. the next legitimate in-order packet still decrypts.
+//
+// On the original (buggy) implementation step 1 throws
+// InvalidCipherTextException and step 2 fails with ERROR_REPLAY_CHECK_FAILED
+// (the forged high sequence number poisoned S_l). After the fix both pass.
+//
+// Author(s):
+// Aaron Clauson
+//
+// History:
+// 02 Jun 2026  Aaron Clauson   Created.
+//
+// License:
+// BSD 3-Clause "New" or "Revised" License, see included LICENSE.md file.
+//-----------------------------------------------------------------------------
+
+using System;
+using SIPSorcery.Net.SharpSRTP.SRTP;
+using Xunit;
+
+namespace SIPSorcery.Net.UnitTests
+{
+    public class SrtpGcmReplayAuthUnitTest
+    {
+        [Fact]
+        public void FailedAeadAuth_DoesNotPoisonSubsequentPackets()
+        {
+            // SRTP_AEAD_AES_128_GCM - the profile OpenAI's realtime endpoint negotiates.
+            var profile = new SrtpProtectionProfileConfiguration(
+                SrtpCiphers.AEAD_AES_128_GCM,
+                cipherKeyLength:  128,
+                cipherSaltLength: 96,
+                maximumLifetime:  int.MaxValue,
+                auth:             SrtpAuth.NONE,
+                authKeyLength:    0,
+                authTagLength:    128);
+
+            // Deterministic key + salt so the test is reproducible.
+            var key  = new byte[16];
+            var salt = new byte[12];
+            for (int i = 0; i < key.Length;  i++) { key[i]  = (byte)(0x10 + i); }
+            for (int i = 0; i < salt.Length; i++) { salt[i] = (byte)(0x80 + i); }
+
+            var sender   = new SrtpContext(SrtpContextType.RTP, profile, key, salt);
+            var attacker = new SrtpContext(SrtpContextType.RTP, profile, key, salt); // produces the forged packet
+            var receiver = new SrtpContext(SrtpContextType.RTP, profile, key, salt);
+
+            const uint ssrc = 0x1234abcdu;
+
+            // ---- 1. Establish in-order receiver state (seqs 1..3) ----
+            for (ushort seq = 1; seq <= 3; seq++)
+            {
+                Assert.True(TryRoundTrip(sender, receiver, ssrc, seq) == 0, $"Round-trip failed for seq {seq}.");
+            }
+
+            // ---- 2. Inject a packet that fails AEAD authentication, with a higher sequence number ----
+            // A forged/corrupted packet whose 16-bit sequence number (100) is greater than the last
+            // accepted one. Pre-fix the receiver advanced S_l to 100 BEFORE the GCM auth ran, then threw.
+            // With the fix it must return ERROR_HMAC_CHECK_FAILED without throwing and without advancing
+            // any state.
+            byte[] forged = EncryptThenCorrupt(attacker, ssrc, seq: 100);
+            var scratch = new byte[forged.Length];
+            int forgedResult = Unprotect(receiver, forged, scratch, out _);
+            Assert.True(forgedResult == SrtpContext.ERROR_HMAC_CHECK_FAILED,
+                $"A packet that fails AEAD authentication should be rejected with ERROR_HMAC_CHECK_FAILED ({SrtpContext.ERROR_HMAC_CHECK_FAILED}) and must not throw; got {forgedResult}.");
+
+            // ---- 3. The next legitimate in-order packet must still decrypt ----
+            // Pre-fix this returned ERROR_REPLAY_CHECK_FAILED (-4) because the forged seq 100 had poisoned
+            // S_l (seq 4 then looked "too old"). With the fix S_l was never advanced, so seq 4 decrypts.
+            int afterResult = TryRoundTrip(sender, receiver, ssrc, seq: 4);
+            Assert.True(afterResult == 0,
+                $"A failed-authentication packet poisoned the SRTP ROC/replay state: the next legitimate packet returned {afterResult} instead of 0. Per RFC 3711 section 3.3 state must only advance after authentication.");
+        }
+
+        // ---- helpers ----
+
+        private static byte[] EncryptThenCorrupt(SrtpContext sender, uint ssrc, ushort seq)
+        {
+            var rtpPacket = MakeRtpPacket(ssrc, seq, payloadType: 96, payload: new byte[16]);
+            int extra = sender.CalculateRequiredSrtpPayloadLength(rtpPacket.Length) - rtpPacket.Length;
+            var encrypted = new byte[rtpPacket.Length + extra];
+
+            int encLen;
+            int encRc = Protect(sender, rtpPacket, encrypted, out encLen);
+            Assert.True(encRc == 0, $"Protect failed (rc={encRc}).");
+
+            var trimmed = new byte[encLen];
+            Buffer.BlockCopy(encrypted, 0, trimmed, 0, encLen);
+
+            // Flip a ciphertext byte (first payload byte, after the 12-byte RTP header) so the GCM
+            // authentication tag no longer matches the payload.
+            trimmed[12] ^= 0xFF;
+            return trimmed;
+        }
+
+        private static int TryRoundTrip(SrtpContext sender, SrtpContext receiver, uint ssrc, ushort seq)
+        {
+            var rtpPacket = MakeRtpPacket(ssrc, seq, payloadType: 96, payload: new byte[16]);
+            int extra = sender.CalculateRequiredSrtpPayloadLength(rtpPacket.Length) - rtpPacket.Length;
+            var encrypted = new byte[rtpPacket.Length + extra];
+
+            int encLen;
+            int encRc = Protect(sender, rtpPacket, encrypted, out encLen);
+            if (encRc != 0) { return encRc; }
+
+            var encryptedTrimmed = new byte[encLen];
+            Buffer.BlockCopy(encrypted, 0, encryptedTrimmed, 0, encLen);
+
+            var decrypted = new byte[encLen];
+            return Unprotect(receiver, encryptedTrimmed, decrypted, out _);
+        }
+
+        private static byte[] MakeRtpPacket(uint ssrc, ushort seq, byte payloadType, byte[] payload)
+        {
+            // Minimal valid RTP packet: 12-byte header + payload. V=2, P=0, X=0, CC=0, M=0, PT=payloadType.
+            var pkt = new byte[12 + payload.Length];
+            pkt[0]  = 0x80;
+            pkt[1]  = payloadType;
+            pkt[2]  = (byte)(seq >> 8);
+            pkt[3]  = (byte)(seq & 0xFF);
+            pkt[8]  = (byte)((ssrc >> 24) & 0xFF);
+            pkt[9]  = (byte)((ssrc >> 16) & 0xFF);
+            pkt[10] = (byte)((ssrc >>  8) & 0xFF);
+            pkt[11] = (byte)( ssrc        & 0xFF);
+            Buffer.BlockCopy(payload, 0, pkt, 12, payload.Length);
+            return pkt;
+        }
+
+#if NET8_0_OR_GREATER
+        private static int Protect(SrtpContext ctx, byte[] input, byte[] output, out int outLen)
+            => ctx.ProtectRtp(input.AsSpan(), output.AsSpan(), out outLen);
+
+        private static int Unprotect(SrtpContext ctx, byte[] input, byte[] output, out int outLen)
+            => ctx.UnprotectRtp(input.AsSpan(), output.AsSpan(), out outLen);
+#else
+        private static int Protect(SrtpContext ctx, byte[] input, byte[] output, out int outLen)
+            => ctx.ProtectRtp(new ArraySegment<byte>(input), output, out outLen);
+
+        private static int Unprotect(SrtpContext ctx, byte[] input, byte[] output, out int outLen)
+            => ctx.UnprotectRtp(new ArraySegment<byte>(input), output, out outLen);
+#endif
+    }
+}