Skip to content

Commit

Permalink
feat: Adding initial Windows directory security messages
Browse files Browse the repository at this point in the history 
Adding initial code to output errors about Windows directory security issues

[Seagate/openSeaChest#161]

Signed-off-by: Tyler Erickson <tyler.erickson@seagate.com>
  • Loading branch information
@vonericsen
vonericsen committed Oct 16, 2024
1 parent 65bd2ff commit cfb7c1e
Showing 1 changed file with 52 additions and 15 deletions.
67 changes: 52 additions & 15 deletions src/windows_secure_file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "string_utils.h"
#include "error_translation.h"
#include "secured_env_vars.h"
#include "io_utils.h"

#if defined (HAVE_NTIFS)
#include <ntifs.h>
Expand Down Expand Up @@ -523,11 +524,27 @@ static bool get_System_Volume(char *winSysVol, size_t winSysVolLen)
return validsystemvol;
}

static bool is_Folder_Secure(const char* securityDescriptorString, const char* dirptr)
FUNC_ATTR_PRINTF(2, 3) static void set_dir_security_output_error_message(char **outputError, const char* format, ...)
{
if (outputError != M_NULLPTR)
{
va_list args;
va_start(args, format);
int result = vasprintf(outputError, format, args);
va_end(args);
if (result < 0 && *outputError != M_NULLPTR)
{
safe_free(outputError);
}
}
}

static bool is_Folder_Secure(const char* securityDescriptorString, const char* dirptr, char **outputError)
{
bool secure = true;
if (securityDescriptorString == M_NULLPTR || dirptr == M_NULLPTR)
{
set_dir_security_output_error_message(outputError, "Invalid security descriptor string or directory given.\n");
return false;
}
DECLARE_ZERO_INIT_ARRAY(char, windowsSystemVolume, MAX_PATH);
Expand All @@ -549,36 +566,42 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
if (FALSE == ConvertStringSidToSidA(mySidStr, &mySid))
{
/* Handle Error */
set_dir_security_output_error_message(outputError, "Failed to convert current user's SID string to sid structure\n");
secure = false;
break;
}
if (FALSE == IsValidSid(mySid))
{
/* Handle Error */
set_dir_security_output_error_message(outputError, "Invalid sid\n");
secure = false;
break;
}
if (FALSE == ConvertStringSecurityDescriptorToSecurityDescriptorA(securityDescriptorString, SDDL_REVISION, &secdesc, &secdesclen))
{
/* Handle Error */
set_dir_security_output_error_message(outputError, "Failed to convert security descriptor string to security descriptor structure\n");
secure = false;
break;
}
if (FALSE == IsValidSecurityDescriptor(secdesc))
{
/* Handle Error */
set_dir_security_output_error_message(outputError, "Invalid security descriptor\n");
secure = false;
break;
}
if (FALSE == GetSecurityDescriptorOwner(secdesc, &userSid, &defaultOwner))
{
/* Handle Error */
set_dir_security_output_error_message(outputError, "Failed to get security descriptor owner\n");
secure = false;
break;
}
if (FALSE == IsValidSid(userSid))
{
/* Handle Error */
set_dir_security_output_error_message(outputError, "Invalid SID for security descriptor owner\n");
secure = false;
break;
}
Expand Down Expand Up @@ -621,13 +644,13 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
{
/* Directory is owned by someone besides user/system/administrator */
secure = false;
#if defined (_DEBUG)
char* sidString = M_NULLPTR;
if (ConvertSidToStringSidA(userSid, &sidString))
{
printf("Untrusted Owner SID: %s\n", sidString);
set_dir_security_output_error_message(outputError, "Directory (%s) owned by SID (not trusted): %s\n", dirptr, sidString);
LocalFree(sidString);
sidString = M_NULLPTR;
}
#endif //_DEBUG
break;
}
}
Expand All @@ -636,27 +659,27 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
{
/* Directory is owned by someone besides user/system/administrator */
secure = false;
#if defined (_DEBUG)
char* sidString = M_NULLPTR;
if (ConvertSidToStringSidA(userSid, &sidString))
{
printf("Untrusted Owner SID: %s\n", sidString);
set_dir_security_output_error_message(outputError, "Directory (%s) owned by SID (not trusted): %s\n", dirptr, sidString);
LocalFree(sidString);
sidString = M_NULLPTR;
}
#endif //_DEBUG
break;
}
}
else
{
/* Directory is owned by someone besides user/system/administrator */
secure = false;
#if defined (_DEBUG)
char* sidString = M_NULLPTR;
if (ConvertSidToStringSidA(userSid, &sidString))
{
printf("Untrusted Owner SID: %s\n", sidString);
set_dir_security_output_error_message(outputError, "Directory (%s) owned by SID (not trusted): %s\n", dirptr, sidString);
LocalFree(sidString);
sidString = M_NULLPTR;
}
#endif //_DEBUG
break;
}
}
Expand Down Expand Up @@ -686,20 +709,23 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
{
/* Handle Error */
secure = false;
set_dir_security_output_error_message(outputError, "Unable to retrieve DACL from security descriptor: %s\n", dirptr);
break;
}

