masterzen / winrm

Command-line tool and library for Windows remote command execution in Go
Apache License 2.0
425 stars 129 forks source link

Concurrency issue with winrm NTLM - http response error: 401 - invalid content type #142

Open NikunjPatel31 opened 12 months ago

NikunjPatel31 commented 12 months ago

When I am trying to connect to windows machine using NTLM authentication concurrently ( using go routine ), winrm behaves randomely ( most of time it gives error ).

Here I am trying to connect on same IP Address concurrently. But when I try to connect on same IP address ( one by one using loop ) it works properly.

Error that I am getting ( durring concurrent connection ) is as follow :-

  • EOF
  • http response error: 401 - invalid content type

I have also ensured that winrm and NTLM both are enabled on machine.

I am using latest merged code of @CalypsoSys for NTLM.

Below is the code :-

func main() {

    Count := 10

    wg := new(sync.WaitGroup)

    wg.Add(Count)

    for i := 1; i <= Count; i++ {

        go execute(i, host, user, pwd, port, wg)
    }

    wg.Wait()
}

func execute(i int, host string, user string, pwd string, port int, wg *sync.WaitGroup) {

    defer wg.Done()

    endpoint := winrm.NewEndpoint(host, port, false, false, nil, nil, nil, 0)

    encryption, err := winrm.NewEncryption("ntlm")

    params := winrm.DefaultParameters

    params.TransportDecorator = func() winrm.Transporter { return encryption }

    client, _ := winrm.NewClientWithParameters(endpoint, user, pwd, params)

    shell, err := client.CreateShell()

    if err != nil {

        return
    }

    defer shell.Close()

    var outWriter, errWriter bytes.Buffer

    _, err = client.RunWithContextWithInput(context.Background(), Command, &outWriter, &errWriter, nil)

    if err != nil {

        fmt.Println("Error in executing command: ", err)

        return
    }
}
masterzen commented 12 months ago

I'm not seeing anything in the NTLM code that was just added that would cause concurrency issues. I don't have a setup allowing me to test concurrent access, so you'll have to help me troubleshoot the problem. Can you add a bit of debug around to check what could be wrong? For instance, you might want to add some debug logs here to print the full body, maybe we could get some more information?

CalypsoSys commented 11 months ago

Yes, I am able to reproduce the issue.

Initial investigation shows that the call in "CreateShell" to "response, err := c.sendRequest(request)" is returning a 401

CalypsoSys commented 11 months ago

I have traced the code to https://github.com/masterzen/winrm/blob/807053a1bcb92228b78594f11403cb140399ea81/encryption.go#L115

and then the code in github.com/bodgit/ntlmssp https://github.com/bodgit/ntlmssp/blob/9c8ca384e779e74e9834e66000103151f33e2b33/http/client.go#L179

it actually makes it through to then end, but a 401 is returned https://github.com/bodgit/ntlmssp/blob/9c8ca384e779e74e9834e66000103151f33e2b33/http/client.go#L248C7-L248C7

I have put a mutex around the Do call as a test and same results. The only way I can make it work is by limiting the concurrent access to your "execute" method for the same IP.

masterzen commented 11 months ago

Is there anything in the body or headers returned with the 401 response? I don't think it is an issue with our code or the ntlmssp library, it sounds more like an issue with WinRM on the other end. To rule out NTLM, would it be possible to do the same test of several concurrent access with unencrypted WinRM to see the result?

NikunjPatel31 commented 11 months ago

@masterzen When I try to make multiple connection ( concurrently without NTLM using normal Winrm ) on same windows machine, I am able to do so. But when I try to make multiple connection with same windows machine using NTLM with winrm I am not able to make connection.

I have also tried to use 5 or fewer goroutine, But still it won't connect. Infact I have tried to use 2 goroutine but the result is still same. I think it is definitely concurrency issue.

NikunjPatel31 commented 11 months ago

I have traced the code to

https://github.com/masterzen/winrm/blob/807053a1bcb92228b78594f11403cb140399ea81/encryption.go#L115

and then the code in github.com/bodgit/ntlmssp https://github.com/bodgit/ntlmssp/blob/9c8ca384e779e74e9834e66000103151f33e2b33/http/client.go#L179

it actually makes it through to then end, but a 401 is returned https://github.com/bodgit/ntlmssp/blob/9c8ca384e779e74e9834e66000103151f33e2b33/http/client.go#L248C7-L248C7

I have put a mutex around the Do call as a test and same results. The only way I can make it work is by limiting the concurrent access to your "execute" method for the same IP.

  • still investigating....

