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

enumshare: instead of names list, the letters of the drive #40

Open
fdefilippo opened this issue Feb 9, 2021 · 27 comments
Open

enumshare: instead of names list, the letters of the drive #40

fdefilippo opened this issue Feb 9, 2021 · 27 comments

Comments

@fdefilippo
Copy link

Hi maybe it's not a library problem but trying and listing the share names I get the letter list:

./smb2-share
ADMIN$
C$
E$
F$
G$
H$
I$
IPC$
J$
K$
L$
M$
N$
O$
P$

using smbclient the names are listed correctly. Has anyone had this problem?

@hirochachacha
Copy link
Owner

It works as intended. Each share has its own type. $ suffix means special type. You can filter it out by using strings.HasSuffix.
See https://docs.microsoft.com/en-us/windows/win32/api/lmshare/ns-lmshare-share_info_1 for detail.

@fdefilippo
Copy link
Author

hello, using smbclient the list of the share are:

# smbclient -L //server.local/ -W DOM -U 'user%password'

        Sharename       Type      Comment
        ---------       ----      -------
        share1            Disk
        share2 Disk
        share2 Disk
        AndSoOn   Disk

using go-smb2 example file:

./smb2-share
ADMIN$
C$
E$
F$
G$
H$

why the sharename aren't show as smbclient?

@hirochachacha
Copy link
Owner

hirochachacha commented Apr 30, 2021

Can you elaborate? Are you saying that ListSharenames should return (ShareType, string, error) instead of (string, error)?

@fdefilippo
Copy link
Author

Hi, the problem occurs only towards some windows servers. The ListSharenames shows me the letters of the Share and not the name as I reported in the example above. Example: If the sharename is TestShare the ListSharenames show H$ while the smbclient show TestShare. I don't know where is the problem, ask me what it takes to understand how to identify the problem. thank you.

@hirochachacha
Copy link
Owner

Can you ensure that smbclient and your smb2-share program connect to same server?
share name like H$ isn't abbreviation of another share name, it's valid share name on windows server.
If use smbclient -L against the same window server, you should see H$ as well.
Perhaps, you're running smbclient and smb2-share on different machines and each machine resolve different ip from server.local.

@fdefilippo
Copy link
Author

fdefilippo commented May 1, 2021

Hi, YES look:

image

image

@hirochachacha
Copy link
Owner

So, you meant smbclient -L output contains all of ./smblistshare output?
If yes, it could be a this library's bug. But as far as I read code, I couldn't find suspicious part.
The another possibility I can imagine is, your server is exposing the target share as SMB1 not SMB2.
Perhaps, using smbclient with -m SMB2 makes different output?

@fdefilippo
Copy link
Author

fdefilippo commented May 1, 2021

No the number of "share" displayed from smbclient are more than those shown by smblistshare output: 34 vs 24.

$ smbclient  -L //sntsmb01.XXXXX.local/ -W XXXXXX -U 'user%pass' |grep Disk|wc -l
34

$ ./smblistshare
[ADMIN$ C$ E$ F$ G$ H$ I$ IPC$ J$ K$ L$ M$ N$ O$ P$ Q$ R$ S$ T$ U$ V$ W$ Y$ Z$]

No, my setup has already a minimum of SMB2

$ grep SMB /etc/samba/smb.conf
  client min protocol = SMB2

and if try to force SMB1 (smbclient -m SMB1):

WARNING: Ignoring invalid value 'SMB1' for parameter 'client max protocol'
and the server say:
SMB1 disabled -- no workgroup available

@hirochachacha
Copy link
Owner

hirochachacha commented May 1, 2021

If I were in your shoes, I would run wireshark and compare the difference.
If you're OK to share your packet capture here, I would do that. If not OK, then you need to do it yourself.
At least, I can't help you with the current information. Sorry about that.

@fdefilippo
Copy link
Author

fdefilippo commented May 1, 2021

Hi, if you give me an email address I will send you the two pcap files. I cannot publish them. anyway look at the screenshot of the request NetShareEnumAll:

image

@hirochachacha
Copy link
Owner

hirochachacha@gmail.com

@hirochachacha
Copy link
Owner

Thank you for the mail. Honestly I don't know this behavior..., but I came up with another idea.
Current implementation uses IP address for mount request instead of hostname.
Perhaps, smb2 server has feature like virtualhost on http server which I didn't recognize.
If so, following patch will print different output.

diff --git a/client.go b/client.go
index fdebf93..20fb1b3 100644
--- a/client.go
+++ b/client.go
@@ -116,7 +116,7 @@ func (c *Session) Mount(sharename string) (*Share, error) {
 }

 func (c *Session) ListSharenames() ([]string, error) {
-       servername := c.addr
+       servername := <SERVERNAME>

        fs, err := c.Mount(fmt.Sprintf(`\\%s\IPC$`, servername))
        if err != nil {

@fdefilippo
Copy link
Author

fdefilippo commented May 1, 2021

what to put in place of <servername> then? c.?

@hirochachacha
Copy link
Owner

sntsmb01.XXXXX.local

@fdefilippo
Copy link
Author

😄 ok but I need the generic solution, that's just a test against a server that do not work . I mean, what is the member of the session data that contains the servername?

@fdefilippo
Copy link
Author

fdefilippo commented May 1, 2021

obviously it works! I'll wait the fix 😄 👍🏻

@hirochachacha
Copy link
Owner

I mean, what is the member of the session data that contains the servername?

Unfortunately, there are no way to recover server name from net.Conn, so I feel like it is a design mistake of this library.
However server returns their information on session setup response step. you can confirm that by wireshark. That's not exactly same value as the servername which we use as argument of net.Dial, but we maybe use the value to solve this issue.

Could you try out following patch?

diff --git a/client.go b/client.go
index fdebf93..37531cb 100644
--- a/client.go
+++ b/client.go
@@ -70,13 +70,14 @@ func (d *Dialer) DialContext(ctx context.Context, tcpConn net.Conn) (*Session, e
 		return nil, err
 	}
 
-	return &Session{s: s, ctx: context.Background(), addr: tcpConn.RemoteAddr().String()}, nil
+	return &Session{s: s, ctx: context.Background(), host: s.serverName, addr: tcpConn.RemoteAddr().String()}, nil
 }
 
 // Session represents a SMB session.
 type Session struct {
 	s    *session
 	ctx  context.Context
+	host string
 	addr string
 }
 
@@ -84,7 +85,7 @@ func (c *Session) WithContext(ctx context.Context) *Session {
 	if ctx == nil {
 		panic("nil context")
 	}
-	return &Session{s: c.s, ctx: ctx, addr: c.addr}
+	return &Session{s: c.s, ctx: ctx, host: c.host, addr: c.addr}
 }
 
 // Logoff invalidates the current SMB session.
@@ -100,7 +101,11 @@ func (c *Session) Mount(sharename string) (*Share, error) {
 	sharename = normPath(sharename)
 
 	if !strings.ContainsRune(sharename, '\\') {
-		sharename = fmt.Sprintf(`\\%s\%s`, c.addr, sharename)
+		if c.host != "" {
+			sharename = fmt.Sprintf(`\\%s\%s`, c.host, sharename)
+		} else {
+			sharename = fmt.Sprintf(`\\%s\%s`, c.addr, sharename)
+		}
 	}
 
 	if err := validateMountPath(sharename); err != nil {
@@ -117,6 +122,9 @@ func (c *Session) Mount(sharename string) (*Share, error) {
 
 func (c *Session) ListSharenames() ([]string, error) {
 	servername := c.addr
+	if c.host != "" {
+		servername = c.host
+	}
 
 	fs, err := c.Mount(fmt.Sprintf(`\\%s\IPC$`, servername))
 	if err != nil {
diff --git a/initiator.go b/initiator.go
index 69b8ed4..2f68288 100644
--- a/initiator.go
+++ b/initiator.go
@@ -13,6 +13,8 @@ type Initiator interface {
 	acceptSecContext(sc []byte) ([]byte, error) // GSS_Accept_sec_context
 	sum(bs []byte) []byte                       // GSS_getMIC
 	sessionKey() []byte                         // QueryContextAttributes(ctx, SECPKG_ATTR_SESSION_KEY, &out)
+
+	serverName() string
 }
 
 // NTLMInitiator implements session-setup through NTLMv2.
@@ -69,3 +71,7 @@ func (i *NTLMInitiator) sessionKey() []byte {
 func (i *NTLMInitiator) infoMap() *ntlm.InfoMap {
 	return i.ntlm.Session().InfoMap()
 }
+
+func (i *NTLMInitiator) serverName() string {
+	return i.infoMap().DnsComputerName
+}
diff --git a/session.go b/session.go
index 0292f9e..aae8a13 100644
--- a/session.go
+++ b/session.go
@@ -246,6 +246,8 @@ func sessionSetup(conn *conn, i Initiator, ctx context.Context) (*session, error
 
 	s.sessionFlags = r.SessionFlags()
 
+	s.serverName = spnego.serverName()
+
 	// now, allow access from receiver
 	s.enableSession()
 
@@ -265,6 +267,8 @@ type session struct {
 	decrypter cipher.AEAD
 
 	// applicationKey []byte
+
+	serverName string
 }
 
 func (s *session) logoff(ctx context.Context) error {
diff --git a/spnego.go b/spnego.go
index f3bb027..9db5fe5 100644
--- a/spnego.go
+++ b/spnego.go
@@ -79,3 +79,7 @@ func (c *spnegoClient) sum(bs []byte) []byte {
 func (c *spnegoClient) sessionKey() []byte {
 	return c.selectedMech.sessionKey()
 }
+
+func (c *spnegoClient) serverName() string {
+	return c.selectedMech.serverName()
+}

@fdefilippo
Copy link
Author

hi, doesn't work

@hirochachacha
Copy link
Owner

Oh, that's sad news 😢
Despite that, thank you for taking your time for testing.
Then we have to change Dialer.Dial signature or introduce new API.
That's difficult choice to me. I want to upgrade library version with other changes rather than adding new ad-hoc API.
But that decision takes much time which I don't have these days. Sorry I can't fix this issue right now.

@hirochachacha hirochachacha reopened this May 1, 2021
@fdefilippo
Copy link
Author

Hi, I'm old school 😄 so I'v add a print of c.host:
./go-smbsharelist -i sntsmb01.XXX.local -u user -p pass -d domain

SFSMC01N010.XXXX.local

why does this hostname?

@hirochachacha
Copy link
Owner

I also noticed that difference. I guess NETBIOS settings and DNS settings are conflicting somehow?

@fdefilippo
Copy link
Author

unfortunately I don't have access to the server so I have no idea.

@hirochachacha
Copy link
Owner

I'm suspecting that https://superuser.com/questions/1240213/cannot-connect-to-windows-share-via-local-network-ip-address-but-can-by-localhost is the same issue. so it's something related to NETBIOS.
Honestly I want to say this is a windows bug if this behavior isn't explicitly defined on smb2 spec.

@fdefilippo
Copy link
Author

fdefilippo commented May 1, 2021

I'm suspecting that https://superuser.com/questions/1240213/cannot-connect-to-windows-share-via-local-network-ip-address-but-can-by-localhost is the same issue. so it's something related to NETBIOS.
Honestly I want to say this is a windows bug if this behavior isn't explicitly defined on smb2 spec.

umm but if it was a windows bug why does smbclient (from samba) work?

You get the servername from sessionSetup, the same answer there is smbclient but he in Tree Connect Request Tree use the right name and not from the sessionSetup response.

@hirochachacha
Copy link
Owner

smbclient is using input argument as is. It neither use IP address nor NTLM information. So that they're managed to avoid edge cases. Actually window behavior are frequently out of the spec despite the fact the they're writing the spec. smb libraries should cover these edge cases. I just complained. never mind.

@ncw
Copy link

ncw commented Jan 6, 2023

@hirochachacha wrote:

Oh, that's sad news cry Despite that, thank you for taking your time for testing. Then we have to change Dialer.Dial signature or introduce new API. That's difficult choice to me. I want to upgrade library version with other changes rather than adding new ad-hoc API. But that decision takes much time which I don't have these days. Sorry I can't fix this issue right now.

We could introduce a DialWithHostname and DialContextWithHostname which would be backwards compatible and a fairly simple change.

Would you accept a PR with that sort of change?

I agree doing the whole /v2 thing is quite a lot of work but once you've made /v2, its easy to make /v3 etc.

PS thank you for a great library - we put it in rclone recently and it works extremely well :-)

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