Skip to content

Commit

Permalink
[core] AES-GCM payload length check (#2591).
Browse files Browse the repository at this point in the history 
The negotiated MTU size value is used to determine the maximum allowed payload size.
Fixed CSndBuffer::addBufferFromFile.
  • Loading branch information
@maxsharabayko
maxsharabayko authored Jan 2, 2023
1 parent 45232ad commit 5889a2c
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 40 deletions.
48 changes: 30 additions & 18 deletions srtcore/buffer_snd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,12 @@ CSndBuffer::~CSndBuffer()

void CSndBuffer::addBuffer(const char* data, int len, SRT_MSGCTRL& w_mctrl)
{
int32_t& w_msgno = w_mctrl.msgno;
int32_t& w_seqno = w_mctrl.pktseq;
int64_t& w_srctime = w_mctrl.srctime;
const int& ttl = w_mctrl.msgttl;
const int iPktLen = m_iBlockLen - m_iAuthTagSize; // Payload length per packet.
int iNumBlocks = len / iPktLen;
if ((len % m_iBlockLen) != 0)
++iNumBlocks;
int32_t& w_msgno = w_mctrl.msgno;
int32_t& w_seqno = w_mctrl.pktseq;
int64_t& w_srctime = w_mctrl.srctime;
const int& ttl = w_mctrl.msgttl;
const int iPktLen = getMaxPacketLen();
const int iNumBlocks = countNumPacketsRequired(len, iPktLen);

HLOGC(bslog.Debug,
log << "addBuffer: needs=" << iNumBlocks << " buffers for " << len << " bytes. Taken=" << m_iCount << "/" << m_iSize);
Expand Down Expand Up @@ -239,20 +237,18 @@ void CSndBuffer::addBuffer(const char* data, int len, SRT_MSGCTRL& w_mctrl)

int CSndBuffer::addBufferFromFile(fstream& ifs, int len)
{
const int iPktLen = m_iBlockLen - m_iAuthTagSize; // Payload length per packet.
int iNumBlocks = len / iPktLen;
if ((len % m_iBlockLen) != 0)
++iNumBlocks;
const int iPktLen = getMaxPacketLen();
const int iNumBlocks = countNumPacketsRequired(len, iPktLen);

HLOGC(bslog.Debug,
log << "addBufferFromFile: size=" << m_iCount << " reserved=" << m_iSize << " needs=" << iPktLen
<< " buffers for " << len << " bytes");

// dynamically increase sender buffer
while (iPktLen + m_iCount >= m_iSize)
while (iNumBlocks + m_iCount >= m_iSize)
{
HLOGC(bslog.Debug,
log << "addBufferFromFile: ... still lacking " << (iPktLen + m_iCount - m_iSize) << " buffers...");
log << "addBufferFromFile: ... still lacking " << (iNumBlocks + m_iCount - m_iSize) << " buffers...");
increase();
}

Expand All @@ -262,7 +258,7 @@ int CSndBuffer::addBufferFromFile(fstream& ifs, int len)

Block* s = m_pLastBlock;
int total = 0;
for (int i = 0; i < iPktLen; ++i)
for (int i = 0; i < iNumBlocks; ++i)
{
if (ifs.bad() || ifs.fail() || ifs.eof())
break;
Expand All @@ -282,7 +278,7 @@ int CSndBuffer::addBufferFromFile(fstream& ifs, int len)
s->m_iMsgNoBitset = m_iNextMsgNo | MSGNO_PACKET_INORDER::mask;
if (i == 0)
s->m_iMsgNoBitset |= PacketBoundaryBits(PB_FIRST);
if (i == iPktLen - 1)
if (i == iNumBlocks - 1)
s->m_iMsgNoBitset |= PacketBoundaryBits(PB_LAST);
// NOTE: PB_FIRST | PB_LAST == PB_SOLO.
// none of PB_FIRST & PB_LAST == PB_SUBSEQUENT.
Expand All @@ -296,7 +292,7 @@ int CSndBuffer::addBufferFromFile(fstream& ifs, int len)
m_pLastBlock = s;

enterCS(m_BufLock);
m_iCount += iPktLen;
m_iCount += iNumBlocks;
m_iBytesCount += total;

leaveCS(m_BufLock);
Expand Down Expand Up @@ -556,6 +552,22 @@ int CSndBuffer::getCurrBufSize() const
return m_iCount;
}

int CSndBuffer::getMaxPacketLen() const
{
return m_iBlockLen - m_iAuthTagSize;
}

int CSndBuffer::countNumPacketsRequired(int iPldLen) const
{
const int iPktLen = getMaxPacketLen();
return countNumPacketsRequired(iPldLen, iPktLen);
}

int CSndBuffer::countNumPacketsRequired(int iPldLen, int iPktLen) const
{
return (iPldLen + iPktLen - 1) / iPktLen;
}

namespace {
int round_val(double val)
{
Expand Down Expand Up @@ -590,7 +602,7 @@ void CSndBuffer::updAvgBufSize(const steady_clock::time_point& now)
m_mavg.update(now, pkts, bytes, timespan_ms);
}

int CSndBuffer::getCurrBufSize(int& w_bytes, int& w_timespan)
int CSndBuffer::getCurrBufSize(int& w_bytes, int& w_timespan) const
{
w_bytes = m_iBytesCount;
/*
Expand Down
17 changes: 16 additions & 1 deletion srtcore/buffer_snd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,22 @@ class CSndBuffer

void updAvgBufSize(const time_point& time);
int getAvgBufSize(int& bytes, int& timespan);
int getCurrBufSize(int& bytes, int& timespan);
int getCurrBufSize(int& bytes, int& timespan) const;


/// Het maximum payload length per packet.
int getMaxPacketLen() const;

/// @brief Count the number of required packets to store the payload (message).
/// @param iPldLen the length of the payload to check.
/// @return the number of required data packets.
int countNumPacketsRequired(int iPldLen) const;

/// @brief Count the number of required packets to store the payload (message).
/// @param iPldLen the length of the payload to check.
/// @param iMaxPktLen the maximum payload length of the packet (the value returned from getMaxPacketLen()).
/// @return the number of required data packets.
int countNumPacketsRequired(int iPldLen, int iMaxPktLen) const;

/// @brief Get the buffering delay of the oldest message in the buffer.
/// @return the delay value.
Expand Down
33 changes: 18 additions & 15 deletions srtcore/core.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -871,13 +871,10 @@ string srt::CUDT::getstreamid(SRTSOCKET u)

// XXX REFACTOR: Make common code for CUDT constructor and clearData,
// possibly using CUDT::construct.
// Initial sequence number, loss, acknowledgement, etc.
void srt::CUDT::clearData()
{
// Initial sequence number, loss, acknowledgement, etc.
int udpsize = m_config.iMSS - CPacket::UDP_HDR_SIZE;

m_iMaxSRTPayloadSize = udpsize - CPacket::HDR_SIZE;

m_iMaxSRTPayloadSize = m_config.iMSS - CPacket::UDP_HDR_SIZE - CPacket::HDR_SIZE;
HLOGC(cnlog.Debug, log << CONID() << "clearData: PAYLOAD SIZE: " << m_iMaxSRTPayloadSize);

m_iEXPCount = 1;
Expand Down Expand Up @@ -6442,15 +6439,20 @@ int srt::CUDT::sendmsg2(const char *data, int len, SRT_MSGCTRL& w_mctrl)
// to modify m_pSndBuffer and m_pSndLossList
const int iPktsTLDropped SRT_ATR_UNUSED = sndDropTooLate();

int minlen = 1; // Minimum sender buffer space required for STREAM API
if (m_config.bMessageAPI)
// For MESSAGE API the minimum outgoing buffer space required is
// the size that can carry over the whole message as passed here.
// Otherwise it is allowed to send less bytes.
const int iNumPktsRequired = m_config.bMessageAPI ? m_pSndBuffer->countNumPacketsRequired(len) : 1;

if (m_bTsbPd && iNumPktsRequired > 1)
{
// For MESSAGE API the minimum outgoing buffer space required is
// the size that can carry over the whole message as passed here.
minlen = (len + m_iMaxSRTPayloadSize - 1) / m_iMaxSRTPayloadSize;
LOGC(aslog.Error,
log << CONID() << "Message length (" << len << ") can't fit into a single data packet ("
<< m_pSndBuffer->getMaxPacketLen() << " bytes max).");
throw CUDTException(MJ_NOTSUP, MN_XSIZE, 0);
}

if (sndBuffersLeft() < minlen)
if (sndBuffersLeft() < iNumPktsRequired)
{
//>>We should not get here if SRT_ENABLE_TLPKTDROP
// XXX Check if this needs to be removed, or put to an 'else' condition for m_bTLPktDrop.
Expand All @@ -6463,15 +6465,15 @@ int srt::CUDT::sendmsg2(const char *data, int len, SRT_MSGCTRL& w_mctrl)

if (m_config.iSndTimeOut < 0)
{
while (stillConnected() && sndBuffersLeft() < minlen && m_bPeerHealth)
while (stillConnected() && sndBuffersLeft() < iNumPktsRequired && m_bPeerHealth)
m_SendBlockCond.wait(sendblock_lock);
}
else
{
const steady_clock::time_point exptime =
steady_clock::now() + milliseconds_from(m_config.iSndTimeOut);
THREAD_PAUSED();
while (stillConnected() && sndBuffersLeft() < minlen && m_bPeerHealth)
while (stillConnected() && sndBuffersLeft() < iNumPktsRequired && m_bPeerHealth)
{
if (!m_SendBlockCond.wait_until(sendblock_lock, exptime))
break;
Expand All @@ -6497,7 +6499,7 @@ int srt::CUDT::sendmsg2(const char *data, int len, SRT_MSGCTRL& w_mctrl)
* we test twice if this code is outside the else section.
* This fix move it in the else (blocking-mode) section
*/
if (sndBuffersLeft() < minlen)
if (sndBuffersLeft() < iNumPktsRequired)
{
if (m_config.iSndTimeOut >= 0)
throw CUDTException(MJ_AGAIN, MN_XMTIMEOUT, 0);
Expand Down Expand Up @@ -9876,7 +9878,8 @@ int srt::CUDT::handleSocketPacketReception(const vector<CUnit*>& incoming, bool&
else
m_pRcvBuffer->dropMessage(u->m_Packet.getSeqNo(), u->m_Packet.getSeqNo(), SRT_MSGNO_NONE);

LOGC(qrlog.Error, log << CONID() << "AEAD decryption failed, breaking the connection.");
LOGC(qrlog.Error, log << CONID() << "AEAD decryption failed. Pkt seqno %" << u->m_Packet.getSeqNo()
<< ", msgno " << u->m_Packet.getMsgSeq(m_bPeerRexmitFlag) << ". Breaking the connection.");
m_bBroken = true;
m_iBrokenCounter = 0;
}
Expand Down
16 changes: 13 additions & 3 deletions srtcore/socketconfig.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ struct CSrtConfigSetter<SRTO_MSS>
{
static void set(CSrtConfig& co, const void* optval, int optlen)
{
int ival = cast_optval<int>(optval, optlen);
const int ival = cast_optval<int>(optval, optlen);
if (ival < int(CPacket::UDP_HDR_SIZE + CHandShake::m_iContentSize))
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);

Expand Down Expand Up @@ -611,7 +611,7 @@ struct CSrtConfigSetter<SRTO_PAYLOADSIZE>

if (val > SRT_LIVE_MAX_PLSIZE)
{
LOGC(aclog.Error, log << "SRTO_PAYLOADSIZE: value exceeds SRT_LIVE_MAX_PLSIZE, maximum payload per MTU.");
LOGC(aclog.Error, log << "SRTO_PAYLOADSIZE: value exceeds " << SRT_LIVE_MAX_PLSIZE << ", maximum payload per MTU.");
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
}

Expand All @@ -632,12 +632,22 @@ struct CSrtConfigSetter<SRTO_PAYLOADSIZE>
if (size_t(val) > efc_max_payload_size)
{
LOGC(aclog.Error,
log << "SRTO_PAYLOADSIZE: value exceeds SRT_LIVE_MAX_PLSIZE decreased by " << fc.extra_size
log << "SRTO_PAYLOADSIZE: value exceeds " << SRT_LIVE_MAX_PLSIZE << " bytes decreased by " << fc.extra_size
<< " required for packet filter header");
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
}
}

// Not checking AUTO to allow defaul 1456 bytes.
if ((co.iCryptoMode == CSrtConfig::CIPHER_MODE_AES_GCM)
&& (val > (SRT_LIVE_MAX_PLSIZE - HAICRYPT_AUTHTAG_MAX)))
{
LOGC(aclog.Error,
log << "SRTO_PAYLOADSIZE: value exceeds " << SRT_LIVE_MAX_PLSIZE << " bytes decreased by " << HAICRYPT_AUTHTAG_MAX
<< " required for AES-GCM.");
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
}

co.zExpPayloadSize = val;
}
};
Expand Down
6 changes: 3 additions & 3 deletions srtcore/strerror_defs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ const char* strerror_msgs_notsup [] = {
"Operation not supported: Socket is not in listening state", // MN_NOLISTEN = 6
"Operation not supported: Listen/accept is not supported in rendezvous connection setup", // MN_ISRENDEZVOUS = 7
"Operation not supported: Cannot call connect on UNBOUND socket in rendezvous connection setup", // MN_ISRENDUNBOUND = 8
"Operation not supported: Incorrect use of Message API (sendmsg/recvmsg).", // MN_INVALMSGAPI = 9
"Operation not supported: Incorrect use of Buffer API (send/recv) or File API (sendfile/recvfile).", // MN_INVALBUFFERAPI = 10
"Operation not supported: Incorrect use of Message API (sendmsg/recvmsg)", // MN_INVALMSGAPI = 9
"Operation not supported: Incorrect use of Buffer API (send/recv) or File API (sendfile/recvfile)", // MN_INVALBUFFERAPI = 10
"Operation not supported: Another socket is already listening on the same port", // MN_BUSY = 11
"Operation not supported: Message is too large to send (it must be less than the SRT send buffer size)", // MN_XSIZE = 12
"Operation not supported: Message is too large to send", // MN_XSIZE = 12
"Operation not supported: Invalid epoll ID", // MN_EIDINVAL = 13
"Operation not supported: All sockets removed from epoll, waiting would deadlock", // MN_EEMPTY = 14
"Operation not supported: Another socket is bound to that port and is not reusable for requested settings", // MN_BUSYPORT = 15
Expand Down

0 comments on commit 5889a2c

Please sign in to comment.