Skip to content

Commit

Permalink
Add a file for storing elevated-only state (#11222)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

This creates an `elevated-state.json` that lives in `%LOCALAPPDATA%` next to `state.json`, that's only writable when elevated. It doesn't _use_ this file for anything, it just puts the framework down for use later.

It's _just like `ApplicationState`_. We'll use it the same way. 

It's readable when unelevated, which is nice, but not writable. If you're dumb and try to write to the file when unelevated, it'll just silently do nothing.

If we try opening the file and find out the permissions are different, we'll _blow the file away entirely_. This is to prevent someone from renaming the original file (which they can do unelevated), then slapping a new file that's writable by them down in it's place. 

## References
* We're going to use this in #11096, but these PRs need to be broken up.

## PR Checklist
* [x] Closes nothing
* [x] I work here
* [x] Tests added/passed
* [ ] Requires documentation to be updated - maybe? not sure we have docs on `state.json` at all yet

## Validation Steps Performed
I've played with this much more in `dev/migrie/f/non-terminal-content-elevation-warning`

###### followed by #11308, #11310
  • Loading branch information
zadjii-msft authored Nov 13, 2021
1 parent 2353349 commit c79334f
Show file tree
Hide file tree
Showing 15 changed files with 511 additions and 265 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/allow.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
admins
apc
Apc
bsd
Expand Down
7 changes: 7 additions & 0 deletions .github/actions/spelling/allow/apis.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
ACCEPTFILES
ACCESSDENIED
acl
aclapi
alignas
alignof
APPLYTOSUBMENUS
Expand All @@ -21,6 +23,7 @@ commandlinetoargv
cstdint
CXICON
CYICON
Dacl
dataobject
dcomp
DERR
Expand Down Expand Up @@ -117,15 +120,19 @@ OSVERSIONINFOEXW
otms
OUTLINETEXTMETRICW
overridable
PACL
PAGESCROLL
PEXPLICIT
PICKFOLDERS
pmr
ptstr
rcx
REGCLS
RETURNCMD
rfind
roundf
RSHIFT
SACL
schandle
semver
serializer
Expand Down
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/names.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Diviness
dsafa
duhowett
ekg
eryksun
ethanschoonover
Firefox
Gatta
Expand Down
59 changes: 21 additions & 38 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include <WtExeUtils.h>
#include <wil/token_helpers.h >

#include "../../types/inc/utils.hpp"

using namespace winrt::Windows::ApplicationModel;
using namespace winrt::Windows::ApplicationModel::DataTransfer;
using namespace winrt::Windows::UI::Xaml;
Expand Down Expand Up @@ -131,38 +133,6 @@ static Documents::Run _BuildErrorRun(const winrt::hstring& text, const ResourceD
return textRun;
}

// Method Description:
// - Returns whether the user is either a member of the Administrators group or
// is currently elevated.
// - This will return **FALSE** if the user has UAC disabled entirely, because
// there's no separation of power between the user and an admin in that case.
// Return Value:
// - true if the user is an administrator
static bool _isUserAdmin() noexcept
try
{
wil::unique_handle processToken{ GetCurrentProcessToken() };
const auto elevationType = wil::get_token_information<TOKEN_ELEVATION_TYPE>(processToken.get());
const auto elevationState = wil::get_token_information<TOKEN_ELEVATION>(processToken.get());
if (elevationType == TokenElevationTypeDefault && elevationState.TokenIsElevated)
{
// In this case, the user has UAC entirely disabled. This is sort of
// weird, we treat this like the user isn't an admin at all. There's no
// separation of powers, so the things we normally want to gate on
// "having special powers" doesn't apply.
//
// See GH#7754, GH#11096
return false;
}

return wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
return false;
}

namespace winrt::TerminalApp::implementation
{
// Function Description:
Expand Down Expand Up @@ -214,7 +184,7 @@ namespace winrt::TerminalApp::implementation
// The TerminalPage has to be constructed during our construction, to
// make sure that there's a terminal page for callers of
// SetTitleBarContent
_isElevated = _isUserAdmin();
_isElevated = ::Microsoft::Console::Utils::IsElevated();
_root = winrt::make_self<TerminalPage>();

_reloadSettings = std::make_shared<ThrottledFuncTrailing<>>(winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() {
Expand Down Expand Up @@ -910,8 +880,6 @@ namespace winrt::TerminalApp::implementation
void AppLogic::_RegisterSettingsChange()
{
const std::filesystem::path settingsPath{ std::wstring_view{ CascadiaSettings::SettingsPath() } };
const std::filesystem::path statePath{ std::wstring_view{ ApplicationState::SharedInstance().FilePath() } };

_reader.create(
settingsPath.parent_path().c_str(),
false,
Expand All @@ -920,14 +888,29 @@ namespace winrt::TerminalApp::implementation
// editors, who will write a temp file, then rename it to be the
// actual file you wrote. So listen for that too.
wil::FolderChangeEvents::FileName | wil::FolderChangeEvents::LastWriteTime,
[this, settingsBasename = settingsPath.filename(), stateBasename = statePath.filename()](wil::FolderChangeEvent, PCWSTR fileModified) {
const auto modifiedBasename = std::filesystem::path{ fileModified }.filename();
[this, settingsBasename = settingsPath.filename()](wil::FolderChangeEvent, PCWSTR fileModified) {
// DO NOT create a static reference to ApplicationState::SharedInstance here.
//
// ApplicationState::SharedInstance already caches its own
// static ref. If _we_ keep a static ref to the member in
// AppState, then our reference will keep ApplicationState alive
// after the `ActionToStringMap` gets cleaned up. Then, when we
// try to persist the actions in the window state, we won't be
// able to. We'll try to look up the action and the map just
// won't exist. We'll explode, even though the Terminal is
// tearing down anyways. So we'll just die, but still invoke
// WinDBG's post-mortem debugger, who won't be able to attach to
// the process that's already exiting.
//
// So DON'T ~give a mouse a cookie~ take a static ref here.

const winrt::hstring modifiedBasename{ std::filesystem::path{ fileModified }.filename().c_str() };

if (modifiedBasename == settingsBasename)
{
_reloadSettings->Run();
}
else if (modifiedBasename == stateBasename)
else if (ApplicationState::SharedInstance().IsStatePath(modifiedBasename))
{
_reloadState();
}
Expand Down
3 changes: 0 additions & 3 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,7 @@ namespace winrt::TerminalApp::implementation
// - true if the ApplicationState should be used.
bool TerminalPage::ShouldUsePersistedLayout(CascadiaSettings& settings) const
{
// GH#5000 Until there is a separate state file for elevated sessions we should just not
// save at all while in an elevated window.
return Feature_PersistedWindowLayout::IsEnabled() &&
!IsElevated() &&
settings.GlobalSettings().FirstWindowPreference() == FirstWindowPreference::PersistedWindowLayout;
}

Expand Down
Loading

0 comments on commit c79334f

Please sign in to comment.