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

Installation with System-Account doesn't work #2546 #2565

Merged
merged 6 commits into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
25 changes: 25 additions & 0 deletions dev/Common/Security.User.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

#ifndef __SECURITY_USER_H
#define __SECURITY_USER_H

namespace Security::User
{
inline bool IsLocalSystem(HANDLE token = nullptr)
{
BYTE localSystemSidBuffer[ SECURITY_MAX_SID_SIZE ];
PSID localSystemSid{ reinterpret_cast<PSID>(localSystemSidBuffer) };
DWORD localSystemSidBufferSize{ ARRAYSIZE(localSystemSidBuffer) };
THROW_IF_WIN32_BOOL_FALSE(CreateWellKnownSid(WinLocalSystemSid, nullptr, localSystemSid, &localSystemSidBufferSize));

wistd::unique_ptr<TOKEN_USER> user{
wil::get_token_information<TOKEN_USER>(
!token ? GetCurrentThreadEffectiveToken() : token) };
PSID userSid{ user->User.Sid };

return !!EqualSid(userSid, localSystemSid);
}
}

#endif // __SECURITY_USER_H
15 changes: 15 additions & 0 deletions installer/dev/InstallActivityContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,21 @@ BOOL WindowsAppRuntimeInstaller::InstallActivity::Context::LogInstallerFailureEv
THROW_IF_WIN32_BOOL_FALSE(LogInstallerFailureEventWithResourceId(EVENTLOG_WARNING_TYPE, hresult, customProvisionMessage));
break;
}
case InstallStage::StagePackage:
{
WCHAR customMessage[1024]{};
auto deploymentActivityId{ winrt::to_hstring(*m_activity.Id()) };
auto customMessageFormat{ L"Staging Package %s with DeploymentExtendedError: 0x%08X, DeploymentExtendedText:%s, DeploymentActivityId: %s" };
FAIL_FAST_IF_FAILED(StringCchPrintfW(customMessage,
ARRAYSIZE(customMessage),
customMessageFormat,
m_currentResourceId,
m_deploymentErrorExtendedHresult,
m_deploymentErrorText,
m_deploymentErrorActivityId));
THROW_IF_WIN32_BOOL_FALSE(LogInstallerFailureEventWithResourceId(EVENTLOG_ERROR_TYPE, hresult, customMessage));
break;
}
default:
break;
}
Expand Down
1 change: 1 addition & 0 deletions installer/dev/InstallActivityContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace WindowsAppRuntimeInstaller::InstallActivity
RegisterPackage = 0x5,
ProvisionPackage = 0x6,
RestartPushNotificationsLRP = 0x7,
StagePackage = 0x8,
};

struct WilFailure
Expand Down
8 changes: 4 additions & 4 deletions installer/dev/console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void WindowsAppRuntimeInstaller::Console::DisplayError(const HRESULT hr)

wil::unique_hlocal_ptr<WCHAR[]> message;
if (FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
nullptr, hr, 0, reinterpret_cast<PWSTR>(&message), 0, nullptr) != 0)
nullptr, hr, 0, reinterpret_cast<PWSTR>(&message), 0, nullptr) != 0)
{
std::wcout << message.get();
}
Expand All @@ -55,8 +55,9 @@ void WindowsAppRuntimeInstaller::Console::DisplayError(const HRESULT hr)
// Don't log redundant Hr information
if (installActivityContext.GetDeploymentErrorExtendedHResult() &&
installActivityContext.GetDeploymentErrorExtendedHResult() != hr &&
(installActivityContext.GetInstallStage() == InstallStage::AddPackage ||
installActivityContext.GetInstallStage() == InstallStage::RegisterPackage))
(installActivityContext.GetInstallStage() == InstallStage::StagePackage ||
installActivityContext.GetInstallStage() == InstallStage::AddPackage ||
installActivityContext.GetInstallStage() == InstallStage::RegisterPackage))
{
std::wcout << "ExtendedError: 0x" << std::hex << installActivityContext.GetDeploymentErrorExtendedHResult() << " ";

Expand All @@ -74,4 +75,3 @@ void WindowsAppRuntimeInstaller::Console::DisplayError(const HRESULT hr)
std::wcout << "ErrorMessage: " << installActivityContext.GetDeploymentErrorText();
}
}

