Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make config1.db readable from AppContainer apps #1080

Merged
merged 1 commit into from
Oct 16, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Make config1.db readable from AppContainer apps
This attempts to avoid the deadlock issue discussed in #1076.

Our current hypothesis to explain #1076 is that "broadFileSystemAccess"
capability given to SearchHost.exe is playing an interesting role.

When TIP DLL calls CreateFileW API to open "config1.db", the process
itself does not have sufficient  permission to open it. Then
"broadFileSystemAccess" capability will take place and

  Windows.Storage.OneCore.dll

will attempts brokered file access after initializing the thread with
RoInitialize() when not yet initialized. If this happens in a certain
situation, TSF runtime gets confused and may start re-initializing Mozc
TIP.

  1. TSF calls back into Mozc TIP
  2. Mozc TIP calls CreateFileW()
  3. The system invokes RoInitialize() before returning from
     CreateFileW()
  4. TSF calls back into Mozc TIP before returning from RoInitialize()
  5. Now Mozc TIP is handling a reentrant callback.

Given that whether the target app has "broadFileSystemAccess" capability
or not is completely out of Mozc TIP's control, the safest option would
be to ensure that the target file/dir is always accessible to
AppContainer processes.

As the first step, this commit makes 'config1.db' readable from
AppContainer processes. There are two cases when the file permission
of 'config1.db' is updated.

 A. When the installer is installing a new version of Mozc.
 B. When 'config1.db' is updated by mozc_server or mozc_tool.

If this commit is not sufficient to address #1076, we then need to take
care of other cases such as calling GetFileAttributes() against the
user profile directory.
yukawa committed Oct 15, 2024
commit d1e0ab375b1378637ccd0eb10a00629708670f22
4 changes: 3 additions & 1 deletion src/base/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -738,7 +738,9 @@ mozc_cc_library(
"@com_google_absl//absl/log:check",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
],
] + mozc_select(
windows = ["//base/win32:win_sandbox"],
),
)

