Skip to content

Commit

Permalink
Mitigate libsrtp wraparound with loss decryption failure
Browse files Browse the repository at this point in the history
Fixes #1437

### Details

- Read the issue, please.
- So solution is that `SeqManager` now includes a second constructor with `initialOutput` and we use it in all `XxxxConsumer` classes.
  • Loading branch information
ibc committed Aug 5, 2024
1 parent e5ed2c3 commit 759cce8
Show file tree
Hide file tree
Showing 5 changed files with 708 additions and 7 deletions.
2 changes: 2 additions & 0 deletions worker/include/RTC/SeqManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace RTC

public:
SeqManager() = default;
SeqManager(T initialOutput);

public:
void Sync(T input);
Expand All @@ -50,6 +51,7 @@ namespace RTC
private:
// Whether at least a sequence number has been inserted.
bool started{ false };
T initialOutput{ 0 };
T base{ 0 };
T maxOutput{ 0 };
T maxInput{ 0 };
Expand Down
2 changes: 1 addition & 1 deletion worker/include/RTC/SimpleConsumer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ namespace RTC
RTC::RtpStreamRecv* producerRtpStream{ nullptr };
bool keyFrameSupported{ false };
bool syncRequired{ false };
RTC::SeqManager<uint16_t> rtpSeqManager;
std::unique_ptr<RTC::SeqManager<uint16_t>> rtpSeqManager;
bool managingBitrate{ false };
std::unique_ptr<RTC::Codecs::EncodingContext> encodingContext;
};
Expand Down
16 changes: 16 additions & 0 deletions worker/src/RTC/SeqManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,17 @@ namespace RTC
return isSeqHigherThan(lhs, rhs);
}

template<typename T, uint8_t N>
SeqManager<T, N>::SeqManager(T initialOutput) : initialOutput(initialOutput)
{
MS_TRACE();
}

template<typename T, uint8_t N>
void SeqManager<T, N>::Sync(T input)
{
MS_TRACE();

// Update base.
this->base = (this->maxOutput - input) & MaxValue;

Expand All @@ -55,6 +63,8 @@ namespace RTC
template<typename T, uint8_t N>
void SeqManager<T, N>::Drop(T input)
{
MS_TRACE();

// Mark as dropped if 'input' is higher than anyone already processed.
if (SeqManager<T, N>::IsSeqHigherThan(input, this->maxInput))
{
Expand All @@ -71,6 +81,8 @@ namespace RTC
template<typename T, uint8_t N>
bool SeqManager<T, N>::Input(T input, T& output)
{
MS_TRACE();

auto base = this->base;

// No dropped inputs to consider.
Expand Down Expand Up @@ -140,6 +152,8 @@ namespace RTC
}
}

output = (output + this->initialOutput) & MaxValue;

return true;
}

Expand All @@ -162,6 +176,8 @@ namespace RTC
template<typename T, uint8_t N>
void SeqManager<T, N>::ClearDropped()
{
MS_TRACE();

// Cleanup dropped values.
if (this->dropped.empty())
{
Expand Down
17 changes: 13 additions & 4 deletions worker/src/RTC/SimpleConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "DepLibUV.hpp"
#include "Logger.hpp"
#include "MediaSoupErrors.hpp"
#include "Utils.hpp"
#include "RTC/Codecs/Tools.hpp"
#include "RTC/SimpleConsumer.hpp"

Expand Down Expand Up @@ -36,6 +37,14 @@ namespace RTC
// Create RtpStreamSend instance for sending a single stream to the remote.
CreateRtpStream();

// Let's chosee an initial output seq number between 1000 and 32768 to avoid
// libsrtp bug:
// https://github.com/versatica/mediasoup/issues/1437
const uint16_t initialOutputSeq =
Utils::Crypto::GetRandomUInt(1000u, std::numeric_limits<uint16_t>::max() / 2);

this->rtpSeqManager.reset(new RTC::SeqManager<uint16_t>(initialOutputSeq));

// Create the encoding context for Opus.
if (
mediaCodec->mimeType.type == RTC::RtpCodecMimeType::Type::AUDIO &&
Expand Down Expand Up @@ -340,7 +349,7 @@ namespace RTC
packet->GetSequenceNumber(),
packet->GetTimestamp());

this->rtpSeqManager.Drop(packet->GetSequenceNumber());
this->rtpSeqManager->Drop(packet->GetSequenceNumber());

#ifdef MS_RTC_LOGGER_RTP
packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::DROPPED_BY_CODEC);
Expand All @@ -363,7 +372,7 @@ namespace RTC
// Packets with only padding are not forwarded.
if (packet->GetPayloadLength() == 0)
{
this->rtpSeqManager.Drop(packet->GetSequenceNumber());
this->rtpSeqManager->Drop(packet->GetSequenceNumber());

#ifdef MS_RTC_LOGGER_RTP
packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD);
Expand All @@ -383,15 +392,15 @@ namespace RTC
MS_DEBUG_TAG(rtp, "sync key frame received");
}

this->rtpSeqManager.Sync(packet->GetSequenceNumber() - 1);
this->rtpSeqManager->Sync(packet->GetSequenceNumber() - 1);

this->syncRequired = false;
}

// Update RTP seq number and timestamp.
uint16_t seq;

this->rtpSeqManager.Input(packet->GetSequenceNumber(), seq);
this->rtpSeqManager->Input(packet->GetSequenceNumber(), seq);

// Save original packet fields.
auto origSsrc = packet->GetSsrc();
Expand Down
Loading

0 comments on commit 759cce8

Please sign in to comment.