Skip to content

IIS, buffer request body before taking lock #1651

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

Closed
wants to merge 1 commit into from
Closed

IIS, buffer request body before taking lock #1651

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
121 changes: 94 additions & 27 deletions iis/mymodule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@

#include "winsock2.h"

// Used to hold each chunk of request body that gets read before the full ModSec engine is invoked
typedef struct preAllocBodyChunk {
preAllocBodyChunk* next;
size_t length;
void* data;
} preAllocBodyChunk;

class REQUEST_STORED_CONTEXT : public IHttpStoredContext
{
public:
Expand Down Expand Up @@ -82,8 +89,11 @@ class REQUEST_STORED_CONTEXT : public IHttpStoredContext
char *m_pResponseBuffer;
ULONGLONG m_pResponseLength;
ULONGLONG m_pResponsePosition;

preAllocBodyChunk* requestBodyBufferHead;
};


//----------------------------------------------------------------------------

char *GetIpAddr(apr_pool_t *pool, PSOCKADDR pAddr)
Expand Down Expand Up @@ -286,6 +296,44 @@ REQUEST_STORED_CONTEXT *RetrieveIISContext(request_rec *r)
return NULL;
}

HRESULT GetRequestBodyFromIIS(IHttpRequest* pRequest, preAllocBodyChunk** head)
{
HRESULT hr = S_OK;
HTTP_REQUEST * pRawRequest = pRequest->GetRawHttpRequest();
preAllocBodyChunk** cur = head;
while (pRequest->GetRemainingEntityBytes() > 0)
{
// Allocate memory for the preAllocBodyChunk linked list structure, and also the actual body content
// HUGE_STRING_LEN is hardcoded because this is also hardcoded in apache2_io.c's call to ap_get_brigade
preAllocBodyChunk* chunk = (preAllocBodyChunk*)malloc(sizeof(preAllocBodyChunk) + HUGE_STRING_LEN);
chunk->next = NULL;

// Pointer to rest of allocated memory, for convenience
chunk->data = chunk + 1;

DWORD readcnt = 0;
hr = pRequest->ReadEntityBody(chunk->data, HUGE_STRING_LEN, false, &readcnt, NULL);
if (ERROR_HANDLE_EOF == (hr & 0x0000FFFF))
{
free(chunk);
hr = S_OK;
break;
}
chunk->length = readcnt;

// Append to linked list
*cur = chunk;
cur = &(chunk->next);

if (hr != S_OK)
{
break;
}
}

return hr;
}

HRESULT CMyHttpModule::ReadFileChunk(HTTP_DATA_CHUNK *chunk, char *buf)
{
OVERLAPPED ovl;
Expand Down Expand Up @@ -752,6 +800,24 @@ CMyHttpModule::OnBeginRequest(
goto Finished;
}

// Get request body without holding lock, because some clients may be slow at sending
LeaveCriticalSection(&m_csLock);
preAllocBodyChunk* requestBodyBufferHead = NULL;
hr = GetRequestBodyFromIIS(pRequest, &requestBodyBufferHead);
if (hr != S_OK)
{
goto FinishedWithoutLock;
}
EnterCriticalSection(&m_csLock);

// Get the config again, in case it changed during the time we released the lock
hr = MODSECURITY_STORED_CONTEXT::GetConfig(pHttpContext, &pConfig);
if (FAILED(hr))
{
hr = S_OK;
goto Finished;
}

// every 3 seconds we check for changes in config file
//
DWORD ctime = GetTickCount();
Expand Down Expand Up @@ -841,6 +907,8 @@ CMyHttpModule::OnBeginRequest(
rsc->m_pRequestRec = r;
rsc->m_pHttpContext = pHttpContext;
rsc->m_pProvider = pProvider;
rsc->requestBodyBufferHead = requestBodyBufferHead;
requestBodyBufferHead = NULL; // This is to indicate to the cleanup process to use rsc->requestBodyBufferHead instead of requestBodyBufferHead now

pHttpContext->GetModuleContextContainer()->SetModuleContext(rsc, g_pModuleContext);

Expand Down Expand Up @@ -1086,6 +1154,16 @@ CMyHttpModule::OnBeginRequest(
Finished:
LeaveCriticalSection(&m_csLock);

FinishedWithoutLock:
// Free the preallocated body in case there was a failure and it wasn't consumed already
preAllocBodyChunk* chunkToFree = requestBodyBufferHead ? requestBodyBufferHead : rsc->requestBodyBufferHead;
while (chunkToFree != NULL)
{
preAllocBodyChunk* next = chunkToFree->next;
free(chunkToFree);
chunkToFree = next;
}

if ( FAILED( hr ) )
{
return RQ_NOTIFICATION_FINISH_REQUEST;
Expand All @@ -1095,40 +1173,29 @@ CMyHttpModule::OnBeginRequest(

apr_status_t ReadBodyCallback(request_rec *r, char *buf, unsigned int length, unsigned int *readcnt, int *is_eos)
{
REQUEST_STORED_CONTEXT *rsc = RetrieveIISContext(r);

*readcnt = 0;

if(rsc == NULL)
{
*is_eos = 1;
return APR_SUCCESS;
}
REQUEST_STORED_CONTEXT *rsc = RetrieveIISContext(r);

IHttpContext *pHttpContext = rsc->m_pHttpContext;
IHttpRequest *pRequest = pHttpContext->GetRequest();
if (rsc->requestBodyBufferHead == NULL)
{
*is_eos = 1;
return APR_SUCCESS;
}

if(pRequest->GetRemainingEntityBytes() == 0)
{
*is_eos = 1;
return APR_SUCCESS;
}
*readcnt = length < (unsigned int) rsc->requestBodyBufferHead->length ? length : (unsigned int) rsc->requestBodyBufferHead->length;
void* src = (char*)rsc->requestBodyBufferHead->data;
memcpy_s(buf, length, src, *readcnt);

HRESULT hr = pRequest->ReadEntityBody(buf, length, false, (DWORD *)readcnt, NULL);
// Remove the front and proceed to next chunk in the linked list
preAllocBodyChunk* chunkToFree = rsc->requestBodyBufferHead;
rsc->requestBodyBufferHead = rsc->requestBodyBufferHead->next;
free(chunkToFree);

if (FAILED(hr))
if (rsc->requestBodyBufferHead == NULL)
{
// End of data is okay.
if (ERROR_HANDLE_EOF != (hr & 0x0000FFFF))
{
// Set the error status.
rsc->m_pProvider->SetErrorStatus( hr );
}

*is_eos = 1;
*is_eos = 1;
}

return APR_SUCCESS;
return APR_SUCCESS;
}

apr_status_t WriteBodyCallback(request_rec *r, char *buf, unsigned int length)
Expand Down