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

Move paths code to shared #4484

Merged
merged 3 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/Runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <winget/Filesystem.h>

using namespace AppInstaller;
using namespace AppInstaller::Filesystem;
using namespace AppInstaller::Runtime;
using namespace TestCommon;

Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/TestHooks.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace AppInstaller
namespace Runtime
{
void TestHook_SetPathOverride(PathName target, const std::filesystem::path& path);
void TestHook_SetPathOverride(PathName target, const PathDetails& details);
void TestHook_SetPathOverride(PathName target, const Filesystem::PathDetails& details);
void TestHook_ClearPathOverrides();
}

Expand Down
2 changes: 0 additions & 2 deletions src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,6 @@
<ExcludedFromBuild Condition="'$(Configuration)'=='Fuzzing'">true</ExcludedFromBuild>
</ClInclude>
<ClInclude Include="Public\winget\NameNormalization.h" />
<ClInclude Include="Public\winget\Filesystem.h" />
<ClInclude Include="Public\winget\NetworkSettings.h" />
<ClInclude Include="Public\winget\PackageDependenciesValidationUtil.h" />
<ClInclude Include="Public\winget\PackageVersionDataManifest.h" />
Expand All @@ -468,7 +467,6 @@
<ClCompile Include="DependenciesGraph.cpp" />
<ClCompile Include="DODownloader.cpp" />
<ClCompile Include="FileCache.cpp" />
<ClCompile Include="Filesystem.cpp" />
<ClCompile Include="FolderFileWatcher.cpp" />
<ClCompile Include="Deployment.cpp" />
<ClCompile Include="Downloader.cpp" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@
<ClInclude Include="Public\winget\Debugging.h">
<Filter>Public\winget</Filter>
</ClInclude>
<ClInclude Include="Public\winget\Filesystem.h">
<Filter>Public\winget</Filter>
</ClInclude>
<ClInclude Include="Public\winget\PortableARPEntry.h">
<Filter>Public\winget</Filter>
</ClInclude>
Expand Down Expand Up @@ -311,9 +308,6 @@
<ClCompile Include="Debugging.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="Filesystem.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="PortableARPEntry.cpp">
<Filter>Source Files</Filter>
</ClCompile>
Expand Down
52 changes: 6 additions & 46 deletions src/AppInstallerCommonCore/Public/AppInstallerRuntime.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#pragma once
#include <AppInstallerVersions.h>
#include <winget/LocIndependent.h>
#include <winget/Filesystem.h>
#include <winget/Runtime.h>

#include <filesystem>
Expand Down Expand Up @@ -57,56 +58,15 @@ namespace AppInstaller::Runtime
Max
};

// The principal that an ACE applies to.
enum class ACEPrincipal : uint32_t
{
CurrentUser,
Admins,
System,
};

// The permissions granted to a specific ACE.
enum class ACEPermissions : uint32_t
{
// This is not "Deny All", but rather, "Not mentioned"
None = 0x0,
Read = 0x1,
Write = 0x2,
Execute = 0x4,
ReadWrite = Read | Write,
ReadExecute = Read | Execute,
ReadWriteExecute = Read | Write | Execute,
// All means that full control will be granted
All = 0xFFFFFFFF
};

DEFINE_ENUM_FLAG_OPERATORS(ACEPermissions);

// Information about a path that we use and how to set it up.
struct PathDetails
{
std::filesystem::path Path;
// Default to creating the directory with inherited ownership and permissions
bool Create = true;
std::optional<ACEPrincipal> Owner;
std::map<ACEPrincipal, ACEPermissions> ACL;

// Shorthand for setting Owner and giving them ACEPermissions::All
void SetOwner(ACEPrincipal owner);

// Determines if the ACL should be applied.
bool ShouldApplyACL() const;

// Applies the ACL unconditionally.
void ApplyACL() const;
};

// Gets the PathDetails used for the given path.
// This is exposed primarily to allow for testing, GetPathTo should be preferred.
PathDetails GetPathDetailsFor(PathName path, bool forDisplay = false);
Filesystem::PathDetails GetPathDetailsFor(PathName path, bool forDisplay = false);

