[reSIProcate-users] Error in ConnectionBase::getWriteBufferForExtraBytes() ?

[Michael Trank <Michael.Trank@xxxxxxxxxxxx>]

Scott and other resiprocate-users:

I have recently come across problems when using the Repro/Resiprocate code with SIP/WSS UA's that send large SDP payloads such that the entire INVITE request is greater than 8192 bytes. This has become more common of late with WebRTC-based UA's with large "candidate" lists.  Specifically, the first INVITE message arriving over WSS is received and handled okay, but all subsequent large INVITE requests elicit parse errors exceptions.

I have traced the problem to ConnectionBase::getWriteBufferForExtraBytes(), which I believe returns the wrong value for the pointer where the TlsConnection should place the "extra bytes" read in from the TLS pipe.

char*
ConnectionBase::getWriteBufferForExtraBytes(int extraBytes)
{
   if (extraBytes > 0)
   {
      char* buffer = MsgHeaderScanner::allocateBuffer((int)mBufferSize + extraBytes);
      memcpy(buffer, mBuffer, mBufferSize);
      delete [] mBuffer;
      mBuffer = buffer;
      buffer += mBufferSize;
      mBufferSize += extraBytes;
      return buffer;
   }
   else
   {
      resip_assert(0);
      return mBuffer;
   }
}



Note that the size of the buffer before reallocation is not necessarily the place where the extra bytes should be placed. In my case, the SIP/WSS UA used the same connection to send serveral INVITE transactions with large payloads bigger than the hard-coded "chunk size", 8192. On the first request, the first SSL_read() gets 8192 bytes, and SSL_pending() gets back, say, 2000. The buffer is reallocated to 10192 bytes, mBufferSize = 10192, and the pointer returned by getWriteBufferForExtraBytes() returns mBuffer + 8192. So far so good.

But on the next large request, SSL_read() gets 8192 bytes, SSL_pending gets back 2000, and then, without checking is the buffer is already big enough, getWriteBufferForExtraBytes() again increases the size of the buffer to 12192, and returns 10192 as the place when the extra bytes should be written. Meaning that the "extra bytes" don't get written into the buffer concatenated with the first 8192 bytes, but rather from positions 10192 through 12191 of the 12192 byte long buffer. Since the client sends these bytes with "masking" the part of the buffer that got passed over is all goblygook when the SIP message is unmasked and parsed from the buffer.

I changed getWriteBufferForExtraBytes() to be the following code, with a parameter for passing in the number of bytes actually read by TlsConnection::read() in the first SSL_read(), so that reallocation only occurs if necessary and the correct position within the buffer is returned. It seems to have fixed the problem.

I invite comments and advice.

Thanks, as always, for your help with this stuff.

char*
ConnectionBase::getWriteBufferForExtraBytes(int readBytes, int extraBytes)
{
        if ((readBytes > 0) && (extraBytes > 0))
        {
                if ((readBytes + extraBytes) > mBufferSize) {
                        mBufferSize = readBytes + extraBytes;
                        char * buffer = MsgHeaderScanner::allocateBuffer(mBufferSize);
                        memcpy(buffer, mBuffer, readBytes);
                        delete[] mBuffer;
                        mBuffer = buffer;
                }
                return &mBuffer[readBytes];
        }
        else
        {
                resip_assert(0);
                return mBuffer;
        }
}