[reSIProcate-users] how big is a valid SIP body (SDP)? (Re: Issues building WebRTC branch)
On 21/03/13 21:05, Nathan Stratton wrote:
> I wanted to rule out JsSIP, so I just tried sipml5 and see the same thing:
>
> DEBUG | 20130321-200133.342 | repro | RESIP:TRANSPORT | 140720189728512 |
> TcpBaseTransport.cxx:263 | Processing write for [ V4
> 176.209.218.197:39628TCP target domain=unspecified mFlowKey=33 ]
> DEBUG | 20130321-200133.342 | repro | RESIP:TRANSPORT | 140720189728512 |
> ConnectionManager.cxx:59 | Found fd 33
> INFO | 20130321-200134.434 | repro | RESIP | 140720189728512 |
> SipMessage.cxx:931 | Content Length (4657) is 3791 bytes larger than body
> (866)! (We are supposed to 400 this)
I've found the root cause of this and can confirm there is an issue here:
resip/stack/ConnectionBase.cxx
The original code in the stack makes a read buffer no bigger than
ChunkSize. Released versions of reSIP have ChunkSize 2048. On main
(and b-webrtc), ChunkSize is 4096, but it is still not enough for large
SDP apparently. Even so, the logic appears to be geared for
preparseNewBytes(), which has some logic for expanding the buffer in
certain situations, but not always enough anyway. However, the way that
the original patch pushed WebRTC data into preparseNewBytes() didn't
fully benefit from this anyway.
The original WebRTC patch also bumped ChunkSize to 0xFFFF, but this has
two problems:
a) for people who want memory-efficiency, 0xFFFF is a lot of waste
b) for any future application that sends some kind of jumbo-SDP (or
other SIP message bodies), it may still be less than big enough
-- so nobody wins
A simple approach would involve expanding the buffer on demand. For a
WS frame, this would mean looking at the frame header for a clue about
how much space is really needed. This could also expose the stack to a
vicious DoS attack though, somebody could send a small WS frame with an
invalid header specifying an obscenely large payload (up to 2^55 - 1),
and a few mallocs later, the process is out of memory.
Can anyone else comment on this issue, not just for WebSockets and SDP,
but for any SIP message body?
My initial feeling about it is that there should be dynamic buffer
allocation up to a configurable limit, just as many web servers impose a
maximum file upload size.
On a side note, the constraint on NewMessage below appears to require a
workaround implementing any non-textual wire formats such as SigComp and
WebSocket, as they have to start the connection off in NewMessage state
even though they will never operate in that mode.
Nathan, I've bumped this to 8192 in the b-webrtc branch to see if I have
correctly diagnosed the problem you experience. However, before the 1.9
release, we will need to tune this properly so it doesn't come back to
bite us again in future.
std::pair<char*, size_t>
ConnectionBase::getWriteBuffer()
{
if (mConnState == NewMessage)
{
if (!mBuffer)
{
DebugLog (<< "Creating buffer for " << *this);
mBuffer =
MsgHeaderScanner::allocateBuffer(ConnectionBase::ChunkSize);
mBufferSize = ConnectionBase::ChunkSize;
}
mBufferPos = 0;
}
return getCurrentWriteBuffer();
}