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

Double-Hop Solution: Pass Credentials #107

Closed
wants to merge 4 commits into from
Closed

Double-Hop Solution: Pass Credentials #107

wants to merge 4 commits into from

Conversation

gramsa49
Copy link
Contributor

@gramsa49 gramsa49 commented May 4, 2021

Description

Added new parameter, winrm_pass_credenitials, to the provider configuration to enable passing Active Directory credentials to the remote WinRM server. This works around the double-hop issue posed when not connecting directly to the Active Directory Domain Controller by creating a 'System.Management.Automation.PSCredential' in the remote PowerShell session.

I coded this capability to require the https protocol to ensure the credentials are passed securely over the wire. I first explored using Kerberos message encryption, but this does not look to be possible with the Go Kerberos client.

The method adopted by this patch is the least intrusive (easiest to adopt) in that it does not require alteration or management of special configurations on the non-AD DC WindRM host.

The credentials passed to the remote secure shell are redacted from the Terraform DEBUG logs. Only the -AD command is logged with '-Credential $Credential' shown at the end of the command being run. This also applies to the 'encoded command' log.

References

Closes #99
https://docs.microsoft.com/en-us/powershell/scripting/learn/remoting/ps-remoting-second-hop?view=powershell-7.1

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@ghost ghost added size/XL labels May 4, 2021
@koikonom
Copy link
Contributor

Hi @gramsa49,

I think your approach will work, but I will need a few more weeks before I merge this because I need to alter it slightly.

We have already started adding too many parameters around establishing and running WinRM commands and I'd like to make it easier to use.

@gramsa49
Copy link
Contributor Author

Hi @gramsa49,

I think your approach will work, but I will need a few more weeks before I merge this because I need to alter it slightly.

We have already started adding too many parameters around establishing and running WinRM commands and I'd like to make it easier to use.

That's great news! Let me know if there is anything I can do to assist.

@juneeighteen
Copy link

I've personally been using a build of the gramsa49:main provider for this solution, and it's been working great. Thank you @gramsa49 !

@luis-garza
Copy link

luis-garza commented May 31, 2021

Is there any chance to move forward this PR?
It's blocking us to use official AD provider instead the community one

@luis-garza
Copy link

BTW, @juneeighteen can you share how you integrated the gramsa49:main provider?
I would like to test it as well.

@koikonom
Copy link
Contributor

koikonom commented Jun 2, 2021

Hi everyone, thanks for your interest and your feedback!

This hasn't been merged yet because the approach used doesn't work out of the box for some of the resources supported by the provider. I am working on getting fixing the ones that don't work and once it's done I'll update this PR.

Again, thanks for your interest :)

@gramsa49
Copy link
Contributor Author

gramsa49 commented Jun 2, 2021

Hi everyone, thanks for your interest and your feedback!

This hasn't been merged yet because the approach used doesn't work out of the box for some of the resources supported by the provider. I am working on getting fixing the ones that don't work and once it's done I'll update this PR.

Again, thanks for your interest :)

Curious which ones if you don't mind sharing. I couldn't thoroughly test every resource type and data source, but I did attempt to update the code path in a way that would include all resources and data sources.

@koikonom
Copy link
Contributor

koikonom commented Jun 2, 2021

Sure!

The GPO related resources failed because the related powershell commands don't use the -Credentials parameter.

There are also a few cases where we pipe one command to another like for example the Delete() method for the OU resource that calls something like this:
Get-ADObject -Properties * -Identity %q | Set-ADObject -ProtectedFromAccidentalDeletion:$false -Passthru | Remove-ADOrganizationalUnit -confirm:$false"

In this case the -Credentials parameter needs to be passed in each of the commands.

@juneeighteen
Copy link

juneeighteen commented Jun 2, 2021

@luis-garza , Sure...

  1. git clone github.com/gramsa49/terraform-provider-ad
  2. cd terraform-provider-ad
  3. go build .
  4. Set this as your ~/.terraformrc file, updating to location of the built binary as needed:
provider_installation {
  dev_overrides {
    "hashicorp/ad" = "/Users/juneeighteen/projects/github.com/gramsa49/terraform-provider-ad"
  }

  direct {}

}
  1. In your Terraform project (provider_ad.tf or similar file) . Note: In this case, I had to match WINPCID.domain.com to the hostname of my Windows instance to avoid certificate issues. I setup a DNS entry, but I suspect a /etc/hosts entry would work the same.
provider "ad" {
  winrm_hostname = "WINPCID.domain.com" 
  winrm_username = "username"
  winrm_use_ntlm = true
  winrm_insecure = true
  winrm_proto = "https"
  winrm_port = 5986
  winrm_pass_credentials = true
}
  1. export AD_USER=username
  2. export AD_PASSWORD=password

@luis-garza
Copy link

luis-garza commented Jun 3, 2021

Thanks @juneeighteen, I was able to build it, and the double hop connection works as a charm!

But testing it I've faced some inconsistencies creating ad_computer objects...
I'll test it more and open an issue if proceeds.

ad_computer.foo: Creating...
╷
│ Error: Provider produced inconsistent result after apply
│ 
│ When applying changes to ad_computer.vm, provider "provider[\"registry.terraform.io/hashicorp/ad\"]" produced
│ an unexpected new value: Root resource was present, but now absent.
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Looking forward for the merge

@koikonom
Copy link
Contributor

koikonom commented Jun 8, 2021

@juneeighteen, @luis-garza, @gramsa49 I pushed #117 that extends this PR.

@koikonom
Copy link
Contributor

koikonom commented Jun 10, 2021

Hi @gramsa49, since PR #117, that includes your work, is now part of the v0.4.3 release (it should be available from the provider registry within the day) I am going to close this now.

Again, thanks for the work! :)

@koikonom koikonom closed this Jun 10, 2021
@gramsa49
Copy link
Contributor Author

Hi @gramsa49, since PR #117, that includes your work, is now part of the v0.4.3 release (it should be available from the provider registry within the day) I am going to close this now.

Again, thanks for the work! :)

Sounds good. Thanks for you help to make my contribution usable!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for double hop authentication
4 participants