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

NTLM HTTP Auth doesn't work (wrong negotiation implementation)? #1

Closed
ddelazerda opened this issue Dec 23, 2015 · 8 comments
Closed

Comments

@ddelazerda
Copy link

Hi, I think there are few issues with the negotiator. Here's roughly how I'm trying to use your library:

package main

import (
    "fmt"
    "github.com/paulmey/go-ntlmssp"
    "log"
    "net/http"
    "net/http/httputil"
)

func dumpResponse(r *http.Response, prefix string) {
    dump, err := httputil.DumpResponse(r, true)
    if err != nil {
        log.Fatal(err)
    }
    fmt.Printf("%s %s\n", prefix, string(dump))
}

func main() {
    someURL := "http://example.com/some/endpoint/behind/ntlm.aspx"
    c := &http.Client{
        Transport: ntlmssp.Negotiator{
            &http.Transport{},
        },
    }
    req, _ := http.NewRequest("GET", someURL, nil)
    req.SetBasicAuth("someuser", "somepass")
    res, err := c.Do(req)
    if err != nil {
        log.Fatal(err)
    }
    dumpResponse(res, "Page retrived after NTLM authentication:\n")
}

This code doesn't work. It panics with a nil pointer de-reference and after fixing that in negotiator.go, it doesn't correctly authenticate. After making the changes in this diff it works as expected. What do you think?

@paulmey
Copy link
Member

paulmey commented Mar 4, 2016

Negotiate and NTLM are different authentication methods. Negotiate can turn out to be NTLM in the end, but it can also be Kerberos. Anyways. This lib was a very minimal implementation to get WinRM working with the default settings on Azure. (See https://github.com/masterzen/winrm#pluggable-authentication-example-negotiatentlm-authentication)
That said, I'm definitely open to making this more generic and usable. 😄 We're going to move this repo to the Azure org, since we're going to have the packer builder depend on it, but we can move it forward there. Your diff has some pretty straightforward improvements with regards to error handling for one. 👍

@paulmey
Copy link
Member

paulmey commented Mar 4, 2016

Oh and sorry for not paying attention to this repo for a couple of months...

@ddelazerda
Copy link
Author

Hey, I still think there's an issue in the negotiator code. Before res.Body.Close() on lines 68 and 96 you still need to drain the res.Body content or else the connection won't be reused see the comment on my diff in the first post. After adding the code to drain res.Body the code in this repo works.

@paulmey
Copy link
Member

paulmey commented Jun 17, 2016

I only recently learned that one was to supposed to drain connections for proper connection reuse. I'll take a stab at it...

@SleepyBrett
Copy link

I'm extra dense today. Do you guys have a fully working example of this thing? The code above does not seem to work for me.

@paulmey
Copy link
Member

paulmey commented Oct 19, 2016

@SleepyBrett
Copy link

SleepyBrett commented Oct 20, 2016

@paulmey I'm not really up on the whole NTLM ecosystem, but what I'm seeing in this library is looking for response headers like WWW-Authenticate: Negotiate <base 64ed foo>, what I'm seeing is WWW-Authenticate: NTLM <base64ed foo>. Pointers, is this an older version of NTLM?

UPDATE:
Turns out with a few tweaks ( mostly grabbing that final if resauth.IsNegotiate() block and testing for NTLM and setting Authorization NTLM instead makes it work.

I cleaned up my hack a bit, if you would like me to pull request it back in let me know.

@paulmey
Copy link
Member

paulmey commented Oct 20, 2016

@SleepyBrett, cool that you got it to work. The docs on this protocol are pretty 'light'. PR's are always welcome, especially if they make this lib usable in more cases! Thanks in advance!

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

3 participants