Windows - fix kfwlogon for Windows 2000

    Windows 2000 does not support the ability to generate SIDs
    from symbolic names.

    Add more debugging and error condition checks.

ticket: new
tags: pullup

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@18387 dc483132-0cff-0310-8789-dd5450dbe970

2 files changed, 80 insertions, 31 deletions

diff --git a/src/windows/kfwlogon/kfwcommon.c b/src/windows/kfwlogon/kfwcommon.c
index a4263c8382..14beef966b 100644
--- a/src/windows/kfwlogon/kfwcommon.c
+++ b/src/windows/kfwlogon/kfwcommon.c
@@ -1,5 +1,5 @@
 /*

-Copyright 2005 by the Massachusetts Institute of Technology

+Copyright 2005,2006 by the Massachusetts Institute of Technology

 

 All rights reserved.

 

@@ -765,15 +765,25 @@ int KFW_set_ccache_dacl(char *filename, HANDLE hUserToken)
 {

     // SID_IDENTIFIER_AUTHORITY authority = SECURITY_NT_SID_AUTHORITY;

     PSID pSystemSID = NULL;

-    DWORD SystemSIDlength, UserSIDlength;

+    DWORD SystemSIDlength = 0, UserSIDlength = 0;

     PACL ccacheACL = NULL;

-    DWORD ccacheACLlength;

+    DWORD ccacheACLlength = 0;

     PTOKEN_USER pTokenUser = NULL;

     DWORD retLen;

+    DWORD gle;

     int ret = 0;  

 

+    if (!filename) {

+	DebugEvent0("KFW_set_ccache_dacl - invalid parms");

+	return 1;

+    }

+

     /* Get System SID */

-    ConvertStringSidToSid(SDDL_LOCAL_SYSTEM, &pSystemSID);

+    if (!ConvertStringSidToSid("S-1-5-18", &pSystemSID)) {

+	DebugEvent("KFW_set_ccache_dacl - ConvertStringSidToSid GLE = 0x%x", GetLastError());

+	ret = 1;

+	goto cleanup;

+    }

 

     /* Create ACL */

     SystemSIDlength = GetLengthSid(pSystemSID);

@@ -801,7 +811,13 @@ int KFW_set_ccache_dacl(char *filename, HANDLE hUserToken)
 	}

     }

 

-    ccacheACL = GlobalAlloc(GMEM_FIXED, ccacheACLlength);

+    ccacheACL = (PACL) LocalAlloc(LPTR, ccacheACLlength);

+    if (!ccacheACL) {

+	DebugEvent("KFW_set_ccache_dacl - LocalAlloc GLE = 0x%x", GetLastError());

+	ret = 1;

+	goto cleanup;

+    }

+

     InitializeAcl(ccacheACL, ccacheACLlength, ACL_REVISION);

     AddAccessAllowedAceEx(ccacheACL, ACL_REVISION, 0,

                          STANDARD_RIGHTS_ALL | SPECIFIC_RIGHTS_ALL,

@@ -816,8 +832,10 @@ int KFW_set_ccache_dacl(char *filename, HANDLE hUserToken)
 				   NULL, 

 				   ccacheACL,

 				   NULL)) {

-	    DebugEvent("SetNamedSecurityInfo DACL failed: GLE = 0x%lX", GetLastError());

-	    ret = 1;

+	    gle = GetLastError();

+	    DebugEvent("SetNamedSecurityInfo DACL failed: GLE = 0x%lX", gle);

+	    if (gle != ERROR_NO_TOKEN)

+		ret = 1;

 	}

 	if (!SetNamedSecurityInfo( filename, SE_FILE_OBJECT,

 				   OWNER_SECURITY_INFORMATION,

@@ -825,8 +843,10 @@ int KFW_set_ccache_dacl(char *filename, HANDLE hUserToken)
 				   NULL, 

 				   NULL,

 				   NULL)) {

-	    DebugEvent("SetNamedSecurityInfo Owner failed: GLE = 0x%lX", GetLastError());

-	    ret = 1;

+	    gle = GetLastError();

+	    DebugEvent("SetNamedSecurityInfo DACL failed: GLE = 0x%lX", gle);

+	    if (gle != ERROR_NO_TOKEN)

+		ret = 1;

 	}

     } else {

 	if (!SetNamedSecurityInfo( filename, SE_FILE_OBJECT,

@@ -835,17 +855,20 @@ int KFW_set_ccache_dacl(char *filename, HANDLE hUserToken)
 				   NULL, 

 				   ccacheACL,

 				   NULL)) {

-	    DebugEvent("SetNamedSecurityInfo failed: GLE = 0x%lX", GetLastError());

-	    ret = 1;

+	    gle = GetLastError();

+	    DebugEvent("SetNamedSecurityInfo DACL failed: GLE = 0x%lX", gle);

+	    if (gle != ERROR_NO_TOKEN)

+		ret = 1;

 	}

     }

 