// Gets the path to the requested location.
std::filesystem::path GetPathTo(PathName path, bool forDisplay = false);
inline std::filesystem::path GetPathTo(PathName path, bool forDisplay = false)
{
return Filesystem::GetPathTo(path, forDisplay);
}

// Gets a new temp file path.
std::filesystem::path GetNewTempFilePath();
Expand Down
164 changes: 1 addition & 163 deletions src/AppInstallerCommonCore/Runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
#include "Public/AppInstallerLogging.h"
#include "Public/AppInstallerRuntime.h"
#include "Public/AppInstallerStrings.h"
#include "Public/winget/Filesystem.h"
#include "Public/winget/UserSettings.h"
#include "Public/winget/Registry.h"
#include <winget/Filesystem.h>


#define WINGET_DEFAULT_LOG_DIRECTORY "DiagOutputDir"
Expand Down Expand Up @@ -184,43 +184,6 @@ namespace AppInstaller::Runtime
return result;
}

DWORD AccessPermissionsFrom(ACEPermissions permissions)
{
DWORD result = 0;

if (permissions == ACEPermissions::All)
{
result |= GENERIC_ALL;
}
else
{
if (WI_IsFlagSet(permissions, ACEPermissions::Read))
{
result |= GENERIC_READ;
}

if (WI_IsFlagSet(permissions, ACEPermissions::Write))
{
result |= GENERIC_WRITE | FILE_DELETE_CHILD;
}

if (WI_IsFlagSet(permissions, ACEPermissions::Execute))
{
result |= GENERIC_EXECUTE;
}
}

return result;
}

// Contains the information about an ACE entry for a given principal.
struct ACEDetails
{
ACEPrincipal Principal;
PSID SID;
TRUSTEE_TYPE TrusteeType;
};

// Try to replace LOCALAPPDATA first as it is the likely location, fall back to trying USERPROFILE.
void ReplaceProfilePathsWithEnvironmentVariable(std::filesystem::path& path)
{
Expand All @@ -238,99 +201,6 @@ namespace AppInstaller::Runtime
s_runtimePathStateName.emplace(std::move(suitablePathPart));
}

void PathDetails::SetOwner(ACEPrincipal owner)
{
Owner = owner;
ACL[owner] = ACEPermissions::All;
}

bool PathDetails::ShouldApplyACL() const
{
// Could be expanded to actually check the current owner/ACL on the path, but isn't worth it currently
return !ACL.empty();
}

