Skip to content

Commit

Permalink
CCAPI client rpc fixes
Browse files Browse the repository at this point in the history
On Windows XP, cci_os_ipc_thread_init() causes additional threads to be
spawned immediately, which results in a vicious cycle until Windows
resources are exhausted.  Instead, defer thread_init() until it is really
needed.

Also, use the MSDN-recommended defaults for RPC calls instead of random
constants.

Signed-off-by: Kevin Wasserman <kevin.wasserman@painless-security.com>

ticket: 7322 (new)
target_version: 1.10.4
tags: pullup
  • Loading branch information
Kevin Wasserman authored and kaduk committed Aug 29, 2012
1 parent c675318 commit 9d528cd
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 24 deletions.
3 changes: 3 additions & 0 deletions src/ccapi/common/win/tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ void tspdata_setUUID(struct tspdata* p, unsigned char __RPC_FAR* uuidString) {
strncpy(p->_uuid, uuidString, UUID_SIZE-1);
};

void tspdata_setListening (struct tspdata* p, BOOL b) {p->_listening = b;}

void tspdata_setConnected (struct tspdata* p, BOOL b) {p->_CCAPI_Connected = b;}

void tspdata_setReplyEvent(struct tspdata* p, HANDLE h) {p->_replyEvent = h;}
Expand All @@ -58,6 +60,7 @@ void tspdata_setSST (struct tspdata* p, time_t t) {p->_sst

void tspdata_setStream (struct tspdata* p, k5_ipc_stream s) {p->_stream = s;}

BOOL tspdata_getListening (const struct tspdata* p) {return p->_listening;}

BOOL tspdata_getConnected (const struct tspdata* p) {return p->_CCAPI_Connected;}

Expand Down
3 changes: 3 additions & 0 deletions src/ccapi/common/win/tls.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
*/

struct tspdata {
BOOL _listening;
BOOL _CCAPI_Connected;
RPC_ASYNC_STATE* _rpcState;
HANDLE _replyEvent;
Expand All @@ -52,6 +53,7 @@ struct tspdata {
struct tspdata* new_tspdata (char* uuid, time_t sst);
void delete_tspdata (struct tspdata* p);

void tspdata_setListening (struct tspdata* p, BOOL b);
void tspdata_setConnected (struct tspdata* p, BOOL b);
void tspdata_setReplyEvent(struct tspdata* p, HANDLE h);
void tspdata_setRpcAState (struct tspdata* p, RPC_ASYNC_STATE* rpcState);
Expand All @@ -60,6 +62,7 @@ void tspdata_setStream (struct tspdata* p, k5_ipc_stream s);
void tspdata_setUUID (struct tspdata* p, unsigned char __RPC_FAR* uuidString);
HANDLE tspdata_getReplyEvent(const struct tspdata* p);

BOOL tspdata_getListening(const struct tspdata* p);
BOOL tspdata_getConnected(const struct tspdata* p);
RPC_ASYNC_STATE* tspdata_getRpcAState(const struct tspdata* p);
time_t tspdata_getSST (const struct tspdata* p);
Expand Down
32 changes: 12 additions & 20 deletions src/ccapi/lib/win/ccapi_os_ipc.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ cc_int32 cci_os_ipc_msg( cc_int32 in_launch_server,
extern "C" cc_int32 cci_os_ipc_process_init (void) {
RPC_STATUS status;

opts.cMinCalls = 1;
opts.cMaxCalls = 20;
if (!isNT()) {
status = RpcServerRegisterIf(ccs_reply_ServerIfHandle, // interface
NULL, // MgrTypeUuid
Expand All @@ -90,7 +88,7 @@ extern "C" cc_int32 cci_os_ipc_process_init (void) {
NULL, // MgrTypeUuid
NULL, // MgrEpv; 0 means default
RPC_IF_ALLOW_SECURE_ONLY,
opts.cMaxCalls,
RPC_C_LISTEN_MAX_CALLS_DEFAULT,
NULL); // No security callback.
}
cci_check_error(status);
Expand Down Expand Up @@ -118,10 +116,6 @@ extern "C" cc_int32 cci_os_ipc_thread_init (void) {

if (!GetTspData(GetTlsIndex(), &ptspdata)) return ccErrNoMem;

opts.cMinCalls = 1;
opts.cMaxCalls = 20;
opts.fDontWait = TRUE;

err = cci_check_error(UuidCreate(&uuid)); // Get a UUID
if (err == RPC_S_OK) { // Convert to string
err = UuidToString(&uuid, &uuidString);
Expand All @@ -131,7 +125,7 @@ extern "C" cc_int32 cci_os_ipc_thread_init (void) {
tspdata_setUUID(ptspdata, uuidString);
endpoint = clientEndpoint((const char *)uuidString);
err = RpcServerUseProtseqEp((RPC_CSTR)"ncalrpc",
opts.cMaxCalls,
RPC_C_PROTSEQ_MAX_REQS_DEFAULT,
(RPC_CSTR)endpoint,
sa.lpSecurityDescriptor); // SD
free(endpoint);
Expand All @@ -155,9 +149,7 @@ extern "C" cc_int32 cci_os_ipc_thread_init (void) {
if (!err) {
static bool bListening = false;
if (!bListening) {
err = RpcServerListen(opts.cMinCalls,
opts.cMaxCalls,
TRUE);
err = RpcServerListen(1, RPC_C_LISTEN_MAX_CALLS_DEFAULT, TRUE);
cci_check_error(err);
}
bListening = err == 0;
Expand Down Expand Up @@ -202,25 +194,29 @@ extern "C" cc_int32 cci_os_ipc_msg( cc_int32 in_launch_server,
PROCESS_INFORMATION pi = { 0 };
HANDLE replyEvent = 0;
BOOL bCCAPI_Connected= FALSE;
BOOL bListening = FALSE;
unsigned char tspdata_handle[8] = { 0 };

if (!in_request_stream) { err = cci_check_error (ccErrBadParam); }
if (!out_reply_stream ) { err = cci_check_error (ccErrBadParam); }

if (!GetTspData(GetTlsIndex(), &ptspdata)) {return ccErrBadParam;}
bListening = tspdata_getListening(ptspdata);
if (!bListening) {
err = cci_check_error(cci_os_ipc_thread_init());
bListening = !err;
tspdata_setListening(ptspdata, bListening);
}

bCCAPI_Connected = tspdata_getConnected (ptspdata);
replyEvent = tspdata_getReplyEvent (ptspdata);
sst = tspdata_getSST (ptspdata);
uuid = tspdata_getUUID(ptspdata);

// Initialize old CCAPI if necessary:
if (!err) if (!Init:: Initialized()) err = cci_check_error(Init:: Initialize( ));
if (!err) if (!Client::Initialized()) err = cci_check_error(Client::Initialize(0));

// The lazy connection to the server has been put off as long as possible!
// ccapi_connect starts listening for replies as an RPC server and then
// calls ccs_rpc_connect.
if (!bCCAPI_Connected) {
if (!err && !bCCAPI_Connected) {
err = cci_check_error(ccapi_connect(ptspdata));
bCCAPI_Connected = !err;
tspdata_setConnected(ptspdata, bCCAPI_Connected);
Expand Down Expand Up @@ -330,10 +326,6 @@ cc_int32 ccapi_connect(const struct tspdata* tsp) {
replyEvent = tspdata_getReplyEvent(tsp);
uuid = tspdata_getUUID(tsp);

opts.cMinCalls = 1;
opts.cMaxCalls = 20;
opts.fDontWait = TRUE;

cci_debug_printf("%s is listening ...", __FUNCTION__);

// Clear replyEvent so we can detect when a reply to our connect request has been received:
Expand Down
10 changes: 6 additions & 4 deletions src/ccapi/lib/win/dllmain.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,12 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, // DLL module handle

memset(ptspdata, 0, sizeof(struct tspdata));

// Initialize CCAPI thread data:
cci_ipc_thread_init();

break;
// Do not call cci_ipc_thread_init() yet; defer until we actually
// need it. On XP, cci_ipc_thread_init() will cause additional
// threads to be immediately spawned, which will bring us right
// back here again ad infinitum, until windows
// resources are exhausted.
break;

// The thread of the attached process terminates:
case DLL_THREAD_DETACH:
Expand Down

0 comments on commit 9d528cd

Please sign in to comment.