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

win become: refactor and add support for passwordless become #48082

Merged
merged 7 commits into from Dec 13, 2018

Conversation

Projects
None yet
6 participants
@jborean93
Contributor

jborean93 commented Nov 5, 2018

SUMMARY

A refactoring of the become and process utils used in Windows. This PR does the following;

  • Move the C# code that calls CreateProcess to a C# util
  • Refactor the become C# code to share the code in the new process util above
  • Also add support for become to another account without setting a password
  • Expand the tests to cover more scenarios

Fixes #34343

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

Ansible.Become.cs
Ansible.Process.cs

ANSIBLE VERSION
devel
@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 5, 2018

Hi @jborean93, thank you for submitting this pull-request!

click here for bot help

@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 5, 2018

been lifted in Ansible 2.5 and a user that is a member of the
``BUILTIN\Administrators`` group should have an elevated token during the
module execution.
been lifted in Ansible 2.5 and the remote user only requires the

This comment has been minimized.

@acozine

acozine Nov 5, 2018

Contributor

If you split out the 2.5-and-forward docs changes, like this section, into a separate PR, it will be easier to backport the right set of changes to 2.7, 2.6, and 2.5.

This comment has been minimized.

@jborean93

jborean93 Nov 5, 2018

Contributor

Ok, will do the doc fixups in another PR and just include the new features in this one.

This comment has been minimized.

@jborean93

jborean93 Nov 6, 2018

Contributor

Moved all the stuff that isn't related to this new feature here #48149

@ansibot ansibot removed the needs_triage label Nov 5, 2018

@bcoca bcoca removed needs_triage labels Nov 5, 2018

@mattclay

This comment has been minimized.

Member

mattclay commented Nov 5, 2018

@mattclay mattclay added the ci_verified label Nov 5, 2018

@jborean93 jborean93 force-pushed the jborean93:win-process-utils branch from a3dc912 to 9ec319c Nov 6, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 6, 2018

@ansibot ansibot removed the ci_verified label Nov 6, 2018

@jborean93 jborean93 force-pushed the jborean93:win-process-utils branch from 9ec319c to ad0da20 Nov 6, 2018

@ansibot ansibot added core_review and removed needs_revision labels Nov 6, 2018

@jborean93

This comment has been minimized.

Contributor

jborean93 commented Nov 20, 2018

The issue has been solved. The reason why it was failing is because the code has moved away from the PInvoke call EnumProcesses to the .NET class method GetProcesses(). The order of the PIDs that are returned is different between the 2 and the latter has a higher chance of returning a more restrictive PID that doesn't have the SeTcbPrivilege required for become to work with an elevated process.

The latest change will add a further check on the enumerated process handle to make sure it has the SeTcbPrivilege before choosing the token. The end result is that the restricted token is skipped because it is useless for our purposes.

jborean93 added some commits Nov 5, 2018

@jborean93 jborean93 force-pushed the jborean93:win-process-utils branch from e015e56 to 8bffcf8 Dec 5, 2018

@nitzmahone

A couple tiny things, otherwise looks awesome.

Show resolved Hide resolved docs/docsite/rst/user_guide/become.rst Outdated
Show resolved Hide resolved docs/docsite/rst/user_guide/become.rst Outdated
Show resolved Hide resolved lib/ansible/module_utils/csharp/Ansible.Become.cs
Show resolved Hide resolved lib/ansible/module_utils/csharp/Ansible.Become.cs Outdated

@ansibot ansibot added needs_revision and removed core_review labels Dec 12, 2018

jborean93 added some commits Dec 12, 2018

@nitzmahone nitzmahone merged commit 190d1ed into ansible:devel Dec 13, 2018

1 check passed

Shippable Run 97925 status is SUCCESS.
Details

@jborean93 jborean93 deleted the jborean93:win-process-utils branch Dec 13, 2018

jborean93 added a commit to jborean93/ansible that referenced this pull request Dec 13, 2018

jborean93 added a commit to jborean93/ansible that referenced this pull request Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment