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

add NTLM Target Info; return broken session with error #29

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ropnop
Copy link

@ropnop ropnop commented Jun 15, 2020

This implements my feature request #28

Here's a sample demonstrating:

func main() {
	conn, err := net.Dial("tcp", "172.16.13.100:445")
	must(err)
	defer conn.Close()

	d := &smb2.Dialer{
		Initiator: &smb2.NTLMInitiator{
			User:     "agreen",
			Password: "wrong password",
		},
	}

	c, err := d.Dial(conn)
	if err != nil {
		if strings.Contains(err.Error(), "logon is invalid") {
			fmt.Printf("[!] Bad creds. But herre's the target info:\n\t%+v\n", c.TargetInfo())
			os.Exit(1)
		} else {
			panic(err)
		}
	}
	defer c.Logoff()

	fmt.Println("Connected!")

Output:

$ go run main.go
[!] Bad creds. But herre's the target info:
	{ServerName:PDC01 DomainName:ROPNOP DnsServerName:pdc01.lab.ropnop.com DnsDomainName:lab.ropnop.com}
exit status 1

Let me know what you think or if there's a cleaner way to implement this!

client.go Outdated Show resolved Hide resolved
@ropnop
Copy link
Author

ropnop commented Jun 22, 2020

Okay, refactored a bit so the NTLMTargetInfo is now part of the NTLMInitiator instead of the Session. I can still call it from outside the package with something like this:

initiator := &smb2.NTLMInitiator{
		User:     "thoffman",
		Password: "Password1234",
	}
	d := &smb2.Dialer{
		Initiator: initiator,
	}

	c, err := d.Dial(conn)

	if err != nil {
		fmt.Printf("signing required? %t\n", c.SigningRequired())
		fmt.Printf("Target info:\n\t%+v\n", initiator.TargetInfo())
		log.Fatal(err)
	}

I'm totally fine with that approach, it works great and seems simpler. I still think the SigningRequired function needs to be on the Client though (not sure where else would make sense)

@hirochachacha
Copy link
Owner

Sorry for the late reply. I'm still unsure of the right approach here.
Because I want to provide hight-level abstraction in this library, I don't want to reveal whole low-level properties.
If the main purpose is improving error messages, we might want to consider extending error structures rather than adding new API.

@gboddin
Copy link

gboddin commented Apr 16, 2022

Just a quick "hello" related to this issue.

We modified the library for recon purpose so NTLMSSP info are easier to grab : https://github.com/LeakIX/go-smb2

It's using https://github.com/bodgit/ntlmssp instead of the "current/messy/seen everywhere" ntlm provider

( we modified it to allow anonymous logons and made a few methods public at https://github.com/LeakIX/ntlmssp )

Let us know if you'd like a pull request for it, but since it's aimed at recon we didn't test further than the test provided with this repo

It could also be "another" initiator : https://github.com/LeakIX/go-smb2/blob/master/ntlmssp_initiator.go

@deitch
Copy link

deitch commented Aug 1, 2023

I cannot speak for @hirochachacha , but I would really like to see that merged in here. Anonymous support is a great thing. I actually tried to use the NTLMSSPInitiator from your repo to pass to Dialer, but it didn't work.

It actually doesn't implement the Initiator interface as that interface has some private methods, which means nothing other than in the same package could implement it. Just opening that up would make it much easier to use other initiators.

In any case, I would like (very much) to see the anonymous capabilities upstreamed here. What can I do to help?

@deitch
Copy link

deitch commented Aug 23, 2023

Asking again, what can I do to help move this along?

@deitch
Copy link

deitch commented Sep 11, 2023

Asking again, how can I help move this along?

@gboddin
Copy link

gboddin commented Sep 11, 2023

@deitch

You probably need a custom Initiator.

Example here : https://github.com/LeakIX/go-smb2/blob/master/ntlmssp_initiator.go

This is custom and the upstream ntlmssp library has been updated since by it's author : https://github.com/bodgit/ntlmssp

Please use it as an example and not production code, our use case is for recon and we haven't tested all scenarios.

You can probably implement a Initiator in a separate repo, no need to do a PR here.

@deitch
Copy link

deitch commented Sep 11, 2023

Hi @gboddin ; thanks for jumping in quickly. 😄

You probably need a custom Initiator

I wanted to, but I cannot. The Dialer is defined here:

// Dialer contains options for func (*Dialer) Dial.
type Dialer struct {
	MaxCreditBalance uint16 // if it's zero, clientMaxCreditBalance is used. (See feature.go for more details)
	Negotiator       Negotiator
	Initiator        Initiator
}

which requires an Initiator structure, defined here:

type Initiator interface {
	oid() asn1.ObjectIdentifier
	initSecContext() ([]byte, error)            // GSS_Init_sec_context
	acceptSecContext(sc []byte) ([]byte, error) // GSS_Accept_sec_context
	sum(bs []byte) []byte                       // GSS_getMIC
	sessionKey() []byte                         // QueryContextAttributes(ctx, SECPKG_ATTR_SESSION_KEY, &out)
}

All of the member functions of the Initiator are private, which means I cannot just plug in another struct from somewhere else, like LeakIX, because it cannot implement the interface. I did try.

If this repo changed the Initiator to have public methods (as well as the implementation NTLMInitiator, of course), then we could plug it in.

Did I miss something?

This is custom and the upstream ntlmssp library has been updated since by it's author https://github.com/bodgit/ntlmssp

I don't understand. This is upstream of what?

@gboddin
Copy link

gboddin commented Sep 12, 2023

Oh I didn't remember that, indeed a good point!

We used another ntlmssp library because it was cleaner than the one provided in this repo and we needed anonymous login support, so I mentioned which one.

Maybe open a new issue to expose the Initiator interface members? After all that's what interfaces are for. Just tell them it'll allow custom implementation without changing a bunch of code in this repo.

Otherwise your only option is to fork like we did.

@deitch
Copy link

deitch commented Sep 12, 2023

Maybe I will just open the PR and see.

@deitch
Copy link

deitch commented Sep 12, 2023

See #85; I hope @hirochachacha can accept it quickly.

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

Successfully merging this pull request may close these issues.

None yet

4 participants