Skip to content

Commit cfb7c1e

Browse files
vonericsenvonericsen
committed
feat: Adding initial Windows directory security messages
Adding initial code to output errors about Windows directory security issues [Seagate/openSeaChest#161] Signed-off-by: Tyler Erickson <[email protected]>
1 parent 65bd2ff commit cfb7c1e

File tree

1 file changed

+52
-15
lines changed

1 file changed

+52
-15
lines changed

‎src/windows_secure_file.c

+52-15
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "string_utils.h"
2929
#include "error_translation.h"
3030
#include "secured_env_vars.h"
31+
#include "io_utils.h"
3132

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

526-
static bool is_Folder_Secure(const char* securityDescriptorString, const char* dirptr)
527+
FUNC_ATTR_PRINTF(2, 3) static void set_dir_security_output_error_message(char **outputError, const char* format, ...)
528+
{
529+
if (outputError != M_NULLPTR)
530+
{
531+
va_list args;
532+
va_start(args, format);
533+
int result = vasprintf(outputError, format, args);
534+
va_end(args);
535+
if (result < 0 && *outputError != M_NULLPTR)
536+
{
537+
safe_free(outputError);
538+
}
539+
}
540+
}
541+
542+
static bool is_Folder_Secure(const char* securityDescriptorString, const char* dirptr, char **outputError)
527543
{
528544
bool secure = true;
529545
if (securityDescriptorString == M_NULLPTR || dirptr == M_NULLPTR)
530546
{
547+
set_dir_security_output_error_message(outputError, "Invalid security descriptor string or directory given.\n");
531548
return false;
532549
}
533550
DECLARE_ZERO_INIT_ARRAY(char, windowsSystemVolume, MAX_PATH);
@@ -549,36 +566,42 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
549566
if (FALSE == ConvertStringSidToSidA(mySidStr, &mySid))
550567
{
551568
/* Handle Error */
569+
set_dir_security_output_error_message(outputError, "Failed to convert current user's SID string to sid structure\n");
552570
secure = false;
553571
break;
554572
}
555573
if (FALSE == IsValidSid(mySid))
556574
{
557575
/* Handle Error */
576+
set_dir_security_output_error_message(outputError, "Invalid sid\n");
558577
secure = false;
559578
break;
560579
}
561580
if (FALSE == ConvertStringSecurityDescriptorToSecurityDescriptorA(securityDescriptorString, SDDL_REVISION, &secdesc, &secdesclen))
562581
{
563582
/* Handle Error */
583+
set_dir_security_output_error_message(outputError, "Failed to convert security descriptor string to security descriptor structure\n");
564584
secure = false;
565585
break;
566586
}
567587
if (FALSE == IsValidSecurityDescriptor(secdesc))
568588
{
569589
/* Handle Error */
590+
set_dir_security_output_error_message(outputError, "Invalid security descriptor\n");
570591
secure = false;
571592
break;
572593
}
573594
if (FALSE == GetSecurityDescriptorOwner(secdesc, &userSid, &defaultOwner))
574595
{
575596
/* Handle Error */
597+
set_dir_security_output_error_message(outputError, "Failed to get security descriptor owner\n");
576598
secure = false;
577599
break;
578600
}
579601
if (FALSE == IsValidSid(userSid))
580602
{
581603
/* Handle Error */
604+
set_dir_security_output_error_message(outputError, "Invalid SID for security descriptor owner\n");
582605
secure = false;
583606
break;
584607
}
@@ -621,13 +644,13 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
621644
{
622645
/* Directory is owned by someone besides user/system/administrator */
623646
secure = false;
624-
#if defined (_DEBUG)
625647
char* sidString = M_NULLPTR;
626648
if (ConvertSidToStringSidA(userSid, &sidString))
627649
{
628-
printf("Untrusted Owner SID: %s\n", sidString);
650+
set_dir_security_output_error_message(outputError, "Directory (%s) owned by SID (not trusted): %s\n", dirptr, sidString);
651+
LocalFree(sidString);
652+
sidString = M_NULLPTR;
629653
}
630-
#endif //_DEBUG
631654
break;
632655
}
633656
}
@@ -636,27 +659,27 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
636659
{
637660
/* Directory is owned by someone besides user/system/administrator */
638661
secure = false;
639-
#if defined (_DEBUG)
640662
char* sidString = M_NULLPTR;
641663
if (ConvertSidToStringSidA(userSid, &sidString))
642664
{
643-
printf("Untrusted Owner SID: %s\n", sidString);
665+
set_dir_security_output_error_message(outputError, "Directory (%s) owned by SID (not trusted): %s\n", dirptr, sidString);
666+
LocalFree(sidString);
667+
sidString = M_NULLPTR;
644668
}
645-
#endif //_DEBUG
646669
break;
647670
}
648671
}
649672
else
650673
{
651674
/* Directory is owned by someone besides user/system/administrator */
652675
secure = false;
653-
#if defined (_DEBUG)
654676
char* sidString = M_NULLPTR;
655677
if (ConvertSidToStringSidA(userSid, &sidString))
656678
{
657-
printf("Untrusted Owner SID: %s\n", sidString);
679+
set_dir_security_output_error_message(outputError, "Directory (%s) owned by SID (not trusted): %s\n", dirptr, sidString);
680+
LocalFree(sidString);
681+
sidString = M_NULLPTR;
658682
}
659-
#endif //_DEBUG
660683
break;
661684
}
662685
}
@@ -686,20 +709,23 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
686709
{
687710
/* Handle Error */
688711
secure = false;
712+
set_dir_security_output_error_message(outputError, "Unable to retrieve DACL from security descriptor: %s\n", dirptr);
689713
break;
690714
}
691715

