diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index 5fdb7efe1..a9edd6c2f 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -4158,6 +4158,136 @@ static bool win_file_security_secure(win_security_t *security, HANDLE file, win_file_acl_secure(security, file, mutation, ancestor); } +/* Is this object's DACL present but EMPTY (zero ACEs)? That denies everyone, + * including the owner, for anything the owner-rights path does not cover. + * + * It needs its own test because win_file_acl_secure() cannot detect it: that + * function scans ACEs for untrusted mutation grants, and a DACL with zero ACEs + * trivially has none, so damage reads as compliance. */ +static bool win_file_dacl_is_empty(win_security_t *security, HANDLE file) { + PACL dacl = NULL; + PSECURITY_DESCRIPTOR descriptor = NULL; + if (security->get_security_info(file, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, + &dacl, NULL, &descriptor) != ERROR_SUCCESS) { + return false; + } + ACL_SIZE_INFORMATION information; + memset(&information, 0, sizeof(information)); + bool empty = dacl && security->is_valid_acl(dacl) && + security->get_acl_information(dacl, &information, sizeof(information), + AclSizeInformation) && + information.AceCount == 0U; + if (descriptor) { + (void)LocalFree(descriptor); + } + return empty; +} + +/* Repair cache/runtime children left unusable by the pre-v0.10.3 DACL regime. + * + * Between v0.9.1-rc and v0.10.2 the runtime directory carried a PROTECTED DACL + * whose ACE was not inheritable. Windows therefore gave every file created + * inside it either an empty DACL or the token default (SYSTEM + TokenOwner + + * logon SID). Under an elevated token TokenOwner is BUILTIN\Administrators, so + * the interactive user ends up with no durable grant at all and the file is + * unreadable after the next logon — #1601, where takeown and icacls both fail + * non-elevated and the daemon can no longer open _config.db. + * + * #1531 fixed the cause forward-only in v0.10.3: the directory ACE is + * inheritable now, so newly created children are fine. Nothing repaired the + * children already damaged, which is why upgrading did not rescue anyone whose + * cache was written under the old regime. This is that repair. + * + * Deliberately bounded and conservative: + * - immediate children only, no recursion, capped; + * - regular files only; directories, reparse points and symlinks are skipped + * entirely rather than followed; + * - a child is touched ONLY when it is demonstrably damaged - an empty DACL, + * or an owner that is not the current user. A child that is merely unusual + * is left alone; + * - failures are counted and reported, never fatal. This runs inside daemon + * startup and must not be able to prevent it. + * + * Scope note: this only ever runs on cbm's own runtime/cache directory, which + * we created and own. It does not reach into user directories. */ +static void win_repair_runtime_children(win_security_t *security, const wchar_t *runtime_dir) { + enum { WIN_CHILD_REPAIR_MAX = 4096 }; + if (!security || !runtime_dir || !security->user_sid) { + return; + } + size_t dir_length = wcslen(runtime_dir); + if (dir_length == 0U || dir_length > 32000U) { + return; + } + wchar_t *pattern = calloc(dir_length + 3U, sizeof(wchar_t)); + if (!pattern) { + return; + } + (void)swprintf(pattern, dir_length + 3U, L"%ls\\*", runtime_dir); + WIN32_FIND_DATAW entry; + HANDLE search = FindFirstFileW(pattern, &entry); + free(pattern); + if (search == INVALID_HANDLE_VALUE) { + return; + } + unsigned examined = 0U; + unsigned repaired = 0U; + unsigned failed = 0U; + do { + if (wcscmp(entry.cFileName, L".") == 0 || wcscmp(entry.cFileName, L"..") == 0) { + continue; + } + if ((entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0U || + (entry.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0U) { + continue; + } + if (++examined > (unsigned)WIN_CHILD_REPAIR_MAX) { + break; + } + size_t name_length = wcslen(entry.cFileName); + size_t child_capacity = dir_length + name_length + 2U; + wchar_t *child_path = calloc(child_capacity, sizeof(wchar_t)); + if (!child_path) { + continue; + } + (void)swprintf(child_path, child_capacity, L"%ls\\%ls", runtime_dir, entry.cFileName); + HANDLE child = CreateFileW(child_path, READ_CONTROL | WRITE_DAC | WRITE_OWNER, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, + OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT, NULL); + free(child_path); + if (child == INVALID_HANDLE_VALUE) { + continue; + } + BY_HANDLE_FILE_INFORMATION child_info; + bool regular = GetFileInformationByHandle(child, &child_info) != 0 && + (child_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0U && + (child_info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0U; + bool damaged = regular && (win_file_dacl_is_empty(security, child) || + !win_file_owner_secure(security, child, true)); + if (damaged) { + if (security->set_security_info( + child, SE_FILE_OBJECT, + (DWORD)OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION | + PROTECTED_DACL_SECURITY_INFORMATION, + security->user_sid, NULL, security->acl, NULL) == ERROR_SUCCESS) { + repaired++; + } else { + failed++; + } + } + (void)CloseHandle(child); + } while (FindNextFileW(search, &entry) != 0); + (void)FindClose(search); + if (repaired > 0U || failed > 0U) { + char repaired_text[16]; + char failed_text[16]; + (void)snprintf(repaired_text, sizeof(repaired_text), "%u", repaired); + (void)snprintf(failed_text, sizeof(failed_text), "%u", failed); + cbm_log_warn("daemon.runtime_child_acl_repaired", "repaired", repaired_text, "failed", + failed_text); + } +} + static bool win_runtime_directory_secure(const wchar_t *runtime_dir) { win_security_t security; if (!win_security_init(&security)) { @@ -4204,8 +4334,31 @@ static bool win_runtime_directory_secure(const wchar_t *runtime_dir) { * refused, and the final validation below still demands the exact user. */ bool owner_ok = owner_exact || (valid_handle && can_write_owner && win_file_owner_secure(&security, directory, false)); + /* Re-stamp only when the directory is not ALREADY correct. + * + * This used to fire on every process start, whether or not anything was + * wrong. Two costs, both observed in the field: + * + * - It rewrites the security descriptor of a directory that already has + * the right one, and Windows propagates that to children. #1601 counted + * ELEVEN "Security change" USN records against a single _config.db in + * one day, none of which changed anything. + * - Every rewrite is a window. #1620 loses an atomic publish to exactly + * this: MoveFileEx needs DELETE on the destination, and a concurrent + * re-protect of the parent is a chance to be refused for a state that is + * about to be correct again anyway. + * + * The repair is what matters, not the ritual. If the owner is already the + * exact current user and the DACL already passes the private-directory + * check, there is nothing to fix and the correct action is to leave it + * alone. When it IS wrong we still repair exactly as before. */ DWORD secure_result = ERROR_ACCESS_DENIED; - if (valid_handle && owner_ok) { + bool already_correct = + valid_handle && owner_exact && + win_file_security_secure(&security, directory, true, win_private_mutation_rights(), false); + if (already_correct) { + secure_result = ERROR_SUCCESS; + } else if (valid_handle && owner_ok) { secure_result = security.set_security_info( directory, SE_FILE_OBJECT, (owner_exact ? 0U : (DWORD)OWNER_SECURITY_INFORMATION) | DACL_SECURITY_INFORMATION | @@ -4220,6 +4373,12 @@ static bool win_runtime_directory_secure(const wchar_t *runtime_dir) { bool final_private = secure_result == ERROR_SUCCESS && win_file_security_secure(&security, directory, true, win_private_mutation_rights(), false); + /* Repair damaged children only once the directory itself is known good. + * Repairing into a parent we have not secured would re-derive the same + * broken state on the next file created there. */ + if (final_private) { + win_repair_runtime_children(&security, runtime_dir); + } (void)CloseHandle(directory); win_security_destroy(&security); return valid_handle && owner_ok && final_private;