11:33 a.m.
passwordsync/passhand.cpp | 13 +++
passwordsync/passhand.h | 1
passwordsync/passhook/passhook.cpp | 150 +++++++++++++++++++++++++++++--------
passwordsync/passsync/syncserv.cpp | 21 +++--
4 files changed, 145 insertions(+), 40 deletions(-)
New commits:
commit 130cb2003ebdfe04b3bc2794a250acc8540fd8b3
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Wed May 15 15:13:08 2013 -0700
Ticket #363 - Passsync/Winsync handles passwords with 8-th bit characters incorrectly
Bug description: Passhook plugin used to store the password in
the intermediate file passhook.dat using _snprintf which just
converts wchar in ascii to char without considering the non-
ascii characters.
Fix description: Instead of using _snprintf, WideCharToMultiByte
is called to convert the Microsoft internal character set to
UTF-8, which is valid in LDAP.
Also, in SyncPasswords (PassSync), it adds LDAP_INAPPROPRIATE_
AUTH to the condition to send the password change on Windows to
the Directory server. Bind returns LDAP_INAPPROPRIATE_AUTH,
when a password is not in the entry for SIMPLE auth. PassSync
should be able to send the password for the case, as well.
https://fedorahosted.org/389/ticket/363
Reviewed by Rich (Thanks!!)
diff --git a/passwordsync/passhand.cpp b/passwordsync/passhand.cpp
index f9b39bd..bbc006f 100644
--- a/passwordsync/passhand.cpp
+++ b/passwordsync/passhand.cpp
@@ -56,6 +56,19 @@ void timeStamp(fstream* outFile)
}
}
+void wtimeStamp(wfstream* outFile)
+{
+ if(outFile->is_open())
+ {
+ char dateBuf[32];
+ char timeBuf[32];
+
+ _strdate(dateBuf);
+ _strtime(timeBuf);
+ *outFile << dateBuf << " " << timeBuf << ":
";
+ }
+}
+
int saveSet(PASS_INFO_LIST* passInfoList, char* filename)
{
int result = 0;
diff --git a/passwordsync/passhand.h b/passwordsync/passhand.h
index 4f60be9..c653756 100644
--- a/passwordsync/passhand.h
+++ b/passwordsync/passhand.h
@@ -71,6 +71,7 @@ typedef list<PASS_INFO> PASS_INFO_LIST;
typedef list<PASS_INFO>::iterator PASS_INFO_LIST_ITERATOR;
void timeStamp(fstream* outFile);
+void wtimeStamp(wfstream* outFile);
int encrypt(char* plainTextBuf, int plainTextLen, char* cipherTextBuf, int cipherTextLen,
int* resultTextLen);
int decrypt(char* cipherTextBuf, int cipherTextLen, char* plainTextBuf, int plainTextLen,
int* resultTextLen);
diff --git a/passwordsync/passhook/passhook.cpp b/passwordsync/passhook/passhook.cpp
index 3218881..801b98f 100644
--- a/passwordsync/passhook/passhook.cpp
+++ b/passwordsync/passhook/passhook.cpp
@@ -56,8 +56,9 @@ NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG
RelativeId,
{
PASS_INFO *newPassInfo = NULL;
HANDLE passhookThreadHandle;
- fstream outLog;
+ wfstream outLog;
DWORD waitRes;
+ int len;
// If UserName is NULL, just return STATUS_SUCCESS
if (UserName == NULL) {
@@ -65,41 +66,121 @@ NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG
RelativeId,
}
// 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);
- if (Password != NULL) {
- newPassInfo->password = (char*)malloc((Password->Length / 2) + 1);
- } else {
- newPassInfo->password = (char*)malloc(1);
- }
- } else {
+ newPassInfo = (PASS_INFO *)calloc(1, sizeof(PASS_INFO));
+ if (NULL == newPassInfo) {
goto exit;
}
+ outLog.open("passhook.log", ios::out | ios::app);
// Fill in the password change struct
- if (newPassInfo->username && newPassInfo->password) {
- _snprintf(newPassInfo->username, (UserName->Length / 2), "%S",
UserName->Buffer);
- newPassInfo->username[UserName->Length / 2] = '\0';
- if (Password != NULL) {
- _snprintf(newPassInfo->password, (Password->Length / 2), "%S",
Password->Buffer);
- newPassInfo->password[Password->Length / 2] = '\0';
- } else {
- newPassInfo->password[0] = '\0';
+ /* Note: LPSTR == "char *" */
+ len = WideCharToMultiByte(CP_UTF8, 0 /* no flags */,
+ // Length is in bytes; assume UCS2
+ UserName->Buffer, UserName->Length/2,
+ newPassInfo->username, 0,
+ NULL, NULL);
+ if (len > 0) {
+ newPassInfo->username = (char*)malloc(len + 1);
+ if (NULL == newPassInfo->username) {
+ if(outLog.is_open()) {
+ wtimeStamp(&outLog);
+ outLog << "Failed to alloc mem for user " <<
UserName->Buffer
+ << "; error (" << GetLastError() << ")"
<< endl;
+ }
+ free(newPassInfo);
+ goto exit;
}
-
- // Backoff
- newPassInfo->backoffCount = 0;
-
- // Load time
- time(&(newPassInfo->atTime));
} else {
- // Memory error. Free everything we allocated.
- free(newPassInfo->username);
- free(newPassInfo->password);
+ newPassInfo->username = NULL;
+ if(outLog.is_open()) {
+ wtimeStamp(&outLog);
+ outLog << "Failed to calc mem for user " << UserName->Buffer
+ << "; error (" << GetLastError() << ")"
<< endl;
+ }
+ free(newPassInfo);
+ goto exit;
+ }
+ len = WideCharToMultiByte(CP_UTF8, 0 /* no flags */,
+ // Length is in bytes; assume UCS2
+ UserName->Buffer, UserName->Length/2,
+ newPassInfo->username, len+1,
+ NULL, NULL);
+ if (0 == len) {
+ if(outLog.is_open()) {
+ wtimeStamp(&outLog);
+ outLog << "Failed to convert user " << UserName->Buffer
+ << "; error (" << GetLastError() << ")"
<< endl;
+ }
free(newPassInfo);
goto exit;
}
+ newPassInfo->username[len] = '\0';
+ if (Password != NULL) {
+ len = WideCharToMultiByte(CP_UTF8, 0 /* no flags */,
+ // Length is in bytes; assume UCS2
+ Password->Buffer, Password->Length/2,
+ newPassInfo->password, 0,
+ NULL, NULL);
+ if (len > 0) {
+ newPassInfo->password = (char*)malloc(len + 1);
+ if (NULL == newPassInfo->password) {
+ if(outLog.is_open()) {
+ wtimeStamp(&outLog);
+ outLog << "Failed to alloc mem for password " <<
Password->Buffer
+ << "; error (" << GetLastError() << ")"
<< endl;
+ }
+ free(newPassInfo->username);
+ free(newPassInfo);
+ goto exit;
+ }
+ } else {
+ newPassInfo->password = NULL;
+ if(outLog.is_open()) {
+ wtimeStamp(&outLog);
+ outLog << "Failed to calc mem for password " <<
Password->Buffer
+ << "; error (" << GetLastError() << ")"
<< endl;
+ }
+ free(newPassInfo->username);
+ free(newPassInfo);
+ goto exit;
+ }
+ len = WideCharToMultiByte(CP_UTF8, 0 /* no flags */,
+ // Length is in bytes; assume UCS2
+ Password->Buffer, Password->Length/2,
+ newPassInfo->password, len+1,
+ NULL, NULL);
+ if (0 == len) {
+ if(outLog.is_open()) {
+ wtimeStamp(&outLog);
+ outLog << "Failed to convert password " << Password->Buffer
+ << "; error (" << GetLastError() << ")"
<< endl;
+ }
+ free(newPassInfo->username);
+ free(newPassInfo->password);
+ free(newPassInfo);
+ goto exit;
+ }
+ newPassInfo->password[len] = '\0';
+ } else {
+ newPassInfo->password = (char*)malloc(1);
+ if (NULL == newPassInfo->password) {
+ if(outLog.is_open()) {
+ wtimeStamp(&outLog);
+ outLog << "Failed to allocate for empty password."
+ << "; error (" << GetLastError() << ")"
<< endl;
+ }
+ free(newPassInfo->username);
+ free(newPassInfo);
+ goto exit;
+ }
+ newPassInfo->password[0] = '\0';
+ }
+
+ // Backoff
+ newPassInfo->backoffCount = 0;
+
+ // Load time
+ time(&(newPassInfo->atTime));
// Fire off a thread to do the real work
passhookThreadHandle = CreateThread(NULL, 0, SavePasshookChange, newPassInfo, 0, NULL);
@@ -114,21 +195,24 @@ NTSTATUS NTAPI PasswordChangeNotify(PUNICODE_STRING UserName, ULONG
RelativeId,
// 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);
+ if(!outLog.is_open()) {
+ outLog.open("passhook.log", ios::out | ios::app);
+ }
if(outLog.is_open()) {
- timeStamp(&outLog);
+ wtimeStamp(&outLog);
outLog << "Failed to start thread. Aborting change for " <<
newPassInfo->username << endl;
}
- outLog.close();
-
// Release mutex
ReleaseMutex(passhookMutexHandle);
}
}
exit:
+ if(outLog.is_open()) {
+ outLog.close();
+ }
return STATUS_SUCCESS;
}
@@ -234,7 +318,9 @@ DWORD WINAPI SavePasshookChange( LPVOID passinfo )
}
// Close the log file before we release the mutex.
- outLog.close();
+ if(outLog.is_open()) {
+ outLog.close();
+ }
// Release the mutex for passhook.dat
ReleaseMutex(passhookMutexHandle);
diff --git a/passwordsync/passsync/syncserv.cpp b/passwordsync/passsync/syncserv.cpp
index bf3fde5..d6409cf 100644
--- a/passwordsync/passsync/syncserv.cpp
+++ b/passwordsync/passsync/syncserv.cpp
@@ -430,14 +430,15 @@ int PassSyncService::SyncPasswords()
timeStamp(&outLog);
outLog << "Multiple results not allowed: " <<
currentPassInfo->username << endl;
}
- else if(LDAP_SUCCESS == (bindRC = CanBind(dn, currentPassInfo->password)))
+ else if (LDAP_SUCCESS == (bindRC = CanBind(dn, currentPassInfo->password)))
{
if(logLevel > 0) {
timeStamp(&outLog);
outLog << "Password match, no modify performed: " <<
currentPassInfo->username << endl;
}
}
- else if(LDAP_INVALID_CREDENTIALS != bindRC)
+ else if ((LDAP_INVALID_CREDENTIALS != bindRC) &&
+ (LDAP_INAPPROPRIATE_AUTH != bindRC))
{
// password check failure.
/*
@@ -446,12 +447,16 @@ int PassSyncService::SyncPasswords()
* in the second round. The following modification invokes the
* server side's WinSync plugin to send the modify back, and this
* code is invoked as the second round. If the return code from
- * CanBind is not LDAP_INVALID_CREDENTIALS (e.g., LDAP_UNWILLING_
- * TO_PERFORM for the inactivated account), the second round Can-
- * Bind wound not return LDAP_SUCCESS even if the password is
- * correctly updated. That's said, if CanBind returns any error
- * other than LDAP_INVALID_CREDENTIALS, we should defer the pass-
- * word update.
+ * CanBind is not LDAP_INVALID_CREDENTIALS or LDAP_INAPPROPRIATE_
+ * AUTH (e.g., LDAP_UNWILLING_TO_PERFORM for the inactivated
+ * account), the second round CanBind wound not return LDAP_
+ * SUCCESS even if the password is correctly updated. That's
+ * said, if CanBind returns any error other than LDAP_INVALID_
+ * CREDENTIALS, we should defer the password update.
+ *
+ * Note: LDAP_INAPPROPRIATE_AUTH is returned when the entry has
+ * no userpassword and trying to auth with LDAP_AUTH_SIMPLE.
+ * This case, the password is to be sync'ed,
*/
timeStamp(&outLog);
outLog << "Checking password failed for remote entry: " << dn
<< endl;