The Samba-Bugzilla – Attachment 12381 Details for
Bug 12149
smbd: cannot load a Windows device driver from a Samba share via SMB2
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.5.0
readx-45.patch (text/plain), 25.11 KB, created by
Uri Simchoni
on 2016-08-18 18:54:00 UTC
(
hide
)
Description:
git-am fix for 4.5.0
Filename:
MIME Type:
Creator:
Uri Simchoni
Created:
2016-08-18 18:54:00 UTC
Size:
25.11 KB
patch
obsolete
>From d8c91e16cd193ca9aae5d3c4ba932f6b837eb110 Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Thu, 4 Aug 2016 12:59:38 +0300 >Subject: [PATCH 1/8] s4-smbtorture: use standard macros in smb2.read test > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> >(cherry picked from commit 20b9a5bd74fafbca4b7cc7952c27033edcf0eeb8) >--- > source4/torture/smb2/read.c | 22 +++++++--------------- > 1 file changed, 7 insertions(+), 15 deletions(-) > >diff --git a/source4/torture/smb2/read.c b/source4/torture/smb2/read.c >index 3600765..c1105a9 100644 >--- a/source4/torture/smb2/read.c >+++ b/source4/torture/smb2/read.c >@@ -27,21 +27,13 @@ > #include "torture/smb2/proto.h" > > >-#define CHECK_STATUS(status, correct) do { \ >- if (!NT_STATUS_EQUAL(status, correct)) { \ >- printf("(%s) Incorrect status %s - should be %s\n", \ >- __location__, nt_errstr(status), nt_errstr(correct)); \ >- ret = false; \ >- goto done; \ >- }} while (0) >- >-#define CHECK_VALUE(v, correct) do { \ >- if ((v) != (correct)) { \ >- printf("(%s) Incorrect value %s=%u - should be %u\n", \ >- __location__, #v, (unsigned)v, (unsigned)correct); \ >- ret = false; \ >- goto done; \ >- }} while (0) >+#define CHECK_STATUS(_status, _expected) \ >+ torture_assert_ntstatus_equal_goto(torture, _status, _expected, \ >+ ret, done, "Incorrect status") >+ >+#define CHECK_VALUE(v, correct) \ >+ torture_assert_int_equal_goto(torture, v, correct, \ >+ ret, done, "Incorrect value") > > #define FNAME "smb2_readtest.dat" > #define DNAME "smb2_readtest.dir" >-- >2.5.5 > > >From c2cda540e35c6fe2824cc4d4d98cddeef13666dc Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Sun, 31 Jul 2016 14:26:24 +0300 >Subject: [PATCH 2/8] s4-selftest: add functions which create with desired > access > >Add functions which create a file or a directory with >specific desired access. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> >(cherry picked from commit 1b06acafa4e9ea91a50e5ed85da881187057da6e) >--- > source4/torture/smb2/util.c | 36 +++++++++++++++++++++++++++++------- > 1 file changed, 29 insertions(+), 7 deletions(-) > >diff --git a/source4/torture/smb2/util.c b/source4/torture/smb2/util.c >index 814e398..c9d47ae 100644 >--- a/source4/torture/smb2/util.c >+++ b/source4/torture/smb2/util.c >@@ -428,19 +428,20 @@ bool torture_smb2_con_sopt(struct torture_context *tctx, > return true; > } > >- > /* > create and return a handle to a test file >+ with a specific access mask > */ >-NTSTATUS torture_smb2_testfile(struct smb2_tree *tree, const char *fname, >- struct smb2_handle *handle) >+NTSTATUS torture_smb2_testfile_access(struct smb2_tree *tree, const char *fname, >+ struct smb2_handle *handle, >+ uint32_t desired_access) > { > struct smb2_create io; > NTSTATUS status; > > ZERO_STRUCT(io); > io.in.oplock_level = 0; >- io.in.desired_access = SEC_RIGHTS_FILE_ALL; >+ io.in.desired_access = desired_access; > io.in.file_attributes = FILE_ATTRIBUTE_NORMAL; > io.in.create_disposition = NTCREATEX_DISP_OPEN_IF; > io.in.share_access = >@@ -459,17 +460,29 @@ NTSTATUS torture_smb2_testfile(struct smb2_tree *tree, const char *fname, > } > > /* >+ create and return a handle to a test file >+*/ >+NTSTATUS torture_smb2_testfile(struct smb2_tree *tree, const char *fname, >+ struct smb2_handle *handle) >+{ >+ return torture_smb2_testfile_access(tree, fname, handle, >+ SEC_RIGHTS_FILE_ALL); >+} >+ >+/* > create and return a handle to a test directory >+ with specific desired access > */ >-NTSTATUS torture_smb2_testdir(struct smb2_tree *tree, const char *fname, >- struct smb2_handle *handle) >+NTSTATUS torture_smb2_testdir_access(struct smb2_tree *tree, const char *fname, >+ struct smb2_handle *handle, >+ uint32_t desired_access) > { > struct smb2_create io; > NTSTATUS status; > > ZERO_STRUCT(io); > io.in.oplock_level = 0; >- io.in.desired_access = SEC_RIGHTS_DIR_ALL; >+ io.in.desired_access = desired_access; > io.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY; > io.in.create_disposition = NTCREATEX_DISP_OPEN_IF; > io.in.share_access = NTCREATEX_SHARE_ACCESS_READ|NTCREATEX_SHARE_ACCESS_WRITE|NTCREATEX_SHARE_ACCESS_DELETE; >@@ -484,6 +497,15 @@ NTSTATUS torture_smb2_testdir(struct smb2_tree *tree, const char *fname, > return NT_STATUS_OK; > } > >+/* >+ create and return a handle to a test directory >+*/ >+NTSTATUS torture_smb2_testdir(struct smb2_tree *tree, const char *fname, >+ struct smb2_handle *handle) >+{ >+ return torture_smb2_testdir_access(tree, fname, handle, >+ SEC_RIGHTS_DIR_ALL); >+} > > /* > create a complex file using SMB2, to make it easier to >-- >2.5.5 > > >From cd84354e209c275285d8bea69bbe6e3e1db943a5 Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Sun, 31 Jul 2016 14:29:37 +0300 >Subject: [PATCH 3/8] s4-selftest: add test for read access check > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> >(cherry picked from commit 55a9d35cabaea6e98211fc058b788cedf9b7b22a) >--- > selftest/knownfail | 3 ++ > source4/torture/smb2/read.c | 74 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 77 insertions(+) > >diff --git a/selftest/knownfail b/selftest/knownfail >index 397e53c..892a5da 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -286,3 +286,6 @@ > ^samba4.krb5.kdc.*as-req-aes.*fl2000dc > # nt4_member and ad_member don't support ntlmv1 > ^samba3.blackbox.smbclient_auth.plain.*_member.*option=clientntlmv2auth=no.member.creds.*as.user >+#new read tests fail >+^samba4.smb2.read.access >+^samba3.smb2.read.access >diff --git a/source4/torture/smb2/read.c b/source4/torture/smb2/read.c >index c1105a9..c4469df 100644 >--- a/source4/torture/smb2/read.c >+++ b/source4/torture/smb2/read.c >@@ -226,6 +226,79 @@ done: > return ret; > } > >+static bool test_read_access(struct torture_context *torture, >+ struct smb2_tree *tree) >+{ >+ bool ret = true; >+ NTSTATUS status; >+ struct smb2_handle h; >+ uint8_t buf[64 * 1024]; >+ struct smb2_read rd; >+ TALLOC_CTX *tmp_ctx = talloc_new(tree); >+ >+ ZERO_STRUCT(buf); >+ >+ /* create a file */ >+ smb2_util_unlink(tree, FNAME); >+ >+ status = torture_smb2_testfile(tree, FNAME, &h); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ status = smb2_util_write(tree, h, buf, 0, ARRAY_SIZE(buf)); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ status = smb2_util_close(tree, h); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* open w/ READ access - success */ >+ status = torture_smb2_testfile_access( >+ tree, FNAME, &h, SEC_FILE_READ_ATTRIBUTE | SEC_FILE_READ_DATA); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ ZERO_STRUCT(rd); >+ rd.in.file.handle = h; >+ rd.in.length = 5; >+ rd.in.offset = 0; >+ status = smb2_read(tree, tree, &rd); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ status = smb2_util_close(tree, h); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* open w/ EXECUTE access - success */ >+ status = torture_smb2_testfile_access( >+ tree, FNAME, &h, SEC_FILE_READ_ATTRIBUTE | SEC_FILE_EXECUTE); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ ZERO_STRUCT(rd); >+ rd.in.file.handle = h; >+ rd.in.length = 5; >+ rd.in.offset = 0; >+ status = smb2_read(tree, tree, &rd); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ status = smb2_util_close(tree, h); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* open without READ or EXECUTE access - access denied */ >+ status = torture_smb2_testfile_access(tree, FNAME, &h, >+ SEC_FILE_READ_ATTRIBUTE); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ ZERO_STRUCT(rd); >+ rd.in.file.handle = h; >+ rd.in.length = 5; >+ rd.in.offset = 0; >+ status = smb2_read(tree, tree, &rd); >+ CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED); >+ >+ status = smb2_util_close(tree, h); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+done: >+ talloc_free(tmp_ctx); >+ return ret; >+} > > /* > basic testing of SMB2 read >@@ -237,6 +310,7 @@ struct torture_suite *torture_smb2_read_init(void) > torture_suite_add_1smb2_test(suite, "eof", test_read_eof); > torture_suite_add_1smb2_test(suite, "position", test_read_position); > torture_suite_add_1smb2_test(suite, "dir", test_read_dir); >+ torture_suite_add_1smb2_test(suite, "access", test_read_access); > > suite->description = talloc_strdup(suite, "SMB2-READ tests"); > >-- >2.5.5 > > >From 6ef3f243089f2e07fb128ba71fc38ae0754877d6 Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Sat, 13 Aug 2016 21:23:34 +0300 >Subject: [PATCH 4/8] seltest: implicit FILE_READ_DATA non-reporting > >This test (passes against Windows Server 2012R2) shows >that the implicit FILE_READ_DATA that is added whenever >FILE_EXECUTE is granted, is not reported back when querying >the handle. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> >(cherry picked from commit 7dc9f582066d500bf57000891560610e8d2e208c) >--- > source4/torture/smb2/getinfo.c | 45 ++++++++++++++++++++++++++++++++++++++++++ > source4/torture/smb2/util.c | 27 +++++++++++++++++++++++++ > 2 files changed, 72 insertions(+) > >diff --git a/source4/torture/smb2/getinfo.c b/source4/torture/smb2/getinfo.c >index 4bf4100..82eda75 100644 >--- a/source4/torture/smb2/getinfo.c >+++ b/source4/torture/smb2/getinfo.c >@@ -126,6 +126,49 @@ static bool torture_smb2_fileinfo(struct torture_context *tctx, struct smb2_tree > return true; > } > >+/* >+ test granted access when desired access includes >+ FILE_EXECUTE and does not include FILE_READ_DATA >+*/ >+static bool torture_smb2_fileinfo_grant_read(struct torture_context *tctx) >+{ >+ struct smb2_tree *tree; >+ bool ret; >+ struct smb2_handle hfile, hdir; >+ NTSTATUS status; >+ uint32_t file_granted_access, dir_granted_access; >+ >+ ret = torture_smb2_connection(tctx, &tree); >+ torture_assert(tctx, ret, "connection failed"); >+ >+ status = torture_smb2_testfile_access( >+ tree, FNAME, &hfile, SEC_FILE_EXECUTE | SEC_FILE_READ_ATTRIBUTE); >+ torture_assert_ntstatus_ok(tctx, status, >+ "Unable to create test file " FNAME "\n"); >+ status = >+ torture_smb2_get_allinfo_access(tree, hfile, &file_granted_access); >+ torture_assert_ntstatus_ok(tctx, status, >+ "Unable to query test file access "); >+ torture_assert_int_equal(tctx, file_granted_access, >+ SEC_FILE_EXECUTE | SEC_FILE_READ_ATTRIBUTE, >+ "granted file access "); >+ smb2_util_close(tree, hfile); >+ >+ status = torture_smb2_testdir_access( >+ tree, DNAME, &hdir, SEC_FILE_EXECUTE | SEC_FILE_READ_ATTRIBUTE); >+ torture_assert_ntstatus_ok(tctx, status, >+ "Unable to create test dir " DNAME "\n"); >+ status = >+ torture_smb2_get_allinfo_access(tree, hdir, &dir_granted_access); >+ torture_assert_ntstatus_ok(tctx, status, >+ "Unable to query test dir access "); >+ torture_assert_int_equal(tctx, dir_granted_access, >+ SEC_FILE_EXECUTE | SEC_FILE_READ_ATTRIBUTE, >+ "granted dir access "); >+ smb2_util_close(tree, hdir); >+ >+ return true; >+} > > /* > test fsinfo levels >@@ -444,5 +487,7 @@ struct torture_suite *torture_smb2_getinfo_init(void) > torture_smb2_qfile_buffercheck); > torture_suite_add_simple_test(suite, "qsec_buffercheck", > torture_smb2_qsec_buffercheck); >+ torture_suite_add_simple_test(suite, "granted", >+ torture_smb2_fileinfo_grant_read); > return suite; > } >diff --git a/source4/torture/smb2/util.c b/source4/torture/smb2/util.c >index c9d47ae..d0fc695 100644 >--- a/source4/torture/smb2/util.c >+++ b/source4/torture/smb2/util.c >@@ -261,6 +261,33 @@ void torture_smb2_all_info(struct smb2_tree *tree, struct smb2_handle handle) > talloc_free(tmp_ctx); > } > >+/* >+ get granted access of a file handle >+*/ >+NTSTATUS torture_smb2_get_allinfo_access(struct smb2_tree *tree, >+ struct smb2_handle handle, >+ uint32_t *granted_access) >+{ >+ NTSTATUS status; >+ TALLOC_CTX *tmp_ctx = talloc_new(tree); >+ union smb_fileinfo io; >+ >+ io.generic.level = RAW_FILEINFO_SMB2_ALL_INFORMATION; >+ io.generic.in.file.handle = handle; >+ >+ status = smb2_getinfo_file(tree, tmp_ctx, &io); >+ if (!NT_STATUS_IS_OK(status)) { >+ DEBUG(0, ("getinfo failed - %s\n", nt_errstr(status))); >+ goto out; >+ } >+ >+ *granted_access = io.all_info2.out.access_mask; >+ >+out: >+ talloc_free(tmp_ctx); >+ return status; >+} >+ > /** > * open a smb2 tree connect > */ >-- >2.5.5 > > >From 993d4d951a24a0dfe86056fbd8fe240bf0ddc2fb Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Mon, 15 Aug 2016 23:39:50 +0300 >Subject: [PATCH 5/8] seltest: allow opening files with arbitrary rights in > smb2.ioctl tests > >Separate file creation (which requires write access) from the >opening of the file for the test (which might be without write >access). > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> >(cherry picked from commit 6ce0304eda4b464972defcecd591fab03428bd03) >--- > source4/torture/smb2/ioctl.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > >diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c >index 8e7f69a..0aadca2 100644 >--- a/source4/torture/smb2/ioctl.c >+++ b/source4/torture/smb2/ioctl.c >@@ -273,20 +273,36 @@ static bool test_setup_create_fill(struct torture_context *torture, > uint32_t file_attributes) > { > bool ok; >+ uint32_t initial_access = desired_access; >+ >+ if (size > 0) { >+ initial_access |= SEC_FILE_APPEND_DATA; >+ } > > smb2_util_unlink(tree, fname); > > ok = test_setup_open(torture, tree, mem_ctx, > fname, > fh, >- desired_access, >+ initial_access, > file_attributes); >- torture_assert(torture, ok, "file open"); >+ torture_assert(torture, ok, "file create"); > > if (size > 0) { > ok = write_pattern(torture, tree, mem_ctx, *fh, 0, size, 0); > torture_assert(torture, ok, "write pattern"); > } >+ >+ if (initial_access != desired_access) { >+ smb2_util_close(tree, *fh); >+ ok = test_setup_open(torture, tree, mem_ctx, >+ fname, >+ fh, >+ desired_access, >+ file_attributes); >+ torture_assert(torture, ok, "file open"); >+ } >+ > return true; > } > >-- >2.5.5 > > >From a34f6193553fb926ee6843940d20f687fccc1677 Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Thu, 4 Aug 2016 13:12:58 +0300 >Subject: [PATCH 6/8] s4-smbtorture: pin copychunk exec right behavior > >Add tests that show copychunk behavior when the >source and dest handles have execute right instead >of read-data right. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> >(cherry picked from commit 5bf11f6f5b4dab4cba4b00674bcb76138fb55974) >--- > selftest/knownfail | 4 ++ > source4/torture/smb2/ioctl.c | 96 ++++++++++++++++++++++++++++++++------------ > 2 files changed, 75 insertions(+), 25 deletions(-) > >diff --git a/selftest/knownfail b/selftest/knownfail >index 892a5da..41cad44 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -289,3 +289,7 @@ > #new read tests fail > ^samba4.smb2.read.access > ^samba3.smb2.read.access >+#new copychunk tests fail >+^samba4.smb2.ioctl.copy_chunk_bad_access >+^samba3.smb2.ioctl.copy_chunk_bad_access >+^samba3.smb2.ioctl fs_specific.copy_chunk_bad_access >diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c >index 0aadca2..0aa3714 100644 >--- a/source4/torture/smb2/ioctl.c >+++ b/source4/torture/smb2/ioctl.c >@@ -1255,16 +1255,66 @@ static bool test_ioctl_copy_chunk_bad_access(struct torture_context *torture, > struct srv_copychunk_copy cc_copy; > enum ndr_err_code ndr_ret; > bool ok; >+ /* read permission on src */ >+ ok = test_setup_copy_chunk(torture, tree, tmp_ctx, 1, /* 1 chunk */ >+ &src_h, 4096, /* fill 4096 byte src file */ >+ SEC_FILE_READ_DATA | SEC_FILE_READ_ATTRIBUTE, >+ &dest_h, 0, /* 0 byte dest file */ >+ SEC_RIGHTS_FILE_ALL, &cc_copy, &ioctl); >+ if (!ok) { >+ torture_fail(torture, "setup copy chunk error"); >+ } > >- /* no read permission on src */ >- ok = test_setup_copy_chunk(torture, tree, tmp_ctx, >- 1, /* 1 chunk */ >+ cc_copy.chunks[0].source_off = 0; >+ cc_copy.chunks[0].target_off = 0; >+ cc_copy.chunks[0].length = 4096; >+ >+ ndr_ret = ndr_push_struct_blob( >+ &ioctl.smb2.in.out, tmp_ctx, &cc_copy, >+ (ndr_push_flags_fn_t)ndr_push_srv_copychunk_copy); >+ torture_assert_ndr_success(torture, ndr_ret, >+ "ndr_push_srv_copychunk_copy"); >+ >+ status = smb2_ioctl(tree, tmp_ctx, &ioctl.smb2); >+ torture_assert_ntstatus_equal(torture, status, NT_STATUS_OK, >+ "FSCTL_SRV_COPYCHUNK"); >+ >+ smb2_util_close(tree, src_h); >+ smb2_util_close(tree, dest_h); >+ >+ /* execute permission on src */ >+ ok = test_setup_copy_chunk(torture, tree, tmp_ctx, 1, /* 1 chunk */ > &src_h, 4096, /* fill 4096 byte src file */ >- SEC_RIGHTS_FILE_WRITE, >- &dest_h, 0, /* 0 byte dest file */ >- SEC_RIGHTS_FILE_ALL, >- &cc_copy, >- &ioctl); >+ SEC_FILE_EXECUTE | SEC_FILE_READ_ATTRIBUTE, >+ &dest_h, 0, /* 0 byte dest file */ >+ SEC_RIGHTS_FILE_ALL, &cc_copy, &ioctl); >+ if (!ok) { >+ torture_fail(torture, "setup copy chunk error"); >+ } >+ >+ cc_copy.chunks[0].source_off = 0; >+ cc_copy.chunks[0].target_off = 0; >+ cc_copy.chunks[0].length = 4096; >+ >+ ndr_ret = ndr_push_struct_blob( >+ &ioctl.smb2.in.out, tmp_ctx, &cc_copy, >+ (ndr_push_flags_fn_t)ndr_push_srv_copychunk_copy); >+ torture_assert_ndr_success(torture, ndr_ret, >+ "ndr_push_srv_copychunk_copy"); >+ >+ status = smb2_ioctl(tree, tmp_ctx, &ioctl.smb2); >+ torture_assert_ntstatus_equal(torture, status, NT_STATUS_OK, >+ "FSCTL_SRV_COPYCHUNK"); >+ >+ smb2_util_close(tree, src_h); >+ smb2_util_close(tree, dest_h); >+ >+ /* neither read nor execute permission on src */ >+ ok = test_setup_copy_chunk(torture, tree, tmp_ctx, 1, /* 1 chunk */ >+ &src_h, 4096, /* fill 4096 byte src file */ >+ SEC_FILE_READ_ATTRIBUTE, &dest_h, >+ 0, /* 0 byte dest file */ >+ SEC_RIGHTS_FILE_ALL, &cc_copy, &ioctl); > if (!ok) { > torture_fail(torture, "setup copy chunk error"); > } >@@ -1288,15 +1338,14 @@ static bool test_ioctl_copy_chunk_bad_access(struct torture_context *torture, > smb2_util_close(tree, dest_h); > > /* no write permission on dest */ >- ok = test_setup_copy_chunk(torture, tree, tmp_ctx, >- 1, /* 1 chunk */ >- &src_h, 4096, /* fill 4096 byte src file */ >- SEC_RIGHTS_FILE_ALL, >- &dest_h, 0, /* 0 byte dest file */ >- (SEC_RIGHTS_FILE_READ >- | SEC_RIGHTS_FILE_EXECUTE), >- &cc_copy, >- &ioctl); >+ ok = test_setup_copy_chunk( >+ torture, tree, tmp_ctx, 1, /* 1 chunk */ >+ &src_h, 4096, /* fill 4096 byte src file */ >+ SEC_FILE_READ_DATA | SEC_FILE_READ_ATTRIBUTE, &dest_h, >+ 0, /* 0 byte dest file */ >+ (SEC_RIGHTS_FILE_ALL & >+ ~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA)), >+ &cc_copy, &ioctl); > if (!ok) { > torture_fail(torture, "setup copy chunk error"); > } >@@ -1320,15 +1369,12 @@ static bool test_ioctl_copy_chunk_bad_access(struct torture_context *torture, > smb2_util_close(tree, dest_h); > > /* no read permission on dest */ >- ok = test_setup_copy_chunk(torture, tree, tmp_ctx, >- 1, /* 1 chunk */ >+ ok = test_setup_copy_chunk(torture, tree, tmp_ctx, 1, /* 1 chunk */ > &src_h, 4096, /* fill 4096 byte src file */ >- SEC_RIGHTS_FILE_ALL, >- &dest_h, 0, /* 0 byte dest file */ >- (SEC_RIGHTS_FILE_WRITE >- | SEC_RIGHTS_FILE_EXECUTE), >- &cc_copy, >- &ioctl); >+ SEC_FILE_READ_DATA | SEC_FILE_READ_ATTRIBUTE, >+ &dest_h, 0, /* 0 byte dest file */ >+ (SEC_RIGHTS_FILE_ALL & ~SEC_FILE_READ_DATA), >+ &cc_copy, &ioctl); > if (!ok) { > torture_fail(torture, "setup copy chunk error"); > } >-- >2.5.5 > > >From 52d2922436034f9b404a36449ffc06b86b71b0d0 Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Sat, 13 Aug 2016 00:19:33 +0300 >Subject: [PATCH 7/8] smbd: look only at handle readability for COPYCHUNK dest > >This commits sets the stage for a change of behavior >in a later commit. > >When checking FILE_READ_DATA on the COPYCHUNK dest handle, >only check the handle readability and not the extra right >that may have been added due to the FILE_EXECUTE right. > >The check for FILE_READ_DATA always seemed strange for the >dest handle, which is not read. It turns out that in Windows, >this check is not done at the SMB layer, but at a lower layer >that processes the IOCTL request - the IOCTL code has bits >that specify what type of access check needs to be done. > >Therefore, this lower layer is unaware of the SMB layer's >practice of granting READ access based on the FILE_EXECUTE >right, and it only checks the handle's readability. > >This subtle difference has observable behavior - the >COPYCHUNK source handle can have FILE_EXECUTE right instead >of FILE_READ_DATA, but the dest handle cannot. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> > >Autobuild-User(master): David Disseldorp <ddiss@samba.org> >Autobuild-Date(master): Tue Aug 16 15:21:03 CEST 2016 on sn-devel-144 > >(cherry picked from commit 3e42b69d5e1216b6af570a09d58040d281bbbf17) >--- > source3/include/smb_macros.h | 8 ++++++++ > source3/smbd/smb2_ioctl_network_fs.c | 4 ++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > >diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h >index 42a9756..f8656c7 100644 >--- a/source3/include/smb_macros.h >+++ b/source3/include/smb_macros.h >@@ -56,6 +56,14 @@ > ((req->flags2 & FLAGS2_READ_PERMIT_EXECUTE) && \ > (fsp->access_mask & FILE_EXECUTE)))) > >+/* An IOCTL readability check (validating read access >+ * when the IOCTL code requires it) >+ * http://social.technet.microsoft.com/wiki/contents/articles/24653.decoding-io-control-codes-ioctl-fsctl-and-deviceiocodes-with-table-of-known-values.aspx >+ * ). On Windows servers, this is done by the IO manager, which is unaware of >+ * the "if execute is granted then also grant read" arrangement. >+ */ >+#define CHECK_READ_IOCTL(fsp, req) (((fsp)->fh->fd != -1) && ((fsp)->can_read)) >+ > #define CHECK_WRITE(fsp) ((fsp)->can_write && ((fsp)->fh->fd != -1)) > > #define ERROR_WAS_LOCK_DENIED(status) (NT_STATUS_EQUAL((status), NT_STATUS_LOCK_NOT_GRANTED) || \ >diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c >index d8590de..c2b889b 100644 >--- a/source3/smbd/smb2_ioctl_network_fs.c >+++ b/source3/smbd/smb2_ioctl_network_fs.c >@@ -117,8 +117,8 @@ static NTSTATUS copychunk_check_handles(uint32_t ctl_code, > * - The Open.GrantedAccess of the destination file does not include > * FILE_READ_DATA, and the CtlCode is FSCTL_SRV_COPYCHUNK. > */ >- if ((ctl_code == FSCTL_SRV_COPYCHUNK) >- && !CHECK_READ(dst_fsp, smb1req)) { >+ if ((ctl_code == FSCTL_SRV_COPYCHUNK) && >+ !CHECK_READ_IOCTL(dst_fsp, smb1req)) { > DEBUG(5, ("copy chunk no read on dest handle (%s).\n", > smb_fname_str_dbg(dst_fsp->fsp_name) )); > return NT_STATUS_ACCESS_DENIED; >-- >2.5.5 > > >From 352af47f78f1d156abd641ca7a975a739a066aa6 Mon Sep 17 00:00:00 2001 >From: Uri Simchoni <uri@samba.org> >Date: Thu, 4 Aug 2016 14:59:23 +0300 >Subject: [PATCH 8/8] smbd: allow reading files based on FILE_EXECUTE access > right > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12149 > >Signed-off-by: Uri Simchoni <uri@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> > >Autobuild-User(master): David Disseldorp <ddiss@samba.org> >Autobuild-Date(master): Thu Aug 18 18:58:22 CEST 2016 on sn-devel-144 > >(cherry picked from commit a6073e6130d39dac58f1e6ea9f41ec4ab34c3e29) >--- > selftest/knownfail | 7 ++----- > source3/smbd/smb2_glue.c | 16 ++++++++++++++++ > 2 files changed, 18 insertions(+), 5 deletions(-) > >diff --git a/selftest/knownfail b/selftest/knownfail >index 41cad44..ffcaf06 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -286,10 +286,7 @@ > ^samba4.krb5.kdc.*as-req-aes.*fl2000dc > # nt4_member and ad_member don't support ntlmv1 > ^samba3.blackbox.smbclient_auth.plain.*_member.*option=clientntlmv2auth=no.member.creds.*as.user >-#new read tests fail >+#nt-vfs server blocks read with execute access > ^samba4.smb2.read.access >-^samba3.smb2.read.access >-#new copychunk tests fail >+#ntvfs server blocks copychunk with execute access on read handle > ^samba4.smb2.ioctl.copy_chunk_bad_access >-^samba3.smb2.ioctl.copy_chunk_bad_access >-^samba3.smb2.ioctl fs_specific.copy_chunk_bad_access >diff --git a/source3/smbd/smb2_glue.c b/source3/smbd/smb2_glue.c >index b41775d..0bb34be 100644 >--- a/source3/smbd/smb2_glue.c >+++ b/source3/smbd/smb2_glue.c >@@ -48,6 +48,22 @@ struct smb_request *smbd_smb2_fake_smb_request(struct smbd_smb2_request *req) > FLAGS2_32_BIT_ERROR_CODES | > FLAGS2_LONG_PATH_COMPONENTS | > FLAGS2_IS_LONG_NAME; >+ >+ /* This is not documented in revision 49 of [MS-SMB2] but should be >+ * added in a later revision (and torture test smb2.read.access >+ * as well as smb2.ioctl_copy_chunk_bad_access against >+ * Server 2012R2 confirms this) >+ * >+ * If FILE_EXECUTE is granted to a handle then the SMB2 server >+ * acts as if FILE_READ_DATA has also been granted. We must still >+ * keep the original granted mask, because with ioctl requests, >+ * access checks are made on the file handle, "below" the SMB2 >+ * server, and the object store below the SMB layer is not aware >+ * of this arrangement (see smb2.ioctl.copy_chunk_bad_access >+ * torture test). >+ */ >+ smbreq->flags2 |= FLAGS2_READ_PERMIT_EXECUTE; >+ > if (IVAL(inhdr, SMB2_HDR_FLAGS) & SMB2_HDR_FLAG_DFS) { > smbreq->flags2 |= FLAGS2_DFS_PATHNAMES; > } >-- >2.5.5 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=12381">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
ddiss
:
review+
Actions:
View
Attachments on
bug 12149
: 12381 |
12382
|
12383