Updates resulting from self-code review.
This commit is contained in:
		| @@ -194,6 +194,9 @@ ObtainSessionToken( | |||||||
|             if (pRespMsg) |             if (pRespMsg) | ||||||
|                free(pRespMsg); |                free(pRespMsg); | ||||||
|  |  | ||||||
|  |             // Clear and free the memory associated with the request message since | ||||||
|  |             // it may contain sensitive information. | ||||||
|  |             memset(pReqMsg, 0, strlen(pReqMsg)); | ||||||
|             free(pReqMsg); |             free(pReqMsg); | ||||||
|          } |          } | ||||||
|          else |          else | ||||||
| @@ -223,6 +226,8 @@ ObtainSessionToken( | |||||||
|          } |          } | ||||||
|  |  | ||||||
|          // Free up the buffer associated with the authentication mechanism token |          // Free up the buffer associated with the authentication mechanism token | ||||||
|  |          // after clearing it since it may contain sensitive information. | ||||||
|  |          memset(pAuthMechToken, 0, strlen(pAuthMechToken)); | ||||||
|          free(pAuthMechToken); |          free(pAuthMechToken); | ||||||
|       } |       } | ||||||
|       else |       else | ||||||
|   | |||||||
| @@ -101,7 +101,6 @@ AuthTokenIf_GetAuthToken( | |||||||
|    TimeStamp         expiry; |    TimeStamp         expiry; | ||||||
|    CredHandle        hCredentials = {0}; |    CredHandle        hCredentials = {0}; | ||||||
|  |  | ||||||
|  |  | ||||||
|    DbgTrace(1, "-AuthTokenIf_GetAuthToken- Start\n", 0); |    DbgTrace(1, "-AuthTokenIf_GetAuthToken- Start\n", 0); | ||||||
|  |  | ||||||
|    // Validate input parameters |    // Validate input parameters | ||||||
| @@ -211,7 +210,9 @@ AuthTokenIf_GetAuthToken( | |||||||
|             // Return the actual size or the size required |             // Return the actual size or the size required | ||||||
|             *pTokenBufLen = encodedTokenLen; |             *pTokenBufLen = encodedTokenLen; | ||||||
|  |  | ||||||
|             // Free the buffer containing the encoded token |             // Free the buffer containing the encoded token after clearing | ||||||
|  |             // its memory to avoid leaking sensitive information. | ||||||
|  |             memset(pEncodedToken, 0, strlen(pEncodedToken)); | ||||||
|             free(pEncodedToken); |             free(pEncodedToken); | ||||||
|          } |          } | ||||||
|  |  | ||||||
| @@ -229,7 +230,10 @@ AuthTokenIf_GetAuthToken( | |||||||
|  |  | ||||||
|       // Free any buffer associated with the sendToken |       // Free any buffer associated with the sendToken | ||||||
|       if (sendTok.pvBuffer) |       if (sendTok.pvBuffer) | ||||||
|  |       { | ||||||
|  |          memset(sendTok.pvBuffer, 0, sendTok.cbBuffer); | ||||||
|          FreeContextBuffer(sendTok.pvBuffer); |          FreeContextBuffer(sendTok.pvBuffer); | ||||||
|  |       } | ||||||
|  |  | ||||||
|       // Free the credential handle obtained |       // Free the credential handle obtained | ||||||
|       FreeCredentialsHandle(&hCredentials); |       FreeCredentialsHandle(&hCredentials); | ||||||
|   | |||||||
| @@ -181,6 +181,10 @@ GetUserCredentials( | |||||||
|       DbgTrace(0, "-GetUserCredentials- Failed to obtain credentials for pw authentication\n", 0); |       DbgTrace(0, "-GetUserCredentials- Failed to obtain credentials for pw authentication\n", 0); | ||||||
|    } |    } | ||||||
|  |  | ||||||
|  |    // Clear out the credential structure to make sure that we are not leaving sensitive | ||||||
|  |    // information on the stack. | ||||||
|  |    memset(&credential, 0, sizeof(credential)); | ||||||
|  |  | ||||||
|    // Return the buffers to the caller if successful |    // Return the buffers to the caller if successful | ||||||
|    if (CASA_SUCCESS(retStatus)) |    if (CASA_SUCCESS(retStatus)) | ||||||
|    { |    { | ||||||
| @@ -324,11 +328,15 @@ AuthTokenIf_GetAuthToken( | |||||||
|             // Return the actual size or the size required |             // Return the actual size or the size required | ||||||
|             *pTokenBufLen = encodedTokenLen; |             *pTokenBufLen = encodedTokenLen; | ||||||
|  |  | ||||||
|             // Free the buffer containing the encoded token |             // Free the buffer containing the encoded token after clearing | ||||||
|  |             // it to avoid leaking sensitive information. | ||||||
|  |             memset(pEncodedToken, 0, strlen(pEncodedToken)); | ||||||
|             free(pEncodedToken); |             free(pEncodedToken); | ||||||
|          } |          } | ||||||
|  |  | ||||||
|          // Free the buffer allocated for the token |          // Free the buffer allocated for the token after clearing it | ||||||
|  |          // to avoid leaving sensitive information behind. | ||||||
|  |          memset(pToken, 0, strlen(pToken)); | ||||||
|          free(pToken); |          free(pToken); | ||||||
|       } |       } | ||||||
|       else |       else | ||||||
| @@ -339,8 +347,9 @@ AuthTokenIf_GetAuthToken( | |||||||
|                                      CASA_STATUS_INSUFFICIENT_RESOURCES); |                                      CASA_STATUS_INSUFFICIENT_RESOURCES); | ||||||
|       } |       } | ||||||
|  |  | ||||||
|       // Free allocated buffers |       // Free allocated buffers after clearing memory holding the password | ||||||
|       free(pUsername); |       free(pUsername); | ||||||
|  |       memset(pPassword, 0, strlen(pPassword)); | ||||||
|       free(pPassword); |       free(pPassword); | ||||||
|    } |    } | ||||||
|    else |    else | ||||||
|   | |||||||
| @@ -128,7 +128,7 @@ BOOL APIENTRY DllMain( | |||||||
| //=======================================================================-- | //=======================================================================-- | ||||||
| { | { | ||||||
|    BOOL  retStatus = TRUE; |    BOOL  retStatus = TRUE; | ||||||
|    char  programFilesFolder[MAX_PATH]; |    char  programFilesFolder[MAX_PATH] = {0}; | ||||||
|  |  | ||||||
|    switch (ul_reason_for_call) |    switch (ul_reason_for_call) | ||||||
|    { |    { | ||||||
|   | |||||||
| @@ -54,12 +54,12 @@ static | |||||||
| HANDLE   hNormalizedHostNameCacheMutex; | HANDLE   hNormalizedHostNameCacheMutex; | ||||||
|  |  | ||||||
| // Client configuration file folder | // Client configuration file folder | ||||||
| char  clientConfigFolder[MAX_PATH]; |  | ||||||
| char  clientConfigFolderPartialPath[] = "Novell\\Casa\\Etc\\Auth"; | char  clientConfigFolderPartialPath[] = "Novell\\Casa\\Etc\\Auth"; | ||||||
|  | char  clientConfigFolder[MAX_PATH + sizeof(clientConfigFolderPartialPath)]; | ||||||
|  |  | ||||||
| // Authentication mechanism configuration file folder | // Authentication mechanism configuration file folder | ||||||
| char  mechConfigFolder[MAX_PATH]; |  | ||||||
| char  mechConfigFolderPartialPath[] = "Novell\\Casa\\Etc\\Auth\\Mechanisms"; | char  mechConfigFolderPartialPath[] = "Novell\\Casa\\Etc\\Auth\\Mechanisms"; | ||||||
|  | char  mechConfigFolder[MAX_PATH + sizeof(mechConfigFolderPartialPath)]; | ||||||
|  |  | ||||||
| // Path separator | // Path separator | ||||||
| char  pathCharString[] = "\\"; | char  pathCharString[] = "\\"; | ||||||
| @@ -81,6 +81,8 @@ CreateUserMutex( | |||||||
| // L2 | // L2 | ||||||
| //=======================================================================-- | //=======================================================================-- | ||||||
| { | { | ||||||
|  | #define USER_MUTEX_NAME_FMT_STRING "Global\\CASA_Auth_Mutex_%s" | ||||||
|  |  | ||||||
|    CasaStatus  retStatus = CASA_STATUS_SUCCESS; |    CasaStatus  retStatus = CASA_STATUS_SUCCESS; | ||||||
|    char        *pUsername = NULL; |    char        *pUsername = NULL; | ||||||
|    DWORD       nameLength = 0; |    DWORD       nameLength = 0; | ||||||
| @@ -99,18 +101,22 @@ CreateUserMutex( | |||||||
|          if (GetUserName(pUsername, &nameLength)) |          if (GetUserName(pUsername, &nameLength)) | ||||||
|          { |          { | ||||||
|             SECURITY_ATTRIBUTES  mutexAttributes; |             SECURITY_ATTRIBUTES  mutexAttributes; | ||||||
|             char                 mutexName[256]; |             char                 *pMutexName; | ||||||
|  |  | ||||||
|  |             // Allocate a buffer to hold the mutex name | ||||||
|  |             pMutexName = (char*) malloc(sizeof(USER_MUTEX_NAME_FMT_STRING) + nameLength); | ||||||
|  |             if (pMutexName) | ||||||
|  |             { | ||||||
|                // Now lets create a global semaphore for the |                // Now lets create a global semaphore for the | ||||||
|                // user and allow its handle to be inherited. |                // user and allow its handle to be inherited. | ||||||
|                mutexAttributes.nLength = sizeof(mutexAttributes); |                mutexAttributes.nLength = sizeof(mutexAttributes); | ||||||
|                mutexAttributes.lpSecurityDescriptor = NULL; |                mutexAttributes.lpSecurityDescriptor = NULL; | ||||||
|                mutexAttributes.bInheritHandle = TRUE; |                mutexAttributes.bInheritHandle = TRUE; | ||||||
|             if (sprintf(mutexName, "Global\\CASA_Auth_Mutex_%s", pUsername) != -1) |                if (sprintf(pMutexName, USER_MUTEX_NAME_FMT_STRING, pUsername) != -1) | ||||||
|                { |                { | ||||||
|                   *phMutex = CreateMutex(&mutexAttributes, |                   *phMutex = CreateMutex(&mutexAttributes, | ||||||
|                                          FALSE, |                                          FALSE, | ||||||
|                                       mutexName); |                                          pMutexName); | ||||||
|                   if (*phMutex == NULL) |                   if (*phMutex == NULL) | ||||||
|                   { |                   { | ||||||
|                      DbgTrace(0, "-CreateUserMutex- CreateMutex failed, error = %d\n", GetLastError()); |                      DbgTrace(0, "-CreateUserMutex- CreateMutex failed, error = %d\n", GetLastError()); | ||||||
| @@ -126,6 +132,17 @@ CreateUserMutex( | |||||||
|                                               CASA_FACILITY_AUTHTOKEN, |                                               CASA_FACILITY_AUTHTOKEN, | ||||||
|                                               CASA_STATUS_UNSUCCESSFUL); |                                               CASA_STATUS_UNSUCCESSFUL); | ||||||
|                } |                } | ||||||
|  |  | ||||||
|  |                // Free the buffer used to hold the user mutex name | ||||||
|  |                free(pMutexName); | ||||||
|  |             } | ||||||
|  |             else | ||||||
|  |             { | ||||||
|  |                DbgTrace(0, "-CreateUserMutex- Buffer allocation failure\n", 0); | ||||||
|  |                retStatus = CasaStatusBuild(CASA_SEVERITY_ERROR, | ||||||
|  |                                            CASA_FACILITY_AUTHTOKEN, | ||||||
|  |                                            CASA_STATUS_INSUFFICIENT_RESOURCES); | ||||||
|  |             } | ||||||
|          } |          } | ||||||
|          else |          else | ||||||
|          { |          { | ||||||
| @@ -415,10 +432,14 @@ NormalizeHostName( | |||||||
|  |  | ||||||
|             // Now try to resolve the normalized name |             // Now try to resolve the normalized name | ||||||
|             pLookupResult = gethostbyname(pHostName); |             pLookupResult = gethostbyname(pHostName); | ||||||
|             if (pLookupResult && pLookupResult->h_addrtype == AF_INET) |             if (pLookupResult | ||||||
|  |                 && pLookupResult->h_addrtype == AF_INET | ||||||
|  |                 && pLookupResult->h_length > 0 | ||||||
|  |                 && pLookupResult->h_addr_list[0] != NULL) | ||||||
|  |             { | ||||||
|  |                char *pDnsHostName = (char*) malloc(NI_MAXHOST + 1); | ||||||
|  |                if (pDnsHostName) | ||||||
|                { |                { | ||||||
|                char  dnsHostName[NI_MAXHOST]; |  | ||||||
|  |  | ||||||
|                   // Set up a sockaddr structure |                   // Set up a sockaddr structure | ||||||
|                   sockAddr.sin_family = AF_INET; |                   sockAddr.sin_family = AF_INET; | ||||||
|                   sockAddr.sin_addr.S_un.S_addr = *((int*) pLookupResult->h_addr_list[0]); |                   sockAddr.sin_addr.S_un.S_addr = *((int*) pLookupResult->h_addr_list[0]); | ||||||
| @@ -426,19 +447,19 @@ NormalizeHostName( | |||||||
|                   // Now try to resolve the name using DNS |                   // Now try to resolve the name using DNS | ||||||
|                   if (getnameinfo((const struct sockaddr*) &sockAddr, |                   if (getnameinfo((const struct sockaddr*) &sockAddr, | ||||||
|                                   sizeof(sockAddr), |                                   sizeof(sockAddr), | ||||||
|                                dnsHostName, |                                   pDnsHostName, | ||||||
|                                sizeof(dnsHostName), |                                   NI_MAXHOST, | ||||||
|                                   NULL, |                                   NULL, | ||||||
|                                   0, |                                   0, | ||||||
|                                   NI_NAMEREQD) == 0) |                                   NI_NAMEREQD) == 0) | ||||||
|                   { |                   { | ||||||
|                      // We resolved the address to a DNS name, use it as the normalized name. |                      // We resolved the address to a DNS name, use it as the normalized name. | ||||||
|                   pEntry->buffLengthRequired = (int) strlen(dnsHostName) + 1; |                      pEntry->buffLengthRequired = (int) strlen(pDnsHostName) + 1; | ||||||
|                      pEntry->pNormalizedHostName = (char*) malloc(pEntry->buffLengthRequired); |                      pEntry->pNormalizedHostName = (char*) malloc(pEntry->buffLengthRequired); | ||||||
|                      if (pEntry->pNormalizedHostName) |                      if (pEntry->pNormalizedHostName) | ||||||
|                      { |                      { | ||||||
|                         // Copy the dns name |                         // Copy the dns name | ||||||
|                      strcpy(pEntry->pNormalizedHostName, dnsHostName); |                         strcpy(pEntry->pNormalizedHostName, pDnsHostName); | ||||||
|                      } |                      } | ||||||
|                      else |                      else | ||||||
|                      { |                      { | ||||||
| @@ -463,6 +484,14 @@ NormalizeHostName( | |||||||
|                         DbgTrace(0, "-NormalizeHostName- Buffer allocation error\n", 0); |                         DbgTrace(0, "-NormalizeHostName- Buffer allocation error\n", 0); | ||||||
|                      } |                      } | ||||||
|                   } |                   } | ||||||
|  |  | ||||||
|  |                   // Free the buffer allocated to hold the DNS name | ||||||
|  |                   free(pDnsHostName); | ||||||
|  |                } | ||||||
|  |                else | ||||||
|  |                { | ||||||
|  |                   DbgTrace(0, "-NormalizeHostName- Buffer allocation failure\n", 0); | ||||||
|  |                } | ||||||
|             } |             } | ||||||
|             else |             else | ||||||
|             { |             { | ||||||
|   | |||||||
| @@ -356,10 +356,14 @@ InternalRpc( | |||||||
| // L2 | // L2 | ||||||
| //=======================================================================-- | //=======================================================================-- | ||||||
| { | { | ||||||
|  | #define RPC_TARGET_FMT_STRING "CasaAuthTokenSvc/Rpc?method=%s" | ||||||
|  |  | ||||||
|  | #ifndef CASA_STATUS_INVALID_SERVER_CERTIFICATE | ||||||
| #define CASA_STATUS_INVALID_SERVER_CERTIFICATE CASA_STATUS_UNSUCCESSFUL // temporary until casa_status.h is updated | #define CASA_STATUS_INVALID_SERVER_CERTIFICATE CASA_STATUS_UNSUCCESSFUL // temporary until casa_status.h is updated | ||||||
|  | #endif | ||||||
|  |  | ||||||
|    CasaStatus  retStatus = CASA_STATUS_SUCCESS; |    CasaStatus  retStatus = CASA_STATUS_SUCCESS; | ||||||
|    char        rpcTarget[256]; |    char        *pRpcTarget; | ||||||
|    LPWSTR      pWideRpcTarget; |    LPWSTR      pWideRpcTarget; | ||||||
|    int         wideRpcTargetLen; |    int         wideRpcTargetLen; | ||||||
|    WCHAR       sendHeaders[] = L"Content-Type: text/html"; |    WCHAR       sendHeaders[] = L"Content-Type: text/html"; | ||||||
| @@ -373,9 +377,12 @@ InternalRpc( | |||||||
|    *ppResponseData = NULL; |    *ppResponseData = NULL; | ||||||
|  |  | ||||||
|    // Create rpc target string and convert it to a wide string |    // Create rpc target string and convert it to a wide string | ||||||
|    sprintf(rpcTarget, "CasaAuthTokenSvc/Rpc?method=%s", pMethod); |    pRpcTarget = (char*) malloc(sizeof(RPC_TARGET_FMT_STRING) + strlen(pMethod)); | ||||||
|    retStatus = CopyMultiToWideAlloc(rpcTarget, |    if (pRpcTarget) | ||||||
|                                     (int) strlen(rpcTarget), |    { | ||||||
|  |       sprintf(pRpcTarget, RPC_TARGET_FMT_STRING, pMethod); | ||||||
|  |       retStatus = CopyMultiToWideAlloc(pRpcTarget, | ||||||
|  |                                        (int) strlen(pRpcTarget), | ||||||
|                                        &pWideRpcTarget, |                                        &pWideRpcTarget, | ||||||
|                                        &wideRpcTargetLen); |                                        &wideRpcTargetLen); | ||||||
|       if (CASA_SUCCESS(retStatus)) |       if (CASA_SUCCESS(retStatus)) | ||||||
| @@ -711,6 +718,17 @@ InternalRpc( | |||||||
|          DbgTrace(0, "-InternalRpc- Error converting method name to wide string\n", 0); |          DbgTrace(0, "-InternalRpc- Error converting method name to wide string\n", 0); | ||||||
|       } |       } | ||||||
|  |  | ||||||
|  |       // Free buffer used to hold the rpc target string | ||||||
|  |       free(pRpcTarget); | ||||||
|  |    } | ||||||
|  |    else | ||||||
|  |    { | ||||||
|  |       DbgTrace(0, "-InternalRpc- Buffer allocation failure\n", 0); | ||||||
|  |       retStatus = CasaStatusBuild(CASA_SEVERITY_ERROR, | ||||||
|  |                                   CASA_FACILITY_AUTHTOKEN, | ||||||
|  |                                   CASA_STATUS_INSUFFICIENT_RESOURCES); | ||||||
|  |    } | ||||||
|  |  | ||||||
|    DbgTrace(1, "-InternalRpc- End, retStatus = %d\n", retStatus); |    DbgTrace(1, "-InternalRpc- End, retStatus = %d\n", retStatus); | ||||||
|  |  | ||||||
|    return retStatus; |    return retStatus; | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user