Skip to content

Commit f87f0c5

Browse files
authored
add check for program data folder permissions during sshd service startup (#686)
1 parent 3645eaa commit f87f0c5

File tree

3 files changed

+96
-1
lines changed

3 files changed

+96
-1
lines changed

contrib/win32/win32compat/misc.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
#include "w32fd.h"
6161
#include "inc\string.h"
6262
#include "inc\time.h"
63+
#include "..\..\..\sshfileperm.h"
6364

6465
#include <wchar.h>
6566

@@ -1443,6 +1444,13 @@ create_directory_withsddl(wchar_t *path_w, wchar_t *sddl_w)
14431444
return -1;
14441445
}
14451446
}
1447+
else {
1448+
// directory already exists; need to confirm permissions are correct
1449+
if (check_secure_folder_permission(path_w, 1) != 0) {
1450+
error("Directory already exists but folder permissions are invalid");
1451+
return -1;
1452+
}
1453+
}
14461454

14471455
return 0;
14481456
}

contrib/win32/win32compat/w32-sshfileperm.c

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,89 @@ check_secure_file_permission(const char *input_path, struct passwd * pw, int rea
173173
return ret;
174174
}
175175

176+
/*
177+
* The function is similar to check_secure_file_permission.
178+
* Check the owner of the file is one of these types: Local Administrators groups or system account
179+
* Check the users have access permission to the file don't violate the following rules:
180+
1. no user other than local administrators group and system account have write permission on the folder
181+
* Returns 0 on success and -1 on failure
182+
*/
183+
int
184+
check_secure_folder_permission(const wchar_t* path_utf16, int read_ok)
185+
{
186+
PSECURITY_DESCRIPTOR pSD = NULL;
187+
PSID owner_sid = NULL, ti_sid = NULL;
188+
PACL dacl = NULL;
189+
DWORD error_code = ERROR_SUCCESS;
190+
BOOL is_valid_sid = FALSE, is_valid_acl = FALSE;
191+
wchar_t* bad_user = NULL;
192+
int ret = 0;
193+
194+
/*Get the owner sid of the file.*/
195+
if ((error_code = GetNamedSecurityInfoW(path_utf16, SE_FILE_OBJECT,
196+
OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION,
197+
&owner_sid, NULL, &dacl, NULL, &pSD)) != ERROR_SUCCESS) {
198+
printf("failed to retrieve the owner sid and dacl of file %S with error code: %d", path_utf16, error_code);
199+
errno = EOTHER;
200+
ret = -1;
201+
goto cleanup;
202+
}
203+
if (((is_valid_sid = IsValidSid(owner_sid)) == FALSE) || ((is_valid_acl = IsValidAcl(dacl)) == FALSE)) {
204+
printf("IsValidSid: %d; is_valid_acl: %d", is_valid_sid, is_valid_acl);
205+
ret = -1;
206+
goto cleanup;
207+
}
208+
if (!IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) &&
209+
!IsWellKnownSid(owner_sid, WinLocalSystemSid)) {
210+
printf("Bad owner on %S", path_utf16);
211+
ret = -1;
212+
goto cleanup;
213+
}
214+
/*
215+
iterate all aces of the file to find out if there is violation of the following rules:
216+
1. no others than administrators group and system account have write permission on the file
217+
*/
218+
for (DWORD i = 0; i < dacl->AceCount; i++) {
219+
PVOID current_ace = NULL;
220+
PACE_HEADER current_aceHeader = NULL;
221+
PSID current_trustee_sid = NULL;
222+
ACCESS_MASK current_access_mask = 0;
223+
224+
if (!GetAce(dacl, i, &current_ace)) {
225+
printf("GetAce() failed");
226+
errno = EOTHER;
227+
ret = -1;
228+
goto cleanup;
229+
}
230+
231+
current_aceHeader = (PACE_HEADER)current_ace;
232+
/* only interested in Allow ACE */
233+
if (current_aceHeader->AceType != ACCESS_ALLOWED_ACE_TYPE)
234+
continue;
235+
236+
PACCESS_ALLOWED_ACE pAllowedAce = (PACCESS_ALLOWED_ACE)current_ace;
237+
current_trustee_sid = &(pAllowedAce->SidStart);
238+
current_access_mask = pAllowedAce->Mask;
239+
240+
/*no need to check administrators group and system account*/
241+
if (IsWellKnownSid(current_trustee_sid, WinBuiltinAdministratorsSid) ||
242+
IsWellKnownSid(current_trustee_sid, WinLocalSystemSid)) {
243+
continue;
244+
}
245+
else if (read_ok && (current_access_mask & (FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA)) == 0) {
246+
/* if read is allowed, allow ACES that do not give write access*/
247+
continue;
248+
}
249+
else {
250+
ret = -1;
251+
}
252+
}
253+
cleanup:
254+
if (bad_user)
255+
LocalFree(bad_user);
256+
if (pSD)
257+
LocalFree(pSD);
258+
if (ti_sid)
259+
free(ti_sid);
260+
return ret;
261+
}

sshfileperm.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,5 @@
2626
#define _SSH_FILE_PERM_H
2727

2828
int check_secure_file_permission(const char *, struct passwd *, int);
29-
#endif /* _SSH_FILE_PERM_H */
29+
int check_secure_folder_permission(const wchar_t*, int);
30+
#endif /* _SSH_FILE_PERM_H */

0 commit comments

Comments
 (0)