From 3b450d5072edddc66216ee2a1eeca827162be7d5 Mon Sep 17 00:00:00 2001 From: Bryan Berns <berns@uwalumni.com> Date: Tue, 5 Jun 2018 00:10:46 -0400 Subject: [PATCH] Updated SSHD Password Generation (#317) Updated SSHD user password generation routine to be longer and more complex. This should satisfy systems with password filters that require more character types or very long passwords. Updated routine to now securely zero memory for the SSHD account password. Corrected attempt to write to NULL pointer by localtime_s() in localtime_r() and made function return NULL on error per specification. Addressed various compiler / code analysis warnings. --- contrib/win32/win32compat/fileio.c | 1 - contrib/win32/win32compat/misc.c | 25 +++++------- contrib/win32/win32compat/pwd.c | 2 +- contrib/win32/win32compat/w32api_proxies.c | 2 +- contrib/win32/win32compat/w32fd.c | 2 +- contrib/win32/win32compat/wmain_sshd.c | 46 +++++++++++++++------- 6 files changed, 44 insertions(+), 34 deletions(-) diff --git a/contrib/win32/win32compat/fileio.c b/contrib/win32/win32compat/fileio.c index d78234b29..51ba51548 100644 --- a/contrib/win32/win32compat/fileio.c +++ b/contrib/win32/win32compat/fileio.c @@ -301,7 +301,6 @@ createFile_flags_setup(int flags, mode_t mode, struct createFile_flags* cf_flags wchar_t sddl[SDDL_LENGTH + 1] = { 0 }, owner_ace[MAX_ACE_LENGTH + 1] = {0}, everyone_ace[MAX_ACE_LENGTH + 1] = {0}; wchar_t owner_access[MAX_ATTRIBUTE_LENGTH + 1] = {0}, everyone_access[MAX_ATTRIBUTE_LENGTH + 1] = {0}, *sid_utf16 = NULL; PACL dacl = NULL; - struct passwd * pwd; PSID owner_sid = NULL; /* diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 3441ad6d9..824603c68 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -931,7 +931,7 @@ realpath(const char *path, char resolved[PATH_MAX]) /* "cd .." from within a drive root */ if (path_len == 6 && !chroot_path) { char *tmplate = "/x:/.."; - strcat(resolved, path); + strcat_s(resolved, PATH_MAX, path); resolved[1] = 'x'; if (strcmp(tmplate, resolved) == 0) { resolved[0] = '/'; @@ -942,13 +942,13 @@ realpath(const char *path, char resolved[PATH_MAX]) if (chroot_path) { resolved[0] = '\0'; - strcat(resolved, chroot_path); + strcat_s(resolved, PATH_MAX, chroot_path); /* if path is relative, add cwd within chroot */ if (path[0] != '/' && path[0] != '\\') { w32_getcwd(resolved + chroot_path_len, PATH_MAX - chroot_path_len); - strcat(resolved, "/"); + strcat_s(resolved, PATH_MAX, "/"); } - strcat(resolved, path); + strcat_s(resolved, PATH_MAX, path); } else if ((path_len >= 2) && (path[0] == '/') && path[1] && (path[2] == ':')) { if((errno = strncpy_s(resolved, PATH_MAX, path + 1, path_len)) != 0 ) /* skip the first '/' */ { @@ -986,9 +986,9 @@ realpath(const char *path, char resolved[PATH_MAX]) if (strlen(tempPath) == strlen(chroot_path)) /* realpath is the same as chroot_path */ - strcat(resolved, "\\"); + strcat_s(resolved, PATH_MAX, "\\"); else - strcat(resolved, tempPath + strlen(chroot_path)); + strcat_s(resolved, PATH_MAX, tempPath + strlen(chroot_path)); if (resolved[0] != '\\') { errno = EACCES; @@ -1547,15 +1547,10 @@ copy_file(char *source, char *destination) return 0; } -struct tm* +struct tm * localtime_r(const time_t *timep, struct tm *result) { - struct tm *t = NULL; - - if(!localtime_s(t, timep)) - memcpy(result, t, sizeof(struct tm)); - - return t; + return localtime_s(result, timep) == 0 ? result : NULL; } void @@ -1596,7 +1591,7 @@ chroot(const char *path) if (chroot_path[strlen(chroot_path) - 1] == '\\') chroot_path[strlen(chroot_path) - 1] = '\0'; - chroot_path_len = strlen(chroot_path); + chroot_path_len = (int) strlen(chroot_path); if ((chroot_pathw = utf8_to_utf16(chroot_path)) == NULL) { errno = ENOMEM; @@ -1648,7 +1643,7 @@ get_user_sid(char* name) HANDLE token = NULL; TOKEN_USER* info = NULL; DWORD info_len = 0; - PSID ret = NULL, psid; + PSID ret = NULL, psid = NULL; wchar_t* name_utf16 = NULL; if (name) { diff --git a/contrib/win32/win32compat/pwd.c b/contrib/win32/win32compat/pwd.c index 04a764d2d..f4301fd51 100644 --- a/contrib/win32/win32compat/pwd.c +++ b/contrib/win32/win32compat/pwd.c @@ -266,7 +266,7 @@ getpwnam_placeholder(const char* user) { errno = EOTHER; goto cleanup; } - pw_name = strdup(user); + pw_name = _strdup(user); pw_dir = utf16_to_utf8(tmp_home); if (!pw_name || !pw_dir) { diff --git a/contrib/win32/win32compat/w32api_proxies.c b/contrib/win32/win32compat/w32api_proxies.c index 952facf06..a9813dd50 100644 --- a/contrib/win32/win32compat/w32api_proxies.c +++ b/contrib/win32/win32compat/w32api_proxies.c @@ -52,7 +52,7 @@ static HMODULE load_module(wchar_t* name) { wchar_t module_path[MAX_PATH + 1]; - wchar_t *system32_path, *p; + wchar_t *system32_path; HMODULE hm; if ((system32_path = system32_dir()) == NULL) diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 2888e12e5..863061ff5 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -124,7 +124,7 @@ fd_table_initialize() if (chroot_pathw != NULL) { if ((chroot_path = utf16_to_utf8(chroot_pathw)) == NULL) return -1; - chroot_path_len = strlen(chroot_path); + chroot_path_len = (int) strlen(chroot_path); } } diff --git a/contrib/win32/win32compat/wmain_sshd.c b/contrib/win32/win32compat/wmain_sshd.c index a210195e1..6177a495f 100644 --- a/contrib/win32/win32compat/wmain_sshd.c +++ b/contrib/win32/win32compat/wmain_sshd.c @@ -104,29 +104,45 @@ static void generate_host_keys() { DWORD dwError = 0; - UUID uuid; - RPC_CWSTR rpc_str; USER_INFO_1 ui; NET_API_STATUS nStatus; STARTUPINFOW si; PROCESS_INFORMATION pi; wchar_t cmdline[MAX_PATH]; - + wchar_t password[PWLEN + 1] = { 0 }; if (am_system()) { - /* create sshd account if it does not exist */ - UuidCreate(&uuid); - UuidToStringW(&uuid, (RPC_WSTR*)&rpc_str); - ui.usri1_name = L"sshd"; - ui.usri1_password = (LPWSTR)rpc_str; - ui.usri1_priv = USER_PRIV_USER; - ui.usri1_home_dir = NULL; - ui.usri1_comment = NULL; - ui.usri1_flags = UF_SCRIPT | UF_DONT_EXPIRE_PASSWD; - ui.usri1_script_path = NULL; - NetUserAdd(NULL, 1, (LPBYTE)&ui, &dwError); - RpcStringFreeW((RPC_WSTR*)&rpc_str); + LPUSER_INFO_0 user_check = NULL; + if (NetUserGetInfo(NULL, L"sshd", 0, (LPBYTE*) &user_check) != NERR_Success) + { + /* account does not exist -- done with existence checking structure */ + NetApiBufferFree(user_check); + + /* create sshd account if it does not exist */ + ui.usri1_name = L"sshd"; + ui.usri1_password = password; + ui.usri1_priv = USER_PRIV_USER; + ui.usri1_home_dir = NULL; + ui.usri1_comment = NULL; + ui.usri1_flags = UF_SCRIPT | UF_DONT_EXPIRE_PASSWD; + ui.usri1_script_path = NULL; + + /* generate a random string */ + if (BCryptGenRandom(NULL, (PUCHAR)password, sizeof(password) - sizeof(WCHAR), + BCRYPT_USE_SYSTEM_PREFERRED_RNG) != 0) { + printf("failed to generate sshd user temporary password"); + exit(255); + } + + /* normalize characters to printable ascii */ + for (int i = 0; i < PWLEN; i++) + password[i] = (password[i] % ((L'~' + 1) - L'!')) + L'!'; + + /* add user to local accounts */ + NetUserAdd(NULL, 1, (LPBYTE)&ui, &dwError); + SecureZeroMemory(password, sizeof(password)); + } /* create host keys if they dont already exist */ ZeroMemory(&si, sizeof(si));