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

Skip calling rtcp callback on packets containing invalid rtcp message

Browse Source 
Low-Coverage-Reason: added code handles invalid rtcp packet scenarios which are covered by rtcp_receiver_fuzzer
Bug: webrtc:5260
Change-Id: Ia6ba0b736f5732fa6516edb2a5ed1b96079eb4ef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296580
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39503}
This commit is contained in:
Danil Chapovalov 2023-03-08 12:34:34 +01:00 committed by WebRTC LUCI CQ
parent f3e90cd03a
commit 6a0997de01
3 changed files with 110 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

176
modules/rtp_rtcp/source/rtcp_receiver.cc
View File

@ -416,59 +416,56 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView<const uint8_t> packet,
// For each remote SSRC we store if we've received a sender report or a DLRR
// block.
flat_map<uint32_t, RtcpReceivedBlock> received_blocks;
for (const uint8_t* next_block = packet.begin(); next_block != packet.end();
bool valid = true;
for (const uint8_t* next_block = packet.begin();
valid && next_block != packet.end();
next_block = rtcp_block.NextPacket()) {
ptrdiff_t remaining_blocks_size = packet.end() - next_block;
RTC_DCHECK_GT(remaining_blocks_size, 0);
if (!rtcp_block.Parse(next_block, remaining_blocks_size)) {
if (next_block == packet.begin()) {
// Failed to parse 1st header, nothing was extracted from this packet.
RTC_LOG(LS_WARNING) << "Incoming invalid RTCP packet";
return false;
}
++num_skipped_packets_;
valid = false;
break;
}
switch (rtcp_block.type()) {
case rtcp::SenderReport::kPacketType:
HandleSenderReport(rtcp_block, packet_information);
valid = HandleSenderReport(rtcp_block, packet_information);
received_blocks[packet_information->remote_ssrc].sender_report = true;
break;
case rtcp::ReceiverReport::kPacketType:
HandleReceiverReport(rtcp_block, packet_information);
valid = HandleReceiverReport(rtcp_block, packet_information);
break;
case rtcp::Sdes::kPacketType:
HandleSdes(rtcp_block, packet_information);
valid = HandleSdes(rtcp_block, packet_information);
break;
case rtcp::ExtendedReports::kPacketType: {
bool contains_dlrr = false;
uint32_t ssrc = 0;
HandleXr(rtcp_block, packet_information, contains_dlrr, ssrc);
valid = HandleXr(rtcp_block, packet_information, contains_dlrr, ssrc);
if (contains_dlrr) {
received_blocks[ssrc].dlrr = true;
}
break;
}
case rtcp::Bye::kPacketType:
HandleBye(rtcp_block);
valid = HandleBye(rtcp_block);
break;
case rtcp::App::kPacketType:
HandleApp(rtcp_block, packet_information);
valid = HandleApp(rtcp_block, packet_information);
break;
case rtcp::Rtpfb::kPacketType:
switch (rtcp_block.fmt()) {
case rtcp::Nack::kFeedbackMessageType:
HandleNack(rtcp_block, packet_information);
valid = HandleNack(rtcp_block, packet_information);
break;
case rtcp::Tmmbr::kFeedbackMessageType:
HandleTmmbr(rtcp_block, packet_information);
valid = HandleTmmbr(rtcp_block, packet_information);
break;
case rtcp::Tmmbn::kFeedbackMessageType:
HandleTmmbn(rtcp_block, packet_information);
valid = HandleTmmbn(rtcp_block, packet_information);
break;
case rtcp::RapidResyncRequest::kFeedbackMessageType:
HandleSrReq(rtcp_block, packet_information);
valid = HandleSrReq(rtcp_block, packet_information);
break;
case rtcp::TransportFeedback::kFeedbackMessageType:
HandleTransportFeedback(rtcp_block, packet_information);
@ -481,10 +478,10 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView<const uint8_t> packet,
case rtcp::Psfb::kPacketType:
switch (rtcp_block.fmt()) {
case rtcp::Pli::kFeedbackMessageType:
HandlePli(rtcp_block, packet_information);
valid = HandlePli(rtcp_block, packet_information);
break;
case rtcp::Fir::kFeedbackMessageType:
HandleFir(rtcp_block, packet_information);
valid = HandleFir(rtcp_block, packet_information);
break;
case rtcp::Psfb::kAfbMessageType:
HandlePsfbApp(rtcp_block, packet_information);
@ -500,6 +497,23 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView<const uint8_t> packet,
}
}
if (num_skipped_packets_ > 0) {
const int64_t now_ms = clock_->TimeInMilliseconds();
if (now_ms - last_skipped_packets_warning_ms_ >= kMaxWarningLogIntervalMs) {
last_skipped_packets_warning_ms_ = now_ms;
RTC_LOG(LS_WARNING)
<< num_skipped_packets_
<< " RTCP blocks were skipped due to being malformed or of "
"unrecognized/unsupported type, during the past "
<< (kMaxWarningLogIntervalMs / 1000) << " second period.";
}
}
if (!valid) {
++num_skipped_packets_;
return false;
}
for (const auto& rb : received_blocks) {
if (rb.second.sender_report && !rb.second.dlrr) {
auto rtt_stats = non_sender_rtts_.find(rb.first);
@ -514,27 +528,14 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView<const uint8_t> packet,
local_media_ssrc(), packet_type_counter_);
}
if (num_skipped_packets_ > 0) {
const int64_t now_ms = clock_->TimeInMilliseconds();
if (now_ms - last_skipped_packets_warning_ms_ >= kMaxWarningLogIntervalMs) {
last_skipped_packets_warning_ms_ = now_ms;
RTC_LOG(LS_WARNING)
<< num_skipped_packets_
<< " RTCP blocks were skipped due to being malformed or of "
"unrecognized/unsupported type, during the past "
<< (kMaxWarningLogIntervalMs / 1000) << " second period.";
}
}
return true;
}
void RTCPReceiver::HandleSenderReport(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleSenderReport(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::SenderReport sender_report;
if (!sender_report.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
const uint32_t remote_ssrc = sender_report.sender_ssrc();
@ -560,16 +561,18 @@ void RTCPReceiver::HandleSenderReport(const CommonHeader& rtcp_block,
packet_information->packet_type_flags |= kRtcpRr;
}
for (const rtcp::ReportBlock& report_block : sender_report.report_blocks())
for (const rtcp::ReportBlock& report_block : sender_report.report_blocks()) {
HandleReportBlock(report_block, packet_information, remote_ssrc);
}
return true;
}
void RTCPReceiver::HandleReceiverReport(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleReceiverReport(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::ReceiverReport receiver_report;
if (!receiver_report.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
const uint32_t remote_ssrc = receiver_report.sender_ssrc();
@ -580,8 +583,11 @@ void RTCPReceiver::HandleReceiverReport(const CommonHeader& rtcp_block,
packet_information->packet_type_flags |= kRtcpRr;
for (const ReportBlock& report_block : receiver_report.report_blocks())
for (const ReportBlock& report_block : receiver_report.report_blocks()) {
HandleReportBlock(report_block, packet_information, remote_ssrc);
}
return true;
}
void RTCPReceiver::HandleReportBlock(const ReportBlock& report_block,
@ -740,12 +746,11 @@ std::vector<rtcp::TmmbItem> RTCPReceiver::BoundingSet(bool* tmmbr_owner) {
return tmmbr_info->tmmbn;
}
void RTCPReceiver::HandleSdes(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleSdes(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::Sdes sdes;
if (!sdes.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
for (const rtcp::Sdes::Chunk& chunk : sdes.chunks()) {
@ -753,18 +758,19 @@ void RTCPReceiver::HandleSdes(const CommonHeader& rtcp_block,
cname_callback_->OnCname(chunk.ssrc, chunk.cname);
}
packet_information->packet_type_flags |= kRtcpSdes;
return true;
}
void RTCPReceiver::HandleNack(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleNack(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::Nack nack;
if (!nack.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
if (receiver_only_ || local_media_ssrc() != nack.media_ssrc()) // Not to us.
return;
return true;
packet_information->nack_sequence_numbers.insert(
packet_information->nack_sequence_numbers.end(),
@ -778,29 +784,34 @@ void RTCPReceiver::HandleNack(const CommonHeader& rtcp_block,
packet_type_counter_.nack_requests = nack_stats_.requests();
packet_type_counter_.unique_nack_requests = nack_stats_.unique_requests();
}
return true;
}
void RTCPReceiver::HandleApp(const rtcp::CommonHeader& rtcp_block,
bool RTCPReceiver::HandleApp(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::App app;
if (app.Parse(rtcp_block)) {
if (app.name() == rtcp::RemoteEstimate::kName &&
app.sub_type() == rtcp::RemoteEstimate::kSubType) {
rtcp::RemoteEstimate estimate(std::move(app));
if (estimate.ParseData()) {
packet_information->network_state_estimate = estimate.estimate();
return;
}
}
if (!app.Parse(rtcp_block)) {
return false;
}
++num_skipped_packets_;
if (app.name() == rtcp::RemoteEstimate::kName &&
app.sub_type() == rtcp::RemoteEstimate::kSubType) {
rtcp::RemoteEstimate estimate(std::move(app));
if (estimate.ParseData()) {
packet_information->network_state_estimate = estimate.estimate();
}
// RemoteEstimate is not a standard RTCP message. Failing to parse it
// doesn't indicates RTCP packet is invalid. It may indicate sender happens
// to use the same id for a different message. Thus don't return false.
}
return true;
}
void RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) {
bool RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) {
rtcp::Bye bye;
if (!bye.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
// Clear our lists.
@ -820,16 +831,16 @@ void RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) {
received_rrtrs_ssrc_it_.erase(it);
}
xr_rr_rtt_ms_ = 0;
return true;
}
void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleXr(const CommonHeader& rtcp_block,
PacketInformation* packet_information,
bool& contains_dlrr,
uint32_t& ssrc) {
rtcp::ExtendedReports xr;
if (!xr.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
ssrc = xr.sender_ssrc();
contains_dlrr = !xr.dlrr().sub_blocks().empty();
@ -844,6 +855,7 @@ void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block,
HandleXrTargetBitrate(xr.sender_ssrc(), *xr.target_bitrate(),
packet_information);
}
return true;
}
void RTCPReceiver::HandleXrReceiveReferenceTime(uint32_t sender_ssrc,
@ -922,12 +934,11 @@ void RTCPReceiver::HandleXrTargetBitrate(
packet_information->target_bitrate_allocation.emplace(bitrate_allocation);
}
void RTCPReceiver::HandlePli(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandlePli(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::Pli pli;
if (!pli.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
if (local_media_ssrc() == pli.media_ssrc()) {
@ -935,14 +946,14 @@ void RTCPReceiver::HandlePli(const CommonHeader& rtcp_block,
// Received a signal that we need to send a new key frame.
packet_information->packet_type_flags |= kRtcpPli;
}
return true;
}
void RTCPReceiver::HandleTmmbr(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleTmmbr(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::Tmmbr tmmbr;
if (!tmmbr.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
uint32_t sender_ssrc = tmmbr.sender_ssrc();
@ -967,14 +978,14 @@ void RTCPReceiver::HandleTmmbr(const CommonHeader& rtcp_block,
packet_information->packet_type_flags |= kRtcpTmmbr;
break;
}
return true;
}
void RTCPReceiver::HandleTmmbn(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleTmmbn(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::Tmmbn tmmbn;
if (!tmmbn.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
TmmbrInformation* tmmbr_info = FindOrCreateTmmbrInfo(tmmbn.sender_ssrc());
@ -982,17 +993,18 @@ void RTCPReceiver::HandleTmmbn(const CommonHeader& rtcp_block,
packet_information->packet_type_flags |= kRtcpTmmbn;
tmmbr_info->tmmbn = tmmbn.items();
return true;
}
void RTCPReceiver::HandleSrReq(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleSrReq(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::RapidResyncRequest sr_req;
if (!sr_req.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
packet_information->packet_type_flags |= kRtcpSrReq;
return true;
}
void RTCPReceiver::HandlePsfbApp(const CommonHeader& rtcp_block,
@ -1017,20 +1029,20 @@ void RTCPReceiver::HandlePsfbApp(const CommonHeader& rtcp_block,
}
RTC_LOG(LS_WARNING) << "Unknown PSFB-APP packet.";
++num_skipped_packets_;
// Application layer feedback message doesn't have a standard format.
// Failing to parse one of known messages doesn't indicate an invalid RTCP.
}
void RTCPReceiver::HandleFir(const CommonHeader& rtcp_block,
bool RTCPReceiver::HandleFir(const CommonHeader& rtcp_block,
PacketInformation* packet_information) {
rtcp::Fir fir;
if (!fir.Parse(rtcp_block)) {
++num_skipped_packets_;
return;
return false;
}
if (fir.requests().empty())
return;
return true;
const int64_t now_ms = clock_->TimeInMilliseconds();
for (const rtcp::Fir::Request& fir_request : fir.requests()) {
@ -1059,6 +1071,7 @@ void RTCPReceiver::HandleFir(const CommonHeader& rtcp_block,
// Received signal that we need to send a new key frame.
packet_information->packet_type_flags |= kRtcpFir;
}
return true;
}
void RTCPReceiver::HandleTransportFeedback(
@ -1068,6 +1081,9 @@ void RTCPReceiver::HandleTransportFeedback(
new rtcp::TransportFeedback());
if (!transport_feedback->Parse(rtcp_block)) {
++num_skipped_packets_;
// Application layer feedback message doesn't have a standard format.
// Failing to parse it as transport feedback messages doesn't indicate an
// invalid RTCP.
return;
}

24
modules/rtp_rtcp/source/rtcp_receiver.h
View File

@ -277,11 +277,11 @@ class RTCPReceiver final {
TmmbrInformation* GetTmmbrInformation(uint32_t remote_ssrc)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleSenderReport(const rtcp::CommonHeader& rtcp_block,
bool HandleSenderReport(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleReceiverReport(const rtcp::CommonHeader& rtcp_block,
bool HandleReceiverReport(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
@ -290,11 +290,11 @@ class RTCPReceiver final {
uint32_t remote_ssrc)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleSdes(const rtcp::CommonHeader& rtcp_block,
bool HandleSdes(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleXr(const rtcp::CommonHeader& rtcp_block,
bool HandleXr(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information,
bool& contains_dlrr,
uint32_t& ssrc)
@ -312,18 +312,18 @@ class RTCPReceiver final {
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleNack(const rtcp::CommonHeader& rtcp_block,
bool HandleNack(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleApp(const rtcp::CommonHeader& rtcp_block,
bool HandleApp(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleBye(const rtcp::CommonHeader& rtcp_block)
bool HandleBye(const rtcp::CommonHeader& rtcp_block)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandlePli(const rtcp::CommonHeader& rtcp_block,
bool HandlePli(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
@ -331,19 +331,19 @@ class RTCPReceiver final {
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleTmmbr(const rtcp::CommonHeader& rtcp_block,
bool HandleTmmbr(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleTmmbn(const rtcp::CommonHeader& rtcp_block,
bool HandleTmmbn(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleSrReq(const rtcp::CommonHeader& rtcp_block,
bool HandleSrReq(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);
void HandleFir(const rtcp::CommonHeader& rtcp_block,
bool HandleFir(const rtcp::CommonHeader& rtcp_block,
PacketInformation* packet_information)
RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_);

5
modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
View File

@ -191,9 +191,8 @@ TEST(RtcpReceiverTest, InvalidFeedbackPacketIsIgnored) {
// Too short feedback packet.
const uint8_t bad_packet[] = {0x81, rtcp::Rtpfb::kPacketType, 0, 0};
// TODO(danilchap): Add expectation RtcpPacketTypesCounterUpdated
// is not called once parser would be adjusted to avoid that callback on
// semi-valid packets.
EXPECT_CALL(mocks.packet_type_counter_observer, RtcpPacketTypesCounterUpdated)
.Times(0);
receiver.IncomingPacket(bad_packet);
}