I searched the last 7 days of my logs for web servers where my crawler ran into this issue. In total 890 web services got crawled. With 8 of them the crawler ran into described issue. I could not reproduce the issue, so I searched for hints/similarities. The similarities I found among all of the them, were:

• They communicate via HTTP (not HTTPs)
• Microsoft IIS web servers
• They respond with "401 unauthorized" and requested NTLM authentication (WWW-Authenticate: NTLM)

(Sorry, can't share the hosts, they are within an internal network.)

As far as I can tell, this can only happen if rt.RoundTrip returns a nil *Response alongside a nil error.

Is your program using a non-default RoundTripper, for example by setting it in the http.Client's Transport field?

Change https://golang.org/cl/221818 mentions this issue: net/http: verify RoundTripper invariants in the send function

Hi,

we are:

• configuring our own transport and client:

	return &http.Transport{
		TLSClientConfig:       utils.InsecureTlsConfigFactory(),
		TLSHandshakeTimeout:   timeout,
		ResponseHeaderTimeout: timeout * 2, // TCP connection succeeded, let's give the web app some more time
		DisableKeepAlives:     false,       // Keep-alive is required for NTLM authentication to work!
		Proxy:                 http.ProxyURL(proxy),
	}

	// Create and return fresh client
	return &http.Client{
		Transport: transport,   // HTTP transport to use, may be a reused one with alive connections
		Timeout:   timeout * 2, // TCP connection succeeded, let's give the web app some more time
		// CheckRedirect: -> default is nil, which tells it to stop after 10 consecutive requests
	}

• Patching the CheckRedirect function to step through location redirects with our own code. Well, this might look funny to you, but this way we didn't need to touch the more complex roundtripper. Instead we are returning after each step and creating a new request for the next step.

	// Patch redirect function to not automatically handle location redirects. We will take care of this manually
	// to give us more insight into what's happening, e.g. to count redirects and detect www-authenticate responses.
	client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
		return http.ErrUseLastResponse // Return first response directly. Might contain location redirect.
	}

• Wrapping the transport by ntlmssp.Negotiatior (for NTLM authentication). I think they are using the default roundtripper but doing some of their own roundtripping logic first. Now I'm thinking the problem could also be in (or in associateion with) their package. Though it is not a shady unknown one...

	negotiatedTransport := ntlmssp.Negotiator{
		RoundTripper: client.Transport,
	}
	client.Transport = negotiatedTransport

I added exhaustive logging to our code to find out how far and which paths it executes in the flawed situations. I need to wait until i setp into the situation again, but then I should see better what happened, e.g. whether it made use of the ntlmssp negotiator already, etc...

Found this:

Azure/go-ntlmssp#9
and
hashicorp/packer#5865 (comment)
and
hashicorp/packer#8369

Ok it seems to be a bug in go-ntlmssp and there seems to be a magic update: Azure/go-ntlmssp#24

So I will update to that new version and hope the bug disappears. I guess you can close this ticket for now, I'd come back if the issue comes up again. Thank's guys for your time and help!

Azure/go-ntlmssp#24 seems to be fixing another instance of the problem described in #377.

Notably, golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow detects the bug:

go-ntlmssp$ git status
HEAD detached at 4a21cbd
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        go.mod
        go.sum

nothing added to commit but untracked files present (use "git add" to track)

go-ntlmssp$ shadow ./...
/tmp/tmp.pMusf64dqg/go-ntlmssp/negotiator.go:87:9: declaration of "err" shadows declaration at line 30

I suspect that that means other tools like staticcheck could also spot the bug.

So, at least we're aware of this class of bug, but it's a shame that the shadow check isn't in more widespread use.

(FYI @ianthehat)

Nice, I didn't know that tool. But also, we don't have build verification/automation yet (where it would fit into perfectly), as it would be overkill in our current state. Great to have that tool, but we can also assume that there will never be a majority knowing and applying a certain one. Best case, the language doesn't allow or mitigate it. Otherwise, the first step is to know this kind of bug :) Anyways, not complaining...