692716
if (FALSE == daclPresent || dacl == M_NULLPTR)
693717
{
694718
/* Missing DACL, so cannot verify permissions */
695719
secure = false;
720+
set_dir_security_output_error_message(outputError, "DACL Missing. Cannot verify permissions: %s\n", dirptr);
696721
break;
697722
}
698723

699724
if (FALSE == IsValidAcl(dacl))
700725
{
701726
/* Handle Error */
702727
secure = false;
728+
set_dir_security_output_error_message(outputError, "Invalid DACL received. Cannot verify permissions: %s\n", dirptr);
703729
break;
704730
}
705731

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

729755
if (!(allowEveryoneGroup && everyoneGroupSID != M_NULLPTR && EqualSid(aceSID, everyoneGroupSID)))
730756
{
731-
secure = false;
732-
#if defined (_DEBUG)
733757
char* sidString = M_NULLPTR;
758+
secure = false;
734759
if (ConvertSidToStringSidA(aceSID, &sidString))
735760
{
736-
printf("Untrusted SID: %s\n", sidString);
761+
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);
762+
LocalFree(sidString);
763+
sidString = M_NULLPTR;
737764
}
738-
#endif //_DEBUG
739765
}
740766
#if defined (_DEBUG)
741767
else
@@ -744,6 +770,8 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
744770
if (ConvertSidToStringSidA(aceSID, &sidString))
745771
{
746772
printf("Trusted SID: %s\n", sidString);
773+
LocalFree(sidString);
774+
sidString = M_NULLPTR;
747775
}
748776
}
749777
#endif //_DEBUG
@@ -761,6 +789,7 @@ static bool is_Folder_Secure(const char* securityDescriptorString, const char* d
761789
}
762790
else
763791
{
792+
set_dir_security_output_error_message(outputError, "Invalid ACE in DACL. Directory (%s) cannot be trusted\n", dirptr);
764793
secure = false;
765794
}
766795
}
@@ -946,6 +975,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
946975
{
947976
/* handle error */
948977
secure = false;
978+
set_dir_security_output_error_message(outputError, "Unable to read directory attributes: %s\n", dirptr);
949979
if (appendedTrailingSlash)
950980
{
951981
safe_free(&dirptr);
@@ -956,6 +986,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
956986
{
957987
/* handle error */
958988
secure = false;
989+
set_dir_security_output_error_message(outputError, "Too many symlinks in path (>%d): %s\n", MAX_SYMLINKS_IN_PATH, dirptr);
959990
free_File_Attributes(&attrs);
960991
if (appendedTrailingSlash)
961992
{
@@ -1005,22 +1036,26 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
10051036
}
10061037
else
10071038
{
1039+
set_dir_security_output_error_message(outputError, "Unable to allocate memory to check reparse point in path: %s\n", dirptr);
10081040
secure = false;
10091041
}
10101042
}
10111043
else
10121044
{
1045+
set_dir_security_output_error_message(outputError, "Unable to calculate memory length to check reparse point in path: %s\n", dirptr);
10131046
secure = false;
10141047
}
10151048
}
10161049
else
10171050
{
1051+
set_dir_security_output_error_message(outputError, "Unable to issue FSCTL_GET_REPASE_POINT to validate reparse point in path: %s\n", dirptr);
10181052
secure = false;
10191053
}
10201054
safe_free_reparse_data_buf(&reparseData);
10211055
}
10221056
else
10231057
{
1058+
set_dir_security_output_error_message(outputError, "Unable to allocate memory to check reparse point in path: %s\n", dirptr);
10241059
secure = false;
10251060
}
10261061
CloseHandle(link);
@@ -1039,6 +1074,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
10391074
}
10401075
else
10411076
{
1077+
set_dir_security_output_error_message(outputError, "Unable to open handle to reparse point in path: %s\n", dirptr);
10421078
secure = false;
10431079
free_File_Attributes(&attrs);
10441080
if (appendedTrailingSlash)
@@ -1053,6 +1089,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
10531089
{
10541090
/* Not a directory */
10551091
secure = false;
1092+
set_dir_security_output_error_message(outputError, "%s is not a directory. Cannot validate as part of path.\n", dirptr);
10561093
free_File_Attributes(&attrs);
10571094
if (appendedTrailingSlash)
10581095
{
@@ -1061,7 +1098,7 @@ static bool internal_OS_Is_Directory_Secure(const char* fullpath, unsigned int n
10611098
break;
10621099
}
10631100

1064-
secure = is_Folder_Secure(attrs->winSecurityDescriptor, dirptr);
1101+
secure = is_Folder_Secure(attrs->winSecurityDescriptor, dirptr, outputError);
10651102
if (appendedTrailingSlash)
10661103
{
10671104
safe_free(&dirptr);

0 commit comments

Comments
 (0)