[PATCH] Fix smbclient regression not printing session setup anymore

Andreas Schneider asn at samba.org
Tue Jun 6 15:56:21 UTC 2017


On Tuesday, 6 June 2017 17:35:52 CEST Andreas Schneider via samba-technical 
wrote:
> Hi,
> 
> with Samba 4.6 we have a regression. We do not get the information of the
> session setup filled out so smbclient can't print it.
> 
> Domain=[SAMBA-TEST] OS=[Windows 6.1] Server=[Samba 4.7.0pre1-DEVELOPERBUILD]
> smb: \>
> 
> The attached patchset addresses the issue. I've open a bug for this:
> 
> https://bugzilla.samba.org/show_bug.cgi?id=12824

I've added a test to confirm that the message is actually printed so we do not 
regress again ...

> 
> Review and push appreciated!
> 
> 
> Thanks,
> 
> 
> 	Andreas


-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From 09ec6a2c21a8a7c97d34526ee9330bfd42aba198 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 6 Jun 2017 17:27:44 +0200
Subject: [PATCH 1/3] libcli:smb: Fix pulling strings from the wire

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12824

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 libcli/smb/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcli/smb/util.c b/libcli/smb/util.c
index ef8c9fafa35..80d34281adf 100644
--- a/libcli/smb/util.c
+++ b/libcli/smb/util.c
@@ -372,6 +372,6 @@ NTSTATUS smb_bytes_pull_str(TALLOC_CTX *mem_ctx, char **_str, bool ucs2,
 			    const uint8_t *buf, size_t buf_len,
 			    size_t *_buf_consumed)
 {
-	return internal_bytes_pull_str(mem_ctx, _str, ucs2, true,
+	return internal_bytes_pull_str(mem_ctx, _str, ucs2, false,
 				       buf, buf_len, _buf_consumed);
 }
-- 
2.13.0


>From c35e71bc71a1d81d3d7b54d29ebe9dbf860ffb5b Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 6 Jun 2017 17:29:16 +0200
Subject: [PATCH 2/3] s3:libsmb: Fix printing the session setup information

This fixes a regression and prints the session setup on connect again:

Domain=[SAMBA-TEST] OS=[Windows 6.1] Server=[Samba 4.7.0pre1-DEVELOPERBUILD]
smb: \>

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12824

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/libsmb/cliconnect.c |  3 ++-
 source3/libsmb/clidfs.c     | 32 +++++++++++++++++++++++++-------
 source3/libsmb/clientgen.c  | 13 -------------
 source3/torture/masktest.c  |  4 +++-
 4 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index a28a5824dc2..08627226629 100644
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -1122,7 +1122,8 @@ static void cli_session_setup_gensec_ready(struct tevent_req *req)
 	server_domain = gensec_ntlmssp_server_domain(
 				state->auth_generic->gensec_security);
 
-	if (state->cli->server_domain[0] == '\0' && server_domain != NULL) {
+	if ((state->cli->server_domain == NULL ||
+	     state->cli->server_domain[0] == '\0') && server_domain != NULL) {
 		TALLOC_FREE(state->cli->server_domain);
 		state->cli->server_domain = talloc_strdup(state->cli,
 					server_domain);
diff --git a/source3/libsmb/clidfs.c b/source3/libsmb/clidfs.c
index c477d7c6a46..09e21ac7f8d 100644
--- a/source3/libsmb/clidfs.c
+++ b/source3/libsmb/clidfs.c
@@ -259,13 +259,31 @@ static NTSTATUS do_connect(TALLOC_CTX *ctx,
 		return status;
 	}
 
-	if ( show_sessetup ) {
-		if (*c->server_domain) {
-			DEBUG(0,("Domain=[%s] OS=[%s] Server=[%s]\n",
-				c->server_domain,c->server_os,c->server_type));
-		} else if (*c->server_os || *c->server_type) {
-			DEBUG(0,("OS=[%s] Server=[%s]\n",
-				 c->server_os,c->server_type));
+	if (show_sessetup) {
+		const char *server_os = "unknown";
+		const char *server_type = "unknown";
+		bool do_print = false;
+
+		if (c->server_os != NULL && c->server_os[0] != '\0') {
+			server_os = c->server_os;
+			do_print = true;
+		}
+		if (c->server_type != NULL && c->server_type[0] != '\0') {
+			server_type = c->server_type;
+			do_print = true;
+		}
+
+		if (c->server_domain != NULL && c->server_domain[0] != '\0') {
+			DEBUG(0, ("Domain=[%s] OS=[%s] Server=[%s]\n",
+				  c->server_domain,
+				  server_os,
+				  server_type));
+		} else {
+			if (do_print) {
+				DEBUG(0, ("OS=[%s] Server=[%s]\n",
+					  server_os,
+					  server_type));
+			}
 		}
 	}
 	DEBUG(4,(" session setup ok\n"));
diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c
index bc5c1b1ce3c..2c0b8c5f081 100644
--- a/source3/libsmb/clientgen.c
+++ b/source3/libsmb/clientgen.c
@@ -104,19 +104,6 @@ struct cli_state *cli_state_create(TALLOC_CTX *mem_ctx,
 		return NULL;
 	}
 
-	cli->server_domain = talloc_strdup(cli, "");
-	if (!cli->server_domain) {
-		goto error;
-	}
-	cli->server_os = talloc_strdup(cli, "");
-	if (!cli->server_os) {
-		goto error;
-	}
-	cli->server_type = talloc_strdup(cli, "");
-	if (!cli->server_type) {
-		goto error;
-	}
-
 	cli->dfs_mountpoint = talloc_strdup(cli, "");
 	if (!cli->dfs_mountpoint) {
 		goto error;
diff --git a/source3/torture/masktest.c b/source3/torture/masktest.c
index 95e0b04b040..57607b069a8 100644
--- a/source3/torture/masktest.c
+++ b/source3/torture/masktest.c
@@ -212,7 +212,9 @@ static struct cli_state *connect_one(char *share)
 	 * mode to turn these on/off ? JRA.
 	 */
 
-	if (*c->server_domain || *c->server_os || *c->server_type)
+	if (c->server_domain != NULL ||
+	    c->server_os != NULL ||
+	    c->server_type != NULL)
 		DEBUG(1,("Domain=[%s] OS=[%s] Server=[%s]\n",
 			c->server_domain,c->server_os,c->server_type));
 
-- 
2.13.0


>From e600314dd2812fc464a862258e7bdfa96fb3146b Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 6 Jun 2017 17:54:18 +0200
Subject: [PATCH 3/3] s3:tests: Add a test which checks that the smbclient
 session setup works

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12824

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/script/tests/test_smbclient_s3.sh | 34 +++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 0ae85e8e647..1359ed2b7bd 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -1212,6 +1212,36 @@ EOF
     fi
 }
 
+test_server_os_message()
+{
+    tmpfile=$PREFIX/smbclient_interactive_prompt_commands
+    cat > $tmpfile <<EOF
+ls
+quit
+EOF
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/tmp -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    rm -f $tmpfile
+
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed to connect error $ret"
+       false
+       return
+    fi
+
+    echo "$out" | grep 'Domain=\[SAMBA.*\] OS=\[Windows [0-9]\.[0-9]\] Server=\[Samba'
+    ret=$?
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed - should get: Domain=[SAMBA-TEST] OS=[Windows 6.1] Server=..."
+       false
+       return
+    fi
+}
+
 LOGDIR_PREFIX=test_smbclient_s3
 
 # possibly remove old logdirs:
@@ -1315,6 +1345,10 @@ testit "follow symlinks = no" \
     test_nosymlinks || \
     failed=`expr $failed + 1`
 
+testit "server os message" \
+    test_server_os_message || \
+    failed=`expr $failed + 1`
+
 testit "rm -rf $LOGDIR" \
     rm -rf $LOGDIR || \
     failed=`expr $failed + 1`
-- 
2.13.0



More information about the samba-technical mailing list