if (FALSE == daclPresent || dacl == M_NULLPTR)
{
/* Missing DACL, so cannot verify permissions */
secure = false;
set_dir_security_output_error_message(outputError, "DACL Missing. Cannot verify permissions: %s\n", dirptr);
break;
}

if (FALSE == IsValidAcl(dacl))
{
/* Handle Error */
secure = false;
set_dir_security_output_error_message(outputError, "Invalid DACL received. Cannot verify permissions: %s\n", dirptr);
break;
}

Expand Down Expand Up @@ -728,14 +754,14 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d

if (!(allowEveryoneGroup && everyoneGroupSID != M_NULLPTR && EqualSid(aceSID, everyoneGroupSID)))
{
secure = false;
#if defined (_DEBUG)
char* sidString = M_NULLPTR;
secure = false;
if (ConvertSidToStringSidA(aceSID, &sidString))
{
printf("Untrusted SID: %s\n", sidString);
set_dir_security_output_error_message(outputError, "Directory (%s) can be accessed by SID (not trusted, must be removed to be secure): %s\n", dirptr, sidString);
LocalFree(sidString);
sidString = M_NULLPTR;
}
#endif //_DEBUG
}
#if defined (_DEBUG)
else
Expand All @@ -744,6 +770,8 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
if (ConvertSidToStringSidA(aceSID, &sidString))
{
printf("Trusted SID: %s\n", sidString);
LocalFree(sidString);
sidString = M_NULLPTR;
}
}
#endif //_DEBUG
Expand All @@ -761,6 +789,7 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
}
else
{
set_dir_security_output_error_message(outputError, "Invalid ACE in DACL. Directory (%s) cannot be trusted\n", dirptr);
secure = false;
}
}
Expand Down Expand Up @@ -946,6 +975,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
{
/* handle error */
secure = false;
set_dir_security_output_error_message(outputError, "Unable to read directory attributes: %s\n", dirptr);
if (appendedTrailingSlash)
{
safe_free(&dirptr);
Expand All @@ -956,6 +986,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
{
/* handle error */
secure = false;
set_dir_security_output_error_message(outputError, "Too many symlinks in path (>%d): %s\n", MAX_SYMLINKS_IN_PATH, dirptr);
free_File_Attributes(&attrs);
if (appendedTrailingSlash)
{
Expand Down Expand Up @@ -1005,22 +1036,26 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
}
else
{
set_dir_security_output_error_message(outputError, "Unable to allocate memory to check reparse point in path: %s\n", dirptr);
secure = false;
}
}
else
{
set_dir_security_output_error_message(outputError, "Unable to calculate memory length to check reparse point in path: %s\n", dirptr);
secure = false;
}
}
else
{
set_dir_security_output_error_message(outputError, "Unable to issue FSCTL_GET_REPASE_POINT to validate reparse point in path: %s\n", dirptr);
secure = false;
}
safe_free_reparse_data_buf(&reparseData);
}
else
{
set_dir_security_output_error_message(outputError, "Unable to allocate memory to check reparse point in path: %s\n", dirptr);
secure = false;
}
CloseHandle(link);
Expand All @@ -1039,6 +1074,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
}
else
{
set_dir_security_output_error_message(outputError, "Unable to open handle to reparse point in path: %s\n", dirptr);
secure = false;
free_File_Attributes(&attrs);
if (appendedTrailingSlash)
Expand All @@ -1053,6 +1089,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
{
/* Not a directory */
secure = false;
set_dir_security_output_error_message(outputError, "%s is not a directory. Cannot validate as part of path.\n", dirptr);
free_File_Attributes(&attrs);
if (appendedTrailingSlash)
{
Expand All @@ -1061,7 +1098,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
break;
}

secure = is_Folder_Secure(attrs->winSecurityDescriptor, dirptr);
secure = is_Folder_Secure(attrs->winSecurityDescriptor, dirptr, outputError);
if (appendedTrailingSlash)
{
safe_free(&dirptr);
Expand Down

0 comments on commit cfb7c1e

Please sign in to comment.