[PATCH] Version 2 - Additional fix and test for bug #12899 - smbclient "setmode" no longer works to clear attribute bits due to dialect upgrade

Jeremy Allison jra at samba.org
Fri Jul 14 23:47:16 UTC 2017


On Fri, Jul 14, 2017 at 04:17:56PM -0700, Jeremy Allison via samba-technical wrote:
> Steve's fix for the smbclient setmode command
> exposed a bug in our SMB1 implementation once I
> wrote the test for it (our SMB2 code was already
> correct).
> 
> An SMB1 SMBsetatr command should ignore the
> set attribute request if a value of *zero* is
> sent, we were ignoring it if a value of
> FILE_ATTRIBUTE_NORMAL was sent.
> 
> FILE_ATTRIBUTE_NORMAL is already correctly
> filtered out inside the set_dosmode() function.
> 
> Our SMB2 code already did this correctly.
> 
> Fix contains the SMB1 fix and then the
> torture test that exposed the bug.

Bah. I missed this fix meant a change in
SMB1 server behavior (SMBsetatr 0 means
clear all attributes, the torture code
depends on that).

Here's the updated version that passes
all the existing samba3.smbtorture_s3
tests. Sorry for the mistake.

Please review,

Jeremy.
-------------- next part --------------
From 31a85e0c70c4e6479dda09e23167192657878f24 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 14 Jul 2017 16:07:18 -0700
Subject: [PATCH 1/2] s3: smbd - SMB1. Allow a request to set attributes of
 FILE_ATTRIBUTE_NORMAL.

This is filtered out inside set_dosmode() and is needed to pass the
following set attribute test in the next commit.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/reply.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 317143f912c..40a11b8265b 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -1525,24 +1525,26 @@ void reply_setatr(struct smb_request *req)
 	mode = SVAL(req->vwv+0, 0);
 	mtime = srv_make_unix_date3(req->vwv+1);
 
-	if (mode != FILE_ATTRIBUTE_NORMAL) {
-		if (VALID_STAT_OF_DIR(smb_fname->st))
-			mode |= FILE_ATTRIBUTE_DIRECTORY;
-		else
-			mode &= ~FILE_ATTRIBUTE_DIRECTORY;
-
-		status = check_access(conn, NULL, smb_fname,
-					FILE_WRITE_ATTRIBUTES);
-		if (!NT_STATUS_IS_OK(status)) {
-			reply_nterror(req, status);
-			goto out;
-		}
+	if (mode == 0) {
+		mode = FILE_ATTRIBUTE_NORMAL;
+	}
 
-		if (file_set_dosmode(conn, smb_fname, mode, NULL,
-				     false) != 0) {
-			reply_nterror(req, map_nt_error_from_unix(errno));
-			goto out;
-		}
+	if (VALID_STAT_OF_DIR(smb_fname->st))
+		mode |= FILE_ATTRIBUTE_DIRECTORY;
+	else
+		mode &= ~FILE_ATTRIBUTE_DIRECTORY;
+
+	status = check_access(conn, NULL, smb_fname,
+				FILE_WRITE_ATTRIBUTES);
+	if (!NT_STATUS_IS_OK(status)) {
+		reply_nterror(req, status);
+		goto out;
+	}
+
+	if (file_set_dosmode(conn, smb_fname, mode, NULL,
+			     false) != 0) {
+		reply_nterror(req, map_nt_error_from_unix(errno));
+		goto out;
 	}
 
 	ft.mtime = convert_time_t_to_timespec(mtime);
-- 
2.13.2.932.g7449e964c-goog


From 6d028431703ef70145d6219c2fdec70f7d9b1762 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 14 Jul 2017 16:09:50 -0700
Subject: [PATCH 2/2] s3: smbclient: Add a test for the setmode command.

Tested over SMB1 and SMB2.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/script/tests/test_smbclient_s3.sh | 51 +++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 3cbe6f51d4b..a343a51da72 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -1232,6 +1232,53 @@ EOF
     fi
 }
 
+# Test smbclient setmode command
+test_setmode()
+{
+    tmpfile=$PREFIX/smbclient_interactive_prompt_commands
+
+    cat > $tmpfile <<EOF
+del test_setmode
+put ${SMBCLIENT} test_setmode
+setmode test_setmode +rsha
+allinfo test_setmode
+setmode test_setmode -rsha
+allinfo test_setmode
+del test_setmode
+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=$?
+
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed setmode test with output $ret"
+	false
+	return
+    fi
+
+    echo "$out" | grep 'attributes: RHSA'
+    ret=$?
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed - should get attributes: RHSA"
+       false
+       return
+    fi
+
+    echo "$out" | grep 'attributes:  (80)'
+    ret=$?
+    if [ $ret -ne 0 ] ; then
+       echo "$out"
+       echo "failed - should also get attributes:  (80)"
+       false
+       return
+    fi
+}
+
+
 test_server_os_message()
 {
     tmpfile=$PREFIX/smbclient_interactive_prompt_commands
@@ -1377,6 +1424,10 @@ testit "server os message" \
     test_server_os_message || \
     failed=`expr $failed + 1`
 
+testit "setmode test" \
+    test_setmode || \
+    failed=`expr $failed + 1`
+
 testit "rm -rf $LOGDIR" \
     rm -rf $LOGDIR || \
     failed=`expr $failed + 1`
-- 
2.13.2.932.g7449e964c-goog



More information about the samba-technical mailing list