75 changes: 61 additions & 14 deletions installer/dev/install.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,25 @@ namespace WindowsAppRuntimeInstaller
return S_OK;
}

HRESULT AddPackage(const Uri& packageUri, const std::unique_ptr<PackageProperties>& packageProperties, bool forceDeployment)
HRESULT AddPackage(
WindowsAppRuntimeInstaller::InstallActivity::Context& installActivityContext,
const Uri& packageUri,
const std::unique_ptr<PackageProperties>& packageProperties,
bool forceDeployment)
{
PackageManager packageManager;

const auto deploymentOptions{ forceDeployment ?
winrt::Windows::Management::Deployment::DeploymentOptions::ForceTargetApplicationShutdown :
winrt::Windows::Management::Deployment::DeploymentOptions::None };
winrt::Windows::Management::Deployment::DeploymentOptions::ForceTargetApplicationShutdown :
winrt::Windows::Management::Deployment::DeploymentOptions::None };

PackageManager packageManager;
const auto deploymentOperation{ packageManager.AddPackageAsync(packageUri, nullptr, DeploymentOptions::None) };
deploymentOperation.get();
if (deploymentOperation.Status() != AsyncStatus::Completed)
{
auto hrAddPackage{ static_cast<HRESULT>(deploymentOperation.ErrorCode()) };
if (hrAddPackage == ERROR_PACKAGE_ALREADY_EXISTS)
{
WindowsAppRuntimeInstaller::InstallActivity::Context::Get().SetInstallStage(InstallStage::RegisterPackage);
installActivityContext.SetInstallStage(InstallStage::RegisterPackage);

// Package already exists (such as via provisioning), re-register it instead.
RETURN_IF_FAILED(RegisterPackage(packageProperties->fullName.get()));
Expand All @@ -61,14 +64,61 @@ namespace WindowsAppRuntimeInstaller
else
{
const auto deploymentResult{ deploymentOperation.GetResults() };
WindowsAppRuntimeInstaller::InstallActivity::Context::Get().SetDeploymentErrorInfo(deploymentResult.ExtendedErrorCode(), deploymentResult.ErrorText().c_str(), deploymentResult.ActivityId());

installActivityContext.SetDeploymentErrorInfo(deploymentResult.ExtendedErrorCode(), deploymentResult.ErrorText().c_str(), deploymentResult.ActivityId());
RETURN_HR(hrAddPackage);
}
}
return S_OK;
}

HRESULT StagePackage(
WindowsAppRuntimeInstaller::InstallActivity::Context& installActivityContext,
const Uri& packageUri)
{
const auto deploymentOptions{ winrt::Windows::Management::Deployment::DeploymentOptions::None };

PackageManager packageManager;
const auto deploymentOperation{ packageManager.StagePackageAsync(packageUri, nullptr, DeploymentOptions::None) };
deploymentOperation.get();
if (deploymentOperation.Status() != AsyncStatus::Completed)
{
auto hrStagePackage{ static_cast<HRESULT>(deploymentOperation.ErrorCode()) };
if (hrStagePackage == ERROR_PACKAGE_ALREADY_EXISTS)
{
// Package already exists, nothing more to do
return S_OK;
}
else
{
const auto deploymentResult{ deploymentOperation.GetResults() };
installActivityContext.SetDeploymentErrorInfo(deploymentResult.ExtendedErrorCode(), deploymentResult.ErrorText().c_str(), deploymentResult.ActivityId());
RETURN_HR(hrStagePackage);
}
}
return S_OK;
}

