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

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

Open
NikunjPatel31 opened this issue Nov 21, 2023 · 12 comments

Comments

@NikunjPatel31
Copy link

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
Copy link
Owner

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
Copy link
Contributor

CalypsoSys commented Nov 22, 2023 via email

@CalypsoSys
Copy link
Contributor

CalypsoSys commented Nov 22, 2023

I have traced the code to

resp, err := e.ntlmhttp.Do(req)

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....

@masterzen
Copy link
Owner

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
Copy link
Author

@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
Copy link
Author

I have traced the code to

resp, err := e.ntlmhttp.Do(req)

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
Copy link
Contributor

@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.

@NikunjPatel31
Copy link
Author

@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
Copy link
Owner

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
Copy link
Contributor

kke commented Nov 29, 2023

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 Azure/go-ntlmssp#40

@XiaoliChan
Copy link

Did this issue got fixed?

@gbnj2004
Copy link

gbnj2004 commented Sep 5, 2024

I also meet this , any solution?
It is the ntlmssp`s bug ,and winrm is based on that,so just waiting....
#bodgit/ntlmssp#51 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants