186657 - Implemented locking around passhook data file access

5 files changed, 231 insertions, 114 deletions

diff --git a/ldap/synctools/passwordsync/passhand.cpp b/ldap/synctools/passwordsync/passhand.cpp
index ddad1977..4cefd58c 100644
--- a/ldap/synctools/passwordsync/passhand.cpp
+++ b/ldap/synctools/passwordsync/passhand.cpp
@@ -112,6 +112,8 @@ int saveSet(PASS_INFO_LIST* passInfoList, char* filename)
 	outFile.close();
 
 exit:
+	// We need to unfreeze plainTextStream so memory gets freed by the destructor
+	plainTextStream.rdbuf()->freeze(false);
 	free(cipherTextBuf);
 	return result;
 }
@@ -119,18 +121,18 @@ exit:
 int loadSet(PASS_INFO_LIST* passInfoList, char* filename)
 {
 	int result = 0;
-	int i;
+	int i = 0;
 	fstream inFile;
 	PASS_INFO newPair;
 	strstream* plainTextStream;
 	char* cipherTextBuf = NULL;
 	char* plainTextBuf = NULL;
-	int usernameLen;
-	int passwordLen;
-	int plainTextLen;
-	int cipherTextLen;
+	int usernameLen = 0;
+	int passwordLen = 0;
+	int plainTextLen = 0;
+	int cipherTextLen = 0;
 	int resultTextLen = 0;
-	int pairCount;
+	int pairCount = 0;
 
 	// Read in cipher text from file
 	inFile.open(filename, ios::in | ios::binary);
@@ -164,6 +166,12 @@ int loadSet(PASS_INFO_LIST* passInfoList, char* filename)
 		goto exit;
 	}
 
+	// Check to see if plainTextbuf contains anything
+	if (resultTextLen <= 0) {
+		result = -1;
+		goto exit;
+	}
+
 	plainTextStream = new strstream(plainTextBuf, resultTextLen);
 
 	plainTextStream->read((char*)&pairCount, sizeof(pairCount));
diff --git a/ldap/synctools/passwordsync/passhand.h b/ldap/synctools/passwordsync/passhand.h
index 344e009a..27674789 100644
--- a/ldap/synctools/passwordsync/passhand.h
+++ b/ldap/synctools/passwordsync/passhand.h
@@ -50,7 +50,9 @@
 #include "prerror.h"
 
 #define PASSHAND_EVENT_NAME "passhand_event"
+#define PASSHOOK_MUTEX_NAME "passhook_mutex"
 
+#define PASSHOOK_TIMEOUT 30000
 #define PASSHAND_BUF_SIZE 256
 
 using namespace std;
diff --git a/ldap/synctools/passwordsync/passhook/passhook.cpp b/ldap/synctools/passwordsync/passhook/passhook.cpp
index eba277a2..e9ec0891 100644
--- a/ldap/synctools/passwordsync/passhook/passhook.cpp
+++ b/ldap/synctools/passwordsync/passhook/passhook.cpp
@@ -48,121 +48,74 @@
 #define STATUS_SUCCESS  ((NTSTATUS)0x00000000L)
 #endif
 
