From 852a74c49d501dc2694ab9f5986d2594c5be2d16 Mon Sep 17 00:00:00 2001 From: Joe Khoobyar Date: Fri, 30 Mar 2018 21:11:53 -0400 Subject: [PATCH 1/5] first attempt at supporting NTLM authentication in Terraform --- communicator/winrm/communicator.go | 6 ++++++ communicator/winrm/provisioner.go | 1 + terraform/eval_validate.go | 1 + website/docs/provisioners/connection.html.markdown | 2 ++ 4 files changed, 10 insertions(+) diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go index 90b9fe915..fa9ee6578 100644 --- a/communicator/winrm/communicator.go +++ b/communicator/winrm/communicator.go @@ -62,6 +62,9 @@ func (c *Communicator) Connect(o terraform.UIOutput) error { params := winrm.DefaultParameters params.Timeout = formatDuration(c.Timeout()) + if c.connInfo.NTLM == true { + params.TransportDecorator = func() winrm.Transporter { return &winrm.ClientNTLM{} } + } client, err := winrm.NewClientWithParameters( c.endpoint, c.connInfo.User, c.connInfo.Password, params) @@ -78,6 +81,7 @@ func (c *Communicator) Connect(o terraform.UIOutput) error { " Password: %t\n"+ " HTTPS: %t\n"+ " Insecure: %t\n"+ + " NTLM: %t\n"+ " CACert: %t", c.connInfo.Host, c.connInfo.Port, @@ -85,6 +89,7 @@ func (c *Communicator) Connect(o terraform.UIOutput) error { c.connInfo.Password != "", c.connInfo.HTTPS, c.connInfo.Insecure, + c.connInfo.NTLM, c.connInfo.CACert != "", )) } @@ -209,6 +214,7 @@ func (c *Communicator) newCopyClient() (*winrmcp.Winrmcp, error) { }, Https: c.connInfo.HTTPS, Insecure: c.connInfo.Insecure, + TransportDecorator: c.client.TransportDecorator, OperationTimeout: c.Timeout(), MaxOperationsPerShell: 15, // lowest common denominator } diff --git a/communicator/winrm/provisioner.go b/communicator/winrm/provisioner.go index 148204245..94e0170e1 100644 --- a/communicator/winrm/provisioner.go +++ b/communicator/winrm/provisioner.go @@ -37,6 +37,7 @@ type connectionInfo struct { Port int HTTPS bool Insecure bool + NTLM bool `mapstructure:"use_ntlm"` CACert string `mapstructure:"cacert"` Timeout string ScriptPath string `mapstructure:"script_path"` diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index 16bca3587..3e5a84ce6 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -157,6 +157,7 @@ func (n *EvalValidateProvisioner) validateConnConfig(connConfig *ResourceConfig) // For type=winrm only (enforced in winrm communicator) HTTPS interface{} `mapstructure:"https"` Insecure interface{} `mapstructure:"insecure"` + NTLM interface{} `mapstructure:"use_ntlm"` CACert interface{} `mapstructure:"cacert"` } diff --git a/website/docs/provisioners/connection.html.markdown b/website/docs/provisioners/connection.html.markdown index 10954f4d1..b86b2fea4 100644 --- a/website/docs/provisioners/connection.html.markdown +++ b/website/docs/provisioners/connection.html.markdown @@ -92,6 +92,8 @@ provisioner "file" { * `insecure` - Set to `true` to not validate the HTTPS certificate chain. +* `use_ntlm` - Set to `true` to use NTLM authentication, rather than default (basic authentication), removing the requirement for basic authentication to be enabled within the target guest. Further reading for remote connection authentication can be found [here](https://msdn.microsoft.com/en-us/library/aa384295(v=vs.85).aspx). + * `cacert` - The CA certificate to validate against. From d7cb9baa43adaca0eb97dec5bd8720734490e59b Mon Sep 17 00:00:00 2001 From: Joe Khoobyar Date: Fri, 30 Mar 2018 21:32:46 -0400 Subject: [PATCH 2/5] cleaner initialization of winrmcp --- communicator/winrm/communicator.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go index fa9ee6578..6510dc49b 100644 --- a/communicator/winrm/communicator.go +++ b/communicator/winrm/communicator.go @@ -214,10 +214,13 @@ func (c *Communicator) newCopyClient() (*winrmcp.Winrmcp, error) { }, Https: c.connInfo.HTTPS, Insecure: c.connInfo.Insecure, - TransportDecorator: c.client.TransportDecorator, OperationTimeout: c.Timeout(), MaxOperationsPerShell: 15, // lowest common denominator } + + if c.connInfo.NTLM == true { + config.TransportDecorator = func() winrm.Transporter { return &winrm.ClientNTLM{} } + } if c.connInfo.CACert != "" { config.CACertBytes = []byte(c.connInfo.CACert) From 481be9da35248502211d22d8114de16026926311 Mon Sep 17 00:00:00 2001 From: Joe Khoobyar Date: Fri, 30 Mar 2018 21:45:09 -0400 Subject: [PATCH 3/5] code reformatted with gofmt --- communicator/winrm/communicator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go index 6510dc49b..18112851d 100644 --- a/communicator/winrm/communicator.go +++ b/communicator/winrm/communicator.go @@ -217,7 +217,7 @@ func (c *Communicator) newCopyClient() (*winrmcp.Winrmcp, error) { OperationTimeout: c.Timeout(), MaxOperationsPerShell: 15, // lowest common denominator } - + if c.connInfo.NTLM == true { config.TransportDecorator = func() winrm.Transporter { return &winrm.ClientNTLM{} } } From 9766cc0aa5b8e4d3b68fd853dcc2ae14a4249b95 Mon Sep 17 00:00:00 2001 From: Joe Khoobyar Date: Fri, 30 Mar 2018 22:01:49 -0400 Subject: [PATCH 4/5] added unit tests --- communicator/winrm/communicator_test.go | 34 +++++++++++++++++++++++++ communicator/winrm/provisioner_test.go | 4 +++ 2 files changed, 38 insertions(+) diff --git a/communicator/winrm/communicator_test.go b/communicator/winrm/communicator_test.go index d903fbc2e..4dd06cd99 100644 --- a/communicator/winrm/communicator_test.go +++ b/communicator/winrm/communicator_test.go @@ -155,6 +155,40 @@ func TestScriptPath(t *testing.T) { } } +func TestTransportDecorator(t *testing.T) { + wrm := newMockWinRMServer(t) + defer wrm.Close() + + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "winrm", + "user": "user", + "password": "pass", + "host": wrm.Host, + "port": strconv.Itoa(wrm.Port), + "use_ntlm": "true", + "timeout": "30s", + }, + }, + } + + c, err := New(r) + if err != nil { + t.Fatalf("error creating communicator: %s", err) + } + + err = c.Connect(nil) + if err != nil { + t.Fatalf("error connecting communicator: %s", err) + } + defer c.Disconnect() + + if c.client.TransportDecorator == nil { + t.Fatal("bad TransportDecorator: got nil") + } +} + func TestScriptPath_randSeed(t *testing.T) { // Pre GH-4186 fix, this value was the deterministic start the pseudorandom // chain of unseeded math/rand values for Int31(). diff --git a/communicator/winrm/provisioner_test.go b/communicator/winrm/provisioner_test.go index b86576bf3..e2494657f 100644 --- a/communicator/winrm/provisioner_test.go +++ b/communicator/winrm/provisioner_test.go @@ -16,6 +16,7 @@ func TestProvisioner_connInfo(t *testing.T) { "host": "127.0.0.1", "port": "5985", "https": "true", + "use_ntlm": "true", "timeout": "30s", }, }, @@ -41,6 +42,9 @@ func TestProvisioner_connInfo(t *testing.T) { if conf.HTTPS != true { t.Fatalf("expected: %v: got: %v", true, conf) } + if conf.NTLM != true { + t.Fatalf("expected: %v: got: %v", true, conf) + } if conf.Timeout != "30s" { t.Fatalf("expected: %v: got: %v", "30s", conf) } From 138e64dee016643dc9976d207739c7ad9eb70397 Mon Sep 17 00:00:00 2001 From: Joe Khoobyar Date: Fri, 30 Mar 2018 22:05:10 -0400 Subject: [PATCH 5/5] more unit tests --- communicator/winrm/communicator_test.go | 35 ++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/communicator/winrm/communicator_test.go b/communicator/winrm/communicator_test.go index 4dd06cd99..f6a049932 100644 --- a/communicator/winrm/communicator_test.go +++ b/communicator/winrm/communicator_test.go @@ -155,6 +155,39 @@ func TestScriptPath(t *testing.T) { } } +func TestNoTransportDecorator(t *testing.T) { + wrm := newMockWinRMServer(t) + defer wrm.Close() + + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "winrm", + "user": "user", + "password": "pass", + "host": wrm.Host, + "port": strconv.Itoa(wrm.Port), + "timeout": "30s", + }, + }, + } + + c, err := New(r) + if err != nil { + t.Fatalf("error creating communicator: %s", err) + } + + err = c.Connect(nil) + if err != nil { + t.Fatalf("error connecting communicator: %s", err) + } + defer c.Disconnect() + + if c.client.TransportDecorator != nil { + t.Fatal("bad TransportDecorator: expected nil, got non-nil") + } +} + func TestTransportDecorator(t *testing.T) { wrm := newMockWinRMServer(t) defer wrm.Close() @@ -185,7 +218,7 @@ func TestTransportDecorator(t *testing.T) { defer c.Disconnect() if c.client.TransportDecorator == nil { - t.Fatal("bad TransportDecorator: got nil") + t.Fatal("bad TransportDecorator: expected non-nil, got nil") } }