Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[core] Drop undecrypted packet based on sequence number. #2654

Merged
merged 2 commits into from
Feb 8, 2023
Merged

[core] Drop undecrypted packet based on sequence number. #2654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16c0f50
[core] Drop undecrypted packet based on sequence number.
maxsharabayko Feb 8, 2023
3b3f525
Added a note on preventing a valid packet
maxsharabayko Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions srtcore/core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9850,7 +9850,7 @@ int srt::CUDT::handleSocketPacketReception(const vector<CUnit*>& incoming, bool&
}
}

int buffer_add_result = m_pRcvBuffer->insert(u);
const int buffer_add_result = m_pRcvBuffer->insert(u);
if (buffer_add_result < 0)
{
// The insert() result is -1 if at the position evaluated from this packet's
Expand Down Expand Up @@ -9878,12 +9878,19 @@ int srt::CUDT::handleSocketPacketReception(const vector<CUnit*>& incoming, bool&
adding_successful = false;
IF_HEAVY_LOGGING(exc_type = "UNDECRYPTED");

// Drop a packet from the receiver buffer.
// Dropping depends on the configuration mode. If message mode is enabled, we have to drop the whole message.
// Otherwise just drop the exact packet.
const int iDropCnt = (m_config.bMessageAPI)
? m_pRcvBuffer->dropMessage(SRT_SEQNO_NONE, SRT_SEQNO_NONE, u->m_Packet.getMsgSeq(m_bPeerRexmitFlag))
: m_pRcvBuffer->dropMessage(u->m_Packet.getSeqNo(), u->m_Packet.getSeqNo(), SRT_MSGNO_NONE);
// If TSBPD is disabled, then SRT either operates in buffer mode, of in message API without a restriction
// of a single message packet. In that case just dropping a packet is not enough.
// In message mode the whole message has to be dropped.
// However, when decryption fails the message number in the packet cannot be trusted.
// The packet has to be removed from the RCV buffer based on that pkt sequence number,
// and the sequence number itself must go into the RCV loss list.
// See issue ##2626.
SRT_ASSERT(m_bTsbPd);

// Drop the packet from the receiver buffer.
// The packet was added to the buffer based on the sequence number, therefore sequence number should be used to drop it from the buffer.
// A drawback is that it would prevent a valid packet with the same sequence number, if it happens to arrive later, to end up in the buffer.
const int iDropCnt = m_pRcvBuffer->dropMessage(u->m_Packet.getSeqNo(), u->m_Packet.getSeqNo(), SRT_MSGNO_NONE);
maxsharabayko marked this conversation as resolved.
Show resolved Hide resolved

LOGC(qrlog.Warn, log << CONID() << "Decryption failed. Seqno %" << u->m_Packet.getSeqNo()
<< ", msgno " << u->m_Packet.getMsgSeq(m_bPeerRexmitFlag) << ". Dropping " << iDropCnt << ".");
Expand Down
2 changes: 1 addition & 1 deletion srtcore/srt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ enum SRT_KM_STATE
SRT_KM_S_NOSECRET = 3, // Stream encrypted and no secret to decrypt Keying Material
SRT_KM_S_BADSECRET = 4 // Stream encrypted and wrong secret is used, cannot decrypt Keying Material
#ifdef ENABLE_AEAD_API_PREVIEW
,SRT_KM_S_BADCRYPTOMODE = 5 // Stream encrypted but wrong ccryptographic mode is used, cannot decrypt. Since v1.6.0.
,SRT_KM_S_BADCRYPTOMODE = 5 // Stream encrypted but wrong cryptographic mode is used, cannot decrypt. Since v1.6.0.
#endif
};

Expand Down