@CalypsoSys If mutex is put on Do method then code will not execute concurretly instead every request will wait for the mutex to release . And mine requirement is to execute multiple command at a time on same machine. When I am trying to make multiple connection on same device with normal Winrm ( Not using NTLM ) I am able to do so. I that case it is working fine and there is no issue of concurreny.

CalypsoSys commented 11 months ago

@NikunjPatel31 - new information from @bodgit - trying those now see https://github.com/bodgit/ntlmssp/issues/51

The mutex statement was more around what I was trying to figure out where the issue was, not as a solution.

NikunjPatel31 commented 11 months ago

@NikunjPatel31 - new information from @bodgit - trying those now see bodgit/ntlmssp#51

The mutex statement was more around what I was trying to figure out where the issue was, not as a solution.

@CalypsoSys I have tried this solution. Below is the code with changes. But this code still doesn't work. Still it gives the same error.

I have made this changes in Encryption.go file.

func (e *Encryption) Transport(endpoint *Endpoint) error {
    e.httpClient = cleanhttp.DefaultPooledClient()
    return e.ntlm.Transport(endpoint)
}

I have also tried the below code with cleanhttp.DefaultPooledClient() instead of http.Client I worked, but when I used above code in winrm with NTLM it does not work.

package main

import (
    "bytes"
    "context"
    "fmt"
    "net/http"
    "sync"

    "github.com/bodgit/ntlmssp"
    ntlmhttp "github.com/bodgit/ntlmssp/http"
)

var (
    host    = "192.168.10.1"
    pwd     = "pwd"
    port    = 5985
    https   = false
    domain   = "my_domain"
    userName = "administrator"
)

func main() {
    Count := 10
    wg := new(sync.WaitGroup)
    wg.Add(Count)
    for i := 1; i <= Count; i++ {
        go ntlmAuth(i, host, domain, userName, pwd, port, https, wg)
    }

    wg.Wait()
    fmt.Println("done")
}

func ntlmAuth(i int, host string, domain string, user string, pwd string, port int, useHttps bool, wg *sync.WaitGroup) {
    defer wg.Done()

    ntlmClient, err := ntlmssp.NewClient(ntlmssp.SetUserInfo(user, pwd), ntlmssp.SetDomain(domain), ntlmssp.SetVersion(ntlmssp.DefaultVersion()))
    if err != nil {
        fmt.Println(i, " - Error in ntlmssp.NewClient: ", err)
        return
    }
    httpClient := &http.Client{}
    ntlmhttp, err := ntlmhttp.NewClient(httpClient, ntlmClient)
    if err != nil {
        fmt.Println(i, " - Error in ntlmhttp.NewClient: ", err)
        return
    }

    var scheme string
    if useHttps {
        scheme = "https"
    } else {
        scheme = "http"
    }

    // should use url.URL, but QD
    endpoint := fmt.Sprintf("%s://%s:%d/wsman", scheme, host, port)
    req, err := http.NewRequest("POST", endpoint, nil)
    if err != nil {
        fmt.Println("error in NewRequest", err)
        return
    }

    req.Header.Set("User-Agent", "WinRM client")
    req.Header.Set("Content-Length", "0")
    req.Header.Set("Content-Type", "application/soap+xml;charset=UTF-8")
    req.Header.Set("Connection", "Keep-Alive")

    resp, err := ntlmhttp.Do(req)
    if err != nil {
        fmt.Println("unknown error do", err)
        return
    }

    if resp.StatusCode != 200 {
        fmt.Println("http error", resp.StatusCode)
    } else {
        fmt.Println(i, " - ok")
    }
}
masterzen commented 11 months ago

I'll have a deeper look later, but my understanding is that NTLM authenticates TCP connections and not HTTP request. Since encryption is dependent on the authentication, it is not possible to reuse a TCP connection for different underlying shells/commands because it would break the encryption scheme (especially during the challenge/response). We need at the library level to use a Transport/RoundTripper that doesn't reuse TCP connections between WinRM commands (which might not be easy to do).

kke commented 11 months ago

I'm seeing this in some logs:

# github.com/Azure/go-ntlmssp [github.com/Azure/go-ntlmssp.test]
Error: go/pkg/mod/github.com/!azure/go-ntlmssp@v0.0.0-20221128193559-754e69321358/nlmp_test.go:52:21: assignment mismatch: 2 variables but GetDomain returns 3 values

I think I accidentally ran tests against the whole go package dir on github runner.

Nevermind, it should only affect tests and there's an issue at https://github.com/Azure/go-ntlmssp/issues/40

XiaoliChan commented 5 months ago

Did this issue got fixed?

gbnj2004 commented 2 months ago

I also meet this , any solution? It is the ntlmssp`s bug ,and winrm is based on that,so just waiting....

https://github.com/bodgit/ntlmssp/issues/51#issue-2006410149