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

[fdefilippo]

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]

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]

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]

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

[fdefilippo]

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]

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]

Hi, YES look:

[hirochachacha]

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]

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]

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]

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:

[hirochachacha]

[email protected]

[hirochachacha]

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 {

[hirochachacha]

https://superuser.com/questions/1240213/cannot-connect-to-windows-share-via-local-network-ip-address-but-can-by-localhost

[fdefilippo]

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

[hirochachacha]

sntsmb01.XXXXX.local

[fdefilippo]

😄 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]

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

[hirochachacha]

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]

hi, doesn't work

[hirochachacha]

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.

[fdefilippo]

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]

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

[fdefilippo]

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

[hirochachacha]

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]

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]

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]

@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 :-)