+  cleanup:

     if (pSystemSID)

 	LocalFree(pSystemSID);

     if (pTokenUser)

 	LocalFree(pTokenUser);

     if (ccacheACL)

-	GlobalFree(ccacheACL);

+	LocalFree(ccacheACL);

     return ret;

 }

 

@@ -853,21 +876,29 @@ int KFW_obtain_user_temp_directory(HANDLE hUserToken, char *newfilename, int siz
 {

     int  retval = 0;

     DWORD dwSize = size-1;	/* leave room for nul */

+    DWORD dwLen  = 0;

+

+    if (!hUserToken || !newfilename || size <= 0)

+	return;

 

     *newfilename = '\0';

 

-    if ( !ExpandEnvironmentStringsForUser(hUserToken, "%TEMP%", newfilename, size) &&

-	 !ExpandEnvironmentStringsForUser(hUserToken, "%TMP%", newfilename, size))

+    dwLen = ExpandEnvironmentStringsForUser(hUserToken, "%TEMP%", newfilename, dwSize);

+    if ( !dwLen || dwLen > dwSize )

+	dwLen = ExpandEnvironmentStringsForUser(hUserToken, "%TMP%", newfilename, dwSize);

+    if ( !dwLen || dwLen > dwSize )

 	return 1;

+

+    newfilename[dwSize] = '\0';

     return 0;

 }

 

 void

 KFW_copy_cache_to_system_file(char * user, char * szLogonId)

 {

-    char filename[256];

+    char filename[MAX_PATH] = "";

     DWORD count;

-    char cachename[264] = "FILE:";

+    char cachename[MAX_PATH + 8] = "FILE:";

     krb5_context		ctx = 0;

     krb5_error_code		code;

     krb5_principal              princ = 0;

@@ -875,7 +906,7 @@ KFW_copy_cache_to_system_file(char * user, char * szLogonId)
     krb5_ccache                 ncc = 0;

     PSECURITY_ATTRIBUTES        pSA = NULL;

     

-    if (!pkrb5_init_context)

+    if (!pkrb5_init_context || !user || !szLogonId)

         return;

 

     count = GetEnvironmentVariable("TEMP", filename, sizeof(filename));

@@ -913,7 +944,8 @@ KFW_copy_cache_to_system_file(char * user, char * szLogonId)
     code = pkrb5_cc_initialize(ctx, ncc, princ);

     if (code) goto cleanup;

 

-    KFW_set_ccache_dacl(filename, NULL);

+    code = KFW_set_ccache_dacl(filename, NULL);

+    if (code) goto cleanup;

 

     code = pkrb5_cc_copy_creds(ctx,cc,ncc);

 

@@ -938,7 +970,7 @@ KFW_copy_cache_to_system_file(char * user, char * szLogonId)
 int

 KFW_copy_file_cache_to_default_cache(char * filename)

 {

-    char cachename[264] = "FILE:";

+    char cachename[MAX_PATH + 8] = "FILE:";

     krb5_context		ctx = 0;

     krb5_error_code		code;

     krb5_principal              princ = 0;

@@ -946,10 +978,10 @@ KFW_copy_file_cache_to_default_cache(char * filename)
     krb5_ccache                 ncc = 0;

     int retval = 1;

 

-    if (!pkrb5_init_context)

+    if (!pkrb5_init_context || !filename)

         return 1;

 

-    if ( strlen(filename) + 6 > sizeof(cachename) )

+    if ( strlen(filename) + sizeof("FILE:") > sizeof(cachename) )

         return 1;

 

     strcat(cachename, filename);

diff --git a/src/windows/kfwlogon/kfwlogon.c b/src/windows/kfwlogon/kfwlogon.c
index e815f73e00..6dcd998708 100644
--- a/src/windows/kfwlogon/kfwlogon.c
+++ b/src/windows/kfwlogon/kfwlogon.c
@@ -101,8 +101,8 @@ UnicodeStringToANSI(UNICODE_STRING uInputString, LPSTR lpszOutputString, int nOu
         lpszOutputString[min(uInputString.Length/2,nOutStringLen-1)] = '\0';

         return TRUE;

     }

-    else

-        lpszOutputString[0] = '\0';

+

+    lpszOutputString[0] = '\0';

     return FALSE;

 }  // UnicodeStringToANSI

 

@@ -163,9 +163,10 @@ DWORD APIENTRY NPLogonNotify(
     /* Convert from Unicode to ANSI */

 

     /*TODO: Use SecureZeroMemory to erase passwords */

-    UnicodeStringToANSI(IL->UserName, uname, MAX_USERNAME_LENGTH);

-    UnicodeStringToANSI(IL->Password, password, MAX_PASSWORD_LENGTH);

-    UnicodeStringToANSI(IL->LogonDomainName, logonDomain, MAX_DOMAIN_LENGTH);

+    if (!UnicodeStringToANSI(IL->UserName, uname, MAX_USERNAME_LENGTH) ||

+	!UnicodeStringToANSI(IL->Password, password, MAX_PASSWORD_LENGTH) ||

+	!UnicodeStringToANSI(IL->LogonDomainName, logonDomain, MAX_DOMAIN_LENGTH))

+	return 0;

 

     /* Make sure AD-DOMAINS sent from login that is sent to us is stripped */

     ctemp = strchr(uname, '@');

@@ -291,11 +292,12 @@ VOID KFW_Logon_Event( PWLX_NOTIFICATION_INFO pInfo )
     char szPath[MAX_PATH] = "";

     char szLogonId[128] = "";

     DWORD count;

-    char filename[256];

-    char newfilename[256];

-    char commandline[512];

+    char filename[MAX_PATH] = "";

+    char newfilename[MAX_PATH] = "";

+    char commandline[MAX_PATH+256] = "";

     STARTUPINFO startupinfo;

     PROCESS_INFORMATION procinfo;

+    HANDLE hf = NULL;

 

     LUID LogonId = {0, 0};

     PSECURITY_LOGON_SESSION_DATA pLogonSessionData = NULL;

@@ -330,9 +332,24 @@ VOID KFW_Logon_Event( PWLX_NOTIFICATION_INFO pInfo )
     strcat(filename, "\\");

     strcat(filename, szLogonId);    

 

-    KFW_set_ccache_dacl(filename, pInfo->hToken);

+    hf = CreateFile(filename, FILE_ALL_ACCESS, 0, NULL, OPEN_EXISTING, 

+		    FILE_ATTRIBUTE_NORMAL, NULL);

+    if (hf == INVALID_HANDLE_VALUE) {

+        DebugEvent0("KFW_Logon_Event - file cannot be opened");

+	return;

+    }

+    CloseHandle(hf);

+

+    if (KFW_set_ccache_dacl(filename, pInfo->hToken)) {

+        DebugEvent0("KFW_Logon_Event - unable to set dacl");

+	DeleteFile(filename);

+	return;

+    }

 

-    KFW_obtain_user_temp_directory(pInfo->hToken, newfilename, sizeof(newfilename));

+    if (KFW_obtain_user_temp_directory(pInfo->hToken, newfilename, sizeof(newfilename))) {

+        DebugEvent0("KFW_Logon_Event - unable to obtain temp directory");

+	return;

+    }

 

     if ( strlen(newfilename) + strlen(szLogonId) + 2 > sizeof(newfilename) ) {

         DebugEvent0("KFW_Logon_Event - new filename too long");