[PATCH] Version 3 - Additional fix and test for bug #12899 - smbclient "setmode" no longer works to clear attribute bits due to dialect
Jeremy Allison
jra at samba.org
Mon Jul 17 19:32:31 UTC 2017
OK, here's the version I think we need for
master.
#1 - revert the incorrect patch.
#2 - Do the actual fix by changing the SMB2 client
code to provide the same ABI as the SMB1
cli_setatr().
#3 - Test this code works. This code is excercised
over both SMB1 and SMB2 so ensures the server
provies the correct semantics for both SMB1
and SMB2.
Fro 4.7.x and earlier we only need patches #2 and #3
for the backport, as those branches never got the
incorrect patch reverted by #1.
Please review and push if happy. Sorry for the original
issue (caused by not having a proper regression
test initially).
Cheers,
Jeremy.
-------------- next part --------------
From f551c22f7b718c1ea2596ee5930bbceddc127bd7 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 17 Jul 2017 10:38:36 -0700
Subject: [PATCH 1/3] Revert "s3:smbclient: Allow last dos attribute to be
cleared"
Incorrect fix - this must be fixed inside cli_setatr(), not
the callers.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12899
This reverts commit a4c3ee6767d768365a47bfda32a26cb7994b3787.
---
source3/client/client.c | 24 +++---------------------
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/source3/client/client.c b/source3/client/client.c
index aa8d949bd0c..d8c96e68190 100644
--- a/source3/client/client.c
+++ b/source3/client/client.c
@@ -4961,21 +4961,11 @@ int set_remote_attr(const char *filename, uint16_t new_attr, int mode)
}
if (mode == ATTR_SET) {
- if (new_attr == old_attr) {
- d_printf("attributes unchanged, cli_setatr skipped\n");
- return 0;
- }
new_attr |= old_attr;
} else {
new_attr = old_attr & ~new_attr;
}
- /*
- * if we are clearing attributes - can't pass 0 in
- * since that means "skip this field. See MS-FSCC section 2.6
- */
- if (new_attr == 0) {
- new_attr = FILE_ATTRIBUTE_NORMAL;
- }
+
status = cli_setatr(cli, filename, new_attr, 0);
if (!NT_STATUS_IS_OK(status)) {
d_printf("cli_setatr failed: %s\n", nt_errstr(status));
@@ -5000,8 +4990,6 @@ int cmd_setmode(void)
int mode = ATTR_SET;
int err = 0;
bool ok;
- bool set = false;
- bool unset = false;
TALLOC_CTX *ctx = talloc_new(NULL);
if (ctx == NULL) {
return 1;
@@ -5029,11 +5017,9 @@ int cmd_setmode(void)
while (*s) {
switch (*s++) {
case '+':
- set = true;
mode = ATTR_SET;
break;
case '-':
- unset = true;
mode = ATTR_UNSET;
break;
case 'r':
@@ -5065,12 +5051,8 @@ int cmd_setmode(void)
DEBUG(2, ("perm set %d %d\n", attr[ATTR_SET], attr[ATTR_UNSET]));
/* ignore return value: server might not store DOS attributes */
- if (set) {
- set_remote_attr(fname, attr[ATTR_SET], ATTR_SET);
- }
- if (unset) {
- set_remote_attr(fname, attr[ATTR_UNSET], ATTR_UNSET);
- }
+ set_remote_attr(fname, attr[ATTR_SET], ATTR_SET);
+ set_remote_attr(fname, attr[ATTR_UNSET], ATTR_UNSET);
out:
talloc_free(ctx);
return err;
--
2.13.2.932.g7449e964c-goog
From 757e8ee63bc00843487b8ab62bebe03d6cbd10e6 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 17 Jul 2017 10:37:15 -0700
Subject: [PATCH 2/3] s3: libsmb: Reverse sense of 'clear all attributes',
ignore attribute change in SMB2 to match SMB1.
SMB1 uses attr == 0 to clear all attributes
on a file (end up with FILE_ATTRIBUTE_NORMAL),
and attr == FILE_ATTRIBUTE_NORMAL to mean ignore
request attribute change.
SMB2 uses exactly the reverse. Unfortunately as the
cli_setatr() ABI is exposed inside libsmbclient,
we must make the SMB2 cli_smb2_setatr() call
export the same ABI as the SMB1 cli_setatr()
which calls it. This means reversing the sense
of the requested attr argument if it's zero
or FILE_ATTRIBUTE_NORMAL.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12899
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/libsmb/cli_smb2_fnum.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index 954f3fe3a7f..6967555797a 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -1698,6 +1698,29 @@ NTSTATUS cli_smb2_setatr(struct cli_state *cli,
inbuf.length = sizeof(inbuf_store);
data_blob_clear(&inbuf);
+ /*
+ * SMB1 uses attr == 0 to clear all attributes
+ * on a file (end up with FILE_ATTRIBUTE_NORMAL),
+ * and attr == FILE_ATTRIBUTE_NORMAL to mean ignore
+ * request attribute change.
+ *
+ * SMB2 uses exactly the reverse. Unfortunately as the
+ * cli_setatr() ABI is exposed inside libsmbclient,
+ * we must make the SMB2 cli_smb2_setatr() call
+ * export the same ABI as the SMB1 cli_setatr()
+ * which calls it. This means reversing the sense
+ * of the requested attr argument if it's zero
+ * or FILE_ATTRIBUTE_NORMAL.
+ *
+ * See BUG: https://bugzilla.samba.org/show_bug.cgi?id=12899
+ */
+
+ if (attr == 0) {
+ attr = FILE_ATTRIBUTE_NORMAL;
+ } else if (attr == FILE_ATTRIBUTE_NORMAL) {
+ attr = 0;
+ }
+
SSVAL(inbuf.data, 32, attr);
if (mtime != 0) {
put_long_date((char *)inbuf.data + 16,mtime);
--
2.13.2.932.g7449e964c-goog
From 0e4bbe0824b1a0d36289f59f16c04f1a393431e3 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 3/3] 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