void PathDetails::ApplyACL() const
{
bool hasCurrentUser = ACL.count(ACEPrincipal::CurrentUser) != 0;
bool hasSystem = ACL.count(ACEPrincipal::System) != 0;

// Configuring permissions for both CurrentUser and SYSTEM while not having owner set as one of them is not valid because
// below we use only the owner permissions in the case of running as SYSTEM.
if ((hasCurrentUser && hasSystem) &&
IsRunningAsSystem() &&
(!Owner || (Owner.value() != ACEPrincipal::CurrentUser && Owner.value() != ACEPrincipal::System)))
{
THROW_HR(HRESULT_FROM_WIN32(ERROR_INVALID_STATE));
}

auto userToken = wil::get_token_information<TOKEN_USER>();
auto adminSID = wil::make_static_sid(SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
auto systemSID = wil::make_static_sid(SECURITY_NT_AUTHORITY, SECURITY_LOCAL_SYSTEM_RID);
PSID ownerSID = nullptr;

ACEDetails aceDetails[] =
{
{ ACEPrincipal::CurrentUser, userToken->User.Sid, TRUSTEE_IS_USER },
{ ACEPrincipal::Admins, adminSID.get(), TRUSTEE_IS_WELL_KNOWN_GROUP},
{ ACEPrincipal::System, systemSID.get(), TRUSTEE_IS_USER},
};

ULONG entriesCount = 0;
std::array<EXPLICIT_ACCESS_W, ARRAYSIZE(aceDetails)> explicitAccess;

// If the current user is SYSTEM, we want to take either the owner or the only configured set of permissions.
// The check above should prevent us from getting into situations outside of the ones below.
std::optional<ACEPrincipal> principalToIgnore;
if (hasCurrentUser && hasSystem && EqualSid(userToken->User.Sid, systemSID.get()))
{
principalToIgnore = (Owner.value() == ACEPrincipal::CurrentUser ? ACEPrincipal::System : ACEPrincipal::CurrentUser);
}

for (const auto& ace : aceDetails)
{
if (principalToIgnore && principalToIgnore.value() == ace.Principal)
{
continue;
}

if (Owner && Owner.value() == ace.Principal)
{
ownerSID = ace.SID;
}

auto itr = ACL.find(ace.Principal);
if (itr != ACL.end())
{
EXPLICIT_ACCESS_W& entry = explicitAccess[entriesCount++];
entry = {};

entry.grfAccessPermissions = AccessPermissionsFrom(itr->second);
entry.grfAccessMode = SET_ACCESS;
entry.grfInheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE;

entry.Trustee.pMultipleTrustee = nullptr;
entry.Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
entry.Trustee.TrusteeForm = TRUSTEE_IS_SID;
entry.Trustee.TrusteeType = ace.TrusteeType;
entry.Trustee.ptstrName = reinterpret_cast<LPWCH>(ace.SID);
}
}

wil::unique_any<PACL, decltype(&::LocalFree), ::LocalFree> acl;
THROW_IF_WIN32_ERROR(SetEntriesInAclW(entriesCount, explicitAccess.data(), nullptr, &acl));

std::wstring path = Path.wstring();
SECURITY_INFORMATION securityInformation = DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION;

if (ownerSID)
{
securityInformation |= OWNER_SECURITY_INFORMATION;
}

THROW_IF_WIN32_ERROR(SetNamedSecurityInfoW(&path[0], SE_FILE_OBJECT, securityInformation, ownerSID, nullptr, acl.get(), nullptr));
}

// Contains all of the paths that are common between the runtime contexts.
PathDetails GetPathDetailsCommon(PathName path, bool forDisplay)
{
Expand Down Expand Up @@ -616,38 +486,6 @@ namespace AppInstaller::Runtime
return result;
}

std::filesystem::path GetPathTo(PathName path, bool forDisplay)
{
PathDetails details = GetPathDetailsFor(path, forDisplay);

if (details.Create)
{
if (details.Path.is_absolute())
{
if (std::filesystem::exists(details.Path) && !std::filesystem::is_directory(details.Path))
{
std::filesystem::remove(details.Path);
}

std::filesystem::create_directories(details.Path);

// Set the ACLs on the directory if needed. We do this after creating the directory because an attacker could
// have created the directory beforehand so we must be able to place the correct ACL on any directory or fail
// to operate.
if (details.ShouldApplyACL())
{
details.ApplyACL();
}
}
else
{
AICLI_LOG(Core, Warning, << "GetPathTo directory creation requested for [" << path << "], but path was not absolute: " << details.Path);
}
}

return std::move(details.Path);
}

std::filesystem::path GetNewTempFilePath()
{
GUID guid;
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@
<ClInclude Include="Public\winget\Certificates.h" />
<ClInclude Include="Public\winget\Compression.h" />
<ClInclude Include="Public\winget\ConfigurationSetProcessorHandlers.h" />
<ClInclude Include="Public\winget\Filesystem.h" />
<ClInclude Include="Public\winget\GroupPolicy.h" />
<ClInclude Include="Public\winget\IConfigurationStaticsInternals.h" />
<ClInclude Include="Public\winget\ILifetimeWatcher.h" />
Expand Down Expand Up @@ -439,6 +440,7 @@
<ClCompile Include="Compression.cpp" />
<ClCompile Include="DateTime.cpp" />
<ClCompile Include="Errors.cpp" />
<ClCompile Include="Filesystem.cpp" />
<ClCompile Include="GroupPolicy.cpp" />
<ClCompile Include="ICU\SQLiteICU.c">
<PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">NotUsing</PrecompiledHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@
<ClInclude Include="Public\winget\Compression.h">
<Filter>Public\winget</Filter>
</ClInclude>
<ClInclude Include="Public\winget\Filesystem.h">
<Filter>Public\winget</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ClCompile Include="pch.cpp">
Expand Down Expand Up @@ -214,6 +217,9 @@
<ClCompile Include="Compression.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="Filesystem.cpp">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down