[PATCH] Fixes for Bug 11438

Christian Ambach ambi at samba.org
Tue May 3 21:32:27 UTC 2016


Hi list,

attached is a patchset that fixes some issues with case sensitivity
with SMB2/SMB3.

Review appreciated!

Thanks,
Christian
-------------- next part --------------
From 650ce0d18c189b34ad32fe3c8e584b7c6226e0be Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Sun, 3 Apr 2016 05:06:05 +0200
Subject: [PATCH 1/5] s3:smbd/service disable case-sensitivity for SMB2/3
 connections

in SMB2, there is no flag to let us know if the client wants to have case-sensitive behavior,
so in Auto mode, disable case-sensitivity

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11438
Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/smbd/service.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 2777a09..e4a910a 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -212,7 +212,9 @@ bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
 			{
 				/* We need this uglyness due to DOS/Win9x clients that lie about case insensitivity. */
 				enum remote_arch_types ra_type = get_remote_arch();
-				if ((ra_type != RA_SAMBA) && (ra_type != RA_CIFSFS)) {
+				if (conn->sconn->using_smb2) {
+					conn->case_sensitive = false;
+				} else if ((ra_type != RA_SAMBA) && (ra_type != RA_CIFSFS)) {
 					/* Client can't support per-packet case sensitive pathnames. */
 					conn->case_sensitive = False;
 				} else {
-- 
1.9.1


From fe9576ab2c818832fb446ccec7405af0261d89e6 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Sun, 3 Apr 2016 05:16:45 +0200
Subject: [PATCH 2/5] s3:smbd/service apply some code formatting

reduce indentation in switch statement, obey 80 char line limit, use C99 bool

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/smbd/service.c | 52 ++++++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index e4a910a..34cc369 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -181,6 +181,7 @@ bool set_conn_connectpath(connection_struct *conn, const char *connectpath)
 bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
 {
 	int snum;
+	enum remote_arch_types ra_type;
 
 	if (!conn)  {
 		last_conn = NULL;
@@ -206,30 +207,35 @@ bool set_current_service(connection_struct *conn, uint16_t flags, bool do_chdir)
 	last_conn = conn;
 	last_flags = flags;
 
-	/* Obey the client case sensitivity requests - only for clients that support it. */
+	/*
+	 * Obey the client case sensitivity requests - only for clients that
+	 * support it. */
 	switch (lp_case_sensitive(snum)) {
-		case Auto:
-			{
-				/* We need this uglyness due to DOS/Win9x clients that lie about case insensitivity. */
-				enum remote_arch_types ra_type = get_remote_arch();
-				if (conn->sconn->using_smb2) {
-					conn->case_sensitive = false;
-				} else if ((ra_type != RA_SAMBA) && (ra_type != RA_CIFSFS)) {
-					/* Client can't support per-packet case sensitive pathnames. */
-					conn->case_sensitive = False;
-				} else {
-					conn->case_sensitive = !(flags & FLAG_CASELESS_PATHNAMES);
-				}
-			}
-			break;
-		case True:
-			conn->case_sensitive = True;
-			break;
-		default:
-			conn->case_sensitive = False;
-			break;
-	}
-	return(True);
+	case Auto:
+		/*
+		 * We need this uglyness due to DOS/Win9x clients that lie
+		 * about case insensitivity. */
+		ra_type = get_remote_arch();
+		if (conn->sconn->using_smb2) {
+			conn->case_sensitive = false;
+		} else if ((ra_type != RA_SAMBA) && (ra_type != RA_CIFSFS)) {
+			/*
+			 * Client can't support per-packet case sensitive
+			 * pathnames. */
+			conn->case_sensitive = false;
+		} else {
+			conn->case_sensitive =
+					!(flags & FLAG_CASELESS_PATHNAMES);
+		}
+	break;
+	case True:
+		conn->case_sensitive = true;
+		break;
+	default:
+		conn->case_sensitive = false;
+		break;
+	}
+	return true;
 }
 
 /****************************************************************************
-- 
1.9.1


From 5fe9b38394be978cb777b05e8bf5434179050559 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Tue, 5 Apr 2016 02:58:48 +0200
Subject: [PATCH 3/5] s3:smbd/filename remove smelly code

not sure how this chunk ended up there, but I agree with
the statement in the comment that behavior should not depend
on developer mode

make test does not seem to depend on it anymore.

This piece had some bad influence on the tests I wrote
for case insensitivite behavior of SMB2/3, so let us
remove this technical debt.

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/smbd/filename.c | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 89c8bd6..7062060 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -942,34 +942,6 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 			TALLOC_FREE(found_name);
 		} /* end else */
 
-#ifdef DEVELOPER
-		/*
-		 * This sucks!
-		 * We should never provide different behaviors
-		 * depending on DEVELOPER!!!
-		 */
-		if (VALID_STAT(smb_fname->st)) {
-			bool delete_pending;
-			uint32_t name_hash;
-
-			status = file_name_hash(conn,
-					smb_fname_str_dbg(smb_fname),
-					&name_hash);
-			if (!NT_STATUS_IS_OK(status)) {
-				goto fail;
-			}
-
-			get_file_infos(vfs_file_id_from_sbuf(conn,
-							     &smb_fname->st),
-				       name_hash,
-				       &delete_pending, NULL);
-			if (delete_pending) {
-				status = NT_STATUS_DELETE_PENDING;
-				goto fail;
-			}
-		}
-#endif
-
 		/*
 		 * Add to the dirpath that we have resolved so far.
 		 */
-- 
1.9.1


From 0c867292a212a19a8ca6de388abbe53bea069695 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Mon, 4 Apr 2016 19:28:05 +0200
Subject: [PATCH 4/5] selftest: test for case insensitivity over SMB2/SMB3

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 .../script/tests/test_smb2_not_casesensitive.sh    | 82 ++++++++++++++++++++++
 source3/selftest/tests.py                          |  1 +
 2 files changed, 83 insertions(+)
 create mode 100755 source3/script/tests/test_smb2_not_casesensitive.sh

diff --git a/source3/script/tests/test_smb2_not_casesensitive.sh b/source3/script/tests/test_smb2_not_casesensitive.sh
new file mode 100755
index 0000000..a643ae7
--- /dev/null
+++ b/source3/script/tests/test_smb2_not_casesensitive.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+#
+# Blackbox test for SMB2 case insensitivity
+#
+
+if [ $# -lt 6 ]; then
+cat <<EOF
+Usage: test_smb2_not_casesensitive SERVER SERVER_IP USERNAME PASSWORD LOCAL_PATH SMBCLIENT
+EOF
+exit 1;
+fi
+
+SERVER=${1}
+SERVER_IP=${2}
+USERNAME=${3}
+PASSWORD=${4}
+LOCAL_PATH=${5}
+SMBCLIENT=${6}
+
+incdir=`dirname $0`/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+failed=0
+
+# Test a file with different case works over SMB2 and later
+test_access_with_different_case()
+{
+	tmpfile=$LOCAL_PATH/testfile.txt
+	echo "foobar" > $tmpfile
+
+	cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT -mSMB3 -U$USERNAME%$PASSWORD "$SERVER" -I $SERVER_IP -c "ls TeStFiLe.TxT" 2>&1'
+	out=`eval $cmd`
+	ret=$?
+
+	rm -f $tmpfile
+
+	if [ $ret = 0 ]; then
+		return 0
+	else
+		echo "$out"
+		echo "failed to get file with different case"
+		return 1
+	fi
+}
+
+# Test that a rename causes a conflict works when target name exists in
+# different case
+test_rename()
+{
+set -x
+	tmpfile=$LOCAL_PATH/torename.txt
+	echo "foobar" > $tmpfile
+	targetfile=$LOCAL_PATH/target.txt
+	touch $targetfile
+
+	cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT -mSMB3 -U$USERNAME%$PASSWORD "$SERVER" -I $SERVER_IP -c "rename ToReNaMe.TxT TaRgEt.txt" 2>&1'
+	out=`eval $cmd`
+	ret=$?
+
+	rm -f $tmpfile
+	rm -f $targetfile
+	rm -f $LOCAL_PATH/TaRgEt.txt
+
+	if [ $ret = 1 -a -z "${out##*COLLISION*}" ]; then
+		return 0
+	else
+		echo "$out"
+		echo "failed to get file with different case"
+		return 1
+	fi
+}
+
+
+testit "accessing a file with different case succeeds" \
+	test_access_with_different_case || \
+	failed=`expr $failed + 1`
+
+testit "renaming a file with different case succeeds" \
+	test_rename || \
+	failed=`expr $failed + 1`
+
+exit $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 2bd4110..077510c 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -190,6 +190,7 @@ for env in ["fileserver"]:
     plantestsuite("samba3.blackbox.smbget (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbget.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', 'smbget_user', '$PASSWORD', '$LOCAL_PATH/smbget', smbget])
     plantestsuite("samba3.blackbox.netshareenum (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_shareenum.sh"), '$SERVER', '$USERNAME', '$PASSWORD', rpcclient])
     plantestsuite("samba3.blackbox.acl_xattr (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_acl_xattr.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls])
+    plantestsuite("samba3.blackbox.smb2.not_casesensitive (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smb2_not_casesensitive.sh"), '//$SERVER/tmp', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', smbclient3])
 
     #
     # tar command tests
-- 
1.9.1


From 64e430d5d336373e8fd99fd68b46a3692a62e934 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Tue, 5 Apr 2016 14:30:47 +0200
Subject: [PATCH 5/5] s3:smbd remove todo comments

as the service is set to be case insensitive for SMB2 now,
there is no need to set FLAG_CASELESS_PATHNAMES as flag

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/smbd/smb2_server.c  | 1 -
 source3/smbd/smb2_setinfo.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 8a4aa96..b0a4afc 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1811,7 +1811,6 @@ static NTSTATUS smbd_smb2_request_check_tcon(struct smbd_smb2_request *req)
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
-	/* should we pass FLAG_CASELESS_PATHNAMES here? */
 	if (!set_current_service(tcon->compat, 0, true)) {
 		return NT_STATUS_ACCESS_DENIED;
 	}
diff --git a/source3/smbd/smb2_setinfo.c b/source3/smbd/smb2_setinfo.c
index a9196fe..2a02610 100644
--- a/source3/smbd/smb2_setinfo.c
+++ b/source3/smbd/smb2_setinfo.c
@@ -298,7 +298,6 @@ static void defer_rename_done(struct tevent_req *subreq)
 		return;
 	}
 
-	/* should we pass FLAG_CASELESS_PATHNAMES here? */
 	ok = set_current_service(state->smb2req->tcon->compat, 0, true);
 	if (!ok) {
 		tevent_req_nterror(state->req, NT_STATUS_ACCESS_DENIED);
-- 
1.9.1



More information about the samba-technical mailing list