HRESULT AddOrStagePackage(
WindowsAppRuntimeInstaller::InstallActivity::Context& installActivityContext,
const Uri& packageUri,
const std::unique_ptr<PackageProperties>& packageProperties,
bool forceDeployment)
{
// Windows doesn't support registering packages for LocalSystem
// If you're doing that you're really intending to provision the package for all users on the machine
// That means we need to Stage the package instead of Add it
if (Security::User::IsLocalSystem())
{
installActivityContext.SetInstallStage(InstallStage::StagePackage);
RETURN_IF_FAILED(StagePackage(installActivityContext, packageUri));
}
else
{
installActivityContext.SetInstallStage(InstallStage::AddPackage);
RETURN_IF_FAILED(AddPackage(installActivityContext, packageUri, packageProperties, forceDeployment));
}
}

HRESULT ProvisionPackage(const std::wstring& packageFamilyName)
{
PackageManager packageManager;
Expand Down Expand Up @@ -252,11 +302,9 @@ namespace WindowsAppRuntimeInstaller
THROW_IF_FAILED(outStream->Commit(STGC_OVERWRITE));
outStream.reset();

installActivityContext.SetInstallStage(InstallStage::AddPackage);

// Add the package
// Add-or-Stage the package
Uri packageUri{ packageFilename };
auto hrAddResult{ AddPackage(packageUri, packageProperties, forceDeployment) };
auto hrAddResult{ AddOrStagePackage(installActivityContext, packageUri, packageProperties, forceDeployment) };
if (!quiet)
{
std::wcout << "Package deployment result : 0x" << std::hex << hrAddResult << " ";
Expand All @@ -266,8 +314,7 @@ namespace WindowsAppRuntimeInstaller

// Framework provisioning is not supported by the PackageManager ProvisionPackageForAllUsersAsync API.
// Hence, skip attempting to provision framework package.
if (!packageProperties->isFramework &&
Security::IntegrityLevel::IsElevated())
if (!packageProperties->isFramework && Security::IntegrityLevel::IsElevated())
{
installActivityContext.SetInstallStage(InstallStage::ProvisionPackage);

Expand Down
1 change: 1 addition & 0 deletions installer/dev/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <winrt/Windows.System.h>

#include <Security.IntegrityLevel.h>
#include <Security.User.h>
#include <PushNotifications-Constants.h>

#include "tracelogging.h"
Expand Down
1 change: 1 addition & 0 deletions test/Common/Common.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
<ClCompile Include="pch.cpp">
<PrecompiledHeader>Create</PrecompiledHeader>
</ClCompile>
<ClCompile Include="Test_Security_User.cpp" />
<ClCompile Include="Test_SelfContained.cpp" />
<ClCompile Include="Test_Utf8.cpp" />
</ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions test/Common/Common.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
<ClCompile Include="Test_Utf8.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="Test_Security_User.cpp">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="pch.h">
Expand Down
35 changes: 35 additions & 0 deletions test/Common/Test_Security_User.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

#include "pch.h"

namespace Test::Common
{
class SecurityUserTests_User
{
public:
BEGIN_TEST_CLASS(SecurityUserTests_User)
TEST_CLASS_PROPERTY(L"ThreadingModel", L"MTA")
TEST_CLASS_PROPERTY(L"RunAs", L"RestrictedUser")
END_TEST_CLASS()

TEST_METHOD(IsLocalSystem)
{
VERIFY_IS_FALSE(::Security::User::IsLocalSystem());
}
};

class SecurityUserTests_System
{
public:
BEGIN_TEST_CLASS(SecurityUserTests_System)
TEST_CLASS_PROPERTY(L"ThreadingModel", L"MTA")
TEST_CLASS_PROPERTY(L"RunAs", L"System")
END_TEST_CLASS()

TEST_METHOD(IsLocalSystem)
{
VERIFY_IS_TRUE(::Security::User::IsLocalSystem());
}
};
}
1 change: 1 addition & 0 deletions test/Common/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <WexTestClass.h>

#include <Microsoft.Utf8.h>
#include <Security.User.h>
#include <WindowsAppRuntime.SelfContained.h>
#include <WindowsAppRuntime.VersionInfo.h>

Expand Down