mozc_cc_test(
24 changes: 23 additions & 1 deletion src/base/config_file_stream.cc
Original file line number Diff line number Diff line change
@@ -38,6 +38,10 @@
#include <string>
#include <utility>

#ifdef _WIN32
#include <windows.h>
#endif // _WIN32

#include "absl/container/flat_hash_map.h"
#include "absl/log/check.h"
#include "absl/log/log.h"
@@ -52,7 +56,7 @@
#include "base/system_util.h"

#ifdef _WIN32
#include <windows.h>
#include "base/win32/win_sandbox.h"
#endif // _WIN32

namespace mozc {
@@ -218,4 +222,22 @@ std::string ConfigFileStream::GetFileName(const absl::string_view filename) {
void ConfigFileStream::ClearOnMemoryFiles() {
Singleton<OnMemoryFileMap>::get()->clear();
}

#ifdef _WIN32
// Check the file permission of "config1.db" if exists to ensure that
// "ALL APPLICATION PACKAGES" have read access to it.
void ConfigFileStream::FixupFilePermission(absl::string_view filename) {
const std::string path = ConfigFileStream::GetFileName(filename);
if (path.empty()) {
return;
}
const absl::Status status = FileUtil::FileExists(path);
if (status.ok()) {
WinSandbox::EnsureAllApplicationPackagesPermisssion(
win32::Utf8ToWide(path),
WinSandbox::AppContainerVisibilityType::kConfigFile);
}
}
#endif // _WIN32

} // namespace mozc
4 changes: 4 additions & 0 deletions src/base/config_file_stream.h
Original file line number Diff line number Diff line change
@@ -107,6 +107,10 @@ class ConfigFileStream {
// Clear all memory:// files. This is a utility method for testing.
static void ClearOnMemoryFiles();

#ifdef _WIN32
static void FixupFilePermission(absl::string_view filename);
#endif // _WIN32

private:
// This function is deprecated. Use OpenReadText or OpenReadBinary instead.
// TODO(yukawa): Move this function to anonymous namespace in
28 changes: 21 additions & 7 deletions src/base/win32/win_sandbox.cc
Original file line number Diff line number Diff line change
@@ -1015,6 +1015,23 @@ bool SetTokenIntegrityLevel(HANDLE token,
return (result != FALSE);
}

constexpr ACCESS_MASK GetAccessMask(
WinSandbox::AppContainerVisibilityType type) {
const ACCESS_MASK kBaseType =
FILE_READ_DATA | FILE_READ_EA | READ_CONTROL | SYNCHRONIZE;

switch (type) {
case WinSandbox::AppContainerVisibilityType::kProgramFiles:
// As of Windows 10 Anniversary Update, following access masks (==0x1200a9)
// are specified to files under Program Files by default.
return FILE_EXECUTE | kBaseType;
case WinSandbox::AppContainerVisibilityType::kConfigFile:
return FILE_GENERIC_READ | FILE_READ_ATTRIBUTES | kBaseType;
default:
return kBaseType;
}
}

} // namespace

std::vector<Sid> WinSandbox::GetSidsToDisable(HANDLE effective_token,
@@ -1232,7 +1249,7 @@ wil::unique_process_handle WinSandbox::GetRestrictedTokenHandleForImpersonation(
}

bool WinSandbox::EnsureAllApplicationPackagesPermisssion(
zwstring_view file_name) {
zwstring_view file_name, AppContainerVisibilityType type) {
// Get "All Application Packages" group SID.
const ATL::CSid all_application_packages(
Sid(WinBuiltinAnyPackageSid).GetPSID());
@@ -1243,10 +1260,7 @@ bool WinSandbox::EnsureAllApplicationPackagesPermisssion(
return false;
}

// As of Windows 10 Anniversary Update, following access masks (==0x1200a9)
// are specified to files under Program Files by default.
const ACCESS_MASK kDesiredMask =
FILE_READ_DATA | FILE_READ_EA | FILE_EXECUTE | READ_CONTROL | SYNCHRONIZE;
const ACCESS_MASK desired_mask = GetAccessMask(type);

// Check if the desired ACE is already specified or not.
for (UINT i = 0; i < dacl.GetAceCount(); ++i) {
@@ -1256,14 +1270,14 @@ bool WinSandbox::EnsureAllApplicationPackagesPermisssion(
dacl.GetAclEntry(i, &ace_sid, &access_mask, &ace_type);
if (ace_sid == all_application_packages &&
ace_type == ACCESS_ALLOWED_ACE_TYPE &&
(access_mask & kDesiredMask) == kDesiredMask) {
(access_mask & desired_mask) == desired_mask) {
// This is the desired ACE. There is nothing to do.
return true;
}
}

// We are here because there is no desired ACE. Hence we do add it.
if (!dacl.AddAllowedAce(all_application_packages, kDesiredMask,
if (!dacl.AddAllowedAce(all_application_packages, desired_mask,
ACCESS_ALLOWED_ACE_TYPE)) {
return false;
}
16 changes: 8 additions & 8 deletions src/base/win32/win_sandbox.h
Original file line number Diff line number Diff line change
@@ -185,15 +185,15 @@ class WinSandbox {
HANDLE effective_token, TokenLevel security_level,
IntegrityLevel integrity_level);

enum class AppContainerVisibilityType {
kProgramFiles = 0,
kConfigFile = 1,
};

// Returns true |file_name| already has or is updated to have an ACE
// (Access Control Entry) for "All Application Packages" group to have the
// following access masks:
// - FILE_READ_DATA
// - FILE_READ_EA
// - FILE_EXECUTE
// - READ_CONTROL
// - SYNCHRONIZE
static bool EnsureAllApplicationPackagesPermisssion(zwstring_view file_name);
// (Access Control Entry) for "All Application Packages" group.
static bool EnsureAllApplicationPackagesPermisssion(
zwstring_view file_name, AppContainerVisibilityType type);

protected:
// Returns SDDL for given |shareble_object_type|.
15 changes: 15 additions & 0 deletions src/config/config_handler.cc
Original file line number Diff line number Diff line change
@@ -53,6 +53,11 @@
#include "base/vlog.h"
#include "protocol/config.pb.h"

#ifdef _WIN32
#include "base/win32/wide_char.h"
#include "base/win32/win_sandbox.h"
#endif // _WIN32

namespace mozc {
namespace config {
namespace {
@@ -179,6 +184,9 @@ void ConfigHandlerImpl::SetConfig(const Config &config) {

MOZC_VLOG(1) << "Setting new config: " << filename_;
ConfigFileStream::AtomicUpdate(filename_, output_config.SerializeAsString());
#ifdef _WIN32
ConfigFileStream::FixupFilePermission(filename_);
#endif // _WIN32

#ifdef DEBUG
std::string debug_content = absl::StrCat(
@@ -309,5 +317,12 @@ Config::SessionKeymap ConfigHandler::GetDefaultKeyMap() {
}
}

#ifdef _WIN32
// static
void ConfigHandler::FixupFilePermission() {
ConfigFileStream::FixupFilePermission(GetConfigFileName());
}
#endif // _WIN32

} // namespace config
} // namespace mozc
4 changes: 4 additions & 0 deletions src/config/config_handler.h
Original file line number Diff line number Diff line change
@@ -86,6 +86,10 @@ class ConfigHandler {

// Get default keymap for each platform.
static Config::SessionKeymap GetDefaultKeyMap();

#ifdef _WIN32
static void FixupFilePermission();
#endif // _WIN32
};

} // namespace config
1 change: 0 additions & 1 deletion src/win32/base/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -53,7 +53,6 @@ mozc_cc_library(
target_compatible_with = ["@platforms//os:windows"],
deps = [
"//base:system_util",
"//client:client_interface",
"//config:config_handler",
"//protocol:config_cc_proto",
"//session:key_info_util",
44 changes: 3 additions & 41 deletions src/win32/base/config_snapshot.cc
Original file line number Diff line number Diff line change
@@ -31,16 +31,13 @@

#include <algorithm>

#include "base/win32/win_util.h"
#include "client/client_interface.h"
#include "config/config_handler.h"
#include "protocol/config.pb.h"

namespace mozc {
namespace win32 {
namespace {

using ::mozc::client::ClientInterface;
using ::mozc::config::Config;

constexpr size_t kMaxDirectModeKeys = 128;
@@ -53,25 +50,7 @@ struct StaticConfigSnapshot {
KeyInformation direct_mode_keys[kMaxDirectModeKeys];
};

bool GetConfigSnapshotForSandboxedProcess(ClientInterface *client,
Config *config) {
if (client == nullptr) {
return false;
}
// config1.db is likely to be inaccessible from sandboxed processes.
// So retrieve the config data from the server process.
// CAVEATS: ConfigHandler::GetConfig always returns true even when it fails
// to load the config file due to the sandbox. b/10449414
if (!client->CheckVersionOrRestartServer()) {
return false;
}
if (!client->GetConfig(config)) {
return false;
}
return true;
}

StaticConfigSnapshot GetConfigSnapshotForNonSandboxedProcess() {
StaticConfigSnapshot GetConfigSnapshotImpl() {
Config config;
// config1.db should be readable in this case.
config::ConfigHandler::GetConfig(&config);
@@ -102,26 +81,9 @@ ConfigSnapshot::Info::Info()
use_mode_indicator(false) {}

// static
bool ConfigSnapshot::Get(client::ClientInterface *client, Info *info) {
// A temporary workaround for b/24793812. Always reload config if the
// process is sandboxed.
// TODO(yukawa): Cache the result once it succeeds.
if (WinUtil::IsProcessSandboxed()) {
Config config;
if (!GetConfigSnapshotForSandboxedProcess(client, &config)) {
return false;
}
info->use_kana_input = (config.preedit_method() == Config::KANA);
info->use_keyboard_to_change_preedit_method =
config.use_keyboard_to_change_preedit_method();
info->use_mode_indicator = config.use_mode_indicator();
info->direct_mode_keys = KeyInfoUtil::ExtractSortedDirectModeKeys(config);
return true;
}

bool ConfigSnapshot::Get(Info *info) {
// Note: Thread-safety is not required.
static const StaticConfigSnapshot cached_snapshot =
GetConfigSnapshotForNonSandboxedProcess();
static const StaticConfigSnapshot cached_snapshot = GetConfigSnapshotImpl();
info->use_kana_input = cached_snapshot.use_kana_input;
info->use_keyboard_to_change_preedit_method =
cached_snapshot.use_keyboard_to_change_preedit_method;
6 changes: 1 addition & 5 deletions src/win32/base/config_snapshot.h
Original file line number Diff line number Diff line change
@@ -35,10 +35,6 @@
#include "session/key_info_util.h"

namespace mozc {
namespace client {
class ClientInterface;
} // namespace client

namespace win32 {

class ConfigSnapshot {
@@ -55,7 +51,7 @@ class ConfigSnapshot {
ConfigSnapshot(const ConfigSnapshot &) = delete;
ConfigSnapshot &operator=(const ConfigSnapshot &) = delete;

static bool Get(client::ClientInterface *client, Info *info);
static bool Get(Info *info);
};

} // namespace win32
31 changes: 27 additions & 4 deletions src/win32/custom_action/custom_action.cc
Original file line number Diff line number Diff line change
@@ -57,6 +57,7 @@
#include "base/win32/win_util.h"
#include "client/client.h"
#include "client/client_interface.h"
#include "config/config_handler.h"
#include "renderer/renderer_client.h"
#include "win32/base/input_dll.h"
#include "win32/base/omaha_util.h"
@@ -209,19 +210,23 @@ BOOL APIENTRY DllMain(HMODULE module, DWORD ul_reason_for_call,
UINT __stdcall EnsureAllApplicationPackagesPermisssions(MSIHANDLE msi_handle) {
DEBUG_BREAK_FOR_DEBUGGER();
if (!mozc::WinSandbox::EnsureAllApplicationPackagesPermisssion(
GetMozcComponentPath(mozc::kMozcServerName))) {
GetMozcComponentPath(mozc::kMozcServerName),
mozc::WinSandbox::AppContainerVisibilityType::kProgramFiles)) {
return ERROR_INSTALL_FAILURE;
}
if (!mozc::WinSandbox::EnsureAllApplicationPackagesPermisssion(
GetMozcComponentPath(mozc::kMozcRenderer))) {
GetMozcComponentPath(mozc::kMozcRenderer),
mozc::WinSandbox::AppContainerVisibilityType::kProgramFiles)) {
return ERROR_INSTALL_FAILURE;
}
if (!mozc::WinSandbox::EnsureAllApplicationPackagesPermisssion(
GetMozcComponentPath(mozc::kMozcTIP32))) {
GetMozcComponentPath(mozc::kMozcTIP32),
mozc::WinSandbox::AppContainerVisibilityType::kProgramFiles)) {
return ERROR_INSTALL_FAILURE;
}
if (!mozc::WinSandbox::EnsureAllApplicationPackagesPermisssion(
GetMozcComponentPath(mozc::kMozcTIP64))) {
GetMozcComponentPath(mozc::kMozcTIP64),
mozc::WinSandbox::AppContainerVisibilityType::kProgramFiles)) {
return ERROR_INSTALL_FAILURE;
}
return ERROR_SUCCESS;
@@ -328,6 +333,24 @@ UINT __stdcall EnableTipProfile(MSIHANDLE msi_handle) {
return ERROR_SUCCESS;
}

UINT __stdcall FixupConfigFilePermission(MSIHANDLE msi_handle) {
bool is_service = false;
if (::mozc::WinUtil::IsServiceAccount(&is_service) && is_service) {
// Do nothing if this is a service account.
return ERROR_SUCCESS;
}

// Check the file permission of "config1.db" if exists to ensure that
// "ALL APPLICATION PACKAGES" have read access to it.
// See https://github.com/google/mozc/issues/1076 for details.
::mozc::config::ConfigHandler::FixupFilePermission();

// Return always ERROR_SUCCESS regardless of the result, as not being able
// to fixup the permission is not problematic enough to block installation
// and upgrading.
return ERROR_SUCCESS;
}

UINT __stdcall SaveCustomActionData(MSIHANDLE msi_handle) {
DEBUG_BREAK_FOR_DEBUGGER();
// store the CHANNEL value specified in the command line argument for
1 change: 1 addition & 0 deletions src/win32/custom_action/custom_action.def
Original file line number Diff line number Diff line change
@@ -14,5 +14,6 @@ EXPORTS
InitialInstallation
InitialInstallationCommit
EnableTipProfile
FixupConfigFilePermission
WriteApValue
WriteApValueRollback
3 changes: 3 additions & 0 deletions src/win32/custom_action/custom_action.h
Original file line number Diff line number Diff line change
@@ -76,6 +76,9 @@ UINT __stdcall InitialInstallationCommit(MSIHANDLE msi_handle);
// Enable TIP profile for the calling user unless it's a service account.
UINT __stdcall EnableTipProfile(MSIHANDLE msi_handle);

// Update config1.db file access controll to make it readable to AppContainer.
UINT __stdcall FixupConfigFilePermission(MSIHANDLE msi_handle);

// Saves omaha's ap value for WriteApValue, WriteApValueRollback, and
// RestoreServiceState.
// Since they are executed as deferred customs actions and most properties
4 changes: 3 additions & 1 deletion src/win32/installer/installer_64bit.wxs
Original file line number Diff line number Diff line change
@@ -145,6 +145,7 @@
<CustomAction Id="InitialInstallation" DllEntry="InitialInstallation" Execute="deferred" Impersonate="no" BinaryRef="GoogleIMEJaInstallerHelper.dll" />
<CustomAction Id="InitialInstallationCommit" DllEntry="InitialInstallationCommit" Execute="commit" Impersonate="no" BinaryRef="GoogleIMEJaInstallerHelper.dll" />
<CustomAction Id="EnableTipProfile" DllEntry="EnableTipProfile" Execute="commit" Impersonate="yes" BinaryRef="GoogleIMEJaInstallerHelper.dll" />
<CustomAction Id="FixupConfigFilePermission" DllEntry="FixupConfigFilePermission" Execute="commit" Impersonate="yes" BinaryRef="GoogleIMEJaInstallerHelper.dll" />
<CustomAction Id="HideCancelButton" DllEntry="HideCancelButton" Return="ignore" BinaryRef="GoogleIMEJaInstallerHelper.dll" />
<CustomAction Id="SaveCustomActionData" DllEntry="SaveCustomActionData" BinaryRef="GoogleIMEJaInstallerHelper.dll" />
<CustomAction Id="RestoreServiceState" DllEntry="RestoreServiceState" Impersonate="no" Execute="deferred" Return="ignore" BinaryRef="GoogleIMEJaInstallerHelper.dll" />
@@ -196,7 +197,8 @@
<Custom Action="RegisterTIPRollback64" Before="RegisterTIP64" Condition="(NOT (REMOVE=&quot;ALL&quot;)) AND (VersionNT64) AND (NOT UPGRADING)" />
<Custom Action="RegisterTIP64" Before="EnsureAllApplicationPackagesPermisssions" Condition="(NOT (REMOVE=&quot;ALL&quot;)) AND (VersionNT64)" />
<Custom Action="EnsureAllApplicationPackagesPermisssions" Before="InitialInstallationCommit" Condition="(NOT (REMOVE=&quot;ALL&quot;))" />
<Custom Action="InitialInstallationCommit" Before="EnableTipProfile" Condition="(NOT (REMOVE=&quot;ALL&quot;)) AND (NOT UPGRADING)" />
<Custom Action="InitialInstallationCommit" Before="FixupConfigFilePermission" Condition="(NOT (REMOVE=&quot;ALL&quot;)) AND (NOT UPGRADING)" />
<Custom Action="FixupConfigFilePermission" Before="EnableTipProfile" Condition="NOT (REMOVE=&quot;ALL&quot;)" />
<Custom Action="EnableTipProfile" Before="InstallFinalize" Condition="(NOT (REMOVE=&quot;ALL&quot;)) AND (NOT UPGRADING)" />
<!-- show reboot dialog to execute pending file opartions -->
<?if ($(var.VSConfigurationName) = "Release") ?>
Loading