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

Fix runas function for System Account #34292

Merged
merged 3 commits into from
Jun 30, 2016
Merged

Conversation

twangboy
Copy link
Contributor

What does this PR do?

Use the elevated token if present when passing runas with an admin account. Only fixes when Salt is running under the LocalSystem account. Need to fix for debug mode under an admin account.

What issues does this PR fix or reference?

https://github.com/saltstack/zh/issues/766

Previous Behavior

When admin credentials were passed to Runas the restricted token was used instead of the full elevated privileges of the user.

New Behavior

Uses the elevated token.

Tests written?

No

@cachedout
Copy link
Contributor

Since I don't understand how this works at all, I would at least like @UtahDave to take a look here so we can get more eyes on this.

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jun 27, 2016
@twangboy
Copy link
Contributor Author

@cachedout When you login as an Administrator on Windows it gives you a restricted token so you're running like a normal user (without admin). When you do something that needs admin, Windows hits you with the UAC Dialog to remove the restrictions on your token. This is like sudo in linux, I believe.

In order to runas on Windows we have to log in (that happens a few lines above). We need to run with the elevated token if it's an admin account, so we get the unrestricted token using the GetTokenInformation command. Then we use that token to execute our command with elevated privileges.

@UtahDave
Copy link
Contributor

@twangboy what happens if the user doesn't have an elevated security token? Will line 302 return the regular token? Or will it stacktrace?

@twangboy
Copy link
Contributor Author

@UtahDave It will stacktrace

@UtahDave
Copy link
Contributor

@twangboy, do we want that to stacktrace? Or should we catch that and do something else, like return an error message or try with the regular security token?

@twangboy
Copy link
Contributor Author

Working on checking for admin before elevating... Of course I don't want it to stacktrace. I didn't check that scenario.

@twangboy
Copy link
Contributor Author

@UtahDave ^^^

@UtahDave
Copy link
Contributor

@twangboy looks like this needs to be rebased.

@twangboy
Copy link
Contributor Author

Rebased

@twangboy
Copy link
Contributor Author

@cachedout Is this OK now?

@cachedout cachedout merged commit ad63b1d into saltstack:2016.3 Jun 30, 2016
@twangboy twangboy deleted the fix_runas branch August 30, 2016 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants