Skip to content

Commit

Permalink
Fix reverting impersonation
Browse files Browse the repository at this point in the history
  • Loading branch information
@nirbar
nirbar committed Apr 2, 2024
1 parent 94ebebf commit 333aef2
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 34 deletions.
54 changes: 32 additions & 22 deletions src/PanelSwCustomActions/ExecOnComponent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,13 +617,12 @@ HRESULT CExecOnComponent::ExecuteOne(const com::panelsw::ca::ExecOnDetails& deta
LPCWSTR szDomain = nullptr;
LPCWSTR szUser = nullptr;
LPCWSTR szPassword = nullptr;
HANDLE hImpersonation = NULL;
bool bImpersonated = false;
CWixString szLog;
HANDLE hStdOut = NULL;
HANDLE hProc = NULL;
PMSIHANDLE hActionData;
CWixString szEnvironmentMultiSz;
IMPERSONATION_CONTEXT ctxImpersonation;

hr = szCommand.Copy((LPCWSTR)(LPVOID)details.command().plain().data());
ExitOnFailure(hr, "Failed to copy string");
Expand All @@ -649,9 +648,8 @@ HRESULT CExecOnComponent::ExecuteOne(const com::panelsw::ca::ExecOnDetails& deta
}

// We only impersonate for the duration of the process creation because I've encountered crashes when logging impersonated
hr = Impersonate(szDomain, szUser, szPassword, details.sessionid(), &hImpersonation, &szEnvironmentMultiSz);
hr = Impersonate(szDomain, szUser, szPassword, details.sessionid(), &szEnvironmentMultiSz, &ctxImpersonation);
ExitOnFailure(hr, "Failed to impersonate");
bImpersonated = (hr == S_OK);

hr = SetEnvironment(&szEnvironmentMultiSz, details.environment());
ExitOnFailure(hr, "Failed setting environment");
Expand All @@ -665,14 +663,7 @@ HRESULT CExecOnComponent::ExecuteOne(const com::panelsw::ca::ExecOnDetails& deta
ExitFunction();
}

if (bImpersonated)
{
bRes = RevertToSelf();
ExitOnNullWithLastError(bRes, hr, "Failed reverting impersonation");
bImpersonated = false;

ReleaseHandle(hImpersonation);
}
Unimpersonate(&ctxImpersonation);

if (SUCCEEDED(hr))
{
Expand Down Expand Up @@ -728,11 +719,7 @@ HRESULT CExecOnComponent::ExecuteOne(const com::panelsw::ca::ExecOnDetails& deta
ExitOnFailure(hr, "Failed to execute command '%ls'", szObfuscatedCommand);

LExit:
if (bImpersonated)
{
::RevertToSelf();
}
ReleaseHandle(hImpersonation);
Unimpersonate(&ctxImpersonation);
if (hStdOut && (hStdOut != INVALID_HANDLE_VALUE))
{
ReleaseHandle(hStdOut);
Expand Down Expand Up @@ -973,7 +960,7 @@ HRESULT CExecOnComponent::SetEnvironment(CWixString* pszEnvironmentMultiSz, cons
{
LPCWSTR szEqual = wcschr(szCurr, L'=');;
DWORD dwLen1 = szEqual - szCurr;
if (!dwLen1)
if (!dwLen1 || !szEqual)
{
continue;
}
Expand Down Expand Up @@ -1096,6 +1083,7 @@ HRESULT CExecOnComponent::LaunchProcess(LPWSTR szCommand, LPCWSTR szWorkingDirec

bRes = ::CreateProcessW(nullptr, szCommand, nullptr, nullptr, TRUE, ::GetPriorityClass(::GetCurrentProcess()) | CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT, (LPVOID)rgszEnvironment, szWorkingDirectory, &si, &pi);
ExitOnNullWithLastError(bRes, hr, "Failed to create process");
WcaLog(LOGLEVEL::LOGMSG_VERBOSE, "Launched process '%ls'", szCommand);

if (phStdOut)
{
Expand All @@ -1122,7 +1110,7 @@ HRESULT CExecOnComponent::LaunchProcess(LPWSTR szCommand, LPCWSTR szWorkingDirec
return hr;
}

HRESULT CExecOnComponent::Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR szPassword, DWORD dwSessionId, HANDLE* phUserToken, CWixString* pszEnvironmentMultiSz)
HRESULT CExecOnComponent::Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR szPassword, DWORD dwSessionId, CWixString* pszEnvironmentMultiSz, IMPERSONATION_CONTEXT *pctxImpersonate)
{
HRESULT hr = S_OK;
BOOL bRes = TRUE;
Expand All @@ -1135,13 +1123,14 @@ HRESULT CExecOnComponent::Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR
CWixString szTempUserName;

ZeroMemory(&profileInfo, sizeof(profileInfo));
Unimpersonate(pctxImpersonate);

// Impersonate current user?
if (dwSessionId != WTS_CURRENT_SESSION)
{
DWORD dwNameSize = 0;

bRes = ::WTSQuerySessionInformationW(NULL, dwSessionId, WTS_INFO_CLASS::WTSUserName, &szSessionUserName, &dwNameSize);
bRes = ::WTSQuerySessionInformation(NULL, dwSessionId, WTS_INFO_CLASS::WTSUserName, &szSessionUserName, &dwNameSize);
ExitOnNullWithLastError(bRes, hr, "Failed to get user name");
WcaLog(LOGLEVEL::LOGMSG_STANDARD, "Impersonating '%ls'", szSessionUserName);

Expand Down Expand Up @@ -1180,7 +1169,7 @@ HRESULT CExecOnComponent::Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR

if (!::LoadUserProfile(hUserToken, &profileInfo))
{
WcaLogError(HRESULT_FROM_WIN32(::GetLastError()), "Failed to load user profile");
WcaLogError(HRESULT_FROM_WIN32(::GetLastError()), "Failed to load user profile, so running without it");
}
}

Expand All @@ -1200,6 +1189,12 @@ HRESULT CExecOnComponent::Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR
ExitOnFailure(hr, "Failed to insert string to array");
}

pctxImpersonate->bImpersonated = bImpersonated;
pctxImpersonate->hUserToken = hUserToken;
pctxImpersonate->hProfile = profileInfo.hProfile;
hUserToken = NULL;
profileInfo.hProfile = NULL;

LExit:
if (pEnvironment)
{
Expand All @@ -1221,6 +1216,21 @@ HRESULT CExecOnComponent::Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR
::NetApiBufferFree(pUserInfo4);
pUserInfo4 = nullptr;
}
return SUCCEEDED(hr) ? bImpersonated ? S_OK : S_FALSE : hr;
}

return SUCCEEDED(hr) ? bImpersonated ? S_FALSE : S_OK : hr;
void CExecOnComponent::Unimpersonate(IMPERSONATION_CONTEXT* pctxImpersonate)
{
if (pctxImpersonate->hUserToken && pctxImpersonate->hProfile)
{
::UnloadUserProfile(pctxImpersonate->hUserToken, pctxImpersonate->hProfile);
pctxImpersonate->hProfile = NULL;
}
if (pctxImpersonate->bImpersonated)
{
WcaLog(LOGLEVEL::LOGMSG_VERBOSE, "Reverting impersonation");
::RevertToSelf();
pctxImpersonate->bImpersonated = FALSE;
}
ReleaseHandle(pctxImpersonate->hUserToken);
}
11 changes: 10 additions & 1 deletion src/PanelSwCustomActions/ExecOnComponent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,18 @@ class CExecOnComponent :

private:

struct IMPERSONATION_CONTEXT
{
HANDLE hUserToken = NULL;
BOOL bImpersonated = FALSE;
HANDLE hProfile = NULL;
};

HRESULT ExecuteOne(const com::panelsw::ca::ExecOnDetails &details);

HRESULT Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR szPassword, DWORD dwSessionId, HANDLE* phUserToken, CWixString* pszEnvironmentMultiSz);
HRESULT Impersonate(LPCWSTR szDomain, LPCWSTR szUser, LPCWSTR szPassword, DWORD dwSessionId, CWixString* pszEnvironmentMultiSz, IMPERSONATION_CONTEXT* pctxImpersonation);

void Unimpersonate(IMPERSONATION_CONTEXT* pctxImpersonation);

HRESULT SetEnvironment(CWixString *pszEnvironmentMultiSz, const ::google::protobuf::Map<std::string, com::panelsw::ca::ObfuscatedString> &customEnv);

Expand Down
2 changes: 1 addition & 1 deletion src/TidyBuild.custom.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Project ToolsVersion="16.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildThisFileDirectory)TidyBuild.user.props" Condition="Exists('$(MSBuildThisFileDirectory)TidyBuild.user.props')"/>
<PropertyGroup>
<FullVersion>3.18.11</FullVersion>
<FullVersion>3.18.12</FullVersion>
<FullVersion Condition=" '$(GITHUB_RUN_NUMBER)'!='' ">$(FullVersion).$(GITHUB_RUN_NUMBER)</FullVersion>
<ProductName>PanelSwWixExtension</ProductName>
<Manufacturer>Panel::Software</Manufacturer>
Expand Down
20 changes: 10 additions & 10 deletions src/wixlib/PanelSwWixExtension.wxs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,19 +311,19 @@

<InstallExecuteSequence>
<Custom Action="ExecOnComponent" Before="ExecOnComponentRollback" Overridable="yes" />
<Custom Action="ExecOnComponentRollback" Before="ProcessComponents" Overridable="yes" />
<Custom Action="ExecOnComponentCommit" Before="InstallFinalize" Overridable="yes" />
<Custom Action="ExecOnComponent_BeforeStop_rollback" Before="ExecOnComponent_BeforeStop_deferred" Overridable="yes" />
<Custom Action="ExecOnComponent_BeforeStop_deferred" Before="StopServices" Overridable="yes" />
<Custom Action="ExecOnComponentRollback" Before="ProcessComponents" Overridable="yes" Condition="ExecOnComponentRollback" />
<Custom Action="ExecOnComponentCommit" Before="InstallFinalize" Overridable="yes" Condition="ExecOnComponentCommit" />
<Custom Action="ExecOnComponent_BeforeStop_rollback" Before="ExecOnComponent_BeforeStop_deferred" Overridable="yes" Condition="ExecOnComponent_BeforeStop_rollback" />
<Custom Action="ExecOnComponent_BeforeStop_deferred" Before="StopServices" Overridable="yes" Condition="ExecOnComponent_BeforeStop_deferred" />

<Custom Action="ExecOnComponent_AfterStop_rollback" Before="ExecOnComponent_AfterStop_deferred" Overridable="yes" />
<Custom Action="ExecOnComponent_AfterStop_deferred" After="StopServices" Overridable="yes" />
<Custom Action="ExecOnComponent_AfterStop_rollback" Before="ExecOnComponent_AfterStop_deferred" Overridable="yes" Condition="ExecOnComponent_AfterStop_rollback" />
<Custom Action="ExecOnComponent_AfterStop_deferred" After="StopServices" Overridable="yes" Condition="ExecOnComponent_AfterStop_deferred" />

<Custom Action="ExecOnComponent_BeforeStart_rollback" Before="ExecOnComponent_BeforeStart_deferred" Overridable="yes" />
<Custom Action="ExecOnComponent_BeforeStart_deferred" Before="StartServices" Overridable="yes" />
<Custom Action="ExecOnComponent_BeforeStart_rollback" Before="ExecOnComponent_BeforeStart_deferred" Overridable="yes" Condition="ExecOnComponent_BeforeStart_rollback" />
<Custom Action="ExecOnComponent_BeforeStart_deferred" Before="StartServices" Overridable="yes" Condition="ExecOnComponent_BeforeStart_deferred" />

<Custom Action="ExecOnComponent_AfterStart_rollback" Before="ExecOnComponent_AfterStart_deferred" Overridable="yes" />
<Custom Action="ExecOnComponent_AfterStart_deferred" After="StartServices" Overridable="yes" />
<Custom Action="ExecOnComponent_AfterStart_rollback" Before="ExecOnComponent_AfterStart_deferred" Overridable="yes" Condition="ExecOnComponent_AfterStart_rollback" />
<Custom Action="ExecOnComponent_AfterStart_deferred" After="StartServices" Overridable="yes" Condition="ExecOnComponent_AfterStart_deferred" />
</InstallExecuteSequence>

<UI>
Expand Down

0 comments on commit 333aef2

Please sign in to comment.