From 880222dc1bf4109ec7b6e8e0d2b974e321f5ef04 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 9 Sep 2021 08:23:59 -0500 Subject: [PATCH] This file isn't even readable by admins, so maybe that's bad --- .../BaseApplicationState.h | 2 +- .../TerminalSettingsModel/ElevatedState.cpp | 76 +++++++++------- .../TerminalSettingsModel/ElevatedState.h | 1 + .../TerminalSettingsModel/FileUtils.cpp | 89 +++++++++++++++++-- .../TerminalSettingsModel/FileUtils.h | 4 +- 5 files changed, 133 insertions(+), 39 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/BaseApplicationState.h b/src/cascadia/TerminalSettingsModel/BaseApplicationState.h index 5713beb93..c34617663 100644 --- a/src/cascadia/TerminalSettingsModel/BaseApplicationState.h +++ b/src/cascadia/TerminalSettingsModel/BaseApplicationState.h @@ -27,7 +27,7 @@ struct BaseApplicationState virtual Json::Value ToJson() const noexcept = 0; protected: - void _write() const noexcept; + virtual void _write() const noexcept; void _read() const noexcept; std::filesystem::path _path; diff --git a/src/cascadia/TerminalSettingsModel/ElevatedState.cpp b/src/cascadia/TerminalSettingsModel/ElevatedState.cpp index 209d3a7b5..783298b12 100644 --- a/src/cascadia/TerminalSettingsModel/ElevatedState.cpp +++ b/src/cascadia/TerminalSettingsModel/ElevatedState.cpp @@ -27,42 +27,42 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static auto state = winrt::make_self(GetBaseSettingsPath() / stateFileName); state->Reload(); - const auto testPath{ GetBaseSettingsPath() / L"test.json" }; + // const auto testPath{ GetBaseSettingsPath() / L"test.json" }; - PSID pEveryoneSID = NULL; - SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_NT_AUTHORITY; - BOOL success = AllocateAndInitializeSid(&SIDAuthWorld, 1, SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0, &pEveryoneSID); + // PSID pEveryoneSID = NULL; + // SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_NT_AUTHORITY; + // BOOL success = AllocateAndInitializeSid(&SIDAuthWorld, 1, SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0, &pEveryoneSID); - EXPLICIT_ACCESS ea[1]; - ZeroMemory(&ea, 1 * sizeof(EXPLICIT_ACCESS)); - ea[0].grfAccessPermissions = KEY_READ; - ea[0].grfAccessMode = SET_ACCESS; - ea[0].grfInheritance = NO_INHERITANCE; - ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID; - ea[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP; - ea[0].Trustee.ptstrName = (LPTSTR)pEveryoneSID; + // EXPLICIT_ACCESS ea[1]; + // ZeroMemory(&ea, 1 * sizeof(EXPLICIT_ACCESS)); + // ea[0].grfAccessPermissions = KEY_READ; + // ea[0].grfAccessMode = SET_ACCESS; + // ea[0].grfInheritance = NO_INHERITANCE; + // ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID; + // ea[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP; + // ea[0].Trustee.ptstrName = (LPTSTR)pEveryoneSID; - ACL acl; - PACL pAcl = &acl; - DWORD dwRes = SetEntriesInAcl(1, ea, NULL, &pAcl); - dwRes; + // ACL acl; + // PACL pAcl = &acl; + // DWORD dwRes = SetEntriesInAcl(1, ea, NULL, &pAcl); + // dwRes; - SECURITY_DESCRIPTOR sd; - success = InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION); - success = SetSecurityDescriptorDacl(&sd, - TRUE, // bDaclPresent flag - pAcl, - FALSE); + // SECURITY_DESCRIPTOR sd; + // success = InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION); + // success = SetSecurityDescriptorDacl(&sd, + // TRUE, // bDaclPresent flag + // pAcl, + // FALSE); - SECURITY_ATTRIBUTES sa; - // Initialize a security attributes structure. - sa.nLength = sizeof(SECURITY_ATTRIBUTES); - sa.lpSecurityDescriptor = &sd; - sa.bInheritHandle = FALSE; - success; + // SECURITY_ATTRIBUTES sa; + // // Initialize a security attributes structure. + // sa.nLength = sizeof(SECURITY_ATTRIBUTES); + // sa.lpSecurityDescriptor = &sd; + // sa.bInheritHandle = FALSE; + // success; - wil::unique_hfile file{ CreateFileW(testPath.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr) }; - THROW_LAST_ERROR_IF(!file); + // wil::unique_hfile file{ CreateFileW(testPath.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr) }; + // THROW_LAST_ERROR_IF(!file); return *state; } @@ -113,4 +113,20 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation MTSM_ELEVATED_STATE_FIELDS(MTSM_ELEVATED_STATE_GEN) #undef MTSM_ELEVATED_STATE_GEN + // Serialized this ApplicationState (in `context`) into the state.json at _path. + // * Errors are only logged. + // * _state->_writeScheduled is set to false, signaling our + // setters that _synchronize() needs to be called again. + void ElevatedState::_write() const noexcept + try + { + Json::Value root{ this->ToJson() }; + + Json::StreamWriterBuilder wbuilder; + const auto content = Json::writeString(wbuilder, root); + // WriteUTF8FileAtomic(_path, content, true); + WriteUTF8File(_path, content, true); + } + CATCH_LOG() + } diff --git a/src/cascadia/TerminalSettingsModel/ElevatedState.h b/src/cascadia/TerminalSettingsModel/ElevatedState.h index 5d7f41731..6f28f331a 100644 --- a/src/cascadia/TerminalSettingsModel/ElevatedState.h +++ b/src/cascadia/TerminalSettingsModel/ElevatedState.h @@ -48,6 +48,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation #undef MTSM_ELEVATED_STATE_GEN }; til::shared_mutex _state; + virtual void _write() const noexcept override; }; } diff --git a/src/cascadia/TerminalSettingsModel/FileUtils.cpp b/src/cascadia/TerminalSettingsModel/FileUtils.cpp index 81fbb6cee..51c588924 100644 --- a/src/cascadia/TerminalSettingsModel/FileUtils.cpp +++ b/src/cascadia/TerminalSettingsModel/FileUtils.cpp @@ -8,6 +8,8 @@ #include #include +#include + static constexpr std::string_view Utf8Bom{ u8"\uFEFF" }; static constexpr std::wstring_view UnpackagedSettingsFolderName{ L"Microsoft\\Windows Terminal\\" }; @@ -105,10 +107,83 @@ namespace Microsoft::Terminal::Settings::Model throw; } } - - void WriteUTF8File(const std::filesystem::path& path, const std::string_view content) + void _setupAttributes(SECURITY_ATTRIBUTES& sa) { - wil::unique_hfile file{ CreateFileW(path.c_str(), GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr) }; + PSID pEveryoneSID = NULL; + SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_NT_AUTHORITY; + BOOL success = AllocateAndInitializeSid(&SIDAuthWorld, 1, SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0, &pEveryoneSID); + + EXPLICIT_ACCESS ea[1]; + ZeroMemory(&ea, 1 * sizeof(EXPLICIT_ACCESS)); + ea[0].grfAccessPermissions = KEY_READ; + ea[0].grfAccessMode = SET_ACCESS; + ea[0].grfInheritance = NO_INHERITANCE; + ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID; + ea[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP; + ea[0].Trustee.ptstrName = (LPTSTR)pEveryoneSID; + + ACL acl; + PACL pAcl = &acl; + DWORD dwRes = SetEntriesInAcl(1, ea, NULL, &pAcl); + dwRes; + + SECURITY_DESCRIPTOR sd; + success = InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION); + success = SetSecurityDescriptorDacl(&sd, + TRUE, // bDaclPresent flag + pAcl, + FALSE); + + // Initialize a security attributes structure. + sa.nLength = sizeof(SECURITY_ATTRIBUTES); + sa.lpSecurityDescriptor = &sd; + sa.bInheritHandle = FALSE; + success; + // return sa; + // wil::unique_hfile file{ CreateFileW(testPath.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr) }; + // THROW_LAST_ERROR_IF(!file); + } + void WriteUTF8File(const std::filesystem::path& path, + const std::string_view content, + const bool elevatedOnly) + { + SECURITY_ATTRIBUTES sa; + if (elevatedOnly) + { + // sa = _setupAttributes(); + + PSID pEveryoneSID = NULL; + SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_NT_AUTHORITY; + BOOL success = AllocateAndInitializeSid(&SIDAuthWorld, 1, SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0, &pEveryoneSID); + + EXPLICIT_ACCESS ea[1]; + ZeroMemory(&ea, 1 * sizeof(EXPLICIT_ACCESS)); + ea[0].grfAccessPermissions = KEY_READ; + ea[0].grfAccessMode = SET_ACCESS; + ea[0].grfInheritance = NO_INHERITANCE; + ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID; + ea[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP; + ea[0].Trustee.ptstrName = (LPTSTR)pEveryoneSID; + + ACL acl; + PACL pAcl = &acl; + DWORD dwRes = SetEntriesInAcl(1, ea, NULL, &pAcl); + dwRes; + + SECURITY_DESCRIPTOR sd; + success = InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION); + success = SetSecurityDescriptorDacl(&sd, + TRUE, // bDaclPresent flag + pAcl, + FALSE); + + // Initialize a security attributes structure. + sa.nLength = sizeof(SECURITY_ATTRIBUTES); + sa.lpSecurityDescriptor = &sd; + sa.bInheritHandle = FALSE; + success; + } + wil::unique_hfile file{ CreateFileW(path.c_str(), GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, elevatedOnly ? &sa : nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr) }; THROW_LAST_ERROR_IF(!file); const auto fileSize = gsl::narrow(content.size()); @@ -121,7 +196,9 @@ namespace Microsoft::Terminal::Settings::Model } } - void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view content) + void WriteUTF8FileAtomic(const std::filesystem::path& path, + const std::string_view content, + const bool elevatedOnly) { // GH#10787: rename() will replace symbolic links themselves and not the path they point at. // It's thus important that we first resolve them before generating temporary path. @@ -139,7 +216,7 @@ namespace Microsoft::Terminal::Settings::Model // * resolve the link manually, which might be less accurate and more prone to race conditions // * write to the file directly, which lets the system resolve the symbolic link but leaves the write non-atomic // The latter is chosen, as this is an edge case and our 'atomic' writes are only best-effort. - WriteUTF8File(path, content); + WriteUTF8File(path, content, elevatedOnly); return; } @@ -147,7 +224,7 @@ namespace Microsoft::Terminal::Settings::Model tmpPath += L".tmp"; // Writing to a file isn't atomic, but... - WriteUTF8File(tmpPath, content); + WriteUTF8File(tmpPath, content, elevatedOnly); // renaming one is (supposed to be) atomic. // Wait... "supposed to be"!? Well it's technically not always atomic, diff --git a/src/cascadia/TerminalSettingsModel/FileUtils.h b/src/cascadia/TerminalSettingsModel/FileUtils.h index d2e2eb53c..66328ff56 100644 --- a/src/cascadia/TerminalSettingsModel/FileUtils.h +++ b/src/cascadia/TerminalSettingsModel/FileUtils.h @@ -6,6 +6,6 @@ namespace Microsoft::Terminal::Settings::Model std::filesystem::path GetBaseSettingsPath(); std::string ReadUTF8File(const std::filesystem::path& path); std::optional ReadUTF8FileIfExists(const std::filesystem::path& path); - void WriteUTF8File(const std::filesystem::path& path, const std::string_view content); - void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view content); + void WriteUTF8File(const std::filesystem::path& path, const std::string_view content, const bool elevatedOnly = false); + void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view content, const bool elevatedOnly = false); }