+DWORD WINAPI SavePasshookChange( LPVOID passinfo );
+static HANDLE passhookMutexHandle;
+static unsigned long logLevel;
+
 NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG RelativeId, PUNICODE_STRING Password)
 {
-	HANDLE passhookEventHandle = OpenEvent(EVENT_MODIFY_STATE, FALSE, PASSHAND_EVENT_NAME);
-	PASS_INFO newPassInfo;
-	PASS_INFO_LIST passInfoList;
-	HKEY regKey;
-	DWORD type;
-	unsigned long buffSize;
-	char regBuff[PASSHAND_BUF_SIZE];
-	unsigned long logLevel;
+	PASS_INFO *newPassInfo = NULL;
+	HANDLE passhookThreadHandle;
 	fstream outLog;
+	DWORD waitRes;
 
-	RegOpenKey(HKEY_LOCAL_MACHINE, "SOFTWARE\\PasswordSync", &regKey);
-	buffSize = PASSHAND_BUF_SIZE;
-	if(RegQueryValueEx(regKey, "Log Level", NULL, &type, (unsigned char*)regBuff, &buffSize) == ERROR_SUCCESS)
-	{
-		logLevel = (unsigned long)atoi(regBuff);
-	}
-	else
-	{
-		logLevel = 0;
-	}
-	if(logLevel > 0)
-	{
-		outLog.open("passhook.log", ios::out | ios::app);
+	// This memory will be freed in SavePasshookChange
+	if ( newPassInfo = (PASS_INFO *) malloc(sizeof(PASS_INFO)) ) {
+		// These get freed in SavePasshookChange by calling clearSet
+		newPassInfo->username = (char*)malloc((UserName->Length / 2) + 1);
+		newPassInfo->password = (char*)malloc((Password->Length / 2) + 1);
+	} else {
+		goto exit;
 	}
-	RegCloseKey(regKey);
 
-	// This memory will be free'd by calling clearSet below
-	newPassInfo.username = (char*)malloc((UserName->Length / 2) + 1);
-	newPassInfo.password = (char*)malloc((Password->Length / 2) + 1);
+	// Fill in the password change struct
+	if (newPassInfo->username && newPassInfo->password) {
+                _snprintf(newPassInfo->username, (UserName->Length / 2), "%S", UserName->Buffer);
+                _snprintf(newPassInfo->password, (Password->Length / 2), "%S", Password->Buffer);
+                newPassInfo->username[UserName->Length / 2] = '\0';
+                newPassInfo->password[Password->Length / 2] = '\0';
+
+		// Backoff
+                newPassInfo->backoffCount = 0;
 
-	if (newPassInfo.username && newPassInfo.password) {
-		_snprintf(newPassInfo.username, (UserName->Length / 2), "%S", UserName->Buffer);
-		_snprintf(newPassInfo.password, (Password->Length / 2), "%S", Password->Buffer);
-		newPassInfo.username[UserName->Length / 2] = '\0';
-		newPassInfo.password[Password->Length / 2] = '\0';
+                // Load time
+                time(&(newPassInfo->atTime));
 	} else {
-		if(outLog.is_open()) {
-			timeStamp(&outLog);
-			outLog << "failed to allocate memory for username and password" << endl;
-		}
-		free(newPassInfo.username);
-		free(newPassInfo.password);
+		// Memory error.  Free everything we allocated.
+		free(newPassInfo->username);
+		free(newPassInfo->password);
+		free(newPassInfo);
 		goto exit;
 	}
 
-	if(outLog.is_open())
-	{
-		timeStamp(&outLog);
-		outLog << "user " << newPassInfo.username << " password changed" << endl;
-		//outLog << "user " << newPassInfo.username << " password changed to " << newPassInfo.password << endl;
-	}
+	// Fire off a thread to do the real work
+	passhookThreadHandle = CreateThread(NULL, 0, SavePasshookChange, newPassInfo, 0, NULL); 
 
-	// loadSet allocates memory for the usernames and password.  We need to be
-	// sure to free it by calling clearSet.
-	if(loadSet(&passInfoList, "passhook.dat") == 0)
-	{
-		if(outLog.is_open())
-		{
-			timeStamp(&outLog);
-			outLog << passInfoList.size() << " entries loaded from file" << endl;
-		}
-	}
-	else
-	{
-		if(outLog.is_open())
-		{
-			timeStamp(&outLog);
-			outLog << "failed to load entries from file" << endl;
-		}
-	}
+	// We need to close the handle to the thread we created.  Doing
+	// this will not terminate the thread.
+	if (passhookThreadHandle != NULL) {
+		CloseHandle(passhookThreadHandle);
+	} else {
+		// Acquire the mutex so we can log an error
+		waitRes = WaitForSingleObject(passhookMutexHandle, PASSHOOK_TIMEOUT);
 
-	// Add the new change to the list
-	passInfoList.push_back(newPassInfo);
-
-	// Save the list to disk
-	if(saveSet(&passInfoList, "passhook.dat") == 0)
-	{
-		if(outLog.is_open())
-		{
-			timeStamp(&outLog);
-			outLog << passInfoList.size() << " entries saved to file" << endl;
-		}
-	}
-	else
-	{
-		if(outLog.is_open())
-		{
-			timeStamp(&outLog);
-			outLog << "failed to save entries to file" << endl;
-		}
-	}
+		// If we got the mutex, log the error, otherwise it's not safe to log
+		if (waitRes == WAIT_OBJECT_0) {
+                	outLog.open("passhook.log", ios::out | ios::app);
 
-	// We need to call clearSet so memory gets free'd
-	clearSet(&passInfoList);
+		        if(outLog.is_open()) {
+       		         	timeStamp(&outLog);
+		                outLog << "Failed to start thread.  Aborting change for " << newPassInfo->username << endl;
+       		 	}
 
-exit:
-	if(passhookEventHandle == NULL)
-	{
-		if(outLog.is_open())
-		{
-			timeStamp(&outLog);
-			outLog << "can not get password sync service event handle, service not running" << endl;
-		}
+			outLog.close();
 
-	}
-	else
-	{
-		SetEvent(passhookEventHandle);
-		CloseHandle(passhookEventHandle);
+			// Release mutex
+			ReleaseMutex(passhookMutexHandle);
+		}
 	}
 
-	outLog.close();
-
+exit:
 	return STATUS_SUCCESS;
 }
 
@@ -173,5 +126,132 @@ BOOL NTAPI PasswordFilter(PUNICODE_STRING UserName, PUNICODE_STRING FullName, PU
 
 BOOL NTAPI InitializeChangeNotify()
 {
-	return TRUE;
+	HKEY regKey;
+	DWORD type;
+	unsigned long buffSize;
+	char regBuff[PASSHAND_BUF_SIZE];
+	fstream outLog;
+
+	// check if logging is enabled
+	RegOpenKey(HKEY_LOCAL_MACHINE, "SOFTWARE\\PasswordSync", &regKey);
+        buffSize = PASSHAND_BUF_SIZE;
+        if(RegQueryValueEx(regKey, "Log Level", NULL, &type, (unsigned char*)regBuff, &buffSize) == ERROR_SUCCESS)
+        {
+                logLevel = (unsigned long)atoi(regBuff);
+        }
+        else
+        {
+                logLevel = 0;
+        }
+        RegCloseKey(regKey);
+
+	// Create mutex for passhook data file and log file access
+	passhookMutexHandle = CreateMutex(NULL, FALSE, PASSHOOK_MUTEX_NAME);
+
+	if (passhookMutexHandle == NULL) {
+		// Log an error.
+		outLog.open("passhook.log", ios::out | ios::app);
+		timeStamp(&outLog);
+		outLog << "Failed to create passhook mutex.  Passhook DLL will not be loaded." << endl;
+		outLog.close();
+
+		return FALSE;
+	} else {
+		return TRUE;
+	}
+}
+
+// This function will save the password change to the passhook data file.  It
+// will be run as a separate thread.
+DWORD WINAPI SavePasshookChange( LPVOID passinfo ) 
+{
+	PASS_INFO *newPassInfo = NULL;
+        PASS_INFO_LIST passInfoList;
+        HANDLE passhookEventHandle = OpenEvent(EVENT_MODIFY_STATE, FALSE, PASSHAND_EVENT_NAME);
+	fstream outLog;
+
+	if ((newPassInfo = (PASS_INFO *)passinfo) == NULL) {
+		goto exit;
+	}
+
+        // Acquire the mutex for passhook.dat.  This mutex also guarantees
+	// that we can write to outLog safely.
+        WaitForSingleObject(passhookMutexHandle, INFINITE);
+
+	// Open the log file if logging is enabled
+        if(logLevel > 0)
+        {
+                outLog.open("passhook.log", ios::out | ios::app);
+        }
+
+	if(outLog.is_open())
+        {
+                timeStamp(&outLog);
+                outLog << "user " <<  newPassInfo->username << " password changed" << endl;
+                //outLog << "user " <<  newPassInfo->username << " password changed to " <<  newPassInfo->passname << endl;
+        }
+
+        // loadSet allocates memory for the usernames and password.  We need to be
+        // sure to free it by calling clearSet.
+        if(loadSet(&passInfoList, "passhook.dat") == 0)
+        {
+                if(outLog.is_open())
+                {
+                        timeStamp(&outLog);
+                        outLog << passInfoList.size() << " entries loaded from file" << endl;
+                }
+        }
+        else
+        {
+                if(outLog.is_open())
+                {
+                        timeStamp(&outLog);
+                        outLog << "failed to load entries from file" << endl;
+                }
+        }
+
+	// Add the new change to the list
+        passInfoList.push_back(*newPassInfo);
+
+        // Save the list to disk
+        if(saveSet(&passInfoList, "passhook.dat") == 0)
+        {
+                if(outLog.is_open())
+                {
+                        timeStamp(&outLog);
+                        outLog << passInfoList.size() << " entries saved to file" << endl;
+                }
+        }
+        else
+        {
+		// We always want to log this error condition
+                if(!outLog.is_open())
+                {
+			// We need to open the log since debug logging is turned off
+			outLog.open("passhook.log", ios::out | ios::app);
+		}
+
+                timeStamp(&outLog);
+                outLog << "failed to save entries to file" << endl;
+        }
+
+	// Close the log file before we release the mutex.
+	outLog.close();
+
+        // Release the mutex for passhook.dat
+        ReleaseMutex(passhookMutexHandle);
+
+        // We need to call clearSet so memory gets free'd
+        clearSet(&passInfoList);
+
+exit:
+	// Free the passed in struct from the heap
+	free(newPassInfo);
+
+        if (passhookEventHandle != NULL) {
+                SetEvent(passhookEventHandle);
+		CloseHandle(passhookEventHandle);
+        }
+
+	return 0;
 }
diff --git a/ldap/synctools/passwordsync/passsync/syncserv.cpp b/ldap/synctools/passwordsync/passsync/syncserv.cpp
index 63bb451e..6e314f8e 100644
--- a/ldap/synctools/passwordsync/passsync/syncserv.cpp
+++ b/ldap/synctools/passwordsync/passsync/syncserv.cpp
@@ -76,6 +76,7 @@ PassSyncService::PassSyncService(const TCHAR *serviceName) : CNTService(serviceN
 	unsigned long size;
 
 	passhookEventHandle = CreateEvent(NULL, FALSE, FALSE, PASSHAND_EVENT_NAME);
+	passhookMutexHandle = CreateMutex(NULL, FALSE, PASSHOOK_MUTEX_NAME);
 	mainLdapConnection = NULL;
 	results = NULL;
 	currentResult = NULL;
@@ -211,10 +212,14 @@ void PassSyncService::Run()
 				timeStamp(&outLog);
 				outLog << "Backing off for " << BackoffTime(GetMinBackoff()) << "ms" << endl;
 			}
-			WaitForSingleObject(passhookEventHandle, BackoffTime(GetMinBackoff()));
+			waitRes = WaitForSingleObject(passhookEventHandle, BackoffTime(GetMinBackoff()));
 			if(logLevel > 0) {
 				timeStamp(&outLog);
-				outLog << "Backoff time expired.  Attempting sync" << endl;
+				if (waitRes == WAIT_TIMEOUT) {
+					outLog << "Backoff time expired.  Attempting sync" << endl;
+				} else {
+					outLog << "Received passhook event.  Attempting sync" << endl;
+				}
 			}
 		}
 
@@ -226,24 +231,38 @@ void PassSyncService::Run()
 
 	if(passInfoList.size() > 0)
 	{
-		if(saveSet(&passInfoList, dataFilename) == 0)
+		// Get mutex for passhook.dat
+		WaitForSingleObject(passhookMutexHandle, INFINITE);
+
+		// Need to loadSet here so we don't overwrite entries that passhook recently added
+		if(loadSet(&passInfoList, dataFilename) == 0)
 		{
-			if(logLevel > 0)
+			if(saveSet(&passInfoList, dataFilename) == 0)
+			{
+				if(logLevel > 0)
+				{
+					timeStamp(&outLog);
+					outLog << passInfoList.size() << " entries saved to data file" << endl;
+				}
+			}
+			else
 			{
 				timeStamp(&outLog);
-				outLog << passInfoList.size() << " entries saved to data file" << endl;
+				outLog << "Failed to save entries to data file" << endl;
 			}
-		}
-		else
-		{
+		} else {
 			timeStamp(&outLog);
-			outLog << "Failed to save entries to data file" << endl;
+			outLog << "Failed to load entries from file" << endl;
 		}
+
+		// Release mutex for passhook.dat
+		ReleaseMutex(passhookMutexHandle);
 	}
 
 exit:
 	clearSet(&passInfoList);
 	CloseHandle(passhookEventHandle);
+	CloseHandle(passhookMutexHandle);
 }
 
 // ****************************************************************
@@ -258,6 +277,9 @@ int PassSyncService::SyncPasswords()
 	char* dn = NULL;
 	int tempSize = passInfoList.size();
 
+	// Get mutex for passhook.dat
+	WaitForSingleObject(passhookMutexHandle, INFINITE);
+
 	if(loadSet(&passInfoList, dataFilename) == 0)
 	{
 		if((passInfoList.size() - tempSize) > 0)
@@ -289,6 +311,9 @@ int PassSyncService::SyncPasswords()
 		outLog << "Failed to load entries from file" << endl;
 	}
 
+	// Release mutex for passhook.dat
+	ReleaseMutex(passhookMutexHandle);
+
 	if(passInfoList.size() > 0)
 	{
 		if(logLevel > 0)
diff --git a/ldap/synctools/passwordsync/passsync/syncserv.h b/ldap/synctools/passwordsync/passsync/syncserv.h
index c072114d..4c1a9a13 100644
--- a/ldap/synctools/passwordsync/passsync/syncserv.h
+++ b/ldap/synctools/passwordsync/passsync/syncserv.h
@@ -82,6 +82,7 @@ private:
 
 	PASS_INFO_LIST passInfoList;
 	HANDLE passhookEventHandle;
+	HANDLE passhookMutexHandle;
 
 	// LDAP variables
 	LDAP* mainLdapConnection;
@@ -104,7 +105,8 @@ private:
 	unsigned long maxBackoffTime;
 	int logLevel;
 	bool isRunning;
+	DWORD waitRes;
 	fstream outLog;
 };
 
-#endif
\ No newline at end of file
+#endif