admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

srtp: demonstrate wraparound with loss decryption failure

Browse Source 
by encryption a packet with sequence number 65535 followed
by a packet with sequence number 1. The second packet is encrypted
with a SRTP ROC of 1 as described in
  https://datatracker.ietf.org/doc/html/rfc3711#section-3.3.1

The packets are (received and) decrypted in a different order,
the packet with sequence number 1 (and ROC=1) is decrypted first.
Since the ROC is maintained locally the decrypting session assumes
it to be 0.

Why is that a problem? The RFC recommends estimating the ROC with +-1 which, as demonstrated by the test, libSRTP does not.
But this is a rare problem that requires a random in a high range combined with packet loss/reordering which turns into no-a-problem if you choose carefully as done by packet_sequencer.cc which restricts the initial sequence number in the range 0..32767 which means you do not run into this issue in production.

See also Q6 in libsrtp's historical documentation at
  https://srtp.sourceforge.net/historical/faq.html

BUG=webrtc:353565743

Change-Id: I9bd72b198c946937aeb25c229005a0c682447f53
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/358360
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#42798}
This commit is contained in:
Philipp Hancke 2024-08-17 07:42:21 -07:00 committed by WebRTC LUCI CQ
parent adb224b3e9
commit 13b327b05f
2 changed files with 60 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
modules/rtp_rtcp/source/packet_sequencer.cc
View File

@ -38,9 +38,10 @@ PacketSequencer::PacketSequencer(uint32_t media_ssrc,
last_packet_marker_bit_(false) {
Random random(clock_->TimeInMicroseconds());
// Random start, 16 bits. Upper half of range is avoided in order to prevent
// wraparound issues during startup. Sequence number 0 is avoided for
// historical reasons, presumably to avoid debugability or test usage
// conflicts.
// SRTP wraparound issues during startup. See this unit test for details:
// SrtpSessionTest.ProtectUnprotectWrapAroundRocMismatch
// Sequence number 0 is avoided for historical reasons, presumably to avoid
// debugability or test usage conflicts.
constexpr uint16_t kMaxInitRtpSeqNumber = 0x7fff; // 2^15 - 1.
media_sequence_number_ = random.Rand(1, kMaxInitRtpSeqNumber);
rtx_sequence_number_ = random.Rand(1, kMaxInitRtpSeqNumber);

56
pc/srtp_session_unittest.cc
View File

@ -284,4 +284,60 @@ TEST_F(SrtpSessionTest, RemoveSsrc) {
EXPECT_TRUE(s2_.RemoveSsrcFromSession(1));
}
TEST_F(SrtpSessionTest, ProtectUnprotectWrapAroundRocMismatch) {
// This unit tests demonstrates why you should be careful when
// choosing the initial RTP sequence number as there can be decryption
// failures when it wraps around with packet loss. Pick your starting
// sequence number in the lower half of the range for robustness reasons,
// see packet_sequencer.cc for the code doing so.
EXPECT_TRUE(s1_.SetSend(kSrtpAes128CmSha1_80, kTestKey1, kTestKeyLen,
kEncryptedHeaderExtensionIds));
EXPECT_TRUE(s2_.SetRecv(kSrtpAes128CmSha1_80, kTestKey1, kTestKeyLen,
kEncryptedHeaderExtensionIds));
// Buffers include enough room for the 10 byte SRTP auth tag so we can
// encrypt in place.
unsigned char kFrame1[] = {
// clang-format off
// PT=0, SN=65535, TS=0, SSRC=1
0x80, 0x00, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
0xBE, 0xEF, // data bytes
// Space for the SRTP auth tag
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
// clang-format on
};
unsigned char kFrame2[] = {
// clang-format off
// PT=0, SN=1, TS=0, SSRC=1
0x80, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
0xBE, 0xEF, // data bytes
// Space for the SRTP auth tag
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
// clang-format on
};
int out_len;
// Encrypt the frames in-order. There is a sequence number rollover from
// 65535 to 1 (skipping 0) and the second packet gets encrypted with a
// roll-over counter (ROC) of 1. See
// https://datatracker.ietf.org/doc/html/rfc3711#section-3.3.1
EXPECT_TRUE(
s1_.ProtectRtp(kFrame1, sizeof(kFrame1) - 10, sizeof(kFrame1), &out_len));
EXPECT_EQ(out_len, 24);
EXPECT_TRUE(
s1_.ProtectRtp(kFrame2, sizeof(kFrame2) - 10, sizeof(kFrame2), &out_len));
EXPECT_EQ(out_len, 24);
// If we decrypt frame 2 first it will have a ROC of 1 but the receiver
// does not know this is a rollover so will attempt with a ROC of 0.
// Note: If libsrtp is modified to attempt to decrypt with ROC=1 for this
// case, this test will fail and needs to be modified accordingly to unblock
// the roll. See https://issues.webrtc.org/353565743 for details.
EXPECT_FALSE(s2_.UnprotectRtp(kFrame2, sizeof(kFrame2), &out_len));
// Decrypt frame 1.
EXPECT_TRUE(s2_.UnprotectRtp(kFrame1, sizeof(kFrame1), &out_len));
// Now decrypt frame 2 again. A rollover is detected which increases
// the ROC to 1 so this succeeds.
EXPECT_TRUE(s2_.UnprotectRtp(kFrame2, sizeof(kFrame2), &out_len));
}
} // namespace rtc