[PATCH] Fixes for Bug 11438
Jeremy Allison
jra at samba.org
Tue May 3 23:27:20 UTC 2016
On Tue, May 03, 2016 at 11:32:27PM +0200, Christian Ambach wrote:
> Hi list,
>
> attached is a patchset that fixes some issues with case sensitivity
> with SMB2/SMB3.
>
> Review appreciated!
>
> Thanks,
> Christian
Great catch and thanks for the test :-). Pushed.
Made me realize there are still some case sensitive
wrinkles I need to look at w.r.t. UNIX extensions
over SMB2.
Cheers,
Jeremy.
> 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